Add spaces to regex for relational operators.#612
Conversation
Since gt is now interpreted as a string, it was removed from the invalid filters test.
|
I think all the letter-based operators need this space - for example I can type I think you were right to exclude the symbol operators - so |
969a97b to
f29e9bc
Compare
Co-Authored-By: Marc-André Rivet <Marc-Andre-Rivet@users.noreply.github.com>
| export const notEqual: IUnboundedLexeme = R.merge({ | ||
| evaluate: relationalEvaluator(([op, exp]) => op !== exp), | ||
| subType: RelationalOperator.NotEqual, | ||
| regexp: /^(!=|ne)/i |
There was a problem hiding this comment.
ne should also have \s
| ), | ||
| subType: RelationalOperator.Equal, | ||
| regexp: /^(=|eq)/i | ||
| regexp: /^(=|eq\s)/i |
There was a problem hiding this comment.
And the other ones :)
|
Needs changelog entry |
Co-Authored-By: Marc-André Rivet <Marc-Andre-Rivet@users.noreply.github.com>
|
@shammamah The percy diffs are interesting.. looks like there’s an extra space in the filter string now. Possibly a side effect of the new selector? There’s code that destructures/restructures that request from the individual column fragments. I suspect the new space is added somewhere in those steps. |
|
There’s a “transform” on the lexemes that might need to be updated on the relational operators to take the (double?) space into account. |
This reverts commit aae4907.
475d999 to
355be90
Compare
| ), | ||
| subType: RelationalOperator.Contains, | ||
| regexp: /^(contains)/i | ||
| regexp: /^((contains)(?=\s|$))/i, |
There was a problem hiding this comment.
I had some browser support concerns but turns out lookahead is widely supported and not an issue. Lookbehind is not https://caniuse.com/#feat=js-regexp-lookbehind
Marc-Andre-Rivet
left a comment
There was a problem hiding this comment.
💃 Looks good. Good job making this change super concise!
Closes #563.