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

Bug/773 search bar boolean matching #776

Conversation

chandlerprall
Copy link
Contributor

@chandlerprall chandlerprall commented May 4, 2018

The search bar's grammar greedily prefers boolean fragments over word fragments, but did not verify that the boolean text it matched again didn't exist within a larger word structure.

e.g. true truest off and offer all match the boolean type because they start with a valid boolean expression.

This PR modifies the containsValue expression to look for boolean followed by whitespace or End Of Input, otherwise containsValue continues to match against word.

true, off match as boolean
truest, offer match as word

Fixes #773

@cjcenizal
Copy link
Contributor

Is there a way to update our examples to verify that this bug is fixed? And can we protect against regressions with a test or two?

@chandlerprall
Copy link
Contributor Author

chandlerprall commented May 4, 2018

Is there a way to update our examples to verify that this bug is fixed?

The public interface on this component does not expose the correct information (bool vs. word term), I had to add some console.logs within the syntax parsing itself.

And can we protect against regressions with a test or two?

None of the internals are exported in a way to test, I'm going to tackle tests on this next week.

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

🔎 Nice fix!

@chandlerprall chandlerprall merged commit b29bbea into elastic:master May 7, 2018
@chandlerprall chandlerprall deleted the bug/773-search-bar-boolean-matching branch May 7, 2018 15:57
@snide snide added the bug label May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants