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

Recent change to link detection in the terminal is a regression #174009

Closed
jribbens opened this issue Feb 10, 2023 · 7 comments · Fixed by #174847
Closed

Recent change to link detection in the terminal is a regression #174009

jribbens opened this issue Feb 10, 2023 · 7 comments · Fixed by #174847
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone

Comments

@jribbens
Copy link

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

  • VS Code Version: 1.75.1
  • OS Version: Windows 11

Steps to Reproduce:

  1. Display a link in the terminal of the form https://example.com/path(abc,def)
  2. Previously the link would be automatically detected correctly and made clickable (although in the debug tab, by contrast, it was not detected properly).
  3. Since version 1.75.0, the link detection wrongly terminates at the , so the link cannot be easily clicked to open it.

I appreciate that you can never please all the people all the time with URL guessing, but I thought it was worth mentioning given that it used to work fine, and to be fair all the characters (, , and ) are standard normal URL code points and always have been.

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member regression Something that used to work is now broken terminal-links labels Feb 10, 2023
@Tyriar
Copy link
Member

Tyriar commented Feb 10, 2023

Can repro after, not before:

image

@Tyriar Tyriar added this to the February 2023 milestone Feb 10, 2023
@Tyriar
Copy link
Member

Tyriar commented Feb 10, 2023

This is actually a regression in monaco's link detection which the terminal leverages for uris:

1.74:
image

1.75:

image

So this was unrelated to the recent terminal link improvements which focused on file detection

@Tyriar Tyriar assigned hediet and alexdima and unassigned Tyriar Feb 10, 2023
@Tyriar Tyriar removed this from the February 2023 milestone Feb 10, 2023
@hediet
Copy link
Member

hediet commented Feb 10, 2023

This PR introduced this change to fix this issue

@hediet hediet added under-discussion Issue is under discussion for relevance, priority, approach and removed bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member regression Something that used to work is now broken new release labels Feb 10, 2023
@Tyriar
Copy link
Member

Tyriar commented Feb 10, 2023

Commas are used pretty often in certain links, not sure how advanced the detection is but it could be improved by only treating it as a separator if after a / or if the word before has a period?

@hediet hediet removed their assignment Feb 11, 2023
@hediet
Copy link
Member

hediet commented Feb 11, 2023

GitHub also doesn't split at comma:
https://github.com/microsoft/vscode,https://github.com/microsoft/vscode

I'd probably revert the fix, but I'm fine with either. Up to you @alexdima.

@Ruler90
Copy link

Ruler90 commented Feb 16, 2023

This issue doesn't just affect the links in the terminal. It also applies to .js, .ts and .md files (and maybe more).

VS Code Version: 1.75.1
OS Version: Windows 10

Steps to reproduce:

  1. Create a .js file and paste the following code into it:
    const url = "https://www.empik.com/samsung-galaxy-sm-s918bzedeuedm3-8-gb-ram-256-gb-bezowy-samsung,p1360111382,elektronika-p";
  2. Create an .md file and paste the following url into it:
    https://www.empik.com/samsung-galaxy-sm-s918bzedeuedm3-8-gb-ram-256-gb-bezowy-samsung,p1360111382,elektronika-p.
  3. In both cases the link detection wrongly terminates at the , so the link cannot be easily clicked to open it.

@alexdima
Copy link
Member

👍 I'm OK to revert.

@alexdima alexdima added bug Issue identified by VS Code Team member as probable bug and removed under-discussion Issue is under discussion for relevance, priority, approach labels Feb 20, 2023
@alexdima alexdima added this to the February 2023 milestone Feb 20, 2023
alexdima added a commit that referenced this issue Feb 20, 2023
Revert "Treat commas as link terminators (#168752)" to fix #174009

This reverts commit f730897.
@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 Feb 20, 2023
@jrieken jrieken added the verified Verification succeeded label Feb 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 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 insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants