Support searchtools override regardless of Sphinx version #26
Conversation
This takes the underlying searchtools.js_t out of the Sphinx distribution path, patches it, and then fills the context and parses it as a templated javascript file. This allows us to remove the script initialization on all versions of this file. The initialization block on searchtools.js_t has not changed in 10 years, so this method should be safe. If the block changes in the future, tests will grab this as we add new versions of Sphinx to our testing. This reorganizes some repetitive code and cleans up a few other pieces as well: File copying is linked to the standard copy_static_files that is run from the builder. Running from an event on build finished was producing files without the template context. This is because build-finished is always triggered, but the static files are not always copied (and therefore did not have the same template context?) Fixes #25 Refs readthedocs/readthedocs.org#2708
agjohnson
force-pushed
the
fix-search-sphinx-15
branch
from
14f7f5f to
0d89eb5
Compare
readthedocs_ext/readthedocs.py
Outdated
|
|
||
| static_override_files = [ | ||
| 'readthedocs-dynamic-include.js_t', | ||
| 'readthedocs-data.js_t', |
There was a problem hiding this comment.
These aren't actually overrides -- it seems we aren't overriding any included JS anymore except for searchtools. Not sure its worth renaming, but feels like old verbage.
There was a problem hiding this comment.
Yeah, Sphinx builders already have copy_static_files and a copy_extra_files, so these are overrides in that they'll clobber anything set up by copy_static_files. I'll think about this a bit more
| yield app | ||
| finally: | ||
| shutil.rmtree('_build') | ||
| os.chdir('../..') |
There was a problem hiding this comment.
We really should just package this up as a package. We have it scattered like 10 places around all our projects.
There was a problem hiding this comment.
There is a sphinx-testing which is this -- some of the utility test functions broken out.
There was a problem hiding this comment.
Aye -- are we doing something different than that, or just not worth adding the dependency?
There was a problem hiding this comment.
I haven't had time to play around with sphinx-testing, i've only known of its existence. It would probably make sense to become acquainted with it and standardize on it. Sphinx core, and to a subset sphinx-testing, have some better patterns for testing doctress/etc.
Also adds Sphinx 1.6b3 testing
|
Okay, renamed some of this with a2355c4 and extended testing support to Sphinx 1.6 beta releases |
Search is broken against on ReadTheDocs. Supposedly, this is now addressed via readthedocs/readthedocs-sphinx-ext#26 Removing version pinning to give it a try
ghost
mentioned this pull request
This takes the underlying
searchtools.js_tout of the Sphinx distributionpath, patches it, and then fills the context and parses it as a templated
javascript file. This allows us to remove the script initialization on all
versions of this file.
The initialization block on
searchtools.js_thas not changed in 10 years, sothis method should be safe. If the block changes in the future, tests will grab
this as we add new versions of Sphinx to our testing.
This reorganizes some repetitive code and cleans up a few other pieces as well:
File copying is linked to the standard
copy_static_filesthat is run fromthe builder. Running from an event on build finished was producing files without
the template context. This is because
build-finishedis always triggered, butthe static files are not always copied (and therefore did not have the same
template context?)
Fixes #25
Refs readthedocs/readthedocs.org#2708