[PR #13024] Merge the TerminalDispatch and AdaptDispatch classes #29342

Open
opened 2026-01-31 09:34:20 +00:00 by claunia · 0 comments
Owner

Original Pull Request: https://github.com/microsoft/terminal/pull/13024

State: closed
Merged: Yes


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.

**Original Pull Request:** https://github.com/microsoft/terminal/pull/13024 **State:** closed **Merged:** Yes --- ## 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`.
claunia added the pull-request label 2026-01-31 09:34:20 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#29342