ENH: Add parameters navigation_startdepth and toc_caption_maxdepth

[SimenZhor]

Hi,

As discussed in Issue #1181 it is quite straight forward to implement a new parameter that allows users to decide which level the sidebar TOC should start at. By introducing a new global theme-parameter, navigation_startdepth , that is passed to the generate_toctree_html function. I have made a separate branch here that showcases only this functionality.

Here is a screenshot showing all top-level TOCs collapsed (navigation_startdepth=0):

As I developed this on my own project, everything worked as expected. But when I tested it on the pydata-sphinx-theme docs I noticed that setting the startdepth to 0 would break the nice looking subcategories generated by the toctree :captions:, as these were only rendered for the first layer of toctrees (discussed in: #192 and #135)

Screenshots:

So, to avoid breaking this feature for navigation_startdepth=0 I've also introduced a parameter for specifying how deep the toctree captions should be rendered: toc_caption_maxdepth.

Here is a screenshot of how it looks when toc_caption_maxdepth is set to a high number:

I had to copy and edit yet another giant function from the Sphinx library into the monolithic __init__.py, so I've not yet written any documentation for these new parameters as I wanted a "go" from one of the maintainers before putting more time into this.

NOTE: In an effort to keep my changes to this Sphinx-function readable, I had to disable pre-commit checks, as it insisted on reformatting the entire __init__.py.

Closes #1181
Other relevant issues: #944 #221

[SimenZhor]

I have now added documentation for the new options.
However, I did discover that neither of them are compatible with the collapsible toc captions functionality (enabled with "show_nav_level": 0).

For now I've added notes about this in the documentation rather than fixing the issues with show_nav_level. I also discovered some other problems with that option that are present on main and should be looked into at the same time. I think improving show_nav_level should probably be its own PR to avoid overcomplicating this one.

[drammock]

@SimenZhor I just want to say thanks for working on this and sorry I haven't had a chance to review it yet, will try to get to it early next week.

[SimenZhor]

@drammock Awesome! And no problem :)

I think I have fixed most of the test-failures now by the way. So it would be nice if you could re-trigger the CI pipeline to see if I've missed something.

Locally I'm able to run nox -s test sucessfully and nox -s test-sphinx successfully for versions 5 and 6 (Sphinx 4 will also fail on main). The PR will still fail linting since pre-commit wants to reformat the entire __init__.py file. I've avoided doing that as it will make this PR quite hard to review...

It was a bit confusing to debug those test failures by the way. nox -s test-sphinx seems to use Sphinx 5.0.0 and 6.0.0, while nox -s test uses Sphinx 6.1.3 - which gave me a pass on Sphinx 6 from test-sphinx, while still failing nox -s test (due to changes between 6.0.0 and 6.1.3). All the while I had no problems running things locally with Sphinx 5.3.0 but got failures for Sphinx 5.0.0 😅 I'm not sure if it's necessary to improve that system or not, but I thought I'd share my experience regardless.

[drammock]

looks like all the CIs passed except lint (expected, since you disabled pre-commit on this PR). As a first pass, here are some questions/comments:

1. Is there a reason we want toc_caption_maxdepth to be limited to the first level? I.e., if a user is specifying :caption:s at deeper levels, why wouldn't we want to show them by default? Offhand I would lean toward always showing all captions regardless of depth (this is a change in behavior, but IMO a bugfix), and removing the new toc_caption_maxdepth config you added. If you agree with this, then the added docs section _toc-caption-levels should change accordingly.

2. I've only skimmed Make it possible to put the entire site toctree in the left sidebar and populate the header with custom links #944 and Show TOC from root index.html on left sidebar #221 but it looks like this PR would fully address them. If you agree can you add closes #944 closes #221 to the end of the PR description here? That will make GitHub automatically close them when this is merged. You should also add closes #1181.

