-
Notifications
You must be signed in to change notification settings - Fork 25
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
Escape backslash #32
base: master
Are you sure you want to change the base?
Escape backslash #32
Conversation
@fehays It seems like this PR's scope is much bigger than simply escaping backslash, no? Is it possible to break it down as I think the escaping backslash is a good addition, but there is probably room for discussing other changes. Thanks |
@jpmonette My mistake. I accidentally committed some other enhancements to this branch that I did not intend for this PR. |
@jpmonette I've reverted the commits and published a new branch in my forked repo with the changes for supporting allowing parenthetical groupings of logical operators (https://github.com/fehays/q/tree/conditional-groups). Let me know if you want to discuss that enhancement |
@fehays Do you mind just adding some simple tests to evaluate scenarios and then we should be good to go. Thanks for the contribution! |
@jpmonette Done. Thanks! |
@fehays Quick update on my previous comment - if you could review. |
@jpmonette Sorry, I may have missed a comment. What would you like me to review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fehays Here!
.add(Q.condition('Name').equalsTo('Acme\\s Carwash')) | ||
.build(); | ||
|
||
System.assertEquals('SELECT Id FROM Account WHERE Name = \'Acme\\\\\\\\s Carwash\'', query); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this is passing tests? Shouldn't it be \'Acme\\\\s Carwash\'
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpmonette Yes, it's passing. It is essentially \'Acme\\\\s Carwash\'
. It looks strange because it's double escaped in the string literal for the expected output.
E.g.
System.debug('SELECT Id FROM Account WHERE Name = \'Acme\\\\\\\\s Carwash\'');
// output:
// SELECT Id FROM Account WHERE Name = 'Acme\\\\s Carwash'
No description provided.