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

Regression in launching links #17323

Closed
chrisant996 opened this issue May 25, 2024 · 6 comments · Fixed by #17326
Closed

Regression in launching links #17323

chrisant996 opened this issue May 25, 2024 · 6 comments · Fixed by #17326
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@chrisant996
Copy link

Windows Terminal version

1.21.1272.0

Windows build number

19045.4412

Other Software

No response

Steps to reproduce

  1. Open a cmd.exe tab in Terminal.
  2. Run echo Hello ^(https://github.com/chrisant996/clink^).
  3. Move the mouse over the hyperlink -- observe that the hyperlink is underlined correctly.
  4. Ctrl+Click the hyperlink -- the ) is mistakenly included in the URL, and causes the web navigation to fail.

Expected Behavior

It should open the link https://github.com/chrisant996/clink.
It should open the same link that it underlined.
Terminal 1.19.11213.0 works correctly.

Actual Behavior

It opened the link https://github.com/chrisant996/clink) -- note that the ) was mistakenly included.
What it opens is different from what it underlined.

@chrisant996 chrisant996 added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 25, 2024
@tusharsnx
Copy link
Contributor

tusharsnx commented May 28, 2024

Thanks for reporting this.

I believe result->stop is an exclusive coord, but it was passed down to the .GetPlainText() which expects inclusive coords:

return _activeBuffer().GetPlainText(result->start, result->stop);

On it.

@DHowett
Copy link
Member

DHowett commented May 28, 2024

@tusharsnx, @lhecker do either of you know if this will impact 1.20 as well?

@DHowett
Copy link
Member

DHowett commented May 28, 2024

Checked it out - yes, 1.20 is impacted. I'll service this.

Thanks @chrisant996 for the report!

@chrisant996
Copy link
Author

Ah, I see, yes it's fresh, introduced by 5b8eadb a month ago. 👍

github-merge-queue bot pushed a commit that referenced this issue May 28, 2024
Closes: #17323 

## Validation Steps Performed
- Run `echo Hello ^(https://github.com/microsoft/terminal^)` in cmd.
- Ctrl+click on the URL opens `https://github.com/microsoft/terminal` in
the browser.
- Hovering over the url in the terminal shows
`https://github.com/microsoft/terminal` in the hover UI.
DHowett pushed a commit that referenced this issue Jun 7, 2024
Closes: #17323

- Run `echo Hello ^(https://github.com/microsoft/terminal^)` in cmd.
- Ctrl+click on the URL opens `https://github.com/microsoft/terminal` in
the browser.
- Hovering over the url in the terminal shows
`https://github.com/microsoft/terminal` in the hover UI.

(cherry picked from commit 212f43e)
Service-Card-Id: 92637504
Service-Version: 1.20
DHowett pushed a commit that referenced this issue Jun 7, 2024
Closes: #17323

## Validation Steps Performed
- Run `echo Hello ^(https://github.com/microsoft/terminal^)` in cmd.
- Ctrl+click on the URL opens `https://github.com/microsoft/terminal` in
the browser.
- Hovering over the url in the terminal shows
`https://github.com/microsoft/terminal` in the hover UI.

(cherry picked from commit 212f43e)
Service-Card-Id: 92637505
Service-Version: 1.21
@Psypher1
Copy link

Psypher1 commented Jun 12, 2024

Hello, I was told my issue #17424 is already being tracked here. My question; I see a merge was done. Does that mean I should update terminal to enact the changes? However the most recent release is still preview...

Because while the merge sort of handle a similar issue, it's not quite what I'm experiencing

@lhecker
Copy link
Member

lhecker commented Jun 17, 2024

Preview will receive the fix sometime soon. Until then you can test out our nightly version here: https://aka.ms/terminal-canary-installer
Don't worry: If you uninstall it later, no traces will be left behind. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants