TermControl: Remove the "hairpin" paste handler, consolidate string writers#20155
Open
TermControl: Remove the "hairpin" paste handler, consolidate string writers#20155
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Member
Author
This comment has been minimized.
This comment has been minimized.
DHowett
force-pushed
the
dev/duhowett/fhl-2026/remove-paste-hairpin-handler
branch
from
36efc00 to
1b3aa5c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
and Port the rest of the app to WriteInputString
I've audited all the events, and the only one that tried to use IControlInteractivity or similar was Paste
DHowett
force-pushed
the
dev/duhowett/fhl-2026/remove-paste-hairpin-handler
branch
from
1b3aa5c to
ed93677
Compare
Member
carlos-zamora
left a comment
carlos-zamora
left a comment
There was a problem hiding this comment.
Really just the one thing I want to double check
| // Return Value: | ||
| // - <none> | ||
| void TermControl::SendInput(const winrt::hstring& wstr) | ||
| void TermControl::WriteInputString(const winrt::hstring& wstr, WriteInputStringType type) |
Member
There was a problem hiding this comment.
nit: update "Arguments" comment above to include type
Member
Author
There was a problem hiding this comment.
frick i meant to murder the arguments section entirely
Comment on lines
+3245
to
+3240
| _core.PasteText(text); | ||
| _core.WriteInputString(text, WriteInputStringType::Raw); |
Member
There was a problem hiding this comment.
Should this be WriteInputStringType::Clipboard?
DHowett
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request rewrites how TermControl takes input from the outside world in a number of ways.
First, it introduces a new API called
WriteInputString, which replaces bothSendInputandPasteText. Those functions all had terrible and confusing names and semantics, but all boiled down to some internal implementation details around what was stripped where and how we added the bracketed paste sequence for clipboard content.(For now, there is a duplicate
WriteInputStringWithoutBroadcastwhich skips some of the loopy callback machinery; it is removed in #20165.)Then, it removes the entire
PasteFromClipboardEventArgsand its "callback" machinery for requesting a paste-if-you-would-just-please-call-me-back from the hosting application.The connection is now 1:1: a control raises
PasteRequested, and the host callsWriteInputString(..., Clipboard)to complete the transaction.This also necessitated changing how we compute whether a paste would have been bracketed for the purpose of displaying warning dialogs and emitting empty paste packets. Now we check the control that originated the event (which required us to change how bubbled events report their sender.)
It might seem like I left weird stubs laying around.
In the future:
StringSentcan be totally removed, as paste content handling is already part of the app layer and drag/drop handling will be soon.anyHasBracketedPaste(andanyHasUnbracketedPaste) will grow to check every control in the "broadcast group" so that we warn appropriately when a paste will touch multiple controls in different encodings.These changes allow us to localize paste encoding behavior to the control (rather than e.g. firing
StringSentwith a bracketed paste sequence to be consumed by a control that has bracketed paste turned off) and improve broadcast without weaving new and expensive event handlers up and down through the stack.While doing this, I noticed that TSF composition doesn't get broadcasted... lol.