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.

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

Documentation search is broken. #1965

Closed
kwsp opened this issue Dec 1, 2023 · 2 comments
Closed

Documentation search is broken. #1965

kwsp opened this issue Dec 1, 2023 · 2 comments

Comments

@kwsp
Copy link
Contributor

kwsp commented Dec 1, 2023

As you may already know, doc search is currently broken. I figured out why.

Sphinx relies on [html_root]/searchindex.js (built along with the static docs) to search. Sphinx used to load this file with jQuery, but now by default uses vanilla JS, and jQuery was removed from Sphinx in ~2022 with the release of Sphinx 6.0.0. However, pydicom uses the sphinx_rtd_theme, which forked the default Sphinx theme a long time ago and still depends on jQuery to load searchindex.js.

For some reason, in our install of Sphinx and sphinx_rtd_theme (as deployed right now and I reproduced locally) doesn't actually come with jQuery, so searchindex.js is never loaded. The easiest, hackiest fix is to simply apply this change to the built artifact doc/_build/html/search.html

  <!-- Comment out the jQuery call -->
  <!--<script>-->
    <!--jQuery(function() { Search.loadIndex("searchindex.js"); });-->
  <!--</script>-->

  <!-- Load with vanilla JS -->
  <script src="searchindex.js" defer=""></script>

Ultimately I think we might just go with a hacky fix for now, and a script to do this is pretty simple since searchindex.js is guaranteed to be generated in the root directory (of the docs build).
I really don't want to "fix" this issue because that involves us adding jQuery to the docs... I'd like sphinx_rtd_theme to remove their dependency on jQuery upstream (let's be real who needs jQuery when you have ES6?)

@kwsp
Copy link
Contributor Author

kwsp commented Dec 1, 2023

Trying to fix it upstream readthedocs/sphinx_rtd_theme#1546

@mrbean-bremen
Copy link
Member

Thanks for the catch and the quick investigation! Indeed an upstream fix would be the preferred way. Let's wait a bit what the maintainers of the theme are going to say.

kwsp added a commit to kwsp/pydicom that referenced this issue Dec 1, 2023
@kwsp kwsp mentioned this issue Dec 1, 2023
7 tasks
darcymason pushed a commit that referenced this issue Dec 2, 2023
* quickfix for doc search #1965

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Update make.bat

Changed spaces to tabs for consistency

* release note

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@kwsp kwsp closed this as completed Dec 2, 2023
darcymason added a commit that referenced this issue Dec 3, 2023
* quickfix for doc search #1965

* Update make.bat

Changed spaces to tabs for consistency

* release note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants