Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add no_title argument to toc #571

Merged
merged 6 commits into from
May 14, 2020
Merged

Conversation

amueller
Copy link
Contributor

no tests yet, let me know if you think this is useful.
basically calling jb toc overwrote the titles with the file names every time and it seem tricky to control. I'd rather just use the titles I gave in the documents inside the toc as well.

@choldgraf
Copy link
Collaborator

Ah yep this is great! Could you add a test? :-). I believe test_toc is the module (sorry, at my phone now)

@amueller
Copy link
Contributor Author

cool will do later. Also looks like you're using black, right?

@choldgraf
Copy link
Collaborator

yep!

@amueller
Copy link
Contributor Author

hm so I overwrote the _toc.yml file in the test folder. Is this file used by other tests? I was a bit surprised that a file that's generated by test_toc is checked into the repo.
Should output the file in my test in a temp folder and then delete it?

@choldgraf
Copy link
Collaborator

oh good call, maybe it was accidentally checked in - do the tests pass if it is deleted before running the test suite? (if so, we can probably delete it and add to .gitignore)

@amueller
Copy link
Contributor Author

I gave up trying to run the tests locally. Seems pretty tricky. Do you run tests locally?

@choldgraf
Copy link
Collaborator

it's not tough if you don't run the PDF tests since those require latex etc...we should document that too!

@choldgraf
Copy link
Collaborator

Actually - now that I look at this and think about some of your comments, I wonder if we should make this PR's behavior the default behavior? AKA, by default titles are not included in the _toc.yml and a user can explicitly ask to include them

@amueller
Copy link
Contributor Author

I'm happy either way :)
If jb requires that there's titles in the files anyway, it seems fine to rely on those. I could also see that for debugging purposes having the file names would be nice.

Maybe having this be the default makes more sense, happy to change the PR. I usually go with backward compatible by default which is why I did it this way first ;)

@choldgraf
Copy link
Collaborator

I think I'd be +1 to change it and make the flag --add-titles or --titles-from-filenames. It seems like otherwise we're going to keep running into the confusion of "the title in my document is X, but the title in the book is Y" because the toc function is using the filenames to create titles.

@choldgraf
Copy link
Collaborator

this is looking good to me - lemme know when it's ready to go

@amueller
Copy link
Contributor Author

should be good, I think?

@choldgraf
Copy link
Collaborator

nice! Thanks so much @amueller :-)

@choldgraf choldgraf merged commit d7233a9 into jupyter-book:master May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants