djangodocs Sphinx extension incompatible with other themes

[Mike Edmunds]

The docs/_ext/djangodocs.py catchall Sphinx extension has three problems that make it difficult or impossible to use with other Sphinx themes. These need to be fixed before the docs/_theme/djangodocs Sphinx theme can be replaced with a third-party, well-maintained theme as proposed in #37138.

1. Outdated workaround to remove table borders crashes other themes

The DjangoHTMLTranslator.visit_table() and depart_table() overrides are working around <table border=1> created by the docutils html4 writer. But Sphinx has used the html5 writer (which doesn't emit the obsolete border attribute) since 2.0. The workaround depends on undocumented Sphinx internal attrs and causes an IndexError when used with (e.g.,) sphinx-pydata-theme.

It can and should be removed.

Edit: similar problem with DjangoHTMLTranslator.visit_desc_parameterlist(), which was meant to prevent <big>(</big> in parameter lists. Sphinx hasn't done that in ages, and the override regularly breaks with new Sphinx versions (and prevents use of Sphinx's multi_line_parameter_list option).

2. console-tabs implementation creates asymmetric DOM

The console-tabs renders structurally different html for the Unix/macOS tab:

<section class="c-content-unix" id="c-content-0-unix">
  <div class="highlight-console notranslate">
    <div class="highlight">...</div>
  </div>
</section>

… compared to the Windows tab:

<section class="c-content-win" id="c-content-0-win">
  <div class="highlight">...</div>
</section>

Notice the expected wrapper <div class="highlight-doscon notranslate">) is missing on the Windows tab. Although this doesn't matter for the djangodocs and djangoproject.com themes (or they have specific workarounds?), it affects spacing in themes like furo and pydata-sphinx-theme. (It would also break a Pygments style that had special rules for the doscon lexer, and it might result in translation tools treating the Windows command shell syntax as English text to be translated.)

One solution is to borrow Sphinx's own machinery to render the Windows literal block, rather than calling the Pygments highlighter directly as now. This would also also simplify the code a bit.

3. console-tabs extension is tightly coupled to djangodocs theme templates

The console-tabs extension relies on a console-tabs.css file that is conditionally included in docs/_theme/djangodocs/layout.html based on context provided by the extension. That doesn't work for third-party themes with their own layout templates. (Or requires maintaining a custom theme inheriting from the third party.)

We should extract console-tabs.css from the djangodocs theme and make it available in the djangodocs extension for any theme. Options:

1. Move console-tabs.css to the project Sphinx static dir (docs/_static) and add it to all pages in docs/conf.py, whether or not they contain console tabs. (Note that docs.djangoproject.com essentially already does this with its customized console-tabs.scss.)
2. Move console-tabs.css to docs/_static but inject it conditionally only in pages that need it in the djangodocs extension. (Extensions can call app.add_css_file() in page-specific hooks since Sphinx 3.5.)
3. Move console-tabs.css to docs/_ext/static and make the djangodocs extension responsible for copying it into the static output. (Requires Sphinx 9.1 or later for app.add_static_dir() support in extensions.)

