-
Notifications
You must be signed in to change notification settings - Fork 328
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
adding captions to left sidebar #135
Conversation
Cool, fully agree this is something we want to support (many docs on rtd eg also use this sphinx feature) Can you add it somewhere to the demo docs, so we can see how it looks?
I don't fully get the problem. What do you mean with "sub-page" here? Toctrees that are not defined on the top-level index.rst?
Also don't fully understand this ;) |
{% for nav_item in main_nav_item.children %} | ||
{% if nav_item.children %} | ||
{% if nav_item["type"] == "caption" %} | ||
<li class="nav-caption">{{ nav_item.text }}</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually want to start a new list after each caption? That's eg how rtd does it (eg http://xarray.pydata.org/en/stable/)
OK, I think the latest push gets us close to what we want. It subclasses the TocTree class to add a method that gets us the toctree for a sub-page of the site, rather than basing it off of the However, when I build the site I am getting this error and for the life of me I cannot figure it out:
It seems to be happening simply because we have called the |
da274c7
to
f7cf135
Compare
gah it gets even weirder, I found that I can replicate this error if I just put the following in my code:
I think that it has something to do with the pickling that happens as well as |
Phewwww that took wayyy longer than I expected. The problem from before was happening when Sphinx tried to build either the This version has this feature working...it is surprisingly difficult to get a TocTree with the second-level pages that have captions. What we have to do is:
Also adds a simple CSS rule @jorisvandenbossche I agree that ultimately it would be better to use a new demo the sidebar here: https://217-130237506-gh.circle-artifacts.com/0/html/demo/index.html note: I just realized this won't currently work for pages that are below the second level...need to figure that out. |
toc_items = [item for child in toctree.children for item in child | ||
if isinstance(item, docutils.nodes.list_item)] | ||
toc_items = [] | ||
for child in toctree.children: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lot of this extra code is because we were making really strong assumptions about the structure of the toctree, so we need to handle some other edge cases
github_user = context['github_user'] | ||
github_repo = context['github_repo'] | ||
github_version = context['github_version'] | ||
raise ExtensionError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I may have accidentally black-ified this file in the process 😬 I am not sure how to only revert the black changes, so hoping it's OK
another round of trying to figure this out, and it is still more difficult than I imagined. I'm going to stop working on this for a while. Anybody else is welcome to pick it up...I think it is probably close, but I need to focus on other things. |
@choldgraf thanks for looking into it anyway! Can you give a small pointer / summary of what is not yet working (or is a full sphinx build simply not yet finishing?) |
Basically, the current approach tries the following:
I think that the general logic is correct in here, and basically there is just a bunch of making sure that edge-cases are dealt with (I ran into a ton of confusion because apparently there are two special pages created called |
Ah I think I might have found a much easier way to do this. It turns out Sphinx has a list of all the toctrees on all pages, which we might be able to use to do this more easily, see here: This would let us skip over the monkey-patching etc |
@choldgraf I am planning to tomorrow (hopefully, finally) take a look at this again. But a question: I suppose you are already happily using this feature in sphinx-book-theme? Any things you discovered there in the meantime ? |
What I've ended up doing is parsing the nav object that is created by this theme with another that generates a full HTML list for the sidebar: this is similar to what I had done in #110 (though that PR is probably stale enough now that it should be done from scratch) I grab the "master toc" and then re-work the URLs so that they are relative to the current page instead of the master doc page: here I grab the list of toctrees on the master page, and then store the first URL in each TOCtree for use later: and finally here I add the caption if we're about to list the sidebar entry for an item that is the first in a toctree w/ a caption: The challenge with this is how to generalize it so that it works on both the first level page (what the book theme uses) and in the second level pages (which pydata sphinx theme needs) |
OK, I think I start to understand the situation / problem .. ;) (saving the toctrees as xml files to look at helps a bit ..) Now, an initial question then comes up: how do we want to put this caption information in the navigation list/dict objects? Currently, for a certain level, we have a list of dicts, like:
Assume we have captions on this level? How do we want the navigation object to look like? I am trying to test what this PR does, but it doesn't seem to work (anymore? after my rebasing, maybe something went wrong here, or the sphinx version ..) on the demo docs, at least I don't see any caption in the output. But from looking at the code, I assume this PR tries to make something like this:
Is that right? I am trying to think through some situations to see if this is the best structure. In principle, an alternative could be to actually add an additional nesting level. Looking at how mkdocs does it (which was an inspiration for those lists of dicts), they seem to have a "section" concept which is a part of the navigation that itself has no page linked to it (https://www.mkdocs.org/user-guide/custom-themes/#navigation-objects, example how it is specififed in the configuration: https://www.mkdocs.org/user-guide/writing-your-docs/#configure-pages-and-navigation). So that could be something like:
On the one hand: this might make it easier to loop through if you want to make separate html lists under each caption. On the other hand: this might give rise to corner cases, like: what to do if you have multiple |
Implementation wise random thought: it seems that all toctrees are available in the (that's a larger refactor, though) |
@choldgraf from the point of view of sphinx-book-theme's customizations, do you have a preference for the flat vs nested layout for the dicts/lists? |
Note that I think this will be superseded by #219 , I am happy to re-implement this once that is merged if we wish. I think it will be simpler with that implementation |
Done in #346 |
This is an attempt at making it possible to add the caption of a TocTree in the sidebar. The idea is that then people can break up their sidebars into natural sections like so:
I think this PR gets quite close, but for some reason the
TocTree(app.env).get_toctree_for
function isn't returning the captions of any sub-page toctrees. Any idea why that is? Am I missing a flag somewhere?In our case, we already do define captions for sub-page toctrees, they just aren't being used at all in the rendered docs
Ah - I think the issue is that the pandas
get_toctree_for
page always gets the toctree for the master doc (e.g., index), and then renders the toctree for the specific sub-page from that:https://github.com/sphinx-doc/sphinx/blob/af62fa61e6cbd88d0798963211e73e5ba0d55e6d/sphinx/environment/adapters/toctree.py#L320
This means that only captions for the master doc toctree will make it into the output.
So I think the ways around this would either be:
get_toctree_for
function, and instead write a similar function that basically does the same thing, but only for the current page, so you get the current page's captionsI think option 2 sounds more reasonable, since it's also basically what we are doing by default.
Update
Demo the sidebar here:
https://217-130237506-gh.circle-artifacts.com/0/html/demo/index.html