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

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

Closed
brechtm opened this issue Jul 15, 2022 · 18 comments
Closed
Labels
wontfix This will not be worked on

Comments

@brechtm
Copy link

brechtm commented Jul 15, 2022

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
{% extends "!layout.html" %}

{% block site_nav %}

  <!-- Main navigation -->
  {% if nav %}
    {% if page and page.meta and page.meta.hide %}
      {% set hidden = "hidden" if "navigation" in page.meta.hide %}
    {% endif %}
    <div
      class="md-sidebar md-sidebar--primary"
      data-md-component="sidebar"
      data-md-type="navigation"
      {{ hidden }}
    >
      <div class="md-sidebar__scrollwrap">
        <div class="md-sidebar__inner">
          {% include "partials/nav.html" %}

          <ul class="md-nav__list">
            <li class="md-nav__item md-nav__item--nested">
              <nav class="md-nav" aria-label="User Manual" data-md-level="1">
                <span class="md-nav__icon md-icon"></span>
                <span class="md-ellipsis">Indices and tables</span>
                <ul class="md-nav__list">
                  <li class="md-nav__item">
                    <a href="{{pathto('genindex.html', 1)}}" class="md-nav__link">
                      <span title="intro (document)" class="md-ellipsis">Index</span>
                    </a>
                  </li>
                  <li class="md-nav__item">
                    <a href="{{pathto('py-modindex.html', 1)}}" class="md-nav__link">
                      <span title="install (document)" class="md-ellipsis">Module Index</span>
                    </a>
                  </li>
                </ul>
              </nav>
          </ul>

        </div>
      </div>
    </div>
  {% endif %}

  <!-- Table of contents -->
  {% 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 %}
    <div
      class="md-sidebar md-sidebar--secondary"
      data-md-component="sidebar"
      data-md-type="toc"
      {{ hidden }}
    >
      <div class="md-sidebar__scrollwrap">
        <div class="md-sidebar__inner">
          {% include "partials/toc.html" %}
        </div>
      </div>
    </div>
  {% 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
Copy link
Owner

jbms commented Jul 15, 2022

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
Copy link
Collaborator

2bndy5 commented Jul 15, 2022

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
Copy link
Contributor

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.

@2bndy5 2bndy5 added the wontfix This will not be worked on label Jul 15, 2022
@brechtm
Copy link
Author

brechtm commented Jul 15, 2022

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
Copy link
Author

brechtm commented Jul 15, 2022

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
Copy link
Owner

jbms commented Jul 15, 2022

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
Copy link
Author

brechtm commented Jul 16, 2022

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
Copy link
Collaborator

2bndy5 commented Jul 16, 2022

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

@brechtm
Copy link
Author

brechtm commented Jul 16, 2022

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
Copy link
Author

brechtm commented Jul 16, 2022

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
Copy link
Collaborator

2bndy5 commented Jul 16, 2022

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

page_title = markupsafe.Markup.escape(
markupsafe.Markup(context.get("title")).striptags()
)
meta = context.get("meta")
if meta is None:
meta = {}

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
Copy link
Author

brechtm commented Jul 17, 2022

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
Copy link
Collaborator

2bndy5 commented Jul 17, 2022

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).

jbms added a commit that referenced this issue Jul 26, 2022
This addresses one issue noted in #132 although due to Sphinx bug
sphinx-doc/sphinx#9819 it is still not
possible to use only directives to selectively include `genindex` and
`modindex` in the global TOC.
jbms added a commit that referenced this issue Jul 26, 2022
This addresses one issue noted in #132 although due to Sphinx bug
sphinx-doc/sphinx#9819 it is still not
possible to use only directives to selectively include `genindex` and
`modindex` in the global TOC.
jbms added a commit that referenced this issue Jul 27, 2022
This addresses one issue noted in #132 although due to Sphinx bug
sphinx-doc/sphinx#9819 it is still not
possible to use only directives to selectively include `genindex` and
`modindex` in the global TOC.
jbms added a commit that referenced this issue Jul 27, 2022
This addresses one issue noted in #132 although due to Sphinx bug
sphinx-doc/sphinx#9819 it is still not
possible to use only directives to selectively include `genindex` and
`modindex` in the global TOC.
@brechtm
Copy link
Author

brechtm commented Aug 17, 2022

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
Copy link
Collaborator

2bndy5 commented Aug 17, 2022

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
Copy link
Author

brechtm commented Aug 17, 2022

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

@jbms
Copy link
Owner

jbms commented Aug 17, 2022

Is the only monkey patch still needed after #139?

@brechtm
Copy link
Author

brechtm commented Aug 17, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants