Remove deprecations for sphinx 1.8 #57
Conversation
Getting ready for sphinx 2.0
|
We still have access to |
|
Also, I was thinking in removing this whole block It's only used to support |
|
Ok, the deprecation from tests are bc of python 2, sad |
This reverts commit 5881848.
|
And we need to consider this #35 (comment), but as I mentioned above, this is only needed for sphinx_rtd_theme < 4.0 (2013) |
| if sphinx.version_info < (1, 8): | ||
| app.add_stylesheet(theme_css) | ||
| else: | ||
| app.add_css_file(theme_css) |
There was a problem hiding this comment.
This may have a different effect since we were adding our .css file as the first element of the list.
So, all the css files added after our could override our own css classes. I'm not sure at what point add_stylesheet and/or add_css_file will add them.
In any case, we should check if inserting it at 0 position is something we really need.
There was a problem hiding this comment.
I saying this because I think the order of the list is the order in which the css files are loaded in the HTML, but I'm not sure either.
There was a problem hiding this comment.
We don't need that for recent projects, users can always pin the extension
There was a problem hiding this comment.
So, this is a breaking change, right?
If so, we won't be able to upgrade our default version installed by RTD.
There was a problem hiding this comment.
It's breaking only for really old projects (sphinx_rtd_theme < 4.0, 2013), and supposing that they haven't updated the theme (they have the theme pinned to a version less than 4.0). And if they have a custom css rule that could interfere with the theme. I'm not really so worry about this change.
There was a problem hiding this comment.
We don't need that for recent projects, users can always pin the extension
Big -1 here. Users should never need to know about our extension, asking them to pin it is definitely terrible UX :)
readthedocs_ext/readthedocs.py
Outdated
| ) | ||
| js_file = '{}javascript/readthedocs-doc-embed.js'.format(STATIC_URL) | ||
| if sphinx.version_info < (1, 8): | ||
| app.add_javascript(js_file) |
There was a problem hiding this comment.
I'd like to probably keep the logic the same as the existing logic, since there could be subtle changes in how this works across Sphinx versions.
There was a problem hiding this comment.
I've changed it, but we have the deprecation from 1.7 actually, https://www.sphinx-doc.org/en/1.7/extdev/appapi.html#sphinx.application.Sphinx.add_javascript was added in 0.5
readthedocs_ext/readthedocs.py
Outdated
| @@ -90,7 +91,10 @@ def update_body(app, pagename, templatename, context, doctree): | |||
| pass | |||
|
|
|||
| if inject_css and theme_css not in app.builder.css_files: | |||
| app.builder.css_files.insert(0, theme_css) | |||
| if sphinx.version_info < (1, 8): | |||
| app.add_stylesheet(theme_css) | |||
There was a problem hiding this comment.
Same here, I don't see a reason to change this logic.
Getting ready for sphinx 2.0
Closes #35