-
Notifications
You must be signed in to change notification settings - Fork 348
Standardize logo image behavior between Sphinx and this theme #1132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5a0d347
830274d
d16b676
0907ab9
ab1dc56
ae04024
5d3fff7
a4afb3f
572c877
23d188e
c0f10e9
2a6e737
6590b4f
3a939b3
dda9956
6bf9fcb
a9522a8
62cce7d
4d24c06
b557691
610169e
74d49fa
377d05a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,12 +12,14 @@ | |
from bs4 import BeautifulSoup as bs | ||
from docutils import nodes | ||
from sphinx import addnodes | ||
from sphinx.application import Sphinx | ||
from sphinx.environment.adapters.toctree import TocTree | ||
from sphinx.addnodes import toctree as toctree_node | ||
from sphinx.transforms.post_transforms import SphinxPostTransform | ||
from sphinx.util.nodes import NodeMatcher | ||
from sphinx.errors import ExtensionError | ||
from sphinx.util import logging | ||
from sphinx.util import logging, isurl | ||
from sphinx.util.fileutil import copy_asset_file | ||
from pygments.formatters import HtmlFormatter | ||
from pygments.styles import get_all_styles | ||
import requests | ||
|
@@ -1084,6 +1086,70 @@ def setup_translators(app): | |
app.set_translator(name, translator, override=True) | ||
|
||
|
||
# ------------------------------------------------------------------------------ | ||
# customize events for logo management | ||
# we use one event to copy over custom logo images to _static | ||
# and another even to link them in the html context | ||
# ------------------------------------------------------------------------------ | ||
|
||
|
||
def setup_logo_path( | ||
app: Sphinx, pagename: str, templatename: str, context: dict, doctree: nodes.Node | ||
) -> None: | ||
"""Set up relative paths to logos in our HTML templates. | ||
|
||
In Sphinx, the context["logo"] is a path to the `html_logo` image now in the output | ||
`_static` folder. | ||
|
||
If logo["image_light"] and logo["image_dark"] are given, we must modify them to | ||
follow the same pattern. They have already been copied to the output folder | ||
in the `update_config` event. | ||
""" | ||
|
||
# get information from the context "logo_url" for sphinx>=6, "logo" sphinx<6 | ||
pathto = context.get("pathto") | ||
logo = context.get("logo_url") or context.get("logo") | ||
theme_logo = context.get("theme_logo", {}) | ||
|
||
# Define the final path to logo images in the HTML context | ||
theme_logo["image_relative"] = {} | ||
for kind in ["light", "dark"]: | ||
image_kind_logo = theme_logo.get(f"image_{kind}") | ||
|
||
# If it's a URL the "relative" path is just the URL | ||
# else we need to calculate the relative path to a local file | ||
if image_kind_logo: | ||
if not isurl(image_kind_logo): | ||
image_kind_name = Path(image_kind_logo).name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is too restrictive, in that it doesn't allow paths relative to static. Is stripping the name needed at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. strip the name is essential if you use files from outside the doc folder (i.e. the root of the repo) which would have path like "../../../my_hidden_logo.png". This file needs to end up in the build _static directory eventually |
||
image_kind_logo = pathto(f"_static/{image_kind_name}", resource=True) | ||
theme_logo["image_relative"][kind] = image_kind_logo | ||
|
||
# If there's no custom logo for this kind, just use `html_logo` | ||
# If `logo` is also None, then do not add this key to context. | ||
elif isinstance(logo, str) and len(logo) > 0: | ||
theme_logo["image_relative"][kind] = logo | ||
|
||
# Update our context logo variables with the new image paths | ||
context["theme_logo"] = theme_logo | ||
|
||
|
||
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 | ||
logo = theme_options.get("logo", {}) | ||
staticdir = Path(app.builder.outdir) / "_static" | ||
for kind in ["light", "dark"]: | ||
path_image = logo.get(f"image_{kind}") | ||
if not path_image or isurl(path_image): | ||
continue | ||
if not (Path(app.srcdir) / path_image).exists(): | ||
logger.warning(f"Path to {kind} image logo does not exist: {path_image}") | ||
Comment on lines
+1148
to
+1149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There probably is not a good way to check if the path exists in a theme-supplied static directory? If not, then maybe just skip this warning or demote to INFO or DEBUG? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, should this not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm -1 on changing the warning, as you mentioned people are listening to warnings in their CI/CD it would mean that nobody would realize that the logos are not displayed anymore until someone check visually the logs/website.
yes we should |
||
copy_asset_file(path_image, staticdir) | ||
|
||
|
||
# ----------------------------------------------------------------------------- | ||
|
||
|
||
|
@@ -1101,7 +1167,9 @@ def setup(app): | |
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) | ||
app.connect("build-finished", copy_logo_images) | ||
|
||
# Include component templates | ||
app.config.templates_path.append(str(theme_path / "components")) | ||
|
Uh oh!
There was an error while loading. Please reload this page.