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.

Sign up for GitHub

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

Jump to bottom

Sometimes links don't work the first time you click #230010

Closed
Tyriar opened this issue Sep 27, 2024 · 2 comments · Fixed by #230042
Closed

Sometimes links don't work the first time you click #230010

Tyriar opened this issue Sep 27, 2024 · 2 comments · Fixed by #230042
Assignees
@Tyriar
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-links upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-fixed The underlying upstream issue has been fixed upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded
Milestone
October 2024

Comments

@Tyriar
Copy link
Member

Tyriar commented Sep 27, 2024

Repro seems somewhat reliable:

  1. Run ls or get-childitem in terminal
  2. Hold ctrl/cmd
  3. Click the links quickly, the first time you click a link after moving to it may not work

Image
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal-links labels Sep 27, 2024
@Tyriar Tyriar added this to the October 2024 milestone Sep 27, 2024
@Tyriar Tyriar self-assigned this Sep 27, 2024
@Tyriar
Copy link
Member Author

Tyriar commented Sep 27, 2024

activate is never reached in this case:

vscode/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLink.ts

Lines 52 to 54 in e0ca49c

activate(event: MouseEvent | undefined, text: string): void {
this._activateCallback(event, text);
}

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Sep 27, 2024
@Tyriar
Fix links sometimes not activating
f764c0a 
This was a very long standing bug that was hard to nail down a repro case. Turns
out this was because at some point the current link can get cleared and re-evaluated,
I think because of a different fix we made. That caused the object check to fail.
So the fix there is to check link equality, not js object equality.

Additionally I found we don't dispose of old linkifiers when open is called
multiple times.

See microsoft/vscode#230010
@Tyriar Tyriar mentioned this issue Sep 27, 2024
Merged
@Tyriar
Copy link
Member Author

Tyriar commented Sep 27, 2024

Upstream fix: xtermjs/xterm.js#5171

@Tyriar Tyriar added upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream upstream-issue-fixed The underlying upstream issue has been fixed labels Sep 27, 2024
Tyriar added a commit that referenced this issue Sep 28, 2024
@Tyriar
[email protected]
3f5fa8a 
Fixes #230010
@Tyriar Tyriar mentioned this issue Sep 28, 2024
Merged
Tyriar added a commit that referenced this issue Sep 28, 2024
@Tyriar
Add support for links oo(l:c) and oo (l:c)
1da740a 
Fixes #230010
Tyriar added a commit that referenced this issue Sep 28, 2024
@Tyriar
Add support for links 'foo(l:c)' and 'foo (l:c)'
58ec7b2 
Fixes #230010
Tyriar added a commit that referenced this issue Sep 28, 2024
@Tyriar
Add support for links 'foo(l:c)' and 'foo (l:c)'
3370c01 
Fixes #230010
@Tyriar Tyriar mentioned this issue Sep 28, 2024
Merged
@vs-code-engineering vs-code-engineering bot 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 Sep 28, 2024
@anthonykim1 anthonykim1 added the verified Verification succeeded label Oct 23, 2024
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Nov 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-links upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-fixed The underlying upstream issue has been fixed upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded
Projects
None yet
Milestone
October 2024
Development

Successfully merging a pull request may close this issue.

[email protected] microsoft/vscode
2 participants
@Tyriar @anthonykim1