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

Fix #8885: html: AttributeError for CSS/JS files on html_context #8893

Merged
merged 2 commits into from
Feb 16, 2021
Merged

Fix #8885: html: AttributeError for CSS/JS files on html_context #8893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8205283
refactor: html: Fix typo
tk0miya Feb 15, 2021
82501a6
Fix #8885: html: AttributeError for CSS/JS files on html_context
tk0miya Feb 15, 2021
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
2 changes: 2 additions & 0 deletions CHANGES
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Bugs fixed
----------

* #8884: html: minified js stemmers not included in the distributed package
* #8885: html: AttributeError is raised if CSS/JS files are installed via
:confval:`html_context`
* #8880: viewcode: ExtensionError is raised on incremental build after
unparsable python module found

Expand Down
16 changes: 14 additions & 2 deletions sphinx/builders/html/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,8 +1035,20 @@ def hasdoc(name: str) -> bool:
templatename = newtmpl

# sort JS/CSS before rendering HTML
tk0miya marked this conversation as resolved.
Show resolved Hide resolved
ctx['script_files'].sort(key=lambda js: js.priority)
ctx['css_files'].sort(key=lambda js: js.priority)
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI Python 3 has this nice pattern for such cases:

Suggested change
try:
with contextlib.suppress(AttributeError):

# Convert script_files to list to support non-list script_files (refs: #8889)
ctx['script_files'] = sorted(list(ctx['script_files']), key=lambda js: js.priority)
except AttributeError:
tk0miya marked this conversation as resolved.
Show resolved Hide resolved
# Skip sorting if users modifies script_files directly (maybe via `html_context`).
# refs: #8885
#
# Note: priority sorting feature will not work in this case.
pass
Copy link
Member Author

Choose a reason for hiding this comment

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

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.


try:
ctx['css_files'] = sorted(list(ctx['css_files']), key=lambda css: css.priority)
except AttributeError:
pass

try:
output = self.templates.render(templatename, ctx)
Expand Down