Also, as part of any rework here, we should eliminate the vendored fa-brands font in the Django docs source (it's used only for console-tabs icons) and update the console-tabs rendering for improved accessibility (see #35874 stage 1)

djangoproject.com considerations

Items 1 and 2 should not affect docs.djangoproject.com.

docs.djangoproject.com will likely want to track any changes to console-tabs.css in its custom console-tabs.scss. That file is used for all recently-rendered versions on docs.djangoproject.com, and older versions are rendered with the older djangodocs Sphinx extension (unless this gets backported). If the rendered DOM for the tabs is changing significantly, we should make it possible for djangoproject.com styles to differentiate docs built with the older tabs from the new tabs (e.g., by changing class="console-tabs" to class="dj-console-tabs").

[Natalia Bidart]

Thank you Mike, this all makes sense and matches what I saw when working on a PoC for #37138. My thoughts on some the questions:

1. Agreed
2. For this I was using a custom CSS rule in console-tabs.css to cope with the different spacing, I guess that's one solution but not robust and does not scale well. I'm not familiar with "Sphinx's own machinery to render the Windows" but sounds like a sensible approach.
3. I think option 1 is the simplest and is acceptable.

[Mike Edmunds]

​https://github.com/django/django/pull/21613

[Mike Edmunds]

Also…

4. Inconsistent handling of versionchanged, versionadded, deprecated directives

The djangodocs extension implements custom .. versionadded:: and .. versionchanged:: directives and overrides Sphinx's visit_versionmodified() in html writers (which also applies to .. deprecated:: and .. versionremoved::). There seem to be three goals:

1. Show "Django N.M" rather than the generic "version N.M" in these messages.
2. Show "Added in Django Development version" rather than "Added in Django N.M" for the current in-development version (as set by conf.py django_next_version).
3. Wrap the changeset message in a <span class="title"> so it can be bolded. (This seems to be an obsolete workaround for a very early version of Sphinx's html generation.)

There are multiple problems with the current implementation:

• The DOM generated by versionadded and versionchanged is significantly different from that for deprecated (which goes through Sphinx's native domains.changeset.VersionModified directive, and outputs a <span class="versionmodified"> for all changeset directives). This results in inconsistent styling with third-party themes.
• Item b is applied only in html-based output formats. Other formats will use Sphinx's standard changeset messages.
• None of this applies to deprecated, so you will see "Deprecated since version 6.2" rather than "Deprecated since Django development version" (in the ​current 6.2 dev build).
• Sphinx 9.0 renamed all these directives to version-changed, version-deprecated, etc. The original names are still supported as aliases. Django's custom handling would not apply to any uses of the new, hyphenated changeset directive names. (And we don't have a linter to prohibit the new names.)

Also, the capitalization seems wrong—it should be "Added in Django development version" not "Added in Django Development version." And there's no handling for versionremoved (though Django's docs don't use that). And it doesn't support localized message strings (but djangoproject.com handles localization through a different mechanism, so this doesn't really matter).

I think a better approach here would be to subclass Sphinx's native VersionModified to replace the django_next_version string while retaining all standard behavior; update Sphinx's changeset label strings to use "Django" rather than "version" in all output formats; and let Sphinx generate consistent HTML for all changeset directives.

Compatibility note: nothing I can find actually relies on the <span class="title">. djangoproject.com has ​rules targeting other changeset classes, and even the local theme's djangodocs.css targets the span.versionmodified present in Sphinx's normal changeset directive output. Changing to Sphinx's native behavior should not break any styles.

[Mike Edmunds]

Replying to Mike Edmunds:

1. Show "Django N.M" rather than the generic "version N.M" in these messages.
2. Show "Added in Django Development version" rather than "Added in Django N.M" for the current in-development version (as set by conf.py django_next_version).

It's worth asking how important these behaviors are. I suspect if someone proposed adding this functionality today, it would be rejected as too complicated for too little value.

An alternative would be to remove these customizations and use Sphinx's built-in behavior:

• "Added in version 6.0" (rather than "Added in Django 6.0")
• "Changed in version 6.2" (rather than "Changed in Django development version" during 6.2 development)
• "Deprecated since version 6.2" (no change)

Of the two, (a) is considerably more complicated/fragile than (b) to maintain. (Though (b) reads awkwardly without (a): "Changed in version development version" or "Changed in version Django development version.")

[Natalia Bidart]

Thank you Mike for the PR and the analysis. I have started a deep review and I have a strong preference to use Sphinx's wording for the version annotations. I have created ​a forum post about it.

One note/question: did you consider ticket #20104 when changing/redesigning how versionchanged should be changed?

[Mike Edmunds]

Replying to Natalia Bidart:

One note/question: did you consider ticket #20104 when changing/redesigning how versionchanged should be changed?

No, I was not aware of that… and had a moment of panic that I've been using those directives wrong in my own projects.

But I think that's outdated: Sphinx commit ​7630467 in Jan 2013 (Sphinx 1.2?) fixed the body-content paragraphs getting dropped. (fwiw, I use change directives both with and without the blank line in other projects, and the argument/content is rendered the same either way—as part of the changenote.)

One more reason not to use our own custom directives.

[Natalia Bidart]

Replying to Mike Edmunds:

[...] had a moment of panic that I've been using those directives wrong in my own projects.

Sorry about this!

But I think that's outdated: Sphinx commit ​7630467 in Jan 2013 (Sphinx 1.2?) fixed the body-content paragraphs getting dropped. (fwiw, I use change directives both with and without the blank line in other projects, and the argument/content is rendered the same either way—as part of the changenote.)

So I robot'd this quest of debugging and apparently the issue being fixed in #20104 was a bug in Django's own directive, not in Sphinx. This is VersionDirective as it stood immediately before ​the fix:

if len(self.arguments) == 2:
    inodes, messages = self.state.inline_text(self.arguments[1], self.lineno+1)
    node.extend(inodes)
    if self.content:
        self.state.nested_parse(self.content, self.content_offset, node)
    ret = ret + messages

Note that if self.content: is nested inside if len(self.arguments) == 2:, so the blank line form silently discarded the description. That matches the data loss reported in #20104 exactly. And the Sphinx commit you found confirms it from the other side: upstream had the two conditions as siblings, we did not. We fixed a bug in our copy of the directive by banning the form that triggered it, instead of un-nesting two lines to match Sphinx.

So there is nothing here worth preserving, and I would rather not carry the restriction forward: both forms render correctly on stock Sphinx, the footgun is gone, and reinstating the check would mean keeping exactly the kind of special treatment this PR is removing. It was never applied evenly anyway, since it only ever covered versionadded and versionchanged and never deprecated. Could you please add Refs #20104 to the commit message, and include in this branch a fix for docs/internals/contributing/writing-documentation.txt, which currently mandates the blank line, so that it reads as our convention rather than as a requirement Sphinx enforces?

[Mike Edmunds]

Replying to Natalia Bidart:

[…] Could you please add Refs #20104 to the commit message, and include in this branch a fix for docs/internals/contributing/writing-documentation.txt, which currently mandates the blank line, so that it reads as our convention rather than as a requirement Sphinx enforces?

👍 will do as part of removing the custom changeset directives.

[nessita <124304+nessita@…>]

In 9158562:

Fixed #37212 -- Improved djangodocs Sphinx extension portability.

• Removed unnecessary table and parameterlist customizations from the djangodocs Sphinx extension's DjangoHTMLTranslator. These were meant to avoid obsolete html from Docutils' html4 writer (border=1 in <table> tags and and <big> around parameter list parens).

Sphinx 2.0 switched to the html5 writer, which doesn't emit that html.
These overrides are no longer necessary, rely on Sphinx internals that
often break in new versions, and interfere with third-party themes.

• Moved the console-tabs.css stylesheet to docs/_static and removed logic for conditionally including it only in pages with console tabs. That depended on custom handling in the djangodocs theme layout.html template, which isn't portable to other themes.

• Switched console tabs from Font Awesome brand icons to text and removed the vendored fa-brands fonts (to avoid moving them from the djangodocs theme to the docs/_static dir).

• Ensured both console tabs include a wrapper div around literal code blocks: <div class="highlight-{lexer} notranslate"> (where lexer is console for Unix and doscon for Windows). This wrapper had been missing for the Windows content, which could cause shifting layouts when changing tabs with third-party Sphinx themes (and may have interfered with browser translation tools).

Fixed by rendering the Windows tab with a temporary literal block,
matching how the Unix tab is rendered. Removed direct call to the
Pygments highlighter.

The Windows tab is no longer force-highlighted, so a doscon lexing
failure now reports a warning rather than being silently ignored.

• Removed custom VersionDirective and HTML writer overrides for visit_versionmodified() and depart_versionmodified(). Sphinx's standard changeset directives are now used. (Refs #20104.)

The original code was meant to substitute "development version" for
conf.py django_next_version and to refer to "Django N.M" rather than
the generic "version N.M" in changeset messages. But it missed the
deprecated directive and aliases like version-modified added in
Sphinx 9.0, didn't apply to non-HTML output formats, and generated
inconsistent HTML that wasn't compatible with standard Sphinx themes.

Related cleanup:

• Updated documentation on using versionadded and versionchanged directives to accurately reflect Sphinx's rendered text, and removed the policy bullet describing the old Django-specific phrasing.

• Enabled biome for all css and js files in docs (except in docs/_build).

• Consistently excluded all non-docs source dirs and requirements.txt from searches for *.txt source files in conf.py exclude_patterns, make.bat, Makefile, and lint.py.

Refs #35874, #37138, #20104.

[Mike Edmunds]

5. Doctree caching bug

The console directive created builder-specific doctrees, resulting in unpredictable output when doctrees were shared between builds with different builders (as is done in our Makefile and in djangoproject.com's build_doc_release command), depending on build order and previous caching.

More details and fix in ​https://github.com/django/django/pull/22073#discussion_r4202127049

[nessita <124304+nessita@…>]

In ad96d15:

Refs #37212 -- Removed unused code in djangodocs Sphinx extension.

simple_option_desc_re was added in 4239bb0 for use by
parse_django_adminopt_node(). That function was removed in e519aab,
which reworked django-admin-option using Sphinx's standard Cmdoption.

[nessita <124304+nessita@…>]

In bbd0198:

Refs #37212 -- Added tests for djangodocs Sphinx extension.

Added test cases for djangodocs extension features that weren't yet
tested. Added a build_sphinx() helper in SimpleSphinxTestCase that
runs Sphinx on given source text and captures the results.

Bumped minimum Sphinx version to 7.3.0, which is where SphinxTestApp
gained the verbosity argument used by build_sphinx().

Thanks to @nessita for review and improvements.

[nessita <124304+nessita@…>]

In b720b73:

Refs #37212 -- Fixed console tabs bug on shared doctrees.

Removed a builder-specific optimization from ConsoleDirective. This
fixes a bug where building with shared doctrees (as docs/Makefile does)
could result in different output depending on the order of builds. The
optimization did not noticeably impact Sphinx build time.

This also resolves a RemovedInSphinx11Warning in ConsoleDirective.run()
that "'sphinx.environment.BuildEnvironment.app' is deprecated."

Thanks to @nessita for review (and for identifying this as a bug, not
just a Sphinx deprecation).

[nessita <124304+nessita@…>]

In 57ddccdc:

Refs #37212 -- Updated docs workflow to run Sphinx extension tests.

Added a 'docs' workflow job to run tests in tests/sphinx_tests. These
are not covered by the 'tests' workflow or Jenkins equivalents, which
specifically ignore docs/ changes and do not have Sphinx installed (so
many of the sphinx_tests are skipped).