sourcegraph / sourcegraph-public-snapshot

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

Remove line matches from Zoekt #39951

Open camdencheek opened 2 years ago

camdencheek commented 2 years ago

Once we're reasonably comfortable with the using chunk matches instead of line matches in Zoekt, we should remove the now-unused line match code. This is not a small task since all the tests and stuff also depend on that representation.

/cc @sourcegraph/search-platform

keegancsmith commented 6 months ago

I'm not totally sure we want to do this, since I believe other consumers of the zoekt repo use linematches. A more conservative thing we could do is make chunkmatches the default and maybe just mark the line matches field as deprecated so tools start to complain?

Either way, not so long ago I removed the duplicate ranking paths we had for chunkmatches and linematches which too me was the largest part of the debt of both paths.

camdencheek commented 6 months ago

If we can clean up the duplicate codepaths and just leave this at the API layer, sounds reasonable to me to just keep it around. Besides, line matches are still easier to use for simple situations.

keegancsmith commented 6 months ago

Right so I took another look. Here are the places we do different things depending on ChunkMatches:

And thats it... I reckon we could change merge candidate behaviour and drop support for line matches in grpc. Keep LineMatch default, but grpc layer enforces chunkmatches for sourcegraph. I think that would be ideal from the perspective of keeping a simpler representation (for tests as well), and isn't that heavy of a cost.

There is the option of exercising LineMatches from the go API totally, and introducing a convertor in zoekt/web pkg for REST. However, TBH that is quite a big change given so many tests depend on linematches right now.