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

Whitelisting conditions #19

Open
braco opened this issue Mar 14, 2018 · 4 comments
Open

Whitelisting conditions #19

braco opened this issue Mar 14, 2018 · 4 comments

Comments

@braco
Copy link

braco commented Mar 14, 2018

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 })

@mericsson
Copy link
Contributor

@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.:

const str = 'title:"Foobar: The story of Steve"';
const searchString = SearchString.parse(str);

Then we do not strip out the : as a delimiter. See this repl example.

Does that make sense?

@braco
Copy link
Author

braco commented Mar 21, 2018

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.

@braco
Copy link
Author

braco commented Mar 21, 2018

Thinking about it, I guess an ignoreEmptyParameters: true feature would help if nothing else. That way "param: " would be parsed as regular text...?

@mericsson
Copy link
Contributor

@braco sorry not totally following. Mind adding a PR with some tests that show the behavior you're looking for?

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

No branches or pull requests

2 participants