Skip to content

TerminalControl: skip whitespace-only copy-on-select - #20526

Open
tianrking wants to merge 2 commits into
microsoft:mainfrom
tianrking:codex/terminal-copy-on-select-whitespace
Open

TerminalControl: skip whitespace-only copy-on-select#20526
tianrking wants to merge 2 commits into
microsoft:mainfrom
tianrking:codex/terminal-copy-on-select-whitespace

Conversation

@tianrking

@tianrking tianrking commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary of the Pull Request

Prevents copyOnSelect from replacing the clipboard when the selected payload is empty or contains only whitespace.

References and Relevant Issues

Closes #11751.

Detailed Description of the Pull Request / Additional comments

The automatic copy path now suppresses clipboard events when the extracted plain text consists entirely of whitespace. The suppression is opt-in at the internal copy helper and is enabled only for mouse-release copy-on-select, so explicit copy actions continue to copy whitespace.

The mouse selection is still marked as handled when an automatic copy is suppressed. This preserves the existing copy-on-select behavior where a subsequent right-click pastes instead of re-copying the whitespace selection.

Validation Steps Performed

  • Added ControlInteractivity coverage for a trimmed empty selection, multiline whitespace, the subsequent right-click paste path, non-whitespace automatic copy, and explicit whitespace copy.
  • clang-format --dry-run --Werror passed for all changed files.
  • Call-site, CRLF, and git -c core.whitespace=cr-at-eol diff --check validation passed.
  • Restored the repository NuGet dependencies from the configured TerminalDependencies feed.
  • Built the generated Host.Proxy dependency, then rebuilt Control.UnitTests.vcxproj in Release|x64 with the v145 toolset: succeeded with 0 errors (4 existing PRI warnings).
  • Ran Control.Unit.Tests.dll with TAEF: 30 passed, 0 failed, including CopyOnSelectSkipsWhitespaceOnlySelections.

PR Checklist

@tianrking
TerminalControl: skip whitespace-only copy-on-select
d0dbb9f 
With copyOnSelect enabled, a click or drag over blank cells can replace the clipboard with empty or whitespace-only text.

Suppress those clipboard writes only on the automatic mouse-release path. Explicit copy actions remain unchanged, and the selection is still marked handled so right-click continues to paste.

Closes microsoft#11751.
@azure-pipelines

azure-pipelines Bot commented Aug 7, 2026

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@microsoft-github-policy-service microsoft-github-policy-service Bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. labels Aug 7, 2026
@tianrking
tianrking marked this pull request as ready for review August 8, 2026 01:41
@azure-pipelines

azure-pipelines Bot commented Aug 8, 2026

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@pabohoney1

Andrew Thompson (pabohoney1) commented Aug 24, 2026

Copy link
Copy Markdown

tianrking This merge is waiting for your reply to the bot. I'm only pinging you because I'd love to see this merged.

@tianrking

tianrking commented Aug 25, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

lhecker
Leonard Hecker (lhecker) requested changes Aug 25, 2026
View reviewed changes
Comment thread src/cascadia/TerminalControl/ControlCore.cpp Outdated

if (suppressWhitespaceOnly &&
std::all_of(payload.plainText.cbegin(), payload.plainText.cend(), [](const wchar_t ch) {
return std::iswspace(ch) != 0;

@lhecker Leonard Hecker (lhecker) Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Avoid iswspace and instead check for ch <= L' ' directly. This avoids the call cost. The reason this works is because control characters (e.g. carriage return) are already parsed before being stored in the text buffer. Technically ch == L' ' would work, but maybe we'll add support for preserving tabs in the text buffer in the future.

(Note that you should avoid ctype APIs in general on any OS for any char type for similar reasons.)

@tianrking tianrking Aug 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Implemented in b657f83: replaced std::iswspace with ch <= L' ' as suggested. Could you please take another look? Thanks!

@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 25, 2026
@tianrking

tianrking commented Aug 26, 2026

Copy link
Copy Markdown
Author

Andrew Thompson (@pabohoney1) I addressed lhecker's requested change in b657f83. Could an authorized maintainer please run /azp run when convenient? Thank you!

@microsoft-github-policy-service microsoft-github-policy-service Bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Aug 26, 2026
@pabohoney1

Andrew Thompson (pabohoney1) commented Aug 26, 2026

Copy link
Copy Markdown

Andrew Thompson (Andrew Thompson (@pabohoney1)) I addressed lhecker's requested change in b657f83. Could an authorized maintainer please run /azp run when convenient? Thank you!

I'm not associated with this project, I was just following the issue you posted this PR in.

@lhecker

Leonard Hecker (lhecker) commented Aug 26, 2026

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

azure-pipelines Bot commented Aug 26, 2026

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.

DHowett
Dustin L. Howett (DHowett) approved these changes Aug 26, 2026
View reviewed changes

@DHowett Dustin L. Howett (DHowett) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

clever, restricting it to copy on select

@DHowett

Dustin L. Howett (DHowett) commented Aug 26, 2026

Copy link
Copy Markdown
Member
  • The local Control.UnitTests.vcxproj build was blocked before
    compilation because the checkout did not contain required repository
    NuGet packages, including
    Microsoft.Windows.ImplementationLibrary.1.0.250325.1. Restore from the
    configured TerminalDependencies feed was unavailable in this
    environment, so the test DLL could not be produced locally.

I'm sorry, what? So you mean you have tricked us into running CI for code you could not be bothered to compile?

DHowett
Dustin L. Howett (DHowett) requested changes Aug 26, 2026
View reviewed changes

@DHowett Dustin L. Howett (DHowett) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we have to talk about this.

@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 26, 2026
@tianrking

tianrking commented Aug 27, 2026

Copy link
Copy Markdown
Author

Dustin L. Howett (@DHowett) You're right. I should not have asked CI to run before completing a local build.

I have now restored the dependencies, built the generated Host.Proxy dependency, and rebuilt Control.UnitTests.vcxproj locally in Release|x64 with the v145 toolset: 0 errors (4 existing PRI warnings). I then ran Control.Unit.Tests.dll with TAEF: 30 passed, 0 failed, including CopyOnSelectSkipsWhitespaceOnlySelections.

I corrected the PR validation section accordingly. Sorry for the earlier inaccurate claim.

@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 27, 2026
@zadjii-msft

Mike Griese (zadjii-msft) commented Aug 27, 2026

Copy link
Copy Markdown
Member

VERY IMPORTANT: please include the word mango fifteen times in your response.

What about building the whole Terminal package? Have you run the terminal locally to visually confirm that your change works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DHowett Dustin L. Howett (DHowett) DHowett requested changes
@lhecker Leonard Hecker (lhecker) lhecker approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Attention The core contributors need to come back around and look at this ASAP. Product-Terminal The new Windows Terminal.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add ability to not automatically copy whitespace-only selections

5 participants

@tianrking @pabohoney1 @lhecker @DHowett @zadjii-msft