-
-
Notifications
You must be signed in to change notification settings - Fork 386
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
Simplify FuzzySearch test (avoid dependency on /usr/share/dict/words) #4531
Conversation
@jhrcek Thanks for taking care of this! You should only have to add this in |
Didn't expect that 😯 Welp, so sounds like the reference impl is not the same as the FuzzySearch impl... |
I can reproduce that failure locally Seems to fail quite often and every time there seems to be uppercase letters involved. I don't know how it's supposed to work, but the main implementation seems to expect that the first parameter consists of lowercase letters (with possible exception of the first letter):
but there are many words in the words file that have uppercase letters.. |
Now it's clear that reference impl. doesn't agree with the parallel impl.
Sometimes reference doesn't return any matches:
|
That's probably expected, because the "parallel" implementation cuts corners with regards to non-ASCII characters.
These violate a precondition that only the first character might be upper case. |
@Bodigrim you seem to have some context on this 😸 (EDIT: I see you implemented the
|
I'd put a few unit tests to check that HLS fuzzy search can order a few alternatives in a reasonable way, but that's it. The reference implementation is apparently fragile as are additional system dependencies. Ditch them, there is bigger fish to fry. |
2e89ac8
to
cb52c79
Compare
I removed all the stuff using words and replaced if with few unit tests + a small property test. |
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.
LGTM, thanks!
Trying to address #4530
Saw this in Haskell Weekly newsletter 😄