New Shorthands #8

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants

tyok commented Sep 19, 2011

In regard to #7. Please give your thought on this.

Owner

igrigorik commented Sep 20, 2011

Hi Mohammad. The implementation itself makes sense, and as you highlighted in the ticket.. extending the grammar is pretty straight forward. If this is a syntax extension that you need for your project, then it definitely makes sense.

Having said that, I'm a little hesitant to merge this because adding / and & into the syntax also means you have to be much more careful in your queries - these characters become reserved, and we don't really provide an escape mechanism. Meaning, if someone actually has a string like "a & b" then they'll run into issues. The idea behind explicitly defining upcase variants was to minimize these collisions (AND, and OR are less likely collision candidates).

tyok commented Sep 20, 2011

Thanks for your feedback. I tried to play with such queries, and it turns out the the parser implementation is quite robust. I can escape a logical keyword by wrapping it around quotes:

TextQuery.new("a 'OR' b").match? "a"
=> false

TextQuery.new("a 'OR' b").match? "a OR b"
=> true

For my needs, this is sufficient. Might not for yours, though. You might still be interested in the behavior shown in my spec above, since it should be present in your master branch too. Maybe you want to document, exploit, or fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment