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

Generate a page's navigation links one time and re-use in the navbar / sidebar #859

Closed
choldgraf opened this issue Aug 6, 2022 · 1 comment · Fixed by #878
Closed

Generate a page's navigation links one time and re-use in the navbar / sidebar #859

choldgraf opened this issue Aug 6, 2022 · 1 comment · Fixed by #878
Labels
kind: enhancement New feature or request

Comments

@choldgraf
Copy link
Collaborator

Context

Right now we have have this function to generate either the navbar navigation or the sidebar navigation:

def generate_nav_html(
kind, startdepth=None, show_nav_level=1, n_links_before_dropdown=5, **kwargs
):
"""
Return the navigation link structure in HTML. Arguments are passed
to Sphinx "toctree" function (context["toctree"] below).
We use beautifulsoup to add the right CSS classes / structure for bootstrap.
See https://www.sphinx-doc.org/en/master/templating.html#toctree.

And we then re-use it in a few places to do the actual HTML generation:

It seems like this function in particular is time-consuming, and I wonder if this is true regardless of whether it's the sidebar/navbar variety. E.g.:

Idea

If indeed this function is what is causing all of the slowness, how about instead of calling it multiple times, we generate the full HTML structure of the page's toctree one time, we store it in the page context as a beautifulsoup object, and then we re-use that object in building the actual sidebar / navbar HTML.

I think this might be a way to reduce redundant computation and maybe speed things up, it also feels a bit simpler to me.

@choldgraf choldgraf added the kind: enhancement New feature or request label Aug 6, 2022
@choldgraf
Copy link
Collaborator Author

I think that this issue is potentially a simpler way to resolve this problem in the long term:

I realized that the proposal here will actually be more complex than I originally thought, because the caption elements aren't generated at the 2nd-and-below level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant