omnibox: open results locally if possible #6781
Conversation
camdencheek
force-pushed
the
cc/open-local-file
branch
from
831f65d to
aea7f6e
Compare
camdencheek
force-pushed
the
cc/open-local-file
branch
from
c4f96f4 to
a7ca0e8
Compare
| const selectionStart = this.lineParamToRange(lineNumberParam).start | ||
| // Opening the file with an active selection is awkward, so use a zero-length | ||
| // selection to focus the target line without highlighting anything | ||
| const selection = new vscode.Range(selectionStart, selectionStart) |
There was a problem hiding this comment.
What about the scenario if the file has been locally modified, meaning the range we'd highlight here doesn't actually match the search result? Should we check for this?
There was a problem hiding this comment.
Do you know how we can check for this? I guess there can be all sorts of reasons why the local is different from the search result (search index behind, additional local commits that touch the file, different branch checked out locally (not sure we cannot for this in search), uncommitted changes...
There was a problem hiding this comment.
Yep, this is definitely an unfortunate side effect of search not being aware of local changes or non-main branches. However, since we're not actually selecting anything (zero-length selection that just scrolls the line into view), it won't be blatantly obvious that we jumped to a (hopefully) subtly wrong place. The worst case scenario is that the user is shown the wrong part of the file. If the file doesn't exist, we'll fall back to showing the remote content.
It would be possible to check for this (comparing local checkout against default branch), but that's a ton more work and I think this provides an immediately better experience for the 90%+ case.
| const selectionStart = this.lineParamToRange(lineNumberParam).start | ||
| // Opening the file with an active selection is awkward, so use a zero-length | ||
| // selection to focus the target line without highlighting anything | ||
| const selection = new vscode.Range(selectionStart, selectionStart) |
There was a problem hiding this comment.
Do you know how we can check for this? I guess there can be all sorts of reasons why the local is different from the search result (search index behind, additional local commits that touch the file, different branch checked out locally (not sure we cannot for this in search), uncommitted changes...
Backport request details: this is a low-risk, high-impact UX change that significantly improves the usefulness of omnibox search results by linking the local version of a file instead of a read-only view of the file from Sourcegraph. This modifies the behavior of clicking search results to link to local files if the result represents a file in your local repo. With the [recent changes](sourcegraph/sourcegraph#2943) that heavily prefer results from your current repo, this should now be most results. ## Test plan [Video walkthrough](https://share.cleanshot.com/kqr06zwK) <br> Backport 7ed51f2 from #6781 Co-authored-by: Camden Cheek <[email protected]>
…6799) Backport request details: this is a low-risk, high-impact UX change that significantly improves the usefulness of omnibox search results by linking the local version of a file instead of a read-only view of the file from Sourcegraph. This modifies the behavior of clicking search results to link to local files if the result represents a file in your local repo. With the [recent changes](sourcegraph/sourcegraph#2943) that heavily prefer results from your current repo, this should now be most results. ## Test plan [Video walkthrough](https://share.cleanshot.com/kqr06zwK) <br> Backport 7ed51f2 from #6781 Co-authored-by: Camden Cheek <[email protected]>
This modifies the behavior of clicking search results to link to local files if the result represents a file in your local repo. With the recent changes that heavily prefer results from your current repo, this should now be most results.
Test plan
Video walkthrough