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

bpo-34398: Allow glossary results to show up on search page #8773

Merged
merged 5 commits into from Dec 18, 2020

Conversation

ammaraskar
Copy link
Member

@ammaraskar ammaraskar commented Aug 15, 2018

Here are some screenshots of what it looks like:

2018-08-15-09-44-59_scrot
2018-08-15-09-46-01_scrot

https://bugs.python.org/issue34398

@ammaraskar
Copy link
Member Author

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:

glossary1

@JulienPalard
Copy link
Member

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 /search.html?q=lbyl&check_keywords=yes&area=default show a link to EAFP, which leads to /search.html?q=lbyl&check_keywords=yes&area=default#term-eafp which is wrong.

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:

  • for "str" I give /library/stdtypes.html#str, sphinx gives /library/email.headerregistry.html?highlight=str#module-email.headerregistry
  • for "re.A" I give /library/re.html#re.A, sphinx gives /library/re.html?highlight=re#module-re
  • for "datetime.datetime" I give /library/datetime.html#datetime.datetime, sphinx gives /library/datetime.html?highlight=datetime%20datetime#module-datetime

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.

@ammaraskar
Copy link
Member Author

Thanks :)

It's a bit hard to put it as the first result because Sphinx's own search code destructively changes the #search-results.search element as the search is in progress.

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.

@JulienPalard
Copy link
Member

I'm currently testing it locally, could you please write a little news entry about it?

@JulienPalard
Copy link
Member

@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.

@matrixise
Copy link
Member

@JulienPalard update?

@JulienPalard
Copy link
Member

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.

@ammaraskar
Copy link
Member Author

ammaraskar commented Sep 16, 2019

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.

@JulienPalard
Copy link
Member

@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?

@ammaraskar
Copy link
Member Author

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/

@JulienPalard JulienPalard merged commit 8c5d034 into python:master Dec 18, 2020
@JulienPalard
Copy link
Member

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/

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants