Parse theme.conf to a new _ConfigFile type (sphinx-doc#12254)

Author: AA-Turner

sphinx/builders/html/__init__.py

@@ -265,8 +265,7 @@ def _get_style_filenames(self) -> Iterator[str]:
         elif self.config.html_style is not None:
             yield from self.config.html_style
         elif self.theme:
-            stylesheet = self.theme.get_config('theme', 'stylesheet')
-            yield from map(str.strip, stylesheet.split(','))
+            yield from self.theme.stylesheets
         else:
             yield 'default.css'
 
@@ -286,13 +285,15 @@ def init_highlighter(self) -> None:
         if self.config.pygments_style is not None:
             style = self.config.pygments_style
         elif self.theme:
-            style = self.theme.get_config('theme', 'pygments_style', 'none')
+            # From the ``pygments_style`` theme setting
+            style = self.theme.pygments_style_default or 'none'
         else:
             style = 'sphinx'
         self.highlighter = PygmentsBridge('html', style)
 
         if self.theme:
-            dark_style = self.theme.get_config('theme', 'pygments_dark_style', None)
+            # From the ``pygments_dark_style`` theme setting
+            dark_style = self.theme.pygments_style_dark
         else:
             dark_style = None
 
@@ -960,13 +961,11 @@ def add_sidebars(self, pagename: str, ctx: dict) -> None:
         def has_wildcard(pattern: str) -> bool:
             return any(char in pattern for char in '*?[')
 
-        sidebars = None
         matched = None
         customsidebar = None
 
         # default sidebars settings for selected theme
-        if theme_default_sidebars := self.theme.get_config('theme', 'sidebars', None):
-            sidebars = [name.strip() for name in theme_default_sidebars.split(',')]
+        sidebars = list(self.theme.sidebar_templates)
 
         # user sidebar settings
         html_sidebars = self.get_builder_config('sidebars', 'html')
@@ -985,7 +984,7 @@ def has_wildcard(pattern: str) -> bool:
                 matched = pattern
                 sidebars = patsidebars
 
-        if sidebars is None:
+        if len(sidebars) == 0:
             # keep defaults
             pass
 

sphinx/theming.py

@@ -27,6 +27,8 @@
     from importlib_metadata import entry_points  # type: ignore[import-not-found]
 
 if TYPE_CHECKING:
+    from collections.abc import Iterable
+
     from sphinx.application import Sphinx
 
 logger = logging.getLogger(__name__)
@@ -45,36 +47,57 @@ def __init__(
         self,
         name: str,
         *,
-        configs: dict[str, configparser.RawConfigParser],
+        configs: dict[str, _ConfigFile],
         paths: list[str],
         tmp_dirs: list[str],
     ) -> None:
         self.name = name
-        self._dirs = paths
+        self._dirs = tuple(paths)
         self._tmp_dirs = tmp_dirs
 
-        theme: dict[str, Any] = {}
         options: dict[str, Any] = {}
+        self.stylesheets: tuple[str, ...] = ()
+        self.sidebar_templates: tuple[str, ...] = ()
+        self.pygments_style_default: str | None = None
+        self.pygments_style_dark: str | None = None
         for config in reversed(configs.values()):
-            theme |= dict(config.items('theme'))
-            if config.has_section('options'):
-                options |= dict(config.items('options'))
+            options |= config.options
+            if len(config.stylesheets):
+                self.stylesheets = config.stylesheets
+            if len(config.sidebar_templates):
+                self.sidebar_templates = config.sidebar_templates
+            if config.pygments_style_default is not None:
+                self.pygments_style_default = config.pygments_style_default
+            if config.pygments_style_dark is not None:
+                self.pygments_style_dark = config.pygments_style_dark
 
-        self._settings = theme
         self._options = options
 
+        if len(self.stylesheets) == 0:
+            msg = __("No loaded theme defines 'theme.stylesheet' in the configuration")
+            raise ThemeError(msg) from None
+
     def get_theme_dirs(self) -> list[str]:
         """Return a list of theme directories, beginning with this theme's,
         then the base theme's, then that one's base theme's, etc.
         """
-        return self._dirs.copy()
+        return list(self._dirs)
 
     def get_config(self, section: str, name: str, default: Any = _NO_DEFAULT) -> Any:
         """Return the value for a theme configuration setting, searching the
         base theme chain.
         """
         if section == 'theme':
-            value = self._settings.get(name, default)
+            if name == 'stylesheet':
+                value = ', '.join(self.stylesheets) or default
+            elif name == 'sidebars':
+                value = ', '.join(self.sidebar_templates) or default
+            elif name == 'pygments_style':
+                value = self.pygments_style_default or default
+            elif name == 'pygments_dark_style':
+                value = self.pygments_style_dark or default
+            else:
+                value = default
         elif section == 'options':
             value = self._options.get(name, default)
         else:
@@ -196,8 +219,8 @@ def _is_archived_theme(filename: str, /) -> bool:
 
 def _load_theme_with_ancestors(
     theme_paths: dict[str, str], name: str, /
-) -> tuple[dict[str, configparser.RawConfigParser], list[str], list[str]]:
-    themes: dict[str, configparser.RawConfigParser] = {}
+) -> tuple[dict[str, _ConfigFile], list[str], list[str]]:
+    themes: dict[str, _ConfigFile] = {}
     theme_dirs: list[str] = []
     tmp_dirs: list[str] = []
 
