jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
196 stars 29 forks source link

Adding links to the index (genindex) and module index to the navigation sidebar #132

Closed brechtm closed 2 years ago

brechtm commented 2 years ago

I'm currently using the RTD Sphinx theme, for which I've added templates to add links to these indices the the TOC as described in this StackOverflow answer. See the Indices and tables section in the sidebar at http://www.mos6581.org/rinohtype/0.5.4/ for the result.

I've been trying to do the same in sphinx-immaterial, but the templates do not seem to define blocks where these can easily be injected. My knowledge of Jinja is limited, but I think it is necessary to replace all of the site_nav block in layout.html, duplicating all of the block's contents and injecting the links. Here is my attempt so far:

_templates/layout.html ```jinja {% extends "!layout.html" %} {% block site_nav %} {% if nav %} {% if page and page.meta and page.meta.hide %} {% set hidden = "hidden" if "navigation" in page.meta.hide %} {% endif %} {% endif %} {% if not "toc.integrate" in features %} {% if page and page.meta and page.meta.hide %} {% set hidden = "hidden" if "toc" in page.meta.hide %} {% endif %} {% endif %} {% endblock %} ```

This produces a this suboptimal result:

image

I was hoping there is a better way to approach this.

Actually, I think a better fix would be for Sphinx to allow including these indices in a toctree. Or perhaps sphinx-immaterial could provide an option to insert links to the indices at the bottom of the navigation tree?

jbms commented 2 years ago

Yes, replacing the entire site_nav block wouldn't be very practical. It sounds like you ultimately just want them included in the toc like anything else? In that case, just fixing the upstream Sphinx issue would be the best solution. Possibly a monkey patch could be developed as well, but just fixing it upstream would be preferable.

In general the index seems redundant with the search functionality, but I suppose some may still prefer to have it.

2bndy5 commented 2 years ago

Or perhaps sphinx-immaterial could provide an option to insert links to the indices at the bottom of the navigation tree?

I'm guessing you want a customizable block that can be more easily overridden in the layout.html template. While that would be sufficient, it may cause merge conflicts with updates from mkdocs-material src. We try not to deviate too much from the mkdocs-material templates, and I don't see this customizable block as something that would be useful for mkdocs-material.

I understand your concern though, if upstream changes it's layout.html template, then you'd undoubtedly have to update your custom solution to match -> not very sustainable.

In general the index seems redundant with the search functionality

It's not redundant if you don't know what to type in the search. Rather, its beneficial to new readers (or those that don't fluently read the docs' language).

mhostetter commented 2 years ago

