sourcegraph / sourcegraph-public-snapshot

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

New/old reference highlighting inconsistent in diff view #14485

Open Strum355 opened 3 years ago

Strum355 commented 3 years ago

Steps to reproduce:

  1. View a code diff eg https://github.com/sourcegraph/sourcegraph/pull/14457/files#diff-07fd52009dd00f44a2dc9ea2a2bfc6a5
  2. Hover over a variable in the before/after diff with a reference in an unchanged section of code

Exhibit A: image

Exhibit B: image

Expected behavior:

When hovering the variable in the old diff, references to the variable outside the diff would be highlighted. In Exhibit A, hovering wg will highlight its use further below in an untouched section of code. In Exhibit B, the same variable does not highlight the same reference.

Actual behavior:

They are not :cowboy_hat_face:

cc @sourcegraph/code-intel @sourcegraph/web

efritz commented 3 years ago

@sourcegraph/web Could we get a (relatively high-level) brain dump of how the diff view stuff works on the web side?

felixfbecker commented 3 years ago

For diffs, we create two TextDocuments, one at the base rev and one at the head rev (with distinct URIs). Depending on what side was hovered, we direct calls to either one or the other. From an extension's perspective, there are simply two distinct text documents open.

When a user hovers inside the red (base) area, what happens is that we ask for all document highlights in the base document at that position. Then all DOM elements are found at the returned ranges and highlighted (I think, you implemented this 😄 ). What seems to be happening here is that the white part of the diff is considered part of the head (that's where hover requests would go to too), but not part of the base. But for document highlight purposes, we need to consider the white part part as matching both base and head.

github-actions[bot] commented 3 years ago

Heads up @joelkw @muratsu - the "team/extensibility" label was applied to this issue.