-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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 RTD to 2.0.0 and fix search #12253
Conversation
Thanks to Balrog, I've been able to test the changes properly. It should be OK to merge. |
# 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: | ||
logger.warning("Python 2 is deprecated with sphinx_rtd_theme, update to Python 3") | ||
app.require_sphinx('1.6') | ||
if sphinx_version <= (2, 0, 0): | ||
logger.warning("Sphinx 1.x is deprecated with sphinx_rtd_theme, update to Sphinx 2.x or greater") | ||
if not app.config.html_experimental_html5_writer: | ||
logger.warning("'html4_writer' is deprecated with sphinx_rtd_theme") | ||
else: | ||
if app.config.html4_writer: | ||
logger.warning("'html4_writer' is deprecated with sphinx_rtd_theme") | ||
logger.error("Python 2 is not supported with sphinx_rtd_theme, update to Python 3.") | ||
|
||
app.require_sphinx('5.0') | ||
if app.config.html4_writer: | ||
logger.error("'html4_writer' is not supported with sphinx_rtd_theme.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- the URL is outdated, resolves to 404;
- https://github.com/readthedocs/sphinx_rtd_theme diverges greatly, including giving back Dockerfile option for local compiling. So it's not really using
__version__ = '2.0.1alpha1'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a direct pull off their git repo for RTD 2.0.0, no other changes were made. It's safe to say at this point that the RTD 2.0.0 release is a mess at the upstream side of things.
DO NOT MERGE THIS WITHOUT REVIEW
Important: Due to the way that the RTD theme imports the search script (it's different between localhost and online due to ajax handling) I was unable to completely test this. Be ready to back this back out immediately if it breaks in production. It SHOULD work, but I can't 100% guarantee it and it's not something I can easily test..
As for what changed:
Three issues needed to be resolved:
First, Sphinx no longer inherently carries JQuery, so any RTD calls to JQuery broke (e.g. search).
Secondly, RTD 2.0.0 came out back in November, and that's going to ne needed going forward for future Sphinx compatibility.
Unfortunately, RTD 2.0.0 still tries to call out to JQuery for search, as noted in #12181
The best workaround as things stand is to update our RTD to V2.0.0, make necessary adjustments to the theme and download template I created back in 2021, and hold things in this place until the RTD team actually fixes their own stuff upstream. Considering it's been three months since the last commit on their theme repo and no response to the issue filed on the search issue, it could be a while before any kind of fix happens.
I'm not entirely thrilled with the results, but it's better than having no search at all (and potentially a completely broken docs site with future Sphinx versions). I'll take a look at refactoring the download panel template at some point in the near future; the changes I had to make were related to the old version of the template completely breaking with the new theme version.