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

Add \t as recognized separator #280

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Apr 15, 2024

Currently \t isn't seen as an recognized separator. This was causing issues for meilisearch, when it was trying to search on a keyword (fuzzy or exact match) and in the document the keyword was present but the character before the keyword was an \t charabia would create a token that was \t<keyword> which in turn led to meilisearch returning the document as part of the search but not returning the positions of matches (_matchesPosition field).

The actual reproducer for this bug was code files of the Linux kernel (such as fs/ext4/readpage.c) which uses tabs for indentation and searching for keywords like while would usually be 'prefixed' by an tab causing the described issue. Making \t a separator fixed this issue.

Currently `\t` isn't seen as an recognized separator. This was causing
issues for meilisearch, when it was trying to search on a
keyword (fuzzy or exact match) and in the document the keyword was
present but the character before the keyword was an `\t` charabia would
create a token that was `\t<keyword>` which in turn led to meilisearch
returning the document as part of the search but not returning the
positions of matches (`_matchesPosition` field).

The actual reproducer for this bug was code files of the Linux
kernel (such as `fs/ext4/readpage.c`) which uses tabs for indentation
and searching for keywords like `while` would usually be 'prefixed' by
an tab causing the described issue. Making `\t` a separator fixed this
issue.
@curquiza curquiza requested a review from ManyTheFish April 16, 2024 08:58
Copy link
Member

@ManyTheFish ManyTheFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes Indeed, you are right,
Thank you for your contribution,
bors merge

Copy link
Contributor

meili-bors bot commented Apr 16, 2024

Build succeeded:

  • tests

@meili-bors meili-bors bot merged commit e3df008 into meilisearch:main Apr 16, 2024
1 check passed
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.

2 participants