sphinx-last-updated-by-git icon indicating copy to clipboard operation
sphinx-last-updated-by-git copied to clipboard

Sphinx pre-8.2.0 master branch (since its 22e0f8944b147d commit) breaks this extension

Open jfbu opened this issue 1 year ago • 6 comments

With minimalistic project created by sphinx-quickstart and adding usage of this extension in conf.py I get

Versions
========

* Platform:         darwin; (macOS-10.13.6-x86_64-i386-64bit)
* Python version:   3.12.3 (CPython)
* Sphinx version:   8.2.0+/c49d925b4
* Docutils version: 0.21.2
* Jinja2 version:   3.1.4
* Pygments version: 2.18.0

Last Messages
=============


    reading sources... [100%]
    index



    getting Git timestamps for source files... [100%]
    .

Loaded Extensions
=================

* sphinx.ext.mathjax (8.2.0+/c49d925b4)
* alabaster (0.7.16)
* sphinxcontrib.applehelp (1.0.8)
* sphinxcontrib.devhelp (1.0.6)
* sphinxcontrib.htmlhelp (2.1.0)
* sphinxcontrib.serializinghtml (1.1.10)
* sphinxcontrib.qthelp (1.0.7)
* sphinx_last_updated_by_git (0.3.8)

Traceback
=========

    Traceback (most recent call last):
      File "/path/to/sphinx/sphinx/events.py", line 404, in emit
        results.append(listener.handler(self.app, *args))
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/somepath/.venv312/lib/python3.12/site-packages/sphinx_last_updated_by_git.py", line 187, in _env_updated
        for dep in env.dependencies[docname]:
                   ~~~~~~~~~~~~~~~~^^^^^^^^^
    KeyError: 'index'
    
    The above exception was the direct cause of the following exception:
    
    Traceback (most recent call last):
      File "/path/to/sphinx/sphinx/cmd/build.py", line 432, in build_main
        app.build(args.force_all, args.filenames)
      File "/path/to/sphinx/sphinx/application.py", line 426, in build
        self.builder.build_update()
      File "/path/to/sphinx/sphinx/builders/__init__.py", line 370, in build_update
        self.build(
      File "/path/to/sphinx/sphinx/builders/__init__.py", line 398, in build
        updated_docnames = set(self.read())
                               ^^^^^^^^^^^
      File "/path/to/sphinx/sphinx/builders/__init__.py", line 559, in read
        for retval in self.events.emit('env-updated', self.env):
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/path/to/sphinx/sphinx/events.py", line 415, in emit
        raise ExtensionError(
    sphinx.errors.ExtensionError: Handler <function _env_updated at 0x10fcec7c0> for event 'env-updated' threw an exception (exception: 'index')

Originally reported at https://github.com/sphinx-doc/sphinx/issues/13325#issuecomment-2656921482

jfbu avatar Feb 13 '25 15:02 jfbu

Caused as Sphinx changed env.dependencies from a defaultdict to dict, hence KeyError is now possible.

The _env_updated() function could add

if docname not in env.dependencies:
    continue

But also reverted in https://github.com/sphinx-doc/sphinx/commit/c68f846edcb93e8e7b744f54c1048352f6c9796b

A

AA-Turner avatar Feb 16 '25 03:02 AA-Turner

Thanks @jfbu for reporting this and @AA-Turner for reverting the change in Sphinx!

I'm not sure if an additional change in my extension would make sense: as long as I'm checking for an existing docname, I'm expecting to successfully get its dependencies (which might be empty).

I don't know whether using a defaultdict is strictly speaking the best solution, because that would mean that I also successfully get (empty) dependencies for a non-existing docname. But in my case that's not a problem.

mgeier avatar Feb 16 '25 10:02 mgeier

Unrelated but the https://www.sphinx-doc.org/en/master/templating.html#last_updated link which is used on the README.md front page of this extension, also on PyPi, returns a 404. It seems one should use this link. Could be useful to indicate which themes among those most used actually do employ the last_updated template variable.

jfbu avatar Feb 16 '25 10:02 jfbu

returns a 404

Thanks for reporting this @jfbu! I've fixed it in #80.

Could be useful to indicate which themes among those most used actually do employ the last_updated template variable.

I agree. But it sounds like too much work for me to generate and maintain such a list.

Another idea would be to actually build the docs for multiple themes (similar to how I did it for nbsphinx here: https://nbsphinx.readthedocs.io/en/0.9.6/usage.html#3rd-Party-Themes), but that's also more work than I'm willing to commit right now.

mgeier avatar Feb 16 '25 11:02 mgeier

I agree. But it sounds like too much work for me to generate and maintain such a list.

I do understand... Maybe (I am thinking here of dummies in my style) it would be good to insist in README that for the extension to actually have some effect, the used theme has to use somewhere the last_updated...

jfbu avatar Feb 16 '25 12:02 jfbu

I think you are right, this information is missing. I've added it in #81.

mgeier avatar Feb 16 '25 16:02 mgeier

I think this can be closed.

As mentioned above, the documentation can for sure be improved, I'm open for PRs!

mgeier avatar May 16 '25 18:05 mgeier