search

sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.1k stars 1.28k forks source link

insights: aggregations 4.0 - only parenthesize filters if they include whitespace #41442

Closed coury-clark closed 2 years ago

coury-clark commented 2 years ago

To reduce the confusion of how users might interact with search filters and avoiding unnecessary syntax, we should consider only wrapping drilldown filters in parenthesis if there is whitespace inside the string. We will continue to anchor all filters. For example:

Santa Claus -> (^Santa Claus$) SantaClaus -> ^SantaClaus

/cc @joelkw @felixfbecker @vovakulikov

felixfbecker commented 2 years ago

Instead of parentheses, can we just use quotes? That's what we use everywhere in our search documentation, filter suggestions etc.

coury-clark commented 2 years ago

@felixfbecker it seems to do so would require double escaping the string:

1. file:"^src/pages/terms/\\[\\.\\.\\.slug\\]\\.tsx$"
2. file:(^src/pages/terms/\[\.\.\.slug\]\.tsx$)

It's very unreadable, in my opinion.

felixfbecker commented 2 years ago

Hu, weird. Any idea why that is?

One thing I'm worried about is that when we add capture group aggregation over groups in file: filters, these groups will count as capture groups (with only one bar). We may even want to have logic to automatically select capture group as the grouping attribute if a group is present, and then that would always only show one bar. We could make it a non-capturing group, but that adds even more syntax noise. Quoting would be the least invasive if double escaping wasn't required

coury-clark commented 2 years ago

@felixfbecker I believe the quote syntax is the "literal" equivalent https://docs.sourcegraph.com/code_search/reference/queries, therefore it is not treating the contents as a regexp pattern.

Agree with your assessment, but I think we're well into the territory of things working together than were not designed to. I think we need to sit with the search team and think more holistically about user input and feedback here. The short term solution to the capture groups problem you posed is to make these non-capturing. Given it's only applied in the case of a space after this PR, I think that would be reasonable.

felixfbecker commented 2 years ago

That's interesting because I noticed (at least for the author: filter) the value is still interpreted as a regex even when quoted, just that you can use whitespace within it. Agree with everything you said