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

Rewrite sidebar TOC generation to be recursive #115

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sunshowers
Copy link

@sunshowers sunshowers commented Aug 18, 2024

Hi there, and thanks for maintaining this plugin!

Currently, sidebar TOCs only assume one level of nesting, and don't obey excludes. This leads to both missing items and unexpected items that should have been excluded.

  • Rewrite it to be recursive, such that all subsections are generated and appended as children.
  • Also obey excludes and don't add anchors for them.

There was also a fixture for nested sections, but no corresponding test. Used that fixture to test this out. (I've also verified that it works against nextest's site.)

The result goes from:

Screenshot from 2024-08-17 22-30-37

to:

Screenshot from 2024-08-17 22-29-43

I believe this should fix #111 and #83.

Currently, sidebar TOCs only assume one level of nesting, and don't obey
excludes. This leads to both missing items and unexpected items that should
have been excluded.

* Rewrite it to be recursive, such that all subsections are generated and
  appended as children.
* Also obey excludes and don't add anchors for them.

There was also a fixture for nested sections, but no corresponding test. Used
that fixture to test this out. (I've also verified that it works against
nextest's site.)
@brenard
Copy link

brenard commented Aug 18, 2024

I tested it and it seem to work pretty great for me!

@sunshowers
Copy link
Author

(I personally would prefer a different numbering scheme btw, but that is a separate consideration)

@timvink
Copy link
Owner

timvink commented Aug 19, 2024

Thanks for taking the effort to tackle this tricky topic ! Really appreciate it.

Couple of comments:

  • There's a conflict to address (another PR merged that solved the excluding page from toc side bar issue)
  • I also don't like the numbering scheme in this new setup. We should change it. Initially, each page is a chapter and would get a number. And sections (these are groups of pages in the navigation) would get a roman numbering. Shall we go implement a 'classic' numbering scheme like 1. section name 1.1 first chapter ?

Would you mind making the updates?

@timvink
Copy link
Owner

timvink commented Sep 12, 2024

I fixed the merge conflict. @sunshowers could you have a look at the other comment?

@sunshowers
Copy link
Author

Thanks, sorry I completely missed this because I fell sick in between :(

I tried doing some small updates and bringing this up-to-date with master, but ran into merge conflicts that I don't have time to resolve. I'd still really love to have this but I don't quite have time to work on it at the moment, work is too busy. I like the 1.1 scheme.

Please feel free to make changes to this and land this as you see fit. I might have time later this year, but please don't count on it.

@sunshowers
Copy link
Author

sunshowers commented Oct 18, 2024

If you can handle the CSS bits (I couldn't really follow them) I can probably take care of the Python. Does that sound okay?

@timvink
Copy link
Owner

timvink commented Oct 22, 2024

Sure

@timvink
Copy link
Owner

timvink commented Oct 22, 2024

This should close #123 also

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.

print_page.html and pdf are missing multi-level toc items that are included in the mkdocs html site
3 participants