From cb5318bdc819d45959c99b312c58442783864eb3 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Sun, 20 Oct 2024 20:15:42 +0100 Subject: [PATCH] Use ``_StrPath`` in ``sphinx.theming`` (#13048) --- sphinx/application.py | 2 +- sphinx/jinja2glue.py | 9 +-- sphinx/registry.py | 8 ++- sphinx/theming.py | 93 ++++++++++++++---------------- tests/test_theming/test_theming.py | 15 +++-- 5 files changed, 62 insertions(+), 65 deletions(-) diff --git a/sphinx/application.py b/sphinx/application.py index 88a9a85be..98d9854ac 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -1586,7 +1586,7 @@ class Sphinx: logger.debug('[app] adding environment collector: %r', collector) 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. The *name* is a name of theme, and *theme_path* is a full path to the diff --git a/sphinx/jinja2glue.py b/sphinx/jinja2glue.py index 929487a6e..3621d417b 100644 --- a/sphinx/jinja2glue.py +++ b/sphinx/jinja2glue.py @@ -13,6 +13,7 @@ from jinja2.utils import open_if_exists, pass_context from sphinx.application import TemplateBridge from sphinx.util import logging +from sphinx.util._pathlib import _StrPath from sphinx.util.osutil import _last_modified_time if TYPE_CHECKING: @@ -170,10 +171,10 @@ class BuiltinTemplateLoader(TemplateBridge, BaseLoader): # the theme's own dir and its bases' dirs pathchain = theme.get_theme_dirs() # 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: - pathchain = list(dirs) - loaderchain = list(dirs) + pathchain = list(map(_StrPath, dirs)) + loaderchain = list(map(_StrPath, dirs)) else: pathchain = [] loaderchain = [] @@ -182,7 +183,7 @@ class BuiltinTemplateLoader(TemplateBridge, BaseLoader): self.templatepathlen = len(builder.config.templates_path) if builder.config.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 loaderchain[0:0] = cfg_templates_path diff --git a/sphinx/registry.py b/sphinx/registry.py index da21aefce..b4c247865 100644 --- a/sphinx/registry.py +++ b/sphinx/registry.py @@ -17,9 +17,11 @@ from sphinx.locale import __ from sphinx.parsers import Parser as SphinxParser from sphinx.roles import XRefRole from sphinx.util import logging +from sphinx.util._pathlib import _StrPath from sphinx.util.logging import prefixed_warnings if TYPE_CHECKING: + import os from collections.abc import Callable, Iterator, Sequence from docutils import nodes @@ -100,7 +102,7 @@ class SphinxComponentRegistry: self.html_assets_policy: str = 'per_page' #: HTML themes - self.html_themes: dict[str, str] = {} + self.html_themes: dict[str, _StrPath] = {} #: js_files; list of JS paths or URLs self.js_files: list[tuple[str | None, dict[str, Any]]] = [] @@ -433,8 +435,8 @@ class SphinxComponentRegistry: if block_renderers is not None: self.html_block_math_renderers[name] = block_renderers - def add_html_theme(self, name: str, theme_path: str) -> None: - self.html_themes[name] = theme_path + def add_html_theme(self, name: str, theme_path: str | os.PathLike[str]) -> None: + self.html_themes[name] = _StrPath(theme_path) def load_extension(self, app: Sphinx, extname: str) -> None: """Load a Sphinx extension.""" diff --git a/sphinx/theming.py b/sphinx/theming.py index 250b5269d..7f1c53d1e 100644 --- a/sphinx/theming.py +++ b/sphinx/theming.py @@ -6,13 +6,12 @@ __all__ = ('Theme', 'HTMLThemeFactory') import configparser import contextlib -import os import shutil import sys import tempfile import tomllib from importlib.metadata import entry_points -from os import path +from pathlib import Path from typing import TYPE_CHECKING, Any 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.locale import __ from sphinx.util import logging +from sphinx.util._pathlib import _StrPath from sphinx.util.osutil import ensuredir if TYPE_CHECKING: @@ -62,8 +62,8 @@ class Theme: name: str, *, configs: dict[str, _ConfigFile], - paths: list[str], - tmp_dirs: list[str], + paths: list[Path], + tmp_dirs: list[Path], ) -> None: self.name = name self._dirs = tuple(paths) @@ -87,11 +87,11 @@ class Theme: 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, 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: """Return the value for a theme configuration setting, searching the @@ -159,17 +159,17 @@ class HTMLThemeFactory: def _load_builtin_themes(self) -> None: """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(): - self._themes[name] = theme + self._themes[name] = _StrPath(theme) def _load_additional_themes(self, theme_paths: list[str]) -> None: """Load additional themes placed at specified directories.""" 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) for name, theme in themes.items(): - self._themes[name] = theme + self._themes[name] = _StrPath(theme) def _load_entry_point_themes(self) -> None: """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 @staticmethod - def _find_themes(theme_path: str) -> dict[str, str]: + def _find_themes(theme_path: Path) -> dict[str, Path]: """Search themes from specified directory.""" - themes: dict[str, str] = {} - if not path.isdir(theme_path): + themes: dict[str, Path] = {} + if not theme_path.is_dir(): return themes - for entry in os.listdir(theme_path): - pathname = path.join(theme_path, entry) - if path.isfile(pathname) and entry.lower().endswith('.zip'): + for pathname in theme_path.iterdir(): + entry = pathname.name + if pathname.is_file() and pathname.suffix.lower() == '.zip': if _is_archived_theme(pathname): - name = entry[:-4] - themes[name] = pathname + themes[pathname.stem] = pathname else: logger.warning( __( @@ -212,9 +211,9 @@ class HTMLThemeFactory: entry, ) else: - toml_path = path.join(pathname, _THEME_TOML) - conf_path = path.join(pathname, _THEME_CONF) - if path.isfile(toml_path) or path.isfile(conf_path): + toml_path = pathname / _THEME_TOML + conf_path = pathname / _THEME_CONF + if toml_path.is_file() or conf_path.is_file(): themes[entry] = pathname return themes @@ -236,7 +235,7 @@ class HTMLThemeFactory: 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.""" try: with ZipFile(filename) as f: @@ -248,13 +247,13 @@ def _is_archived_theme(filename: str, /) -> bool: def _load_theme_with_ancestors( name: str, - theme_paths: dict[str, str], + theme_paths: dict[str, _StrPath], 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] = {} - theme_dirs: list[str] = [] - tmp_dirs: list[str] = [] + theme_dirs: list[Path] = [] + tmp_dirs: list[Path] = [] # having 10+ theme ancestors is ludicrous for _ in range(10): @@ -287,23 +286,23 @@ def _load_theme_with_ancestors( def _load_theme( - name: str, theme_path: str, / -) -> tuple[str, str, str | None, _ConfigFile]: - if path.isdir(theme_path): + name: str, theme_path: Path, / +) -> tuple[str, Path, Path | None, _ConfigFile]: + if theme_path.is_dir(): # already a directory, do nothing tmp_dir = None theme_dir = theme_path else: # extract the theme to a temp directory - tmp_dir = tempfile.mkdtemp('sxt') - theme_dir = path.join(tmp_dir, name) + tmp_dir = Path(tempfile.mkdtemp('sxt')) + theme_dir = tmp_dir / name _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) inherit = _validate_theme_toml(_cfg_table, name) 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) inherit = _validate_theme_conf(_cfg_parser, name) config = _convert_theme_conf(_cfg_parser) @@ -313,7 +312,7 @@ def _load_theme( 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.""" ensuredir(target_dir) @@ -321,16 +320,13 @@ def _extract_zip(filename: str, target_dir: str, /) -> None: for name in archive.namelist(): if name.endswith('/'): continue - entry = path.join(target_dir, name) - ensuredir(path.dirname(entry)) - with open(path.join(entry), 'wb') as fp: - fp.write(archive.read(name)) + entry = target_dir / name + ensuredir(entry.parent) + entry.write_bytes(archive.read(name)) -def _load_theme_toml(config_file_path: str, /) -> _ThemeToml: - with open(config_file_path, encoding='utf-8') as f: - config_text = f.read() - c = tomllib.loads(config_text) +def _load_theme_toml(config_file_path: Path, /) -> _ThemeToml: + c = tomllib.loads(config_file_path.read_text(encoding='utf-8')) 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.read(config_file_path, encoding='utf-8') return c @@ -487,9 +483,9 @@ def _migrate_conf_to_toml(argv: list[str]) -> int: if len(argv) != 1: print('Usage: python -m sphinx.theming conf_to_toml ') # NoQA: T201 raise SystemExit(1) - theme_dir = path.realpath(argv[0]) - conf_path = path.join(theme_dir, _THEME_CONF) - if not path.isdir(theme_dir) or not path.isfile(conf_path): + theme_dir = Path(argv[0]).resolve() + conf_path = theme_dir / _THEME_CONF + if not theme_dir.is_dir() or not conf_path.is_file(): print( # NoQA: T201 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 ] - toml_path = path.join(theme_dir, _THEME_TOML) - with open(toml_path, 'w', encoding='utf-8') as f: - f.write('\n'.join(toml_lines) + '\n') + toml_path = theme_dir / _THEME_TOML + toml_path.write_text('\n'.join(toml_lines) + '\n', encoding='utf-8') print(f'Written converted settings to {toml_path!r}') # NoQA: T201 return 0 diff --git a/tests/test_theming/test_theming.py b/tests/test_theming/test_theming.py index f0c159a12..afd15f838 100644 --- a/tests/test_theming/test_theming.py +++ b/tests/test_theming/test_theming.py @@ -1,6 +1,5 @@ """Test the Theme class.""" -import os import shutil from pathlib import Path from xml.etree.ElementTree import ParseError @@ -52,11 +51,11 @@ def test_theme_api(app): # test Theme class API 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' ) - assert app.registry.html_themes['ziptheme'] == str(app.srcdir / 'ziptheme.zip') - assert app.registry.html_themes['staticfiles'] == str( + assert app.registry.html_themes['ziptheme'] == (app.srcdir / 'ziptheme.zip') + assert app.registry.html_themes['staticfiles'] == ( app.srcdir / 'test_theme' / 'staticfiles' ) @@ -88,14 +87,14 @@ def test_theme_api(app): # cleanup temp directories 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): # Check that error occurs with a non-existent theme.toml or theme.conf # (https://github.com/sphinx-doc/sphinx/issues/11668) with pytest.raises(ThemeError): - _load_theme('', str(tmp_path)) + _load_theme('', tmp_path) @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(): config_path = HERE / 'theme.toml' - cfg = _load_theme_toml(str(config_path)) + cfg = _load_theme_toml(config_path) config = _convert_theme_toml(cfg) assert config == _ConfigFile( @@ -238,7 +237,7 @@ def test_config_file_toml(): def test_config_file_conf(): config_path = HERE / 'theme.conf' - cfg = _load_theme_conf(str(config_path)) + cfg = _load_theme_conf(config_path) config = _convert_theme_conf(cfg) assert config == _ConfigFile(