From c57badca714a5592427e220bf3556a64e9e1b4ca Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Fri, 17 Feb 2023 23:53:17 +0100 Subject: [PATCH 1/8] Properly set configuration --- src/pydata_sphinx_theme/__init__.py | 34 ++++++++++++----------------- tests/sites/deprecated/conf.py | 2 +- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index e3f0a1f1b..88dfc281f 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -33,7 +33,12 @@ 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"): @@ -157,29 +162,16 @@ def update_config(app): # 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. - """ + app.config.values["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 +253,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 +898,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 +1115,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"]: @@ -1147,7 +1142,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) diff --git a/tests/sites/deprecated/conf.py b/tests/sites/deprecated/conf.py index 5c41a34db..6d8ce3eed 100644 --- a/tests/sites/deprecated/conf.py +++ b/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"]} From bf5376295872da02b9bf9e4d1d5057b6ead0bf31 Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Sun, 19 Feb 2023 04:07:16 +0100 Subject: [PATCH 2/8] Dict to values --- src/pydata_sphinx_theme/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 88dfc281f..e51d138df 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -74,8 +74,8 @@ def update_config(app): ) # 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:]) + icon_default = app.config.__dict__["html_permalinks_icon"] + app.config.__dict__["html_permalinks_icon"] = ("#", *icon_default[1:]) # Raise a warning for a deprecated theme switcher config # TODO: deprecation; remove after 0.13 release @@ -162,7 +162,7 @@ def update_config(app): # Update ABlog configuration default if present if "ablog" in app.config.extensions: - app.config.values["fontawesome_included"] = True + app.config.__dict__["fontawesome_included"] = True # Prepare the logo config dictionary theme_logo = theme_options.get("logo") From ff854adbcb26e41015077d3a0b65071542f9e6e4 Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Sun, 19 Feb 2023 04:19:52 +0100 Subject: [PATCH 3/8] Making it explicit in a function --- src/pydata_sphinx_theme/__init__.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index e51d138df..44d3cc9f0 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -32,6 +32,10 @@ logger = logging.getLogger(__name__) +def _was_provided_by_user(app, key): + return any(key in ii for ii in [app.config.overrides, app.config._raw_config]) + + def update_config(app): """Update config with new default values and handle deprecated keys.""" # By the time `builder-inited` happens, `app.builder.theme_options` already exists. @@ -73,9 +77,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.__dict__["html_permalinks_icon"] - app.config.__dict__["html_permalinks_icon"] = ("#", *icon_default[1:]) + # Set the anchor link default to be # if the user hasn't provided their own + if not _was_provided_by_user(app, "html_permalinks_icon"): + app.config.__dict__["html_permalinks_icon"] = "#" # Raise a warning for a deprecated theme switcher config # TODO: deprecation; remove after 0.13 release From 67324505872a6f4a012b75abcfaeabf6f09319b1 Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Sun, 19 Feb 2023 04:20:08 +0100 Subject: [PATCH 4/8] Better name --- src/pydata_sphinx_theme/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 44d3cc9f0..ef4587b58 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -32,7 +32,7 @@ logger = logging.getLogger(__name__) -def _was_provided_by_user(app, key): +def _config_provided_by_user(app, key): return any(key in ii for ii in [app.config.overrides, app.config._raw_config]) @@ -78,7 +78,7 @@ def update_config(app): ) # Set the anchor link default to be # if the user hasn't provided their own - if not _was_provided_by_user(app, "html_permalinks_icon"): + if not _config_provided_by_user(app, "html_permalinks_icon"): app.config.__dict__["html_permalinks_icon"] = "#" # Raise a warning for a deprecated theme switcher config From 2facc19d88c411822938dea156f78fea2a7f66c9 Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Sun, 19 Feb 2023 05:00:27 +0100 Subject: [PATCH 5/8] Fix test --- docs/community/topics/config.md | 25 +++++++++++++++++++++++++ src/pydata_sphinx_theme/__init__.py | 20 +++++++++++++++----- 2 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 docs/community/topics/config.md diff --git a/docs/community/topics/config.md b/docs/community/topics/config.md new file mode 100644 index 000000000..4efd9e04b --- /dev/null +++ b/docs/community/topics/config.md @@ -0,0 +1,25 @@ +# 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.__dict__` + +For example, `app.config.__dict__["foot"] = "bar"`. + +Even better, use our provided helper function: + +```python +_set_config_if_not_provided_by_user(app, "foo", "bar") +``` + +## 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. diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index ef4587b58..33536a168 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -32,8 +32,19 @@ logger = logging.getLogger(__name__) -def _config_provided_by_user(app, key): - return any(key in ii for ii in [app.config.overrides, app.config._raw_config]) +def _set_config_if_not_provided_by_user(app, key, value): + """Set a configuration value unless the user has provided their own. + + Sphinx makes it hard to know which config attribute to update, + so just use this function. + """ + # Check if the user has manually provided the config + if any(key in ii for ii in [app.config.overrides, app.config._raw_config]): + return + + # If not, then set the config value + app.config.__dict__[key] = value + return app def update_config(app): @@ -78,8 +89,7 @@ def update_config(app): ) # 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.__dict__["html_permalinks_icon"] = "#" + _set_config_if_not_provided_by_user(app, "html_permalinks_icon", "#") # Raise a warning for a deprecated theme switcher config # TODO: deprecation; remove after 0.13 release @@ -166,7 +176,7 @@ def update_config(app): # Update ABlog configuration default if present if "ablog" in app.config.extensions: - app.config.__dict__["fontawesome_included"] = True + _set_config_if_not_provided_by_user(app, "fontawesome_included", True) # Prepare the logo config dictionary theme_logo = theme_options.get("logo") From 6c26f33d4670ed9048dd09eac2282434aa1a0379 Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Sun, 19 Feb 2023 05:01:27 +0100 Subject: [PATCH 6/8] Foot --- docs/community/topics/config.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/community/topics/config.md b/docs/community/topics/config.md index 4efd9e04b..2618bc5ac 100644 --- a/docs/community/topics/config.md +++ b/docs/community/topics/config.md @@ -7,7 +7,7 @@ Here are some tips to do this the "right" way in Sphinx. ## Update config: use `app.config.__dict__` -For example, `app.config.__dict__["foot"] = "bar"`. +For example, `app.config.__dict__["foo"] = "bar"`. Even better, use our provided helper function: @@ -23,3 +23,4 @@ For example, `app.builder.theme_options["logo"] = {"text": "Foo"}`. 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. From 490d8b9c9a43c4077ed664ada5b23c6717555ff0 Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Sun, 19 Feb 2023 05:44:07 +0100 Subject: [PATCH 7/8] Revert complex config set --- src/pydata_sphinx_theme/__init__.py | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 33536a168..ff4f0a2a2 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -32,19 +32,9 @@ logger = logging.getLogger(__name__) -def _set_config_if_not_provided_by_user(app, key, value): - """Set a configuration value unless the user has provided their own. - - Sphinx makes it hard to know which config attribute to update, - so just use this function. - """ - # Check if the user has manually provided the config - if any(key in ii for ii in [app.config.overrides, app.config._raw_config]): - return - - # If not, then set the config value - app.config.__dict__[key] = value - return app +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): @@ -89,7 +79,8 @@ def update_config(app): ) # Set the anchor link default to be # if the user hasn't provided their own - _set_config_if_not_provided_by_user(app, "html_permalinks_icon", "#") + 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 @@ -175,8 +166,10 @@ def update_config(app): app.add_js_file(None, body=gid_script) # Update ABlog configuration default if present - if "ablog" in app.config.extensions: - _set_config_if_not_provided_by_user(app, "fontawesome_included", True) + 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 = theme_options.get("logo") From 92bd4e25dc1a540b48290ecab71adeefad4e7348 Mon Sep 17 00:00:00 2001 From: Chris Holdgraf Date: Sun, 19 Feb 2023 09:06:20 +0100 Subject: [PATCH 8/8] Clarify docs --- docs/community/topics/config.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/community/topics/config.md b/docs/community/topics/config.md index 2618bc5ac..1f3323d0d 100644 --- a/docs/community/topics/config.md +++ b/docs/community/topics/config.md @@ -5,15 +5,10 @@ For example, if you want to set a default if the user hasn't provided a value, o Here are some tips to do this the "right" way in Sphinx. -## Update config: use `app.config.__dict__` +## Update config: use `app.config` -For example, `app.config.__dict__["foo"] = "bar"`. - -Even better, use our provided helper function: - -```python -_set_config_if_not_provided_by_user(app, "foo", "bar") -``` +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` @@ -24,3 +19,12 @@ For example, `app.builder.theme_options["logo"] = {"text": "Foo"}`. 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`.