search

bripkens / lucene

Node.js lib to transform: lucene query → syntax tree → lucene query
MIT License
72 stars 33 forks source link

support no space before operator and left paren #15

Closed ghost closed 6 years ago

ghost commented 6 years ago 
NOT(text:go)
coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.1%) to 78.582% when pulling 99c376165430206db14a876606f5c7d3610b9bc5 on uxnow:feature-nospace-between-operator-and-left-paren into c51f4e08c00e3b7aaa81d3105874c18ded247043 on bripkens:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.1%) to 78.582% when pulling 99c376165430206db14a876606f5c7d3610b9bc5 on uxnow:feature-nospace-between-operator-and-left-paren into c51f4e08c00e3b7aaa81d3105874c18ded247043 on bripkens:master.

bripkens commented 6 years ago

Your tests are not testing what you are describing in the message of your PR. Also, could you please also take care that the AST can be turned into the same string again via toString?

ghost commented 6 years ago

ok, I will look into the tests

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.04%) to 79.764% when pulling 932e792560bf40217c1591af3117e1f8ed2808a1 on uxnow:feature-nospace-between-operator-and-left-paren into c51f4e08c00e3b7aaa81d3105874c18ded247043 on bripkens:master.

ghost commented 6 years ago

Is It better to convert NOT(text:go) to NOT (text:go)?

bripkens commented 6 years ago

it should preferably be exactly (or very closely) what the user typed in. Meaning in this case, it there is a space, then we should retain the space character.

ghost commented 6 years ago

I see, thank you for your explanation which reminds me some good ideas. I will close this pull request first, thank you again.