-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
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
bpo-34398: Allow glossary results to show up on search page #8773
Conversation
4b67840
to
41a7b38
Compare
41a7b38
to
826ac81
Compare
Revived thanks to @csabella. As Berker requested, just updated to include the title of the glossary item in the result. Poke @berkerpeksag. Also @JulienPalard, Cheryl identified you as a potential reviewer as the docs expert. Here is what it looks like now: |
Hi! Thanks for this contributions, I like it! I have something like this in the back of my mind for a few time, but didn't take time trying to implement it. Found an issue: Links from the glossary definition does not work, typically Idea: Why not displaying this as a normal search result, but in the first position? Like: It would open the possibility to enhance this nice piece of code to give more exact matches than the glossary, like exact matches out of the whole index, a bit like I do with https://github.com/JulienPalard/pydocsearch (I use it as an IRC bot on #python-fr on freenode to rapidly point users to part of the doc and it works really well). pydocsearch works better than the sphinx search for exact matches, a few examples:
I'm not asking you to merge my pydocsearch in your PR, it's another story, but if you could display the result in a way that would allow future enhancements like using pydocsearch (or something like this), it would be probably be great. It does not necessarily mean displaying the search result as a regular one, but at least provide a link to the place where it was found in the documentation. |
Thanks :) It's a bit hard to put it as the first result because Sphinx's own search code destructively changes the However, adding in a link to the item is a very good idea. This is done as well as fixing the local links you talked about above. |
I'm currently testing it locally, could you please write a little news entry about it? |
@ammaraskar Hi! local tests are working great! I'm really not a fan of the current fix for links, so I tried hard to do better, but I failed. There may be a way, still, but I'm not fluent enough with docutils ^^ Anyway, doing my research I thought about a thing: Why not pushing this directly to sphinx? As glossary is not a Python specific thing, but a sphinx thing, every sphinx projets could benefit from it. |
@JulienPalard update? |
I'm still strugling to find a less specific fix for this issue, I recently opened sphinx-doc/sphinx#6692 to gather some feedback about a similar idea, to fix it sphinx-side. |
I agree with Julien, let's try to improve search upstream in Sphinx. If there's not much interest within a few months, a CPython specific solution might be better. |
3c046be
to
d9cb1da
Compare
@ammaraskar No much move Sphinx side, I tested your code again, and rebased it on top of master (there were conflicts). I think this could be merged. We'll still be able to remove it later if it become part of Sphine one way or another. We could also move the other way around by trying to produce more "infobox" than just the glossary, but that's a whole world, clearly for a distinct PR. What do you think? |
Thanks for the ping, Julien. I am good with merging this as-is. Long term, we might consider switching our search to use something like Algolia like requests does here: https://requests.readthedocs.io/en/master/ |
Thanks @ammaraskar for the PR! Took quiet some time to be merged þ And it works: https://docs.python.org/dev/search.html?q=coroutine&check_keywords=yes&area=default \o/ |
Here are some screenshots of what it looks like:
https://bugs.python.org/issue34398