Skip to content

Conversation

@albus-droid
Copy link
Contributor

@albus-droid albus-droid commented May 8, 2025

Summary of the Pull Request

This fixes #18877, This iteratively checks to see if a line is wrapped and moves up or down accordingly.

Detailed Description of the Pull Request / Additional comments

Current behavior: When a user triple-clicks on a line that’s visually wrapped by the terminal, only the single physical row that was clicked gets selected.

Expected behavior: A triple-click like in xterm, should select the entire logical line including all of its wrapped segments, from the true start through its true end, regardless of where the wrap occurred.

Why it matters: Logical line selection is what users expect when they’re trying to grab one command or output block in full. Limiting the selection to just the current physical row can lead to copy/paste mistakes and a confusing experience whenever a long line wraps.

Validation Steps Performed

I ran the existing tests using Invoke-OpenConsoleTests and they were passing and I was also able to test the build on my machine. I added a test case as well

PR Checklist

@albus-droid
Copy link
Contributor Author

@albus-droid please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@albus-droid albus-droid marked this pull request as ready for review May 9, 2025 06:41
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thanks!

@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.22 Servicing Pipeline May 9, 2025
@carlos-zamora carlos-zamora moved this from To Cherry Pick to To Consider in 1.22 Servicing Pipeline May 9, 2025
@carlos-zamora
Copy link
Member

carlos-zamora commented May 9, 2025

Demo from PR Body

Screenshot (138)

@carlos-zamora
Copy link
Member

@DHowett we should consider this for 1.23 as well

/azp run

@carlos-zamora carlos-zamora enabled auto-merge (squash) May 9, 2025 20:39
@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.23 Servicing Pipeline May 9, 2025
@carlos-zamora carlos-zamora moved this from To Cherry Pick to To Consider in 1.23 Servicing Pipeline May 9, 2025
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for doing this! excellent first contribution!

@DHowett DHowett disabled auto-merge May 9, 2025 22:26
@DHowett
Copy link
Member

DHowett commented May 9, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DHowett DHowett enabled auto-merge (squash) May 9, 2025 22:27
@albus-droid
Copy link
Contributor Author

Thank you so much. This was a great learning experience.

@DHowett DHowett merged commit 976a54d into microsoft:main May 9, 2025
12 of 14 checks passed
@elibarzilay
Copy link

@albus-droid Thanks!! I'd eagerly wait for this to make its way to the standard version...

But two possible issues:

  1. Looks like the code will not go beyond the currently visible part of the screen--? A particular case where this helps is when looking at rally long log lines, including stuff above/below the visible portion.

    I also tried to see what happens when a word is partially visible on the screen, and a double-click does select the whole word, including the off-screen part.

  2. [Actually, I convinced myself that the second thing I had should be filed as a separate issue. I'll do that now.]

@DHowett
Copy link
Member

DHowett commented May 10, 2025

Looks like the code will not go beyond the currently visible part of the screen--?

FWIW, this part of the code uses buffer coordinates where 0 is the top of the entire scrollback. It does indeed work if you select part of a line wrapped from above the top of the viewport.

(In later issue replies I will probably use the terms "buffer" and "viewport"; the buffer is the entire scrollback plus active area, and the viewport is only the visible subset of that area.)

@elibarzilay
Copy link

@DHowett Ah, in that case this is perfect.

(And apologies for talking before trying it; I'm still trying to convince my dev VM to work so I can install VS there to try it.)

@DHowett
Copy link
Member

DHowett commented May 10, 2025

It's out in tonight's canary build if you'd rather not build it yourself!

https://aka.ms/terminal-canary-installer

@DHowett DHowett moved this from To Consider to To Cherry Pick in 1.23 Servicing Pipeline May 13, 2025
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.23 Servicing Pipeline May 13, 2025
DHowett pushed a commit that referenced this pull request May 13, 2025
This fixes #18877, by iteratively checking to see if a line is wrapped
and moving up or down accordingly.

**Current behavior:** When a user triple-clicks on a line that’s
visually wrapped by the terminal, only the single physical row that was
clicked gets selected.

**Expected behavior:** A triple-click like in xterm, should select the
entire logical line including all of its wrapped segments, from the true
start through its true end, regardless of where the wrap occurred.

**Why it matters:** Logical line selection is what users expect when
they’re trying to grab one command or output block in full. Limiting the
selection to just the current physical row can lead to copy/paste
mistakes and a confusing experience whenever a long line wraps.

## Validation Steps Performed
I ran the existing tests using `Invoke-OpenConsoleTests` and they were
passing and I was also able to test the build on my machine. I added a
test case as well

## PR Checklist
Closes #18877

(cherry picked from commit 976a54d)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgaHgMU PVTI_lADOAF3p4s4AxadtzgaMvXs
Service-Version: 1.23
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.23 Servicing Pipeline Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Consider

Development

Successfully merging this pull request may close these issues.

Triple-click to select logical line

4 participants