starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[PR #12703] [MERGED] Further refactor and simplify the ConGetSet API #29193

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

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/12703
Author: @j4james
Created: 3/15/2022
Status: ✅ Merged
Merged: 4/14/2022
Merged by: @undefined

Base: main ← Head: refactor-congetset-2

📝 Commits (10+)

  • 4b8ff8f Add GetTextBuffer and GetViewport to ConGetSet interface.
  • 6f5b9d3 Replace most GetConsoleScreenBufferInfoEx usage.
  • e449c64 Make more use of GetTextBuffer where possible.
  • 981075d Remove unused ConGetSet methods.
  • 93fee50 Move IL/DL implementation to AdaptDispatch.
  • 20ab0c4 Reimplement RI in AdaptDispatch.
  • 6a0744b Remove unused SCREEN_INFORMATION margin methods.
  • 177f5ec Reimplement DECCOLM using ResizeWindow.
  • ae439db Reimplement EraseAll in AdaptDispatch.
  • 23275ab Replace SetWindowInfo with SetViewportPosition.

📊 Changes

28 files changed (+1097 additions, -1974 deletions)

View changed files

📝 src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp (+7 -6)
📝 src/host/PtySignalInputThread.cpp (+4 -2)
📝 src/host/getset.cpp (+0 -3)
📝 src/host/outputStream.cpp (+41 -564)
📝 src/host/outputStream.hpp (+5 -56)
📝 src/host/screenInfo.cpp (+6 -99)
📝 src/host/screenInfo.hpp (+0 -4)
📝 src/host/ut_host/ApiRoutinesTests.cpp (+1 -2)
📝 src/host/ut_host/ClipboardTests.cpp (+1 -1)
📝 src/host/ut_host/CommandLineTests.cpp (+1 -1)
📝 src/host/ut_host/CommandListPopupTests.cpp (+1 -1)
📝 src/host/ut_host/CommandNumberPopupTests.cpp (+1 -1)
📝 src/host/ut_host/ConptyOutputTests.cpp (+1 -1)
📝 src/host/ut_host/CopyFromCharPopupTests.cpp (+1 -1)
📝 src/host/ut_host/CopyToCharPopupTests.cpp (+1 -1)
📝 src/host/ut_host/ObjectTests.cpp (+1 -1)
📝 src/host/ut_host/ScreenBufferTests.cpp (+24 -13)
📝 src/host/ut_host/SelectionTests.cpp (+1 -1)
📝 src/interactivity/inc/IConsoleInputThread.hpp (+1 -1)
📝 src/renderer/base/renderer.hpp (+9 -0)

...and 8 more files

📄 Description

This is an attempt to simplify the ConGetSet interface down to the
smallest set of methods necessary to support the AdaptDispatch class.
The idea is that it should then be easier to implement that interface in
Windows Terminal, so we can replace the TerminalDispatch class with
AdaptDispatch.

This is a continuation of the refactoring started in #12247, and a
significant step towards #3849.

Detailed Description of the Pull Request / Additional comments

The general idea was to give the AdaptDispatch class direct access to
the high-level structures on which it needs to operate. Some of these
structures are now passed in when the class is constructed (the
Renderer, RenderSettings, and TerminalInput), and some are exposed
as new methods in ConGetSet (GetStateMachine, GetTextBuffer, and
GetViewport).

Many of the existing ConhostInternalGetSet methods could easily then
be reimplemented in AdaptDispatch, since they were often simply
forwarding to methods in one of the above structures. Some were a little
more complicated, though, and require further explanation.

  • GetConsoleScreenBufferInfoEx: What we were typically using this for
    was to obtain the viewport, although what we really wanted was the
    virtual viewport, which is now accessible via the GetViewport
    method. This was also used to obtain the cursor position and buffer
    width, which we can now get via the GetTextBuffer method.

  • SetConsoleScreenBufferInfoEx: This was only really used for the
    AdaptDispatch::SetColumns implementation (for DECCOLM mode), and
    that could be replaced with ResizeWindow. This is a slight change in
    behaviour (it sizes the window rather than the buffer), but neither is
    technically correct for DECCOLM, so I think it's good enough for
    now, and at least it's consistent with the other VT sizing operations.

  • SetCursorPosition: This could mostly be replaced with direct
    manipulation of the Cursor object (accessible via the text buffer),
    although again this is a slight change in behavior. The original code
    would also have made a call to ConsoleImeResizeCompStrView (which I
    don't think is applicable to VT movement), and would potentially have
    moved the viewport (not essential for now, but could later be
    supported by DECHCCM). It also called VtIo::SetCursorPosition to
    handle cursor inheritance, but that should only apply to
    InteractDispatch, so I've moved that to the
    InteractDispatch::MoveCursor method.

  • ScrollRegion: This has been replaced by two simple helper methods in
    AdaptDispatch which better meet the VT requirements -
    _ScrollRectVertically and _ScrollRectHorizontally. Unlike the
    original ScrollRegion implementation, these don't generate
    EVENT_CONSOLE_UPDATE_SCROLL events (see #12656 for more details).

  • FillRegion: This has been replaced by the _FillRect helper method
    in AdaptDispatch. It differs from the original FillRegion in that
    it takes a rect rather than a start position and length, which gives
    us more flexibility for future operations.

  • ReverseLineFeed: This has been replaced with a somewhat refactored
    reimplementation in AdaptDispatch, mostly using the
    _ScrollRectVertically helper described above.

  • EraseAll: This was previously handled by
    SCREEN_INFORMATION::VtEraseAll, but has now been entirely
    reimplemented in the AdaptDispatch::_EraseAll method.

  • DeleteLines/InsertLines/_modifyLines: These have been replaced
    by the _InsertDeleteLineHelper method in AdaptDispatch, which
    mostly relies on the _ScrollRectVertically helper described above.

Finally there were a few methods that weren't actually needed in the
ConGetSet interface:

  • MoveToBottom: This was really just a hack to get the virtual
    viewport from GetConsoleScreenBufferInfoEx. We may still want
    something like in the future (e.g. to support DECVCCM or #8879), but
    I don't think it's essential for now.

  • SuppressResizeRepaint: This was only needed in InteractDispatch
    and PtySignalInputThread, and they could easily access the VtIo
    object to implement it themselves.

  • ClearBuffer: This was only used in PtySignalInputThread, and that
    could easily access the buffer directly via the global console
    information.

  • WriteControlInput: This was only used in InteractDispatch, and
    that could easily be replaced with a direct call to
    HandleGenericKeyEvent.

As part of these changes, I've also refactored some of the existing
AdaptDispatch code:

  • _InsertDeleteHelper (renamed _InsertDeleteCharacterHelper) is now
    just a straightforward call to the new _ScrollRectHorizontally
    helper.

  • EraseInDisplay and EraseInLine have been implemented as a series
    of _FillRect calls, so _EraseSingleLineHelper is no longer
    required.

  • _EraseScrollback is a essentially a special form of scrolling
    operation, which mostly depends on the TextBuffer::ScrollRows
    method, and with the filling now provided by the new _FillRect
    helper.

  • There are quite a few operations now in AdaptDispatch that are
    affected by the scrolling margins, so I've pulled out the common
    margin setup into a new _GetVerticalMargins helper method. This also
    fixes some edge cases where margins could end up out of range.

Validation Steps Performed

There were a number of unit tests that needed to be updated to work
around functions that have now been removed, but these substitutions
were fairly straightforward for the most part.

The adapter tests were a different story, though. In that case we were
explicitly testing how operations were passed through to the ConGetSet
interface, but with more than half those methods now gone, a significant
rewrite was required.

I've tried to retain the crux of the original tests, but we now have to
validate the state changes on the underlying data structures, where
before that state would have been tracked in the TestGetSet mock. And
in some cases we were testing what happened when a method failed, but
since that scenario is no longer possible, I've simply removed those
tests.

I've also tried to manually test all the affected operations to confirm
that they're still working as expected, both in vttest as well as my own
test scripts.

Closes #12662

🔄 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/12703 **Author:** [@j4james](https://github.com/j4james) **Created:** 3/15/2022 **Status:** ✅ Merged **Merged:** 4/14/2022 **Merged by:** [@undefined](undefined) **Base:** `main` ← **Head:** `refactor-congetset-2` --- ### 📝 Commits (10+) - [`4b8ff8f`](https://github.com/microsoft/terminal/commit/4b8ff8f1b007369c19ec77a8b4aba7ec84a76570) Add GetTextBuffer and GetViewport to ConGetSet interface. - [`6f5b9d3`](https://github.com/microsoft/terminal/commit/6f5b9d32cc2b3644a1f1f8d889a9f3283ac8ef4b) Replace most GetConsoleScreenBufferInfoEx usage. - [`e449c64`](https://github.com/microsoft/terminal/commit/e449c646d0195a033275e7fab5829913414632ff) Make more use of GetTextBuffer where possible. - [`981075d`](https://github.com/microsoft/terminal/commit/981075daa76fdebf7d8065ebbbd0f23ea3942d04) Remove unused ConGetSet methods. - [`93fee50`](https://github.com/microsoft/terminal/commit/93fee5054d4601773047429b4a14eb9d20a697c4) Move IL/DL implementation to AdaptDispatch. - [`20ab0c4`](https://github.com/microsoft/terminal/commit/20ab0c4b47dff016163f1cf6024bc0ce186a5785) Reimplement RI in AdaptDispatch. - [`6a0744b`](https://github.com/microsoft/terminal/commit/6a0744b3ad4a907b267b89ff417e9fd8d9a7b4cc) Remove unused SCREEN_INFORMATION margin methods. - [`177f5ec`](https://github.com/microsoft/terminal/commit/177f5ecd7d622c88563dc1003d5bd6b4953e819c) Reimplement DECCOLM using ResizeWindow. - [`ae439db`](https://github.com/microsoft/terminal/commit/ae439dbec004d1c494b263e7ecfd0f0ad301c79b) Reimplement EraseAll in AdaptDispatch. - [`23275ab`](https://github.com/microsoft/terminal/commit/23275abf993399f30e5676232392b53df5aa9a27) Replace SetWindowInfo with SetViewportPosition. ### 📊 Changes **28 files changed** (+1097 additions, -1974 deletions) <details> <summary>View changed files</summary> 📝 `src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp` (+7 -6) 📝 `src/host/PtySignalInputThread.cpp` (+4 -2) 📝 `src/host/getset.cpp` (+0 -3) 📝 `src/host/outputStream.cpp` (+41 -564) 📝 `src/host/outputStream.hpp` (+5 -56) 📝 `src/host/screenInfo.cpp` (+6 -99) 📝 `src/host/screenInfo.hpp` (+0 -4) 📝 `src/host/ut_host/ApiRoutinesTests.cpp` (+1 -2) 📝 `src/host/ut_host/ClipboardTests.cpp` (+1 -1) 📝 `src/host/ut_host/CommandLineTests.cpp` (+1 -1) 📝 `src/host/ut_host/CommandListPopupTests.cpp` (+1 -1) 📝 `src/host/ut_host/CommandNumberPopupTests.cpp` (+1 -1) 📝 `src/host/ut_host/ConptyOutputTests.cpp` (+1 -1) 📝 `src/host/ut_host/CopyFromCharPopupTests.cpp` (+1 -1) 📝 `src/host/ut_host/CopyToCharPopupTests.cpp` (+1 -1) 📝 `src/host/ut_host/ObjectTests.cpp` (+1 -1) 📝 `src/host/ut_host/ScreenBufferTests.cpp` (+24 -13) 📝 `src/host/ut_host/SelectionTests.cpp` (+1 -1) 📝 `src/interactivity/inc/IConsoleInputThread.hpp` (+1 -1) 📝 `src/renderer/base/renderer.hpp` (+9 -0) _...and 8 more files_ </details> ### 📄 Description This is an attempt to simplify the `ConGetSet` interface down to the smallest set of methods necessary to support the `AdaptDispatch` class. The idea is that it should then be easier to implement that interface in Windows Terminal, so we can replace the `TerminalDispatch` class with `AdaptDispatch`. This is a continuation of the refactoring started in #12247, and a significant step towards #3849. ## Detailed Description of the Pull Request / Additional comments The general idea was to give the `AdaptDispatch` class direct access to the high-level structures on which it needs to operate. Some of these structures are now passed in when the class is constructed (the `Renderer`, `RenderSettings`, and `TerminalInput`), and some are exposed as new methods in `ConGetSet` (`GetStateMachine`, `GetTextBuffer`, and `GetViewport`). Many of the existing `ConhostInternalGetSet` methods could easily then be reimplemented in `AdaptDispatch`, since they were often simply forwarding to methods in one of the above structures. Some were a little more complicated, though, and require further explanation. * `GetConsoleScreenBufferInfoEx`: What we were typically using this for was to obtain the viewport, although what we really wanted was the virtual viewport, which is now accessible via the `GetViewport` method. This was also used to obtain the cursor position and buffer width, which we can now get via the `GetTextBuffer` method. * `SetConsoleScreenBufferInfoEx`: This was only really used for the `AdaptDispatch::SetColumns` implementation (for `DECCOLM` mode), and that could be replaced with `ResizeWindow`. This is a slight change in behaviour (it sizes the window rather than the buffer), but neither is technically correct for `DECCOLM`, so I think it's good enough for now, and at least it's consistent with the other VT sizing operations. * `SetCursorPosition`: This could mostly be replaced with direct manipulation of the `Cursor` object (accessible via the text buffer), although again this is a slight change in behavior. The original code would also have made a call to `ConsoleImeResizeCompStrView` (which I don't think is applicable to VT movement), and would potentially have moved the viewport (not essential for now, but could later be supported by `DECHCCM`). It also called `VtIo::SetCursorPosition` to handle cursor inheritance, but that should only apply to `InteractDispatch`, so I've moved that to the `InteractDispatch::MoveCursor` method. * `ScrollRegion`: This has been replaced by two simple helper methods in `AdaptDispatch` which better meet the VT requirements - `_ScrollRectVertically` and `_ScrollRectHorizontally`. Unlike the original `ScrollRegion` implementation, these don't generate `EVENT_CONSOLE_UPDATE_SCROLL` events (see #12656 for more details). * `FillRegion`: This has been replaced by the `_FillRect` helper method in `AdaptDispatch`. It differs from the original `FillRegion` in that it takes a rect rather than a start position and length, which gives us more flexibility for future operations. * `ReverseLineFeed`: This has been replaced with a somewhat refactored reimplementation in `AdaptDispatch`, mostly using the `_ScrollRectVertically` helper described above. * `EraseAll`: This was previously handled by `SCREEN_INFORMATION::VtEraseAll`, but has now been entirely reimplemented in the `AdaptDispatch::_EraseAll` method. * `DeleteLines`/`InsertLines`/`_modifyLines`: These have been replaced by the `_InsertDeleteLineHelper` method in `AdaptDispatch`, which mostly relies on the `_ScrollRectVertically` helper described above. Finally there were a few methods that weren't actually needed in the `ConGetSet` interface: * `MoveToBottom`: This was really just a hack to get the virtual viewport from `GetConsoleScreenBufferInfoEx`. We may still want something like in the future (e.g. to support `DECVCCM` or #8879), but I don't think it's essential for now. * `SuppressResizeRepaint`: This was only needed in `InteractDispatch` and `PtySignalInputThread`, and they could easily access the `VtIo` object to implement it themselves. * `ClearBuffer`: This was only used in `PtySignalInputThread`, and that could easily access the buffer directly via the global console information. * `WriteControlInput`: This was only used in `InteractDispatch`, and that could easily be replaced with a direct call to `HandleGenericKeyEvent`. As part of these changes, I've also refactored some of the existing `AdaptDispatch` code: * `_InsertDeleteHelper` (renamed `_InsertDeleteCharacterHelper`) is now just a straightforward call to the new `_ScrollRectHorizontally` helper. * `EraseInDisplay` and `EraseInLine` have been implemented as a series of `_FillRect` calls, so `_EraseSingleLineHelper` is no longer required. * `_EraseScrollback` is a essentially a special form of scrolling operation, which mostly depends on the `TextBuffer::ScrollRows` method, and with the filling now provided by the new `_FillRect` helper. * There are quite a few operations now in `AdaptDispatch` that are affected by the scrolling margins, so I've pulled out the common margin setup into a new `_GetVerticalMargins` helper method. This also fixes some edge cases where margins could end up out of range. ## Validation Steps Performed There were a number of unit tests that needed to be updated to work around functions that have now been removed, but these substitutions were fairly straightforward for the most part. The adapter tests were a different story, though. In that case we were explicitly testing how operations were passed through to the `ConGetSet` interface, but with more than half those methods now gone, a significant rewrite was required. I've tried to retain the crux of the original tests, but we now have to validate the state changes on the underlying data structures, where before that state would have been tracked in the `TestGetSet` mock. And in some cases we were testing what happened when a method failed, but since that scenario is no longer possible, I've simply removed those tests. I've also tried to manually test all the affected operations to confirm that they're still working as expected, both in vttest as well as my own test scripts. Closes #12662 --- <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:33:25 +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#29193
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?