@@ -227,9 +250,7 @@ def _load_theme_with_ancestors(
     return themes, theme_dirs, tmp_dirs
 
 
-def _load_theme(
-    name: str, theme_path: str, /
-) -> tuple[str, str, str | None, configparser.RawConfigParser]:
+def _load_theme(name: str, theme_path: str, /) -> tuple[str, str, str | None, _ConfigFile]:
     if path.isdir(theme_path):
         # already a directory, do nothing
         tmp_dir = None
@@ -240,12 +261,13 @@ def _load_theme(
         theme_dir = path.join(tmp_dir, name)
         _extract_zip(theme_path, theme_dir)
 
-    config = _load_theme_conf(theme_dir)
-    try:
-        inherit = config.get('theme', 'inherit')
-    except (configparser.NoOptionError, configparser.NoSectionError):
-        msg = __('The %r theme must define the "theme.inherit" setting') % name
-        raise ThemeError(msg) from None
+    if os.path.isfile(conf_path := path.join(theme_dir, _THEME_CONF)):
+        _cfg_parser = _load_theme_conf(conf_path)
+        inherit = _validate_theme_conf(_cfg_parser, name)
+        config = _convert_theme_conf(_cfg_parser)
+    else:
+        raise ThemeError(__('no theme configuration file found in %r') % theme_dir)
+
     return inherit, theme_dir, tmp_dir, config
 
 
@@ -263,10 +285,92 @@ def _extract_zip(filename: str, target_dir: str, /) -> None:
                 fp.write(archive.read(name))
 
 
-def _load_theme_conf(theme_dir: os.PathLike[str] | str, /) -> configparser.RawConfigParser:
+def _load_theme_conf(config_file_path: str, /) -> configparser.RawConfigParser:
     c = configparser.RawConfigParser()
-    config_file_path = path.join(theme_dir, _THEME_CONF)
-    if not os.path.isfile(config_file_path):
-        raise ThemeError(__('theme configuration file %r not found') % config_file_path)
     c.read(config_file_path, encoding='utf-8')
     return c
+
+
+def _validate_theme_conf(cfg: configparser.RawConfigParser, name: str) -> str:
+    if not cfg.has_section('theme'):
+        raise ThemeError(__('theme %r doesn\'t have the "theme" table') % name)
+    if inherit := cfg.get('theme', 'inherit', fallback=None):
+        return inherit
+    msg = __('The %r theme must define the "theme.inherit" setting') % name
+    raise ThemeError(msg)
+
+
+def _convert_theme_conf(cfg: configparser.RawConfigParser, /) -> _ConfigFile:
+    if stylesheet := cfg.get('theme', 'stylesheet', fallback=''):
+        stylesheets: tuple[str, ...] = tuple(map(str.strip, stylesheet.split(',')))
+    else:
+        stylesheets = ()
+    if sidebar := cfg.get('theme', 'sidebars', fallback=''):
+        sidebar_templates: tuple[str, ...] = tuple(map(str.strip, sidebar.split(',')))
+    else:
+        sidebar_templates = ()
+    pygments_style_default: str | None = cfg.get('theme', 'pygments_style', fallback=None)
+    pygments_style_dark: str | None = cfg.get('theme', 'pygments_dark_style', fallback=None)
+    options = dict(cfg.items('options')) if cfg.has_section('options') else {}
+    return _ConfigFile(
+        stylesheets=stylesheets,
+        sidebar_templates=sidebar_templates,
+        pygments_style_default=pygments_style_default,
+        pygments_style_dark=pygments_style_dark,
+        options=options,
+    )
+
+
+class _ConfigFile:
+    __slots__ = (
+        'stylesheets',
+        'sidebar_templates',
+        'pygments_style_default',
+        'pygments_style_dark',
+        'options',
+    )
+
+    def __init__(
+        self,
+        stylesheets: Iterable[str],
+        sidebar_templates: Iterable[str],
+        pygments_style_default: str | None,
+        pygments_style_dark: str | None,
+        options: dict[str, str],
+    ) -> None:
+        self.stylesheets: tuple[str, ...] = tuple(stylesheets)
+        self.sidebar_templates: tuple[str, ...] = tuple(sidebar_templates)
+        self.pygments_style_default: str | None = pygments_style_default
+        self.pygments_style_dark: str | None = pygments_style_dark
+        self.options: dict[str, str] = options.copy()
+
+    def __repr__(self) -> str:
+        return (
+            f'{self.__class__.__qualname__}('
+            f'stylesheets={self.stylesheets!r}, '
+            f'sidebar_templates={self.sidebar_templates!r}, '
+            f'pygments_style_default={self.pygments_style_default!r}, '
+            f'pygments_style_dark={self.pygments_style_dark!r}, '
+            f'options={self.options!r})'
+        )
+
+    def __eq__(self, other: object) -> bool:
+        if isinstance(other, _ConfigFile):
+            return (
+                self.stylesheets == other.stylesheets
+                and self.sidebar_templates == other.sidebar_templates
+                and self.pygments_style_default == other.pygments_style_default
+                and self.pygments_style_dark == other.pygments_style_dark
+                and self.options == other.options
+            )
+        return NotImplemented
+
+    def __hash__(self) -> int:
+        return hash((
+            self.__class__.__qualname__,
+            self.stylesheets,
+            self.sidebar_templates,
+            self.pygments_style_default,
+            self.pygments_style_dark,
+            self.options,
+        ))

tests/test_theming/test_theming.py

@@ -9,7 +9,7 @@
 
 import sphinx.builders.html
 from sphinx.errors import ThemeError
-from sphinx.theming import _load_theme_conf
+from sphinx.theming import _load_theme
 
 
 @pytest.mark.sphinx(
@@ -81,7 +81,7 @@ def test_nonexistent_theme_conf(tmp_path):
     # Check that error occurs with a non-existent theme.conf
     # (https://github.com/sphinx-doc/sphinx/issues/11668)
     with pytest.raises(ThemeError):
-        _load_theme_conf(tmp_path)
+        _load_theme('', str(tmp_path))
 
 
 @pytest.mark.sphinx(testroot='double-inheriting-theme')