-
-
Notifications
You must be signed in to change notification settings - Fork 26k
DOC Fixes search on webpage #24128
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
DOC Fixes search on webpage #24128
Conversation
Using Sphinx's searchtools.js leads to more HTMP calls because it grabs the context for each search result. The custom |
With |
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.
LGTM
Timing is a bit unfortunate as 1.1.2 was just shipped. Do we find another release manager for 1.1.3 or shall we defer to 1.2?
We can cherry-pick this PR into the |
Trying the search in the CircleCI artifact e.g. https://output.circle-artifacts.com/output/job/41a9e69d-f629-4a96-9d64-ccd07f94afaf/artifacts/0/doc/search.html?q=neighbors there seems to be some weird matches that contains some inline HTML e.g.: If that's more than 15 minutes to fix, I am fine with merging as is, as this is a lot better than a completely broken search. |
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.
If that's more than 15 minutes to fix, I am fine with merging as is, as this is a lot better than a completely broken search.
+1. I think we can inspect the reason of this faulty text generation then.
The incorrect render was a Sphinx bug. I ran the following to update just the doc dependencies: python build_tools/update_environments_and_lock_files.py --select-build doc The search page now renders correctly. |
Thanks a lot, let's merge this one! |
The search now works for the stable version: |
Reference Issues/PRs
Fixes #24127
What does this implement/fix? Explain your changes.
There are many changes to Sphinx's javascript and updating our own
searchtools.js
feel like too much maintenance on us. This PR leverages Sphins'xsearchtools.js
directly to fix search.I'll say we backport this into 1.1.X too so that the live site gets fixed.