Opened 3 weeks ago

Last modified 3 days ago

#37212 assigned Cleanup/optimization

djangodocs Sphinx extension incompatible with other themes

Reported by: Mike Edmunds Owned by: Mike Edmunds
Component: Documentation Version: 6.0
Severity: Normal Keywords: sphinx, djangodocs, djangoproject.com, sphinx-extension
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by 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").

Change History (6)

comment:1 by Natalia Bidart, 3 weeks ago

Triage Stage: UnreviewedAccepted

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.

comment:2 by Mike Edmunds, 3 weeks ago

Description: modified (diff)

comment:4 by Mike Edmunds, 2 weeks ago

Patch needs improvement: set

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.

Last edited 2 weeks ago by Mike Edmunds (previous) (diff)

comment:5 by Mike Edmunds, 2 weeks ago

Patch needs improvement: unset

in reply to:  4 comment:6 by Mike Edmunds, 3 days ago

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.

Version 0, edited 3 days ago by Mike Edmunds (next)
Note: See TracTickets for help on using tickets.
Back to Top