search: Distinguish file content matches from path matches in the UI when select:file is present#41297
Conversation
During construction of zoekt.GlobalTextSearchJob and zoekt.RepoSubsetTextSearchJob, compile the query patterns being sent to zoekt to regexps. These regexps are used during match construction to compute path match ranges. These path match ranges are propagated back up to the client (web app) where they are then highlighted in the UI.
During construction of searcher.TextSearchJob, compile the text pattern into a regexp. When converting searcher matches to result.FileMatches, use that regexp to compute path match ranges, which are then propagated back up to the client (web app) where they are highlighted in the UI.
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff b026def...2b6a03d.
|
There was a problem hiding this comment.
I agree that it's confusing that we call the results of select:file a "path match", but this change doesn't quite fit my mental model of what's happening here.
A couple of statements:
type:file abcwill return both content matches and path matchestype:file abc select:filewill convert any content matches into path matches, and pass through any path matches unmodified- Path matches, content matches, and symbol matches are all represented by the
FileMatchbackend type, withlen(fm.ChunkMatches) == 0 && len(fm.Symbols)indicating it's a path match.
(1) is confusing because type:file will return two different result types
(2) is confusing because, even though we use type:file to mean "path matches and content matches", select:file means "select the path from any type of file match"
(3) is confusing because we don't have a dedicated type for path matches, which means we have no way distinguish between path matches and content matches with zero matched ranges. I tried to fix this at one point, but there were many edge cases that I didn't have time to track down. Some more history
So...why does this change feel wrong? Because access_token select:file does actually return a "path match" by this definition. It converts content matches into path matches -- those content matches are no longer content matches. Similarly, if you did select:repo, even if we started with content matches, they become repo matches via selection. All that to say, I think it's inaccurate to say that access_token select:file returns content matches.
|
@camdencheek So, it wasn't my intent to label these matches as being content matches, because what we're presenting is a path match. That hasn't changed. My intent was to indicate more specifically to the user why certain results are included when I think that adding path match highlighting is going to affect how users interpret the label "Path match." If I'm a user, and I see a path match result, I see that 1) the matching part of the file path is highlighted, and 2) it's labeled as a "Path match." So I think, "A path match is where the file path itself matches my query and the matching part will be highlighted." I think the label "Path match" implies that the file path itself contains a match on the query, but this is not always true. (I actually have always felt this way, way before working on the highlighting.) Now if I see a result like Would it be better to phrase it as something like "File contains matching content"? Instead of "File content match," which implies that the match itself is a content match. |
|
Updated the displayed message to be more specific |
|
Updated wording seems clear to me and I think would make using |
…lect:file` is present `select:file` restricts result rendering to only show file paths. These results can be either 1) a match on the literal file path string, or 2) a file containing a content match, but not necessarily a path match. Currently, we always display the text "Path match" when a result is rendered as file path-only. This is confusing when the result is a content match but not a path match. This commit makes a distinction between a path match and a content match in the UI for `select:file` results to address this issue.
Display the text "File contains matching content" when a path match is in the result set because it contains a content match (i.e. it was a content match that was converted to a path match in the backend due to `select:file`). This is more specific than the initial label "File content match" which implies that the match itself is a content match, when it is in fact still a path match.
1c3477a to
2b6a03d
Compare
Stacked on #41296
select:filerestricts result rendering to only show file paths. These results can be either:Regardless,
select:filewill cause all results to be converted to path matches.Currently, we always label a result with "Path match" when it is rendered as a path match. This is confusing when a result is a content match that has been converted to a path match, because the label "Path match" implies that the file path itself contains a match, when this is not always the case.
Now that path match ranges are available in the client, we can make the distinction between a "natural" path match and a "converted" path match in the UI in a way that's informative to users.
Before:
After:
Test plan
Visually confirm the change in the UI