mirror of
https://github.com/microsoft/terminal.git
synced 2025-12-11 04:20:31 -06:00
87 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
James Holderness
|
4abc041eb7
|
Add support for OSC 52 clipboard copy in conhost (#18949)
This adds support for copying to the clipboard in conhost using the OSC 52 escape sequence, extending the original implementation which was for Windows Terminal only. The Windows Terminal implementation was added in PR #5823. Because the clipboard can't be accessed from a background thread, this works by saving the content in a global variable, and then posting a custom message to the main GUI thread, which takes care of the actual copy operation. Validation: I've manually confirmed that tmux copy mode is now able to copy to the system clipboard. Closes #18943 |
||
|
Leonard Hecker
|
4386bf07fd
|
Avoid focus loops in ConPTY (#17829)
This fixes a lot of subtle issues:
* Avoid emitting another de-/iconify VT sequence when
we encounter a (de)iconify VT sequence during parsing.
* Avoid emitting a de-/iconify VT sequence when
a focus event is received on the signal pipe.
* Avoid emitting such sequences on startup.
* Avoid emitting multiple such sequences
when rapidly un-/focusing the window.
It's also a minor cleanup, because the `GA_ROOTOWNER` is not security
relevant. It was added because there was concern that someone can just
spawn a ConPTY session, tell it that it's focused, and spawn a child
which is now focused. But why would someone do that, when the console
IOCTLs to do so are not just publicly available but also documented?
I also disabled the IME window.
## Validation Steps Performed
* First:
```cpp
int main() {
for (bool show = false;; show = !show) {
printf(show ? "Show in 3s...\n" : "Hide in 3s...\n");
Sleep(3000);
ShowWindow(GetConsoleWindow(), show ? SW_SHOW : SW_HIDE);
}
}
```
* PowerShell 5's `Get-Credential` gains focus ✅
* `sleep 5; Get-Credential` and focus another app. WT should start
blinking in the taskbar. Restore it. The popup has focus ✅
* Run `:hardcopy` in vim: Window is shown centered at (0,0) ✖️
But that's okay because it does that already anyway ✅
* `Connect-AzAccount` doesn't crash PowerShell ✅
|
||
|
James Holderness
|
aa256ad5c9
|
Add support for the S8C1T/S7C1T escape sequences (#17945)
This PR adds support for the `S8C1T` and `S7C1T` commands, which enable an application to choose whether the terminal should use C1 controls when sending key sequences and query responses. This also updates the `DOCS` command to set both the input and output code pages. So when switched to ISO2022 mode, the C1 controls will be transmitted as 8-bit, which is what legacy systems would be expecting. ## Detailed Description of the Pull Request / Additional comments While adding the input code page support, I also reworked the way we handle the code page reset in `RIS`. In the original implementation we saved the active code page when the `DOCS` sequence was first used, and that would become the default value for a reset. With this PR I'm now saving the code pages whenever `SetConsoleCP` or `SetConsoleOutputCP` is called, so those APIs now control what the default values will be. This feels more consistent than the previous approach. And this is how WSL sets its initial code page to UTF-8. ## Validation Steps Performed I've added a couple of unit tests that check one of each applicable C1 control in the key sequences and query reports. I also built myself a code page aware telnet client so I could log into WSL in 8-bit mode, and confirmed that the C1 transmissions are working as expected in vttest. Closes #17931 Tests added/passed |
||
|
James Holderness
|
edfa3ea0f0
|
Remove SetTextAttributes from the ITerminalApi interface (#17685)
The only reason we had the `SetTextAttributes` method in `ITerminalApi` was to allow for conhost to remap the default color attributes when the VT PowerShell quirk was active. Since that quirk has now been removed, there's no need for this API anymore. ## References and Relevant Issues The PowerShell quirk was removed in PR #17666. ## Validation Steps Performed I've had to update all the attribute tests in adapterTest to manually check the expected attributes, since those checks were previously being handled in a `SetTextAttributes` mock which no longer exists. I've also performed some manual tests of the VT attribute operations to double check that they're still working as expected. |
||
|
James Holderness
|
746cf1f148
|
Add support for the VT answerback capability (#17660)
The answerback feature allows for the user to define a message that the terminal will transmit to the host whenever an `ENQ` (enquiry) control character is received. ## Detailed Description of the Pull Request / Additional comments In Windows Terminal, the message can be configured at the profile level of the settings file, as a string property named `AnswerbackMessage`. In ConHost, the message can be configured in the registry, again as a string value with the name `AnswerbackMessage`. ## Validation Steps Performed I've confirmed that the control is working as intended in both Windows Terminal and ConHost using Vttest. Closes #11946 |
||
|
Leonard Hecker
|
450eec48de
|
A minor ConPTY refactoring: Goodbye VtEngine Edition (#17510)
The idea is that we can translate Console API calls directly to VT at least as well as the current VtEngine setup can. For instance, a call to `SetConsoleCursorPosition` clearly translates directly to a `CUP` escape sequence. Effectively, instead of translating output asynchronously in the renderer thread, we'll do it synchronously right during the Console API call. Most importantly, the this means that any VT output that an application generates will now be given to the terminal unmodified. Aside from reducing our project's complexity quite a bit and opening the path towards various interesting work like sixels, Device Control Strings, buffer snapshotting, synchronized updates, and more, it also improves performance for mixed text output like enwik8.txt in conhost to 1.3-2x and in Windows Terminal via ConPTY to roughly 20x. This adds support for overlapped IO, because now that output cannot be "skipped" anymore (VtEngine worked like a renderer after all) it's become crucial to block conhost as little as possible. ⚠️ Intentionally unresolved changes/quirks: * To force a delayed EOL wrap to wrap, `WriteCharsLegacy` emits a `\r\n` if necessary. This breaks text reflow on window resize. We cannot emit ` \r` the way readline does it, because this would overwrite the first column in the next row with a whitespace. The alternative is to read back the affected cell from the buffer and emit that character and its attributes followed by a `\r`. I chose to not do that, because buffer read-back is lossy (= UCS2). Unless the window is resized, the difference is unnoticeable and historically, conhost had no support for buffer reflow anyway. * If `ENABLE_VIRTUAL_TERMINAL_PROCESSING` is set while `DISABLE_NEWLINE_AUTO_RETURN` is reset, we'll blindly replace all LF with CRLF. This may hypothetically break DCS sequences, but it's the only way to do this without parsing the given VT string and thus the only way we can achieve passthrough mode in the future. * `ENABLE_WRAP_AT_EOL_OUTPUT` is translated to `DECAWM`. Between Windows XP and Windows 11 21H2, `ENABLE_WRAP_AT_EOL_OUTPUT` being reset would cause the cursor position to reset to wherever a write started, _if_ the write, including expanded control chars, was less than 100 characters long. If it was longer than that, the cursor position would end up in an effectively random position. After lengthy research I believe that this is a bug introduced in Windows XP and that the original intention was for this mode to be equivalent to `DECAWM`. This is compounded by MSDN's description (emphasis mine): > If this mode is disabled, the **last character** in the row is > overwritten with any subsequent characters. ⚠️ Unresolved issues/quirks: * Focus/Unfocus events are injected into the output stream without checking whether the VT output is currently in a ground state. This may break whatever VT sequence is currently ongoing. This is an existing issue. * `VtIo::Writer::WriteInfos` should properly verify the width of each individual character. * Using `SetConsoleActiveScreenBuffer` destroys surrogate pairs and extended (VT) attributes. It could be translated to VT pages in the long term. * Similarly, `ScrollConsoleScreenBuffer` results in the same and could be translated to `DECCRA` and `DECFRA` in the near term. This is important because otherwise `vim` output may loose its extended attributes during scrolling. * Reflowing a long line until it wraps results in the cooked read prompt to be misaligned vertically. * `SCREEN_INFORMATION::s_RemoveScreenBuffer` should trigger a buffer switch similar to `SetConsoleActiveScreenBuffer`. * Translation of `COMMON_LVB_GRID_HORIZONTAL` to `SGR 53` was dropped and may be reintroduced alongside `UNDERSCORE` = `SGR 4`. * Move the `OSC 0 ; P t BEL` sequence to `WriteWindowTitle` and swap the `BEL` with the `ST` (`ESC \`). * PowerShell on Windows 10 ships with PSReadLine 2.0.0-beta2 which emits SGR 37/40 instead of 39/49. This results in black spaces when typing and there's no good way to fix that. * A test is missing that ensures that `FillConsoleOutputCharacterW` results in a `CSI n J` during the PowerShell shim. * A test is missing that ensures that `PtySignal::ClearBuffer` does not result in any VT being generated. Closes #262 Closes #1173 Closes #3016 Closes #4129 Closes #5228 Closes #8698 Closes #12336 Closes #15014 Closes #15888 Closes #16461 Closes #16911 Closes #17151 Closes #17313 |
||
|
Leonard Hecker
|
c9e200734e
|
Improve Viewport and Viewport::WalkInBounds (#17143)
This removes all of the 2D iteration machinery. Imagine the text buffer as a `Cell[w][h]` grid. Clearly, this is identical to a `Cell[w*h]` array, which shows that copying between overlapping ranges only needs either forward or backward copying, and not left/right/top/down. With `WalkDir` removed, `WalkInBounds` can be rewritten with basic arithmetic which allows `pos` to be an exclusive end coordinate. |
||
|
Carlos Zamora
|
6589957d4d
|
Add Quick Fix UI and support for custom CommandNotFound OSC (#16848)
### `OSC 9001; CmdNotFound; <missingCmd>` Adds support for custom OSC "command not found" sequence `OSC 9001; CmdNotFound; <missingCmd>`. Upon receiving the "CmdNotFound" variant with the missing command payload, we send the missing command up to the Quick Fix menu and add it in as `winget install <missingCmd>`. ### Quick Fix UI The Quick Fix UI is a new UI surface that lives in the gutter (left padding) of your terminal. The button appears if quick fixes are available. When clicked, a list of suggestions appears in a flyout. If there is not enough space in the gutter, the button will be presented in a collapsed version that expands to a normal size upon hovering over it. The Quick Fix UI was implemented similar to the context menu. The UI itself lives in TermControl, but it can be populated by other layers (i.e. TermApp layer). Quick Fix suggestions are also automatically loaded into the Suggestions UI. If a quick fix is available and a screen reader is attached, we dispatch an announcement that quick fixes are available to notify the user that that's the case. Spec: #17005 #16599 ### Follow-ups - #17377: Add a key binding for quick fix - #17378: Use winget to search for packages using `missingCmd` --------- Co-authored-by: Dustin L. Howett <duhowett@microsoft.com> Co-authored-by: Dustin L. Howett <dustin@howett.net> |
||
|
James Holderness
|
4a243f0445
|
Add support for VT paging operations (#16615)
This PR adds support for multiples pages in the VT architecture, along with new operations for moving between those pages: `NP` (Next Page), `PP` (Preceding Page), `PPA` (Page Position Absolute), `PPR` (Page Position Relative), and `PPB` (Page Position Back). There's also a new mode, `DECPCCM` (Page Cursor Coupling Mode), which determines whether or not the active page is also the visible page, and a new query sequence, `DECRQDE` (Request Displayed Extent), which can be used to query the visible page. ## References and Relevant Issues When combined with `DECCRA` (Copy Rectangular Area), which can copy between pages, you can layer content on top of existing output, and still restore the original data afterwards. So this could serve as an alternative solution to #10810. ## Detailed Description of the Pull Request / Additional comments On the original DEC terminals that supported paging, you couldn't have both paging and scrollback at the same time - only the one or the other. But modern terminals typically allow both, so we support that too. The way it works, the currently visible page will be attached to the scrollback, and any content that scrolls off the top will thus be saved. But the background pages will not have scrollback, so their content is lost if it scrolls off the top. And when the screen is resized, only the visible page will be reflowed. Background pages are not affected by a resize until they become active. At that point they just receive the traditional style of resize, where the content is clipped or padded to match the new dimensions. I'm not sure this is the best way to handle resizing, but we can always consider other approaches once people have had a chance to try it out. ## Validation Steps Performed I've added some unit tests covering the new operations, and also done a lot of manual testing. Closes #13892 Tests added/passed |
||
|
Leonard Hecker
|
360e86b536
|
Fix search highlights during reflow (#17092)
This PR extends `til::throttled_func` to also support debouncing: * throttling: "At most 1 call every N seconds" * debouncing: "Exactly 1 call after N seconds of inactivity" Based on the latter the following series of changes were made: * An `OutputIdle` event was added to `ControlCore` which is raised once there hasn't been any incoming data in 100ms. This also triggers an update of our regex patterns (URL detection). * The event is then caught by `TermControl` which calls `Search()`. * `Search()` in turn was modified to return its results by-value as a struct, which avoids the need for a search-update event and simplifies how we update the UI. This architectural change, most importantly the removal of the `TextLayoutUpdated` event, fixes a DoS bug in Windows Terminal: As the event leads to UI thread activity, printing lots of text continuously results in the UI thread becoming unresponsive. On top of these, a number of improvements were made: * `IRenderEngine::InvalidateHighlight` was changed to take the `TextBuffer` by-reference which avoids the need to accumulate the line renditions in a `std::vector` first. This improves Debug build performance during reflow by what I guess must be roughly a magnitude faster. This difference is very noticeable. * When closing the search box, `ClearSearch()` is called to remove the highlights. The search text is restored when it's reopened, however the current search position isn't. Closes #17073 Closes #17089 ## Validation Steps Performed * UIA announcements: * Pressing Ctrl+Shift+F the first time does not lead to one ✅ * Typing the first letter does ✅ * Closing doesn't ✅ * Reopening does (as it restores the letter) ✅ * Closing the search box dismisses the highlights ✅ * Resizing the window recalculates the highlights ✅ * Changing the terminal output while the box is open recalculates the highlights ✅ |
||
|
Tushar Singh
|
90b8bb7c2d
|
Improve Search Highlighting (#16611)
### The changeset involves: - Decoupling Selection and Search Highlighting code paths. - We no longer invalidate search highlights when: - Left-clicking on terminal - A new selection is made - Left-clicking on Search-box - Dispatching Find Next/Prev Match Action. (The search highlight was removed after pressing the first key of the Action's key combination) - And, anything that doesn't change buffer content, shouldn't invalidate the highlighted region (E.g. Cursor movement) - Highlighting foreground color is *actually* applied to the highlighted text. - Double-clicking on SearchBox no longer starts a text selection in the terminal. - Selected text is properly populated in the Search Box (#16355) Closes: #16355  ## Some Implementation Details ### Detecting text layout changes in the Control layer As Search Highlight regions need to be removed when new text is added, or the existing text is re-arranged due to window resize or similar events, a new event `TextLayoutUpdated` is added that notifies `CoreControl` of any text layout changes. The event is used to invalidate and remove all search highlight regions from the buffer (because the regions might not be _fresh_ anymore. The new event is raised when: 1. `AdaptDispatch` writes new text into the buffer. 2. MainBuffer is switched to AltBuffer or vice-versa. 3. The user resized the window. 4. Font size changed. 5. Zoom level changed. (Intensionally,) It's not raised when: 1. Buffer is scrolled. 2. The text cursor is moved. When `ControlCore` receives a `TextLayoutUpdated` event, it clears the Search Highlights in the *render data*, and raises an `UpdateSearchResults` event to notify `TermControl` to update the Search UI (`SearchBoxControl`). In the future, we can use `TextLayoutUpdated` event to start a new search which would refresh the results automatically after a slight delay (throttled). *VSCode already does this today*. ### How does AtlasEngine draw the highlighted regions? We follow a similar idea as for drawing the Selection region. When new regions are available, the old+new regions are marked invalidated. Later, a call to `_drawHighlighted()` is made at the end of `PaintBufferLine()` to override the highlighted regions' colors with highlight colors. The highlighting colors replace the buffer colors while search highlights are active. Note that to paint search highlights, we currently invalidate the row completely. This forces text shaping for the rows in the viewport that have at least one highlighted region. This is done to keep the (already lengthy) PR... simple. We could take advantage of the fact that only colors have changed and not the characters (or glyphs). I'm expecting that this could be improved like: 1. When search regions are added, we add the highlighting colors to the color bitmaps without causing text shaping. 2. When search regions are removed, we re-fill the color bitmaps with the original colors from the Buffer. ## Validation Steps: - New text, window resize, font size changes, zooming, and pasting content into the terminal removes search highlights. - highlighting colors override the foreground and background color of the text (in the rendered output). - Blinking, faded, reverse video, Intense text is highlighted as expected. |
||
|
Leonard Hecker
|
5f3a857192
|
Replace WinRT clipboard API with Win32 for copying (#17006)
In the spirit of #15360 this implements the copy part. The problem is that we have an issue accessing the clipboard while other applications continue to work just fine. The major difference between us and the others is that we use the WinRT clipboard APIs. So, the idea is that we just use the Win32 APIs instead. The feel-good side-effect is that this is (no joke) 200-1000x faster, but I suspect no one will notice the -3ms difference down to <0.01ms. The objective effect however is that it just works. This may resolve #16982. ## Validation Steps Performed * Cycle through Text/HTML/RTF-only in the Interaction settings * Paste the contents into Word each time * Text is plain and HTML/RTF are colored ✅ |
||
|
Mike Griese
|
c3f44f7730
|
Rewrite how marks are stored & add reflow (#16937)
This is pretty much a huge refactoring of how marks are stored in the buffer. Gone is the list of `ScrollMark`s in the buffer that store regions of text as points marking the ends. Those would be nigh impossible to reflow nicely. Instead, we're going to use `TextAttribute`s to store the kind of output we've got - `Prompt`, `Command`, `Output`, or, the default, `None`. Those already reflow nicely! But we also need to store things like, the exit code for the command. That's why we've now added `ScrollbarData` to `ROW`s. There's really only going to be one prompt->output on a single row. So, we only need to store one ScrollbarData per-row. When a command ends, we can just go update the mark on the row that started that command. But iterating over the whole buffer to find the next/previous prompt/command/output region sounds complicated. So, to avoid everyone needing to do some variant of that, we've added `MarkExtents` (which is literally just the same mark structure as before). TextBuffer can figure out where all the mark regions are, and hand that back to callers. This allows ControlCore to be basically unchanged. _But collecting up all the regions for all the marks sounds expensive! We need to update the scrollbar frequently, we can't just collect those up every time!_ No we can't! But we also don't need to. The scrollbar doesn't need to know where all the marks start and end and if they have commands and this and that - no. We only need to know the rows that have marks on them. So, we've now also got `ScrollMark` to represent just a mark on a scrollbar at a specific row on the buffer. We can get those quickly. * [x] I added a bunch of tests for this. * [x] I played with it and it feels good, even after a reflow (finally) * See: * #11000 * #15057 (I'm not marking this as closed. The stacked PR will close this, when I move marks to Stable) |
||
|
James Holderness
|
33589cd8db
|
Add support for the DECSWT (Set Window Title) escape sequence (#16804)
This PR adds support for the `OSC 21` sequence used on DEC terminals to set the window title. It's just an alias of the `OSC 2` title sequence used by XTerm. This PR also corrects the handling of blank title sequences, which are supposed to reset the title to its default value, but were previously ignored. ## Detailed Description of the Pull Request / Additional comments To handle the blank title parsing correctly, I had to make some changes to the state machine. Previously it would not have dispatched an `OSC` sequence unless it received a semicolon following the `OSC` number, but when there's a blank string, that semicolon should not be required. I also took this opportunity to simplify the `OSC` parsing in the state machine, and eliminate the `_GetOscTitle` method which no longer served any purpose. ## Validation Steps Performed I've manually confirmed that the title sequences are now working as expected, and added some state machine unit tests covering the blank value handling for these sequences. I also had to update one of the existing state machine tests to account for the changes I made to allow the semicolon to be omitted. Closes #16783 Closes #16784 |
||
|
Leonard Hecker
|
ab7a2f10c5
|
Fix scroll-forward-disable setting (#16411)
The final parameter, `updateBottom`, controls not just whether the
`_virtualBottom` is updated, but also whether the position is clamped
to be within the existing `_virtualBottom`. Setting this to `false`
thus broke scroll-forward as the `_virtualBottom` was now a constant.
## Validation Steps Performed
* Disable scroll-foward
* Press and hold Ctrl+C
* It scrolls past the viewport bottom ✅
|
||
|
Leonard Hecker
|
0da37a134a
|
Avoid encoding VT via win32 input mode (#16407)
This changeset avoids re-encoding output from `AdaptDispatch` via the win32-input-mode mechanism when VT input is enabled. That is, an `AdaptDispatch` output like `\x1b[C` would otherwise result in dozens of characters of input. Related to #16343 ## Validation Steps Performed * Replace conhost with this * Launch a Win32 application inside WSL * ASCII keyboard inputs are represented as single `INPUT_RECORD`s ✅ |
||
|
Leonard Hecker
|
741633ef7a
|
ConPTY: Avoid WINDOW_BUFFER_SIZE_EVENT when the viewport moves (#15935)
`SetConsoleWindowInfoImpl` calls `PostUpdateWindowSize`, which emits a `CM_SET_WINDOW_SIZE` event, which causes `_InternalSetWindowSize` to be called, which calls `ScreenBufferSizeChange` which then finally emits a `WINDOW_BUFFER_SIZE_EVENT` event into the client input buffer. This messes up applications like which make use of `WINDOW_BUFFER_SIZE_EVENT` to perform potentially lossy operations. In case of SSH this results in a resize (SIGWINCH) of the server-side screen which similarly may result in a response by the shell, etc. Since that happens over networks and is async, and because our conhost VT viewport implementation appears to have a number of subtle bugs, this results in duplicate output lines (sometimes hundreds). Under Windows Terminal this issue is not as apparent, since ConPTY has no viewport that can be moved and no scrollback. It only appears as an issue if a terminal application reacts poorly to the SIGWINCH event. Closes #15769 ## Validation Steps Performed * Set a breakpoint in `SynthesizeWindowBufferSizeEvent` * Launch WSL and cause the viewport to move down No calls to `SynthesizeWindowBufferSizeEvent` ✅ * Execute `tput reset` Input line moves to row 0 ✅ |
||
|
Mike Griese
|
a0c88bb511
|
Add Suggestions UI & experimental shell completions support (#14938)
There's two parts to this PR that should be considered _separately_. 1. The Suggestions UI, a new graphical menu for displaying suggestions / completions to the user in the context of the terminal the user is working in. 2. The VsCode shell completions protocol. This enables the shell to invoke this UI via a VT sequence. These are being introduced at the same time, because they both require one another. However, I need to absolutely emphasize: ### THE FORMAT OF THE COMPLETION PROTOCOL IS EXPERIMENTAL AND SUBJECT TO CHANGE This is what we've prototyped with VsCode, but we're still working on how we want to conclusively define that protocol. However, we can also refine the Suggestions UI independently of how the protocol is actually implemented. This will let us rev the Suggestions UI to support other things like tooltips, recent commands, tasks, INDEPENDENTLY of us rev'ing the completion protocol. So yes, they're both here, but let's not nitpick that protocol for now. ### Checklist * Doesn't actually close anything * Heavily related to #3121, but I'm not gonna say that's closed till we settle on the protocol * See also: * #1595 * #14779 * https://github.com/microsoft/vscode/pull/171648 ### Detailed Description #### Suggestions UI The Suggestions UI is spec'ed over in #14864, so go read that. It's basically a transient Command Palette, that floats by the user's cursor. It's heavily forked from the Command Palette code, with all the business about switching modes removed. The major bit of new code is `SuggestionsControl::Anchor`. It also supports two "modes": * A "palette", which is like the command palette - a list with a text box * A "menu", which is more like the intellisense flyout. No text box. This is the mode that the shell completions use #### Shell Completions Protocol I literally cannot say this enough times - this protocol is experimental and subject to change. Build on it at your own peril. It's disabled in Release builds (but available in preview behind `globals.experimental.enableShellCompletionMenu`), so that when it ships, no one can take a dependency on it accidentally. Right now we're just taking a blob of JSON, passing that up to the App layer, who asks `Command` to parse it and build a list of `sendInput` actions to populate the menu with. It's not a particularly elegant solution, but it's good enough to prototype with. #### How do I test this? I've been testing this in two parts. You'll need a snippet in your powershell profile, and a keybinding in the Terminal settings to trigger it. The work together by binding <kbd>Ctrl+space</kbd> to _essentially_ send <kbd>F12</kbd><kbd>b</kbd>. Wacky, but it works. ```json { "command": { "action": "sendInput","input": "\u001b[24~b" }, "keys": "ctrl+space" }, ``` ```ps1 function Send-Completions2 { $commandLine = "" $cursorIndex = 0 # TODO: Since fuzzy matching exists, should completions be provided only for character after the # last space and then filter on the client side? That would let you trigger ctrl+space # anywhere on a word and have full completions available [Microsoft.PowerShell.PSConsoleReadLine]::GetBufferState([ref]$commandLine, [ref]$cursorIndex) $completionPrefix = $commandLine # Get completions $result = "`e]633;Completions" if ($completionPrefix.Length -gt 0) { # Get and send completions $completions = TabExpansion2 -inputScript $completionPrefix -cursorColumn $cursorIndex if ($null -ne $completions.CompletionMatches) { $result += ";$($completions.ReplacementIndex);$($completions.ReplacementLength);$($cursorIndex);" $result += $completions.CompletionMatches | ConvertTo-Json -Compress } } $result += "`a" Write-Host -NoNewLine $result } function Set-MappedKeyHandlers { # VS Code send completions request (may override Ctrl+Spacebar) Set-PSReadLineKeyHandler -Chord 'F12,b' -ScriptBlock { Send-Completions2 } } # Register key handlers if PSReadLine is available if (Get-Module -Name PSReadLine) { Set-MappedKeyHandlers } ``` ### TODO * [x] `(prompt | format-hex).`<kbd>Ctrl+space</kbd> -> This always throws an exception. Seems like the payload is always clipped to ```{"CompletionText":"Ascii","ListItemText":"Ascii","ResultType":5,"ToolTip":"string Ascii { get``` and that ain't JSON. Investigate on the pwsh side? |
||
|
Leonard Hecker
|
5b44476048
|
Replace IInputEvent with INPUT_RECORD (#15673)
`IInputEvent` makes adding Unicode support to `InputBuffer` more
difficult than necessary as the abstract class makes downcasting
as well as copying quite verbose. I found that using `INPUT_RECORD`s
directly leads to a significantly simplified implementation.
In addition, this commit fixes at least one bug: The previous approach
to detect the null key via `DoActiveModifierKeysMatch` didn't work.
As it compared the modifier keys as a bitset with `==` it failed to
match whenever the numpad key was set, which it usually is.
## Validation Steps Performed
* Unit and feature tests are ✅
|
||
|
Mike Griese
|
b4042eaaf0
|
Let marks be cleared by clear (and friends) (#15686)
Move scroll marks to `TextBuffer`, so they can be cleared by EraseInDisplay and EraseScrollback. Also removes the namespacing on them. ## References and Relevant Issues * see also #11000 and #15057 * Resize/Reflow _doesn't_ work yet and I'm not attempting this here. ## Validation Steps Performed * `cls` works * `Clear-Host` works * `clear` works * the "Clear buffer" action works * They work when there's marks above the current viewport, and clear the scrollback * they work if you clear multiple "pages" of output, then scroll back to where marks previously were * resizing doesn't totally destroy the marks Closes #15426 |
||
|
Leonard Hecker
|
c183d12649
|
Move AdaptDispatch::_FillRect into TextBuffer (#15541)
This commit makes 2 changes: * Expose dirty-range information from `ROW::CopyTextFrom` This will allow us to call `TriggerRedraw`, which is an aspect I haven't previously considered as something this API needs. * Add a `FillRect` API to `TextBuffer` and refactor `AdeptDispatch` to use that API. Even if we determine that the new text APIs are unfit (for instance too difficult to use), this will make it simpler to write efficient implementations right inside `TextBuffer`. Since the new `FillRect` API lacks bounds checks the way `WriteLine` has them, it breaks `AdaptDispatch::_EraseAll` which failed to adjust the bottom parameter after scrolling the contents. This would result in more rows being erased than intended. ## Validation Steps Performed * `chcp 65001` * Launch `pwsh` * ``"`e[29483`$x"`` fills the viewport with cats ✅ * `ResizeTraditional` still doesn't work any worse than it used to ✅ |
||
|
James Holderness
|
7a3bf7017c
|
Add support for Erase Color Mode (DECECM) (#15469)
The _Erase Color Mode_ determines what attributes are written to the buffer when erasing content, or when new content is scrolled onto the screen. When the mode is reset (which is the default), we erase with the active colors, but with rendition attributes cleared. When the mode is set, we erase with the default attributes, i.e. with neither color nor rendition attributes applied. This could be used to address the problem described in issue #10556. Most of the affected operations are handled within the `AdaptDispatch` class, so I've simply updated them all to use a new helper method which returns the appropriate erase attributes for the active mode. However, there were a couple of operations that are handled elsewhere, and which now require the erase attributes to be passed to them as a parameter. * The `TextBuffer::IncrementCircularBuffer` method, which is used to recycle the topmost row when scrolling past the bottom of the buffer. * The `TextBuffer::SetCurrentLineRendition` method, which has to clear the second half of the line when switching to a double width rendition. * The `ITerminalApi::UseAlternateScreenBuffer` method, which has to clear the screen when switching to the alternate buffer. Then there is also a Clear Buffer action in Windows Terminal, which is ultimately handled by the `SCREEN_INFORMATION::ClearBuffer` method in ConHost. That class has no access to the erase color mode, so has no way of knowing which attributes to use. So I've now rewritten it to use the `AdaptDispatch::EraseInDisplay` method to handle the erasing. ## Validation Steps Performed I wrote a little test script that exercises the operations affected by `DECECM`, which @al20878 kindly tested for us on a real DEC VT525, and provided screenshots of the output. I've manually confirmed that our implementation exactly matches those results. I've also added a unit test that runs through the same set of operations and verified that each of them is using the appropriate attributes when `DECECM` is enabled and enabled. Closes #14983 |
||
|
James Holderness
|
3d737214a4
|
Add support for LNM (Line Feed/New Line Mode) (#15261)
This PR adds support for the ANSI Line Feed/New Line mode (`LNM`), which determines whether outputting a linefeed control should also trigger a carriage return, and whether the `Return` key should generate an `LF` in addition to `CR`. ## Detailed Description of the Pull Request / Additional comments In ConHost, there was already a console mode which handled the output side of things, but I've now also added a `TerminalInput` mode that controls the behavior of the `Return` key. When `LNM` is set, both the output and input modes are enabled, and when reset, they're disabled. If they're not already matching, then `LNM` has no effect, and will be reported as unknown when queried. This is the typical state for legacy console applications, which expect a linefeed to trigger a carriage return, but wouldn't want the `Return` key generating both `CR`+`LF`. As part of this PR, I've also refactored the `ITerminalApi` interface to consolidate what I'm now calling the "system" modes: bracketed paste, auto wrap, and the new line feed mode. This closes another gap between Terminal and ConHost, so both auto wrap, and line feed mode will now be supported for conpty pass through. ## Validation Steps Performed I've added an `LNM` test that checks the escape sequence is triggering both of the expected mode changes, and added an additional `DECRQM` test covering the currently implemented standard modes: the new `LNM`, and the existing `IRM` (which wasn't previously tested). I've also extended the `DECRQM` private mode test to cover `DECAWM` and Bracketed Paste (which we also weren't previously testing). I've manually tested `LNM` in Vttest to confirm the keyboard is working as expected. Closes #15167 |
||
|
James Holderness
|
8c28e132b5
|
Preserve active attributes during VT resize operations (#15269)
## Summary of the Pull Request When the screen is resized in ConHost via a VT escape sequence, the active text attributes could end up being corrupted if they were set to something that the legacy console APIs didn't support (e.g. RGB colors). This PR fixes that issue. ## Detailed Description of the Pull Request / Additional comments The way a resize is implemented is by retrieving the buffer information with `GetConsoleScreenBufferInfoEx`, updating the size fields, and then writing the data back out again with `SetConsoleScreenBufferInfoEx`. However, this also results in the active attributes being updated via the `wAttributes` field, and that's only capable of representing legacy console attributes. We address this by saving the full `TextAttribute` value before it gets corrupted in the `SetConsoleScreenBufferInfoEx` call, and then restore it again afterwards. ## Validation Steps Performed I've added a unit test to verify the attributes are correctly preserved for all VT resize operations, and I've also manually confirmed the test case in #2540 is now working as expected. ## PR Checklist - [x] Closes #2540 - [x] Tests added/passed - [ ] Documentation updated - [ ] Schema updated (if necessary) |
||
|
James Holderness
|
6030616d27
|
Add support for bracketed paste mode in ConHost (#15155)
This adds support for XTerm's "bracketed paste" mode in ConHost. When enabled, any pasted text is bracketed with a pair of escape sequences, which lets the receiving application know that the content was pasted rather than typed. ## References and Relevant Issues Bracketed paste mode was added to Windows Terminal in PR #9034. Adding it to ConHost ticks one more item off the list in #13408. ## Detailed Description of the Pull Request / Additional comments This only applies when VT input mode is enabled, since that is the way Windows Terminal currently works. When it comes to filtering, though, the only change I've made is to filter out the escape character, and only when bracketed mode is enabled. That's necessary to prevent any attempts to bypass the bracketing, but I didn't want to mess with the expected behavior for legacy apps if bracketed mode is disabled. ## Validation Steps Performed Manually tested in bash with `bind 'set enable-bracketed-paste on'` and confirmed that pasted content is now buffered, instead of being executed immediately. Also tested in VIM, and confirmed that you can now paste preformatted code without the autoindent breaking the formatting. Closes #395 |
||
|
James Holderness
|
fc95802531
|
Merge the LineFeed functionality into AdaptDispatch (#14874)
The main purpose of this PR was to merge the `ITerminalApi::LineFeed` implementations into a shared method in `AdaptDispatch`, and avoid the VT code path depending on the `AdjustCursorPosition` function (which could then be massively simplified). This refactoring also fixes some bugs that existed in the original `LineFeed` implementations. ## References and Relevant Issues This helps to close the gap between the Conhost and Terminal (#13408). This improves some of the scrollbar mark bugs mentioned in #11000. ## Detailed Description of the Pull Request / Additional comments I had initially hoped the line feed functionality could be implemented entirely within `AdaptDispatch`, but there is still some Conhost and Terminal-specific behavior that needs to be triggered when we reach the bottom of the buffer, and the row coordinates are cycled. In Conhost we need to trigger an accessibility scroll event, and in Windows Terminal we need to update selection and marker offsets, reset pattern intervals, and preserve the user's scroll offset. This is now handled by a new `NotifyBufferRotation` method in `ITerminalApi`. But this made me realise that the `_EraseAll` method should have been doing the same thing when it reached the bottom of the buffer. So I've added a call to the new `NotifyBufferRotation` API from there as well. And in the case of Windows Terminal, the scroll offset preservation was something that was also needed for a regular viewport pan. So I've put that in a separate `_PreserveUserScrollOffset` method which is called from the `SetViewportPosition` handler as well. ## Validation Steps Performed Because of the API changes, there were a number of unit tests that needed to be updated: - Some of the `ScreenBufferTests` were accessing margin state in the `SCREEN_INFORMATION` class which doesn't exist anymore, so I had to add a little helper function which now manually detects the active margins. - Some of the `AdapterTest` tests were dependent on APIs that no longer exist, so they needed to be rewritten so they now check the resulting state rather than expecting a mock API call. - The `ScrollWithMargins` test in `ConptyRoundtripTests` was testing functionality that didn't previously work correctly (issue #3673). Now that it's been fixed, that test needed to be updated accordingly. Other than getting the unit tests working, I've manually verified that issue #3673 is now fixed. And I've also checked that the scroll markers, selections, and user scroll offset are all being updated correctly, both with a regular viewport pan, as well as when overrunning the buffer. Closes #3673 |
||
|
James Holderness
|
282c583731
|
Make all console output modes more strictly buffer state (#14735)
The original console output modes were considered attributes of the buffer, while later additions were treated as global state, and yet both were accessed via the same buffer-based API. This could result in the reported modes being out of sync with the way the system was actually behaving, and a call to `SetConsoleMode` without updating anything could still trigger unpredictable changes in behavior. This PR attempts to address that problem by making all modes part of the buffer state, and giving them predictable default values. While this won't solve all the tmux layout-breaking issues in #6987, it does at least fix one case which was the result of an unexpected change in the `DISABLE_NEWLINE_AUTO_RETURN` mode. All access to the output modes is now done via the `OutputMode` field in `SCREEN_INFORMATION`. The fields that were tracking global state in the `Settings` class (`_fAutoReturnOnNewline` and `_fRenderGridWorldwide`) have now been removed. We still have a global `_dwVirtTermLevel` field, though, but that now serves as a default value for the `ENABLE_VIRTUAL_TERMINAL_PROCESSING` mode when creating a new buffer. It's enabled for conpty mode, and when the VT level in the registry is not 0. That default doesn't change. For the VT alternate buffer, things works slightly differently, since there is an expectation that VT modes are global. So when creating an alt buffer, we copy the current modes from the main buffer, and when it's closed, we copy them back again. ## Validation Steps Performed I've manually confirmed that this fixes the problem described in issue #14690. I've also added a basic feature test that confirms the modes are initialized as expected when creating new buffers, and changes to the modes in one buffer do not impact any other buffers. Closes #14690 |
||
|
James Holderness
|
a1865b9cf7
|
Merge the PrintString functionality into AdaptDispatch (#14640)
The main purpose of this PR was to merge the `ITerminalApi::PrintString` implementations into a shared method in `AdaptDispatch`, and avoid the VT code path depending on `WriteCharsLegacy`. But this refactoring has also fixed some bugs that existed in the original implementations. This helps to close the gap between the Conhost and Terminal (#13408). I started by taking the `WriteCharsLegacy` implementation, and stripping out everything that didn't apply to the VT code path. What was left was a fairly simple loop with the following steps: 1. Check if _delayed wrap_ is set, and if so, move to the next line. 2. Write out as much of the string as will fit on the current line. 3. If we reach the end of the line, set the _delayed wrap_ flag again. 4. Repeat the loop until the entire string has been output. But step 2 was a little more complicated than necessary because of its legacy history. It was copying the string into a temporary buffer, manually estimated how much of it would fit, and then passing on that partial buffer to the `TextBuffer::Write` method. In the new implementation, we just pass the entire string directly to `TextBuffer::WriteLine`, and that handles the clipping itself. The returned `OutputCellIterator` tells us how much of the string is left. This approach fixes some issues with wide characters, and should also cope better with future buffer enhancements. Another improvement from the new implementation is that the Terminal now handles delayed EOL wrap correctly. However, the downside of this is that it introduced a cursor-dropping bug that previously only affected conhost. I hadn't originally intended to fix that, but it became more of an issue now. The root cause was the fact that we called `cursor.StartDeferDrawing()` before outputting the text, and this was something I had adopted in the new implementation as well. But I've now removed that, and instead just call `cursor.SetIsOn(false)`. This seems to avoid the cursor droppings, and hopefully still has similar performance benefits. The other thing worth mentioning is that I've eliminated some special casing handling for the `ENABLE_VIRTUAL_TERMINAL_PROCESSING` mode and the `WC_DELAY_EOL_WRAP` flag in the `WriteCharsLegacy` function. They were only used for VT output, so aren't needed here anymore. ## Validation Steps Performed I've just been testing manually, writing out sample text both in ASCII and with wide Unicode chars. I've made sure it wraps correctly when exceeding the available space, but doesn't wrap when stopped at the last column, and with `DECAWM` disabled, it doesn't wrap at all. I've also confirmed that the test case from issue #12739 is now working correctly, and the cursor no longer disappears in Windows Terminal when writing to the last column (i.e. the delayed EOL wrap is working). Closes #780 Closes #6162 Closes #6555 Closes #12440 Closes #12739 |
||
|
Mike Griese
|
37aa29545b
|
Implement the rest of the FTCS marks (#14341)
As noted in #11000. This adds support for `FTCS_COMMAND_START`, `FTCS_COMMAND_EXECUTED` and `FTCS_COMMAND_FINISHED`, which allow a shell to more clearly markup parts of the buffer. As a trick, I'm also making the `experimental.autoMarkPrompts` setting act like a `FTCS_COMMAND_EXECUTED` if it comes after a `FTCS_COMMAND_START`. This lets the whole sequence work for cmd.exe (which wouldn't otherwise be possible). * My cmd prompt ```bat PROMPT $e]133;D$e\$e]133;A$e\$e]9;9;$P$e\[$T]$e[97;46m%_seperator%$P$e[36;49m%_seperator%$e[0m$_$e[0m%_GITPROMPT%$e[94m%username%$e[0m@$e[32m%computername%$e[0m$G$e]133;B$e\ ``` * pwsh profile, heavily cribbed from vscode ```pwsh $Global:__LastHistoryId = -1 function Global:__Terminal-Get-LastExitCode { if ($? -eq $True) { return 0 } # TODO: Should we just return a string instead? # return -1 if ("$LastExitCode" -ne "") { return $LastExitCode } return -1 } function prompt { # $gle = $LastExitCode $gle = $(__Terminal-Get-LastExitCode); $LastHistoryEntry = $(Get-History -Count 1) # Skip finishing the command if the first command has not yet started if ($Global:__LastHistoryId -ne -1) { if ($LastHistoryEntry.Id -eq $Global:__LastHistoryId) { # Don't provide a command line or exit code if there was no history entry (eg. ctrl+c, enter on no command) $out += "`e]133;D`a" } else { # Command finished exit code # OSC 633 ; D [; <ExitCode>] ST $out += "`e]133;D;$gle`a" } } $loc = $($executionContext.SessionState.Path.CurrentLocation); # IMPORTANT: Make sure there's a printable charater _last_ in the prompt. # Otherwise, PSReadline is gonna use the terminating `\` here and colorize # that if it detects a syntax error $out += "`e]133;A$([char]07)"; $out += "`e]9;9;`"$loc`"$([char]07)"; $out += "PWSH $loc$('>' * ($nestedPromptLevel + 1)) "; $out += "`e]133;B$([char]07)"; $Global:__LastHistoryId = $LastHistoryEntry.Id return $out } ``` * Doesn't close any issues, because this was always just an element in #11000 * I work here * From FHL code |
||
|
Leonard Hecker
|
0eff8c06e3
|
Clean up til::point/size/rect member usage (#14458)
This is a follow-up of #13025 to make the members of `til::point/size/rect` uniform and consistent without the use of `unions`. The only file that has any changes is `src/host/getset.cpp` where an if condition was simplified. ## Validation Steps Performed * Host unit tests ✅ * Host feature tests ✅ * ControlCore feature tests ✅ |
||
|
James Holderness
|
437914807a
|
Add support for the DECRQM escape sequence (#14444)
This PR adds support for the `DECRQM` (Request Mode) escape sequence, which allows applications to query the state of the various modes supported by the terminal. It also adds support for the `DECNKM` mode, which aliases the existing `DECKPAM` and `DECKPNM` operations, so they can be queried with `DECRQM` too. This is one solution for #10153 (saving and restoring the state of bracketed paste mode), and should also help with #1040 (providing a way for clients to determine the capabilities of the terminal). Prior to adding `DECRQM`, I also did some refactoring of the mode handling to get rid of the mode setting methods in the `ITermDispatch` interface that had no need to be there. Most of them were essentially a single line of code that could easily be executed directly from the `_ModeParamsHelper` handler anyway. As part of this refactoring I combined all the internal `AdaptDispatch` modes into an `enumset` to allow for easier management, and made sure all modes were correctly reset in the `HardReset` method (prior to this, there were a number of modes that we weren't restoring when we should have been). And note that there are some differences in behavior between conhost and Windows Terminal. In conhost, `DECRQM` will report bracketed paste mode as unsupported, and in Terminal, both `DECCOLM` and `AllowDECCOLM` are reported as unsupported. And `DECCOLM` is now explicitly ignored in conpty mode, to avoid the conpty client and conhost getting out of sync. ## Validation Steps Performed I've manually confirmed that all the supported modes are reported in the `DECRQM` tests in Vttest, and I have my own test scripts which I've used to confirm that `RIS` is now resetting the modes correctly. I've also added a unit test in `AdapterTest` that iterates through the modes, checking the responses from `DECRQM` for both the set and reset states. I should also mention that I had to do some refactoring of the existing tests to compensate for methods that were removed from `ITermDispatch`, particularly in `OutputEngineTest`. In many cases, though, these tests weren't doing much more than testing the test framework. |
||
|
Leonard Hecker
|
b4fce27203
|
Skip DECPS/MIDI output on Ctrl+C/Break (#14214)
Silent MIDI notes can be used to seemingly deny a user's input for long
durations (multiple minutes). This commit improves the situation by ignoring
all DECPS sequences for a second when Ctrl+C/Ctrl+Break is pressed.
Additionally it fixes a regression introduced in 666c446:
When we close a tab we need to unblock/shutdown `MidiAudio` early,
so that `ConptyConnection::Close()` can run down as fast as possible.
## Validation Steps Performed
* In pwsh in Windows Terminal 1.16 run ``while ($True) { echo "`e[3;8;3,~" }``
* Ctrl+C doesn't do anything ❎
* Closing the tab doesn't do anything ❎
* With these modifications in Windows Terminal:
* Ctrl+C stops the output ✅
* Closing the tab completes instantly ✅
* With these modifications in OpenConsole:
* Ctrl+C stops the output ✅
* Closing the window completes instantly ✅
|
||
|
Mike Griese
|
848314ef17
|
Fix a deadlock in ShowWindow (#13309)
When we send this ShowWindow message, if we send it to it's going to need to get processed by the window message thread before returning. We're handling this message under lock. However, the first thing the conhost message thread does is lock the console. That'll deadlock us. So unlock here, first, to let the message thread deal with this message, then re-lock so later on this thread can unlock again safely. * [x] Closes #13301 * [x] Tested conhost * [x] Tested terminal |
||
|
Mike Griese
|
799b5d4add
|
Experimental: add support for scrollbar marks (#12948)
Adds support for marks in the scrollbar. These marks can be added in 3 ways: * Via the iterm2 `OSC 1337 ; SetMark` sequence * Via the `addMark` action * Automatically when the `experimental.autoMarkPrompts` per-profile setting is enabled. #11000 has more tracking for the big-picture for this feature, as well as additional follow-ups. This set of functionality seemed complete enough to send a review for now. That issue describes these how I wish these actions to look in the fullness of time. This is simply the v0.1 from the hackathon last month. #### Actions * `addMark`: add a mark to the buffer. If there's a selection, use place the mark covering at the selection. Otherwise, place the mark on the cursor row. - `color`: a color for the scrollbar mark. This is optional - defaults to the `foreground` color of the current scheme if omitted. * `scrollToMark` - `direction`: `["first", "previous", "next", "last"]` * `clearMark`: Clears marks at the current postition (either the selection if there is one, or the cursor position. * `clearAllMarks`: Don't think this needs explanation. #### Per-profile settings * `experimental.autoMarkPrompts`: `bool`, default `false`. * `experimental.showMarksOnScrollbar`: `bool` ## PR Checklist * [x] Closes #1527 * [x] Closes #6232 ## Detailed Description of the Pull Request / Additional comments This is basically hackathon code. It's experimental! That's okay! We'll figure the rest of the design in post. Theoretically, I should make these actions `experimental.` as well, but it seemed like since the only way to see these guys was via the `experimental.showMarksOnScrollbar` setting, you've already broken yourself into experimental jail, and you know what you're doing. Things that won't work as expected: * resizing, ESPECIALLY reflowing * Clearing the buffer with ED sequences / Clear Buffer I could theoretically add velocity around this in the `TermControl` layer. Always prevent marks from being visible, ignore all the actions. Marks could still be set by VT and automark, but they'd be useless. Next up priorities: * Making this work with the FinalTerm sequences * properly speccing * adding support for `showMarksOnScrollbar: flags(categories)`, so you can only display errors on the scrollbar * adding the `category` flag to the `addMark` action ## Validation Steps Performed I like using it quite a bit. The marks can get noisy if you have them emitted on every prompt and the buffer has 9000 lines. But that's the beautiful thing, the actions work even if the marks aren't visible, so you can still scroll between prompts. <details> <summary>Settings blob</summary> ```jsonc // actions { "keys": "ctrl+up", "command": { "action": "scrollToMark", "direction": "previous" }, "name": "Previous mark" }, { "keys": "ctrl+down", "command": { "action": "scrollToMark", "direction": "next" }, "name": "Next mark" }, { "keys": "ctrl+pgup", "command": { "action": "scrollToMark", "direction": "first" }, "name": "First mark" }, { "keys": "ctrl+pgdn", "command": { "action": "scrollToMark", "direction": "last" }, "name": "Last mark" }, { "command": { "action": "addMark" } }, { "command": { "action": "addMark", "color": "#ff00ff" } }, { "command": { "action": "addMark", "color": "#0000ff" } }, { "command": { "action": "clearAllMarks" } }, // profiles.defaults "experimental.autoMarkPrompts": true, "experimental.showMarksOnScrollbar": true, ``` </details> |
||
|
Leonard Hecker
|
ed27737233
|
Use 32-bit coordinates throughout the project (#13025)
Previously this project used a great variety of types to present text buffer coordinates: `short`, `unsigned short`, `int`, `unsigned int`, `size_t`, `ptrdiff_t`, `COORD`/`SMALL_RECT` (aka `short`), and more. This massive commit migrates almost all use of those types over to the centralized types `til::point`/`size`/`rect`/`inclusive_rect` and their underlying type `til::CoordType` (aka `int32_t`). Due to the size of the changeset and statistics I expect it to contain bugs. The biggest risk I see is that some code potentially, maybe implicitly, expected arithmetic to be mod 2^16 and that this code now allows it to be mod 2^32. Any narrowing into `short` later on would then throw exceptions. ## PR Checklist * [x] Closes #4015 * [x] I work here * [x] Tests added/passed ## Validation Steps Performed Casual usage of OpenConsole and Windows Terminal. ✅ |
||
|
James Holderness
|
9dca6c27ee
|
Add support for the DECPS (Play Sound) escape sequence (#13208)
## Summary of the Pull Request The `DECPS` (Play Sound) escape sequence provides applications with a way to play a basic sequence of musical notes. This emulates functionality that was originally supported on the DEC VT520 and VT525 hardware terminals. ## PR Checklist * [x] Closes #8687 * [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: #8687 ## Detailed Description of the Pull Request / Additional comments When a `DECPS` control is executed, any further output is blocked until all the notes have finished playing. So to prevent the UI from hanging during this period, we have to temporarily release the console/terminal lock, and then reacquire it before returning. The problem we then have is how to deal with the terminal being closed during that unlocked interval. The way I've dealt with that is with a promise that is set to indicate a shutdown. This immediately aborts any sound that is in progress, but also signals the thread that it needs to exit as soon as possible. The thread exit is achieved by throwing a custom exception which is recognised by the state machine and rethrown instead of being logged. This gets it all the way up to the root of the write operation, so it won't attempt to process anything further output that might still be buffered. ## Validation Steps Performed Thanks to the testing done by @jerch on a real VT525 terminal, we have a good idea of how this sequence is supposed to work, and I'm fairly confident that our implementation is reasonably compatible. The only significant difference I'm aware of is that we support multiple notes in a sequence. That was a feature that was documented in the VT520/VT525 manual, but didn't appear to be supported on the actual device. |
||
|
Mike Griese
|
77215d9d77
|
Fix ShowWindow(GetConsoleWindow()) (#13118)
A bad merge, that actually revealed a horrible bug. There was a secret conflict between the code in #12526 and #12515. 69b77ca was a bad merge that hid just how bad the issue was. Fixing the one line `nullptr`->`this` in `InteractivityFactory` resulted in a window that would flash uncontrollably, as it minimized and restored itself in a loop. Great. This can seemingly be fixed by making sure that the conpty window is initially created with the owner already set, rather than relying on a `SetParent` call in post. This does pose some complications for the #1256 future we're approaching. However, this is a blocking bug _now_, and we can figure out the tearout/`SetParent` thing in post. * fixes #13066. * Tested with the script in that issue. * Window doesn't flash uncontrollably. * `gci | ogv` still works right * I work here. * Opening a new tab doesn't spontaneously cause the window to minimize * Restoring from minimized doesn't yeet focus to an invisible window * Opening a new tab doesn't yeet focus to an invisible window * There _is_ a viable way to call `GetAncestor` s.t. it returns the Terminal's hwnd in Terminal, and the console's in Conhost The `SW_SHOWNOACTIVATE` change is also quite load bearing. With just `SW_NORMAL`, the pseudo window (which is invisible!) gets activated whenever the terminal window is restored from minimized. That's BAD. There's actually more to this as well. Calling `SetParent` on a window that is `WS_VISIBLE` will cause the OS to hide the window, make it a _child_ window, then call `SW_SHOW` on the window to re-show it. `SW_SHOW`, however, will cause the OS to also set that window as the _foreground_ window, which would result in the pty's hwnd stealing the foreground away from the owning terminal window. That's bad. `SetWindowLongPtr` seems to do the job of changing who the window owner is, without all the other side effects of reparenting the window. Without `SetParent`, however, the pty HWND is no longer a descendant of the Terminal HWND, so that means `GA_ROOT` can no longer be used to find the owner's hwnd. For even more insanity, without `WS_POPUP`, none of the values of `GetAncestor` will actually get the terminal HWND. So, now we also need `WS_POPUP` on the pty hwnd. To get at the Terminal hwnd, you'll need ```c++ GetAncestor(GetConsoleWindow(), GA_ROOTOWNER) ``` |
||
|
James Holderness
|
f4e0d9f2bd
|
Merge the TerminalDispatch and AdaptDispatch classes (#13024)
## 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`. |
||
|
James Holderness
|
e3f4ec88ec
|
Prevent the virtual viewport bottom being updated incorrectly (#12972)
The "virtual bottom" marks the last line of the mutable viewport area, which is the part of the buffer that VT sequences can write to. This region should typically only move downwards as new lines are added to the buffer, but there were a number of cases where it was incorrectly being moved up, or moved down further than necessary. This PR attempts to fix that. There was an earlier, unsuccessful attempt to fix this in PR #9770 which was later reverted (issue #9872 was the reason it had to be reverted). PRs #2666, #2705, and #5317 were fixes for related virtual viewport problems, some of which have either been extended or superseded by this PR. `SetConsoleCursorPositionImpl` is one of the cases that actually does need to move the virtual viewport upwards sometimes, in particular when the cmd shell resets the buffer with a `CLS` command. But when this operation "snaps" the viewport to the location of the cursor, it needs to use the virtual viewport as the frame of reference. This was partially addressed by PR #2705, but that only applied in terminal-scrolling mode, so I've now applied that fix regardless of the mode. `SetViewportOrigin` takes a flag which determines whether it will also move the virtual bottom to match the visible viewport. In some case this is appropriate (`SetConsoleCursorPositionImpl` being one example), but in other cases (e.g. when panning the viewport downwards in the `AdjustCursorPosition` function), it should only be allowed to move downwards. We can't just not set the update flag in those cases, because that also determines whether or not the viewport would be clamped, and we don't want change that. So what I've done is limit `SetViewportOrigin` to only move the virtual bottom downwards, and added an explicit `UpdateBottom` call in those places that may also require upward movement. `ResizeWindow` in the `ConhostInternalGetSet` class has a similar problem to `SetConsoleCursorPositionImpl`, in that it's updating the viewport to account for the new size, but if that visible viewport is scrolled back or forward, it would end up placing the virtual viewport in the wrong place. So again the solution here was to use the virtual viewport as the frame of reference for the position. However, if the viewport is being shrunk, this can still result in the cursor falling below the bottom, so we need an additional check to adjust for that. This can't be applied in pty mode, though, because that would break the conpty resizing operation. `_InternalSetViewportSize` comes into play when resizing the window manually, and again the viewport after the resize can end up truncating the virtual bottom if not handled correctly. This was partially addressed in the original code by clamping the new viewport above the virtual bottom under certain conditions, and only in terminal scrolling mode. I've replaced that with a new algorithm which links the virtual bottom to the visible viewport bottom if the two intersect, but otherwise leaves it unchanged. This applies regardless of the scrolling mode. `ResizeWithReflow` is another sizing operation that can affect the virtual bottom. This occurs when a change of the window width requires the buffer to be reflowed, and we need to reposition the viewport in the newly generated buffer. Previously we were just setting the virtual bottom to align with the new visible viewport, but that could easily result in the buffer truncation if the visible viewport was scrolled back at the time. We now set the virtual bottom to the last non-space row, or the cursor row (whichever is larger). There'll be edge cases where this is probably not ideal, but it should still work reasonably well. `MakeCursorVisible` was another case where the virtual bottom was being updated (when requested with a flag) via a `SetViewportOrigin` call. When I checked all the places it was used, though, none of them actually required that behavior, and doing so could result in the virtual bottom being incorrectly positioned, even after `SetViewportOrigin` was limited to moving the virtual bottom downwards. So I've now made it so that `MakeCursorVisible` never updates the virtual bottom. `SelectAll` in the `Selection` class was a similar case. It was calling `SetViewportOrigin` with the `updateBottom` flag set when that really wasn't necessary and could result in the virtual bottom being incorrectly set. I've changed the flag to false now. ## Validation Steps Performed I've manually confirmed that the test cases in issue #9754 are working now, except for the one involving margins, which is bigger problem with `AdjustCursorPosition` which will need to be addressed separately. I've also double checked the test cases from several other virtual bottom issues (#1206, #1222, #5302, and #9872), and confirmed that they're still working correctly with these changes. And I've added a few screen buffer tests in which I've tried to cover as many of the problematic code paths as possible. Closes #9754 |
||
|
Michael Niksa
|
6b936d9a74
|
Propagate show/hide window calls against the ConPTY pseudo window to the Terminal (#12515)
Propagate show/hide window calls against the ConPTY pseudo window to the Terminal ## PR Checklist * [x] Closes #12570 * [x] I work here * [x] Manual Tests passed * [x] Spec Link: →[Doc Link](https://github.com/microsoft/terminal/blob/dev/miniksa/msgs/doc/specs/%2312570%20-%20Show%20Hide%20operations%20on%20GetConsoleWindow%20via%20PTY.md)← ## Detailed Description of the Pull Request / Additional comments - See the spec. It's pretty much everything I went through deciding on this. ## Validation Steps Performed - [x] Manual validation against scratch application calling all of the `::ShowWindow` commands against the pseudo console "fake window" and observing the real terminal window state |
||
|
Leonard Hecker
|
57c3953aca
|
Use type inference throughout the project (#12975)
#4015 requires sweeping changes in order to allow a migration of our buffer coordinates from `int16_t` to `int32_t`. This commit reduces the size of future commits by using type inference wherever possible, dropping the need to manually adjust types throughout the project later. As an added bonus this commit standardizes the alignment of cv qualifiers to be always left of the type (e.g. `const T&` instead of `T const&`). The migration to type inference with `auto` was mostly done using JetBrains Resharper with some manual intervention and the standardization of cv qualifier alignment using clang-format 14. ## References This is preparation work for #4015. ## Validation Steps Performed * Tests pass ✅ |
||
|
Leonard Hecker
|
7af134cc7f
|
Introduce VTInt to represent VT parameters (#12207)
This commit replaces our use of `size_t` to represent VT parameters with `int32_t`. While unsigned integers have the inherent benefit of being less ambiguous and enjoying two's complement, our buffer coordinates use signed integers. Since a number of VT functions need to convert their parameters to coordinates, this commit makes the conversion easier. The benefit of this change becomes even more apparent if one considers that a number of places performed unsafe conversions of their size_t parameters to int or short already. Files that had to be modified were converted to use til wrappers instead of COORD or SMALL_RECT wherever possible. ## References This commit contains about 20% of the work for #4015. ## PR Checklist * [x] I work here * [x] Tests added/passed ## Validation Steps Performed I'm mostly relying on our unit tests here. Both OpenConsole and WT appear to work fine. |
||
|
Mike Griese
|
0da5bd7726
|
Ensure a terminal requesting FG rights actually has them (#12899)
#### ⚠️ _Targets #12799_ ⚠️ This is an atomic bit of code that partners with #12799. It's separated as an individual PR to keep diffs more simple. This ensures that when a terminal tells ConPTY that it's focused, that ConPTY doesn't do the `ConsoleControl(CONSOLE_FOREGROUND` thing unless the terminal application is actually in the foreground. This prevents a trivial exploit whereby a `malicious.exe` could create a PTY, tell ConPTY it has focus (when it doesn't), then use this mechanism to launch an instance of itself into the foreground. When the terminal tells us it's in the foreground, we're gonna look at the owner of the ConPTY window handle. If that owner has focus, then cool, this is allowed. Otherwise, we won't grant them the FG right. For this to work, the terminal just have already called `ReparentPseudoConsole`. * built on top of #12799 and #12526 * [x] Part of #2988 * [x] Tested manually. |
||
|
James Holderness
|
7f5caa1ba7
|
Further refactor and simplify the ConGetSet API (#12703)
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 |
||
|
Mike Griese
|
26d67d9c0a
|
Enable the Terminal to tell ConPTY who the owner is (#12526)
## Window shenanigans, part the first: This PR enables terminals to tell ConPTY what the owning window for the pseudo window should be. This allows thigs like MessageBoxes created by console applications to work. It also enables console apps to use `GetAncestor(GetConsoleWindow(), GA_ROOT)` to get directly at the HWND of the Terminal (but _don't please_). This is tested with our internal partners and seems to work for their scenario. See #2988, #12799, #12515, #12570. ## PR Checklist This is 1/3 of #2988. |
||
|
Michael Niksa
|
2c2f4f9be2
|
[FHL] Make VTApiRoutines, which does VT translation for output (#11264)
Make a VTApiRoutines servicer that does minimal translations instead of environmental simulation for some output methods. Remaining methods are backed on the existing console host infrastructure (primarily input related methods). ## PR Checklist * [x] I work here * [x] It's Fix-Hack-Learn quality so it's behind a feature gate so we can keep refining it. But it's a start! To turn this on, you will have to be in the Dev or Preview rings (feature staged). Then add `experimental.connection.passthroughMode: true` to a profile and on the next launch, the flags will propagate down through the `ConptyConnection` into the underlying `Openconsole.exe` startup and tell it to use the passthrough mode instead of the full simulation mode. ## Validation Steps Performed - Played with it manually in CMD.exe, it seems to work mostly. - Played with it manually in Ubuntu WSL, it seems to work. - Played with it manually in Powershell and it's mostly sad. It'll get there. Starts #1173 |
||
|
James Holderness
|
f936c4443d
|
Eliminate the AdaptDefaults class (#12390)
## Summary of the Pull Request The only method that was really needed in the `AdaptDefault` class was the `PrintString` method, and that could easily be moved into the `ConGetSet` interface. That lets us get rid of the `AdaptDefault` class altogether, simplifying the construction of the `AdaptDispatch` class, and brings us more in line with the `TerminalDispatch` implementation. ## PR Checklist * [x] Closes #12318 * [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: #12318 ## Detailed Description of the Pull Request / Additional comments The `Execute` method was never used at all, so there was no problem with losing that. But I also noticed there was an equivalent `ExecuteChar` method in the `ITerminalApi` interface, which also wasn't being used, so I've removed that too. Then there was a `Print` method taking a single `wchar_t` parameter, but that was ultimately implemented as a `PrintString` call anyway, so that translation could easily be accomplished in the `AdaptDispatch` calling code, the same way it's done in `TerminalDispatch`. That left us with the `PrintString` method, which could simply be moved into the `ConGetSet` interface, which would then more closely match the `ITerminalApi` interface. There was also a `GetResult` method, that returned the status of the last `PrintString`, but that result was never actually checked anywhere. What I've done now is make the `PrintString` method throw an exception on failure, and that will be caught and logged in the `StateMachine`. ## Validation Steps Performed I've started a bash shell in conhost to verify that it still works. Since almost everything goes through `PrintString` in VT mode, it should be obvious if something was broken. |
||
|
James Holderness
|
d63ab27c0c
|
Eliminate the DispatchCommon class (#12389)
## Summary of the Pull Request Other than the `s_ResizeWindow` function, the `DispatchCommon` class was just a couple of static functions indirectly calling the `ConGetSet` interface. So by moving the `s_ResizeWindow` implementation into the `ConhostInternalGetSet` class, we could easily replace all usage of `DispatchCommon` with direct calls to `ConGetSet`. ## PR Checklist * [x] Closes #12253 * [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: #12253 ## Validation Steps Performed I've manually confirmed the resizing operations still work as expected. The other functions are harder to test, but were trivial replacements. |
||
|
James Holderness
|
5238235268
|
Refactor and simplify the ConGetSet API (#12247)
## Summary of the Pull Request This PR refactors the `ConGetSet` API, eliminating some of the bloat produced by the `DoSrvPrivateXXXX` functions, simplifying the method naming to more closely match the `ITerminalApi` interface, and making better use of exceptions for error conditions in place of boolean return values. ## References This is another small step towards merging the `AdaptDispatch` and `TerminalDispatch` classes (#3849). ## PR Checklist * [x] Closes #12193 * [x] Closes #12194 * [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: #3849 ## Detailed Description of the Pull Request / Additional comments There are two main parts to this. The first step was to get rid of all the `DoSrvPrivateXXX` functions, and move their implementation directly into the `ConhostInternalGetSet` class. For the most part this was just copying and pasting the code, but I also fixed a couple of bugs where we were using the wrong output buffer (the global buffer rather than the one associated with the output handle), and got rid of some unnecessary calls to `GetActiveBuffer`. The second part was to make better use of exceptions for error conditions. Instead of catching the exceptions at the `ConGetSet` level, we now allow them to fall through all the way to the `StateMachine`. This greatly simplifies the `AdaptDispatch` implementation since it no longer needs to check a boolean return value on every `ConGetSet` call. This also enables the getter methods to return properties directly instead of having to use a reference parameter. ## Validation Steps Performed A number of the unit tests had to be updated to match the new API. Sometimes this just required changes to method names, but in other cases error conditions that were previously detected with boolean returns now needed to be caught as exceptions. There were also a few direct calls to `DoSrvPrivateXXX` functions that now needed to be invoked through other means: either by generating an equivalent escape sequence, or calling a lower level API serving the same purpose. And in the adapter tests, the mock `ConGetSet` implementation required significant refactoring to match the new interface, mostly to account for the changes in error handling. |
||
|
James Holderness
|
62c95b5017
|
Move the common render settings into a shared class (#12127)
## Summary of the Pull Request This PR moves the color table and related render settings, which are common to both conhost and Windows Terminal, into a shared class that can be accessed directly from the renderer. This avoids the overhead of having to look up these properties via the `IRenderData` interface, which relies on inefficient virtual function calls. This also introduces the concept of color aliases, which determine the position in the color table that colors like the default foreground and background are stored. This allows the option of mapping them to one of the standard 16 colors, or to have their own separate table entries. ## References This is a continuation of the color table refactoring started in #11602 and #11784. The color alias functionality is a prerequisite for supporting a default bold color as proposed in #11939. The color aliases could also be a way for us to replace the PowerShell color quirk for #6807. ## PR Checklist * [x] Closes #12002 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Detailed Description of the Pull Request / Additional comments In addition to the color table, this new `RenderSettings` class manages the blinking state, the code for adjusting indistinguishable colors, and various boolean properties used in the color calculations. These boolean properties are now stored in a `til::enumset` so they can all be managed through a single `SetRenderMode` API, and easily extended with additional modes that we're likely to need in the future. In Windows Terminal we have an instance of `RenderSettings` stored in the `Terminal` class, and in conhost it's stored in the `Settings` class. In both cases, a reference to this class is passed to the `Renderer` constructor, so it now has direct access to that data. The renderer can then pass that reference to the render engines where it's needed in the `UpdateDrawingBrushes` method. This means the renderer no longer needs the `IRenderData` interface to access the `GetAttributeColors`, `GetCursorColor`, or `IsScreenReversed` methods, so those have now been removed. We still need access to `GetAttributeColors` in certain accessibility code, though, so I've kept that method in the `IUIAData` interface, but the implementation just forwards to the `RenderSettings` class. The implementation of the `RenderSettings::GetAttributeColors` method is loosely based on the original `Terminal` code, only the `CalculateRgbColors` call has now been incorporated directly into the code. This let us deduplicate some bits that were previously repeated in the section for adjusting indistinguishable colors. The last steps, where we calculate the alpha components, have now been split in to a separate `GetAttributeColorsWithAlpha` method, since that's typically not needed. ## Validation Steps Performed There were quite a lot changes needed in the unit tests, but they're mostly straightforward replacements of one method call with another. In the `TextAttributeTests`, where we were previously testing the `CalculateRgbColors` method, we're now running those tests though `RenderSettings::GetAttributeColors`, which incorporates the same functionality. The only complication is when testing the `IntenseIsBright` option, that needs to be set with an additional `SetRenderMode` call where previously it was just a parameter on `CalculateRgbColors`. In the `ScreenBufferTests` and `TextBufferTests`, calls to `LookupAttributeColors` have again been replaced by the `RenderSettings::GetAttributeColors` method, which serves the same purpose, and calls to `IsScreenReversed` have been replaced with an appropriate `GetRenderMode` call. In the `VtRendererTests`, all the calls to `UpdateDrawingBrushes` now just need to be passed a reference to a `RenderSettings` instance. |