-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
search: order non-main index entries after other results #11696
Conversation
Since commit 8ae8183 (Support searching for index entries (sphinx-doc#10819), 2022-09-20, v5.2.0~16), index entries are returned as search results. When the query string exactly matches an indexed term, all index entries become search results with score 100. This places them above most other search results. Reporting "main" index entries early in the search results makes sense, but non-main index entries are often just arbitrary cross-references to the indexed term. Collect them in a separate group that is always placed after other results. This avoids obscuring the main entries and other more-important matches such as document titles. Fixes: sphinx-doc#11578
703aa9b
to
dee464c
Compare
I added a merge commit to resolve conflicts with |
I think it's fine since we squash the commits at the end. |
I'll review it by the end of this weekend. Seems fine for me though (but @AA-Turner should review as well). By the way, could the OP confirm that this PR will not break existing project? Like, is it ensured that nonmain index entries are really irrelevant compared to main ones in general? |
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.
For me it's fine. But could I know its real effect on existing projects such as the Python docs (compared to the original issue where we introduced this search). Also, did it actually fix the CMake documentation?
AFAICT the original motivation was only for main entries. #6692 requests support for the "index directive". Review of #10819 did not raise the distinction.
Yes. That's what's motivating this PR. |
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.
Ah sorry! as I said on another post, I am quite unable to review anything. However, if everything works well for both CMake and Python, then I have nothing more to
By the way, I don't have any write access and we usually wait for Adam's reviews before merging (sometimes, I miss obvious issues) but since they're also busy, I don't know when this will be reviewed.
@picnixz @AA-Turner I've updated this PR to resolve conflicts with master. Please review again. |
Errr.. for me it's fine? I mean, I don't think there has been any difference with what I've already reviewed and the fact that no changes should occur with CMake and Python is a good indication. Since I'm only passing by, I don't want to merge now (if there's an issue afterwards, I won't be able to react fast enough) so I'll let @AA-Turner have a look as well. |
I think it's better if we don't wait too long for that one. I'll endorse the merge (just resolve the conflicts and we're good to go). |
Thank you! |
Since #10819, index entries are returned as search results. When the query string exactly matches an indexed term, all index entries become search results with score 100. This places them above most other search results.
Reporting "main" index entries early in the search results makes sense, but non-main index entries are often just arbitrary cross-references to the indexed term. Collect them in a separate group that is always placed after other results. This avoids obscuring the main entries and other more-important matches such as document titles.
Fixes: #11578
Supersedes: #11695