-
Notifications
You must be signed in to change notification settings - Fork 7
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
Whitelisting conditions #19
Comments
@braco sorry for the delay. Thanks for the kind words and advice! I think that currently works by wrapping that text in quotes e.g.:
Then we do not strip out the Does that make sense? |
That does make sense, but I think it's asking too much of the user. It would depend on the application, but in this particular one and Google who normalized this parameter style, these are advanced/discoverable features. I built a wrapper that tries to restore the original query text, but the word order can't be guaranteed, so it's not ideal. Without doing that, words would just fall into a black hole. |
Thinking about it, I guess an ignoreEmptyParameters: true feature would help if nothing else. That way "param: " would be parsed as regular text...? |
@braco sorry not totally following. Mind adding a PR with some tests that show the behavior you're looking for? |
Great job on this library!
One thing: it seems important to support a whitelist of conditions. What if I wanted to search for text that had colons in it? Let's say a book is called "Foobar: The story of Steve", and a user tries to search for that phrase? You'll currently strip Foobar: out of the text.
Most query DSLs will have a specific set of conditions that are valid.
search-query-parser
shows their usage here. It would make sense to ignore anything else, imo.I guess you'd need a breaking change that looked something like this:
parse(string, { config })
The text was updated successfully, but these errors were encountered: