Drop usage of internal API when conditionally including assets (#198)

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.3.0 → v4.6.0](pre-commit/pre-commit-hooks@v4.3.0...v4.6.0)
- [github.com/mgedmin/check-manifest: 0.48 → 0.49](mgedmin/check-manifest@0.48...0.49)
- [github.com/psf/black: 22.3.0 → 24.8.0](psf/black@22.3.0...24.8.0)
- [github.com/PyCQA/pylint: v2.14.3 → v3.2.6](pylint-dev/pylint@v2.14.3...v3.2.6)

* bump codecov-action to v4

codecov-action v1 is really old, bump to v4.

Signed-off-by: Benjamin Cabé <[email protected]>

* Drop usage of internal API when conditionally including assets

Fixes #197 by reversing the logic of how JS/CSS assets are added to pages.
Rather than adding assets to the app and using undocumented API to *remove* them when not needed, do the exact opposite and only add them to the local html-page-context when building a page.

Signed-off-by: Benjamin Cabé <[email protected]>

* drop support for really old Sphinx versions

Drop the code that was used to support Sphinx version < 1.8, released 6 years ago.

Signed-off-by: Benjamin Cabé <[email protected]>

* pylint: drop usage of bad-continuation

`bad-continuation` has been removed from pylint
See pylint-dev/pylint#3571

Signed-off-by: Benjamin Cabé <[email protected]>

---------

Signed-off-by: Benjamin Cabé <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

.github/workflows/tests.yml

@@ -38,7 +38,7 @@ jobs:
         pytest --cov=sphinx_tabs --cov-report=xml --cov-report=term-missing
     - name: Upload to Codecov
       if: matrix.python-version == '3.10' && github.repository == 'executablebooks/sphinx-tabs'
-      uses: codecov/codecov-action@v1
+      uses: codecov/codecov-action@v4
       with:
         name: sphinx-tabs-pytests-py3.10
         flags: pytests

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 repos:
 
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.3.0
+    rev: v4.6.0
     hooks:
     - id: check-json
     - id: check-yaml
@@ -11,21 +11,21 @@ repos:
         ".xml"
 
   - repo: https://github.com/mgedmin/check-manifest
-    rev: "0.48"
+    rev: "0.49"
     hooks:
     - id: check-manifest
 
   - repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 24.8.0
     hooks:
     - id: black
 
   - repo: https://github.com/PyCQA/pylint
-    rev: v2.14.3
+    rev: v3.2.6
     hooks:
     - id: pylint
       args:
-        - --disable=missing-docstring,similarities,fixme,bad-continuation
+        - --disable=missing-docstring,similarities,fixme
       additional_dependencies:
         - sphinx
         - docutils

pylint.cfg

@@ -2,7 +2,7 @@
 extension-pkg-whitelist=lxml
 
 [MESSAGES CONTROL]
-disable=missing-docstring,similarities,fixme,bad-continuation
+disable=missing-docstring,similarities,fixme
 
 [REPORTS]
 reports=no

setup.py

@@ -25,7 +25,7 @@ def get_version():
     url="https://github.com/executablebooks/sphinx-tabs",
     license="MIT",
     python_requires=">=3.7",
-    install_requires=["sphinx", "pygments", "docutils"],
+    install_requires=["sphinx>=1.8", "pygments", "docutils"],
     extras_require={
         "testing": [
             "coverage",

sphinx_tabs/tabs.py

@@ -14,11 +14,13 @@
 from sphinx.directives.code import CodeBlock
 
 
-FILES = [
+JS_FILES = [
     "tabs.js",
-    "tabs.css",
 ]
 
+CSS_FILES = [
+    "tabs.css",
+]
 
 LEXER_MAP = {}
 for lexer in get_all_lexers():
@@ -295,21 +297,6 @@ def found_tabs_directive(self):
         return self._found
 
 
-def update_config(app, config):
-    """Adds sphinx-tabs CSS and JS asset files"""
-    for path in [Path(path) for path in FILES]:
-        if not config.sphinx_tabs_disable_css_loading and path.suffix == ".css":
-            if "add_css_file" in dir(app):
-                app.add_css_file(path.as_posix())
-            else:
-                app.add_stylesheet(path.as_posix())
-        if path.suffix == ".js":
-            if "add_script_file" in dir(app):
-                app.add_script_file(path.as_posix())
-            else:
-                app.add_js_file(path.as_posix())
-
-
 # pylint: disable=unused-argument
 def update_context(app, pagename, templatename, context, doctree):
     """Remove sphinx-tabs CSS and JS asset files if not used in a page"""
@@ -322,18 +309,12 @@ def update_context(app, pagename, templatename, context, doctree):
     if sphinx.version_info >= (4, 1, 0):
         include_assets_in_all_pages = app.registry.html_assets_policy == "always"
 
-    if not visitor.found_tabs_directive and not include_assets_in_all_pages:
-        paths = [Path("_static") / f for f in FILES]
-        if "css_files" in context:
-            context["css_files"][:] = context["css_files"]
-            for path in paths:
-                if path.suffix == ".css" and path in context["css_files"]:
-                    context["css_files"].remove(path.as_posix())
-        if "script_files" in context:
-            context["script_files"][:] = context["script_files"]
-            for path in paths:
-                if path.suffix == ".js" and path.as_posix() in context["script_files"]:
-                    context["script_files"].remove(path.as_posix())
+    if visitor.found_tabs_directive or include_assets_in_all_pages:
+        if not app.config.sphinx_tabs_disable_css_loading:
+            for css in CSS_FILES:
+                app.add_css_file(css)
+        for js in JS_FILES:
+            app.add_js_file(js)
 
 
 # pylint: enable=unused-argument
@@ -357,7 +338,6 @@ def setup(app):
         "builder-inited",
         (lambda app: app.config.html_static_path.insert(0, static_dir.as_posix())),
     )
-    app.connect("config-inited", update_config)
     app.connect("html-page-context", update_context)
 
     return {

tests/conftest.py

@@ -4,7 +4,7 @@
 from bs4 import BeautifulSoup
 import sphinx
 
-from sphinx_tabs.tabs import FILES
+from sphinx_tabs.tabs import JS_FILES, CSS_FILES
 
 pytest_plugins = "sphinx.testing.fixtures"
 
@@ -152,9 +152,6 @@ def check(
     ):
         content = get_sphinx_app_output(app, buildername, filename, encoding)
 
-        css_assets = [f for f in FILES if f.endswith(".css")]
-        js_assets = [f for f in FILES if f.endswith(".js")]
-
         soup = BeautifulSoup(content, "html.parser")
         stylesheets = soup.find_all("link", {"rel": "stylesheet"}, href=True)
         css_refs = [s["href"] for s in stylesheets]
@@ -165,12 +162,12 @@ def check(
         all_refs = css_refs + js_refs
 
         if cssPresent:
-            css_present = all(any(a in ref for ref in all_refs) for a in css_assets)
+            css_present = all(any(a in ref for ref in all_refs) for a in CSS_FILES)
             assert css_present
         else:
             assert not "sphinx_tabs" in css_refs
         if jsPresent:
-            js_present = all(any(a in ref for ref in js_refs) for a in js_assets)
+            js_present = all(any(a in ref for ref in js_refs) for a in JS_FILES)
             assert js_present
         else:
             assert not "sphinx_tabs" in js_refs