From this StackOverflow answer you can do this by adding genindex to a toctree, but then you specifically have to create a file called genindex.rst (even though Sphinx says you shouldn't do this).

index.rst

.. toctree::

   page1.rst
   page2.rst
   genindex

genindex.rst

Index
=====

I did this in my repo.

brechtm commented 2 years ago

Yes, replacing the entire site_nav block wouldn't be very practical. It sounds like you ultimately just want them included in the toc like anything else? In that case, just fixing the upstream Sphinx issue would be the best solution. Possibly a monkey patch could be developed as well, but just fixing it upstream would be preferable.

I'll look into this.

In general the index seems redundant with the search functionality, but I suppose some may still prefer to have it.

Please don't underestimate the usefulness of a handcrafted index! While the search function will yield all occurrences of a word, a good index will only reference the most useful uses.

brechtm commented 2 years ago

From this StackOverflow answer you can do this by adding genindex to a toctree, but then you specifically have to create a file called genindex.rst (even though Sphinx says you shouldn't do this).

Thanks for the suggestion. I did stumble upon that SO question, but I didn't want to go down that route since it is explicitly discouraged.

jbms commented 2 years ago

I think currently the search functionality does not make use of manual index directives (https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#directive-index) but it would probably make sense to add that, and give those terms higher weight.

brechtm commented 2 years ago

I was able to adjust Sphinx to accept genindex, modindex and search entries in toctree directives. I'll submit a PR.

I only want the Indices toctree in my HTML output, so I placed it inside an only directive. Unfortunately, sphinx-immateraterial crashes on it:

Handler <function _html_page_context at 0x104751090> for event 'html-page-context' threw an exception (exception: <class 'sphinx_immaterial.nav_adapt._TocVisitor'> visiting unknown node type: only)
2bndy5 commented 2 years ago

@brechtm Could post the result from a verbose build log (sphinx-build -v)? That would better show the traceback we need to see.

brechtm commented 2 years ago
Traceback (most recent call last):
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/events.py", line 94, in emit
    results.append(listener.handler(self.app, *args))
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx_immaterial/nav_adapt.py", line 720, in _html_page_context
    global_toc, local_toc, integrated_local_toc = _get_mkdocs_tocs(
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx_immaterial/nav_adapt.py", line 689, in _get_mkdocs_tocs
    local_toc = _get_mkdocs_toc(local_toc_node, builder)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx_immaterial/nav_adapt.py", line 327, in _get_mkdocs_toc
    toc_node.walk(visitor)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/docutils/nodes.py", line 191, in walk
    if child.walk(visitor):
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/docutils/nodes.py", line 183, in walk
    visitor.dispatch_visit(self)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/docutils/nodes.py", line 2021, in dispatch_visit
    return method(node)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx_immaterial/nav_adapt.py", line 312, in visit_list_item
    child.walk(child_visitor)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/docutils/nodes.py", line 191, in walk
    if child.walk(visitor):
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/docutils/nodes.py", line 183, in walk
    visitor.dispatch_visit(self)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/docutils/nodes.py", line 2021, in dispatch_visit
    return method(node)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/docutils/nodes.py", line 2044, in unknown_visit
    raise NotImplementedError(
NotImplementedError: <class 'sphinx_immaterial.nav_adapt._TocVisitor'> visiting unknown node type: only

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/cmd/build.py", line 276, in build_main
    app.build(args.force_all, filenames)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/application.py", line 329, in build
    self.builder.build_update()
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/builders/__init__.py", line 288, in build_update
    self.build(to_build,
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/builders/__init__.py", line 352, in build
    self.write(docnames, list(updated_docnames), method)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/builders/__init__.py", line 544, in write
    self._write_serial(sorted(docnames))
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/builders/__init__.py", line 554, in _write_serial
    self.write_doc(docname, doctree)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/builders/html/__init__.py", line 649, in write_doc
    self.handle_page(docname, ctx, event_arg=doctree)
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/builders/html/__init__.py", line 1051, in handle_page
    newtmpl = self.app.emit_firstresult('html-page-context', pagename,
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/application.py", line 457, in emit_firstresult
    return self.events.emit_firstresult(event, *args,
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/events.py", line 112, in emit_firstresult
    for result in self.emit(name, *args, allowed_exceptions=allowed_exceptions):
  File "/Users/brechtm/Code/rinohtype/.venv/lib/python3.10/site-packages/sphinx/events.py", line 102, in emit
    raise ExtensionError(__("Handler %r for event %r threw an exception") %
sphinx.errors.ExtensionError: Handler <function _html_page_context at 0x1078baa70> for event 'html-page-context' threw an exception (exception: <class 'sphinx_immaterial.nav_adapt._TocVisitor'> visiting unknown node type: only)

Extension error (sphinx_immaterial.nav_adapt):
Handler <function _html_page_context at 0x1078baa70> for event 'html-page-context' threw an exception (exception: <class 'sphinx_immaterial.nav_adapt._TocVisitor'> visiting unknown node type: only)

PS. PR submitted with Sphinx: sphinx-doc/sphinx#10673

brechtm commented 2 years ago

I just noticed that the HTML page titles for the general index and the module index say "None". sphinx-immaterial doesn't provide a search.html, so there's not issue there :-)

2bndy5 commented 2 years ago

I just noticed that the HTML page titles for the general index and the module index say "None"

It would seem that the generation of index needs some tweaking. I played around a bit with https://github.com/jbms/sphinx-immaterial/blob/8dd1888819e412537de6f182d1ad72a839387a90/sphinx_immaterial/nav_adapt.py#L714-L719 but the index.html doesn't have a ToC to extract the title from.

sphinx-immaterial doesn't provide a search.html, so there's not issue there :-)

yeah the utility of search.html is integrated into all pages, so it isn't generated for this theme.

toctree in my HTML output, so I placed it inside an only directive. Unfortunately, sphinx-immateraterial crashes on it:

I'm only guessing here, but I think the theme's custom toc visitor needs to compensate for the only directive. I was able to reproduce this with a toctree provided as content. The only directive works fine for page contents.

FWIW, we really only aim to support HTML builders right now. We have limited latex builder support, but it is flawed (particularly for references IIRC).

brechtm commented 2 years ago

I'm only guessing here, but I think the theme's custom toc visitor needs to compensate for the only directive. I was able to reproduce this with a toctree provided as content. The only directive works fine for page contents.

Starting to investigate, I added the following method to _TocVisitor:

    # import sphinx.addnodes
    def visit_only(self, node: sphinx.addnodes.only):
        raise docutils.nodes.SkipChildren

I expected this to simply drop the toctree contained within the only directive, but it doesn't. I don't know why, but it seems that is all that is needed to support the only directive. 🤷

(EDIT: visit_toctree is also a no-op)

FWIW, we really only aim to support HTML builders right now. We have limited latex builder support, but it is flawed (particularly for references IIRC).

At first, I didn't understand why you explicitly mention that a HTML theme would only support HTML builders, but then I noticed that sphinx-immaterial also brings some of the mkdocs features (document elements) over to Sphinx. Nice! I suppose that is what you are referring to?

I'm personally not using any of these features (yet), so it would be nice if I could use sphinx-immaterial for HTML output and rinohtype for PDF output.

2bndy5 commented 2 years ago

I noticed that sphinx-immaterial also brings some of the mkdocs features (document elements) over to Sphinx. Nice! I suppose that is what you are referring to?

If by "document elements" you mean things like customizable checkboxes, content tabs, mermaid diagrams, and etc, then yes, that's what I was referring to. At first I had RTD generating PDFs from this theme's docs using sphinx's latex builder, but this had to stop because the conversion from latex output to PDF was failing (again I think it was about cross references).


I think it would be prudent to keep testing your _TocVisitor.visit_only() solution, but that is exactly what I was thinking of doing. I suspect the node is already walk()ed before it gets passed to visit_only().


I could use sphinx-immaterial for HTML output and rinohtype for PDF output.

I'm not entirely familiar with your rinohtype lib - it seems enticing. In the past, I've used rst2pdf (which includes a custom sphinx builder), but they haven't been keeping up with Sphinx updates (their dev cycle is very slow - like bi-annual releases slow).

brechtm commented 2 years ago

Update: I've managed to migrate from the RTD theme to Sphinx-Immaterial by using my Sphinx PR branch (sphinx-doc/sphinx#10673) and some Sphinx-Immaterial monkey-patching and CSS tweaking. I love the look and absolutely adore the live search results. Thanks!

You can see the result here: http://www.mos6581.org/rinohtype/master/

2bndy5 commented 2 years ago

something's up with your version selector placement. image

dev console points to

/* tweaks.css line 11 */
.md-version {
  margin-left: -5.2rem; /* overlaps hamburger menu button */
}

I think you need

@media screen and (min-width: 76.25em)
.md-version {
  margin-left: -5.2rem; /* only used for sufficiently wide viewports */
}

Otherwise well done!

brechtm commented 2 years ago

@2bndy5 Thanks for letting me know! I didn't check the narrow version. Fixed now.

jbms commented 2 years ago

Is the only monkey patch still needed after https://github.com/jbms/sphinx-immaterial/pull/139?

brechtm commented 2 years ago

Is the only monkey patch still needed after #139?

Nope, it isn't. I didn't realize this fix was already available in 0.8.1.