Skip to content

compute: recognize replace syntax

Administrator requested to merge rvt/compute-replace-pred-55 into main

Created by: rvantonder

Stacked on https://github.com/sourcegraph/sourcegraph/pull/26477.

This introduces a special syntax (only on the compute endpoint):

content:replace(foo -> bar)

The special syntax recognizes replace on the field content, and only if the arguments to replace has some arrow syntax ->.

Special note to @coury-clark: this does mean the above syntax and behavior will be exposed to Code Insights work that use the compute endpoint. It should be exceedingly rare that any normal search query conflicts with this syntax, and I've taken care to basically make it very specific to trigger. In general, there is an upside here for both the stuff I'm exploring, and Code Insights, I believe, to have a way to perform basic search-replace, so I do think it is appropriate for this syntax to live adjacent to the current regex capture group logic.

This syntax is not going to be documented or advertised yet, but I'm just making you aware.

Right now the compute endpoint only recognizes regular expression patterns/search, and so in the exceedingly rare case that the above conflicts for a literal search, it is possible to escape either with any of, e.g.,

content:'content:replace(foo -> bar)'

or some uses of \ thrown in.


Side notes for me while I worked on this, that I would like to follow up on in other search work. Do not read this to review the PR. But if you want more context go ahead.

The predicate data types/functions don't work as well generally as I think it could. The main problems are that:

  • We use this registry type to do a syntax scan, but we don't actually care about the values in that registry. When I wanted to reuse the code that does the syntax scan, I had to mint a noop value so that the scanning could reuse it and accept this datatype.
  • The above also leads to a roundabout/heavyweight minting of a computePredicateRegistry to reuse the scanning function. There are ways to simplify the scanner to avoid the need for this, but I really don't want to muck around with the scanning/parser code in a significant way right now.
  • I started implementing compute Commands as a Predicate type, which I thought might have been elegant. The problem is that the interface doesn't really gel that well at this stage:
    • there's no need for a Plan method here. I think we can generalize this interface so that Plan is more along the lines of Run, where Run has variants like generate subexpression (i.e., a Plan) versus "other stuff".
    • pred.ParseParams feels roundabout in conjunction with ParseAsPredicate--I know why we do this for search code, but something itching me just says there has to be a better way. Basically, reusing some parts of the predicate work just shows that the whole ParseParams thing isn't needed when we have a ParseAsPredicate, but ParseAsPredicate can also probably just be taken care of in a general purpose scanner.

Merge request reports

Loading