Skip to content

add file highlighting for structural search

Warren Gifford requested to merge rvt/highlight-structural-search-v2 into master

Created by: rvantonder

I have cleaned up the logic compared to #67. Notable things:

It is hard to work with a highlighting procedure that assumes all highlights happen on one line, with the highlight{line: 1, character: offset, length: length} and assumption that this is relative to the preview string. I tried hard to make the function applyHighlights work for a use case where the FileMatch data can be used as ranges to highlight things given the file content, but eventually stopped wasting time on this and split it out to applyHighlightsForFile.

To convince you that it is not a totally insane thing to want access to the file contents as opposed to making Preview be some kind of source of truth, I worked on trying to get some context lines to show around matches in #71 but it didn't pan out because it needs to take into account the line numbering/padding that gets written to terminal. I realize that the idea of context lines is useful generally and it's too bad we don't support this more directly in the API (I believe), but in lieu of that, it is nice that we have the possibility of sidestepping this in src-cli to highlight around context lines, but it would need more effort and #71 might help guide that.

The last thing that is annoying is the reported "number of matches" which is a lie for structural search/multiline matches, due to the line-based assumption that FileMatch contains complete matches. Not specific to this change or src-cli, but relevant to the whole highlighting business and lack of ranges.

This PR still needs tests, but can be reviewed as is.

More inline comments.

Merge request reports

Loading