starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-07-08 18:16:28 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[PR #13024] [MERGED] Merge the TerminalDispatch and AdaptDispatch classes #29338

New Issue
Open
opened 2026-01-31 09:34:20 +00:00 by claunia · 0 comments
claunia commented 2026-01-31 09:34:20 +00:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/13024
Author: @j4james
Created: 5/2/2022
Status: ✅ Merged
Merged: 5/4/2022
Merged by: @undefined

Base: main ← Head: merge-dispatch-classes

📝 Commits (10+)

  • 99b8ae4 Pass ConGetSet to AdaptDispatch by reference.
  • eb0a2e8 Make api a value field in InteractDispatch and PtySignalInputThread.
  • 708c56d Rename ConGetSet interface to ITerminalApi.
  • d910031 Add conhost stubs for Terminal-only APIs.
  • 63515d3 Move ReparentWindow implementation into PtySignalInputThread.
  • a75aa49 Replace WriteInput with ReturnResponse.
  • 6326593 Get Terminal to implement VirtualTerminal::ITerminalApi.
  • 6a80e72 Replace TerminalDispatch with AdaptDispatch.
  • 70f74ba Handle EraseAll at the start of the buffer as a special case.
  • bd58211 Get rid of unneeded terminal api methods.

📊 Changes

42 files changed (+628 additions, -2285 deletions)

View changed files

📝 src/buffer/out/search.cpp (+1 -1)
📝 src/buffer/out/search.h (+1 -1)
📝 src/cascadia/PublicTerminalCore/HwndTerminal.cpp (+8 -8)
📝 src/cascadia/TerminalControl/ControlCore.cpp (+9 -21)
📝 src/cascadia/TerminalControl/ControlCore.h (+1 -1)
➖ src/cascadia/TerminalCore/ITerminalApi.hpp (+0 -80)
📝 src/cascadia/TerminalCore/Terminal.cpp (+35 -23)
📝 src/cascadia/TerminalCore/Terminal.hpp (+31 -48)
📝 src/cascadia/TerminalCore/TerminalApi.cpp (+61 -442)
➖ src/cascadia/TerminalCore/TerminalDispatch.cpp (+0 -893)
➖ src/cascadia/TerminalCore/TerminalDispatch.hpp (+0 -135)
➖ src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp (+0 -267)
📝 src/cascadia/TerminalCore/lib/terminalcore-lib.vcxproj (+3 -0)
📝 src/cascadia/TerminalCore/terminalcore-common.vcxitems (+0 -4)
📝 src/cascadia/TerminalCore/terminalrenderdata.cpp (+2 -2)
📝 src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp (+1 -1)
📝 src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp (+9 -9)
📝 src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp (+10 -10)
📝 src/host/PtySignalInputThread.cpp (+13 -5)
📝 src/host/PtySignalInputThread.hpp (+2 -5)

...and 22 more files

📄 Description

Summary of the Pull Request

This PR replaces the TerminalDispatch class with the AdaptDispatch class from conhost, so we're no longer duplicating the VT functionality in two places. It also gives us a more complete VT implementation on the Terminal side, so it should work better in pass-through mode.

References

This is essentially part two of PR #12703.

PR Checklist

Detailed Description of the Pull Request / Additional comments

The first thing was to give the ConGetSet interface a new name, since it's now no longer specific to conhost. I went with ITerminalApi, since that was the equivalent interface on the terminal side, and it still seemed like a generic enough name. I also changed the way the api is managed by the AdaptDispatch class, so it's now stored as a reference rather than a unique_ptr, which more closely matches the way the TerminalDispatch class worked.

I then had to make sure that AdaptDispatch actually included all of the functionality currently in TerminalDispatch. That meant copying across the code for bracketed paste mode, the copy to clipboard operation, and the various ConEmu OSC operations. This also required a few new methods to the ConGetSet/ITerminalApi interface, but for now these are just stubs in conhost.

Then there were a few thing in the api interface that needed cleaning up. The ReparentWindow method doesn't belong there, so I've moved that into PtySignalInputThread class. And the WriteInput method was too low-level for the Terminal requirements, so I've replaced that with a ReturnResponse method which takes a wstring_view.

It was then a matter of getting the Terminal class to implement all the methods in the new ITerminalApi interface that it didn't already have. This was mostly mapping to existing functionality, but there are still a number of methods that I've had to leave as stubs for now. However, what we have is still good enough that I could then nuke the TerminalDispatch class from the Terminal code and replace it with AdaptDispatch.

One oddity that came up in testing, though, was the AdaptDispatch implementation of EraseAll would push a blank line into the scrollback when called on an empty buffer, whereas the previous terminal implementation did not. That caused problems for the conpty connection, because one of the first things it does on startup is send an ED 2 sequence. I've now updated the AdaptDispatch implementation to match the behavior of the terminal implementation in that regard.

Another problem was that the terminal implementation of the color table commands had special handling for the background color to notify the application window that it needed to repaint the background. I didn't want to have to push the color table operations through the ITerminalApi interface, so I've instead moved the handling of the background update into the renderer, initiated by a flag on the TriggerRefreshAll method.

Validation Steps Performed

Surprisingly this PR didn't require a lot of changes to get the unit tests working again. There were just a few methods used from the original ITerminalApi that have now been removed, and which needed an equivalent replacement. Also the updated behavior of the EraseAll method in conhost resulted in a change to the expected cursor position in one of the screen buffer tests.

In terms of manual testing, I've tried out all the different shells in Windows Terminal to make sure there wasn't anything obviously wrong. And I've run a bunch of the tests from vttest to try and get a wider coverage of the VT functionality, and confirmed everything still works at least as well as it used to. I've also run some of my own tests to verify the operations that had to be copied from TerminalDispatch to AdaptDispatch.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/13024 **Author:** [@j4james](https://github.com/j4james) **Created:** 5/2/2022 **Status:** ✅ Merged **Merged:** 5/4/2022 **Merged by:** [@undefined](undefined) **Base:** `main` ← **Head:** `merge-dispatch-classes` --- ### 📝 Commits (10+) - [`99b8ae4`](https://github.com/microsoft/terminal/commit/99b8ae4216d037f245000c309ec2f1a57dee7619) Pass ConGetSet to AdaptDispatch by reference. - [`eb0a2e8`](https://github.com/microsoft/terminal/commit/eb0a2e82c46c1e34f3d35cf326679de09d33d4ed) Make api a value field in InteractDispatch and PtySignalInputThread. - [`708c56d`](https://github.com/microsoft/terminal/commit/708c56d16879ee04f5cef974bc3af0af01c32d51) Rename ConGetSet interface to ITerminalApi. - [`d910031`](https://github.com/microsoft/terminal/commit/d9100314b8cce352509c4c33ba2b7332445555e0) Add conhost stubs for Terminal-only APIs. - [`63515d3`](https://github.com/microsoft/terminal/commit/63515d3eafa785caa95831667ba823db6ed61f71) Move ReparentWindow implementation into PtySignalInputThread. - [`a75aa49`](https://github.com/microsoft/terminal/commit/a75aa49ceb40bd8162a56b031fe4759cb098f1f1) Replace WriteInput with ReturnResponse. - [`6326593`](https://github.com/microsoft/terminal/commit/632659321596ebabb31c9bb12981d1ffacab8857) Get Terminal to implement VirtualTerminal::ITerminalApi. - [`6a80e72`](https://github.com/microsoft/terminal/commit/6a80e7231275645c92d6e5b49c9f7290ab39e175) Replace TerminalDispatch with AdaptDispatch. - [`70f74ba`](https://github.com/microsoft/terminal/commit/70f74ba20119c8f8172916b70bcca5824abb12f7) Handle EraseAll at the start of the buffer as a special case. - [`bd58211`](https://github.com/microsoft/terminal/commit/bd58211f6f65f492bc2afacd434a934effd98842) Get rid of unneeded terminal api methods. ### 📊 Changes **42 files changed** (+628 additions, -2285 deletions) <details> <summary>View changed files</summary> 📝 `src/buffer/out/search.cpp` (+1 -1) 📝 `src/buffer/out/search.h` (+1 -1) 📝 `src/cascadia/PublicTerminalCore/HwndTerminal.cpp` (+8 -8) 📝 `src/cascadia/TerminalControl/ControlCore.cpp` (+9 -21) 📝 `src/cascadia/TerminalControl/ControlCore.h` (+1 -1) ➖ `src/cascadia/TerminalCore/ITerminalApi.hpp` (+0 -80) 📝 `src/cascadia/TerminalCore/Terminal.cpp` (+35 -23) 📝 `src/cascadia/TerminalCore/Terminal.hpp` (+31 -48) 📝 `src/cascadia/TerminalCore/TerminalApi.cpp` (+61 -442) ➖ `src/cascadia/TerminalCore/TerminalDispatch.cpp` (+0 -893) ➖ `src/cascadia/TerminalCore/TerminalDispatch.hpp` (+0 -135) ➖ `src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp` (+0 -267) 📝 `src/cascadia/TerminalCore/lib/terminalcore-lib.vcxproj` (+3 -0) 📝 `src/cascadia/TerminalCore/terminalcore-common.vcxitems` (+0 -4) 📝 `src/cascadia/TerminalCore/terminalrenderdata.cpp` (+2 -2) 📝 `src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp` (+1 -1) 📝 `src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp` (+9 -9) 📝 `src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp` (+10 -10) 📝 `src/host/PtySignalInputThread.cpp` (+13 -5) 📝 `src/host/PtySignalInputThread.hpp` (+2 -5) _...and 22 more files_ </details> ### 📄 Description ## Summary of the Pull Request This PR replaces the `TerminalDispatch` class with the `AdaptDispatch` class from conhost, so we're no longer duplicating the VT functionality in two places. It also gives us a more complete VT implementation on the Terminal side, so it should work better in pass-through mode. ## References This is essentially part two of PR #12703. ## PR Checklist * [x] Closes #3849 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [x] I've discussed this with core contributors already. Issue number where discussion took place: #12662 ## Detailed Description of the Pull Request / Additional comments The first thing was to give the `ConGetSet` interface a new name, since it's now no longer specific to conhost. I went with `ITerminalApi`, since that was the equivalent interface on the terminal side, and it still seemed like a generic enough name. I also changed the way the api is managed by the `AdaptDispatch` class, so it's now stored as a reference rather than a `unique_ptr`, which more closely matches the way the `TerminalDispatch` class worked. I then had to make sure that `AdaptDispatch` actually included all of the functionality currently in `TerminalDispatch`. That meant copying across the code for bracketed paste mode, the copy to clipboard operation, and the various ConEmu OSC operations. This also required a few new methods to the `ConGetSet`/`ITerminalApi` interface, but for now these are just stubs in conhost. Then there were a few thing in the api interface that needed cleaning up. The `ReparentWindow` method doesn't belong there, so I've moved that into `PtySignalInputThread` class. And the `WriteInput` method was too low-level for the Terminal requirements, so I've replaced that with a `ReturnResponse` method which takes a `wstring_view`. It was then a matter of getting the `Terminal` class to implement all the methods in the new `ITerminalApi` interface that it didn't already have. This was mostly mapping to existing functionality, but there are still a number of methods that I've had to leave as stubs for now. However, what we have is still good enough that I could then nuke the `TerminalDispatch` class from the Terminal code and replace it with `AdaptDispatch`. One oddity that came up in testing, though, was the `AdaptDispatch` implementation of `EraseAll` would push a blank line into the scrollback when called on an empty buffer, whereas the previous terminal implementation did not. That caused problems for the conpty connection, because one of the first things it does on startup is send an `ED 2` sequence. I've now updated the `AdaptDispatch` implementation to match the behavior of the terminal implementation in that regard. Another problem was that the terminal implementation of the color table commands had special handling for the background color to notify the application window that it needed to repaint the background. I didn't want to have to push the color table operations through the `ITerminalApi` interface, so I've instead moved the handling of the background update into the renderer, initiated by a flag on the `TriggerRefreshAll` method. ## Validation Steps Performed Surprisingly this PR didn't require a lot of changes to get the unit tests working again. There were just a few methods used from the original `ITerminalApi` that have now been removed, and which needed an equivalent replacement. Also the updated behavior of the `EraseAll` method in conhost resulted in a change to the expected cursor position in one of the screen buffer tests. In terms of manual testing, I've tried out all the different shells in Windows Terminal to make sure there wasn't anything obviously wrong. And I've run a bunch of the tests from _vttest_ to try and get a wider coverage of the VT functionality, and confirmed everything still works at least as well as it used to. I've also run some of my own tests to verify the operations that had to be copied from `TerminalDispatch` to `AdaptDispatch`. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:34:20 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#29338
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?