Properly set configuration with app.builder.theme_options (#1199)

* Properly set configuration

* Dict to values

* Making it explicit in a function

* Better name

* Fix test

* Foot

* Revert complex config set

* Clarify docs

docs/community/topics/config.md

@@ -0,0 +1,30 @@
+# Update Sphinx configuration during the build
+
+Sometimes you want to update configuration values _during a build_.
+For example, if you want to set a default if the user hasn't provided a value, or if you want to move the value from one keyword to another for a deprecation.
+
+Here are some tips to do this the "right" way in Sphinx.
+
+## Update config: use `app.config`
+
+For example, `app.config.foo = "bar"`.
+For some reason, when Sphinx sets things it directly uses `__dict__` but this doesn't seem to be different from the pattern described here.
+
+## Update theme options: use `app.builder.theme_options`
+
+For example, `app.builder.theme_options["logo"] = {"text": "Foo"}`.
+
+## Check if a user has provided a default: `app.config._raw_config`
+
+The `app.config._raw_config` attribute contains all of the **user-provided values**.
+Use this if you want to check whether somebody has manually specified something.
+For example, `"somekey" in app.config._raw_config` will be `False` if a user has _not_ provided that option.
+
+You can also check `app.config.overrides` for any CLI-provided overrides.
+
+We bundle both checks in a helper function called `_config_provided_by_user`.
+
+## Avoid the `config-inited` event
+
+This theme is activated **after** `config-inited` is triggered, so if you write an event that depends on it in this theme, then it will never occur.
+The earliest event you can use is `builder-inited`.

src/pydata_sphinx_theme/__init__.py

@@ -32,8 +32,18 @@
 logger = logging.getLogger(__name__)
 
 
+def _config_provided_by_user(app, key):
+    """Check if the user has manually provided the config."""
+    return any(key in ii for ii in [app.config.overrides, app.config._raw_config])
+
+
 def update_config(app):
-    theme_options = app.config.html_theme_options
+    """Update config with new default values and handle deprecated keys."""
+    # By the time `builder-inited` happens, `app.builder.theme_options` already exists.
+    # At this point, modifying app.config.html_theme_options will NOT update the
+    # page's HTML context (e.g. in jinja, `theme_keyword`).
+    # To do this, you must manually modify `app.builder.theme_options`.
+    theme_options = app.builder.theme_options
 
     # TODO: deprecation; remove after 0.14 release
     if theme_options.get("logo_text"):
@@ -68,9 +78,9 @@ def update_config(app):
             f"type {type(theme_options.get('icon_links'))}."
         )
 
-    # Update the anchor link (it's a tuple, so need to overwrite the whole thing)
-    icon_default = app.config.values["html_permalinks_icon"]
-    app.config.values["html_permalinks_icon"] = ("#", *icon_default[1:])
+    # Set the anchor link default to be # if the user hasn't provided their own
+    if not _config_provided_by_user(app, "html_permalinks_icon"):
+        app.config.html_permalinks_icon = "#"
 
     # Raise a warning for a deprecated theme switcher config
     # TODO: deprecation; remove after 0.13 release
@@ -156,30 +166,19 @@ def update_config(app):
             app.add_js_file(None, body=gid_script)
 
     # Update ABlog configuration default if present
-    if "ablog" in app.config.extensions:
-        app.config.__dict__["fontawesome_included"] = True
-
-
-def prepare_html_config(app, pagename, templatename, context, doctree):
-    """Prepare some configuration values for the HTML build.
-
-    For some reason updating the html_theme_options in an earlier Sphinx
-    event doesn't seem to update the values in context, so we manually update
-    it here with our config.
-    """
+    if "ablog" in app.config.extensions and not _config_provided_by_user(
+        app, "fontawesome_included"
+    ):
+        app.config.fontawesome_included = True
 
     # Prepare the logo config dictionary
-    theme_logo = context.get("theme_logo")
+    theme_logo = theme_options.get("logo")
     if not theme_logo:
         # In case theme_logo is an empty string
         theme_logo = {}
     if not isinstance(theme_logo, dict):
         raise ValueError(f"Incorrect logo config type: {type(theme_logo)}")
-
-    context["theme_logo"] = theme_logo
-
-    # update version number
-    context["theme_version"] = __version__
+    theme_options["logo"] = theme_logo
 
 
 def update_and_remove_templates(app, pagename, templatename, context, doctree):
@@ -261,6 +260,9 @@ def _remove_empty_templates(tname):
         """
         app.add_js_file(None, body=js)
 
+    # Update version number for the "made with version..." component
+    context["theme_version"] = __version__
+
 
 def add_inline_math(node):
     """Render a node with HTML tags that activate MathJax processing.
@@ -903,7 +905,7 @@ def _overwrite_pygments_css(app, exception=None):
         style_key = f"pygment_{light_or_dark}_style"
 
         # globalcontext sometimes doesn't exist so this ensures we do not error
-        theme_name = app.config.html_theme_options.get(style_key, None)
+        theme_name = app.builder.theme_options.get(style_key, None)
         if theme_name is None and hasattr(app.builder, "globalcontext"):
             theme_name = app.builder.globalcontext.get(f"theme_{style_key}")
 
@@ -1120,7 +1122,7 @@ def copy_logo_images(app: Sphinx, exception=None) -> None:
     If logo image paths are given, copy them to the `_static` folder
     Then we can link to them directly in an html_page_context event
     """
-    theme_options = app.config.html_theme_options
+    theme_options = app.builder.theme_options
     logo = theme_options.get("logo", {})
     staticdir = Path(app.builder.outdir) / "_static"
     for kind in ["light", "dark"]:
@@ -1153,7 +1155,6 @@ def setup(app):
     app.connect("builder-inited", update_config)
     app.connect("html-page-context", setup_edit_url)
     app.connect("html-page-context", add_toctree_functions)
-    app.connect("html-page-context", prepare_html_config)
     app.connect("html-page-context", update_and_remove_templates)
     app.connect("html-page-context", setup_logo_path)
     app.connect("build-finished", _overwrite_pygments_css)

tests/sites/deprecated/conf.py

@@ -26,7 +26,7 @@
         "edit-this-page.html",
         "sourcelink.html",
     ],
-    "footer_items": ["test.html"],
+    "footer_items": ["page-toc.html"],
 }
 
 html_sidebars = {"section1/index": ["sidebar-nav-bs.html"]}