3. A couple of edits to the docs page you edited, but that aren't strictly related to this PR (but since you're touching that file anyway, might as well fix them now):
   a. Am I correct that this line would be more accurate if it said this "To load your site with all nav levels collapsed behind their top-level captions, set the show_nav_level value to 0, like below:"
   b. On this line I tripped on the phrase "drop boxes" to describe the progressive disclosure of nav items. Can you change "drop boxes" to "caret buttons" or "chevron buttons"? Use whichever one you think is more widespread of a name for that shape.

[SimenZhor]

Thanks for the good feedback, @drammock

1.

Is there a reason we want toc_caption_maxdepth to be limited to the first level?

This is my first PR to this repo, so my main motivation behind this choice was to avoid breaking changes. I do agree with your observation that it does seem like a bugfix, so I wouldn't vote against removing the toc_caption_maxdepth option.

However, as noted earlier, my current implementation will introduce a quite nasty bug for "show_nav_level": 0, where all captions will be reorganized to the top level of the toctree, and some may even be dissociated with their content. So the PR needs quite a bit more work before making this the default behavior.

That being said, I do have a working proof-of-concept on this branch, but a side effect is that it introduces a new toctree-level for the captions themselves, which means that the behavior of navigation_depth changes. Even worse, this means that it's possible to end up with a caption/header that has no children (on the deepest toctree-level). I'm a bit stuck trying to sort that one out, unfortunately, so this may take me a while to finish...

2.

I'm not addressing the second part of #944 (header with only custom links). Do you know if that has been addressed elsewhere already? I think you are right that it closes #221, but that one is discussing many other, somewhat unrelated, design choices as well. I think this PR will address the original issue as it was described though, so maybe one of the stakeholders from the thread could take a closer look at the discussion - and open separate issues for anything that remains open? I'll add closes #1181 right away

3.

Good observations. I'll fix those.

[drammock]

my current implementation will introduce a quite nasty bug for "show_nav_level": 0, where all captions will be reorganized to the top level of the toctree

dang. I didn't catch that before.

I do have a working proof-of-concept on this branch, but a side effect is that it introduces a new toctree-level for the captions themselves, which means that the behavior of navigation_depth changes.

my instinct is that we shouldn't be introducing new TOC levels for captions. Maybe if we can do it in a way that is fully invisible to the end user... but it seems likely to be complicated to implement and thus prone to nasty bugs.

I'll try to find some time to play around with this, and see if I can help figure it out.

[SimenZhor]

my instinct is that we shouldn't be introducing new TOC levels for captions.

I totally agree. I've been trying to get rid of this side effect without luck so far. I don't remember all the details off the top of my head, but I believe the reason it happens is because I'm packing the list items (<li>) that belong to a captioned toctree inside a bullet_list (<ul>). And that this is interpreted by Sphinx somewhere as a new level, but I haven't identified where.

The system with <ul> for toc-items is already used in the public implementation of "show_nav_level": 0, by the way. But currently this tag is only applied to the first toc level that is processed (which is the main reason for all the toc-caption related changes I've had to make for this PR). What I tried was to apply this <ul> tag to sub-tocs as well, which works but has the mentioned side effect.

I'm not very proficient with HTML, but it feels like I'm quite close and that there is an obvious solution I'm not seeing (like swapping out <ul> with another tag that does not increase the toc depth). At the same time, I feel like I've tried everything without luck...

I'll try to find some time to play around with this, and see if I can help figure it out.

I would truly appreciate some help, or pointers on what to try :) I won't be able to look more at this until next week at the earliest.

[leycec]

Gah! This critical improvement never happened, merge conflicts invariably cropped up, and now everybody is tired. This saddens me. At least we always have sphinx-book-theme, which forcefully enables this functionality in the way that everybody expects and wants. Still... 😮‍💨

[12rambau]

Gah!

@leycec thank you for you for this very constructive and somewhat disrespectful comment. Note that if this functionality is critical to you, we will appreciate any contributions.

@drammock, @SimenZhor thanks for all the work you did so far and I hope we will eventully merge it. Don't loose hope our record is currently this one: #399 that we eventually merged in smaller pieces.