Skip to content

Conversation

@wlach
Copy link
Contributor

@wlach wlach commented Nov 24, 2023

Subject: Fix exact matching on title when there are leading/trailing spaces

Feature or Bugfix

  • Bugfix

Purpose

When there is a search term with whitespace (e.g. Superman), the override to boost entries that match on the title (e.g. Superman) doesn't work properly. This PR fixes that.

Detail

N/A

Relates

No ticket filed for this issue, just noticed it.

When there is a search term with whitespace (e.g. ` Superman`), the
override to boost entries that match on the title (e.g. `Superman`)
didn't work.
@wlach
Copy link
Contributor Author

wlach commented Dec 18, 2023

@AA-Turner sorry for the direct ping, but would you mind reviewing this? I see you've reviewed and written code with this file before. It's a very simple fix and I think low risk.

@picnixz
Copy link
Member

picnixz commented Dec 24, 2023

I don't think there is anything wrong with that but to be sure, do we actually assume that spaces are important? Or is it possible to keep the spaces when we use quotes? the search we provide is not that good because there are many results that are not always the best ones but I always forget whether we support quotes or not...

Btw, Adam is preetty busy and me as well. The project has been stalled for some time but we expect some changes after New Year's Eve. I personally won't be available until mid Feb so don't expect too much from me.

@wlach
Copy link
Contributor Author

wlach commented Dec 24, 2023

I don't think there is anything wrong with that but to be sure, do we actually assume that spaces are important? Or is it possible to keep the spaces when we use quotes? the search we provide is not that good because there are many results that are not always the best ones but I always forget whether we support quotes or not...

I don't see any sign the search code handles quoted exact matches.

The existing code trims forward/trailing spaces when not matching the title and tokenizing the terms passed in, so this is just making the title exact match subcase (which was fairly recently added) match its behaviour:

const objectTerms = new Set(splitQuery(query.toLowerCase().trim()));

Btw, Adam is preetty busy and me as well. The project has been stalled for some time but we expect some changes after New Year's Eve. I personally won't be available until mid Feb so don't expect too much from me.

No problem, thanks for the heads up!

@AA-Turner AA-Turner changed the title Fix exact matching on title when there are leading/trailing spaces HTML search: Fix exact matching on titles with extra whitespace Dec 26, 2023
@AA-Turner AA-Turner merged commit 6e18d06 into sphinx-doc:master Dec 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2024
@wlach wlach deleted the search-title-whitespace branch February 4, 2024 18:50
@AA-Turner AA-Turner added this to the 7.3.0 milestone Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants