Skip to content
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.

Sign up for GitHub

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

Update layout.html to support a sphinx version that is not three-integers #1345

Merged
merged 4 commits into from
Oct 3, 2022
Merged

Update layout.html to support a sphinx version that is not three-integers #1345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bccb999
Update layout.html to support a sphinx version that is not three-inte…
pelson Sep 26, 2022
f9984d1
Apply suggestions from code review
pelson Sep 28, 2022
dc753f1
No reason to only have the first three elements from the sphinx version
pelson Sep 28, 2022
0f1d66e
Merge branch 'master' into fix/sphinx-version-not-three-ints
benjaoming Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions sphinx_rtd_theme/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ def config_initiated(app, config):
_('The canonical_url option is deprecated, use the html_baseurl option from Sphinx instead.')
)


def extend_html_context(app, pagename, templatename, context, doctree):
# Add ``sphinx_version_info`` tuple for use in Jinja templates
context['sphinx_version_info'] = sphinx_version
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: sphinx_version is now just the documented value at https://www.sphinx-doc.org/en/master/extdev/appapi.html#sphinx.version_info.

I could definitely imagine regretting only having the first 3 elements from that object in the future, and it makes no difference to the existing usage of the sphinx_version_info variable (the tuple comparison works the same if it is just the 3 elements, or the full thing)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just to get you correctly: sphinx_version_info is now completely identical to sphinx_version and is just an alias because we cannot know if sphinx_version_info was used by other themes or template customization, right?

Copy link
Collaborator

@agjohnson agjohnson Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mostly certain we'll never need more than major and minor versions here, but the additional specifiers don't seem like they'll hurt anything either. This seems fine, unless we hit something with downstream usage.

This is a place I'd still avoid backwards incompatible changes, so a separate variable seems wise for now.



# See http://www.sphinx-doc.org/en/stable/theming.html#distribute-your-theme-as-a-python-package
def setup(app):
if python_version[0] < 3:
Expand Down Expand Up @@ -60,4 +66,7 @@ def setup(app):
else:
app.config.html_add_permalinks = "\uf0c1"

# Extend the default context when rendering the templates.
app.connect("html-page-context", extend_html_context)
pelson marked this conversation as resolved.
Show resolved Hide resolved

return {'parallel_read_safe': True, 'parallel_write_safe': True}
4 changes: 0 additions & 4 deletions sphinx_rtd_theme/layout.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@
{%- set lang_attr = 'en' if language == None else (language | replace('_', '-')) %}
{%- set sphinx_writer = 'writer-html5' if html5_doctype else 'writer-html4' -%}

{# Build sphinx_version_info tuple from sphinx_version string in pure Jinja #}
{%- set (_ver_major, _ver_minor, _ver_bugfix) = sphinx_version.split('.') | map('int') -%}
{%- set sphinx_version_info = (_ver_major, _ver_minor, _ver_bugfix) -%}

<!DOCTYPE html>
<html class="{{ sphinx_writer }}" lang="{{ lang_attr }}" >
<head>
Expand Down