Use `_StrPath in sphinx.theming` (#13048)

This commit is contained in:
Adam Turner 2024-10-20 20:15:42 +01:00 committed by GitHub
parent 88f560c761
commit cb5318bdc8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 62 additions and 65 deletions

View File

@ -1586,7 +1586,7 @@ class Sphinx:
logger.debug('[app] adding environment collector: %r', collector) logger.debug('[app] adding environment collector: %r', collector)
collector().enable(self) collector().enable(self)
def add_html_theme(self, name: str, theme_path: str) -> None: def add_html_theme(self, name: str, theme_path: str | os.PathLike[str]) -> None:
"""Register a HTML Theme. """Register a HTML Theme.
The *name* is a name of theme, and *theme_path* is a full path to the The *name* is a name of theme, and *theme_path* is a full path to the

View File

@ -13,6 +13,7 @@ from jinja2.utils import open_if_exists, pass_context
from sphinx.application import TemplateBridge from sphinx.application import TemplateBridge
from sphinx.util import logging from sphinx.util import logging
from sphinx.util._pathlib import _StrPath
from sphinx.util.osutil import _last_modified_time from sphinx.util.osutil import _last_modified_time
if TYPE_CHECKING: if TYPE_CHECKING:
@ -170,10 +171,10 @@ class BuiltinTemplateLoader(TemplateBridge, BaseLoader):
# the theme's own dir and its bases' dirs # the theme's own dir and its bases' dirs
pathchain = theme.get_theme_dirs() pathchain = theme.get_theme_dirs()
# the loader dirs: pathchain + the parent directories for all themes # the loader dirs: pathchain + the parent directories for all themes
loaderchain = pathchain + [path.join(p, '..') for p in pathchain] loaderchain = pathchain + [p.parent for p in pathchain]
elif dirs: elif dirs:
pathchain = list(dirs) pathchain = list(map(_StrPath, dirs))
loaderchain = list(dirs) loaderchain = list(map(_StrPath, dirs))
else: else:
pathchain = [] pathchain = []
loaderchain = [] loaderchain = []
@ -182,7 +183,7 @@ class BuiltinTemplateLoader(TemplateBridge, BaseLoader):
self.templatepathlen = len(builder.config.templates_path) self.templatepathlen = len(builder.config.templates_path)
if builder.config.templates_path: if builder.config.templates_path:
cfg_templates_path = [ cfg_templates_path = [
path.join(builder.confdir, tp) for tp in builder.config.templates_path builder.confdir / tp for tp in builder.config.templates_path
] ]
pathchain[0:0] = cfg_templates_path pathchain[0:0] = cfg_templates_path
loaderchain[0:0] = cfg_templates_path loaderchain[0:0] = cfg_templates_path

View File

@ -17,9 +17,11 @@ from sphinx.locale import __
from sphinx.parsers import Parser as SphinxParser from sphinx.parsers import Parser as SphinxParser
from sphinx.roles import XRefRole from sphinx.roles import XRefRole
from sphinx.util import logging from sphinx.util import logging
from sphinx.util._pathlib import _StrPath
from sphinx.util.logging import prefixed_warnings from sphinx.util.logging import prefixed_warnings
if TYPE_CHECKING: if TYPE_CHECKING:
import os
from collections.abc import Callable, Iterator, Sequence from collections.abc import Callable, Iterator, Sequence
from docutils import nodes from docutils import nodes
@ -100,7 +102,7 @@ class SphinxComponentRegistry:
self.html_assets_policy: str = 'per_page' self.html_assets_policy: str = 'per_page'
#: HTML themes #: HTML themes
self.html_themes: dict[str, str] = {} self.html_themes: dict[str, _StrPath] = {}
#: js_files; list of JS paths or URLs #: js_files; list of JS paths or URLs
self.js_files: list[tuple[str | None, dict[str, Any]]] = [] self.js_files: list[tuple[str | None, dict[str, Any]]] = []
@ -433,8 +435,8 @@ class SphinxComponentRegistry:
if block_renderers is not None: if block_renderers is not None:
self.html_block_math_renderers[name] = block_renderers self.html_block_math_renderers[name] = block_renderers
def add_html_theme(self, name: str, theme_path: str) -> None: def add_html_theme(self, name: str, theme_path: str | os.PathLike[str]) -> None:
self.html_themes[name] = theme_path self.html_themes[name] = _StrPath(theme_path)
def load_extension(self, app: Sphinx, extname: str) -> None: def load_extension(self, app: Sphinx, extname: str) -> None:
"""Load a Sphinx extension.""" """Load a Sphinx extension."""

View File

@ -6,13 +6,12 @@ __all__ = ('Theme', 'HTMLThemeFactory')
import configparser import configparser
import contextlib import contextlib
import os
import shutil import shutil
import sys import sys
import tempfile import tempfile
import tomllib import tomllib
from importlib.metadata import entry_points from importlib.metadata import entry_points
from os import path from pathlib import Path
from typing import TYPE_CHECKING, Any from typing import TYPE_CHECKING, Any
from zipfile import ZipFile from zipfile import ZipFile
@ -21,6 +20,7 @@ from sphinx.config import check_confval_types as _config_post_init
from sphinx.errors import ThemeError from sphinx.errors import ThemeError
from sphinx.locale import __ from sphinx.locale import __
from sphinx.util import logging from sphinx.util import logging
from sphinx.util._pathlib import _StrPath
from sphinx.util.osutil import ensuredir from sphinx.util.osutil import ensuredir
if TYPE_CHECKING: if TYPE_CHECKING:
@ -62,8 +62,8 @@ class Theme:
name: str, name: str,
*, *,
configs: dict[str, _ConfigFile], configs: dict[str, _ConfigFile],
paths: list[str], paths: list[Path],
tmp_dirs: list[str], tmp_dirs: list[Path],
) -> None: ) -> None:
self.name = name self.name = name
self._dirs = tuple(paths) self._dirs = tuple(paths)
@ -87,11 +87,11 @@ class Theme:
self._options = options self._options = options
def get_theme_dirs(self) -> list[str]: def get_theme_dirs(self) -> list[_StrPath]:
"""Return a list of theme directories, beginning with this theme's, """Return a list of theme directories, beginning with this theme's,
then the base theme's, then that one's base theme's, etc. then the base theme's, then that one's base theme's, etc.
""" """
return list(self._dirs) return list(map(_StrPath, self._dirs))
def get_config(self, section: str, name: str, default: Any = _NO_DEFAULT) -> Any: def get_config(self, section: str, name: str, default: Any = _NO_DEFAULT) -> Any:
"""Return the value for a theme configuration setting, searching the """Return the value for a theme configuration setting, searching the
@ -159,17 +159,17 @@ class HTMLThemeFactory:
def _load_builtin_themes(self) -> None: def _load_builtin_themes(self) -> None:
"""Load built-in themes.""" """Load built-in themes."""
themes = self._find_themes(path.join(package_dir, 'themes')) themes = self._find_themes(Path(package_dir, 'themes'))
for name, theme in themes.items(): for name, theme in themes.items():
self._themes[name] = theme self._themes[name] = _StrPath(theme)
def _load_additional_themes(self, theme_paths: list[str]) -> None: def _load_additional_themes(self, theme_paths: list[str]) -> None:
"""Load additional themes placed at specified directories.""" """Load additional themes placed at specified directories."""
for theme_path in theme_paths: for theme_path in theme_paths:
abs_theme_path = path.abspath(path.join(self._app.confdir, theme_path)) abs_theme_path = (self._app.confdir / theme_path).resolve()
themes = self._find_themes(abs_theme_path) themes = self._find_themes(abs_theme_path)
for name, theme in themes.items(): for name, theme in themes.items():
self._themes[name] = theme self._themes[name] = _StrPath(theme)
def _load_entry_point_themes(self) -> None: def _load_entry_point_themes(self) -> None:
"""Try to load a theme with the specified name. """Try to load a theme with the specified name.
@ -191,18 +191,17 @@ class HTMLThemeFactory:
self._entry_point_themes[entry_point.name] = _load_theme_closure self._entry_point_themes[entry_point.name] = _load_theme_closure
@staticmethod @staticmethod
def _find_themes(theme_path: str) -> dict[str, str]: def _find_themes(theme_path: Path) -> dict[str, Path]:
"""Search themes from specified directory.""" """Search themes from specified directory."""
themes: dict[str, str] = {} themes: dict[str, Path] = {}
if not path.isdir(theme_path): if not theme_path.is_dir():
return themes return themes
for entry in os.listdir(theme_path): for pathname in theme_path.iterdir():
pathname = path.join(theme_path, entry) entry = pathname.name
if path.isfile(pathname) and entry.lower().endswith('.zip'): if pathname.is_file() and pathname.suffix.lower() == '.zip':
if _is_archived_theme(pathname): if _is_archived_theme(pathname):
name = entry[:-4] themes[pathname.stem] = pathname
themes[name] = pathname
else: else:
logger.warning( logger.warning(
__( __(
@ -212,9 +211,9 @@ class HTMLThemeFactory:
entry, entry,
) )
else: else:
toml_path = path.join(pathname, _THEME_TOML) toml_path = pathname / _THEME_TOML
conf_path = path.join(pathname, _THEME_CONF) conf_path = pathname / _THEME_CONF
if path.isfile(toml_path) or path.isfile(conf_path): if toml_path.is_file() or conf_path.is_file():
themes[entry] = pathname themes[entry] = pathname
return themes return themes
@ -236,7 +235,7 @@ class HTMLThemeFactory:
return Theme(name, configs=themes, paths=theme_dirs, tmp_dirs=tmp_dirs) return Theme(name, configs=themes, paths=theme_dirs, tmp_dirs=tmp_dirs)
def _is_archived_theme(filename: str, /) -> bool: def _is_archived_theme(filename: Path, /) -> bool:
"""Check whether the specified file is an archived theme file or not.""" """Check whether the specified file is an archived theme file or not."""
try: try:
with ZipFile(filename) as f: with ZipFile(filename) as f:
@ -248,13 +247,13 @@ def _is_archived_theme(filename: str, /) -> bool:
def _load_theme_with_ancestors( def _load_theme_with_ancestors(
name: str, name: str,
theme_paths: dict[str, str], theme_paths: dict[str, _StrPath],
entry_point_themes: dict[str, Callable[[], None]], entry_point_themes: dict[str, Callable[[], None]],
/, /,
) -> tuple[dict[str, _ConfigFile], list[str], list[str]]: ) -> tuple[dict[str, _ConfigFile], list[Path], list[Path]]:
themes: dict[str, _ConfigFile] = {} themes: dict[str, _ConfigFile] = {}
theme_dirs: list[str] = [] theme_dirs: list[Path] = []
tmp_dirs: list[str] = [] tmp_dirs: list[Path] = []
# having 10+ theme ancestors is ludicrous # having 10+ theme ancestors is ludicrous
for _ in range(10): for _ in range(10):
@ -287,23 +286,23 @@ def _load_theme_with_ancestors(
def _load_theme( def _load_theme(
name: str, theme_path: str, / name: str, theme_path: Path, /
) -> tuple[str, str, str | None, _ConfigFile]: ) -> tuple[str, Path, Path | None, _ConfigFile]:
if path.isdir(theme_path): if theme_path.is_dir():
# already a directory, do nothing # already a directory, do nothing
tmp_dir = None tmp_dir = None
theme_dir = theme_path theme_dir = theme_path
else: else:
# extract the theme to a temp directory # extract the theme to a temp directory
tmp_dir = tempfile.mkdtemp('sxt') tmp_dir = Path(tempfile.mkdtemp('sxt'))
theme_dir = path.join(tmp_dir, name) theme_dir = tmp_dir / name
_extract_zip(theme_path, theme_dir) _extract_zip(theme_path, theme_dir)
if path.isfile(toml_path := path.join(theme_dir, _THEME_TOML)): if (toml_path := theme_dir / _THEME_TOML).is_file():
_cfg_table = _load_theme_toml(toml_path) _cfg_table = _load_theme_toml(toml_path)
inherit = _validate_theme_toml(_cfg_table, name) inherit = _validate_theme_toml(_cfg_table, name)
config = _convert_theme_toml(_cfg_table) config = _convert_theme_toml(_cfg_table)
elif path.isfile(conf_path := path.join(theme_dir, _THEME_CONF)): elif (conf_path := theme_dir / _THEME_CONF).is_file():
_cfg_parser = _load_theme_conf(conf_path) _cfg_parser = _load_theme_conf(conf_path)
inherit = _validate_theme_conf(_cfg_parser, name) inherit = _validate_theme_conf(_cfg_parser, name)
config = _convert_theme_conf(_cfg_parser) config = _convert_theme_conf(_cfg_parser)
@ -313,7 +312,7 @@ def _load_theme(
return inherit, theme_dir, tmp_dir, config return inherit, theme_dir, tmp_dir, config
def _extract_zip(filename: str, target_dir: str, /) -> None: def _extract_zip(filename: Path, target_dir: Path, /) -> None:
"""Extract zip file to target directory.""" """Extract zip file to target directory."""
ensuredir(target_dir) ensuredir(target_dir)
@ -321,16 +320,13 @@ def _extract_zip(filename: str, target_dir: str, /) -> None:
for name in archive.namelist(): for name in archive.namelist():
if name.endswith('/'): if name.endswith('/'):
continue continue
entry = path.join(target_dir, name) entry = target_dir / name
ensuredir(path.dirname(entry)) ensuredir(entry.parent)
with open(path.join(entry), 'wb') as fp: entry.write_bytes(archive.read(name))
fp.write(archive.read(name))
def _load_theme_toml(config_file_path: str, /) -> _ThemeToml: def _load_theme_toml(config_file_path: Path, /) -> _ThemeToml:
with open(config_file_path, encoding='utf-8') as f: c = tomllib.loads(config_file_path.read_text(encoding='utf-8'))
config_text = f.read()
c = tomllib.loads(config_text)
return {s: c[s] for s in ('theme', 'options') if s in c} # type: ignore[return-value] return {s: c[s] for s in ('theme', 'options') if s in c} # type: ignore[return-value]
@ -381,7 +377,7 @@ def _convert_theme_toml(cfg: _ThemeToml, /) -> _ConfigFile:
) )
def _load_theme_conf(config_file_path: str, /) -> configparser.RawConfigParser: def _load_theme_conf(config_file_path: Path, /) -> configparser.RawConfigParser:
c = configparser.RawConfigParser() c = configparser.RawConfigParser()
c.read(config_file_path, encoding='utf-8') c.read(config_file_path, encoding='utf-8')
return c return c
@ -487,9 +483,9 @@ def _migrate_conf_to_toml(argv: list[str]) -> int:
if len(argv) != 1: if len(argv) != 1:
print('Usage: python -m sphinx.theming conf_to_toml <theme path>') # NoQA: T201 print('Usage: python -m sphinx.theming conf_to_toml <theme path>') # NoQA: T201
raise SystemExit(1) raise SystemExit(1)
theme_dir = path.realpath(argv[0]) theme_dir = Path(argv[0]).resolve()
conf_path = path.join(theme_dir, _THEME_CONF) conf_path = theme_dir / _THEME_CONF
if not path.isdir(theme_dir) or not path.isfile(conf_path): if not theme_dir.is_dir() or not conf_path.is_file():
print( # NoQA: T201 print( # NoQA: T201
f'{theme_dir!r} must be a path to a theme directory containing a "theme.conf" file' f'{theme_dir!r} must be a path to a theme directory containing a "theme.conf" file'
) )
@ -543,9 +539,8 @@ def _migrate_conf_to_toml(argv: list[str]) -> int:
if (d := default.replace('"', r'\"')) or True if (d := default.replace('"', r'\"')) or True
] ]
toml_path = path.join(theme_dir, _THEME_TOML) toml_path = theme_dir / _THEME_TOML
with open(toml_path, 'w', encoding='utf-8') as f: toml_path.write_text('\n'.join(toml_lines) + '\n', encoding='utf-8')
f.write('\n'.join(toml_lines) + '\n')
print(f'Written converted settings to {toml_path!r}') # NoQA: T201 print(f'Written converted settings to {toml_path!r}') # NoQA: T201
return 0 return 0

