search: support "exact match" pattern using "..."#59057
Conversation
Relates to #58815 With this change, a quoted pattern, like in `"foo bar"`, is interpreted literally IE spaces are interpreted as spaces instead of as logical AND. Quotes, which are part of the pattern have to be escaped. Example: searching for the literal `foo "bar"`, where "bar" is surrounded by quotes, the query is `"foo \"bar\""` (or equivalently `'foo "bar"'`) Note: This only applies for our keyword search prototype. Test plan: - updated unit test - manual testing - tried out various combinations
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff d5a6a0a...3728e28.
|
jtibshirani
left a comment
There was a problem hiding this comment.
This change turned out really cleanly!
I'm not totally convinced we should be supporting the single quote version. I haven't seen this in any other search query language ... I've only seen it in the context of string syntax in certain programming languages. I don't feel strongly though. What do you think?
Also adding @camdencheek as a reviewer so he can weigh in on these types of changes.
| // than canonical form (r: instead of repo:) | ||
| IsAlias | ||
| Standard | ||
| QuotesAsLiterals |
There was a problem hiding this comment.
Is this actually added to any nodes? In all the added tests, I don't see this show up anywhere. It seems like this is being used as an option rather than a label, which is a little confusing to me. That said...seems like we might be doing that already with Standard 😬
There was a problem hiding this comment.
It seems like this is being used as an option rather than a label, which is a little confusing to me. That said...seems like we might be doing that already with Standard
You are correct, it's used as an option, not as a label. I modelled it after Standard. I can see that this might be confusing. Given that Standard follows the same approach and given that this change here was straightforward by treating the label as option, I wonder whether it is worth it to introduce dedicated parser options to distinguish between the two concepts.
|
No strong opinion personally on single/double quotes. It is pretty nice to be able to just switch to the other type of quote if the literal I'm looking for contains quotes. |
|
I think the single quotes are a nice touch, but not essential. I usually try if single quotes work if escaping double quotes gets out of hand. If you don't mind, I suggest we leave it in for now. We still have some time to get a feeling for this. |
Relates to #58815
With this change, a quoted pattern, like
"foo bar", is interpreted literally IE spaces are interpreted as spaces instead of as logicalAND. Quotes that should be matched literally have to be escaped.Example:
To search for the text
foo "bar", wherebaris surrounded by quotes, the query is either"foo \"bar\""or, if we use single quotes,'foo "bar"'.Note: This change only affects our keyword search prototype.
Test plan: