Fix #8885: html: AttributeError for CSS/JS files on html_context #8893
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tk0miya
force-pushed
the
8885_AttributeError_html_context
branch
from
892fcf3
to
3ae9483
Compare
webknjaz
reviewed
Feb 15, 2021
webknjaz
reviewed
Feb 15, 2021
sphinx/builders/html/__init__.py
Outdated
| ctx['script_files'].sort(key=lambda js: js.priority) | ||
| ctx['css_files'].sort(key=lambda js: js.priority) | ||
| try: | ||
| ctx['script_files'].sort(key=lambda js: js.priority) |
There was a problem hiding this comment.
Unrelated to this PR but you may consider just using something like
Suggested change
| ctx['script_files'].sort(key=lambda js: js.priority) | |
| get_priority = operator.attrgetter('priority') | |
| ctx['script_files'].sort(key=get_priority) |
in the future.
webknjaz
reviewed
Feb 15, 2021
tk0miya
force-pushed
the
8885_AttributeError_html_context
branch
from
5161d87
to
6bcc9c0
Compare
webknjaz
reviewed
Feb 15, 2021
tk0miya
force-pushed
the
8885_AttributeError_html_context
branch
from
6bcc9c0
to
4901324
Compare
…ntext Since 3.5.0, priority has been added to control CSS/JS files. But it's not working if projects installs custom CSS/JS files via `html_context` instead of `html_css_files` and `html_js_files`. This avoids the crash to keep it "not broken".
tk0miya
force-pushed
the
8885_AttributeError_html_context
branch
from
4901324
to
82501a6
Compare
tk0miya
commented
Feb 15, 2021
| # refs: #8885 | ||
| # | ||
| # Note: priority sorting feature will not work in this case. | ||
| pass |
There was a problem hiding this comment.
Finally, I determined to keep the list as is and not emitting a warning. It means priority sort will not work. But no extensions do not use it yet.
And I'll deprecate html_context['*_files]' in another step to migrate to html_*_files.
webknjaz
reviewed
Feb 15, 2021
| @@ -1035,8 +1035,20 @@ def hasdoc(name: str) -> bool: | |||
| templatename = newtmpl | |||
|
|
|||
| # sort JS/CSS before rendering HTML | |||
| ctx['script_files'].sort(key=lambda js: js.priority) | |||
| ctx['css_files'].sort(key=lambda js: js.priority) | |||
| try: | |||
There was a problem hiding this comment.
FYI Python 3 has this nice pattern for such cases:
Suggested change
| try: | |
| with contextlib.suppress(AttributeError): |
jschwab
added a commit
to MESAHub/mesa
that referenced
this pull request
Feb 17, 2021
This adds a css file via the html_css_files option introduced in Sphinx 1.8. Our current use of html_context is broken in Sphinx 3.5.0. It looks like they are going to fix that in 3.5.1 (sphinx-doc/sphinx#8893), but since the modern way seems clearer, I switched to it.
jschwab
added a commit
to MESAHub/mesa
that referenced
this pull request
Feb 17, 2021
This adds a css file via the html_css_files option introduced in Sphinx 1.8. Our current use of html_context is broken in Sphinx 3.5.0. It looks like they are going to fix that in 3.5.1 (sphinx-doc/sphinx#8893), but since the modern way seems clearer, I switched to it.
jschwab
added a commit
to MESAHub/mesa
that referenced
this pull request
Feb 17, 2021
This adds a css file via the html_css_files option introduced in Sphinx 1.8. Our current use of html_context is broken in Sphinx 3.5.0. It looks like they are going to fix that in 3.5.1 (sphinx-doc/sphinx#8893), but since the modern way seems clearer, I switched to it.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature or Bugfix
Purpose
not working if projects installs custom CSS/JS files via
html_contextinstead of
html_css_filesandhtml_js_files. This avoids the crashto keep it "not broken".
AttributeError: 'str' object has no attribute 'priority'in Sphinx 3.5.0 #8885