Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ Release 7.4.5 (in development)
Bugs fixed
----------

* #12593, #12600: Revert coercing the type of selected :confval:`html_sidebars`
values to a list.
Log an error message when string values are detected.
Patch by Adam Turner.

Release 7.4.4 (released Jul 15, 2024)
=====================================
Expand Down
29 changes: 24 additions & 5 deletions sphinx/builders/html/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -982,11 +982,10 @@ def has_wildcard(pattern: str) -> bool:
matched = pattern
sidebars = pat_sidebars

if len(sidebars) == 0:
# keep defaults
pass

ctx['sidebars'] = list(sidebars)
# See error_on_html_sidebars_string_values.
# Replace with simple list coercion in Sphinx 8.0
# xref: RemovedInSphinx80Warning
ctx['sidebars'] = sidebars

# --------- these are overwritten by the serialization builder

Expand Down Expand Up @@ -1287,6 +1286,25 @@ def validate_html_favicon(app: Sphinx, config: Config) -> None:
config.html_favicon = None


def error_on_html_sidebars_string_values(app: Sphinx, config: Config) -> None:
"""Support removed in Sphinx 2."""
errors = {}
for pattern, pat_sidebars in config.html_sidebars.items():
if isinstance(pat_sidebars, str):
errors[pattern] = [pat_sidebars]
if not errors:
return
msg = __("Values in 'html_sidebars' must be a list of strings. "
"At least one pattern has a string value: %s. "
"Change to `html_sidebars = %r`.")
bad_patterns = ', '.join(map(repr, errors))
fixed = config.html_sidebars | errors
logger.error(msg, bad_patterns, fixed)
# Enable hard error in next major version.
# xref: RemovedInSphinx80Warning
# raise ConfigError(msg % (bad_patterns, fixed))


def error_on_html_4(_app: Sphinx, config: Config) -> None:
"""Error on HTML 4."""
if config.html4_writer:
Expand Down Expand Up @@ -1357,6 +1375,7 @@ def setup(app: Sphinx) -> ExtensionMetadata:
app.connect('config-inited', validate_html_static_path, priority=800)
app.connect('config-inited', validate_html_logo, priority=800)
app.connect('config-inited', validate_html_favicon, priority=800)
app.connect('config-inited', error_on_html_sidebars_string_values, priority=800)
app.connect('config-inited', error_on_html_4, priority=800)
app.connect('builder-inited', validate_math_renderer)
app.connect('html-page-context', setup_resource_paths)
Expand Down
28 changes: 27 additions & 1 deletion tests/test_builders/test_build_html.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Test the HTML builder and check output against XPath."""

import contextlib
import os
import posixpath
import re
Expand All @@ -8,14 +9,39 @@

from sphinx.builders.html import validate_html_extra_path, validate_html_static_path
from sphinx.deprecation import RemovedInSphinx80Warning
from sphinx.errors import ConfigError
from sphinx.errors import ConfigError, ThemeError
from sphinx.util.console import strip_colors
from sphinx.util.inventory import InventoryFile

from tests.test_builders.xpath_data import FIGURE_CAPTION
from tests.test_builders.xpath_util import check_xpath


def test_html_sidebars_error(make_app, tmp_path):
(tmp_path / 'conf.py').touch()
(tmp_path / 'index.rst').touch()
app = make_app(
buildername='html',
srcdir=tmp_path,
confoverrides={'html_sidebars': {'index': 'searchbox.html'}},
)

# Test that the error is logged
warnings = app.warning.getvalue()
assert ("ERROR: Values in 'html_sidebars' must be a list of strings. "
"At least one pattern has a string value: 'index'. "
"Change to `html_sidebars = {'index': ['searchbox.html']}`.") in warnings

# But that the value is unchanged.
# (Remove this bit of the test in Sphinx 8)
def _html_context_hook(app, pagename, templatename, context, doctree):
assert context["sidebars"] == 'searchbox.html'
app.connect('html-page-context', _html_context_hook)
with contextlib.suppress(ThemeError):
# ignore template rendering issues (ThemeError).
app.build()


def test_html4_error(make_app, tmp_path):
(tmp_path / 'conf.py').write_text('', encoding='utf-8')
with pytest.raises(
Expand Down