View File

@ -1,6 +1,5 @@
"""Test the Theme class.""" """Test the Theme class."""
import os
import shutil import shutil
from pathlib import Path from pathlib import Path
from xml.etree.ElementTree import ParseError from xml.etree.ElementTree import ParseError
@ -52,11 +51,11 @@ def test_theme_api(app):
# test Theme class API # test Theme class API
assert set(app.registry.html_themes.keys()) == set(themes) assert set(app.registry.html_themes.keys()) == set(themes)
assert app.registry.html_themes['test-theme'] == str( assert app.registry.html_themes['test-theme'] == (
app.srcdir / 'test_theme' / 'test-theme' app.srcdir / 'test_theme' / 'test-theme'
) )
assert app.registry.html_themes['ziptheme'] == str(app.srcdir / 'ziptheme.zip') assert app.registry.html_themes['ziptheme'] == (app.srcdir / 'ziptheme.zip')
assert app.registry.html_themes['staticfiles'] == str( assert app.registry.html_themes['staticfiles'] == (
app.srcdir / 'test_theme' / 'staticfiles' app.srcdir / 'test_theme' / 'staticfiles'
) )
@ -88,14 +87,14 @@ def test_theme_api(app):
# cleanup temp directories # cleanup temp directories
theme._cleanup() theme._cleanup()
assert not any(map(os.path.exists, theme._tmp_dirs)) assert not any(p.exists() for p in theme._tmp_dirs)
def test_nonexistent_theme_settings(tmp_path): def test_nonexistent_theme_settings(tmp_path):
# Check that error occurs with a non-existent theme.toml or theme.conf # Check that error occurs with a non-existent theme.toml or theme.conf
# (https://github.com/sphinx-doc/sphinx/issues/11668) # (https://github.com/sphinx-doc/sphinx/issues/11668)
with pytest.raises(ThemeError): with pytest.raises(ThemeError):
_load_theme('', str(tmp_path)) _load_theme('', tmp_path)
@pytest.mark.sphinx('html', testroot='double-inheriting-theme') @pytest.mark.sphinx('html', testroot='double-inheriting-theme')
@ -224,7 +223,7 @@ def test_theme_builds(make_app, rootdir, sphinx_test_tempdir, theme_name):
def test_config_file_toml(): def test_config_file_toml():
config_path = HERE / 'theme.toml' config_path = HERE / 'theme.toml'
cfg = _load_theme_toml(str(config_path)) cfg = _load_theme_toml(config_path)
config = _convert_theme_toml(cfg) config = _convert_theme_toml(cfg)
assert config == _ConfigFile( assert config == _ConfigFile(
@ -238,7 +237,7 @@ def test_config_file_toml():
def test_config_file_conf(): def test_config_file_conf():
config_path = HERE / 'theme.conf' config_path = HERE / 'theme.conf'
cfg = _load_theme_conf(str(config_path)) cfg = _load_theme_conf(config_path)
config = _convert_theme_conf(cfg) config = _convert_theme_conf(cfg)
assert config == _ConfigFile( assert config == _ConfigFile(