Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clicking the arrows in diffs on deleted lines doesn't work #179114

Closed
jespertheend opened this issue Apr 4, 2023 · 13 comments · Fixed by #179387 or #181040
Closed

Clicking the arrows in diffs on deleted lines doesn't work #179114

jespertheend opened this issue Apr 4, 2023 · 13 comments · Fixed by #179387 or #181040
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release diff-editor Diff editor issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone

Comments

@jespertheend
Copy link

Does this issue occur when all extensions are disabled?: Yes

Version: 1.76.2
Commit: ee2b180
Date: 2023-03-14T17:57:21.103Z
Electron: 19.1.11
Chromium: 102.0.5005.196
Node.js: 16.14.2
V8: 10.2.154.26-electron.0
OS: Linux x64 5.19.0-38-generic
Sandboxed: No

Steps to Reproduce:

  1. Open a git diff view and delete one of the lines.
  2. Click the right pointing arrow in the diff view.
simplescreenrecorder-2023-04-04_11.56.36.mp4
@alexdima
Copy link
Member

alexdima commented Apr 5, 2023

I cannot reproduce this. Do you see any errors in developer tools? Does this reproduce with any file and with any change or is it specific to this particular file?

Kapture.2023-04-05.at.12.01.38.mp4

@alexdima alexdima added the info-needed Issue requires more information from poster label Apr 5, 2023
@jespertheend
Copy link
Author

I can't reproduce it anymore either. I just updated to 1.77.0. Maybe it just happened to get fixed in that version? I can still reproduce #179115 though.

@jespertheend
Copy link
Author

Hmm no I can still reproduce it in 1.77.0.

@jespertheend
Copy link
Author

Found it! Seems like the issue only occurs after line 99

@jespertheend
Copy link
Author

Hmm and another requirement is that the file has 1000 lines in total or more I think.

@alexdima
Copy link
Member

alexdima commented Apr 5, 2023

@jespertheend I cannot reproduce. I tried with a file which has 1353 lines and I do an edit at line 600. Do you see any errors in Developer Tools (Help > Toggle Developer Tools)?

Kapture.2023-04-05.at.18.01.12.mp4

@jespertheend
Copy link
Author

@alexdima I don't think the devtools console contains anything of interest:

WARN Via 'product.json#extensionEnabledApiProposals' extension 'ms-vscode.vscode-selfhost-test-provider' wants API proposal 'testContinuousRun' but that proposal DOES NOT EXIST. Likely, the proposal has been finalized (check 'vscode.d.ts') or was abandoned.
log.ts:431 WARN Via 'product.json#extensionEnabledApiProposals' extension 'ms-vscode.remote-repositories' wants API proposal 'contribEditorGutterMenu' but that proposal DOES NOT EXIST. Likely, the proposal has been finalized (check 'vscode.d.ts') or was abandoned.
log.ts:431 WARN Via 'product.json#extensionEnabledApiProposals' extension 'github.vscode-pull-request-github' wants API proposal 'contribEditorGutterMenu' but that proposal DOES NOT EXIST. Likely, the proposal has been finalized (check 'vscode.d.ts') or was abandoned.
log.ts:431 WARN Via 'product.json#extensionEnabledApiProposals' extension 'ms-toolsai.jupyter' wants API proposal 'notebookControllerKind' but that proposal DOES NOT EXIST. Likely, the proposal has been finalized (check 'vscode.d.ts') or was abandoned.
log.ts:441 ERR Extension 'github.copilot-nightly' appears in product.json but enables LESS API proposals than the extension wants.
package.json (LOSES): inlineCompletionsAdditions, interactive
product.json (WINS): inlineCompletionsAdditions
log.ts:421 INFO [perf] Render performance baseline is 62ms

I've found that the line 100 rule doesn't completely hold for me either, when make the right panel very big it becomes clickable again.

@jespertheend
Copy link
Author

So far my hypothesis is that you need a long word wrapped line above the one you're trying to reset. But honestly I'm not sure anymore at this point.

simplescreenrecorder-2023-04-05_18.27.57.mp4

@alexdima
Copy link
Member

alexdima commented Apr 6, 2023

Hooray, this reproduces when using word wrapping! The root cause is that some properties on the mouse event are not converted from view coordinates to model coordinates.

This reproduces with the following word wrapped .txt file:

very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line very long line 

short line
short line
short line

short line
short line
short line

alexdima added a commit that referenced this issue Apr 6, 2023
@alexdima alexdima added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Apr 6, 2023
@alexdima alexdima added this to the April 2023 milestone Apr 6, 2023
alexdima added a commit that referenced this issue Apr 6, 2023
…tting the event (#179387)

Fixes #179114: Convert `IMouseTargetViewZone.detail` to model coordinates before emitting the event
@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Apr 6, 2023
@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Apr 7, 2023
@ulugbekna
Copy link
Contributor

ulugbekna commented Apr 27, 2023

Seems like I can still reproduce (with and w/o word-wrapping):

Screen.Recording.2023-04-27.at.11.10.28.mov
Screen.Recording.2023-04-27.at.11.14.43.mov

@ulugbekna ulugbekna added the verification-found Issue verification failed label Apr 27, 2023
@ulugbekna ulugbekna reopened this Apr 27, 2023
@vscodenpa vscodenpa removed the insiders-released Patch has been released in VS Code Insiders label Apr 27, 2023
@alexdima
Copy link
Member

I ran git bisect and I had to skip a few commits due to build problems, but I believe this is caused by #179130

The first bad commit could be any of:
5132503
c420cbd
bdf6efe

@alexdima alexdima removed their assignment Apr 27, 2023
@hediet hediet added the candidate Issue identified as probable candidate for fixing in the next release label Apr 27, 2023
@roblourens
Copy link
Member

So merging the PR will make the arrows work but will also bring back the issue of also setting breakpoints? That is unfortunate but I think I'd rather have the arrows working.

@alexdima
Copy link
Member

👍 I think it's better to go back to the state we had in 1.77 and the risk is minimal as it is a simple revert.

@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Apr 27, 2023
@roblourens roblourens removed the verification-found Issue verification failed label Apr 28, 2023
@rzhao271 rzhao271 added the verified Verification succeeded label May 1, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release diff-editor Diff editor issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
7 participants