[ENH] Use Sphinx HTML assets policy to decide whether include assets #127
Conversation
|
Thanks for submitting your first pull request! You are awesome! 🤗 |
humitos
force-pushed
the
humitos/use-assets-policy
branch
from
7ae1b10 to
1b303ea
Compare
humitos
changed the title
humitos
force-pushed
the
humitos/use-assets-policy
branch
from
1b303ea to
8f68aaf
Compare
…sets Sphinx v4.1.0 will include `Sphinx.registry.html_assets_policy` that defines the policy to whether include or not HTML assets in pages where the extension is not used. This PR makes usage of this policy to decide if sphinx-tabs assets should be included or not in the page that's being rendered. Reference: sphinx-doc/sphinx#9115
| sphinx.version_info[:2] >= (4, 1), reason="Test uses Sphinx 4.1 config" | ||
| ) | ||
| def test_conditional_assets(app, docname, check_asset_links): | ||
| check_asset_links(app, filename=docname + ".html") |
There was a problem hiding this comment.
Test is currently failing - I think it should use the new sphinx API?
| check_asset_links(app, filename=docname + ".html") | |
| app.registry.html_assets_policy == "always" | |
| check_asset_links(app, filename=docname + ".html") |
There was a problem hiding this comment.
Yes. I updated this test.
However, it kept failing because it seems the output is firstly generated by test_conditional_assets test (without app.set_html_assets_policy("always")) and the result is cached.
What should I do here? Create a new testroot= equal to conditionalassets?
NOTE: in an extension that I'm a maintainer, I'm cleaning the output's path after the test is ran to avoid this problem, see https://github.com/readthedocs/sphinx-hoverxref/blob/master/tests/conftest.py#L9 --maybe something like that is a good approach here as well?
There was a problem hiding this comment.
Thanks, that looks spot on.
It looks like this is passing the CI tests now. I'm surprised though, given the issue that you describe using the same testroot. Is this failing locally for you?
If yes, please do create a separate root folder for this test. I like your fixture for cleaning up after tests, but do find the test outputs useful for debugging when the tests have failed (locally).
pre-commit is picking up on a couple of minor formatting issues below
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
==========================================
- Coverage 92.55% 92.23% -0.33%
==========================================
Files 2 2
Lines 215 219 +4
==========================================
+ Hits 199 202 +3
- Misses 16 17 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Thank @humitos, shall get the next version released including this change 😄 |
|
Thanks, @foster999 for continuing this work, and I'm sorry I wasn't able to make the latest required changes. I was on vacation 😃 |
|
No problem at all, hope you had a nice break! 😁 |
Sphinx v4.1.0 will include
Sphinx.registry.html_assets_policythat defines thepolicy to whether include or not HTML assets in pages where the extension is not
used.
This PR makes usage of this policy to decide if sphinx-tabs assets should be
included or not in the page that's being rendered.
Reference: sphinx-doc/sphinx#9115