Something went wrong while fetching comments. Please try again.
Created by: rvantonder
Stacked on https://github.com/sourcegraph/sourcegraph/pull/37658
All suggested queries returned to a user from inside Sourcegraph will now explicitly enclose regular expression patterns with /.../. For example, if you enter derp patterntype:regexp then you are going to unconditionally get a suggested query /derp/ if there is some reason we're suggesting you a query that happens to contain regex. We don't have any suggested queries based on regular expressions in our normal app/workflow that I know of, so in theory, this should be nothing to worry about. Also, /.../ syntax is compatible with regex mode, and will be compatible with the new standard query string syntax (RFC 675). Since it's never possible to have a regular expression in "literal" search mode, this change is conflict free in the printer implementation and we can just unconditionally enclose regex patterns in /.../. That is, we will not be special casing the pretty print suggestions based on the input patterntype (we never have! instead, we had to always return patterntype with the associated query, both are yuck!).
This change touches code insights tests. Not sure if this output is used only in tests, or if it's rendered to the user. It is only used here: https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/enterprise/internal/insights/query/querybuilder/builder.go?L36:21. @camdencheek or @sourcegraph/code-insights can you confirm that queries going into Sourcegraph via this call are never retrieved back from somewhere and displayed from the user? If it's never retrieved back, and only for tests, don't worry about this PR. On the other hand, if it is displayed to a user...
then this will now render regular expressions enclosed in /.../. This because we're standardizing on a single query input string as per RFC 675 and patterntype:literal/patterntype:regexp should be avoided. By standardizing, you can fearlessly link to Sourcegraph with just the string and not worry about patterntype stuff. So
it's probably better to get users accustomed to /.../ if it does get input into Sourcegraph and then rendered out again using this StringHuman printer function. If this rendering behavior does exist in the Code Insights app and it's not desirable to enclose patterns with /.../ then the team should fork/maintain the preferred and custom query printing, and maintain the bridge. If this is an issue and let me know, I'm happy to fork the printing function but will not be maintaining it going forward.
Added test