Implements the **FTCS_PROMPT** sequence, `OSC 133 ; A ST`. In this PR, it's just used to set a simple Prompt mark on the current line, in the same way that the iTerm2 sequence works.
There's rumination in #11000 on how to implement the rest of the FTCS sequences.
This is broken into its own PR at the moment. [Quoth j4james](https://github.com/microsoft/terminal/pull/12948#issuecomment-1136360132):
> That should be just as easy, and I've noticed a couple of other terminals that are doing that, so it's not unprecedented. If we don't have any immediate use for the other options, there shouldn't be any harm in ignoring them initially.
>
> And the benefit of going with the more widely supported sequence is that we're more likely to benefit from any shells that have this functionality built in. Otherwise they're forced to try and detect the terminal, which is practically impossible for Windows Terminal. Even iTerm2 supports the `OSC 133` sequence, so we'd probably be the only odd one out.
This part of the plumbing is super easy, so I thought it would be valuable to add regardless if we get to the whole of FTCS in 1.15.
* [x] I work here
* [x] Tested manually - in my pwsh `$PROFILE`:
```pwsh
function prompt {
$loc = $($executionContext.SessionState.Path.CurrentLocation);
$out = "PS $loc$('>' * ($nestedPromptLevel + 1)) ";
$out += "$([char]27)]9;9;`"$loc`"$([char]07)";
$out += "$([char]27)]133;A;$([char]7)"; # add the FTCS_PROMPT to the... well, end, but you get the point
return $out
}
```
* See also #11000
* conhost requires an additional dependency in Windows, which might
cause us trouble in WPG
* Terminal requires an additional *package* dependency, which *will*
cause us trouble in WPG (since GmDls is about 3MB)
I chose to scope the feature checks to MidiOut directly, as I wanted to
keep the delay behavior in MidiAudio::PlayNote. This is negotiable.
References #13252
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>
ed27737 contains a regression where (pseudocode)
```c
unsigned long ulActualDelta;
short ScreenInfo.WheelDelta;
delta *= (ScreenInfo.WheelDelta / (short)ulActualDelta);
// ^^^^^^^
```
was changed to
```c
delta *= (ScreenInfo.WheelDelta / ulActualDelta);
```
Due to `ulActualDelta` being unsigned, the new code casts the signed integer
to a unsigned one first, before doing the division. This causes scrolling
downwards (`WheelDelta` is negative) to appear as a large positive `delta`.
## PR Checklist
* [x] Closes#13253
* [x] I work here
## Validation Steps Performed
* Scrolling up/down works in OpenConsole again ✅
When #13160 introduced a new interface to the IConsoleHandoff idl, it
changed midl's RPC proxy stub lookup algorithm from a direct GUID
comparison to an unrolled binary search. Now, that would ordinarily not
be a problem...
However, in #11610, we took a shortcut and replaced `memcmp` -- used
only by RPC for GUID comparison -- with a direct GUID-only equality
comparator. This worked totally fine, and ordinarily would not be a
problem...
The unrolled binary search unfortunately _relies on memcmp's contract_:
it uses memcmp to match against a fully sorted set. Our memcmp only
returned 0 or 1 (equal or not), and it knew nothing about ordering.
When a package that contains a PackagedCOM proxy stub is installed, it
is selected as the primary proxy stub for any interfaces it can proxy.
After all, interfaces are immutable, so it doesn't matter whose proxy
you're using. Now, given that we installed a *broken* proxy... *all*
IIDs that got whacked by our memcmp issue broke for every consumer.
To fix it: instead of implementing memcmp ourselves, we're just going to
take a page out of WinAppSDK's book and link this binary using the
"Hybrid CRT" model. It will statically link any parts of the STL it uses
(none) and dynamically link the ucrt (which is guaranteed to be present
on Windows.)
Sure, the binary size goes up from 8k to 24k, but... the cost is never
having to worry about this again.
Closes#13251
## Summary of the Pull Request
When a profile gets deleted, we were navigating to the next item assuming it was a profile when it may not be. This commit fixes this by checking the tag of the next menu item before we navigate to it.
## PR Checklist
* [x] Closes#13125
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I work here
## Validation Steps Performed
Deleting the last profile in the SUI doesn't cause a crash
If we want to make Windows Terminal the default terminal under Windows,
we'll have to make conhost "handoff" incoming connections by default.
But this poses a problem: How can the seldomly updated conhost know
whether the routinely updated Windows Terminal version is actually willing
to accept such handoffs by default (it might be unwilling due to bugs, etc.)?
This commit solves the issue by introducing:
* A marker interface (`IDefaultTerminalMarker`): If it exists,
Windows Terminal indicates its willingness to accept the handoff.
* Turning the all-0 GUID from being synonymous for conhost,
to being synonymous for "Let Windows decide". Without this we wouldn't
be able to differentiate between users who consciously chose conhost
as their default terminal, vs. users who want the standard behavior.
## Validation Steps Performed
Testing fallback behavior:
* Install "Terminal" 1.13
* Delete the 2 keys below `HKCU\Console\%%Startup`
* Enable `Feature_AttemptHandoff` in `features.xml`
Return `true` from `DefaultApp::CheckShouldTerminalBeDefault`
* Replace `conhost.exe` and `console.dll` with `sfpcopy` after building
* Launching `cmd.exe` launches as a conhost window ✅
(because "Terminal" 1.13 lacks the marker interface)
* Open properties page in `conhost.exe`
"Let Windows decide" is select by default ✅
* Changing the selection writes the new value ✅
Testing the new behavior:
* Delete the 2 keys below `HKCU\Console\%%Startup`
* Enable `Feature_AttemptHandoff` in `features.xml`
Return `true` from `DefaultApp::CheckShouldTerminalBeDefault`
* Use `CLSID_WindowsTerminalConsoleDev` and `CLSID_WindowsTerminalTerminalDev`
for the initialization of `TerminalDelegationPair`
* Replace `conhost.exe` and `console.dll` with `sfpcopy` after building
* Deploy the "Terminal Dev" package
* Launching `cmd.exe` launches "Terminal Dev" ✅
(because "Terminal Dev" has the marker interface)
* Open the settings tab
"Let Windows decide" is select by default ✅
* Changing the selection and saving writes the new value ✅
## Summary of the Pull Request
This introduced the `toggleBlockSelection` action to allow users to create a block selection using only the keyboard. This is not bound to any keys by default, however it is added to the command palette.
## References
#4993 - Epic
#5804 - Spec
## Validation Steps Performed
- [X] Mark mode always starts in line selection mode
- [X] Mouse selections are always in line selection mode by default
- [X] Can toggle block selection for an existing selection (regardless of how it was created)
- [X] The selection is copied properly (aka, no rendering issues)
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. ✅
This PR introduces the framework for the `DECRSTS` sequence which is
used to restore terminal state reports. But to start with, I've just
implemented the `DECCTR` color table report, which provides a way for
applications to alter the terminal's color scheme.
## PR Checklist
* [x] Closes#13132
* [x] CLA signed.
* [x] 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
I've added the functions for parsing DEC RGB and HLS color formats into
the `Utils` class, where we've got all our other color parsing routines,
since this functionality will eventually be needed in other VT protocols
like Sixel and ReGIS.
Since `DECRSTS` is a `DCS` sequence, this only works in conhost for now,
or when using the experimental passthrough mode in Windows Terminal.
## Validation Steps Performed
I've added a number of unit tests to check that the `DECCTR` report is
being interpreted as expected. This includes various edge cases (e.g.
omitted and out-of-range parameters), which I have confirmed to match
the color parsing on a real VT240 terminal.
## 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.
MSFT-33471786 is one of the most common crashes we have right now.
Memory dumps suggest that `VtEngine::UpdateViewport` is called with a rectangle
like `(0, 46, 119, 29)` (left, top, right, bottom), which is a rectangle of
negative height. When the `_invalidMap` is resized the negative size gets
turned into a very large unsigned integer, which results in an OOM exception,
crashing OpenConsole.
`VtEngine::UpdateViewport` is called by `Renderer::_CheckViewportAndScroll`
which holds a (cached) old and a new viewport. The old viewport was
`(0, 46, 119, 75)` which is exceedingly similar to the invalid, new viewport.
It's bottom coordinate is also coincidentally larger by exactly 46 (top).
The viewport comes from the `SCREEN_INFORMATION` class whose `SetViewport`
function was highly suspicious as it has a branch which updates the bottom
to be the buffer height, but leaves the top unmodified.
`SCREEN_INFORMATION::SetViewport` is called by `SetConsoleWindowInfo` which
processes user-provided data. A repro of the crash can be constructed with:
```
SMALL_RECT rect{0, 46, 119, 75};
SetConsoleWindowInfo(GetStdHandle(STD_OUTPUT_HANDLE), TRUE, &rect);
```
Closes#13193
Closes MSFT-33471786
## Validation Steps Performed
Ensured the following code doesn't crash when run under Windows Terminal:
```
SMALL_RECT rect{0, 46, 119, 75};
SetConsoleWindowInfo(GetStdHandle(STD_OUTPUT_HANDLE), TRUE, &rect);
```
This commit hides the "Open in Terminal" context menu option when the
context menu is opened in a non-filesystem path like "Quick Actions".
Closes#12578
This is a big hammer to put out this fire. We're keeping the hiding around for now, cause we think that's likely the one that the internal tests use that we really care about here. If we need to bring this back, we can.
* [x] Closes#13158
* [x] Closes#13162
* [x] Validated these both manually
* [x] `[Native]::ShowWindow([Native]::GetConsoleWindow(), 6)` still works
Prevents a null pointer dereference when attempting to split a settings tab, due to it not being a terminal tab.
* [x] Closes#13166
## Validation Steps Performed
Manually tested.
This only impacts the UI. We can take a workitem to rename the loc data
later. When the user specifies zh-Hans/zh-Hant, the resource mapper does
the right thing.
Related to #8984
Use a throttled update to update our window state. Throttling should prevent scenarios where the Terminal window state and PTY window state get de-sync'd, and cause the window to minimize/restore constantly in a loop. "Should" is doing a lot of work in this sentence.
A 200ms delay was chosen because it's the typical animation timeout in Windows. This does result in a delay between the PTY requesting a change to the window state and the Terminal realizing it, but should mitigate issues where the Terminal and PTY get desync'd.
I think we're overall not super confident that this fixes the root causes of the issue. Rather, we're hopeful that a small amount of throttling here should leave time for the Terminal and pty to sync back up. We're comfortable enough with that as a bandaid for 1.14 preview, to see how this behaves in the wild.
ThemeResources are a persistent pain.
Regressed in #13083. See also #12775 et. al.
We can't just put those here though as StaticResources, because XAML will evaluate their values when the App is first loaded, and we'll always use the value from the OS theme, regarless of the requested theme. Kinda the same thing we've had to do with TabViewBackground in the past.
* [x] Fixes something we noticed right before shipping
We're doing it this way because ThemeResources are tricky. We
default in XAML to using the appropriate ThemeResource background
color for our TabRow. When tabs in the titlebar are _disabled_,
this will ensure that the tab row has the correct theme-dependent
value. When tabs in the titlebar are _enabled_ (the default),
we'll switch the BG to Transparent, to let the Titlebar Control's
background be used as the BG for the tab row.
We can't do it the other way around (default to Transparent, only
switch to a color when disabling tabs in the titlebar), because
looking up the correct ThemeResource from and App dictionary is a
capital-H Hard problem.
* [x] Closes#13143
* [x] I work here
* [x] validated manually:
- [x] showTabsInTitlebar: false, true
- [x] useAcrylicInTabRow: false, true
- [x] theme: light, dark
* [x] Need to check if this is regressed the same in 1.13. I suspect it is.
## Summary of the Pull Request
Introduces a non-configurable version of mark mode to Windows Terminal. It has the following interactions defined:
- <kbd>ctrl+shift+m</kbd> --> Enter Mark Mode
- when in Mark Mode...
- <kbd>ESC</kbd> --> Exit Mark Mode
- arrow keys --> move "start"
- <kbd>shift</kbd> + arrow keys --> anchor "start", move "end"
- <kbd>ctrl+a</kbd> --> select all
- when a selection is active...
When in mark mode, the cursor does not blink.
## References
#4993 - [Epic] Keyboard Selection
## PR Checklist
* [X] Closes#715
* [X] Provides a resolution for #11985
## Detailed Description of the Pull Request / Additional comments
- `TermControl`:
- `TermControl.cpp` just adds logic to prevent the cursor from blinking when in mark mode
- `ControlCore`
- in the same place we handle quick edit, we add an entry point to mark mode
- `TerminalCore`
- this leverages `UpdateSelection()` and other quick edit functions to make mark mode happen
## Validation Steps Performed
- [x] Make selection, split pane, close pane
- NOTE: A similar scenario caused a crash at one point. Really weird. Keep an eye on it.
- [x] Cursor is off when in mark mode
- [x] general movement/selection
- [x] general movement/selection that forces the viewport to move
- [x] In mark mode, selectAll...
- [x] arrow keys --> move start
- [x] shift + arrow keys --> move end
- [x] (regardless of mark mode) if selection active, enter --> copy to clipboard
Well this one feels dumb.
Make sure to also initially set the visibility of ConPTY windows created for DefTerm connections.
* [x] Closes#13066 for real.
* [x] tested manually.
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)
```
This PR adds support for the VT line rendition attributes in the DirectX
renderer, which allows for double-width and double-height line
renditions.
Line renditions were first implemented in conhost (with the GDI
renderer) in PR #8664. Supporting them in the DX renderer now is a
small step towards #11595.
The DX implementation is very similar to the GDI one. When a particular
line rendition is requested, we create a transform that is applied to
the render target. And in the case of double-height renditions, we also
initialize some clipping offsets to allow for the fact that we only
render half of a line at a time.
One additional complication exists when drawing the cursor, which
requires a two part process where it first renders to a command list,
and then draw the command list in a second step. We need to temporarily
reset the transform in that first stage otherwise it ends up being
applied twice.
I've manually tested the renderer in conhost by setting the `UseDx`
registry entry and confirmed that it passes the _Vttest_ double-size
tests as well as several of my own tests. I've also checked that the
renderer can now handle horizontal scrolling, which is a feature we get
for free with the transforms.
## Summary of the Pull Request
When the conpty passthrough mode is enabled, it often needs to send `DSR-CPR` queries (cursor position reports) to the client terminal to obtain the current cursor position. However, the code that originally handled the responses to these queries got broken by the refactoring of the `ConGetSet` API. This PR is an attempt to correct that regression.
## References
The conpty passthrough mode was introduced in PR #11264.
The refactoring that broke the cursor position handling was in PR #12703.
## PR Checklist
* [x] Closes#13106
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] 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
Prior to the `ConGetSet` refactoring, the code that handled `DSR-CPR` responses (`InteractDispatch::MoveCursor`) would pass the cursor position to `ConGetSet::SetCursorPosition`, which in turn would forward it to the `SetConsoleCursorPositionImpl` API, and from there to the `VtIo` class.
After the refactor, all of those intermediate steps were removed - the cursor was simply updated directly in `InteractDispatch::MoveCursor`, and the `VtIo` call was moved from `SetConsoleCursorPositionImpl` to `InteractDispatch` (since that was the only place it was actually required).
However, when the conpty passthrough mode was introduced - which happened in parallel - it relied on the `SetConsoleCursorPositionImpl` API being called from `InteractDispatch` in order to handle its own `DSR-CPR` responses, and that's why things stopped working when the two PRs merged.
So what I've done now is made `InteractDispatch::MoveCursor` method call `SetConsoleCursorPositionImpl` again (although without the intermediate `ConGetSet` overhead), and moved the `VtIo::SetCursorPosition` call back into `SetConsoleCursorPositionImpl`.
This is not ideal, and there are still a bunch of problems with the `DSR-CPR` handling in passthrough mode, but it's at least as good as it was before.
## Validation Steps Performed
I've just manually tested various shells with passthrough mode enabled, and confirmed that they're working better now. There are still issues, but nothing that wasn't already a problem in the initial implementation, at least as far as I can tell.
This regressed in ad2358d.
We're interested in the size of the viewport only, but it can shift up/down
during scrolling. In these situations we shouldn't resize our buffers of course.
## Validation Steps Performed
* Scroll
* Not setting `ApiInvalidations::Size` ✅
## Summary of the Pull Request
Add `Find` to tab context menu as describe in issue #5633.
## PR Checklist
* [x] Closes#5633
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
## Detailed Description of the Pull Request / Additional comments
Just wanted to solve `Easy Starter` issue, so any corrections/suggestions welcome. There's a couple of points I'm not sure of
* Placement of item within menu, currently it's at the end before close tab block.
* Should it be named longer, something like `Find in Tab` of just `Find` is fine?
* The workaround for focus similar to tab rename is a bit annoying, especially because it required adding a method to `TermControl` but without it find window obviously opens without focus which is bad.
## Validation Steps Performed
Open menu, press menu item try to find things via opened find dialog.
This commit includes various minor improvements to til::hash/point/size/rect
which accumulated while working on #4015.
* Allow xvalue containers and non-`size_t` indices in `til::at`.
* `til::as_unsigned` can be used to reinterpret a potentially signed integer
as a unsigned one. This can potentially enable some optimizations as no sign
extension is needed anymore. `til::hash` can make use of this to drop about
20% of the hashing of signed integers <= 32 bit. On x86 this translates to
a `mov` (virtually no latency) or no instructions at all, instead of
requiring a `movsx` (some latency) for sign extension.
* `til::point` operators that prefer mutability.
This is a opinionated change, but it follows the STL style beter and
generates less assembly.
* Simpler `rect` scale_up/down and `size` divide_ceil.
`scale_up` will not depend on the operator header anymore.
`scale_down` / `divide_ceil` can be implemented without checked numerics,
so I did. It also follows the related GdiEngine code better now, which
makes me confident that we can replace GdiEngine's code with this.
* Removal of rect-size-shift operators.
They were only used in DxEngine and confusing as they weren't commutative.
Adding and then subtracting a size from a rect (and vice versa) didn't do
what you'd intuitively think it'd do. The code was replaced with addition
and clamps in DxEngine.
* Various unsafe `as_` casts for point/size/rect.
This will aid the migration in #4015.
## Validation Steps Performed
* Vertical scrolling works in `DxEngine` ✅
This commit is one of the more difficult rewrites that were necessary as part
of #4015, but still simple enough that it can be done as a separate commit.
The search for the `lastNonSpace` was replaced with a simpler
`std::string_view::find_last_not_of`.
## Validation Steps Performed
ConPTY appears to work ✅
This reverts commit 14098d71f2.
## Summary of the Pull Request
@zadjii-msft found that this is causing persisted windows on a secondary monitor to shrink a little each time. We're choosing to revert this commit until that gets resolved.
## References
#12979
This commit fixes various bugs in our unit/feature test suite:
* 2 tests failed at 150% scale.
* The "null key" (@ on a US keyboard) isn't necessarily Shift+2.
The proper way to get it is with `LOBYTE(VkKeyScanW(0))`
* `InputEngineTest::C0Test` never worked as it overwrote
the loop variable, exiting the loop early
Fixes the following issues:
* `desktopWallpaper` not working
* switching tabs/panes causes the background to flicker
* settings preview having a transparent background
## PR Checklist
* [x] Closes#13002
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
Tested the 3 cases above. ✅
## Summary of the Pull Request
For some reason, the PGO tests (specifically the `RunMakeKillTabs` test) started to fail after #12979 merged. After closer inspection, the test was actually improperly written. We should be using <kbd>ctrl+shift+t</kbd> to open new tabs, not <kbd>alt+shift+t</kbd>. Presumably, the <kbd>alt</kbd> was copied over from the previous test, because they look _very_ similar.
So I went ahead and fixed the test, and it now (1) tests what it's intended to test and (2) doesn't fail. Why did #12979 cause the tests to fail? idk, but it works now.
## References
#10071 - Introduce PGO Tests
## Validation Steps Performed
Ran PGO tests locally and confirmed that it works.
Ran PGO pipeline and confirmed that it works.
When the buffer is resized with a reflow, we were previously calculating
the new virtual bottom based on the position of last non-space
character. If the viewport was largely blank when resized, this could
result in the new virtual bottom being higher than it should be.
This PR attempts to address that problem by restoring the virtual bottom
to a position that is the same distance from the cursor row as it was
prior to the resize.
This was a regression introduced in PR #12972.
We still take the last non-space row into account when determining the
virtual bottom, because if the content of the screen is forced to wrap,
the virtual bottom will need to be lower (relative to the cursor) than
it was before.
We also need to check that we don't overflow the bottom of the buffer,
which can occur when the viewport is at the bottom of the buffer, and
the cursor position is pushed down as a result of content wrapping above
it.
I've manually confirmed that this fixes the problem reported in issue
#13078, and I've also extended the existing `RefreshWithReflow` unit
test to cover that particular scenario.
Closes#13078
The `DECAC` (Assign Colors) escape sequence controls which color table
entries are associated with the default foreground and background
colors. This is how you would change the default colors on the the
original DEC VT525 terminals.
But `DECAC` also allows you to assign the color table entries for the
"window frame", which in our case is mapped to the tab color (just the
background for now). So this now gives us a way to control the tab color
via an escape sequence as well.
DETAILS
-------
The way this works is there are now two new entries in the color table
for the frame colors, and two new aliases in the color alias table that
are mapped to those color table entries. As previously mentioned, only
the background is used for now.
By default, the colors are set to `INVALID_COLOR`, which indicates that
the system colors should be used. But if the user has set a `tabColor`
property in their profile, the frame background will be initialized with
that value instead.
And note that some of the existing color table entries are now
renumbered for compatibility with XTerm, which uses entries 256 to 260
for special colors which we don't yet support. Our default colors are
now at 261 and 262, the frame colors are 263 and 264, and the cursor
color is 265.
So at runtime, you can change the tab color programmatically by setting
the color table entry at index 262 using `OSC 4` (assuming you need a
specific RGB value). Otherwise if you just want to set the tab color to
an existing color index, you can use `DECAC 2`.
You can even make the tab color automatically match background color by
mapping the frame background alias to the color table entry for the
default background, using `DECAC 2;261;262` (technically this is mapping
both the the foreground and background).
This PR doesn't include support for querying the color mapping with
`DECRQSS`, and doesn't support resetting the colors with `RIS`, but
hopefully those can be figured out in a future PR - there are some
complications that'll need to be resolved first.
## Validation Steps Performed
I've added a basic unit test that confirms the `DECAC` escape sequence
updates the color aliases in the render settings as expected. I've also
manually confirmed that the tab color in Windows Terminal is updated by
`DECAC 2`, and the default colors are updated in both conhost and WT
using `DECAC 1`.
Closes#6574
Fixes the SUI background being red in high contrast mode. The issue was
that `SolidBackgroundFillColorTertiary` purposefully has a bad High
Contrast color[^1].
The fix was to be explicit in the theme resources so that
`SolidBackgroundFillColorTertiary` is used in light and dark mode, but
the standard high contrast one is used in high contrast mode. Since the
page is the top-level XAML element in the Editor project, I had to
introduce this in the App.xaml resources so that the page can find the
theme resource.
Closes#13065Closes#13070
[^1]: 40df43a61c/dev/CommonStyles/Common_themeresources_any.xaml (L650-L651)
## Summary of the Pull Request
When calculating the position of the virtual bottom after a resize with
reflow, it was possible for it to end up less than the height of the
viewport. This meant that the top of the virtual viewport would be
negative, which resulted in other operations failing further down the
line. This PR updates the virtual bottom calculation to fix that
scenario.
## References
This was probably a regression introduced in PR #12972.
## PR Checklist
* [x] Closes#13034
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number where discussion took place: #13034
## Validation Steps Performed
I wasn't able to replicate the exact case described in issue #13034,
because I don't have Windows 11, so can't configure the default
terminal. However, I was able to reproduce a similar failure using a
`SetConsoleScreenBufferInfoEx` call, and I've confirmed that this PR
has fixed that.
I've also added another screen buffer test to make sure the
`ResizeWithReflow` method doesn't shrink the virtual bottom when
resizing at the top of the buffer.
When we start up, our window is initially just a frame with a transparent content area. We're gonna do all this startup init on the UI thread, so the UI won't actually paint till it's all done. This results in a few frames where the frame is visible, before the page paints for the first time, before any tabs appears, etc.
To mitigate this, we're gonna wait for the UI thread to finish everything it's gotta do for the initial init, and _then_ fire our Initialized event. By waiting for everything else to finish (`CoreDispatcherPriority::Low`), we let all the tabs and panes actually get created. In the window layer, we're gonna ~cloak~ just not show the window till this event is fired, so we don't actually see this frame until we're actually all ready to go. **This will result in the window seemingly not loading as fast**, but it will actually take exactly the same amount of time before it's usable.
I also experimented with drawing a solid BG color before the initialization is finished. However, there are still a few frames after the frame is displayed before the XAML content first draws, so that didn't actually resolve any issues.
* [x] Closes#11561
* [x] Tested manually
* [x] I work here.
* [x] Accidentally also closes#9053. By switching the initial call from `ShowWindow(SW_SHOW)` to `ShowWindow(SW_SHOWDEFAULT)`, we actually obey the startup info now.
Adds the `selectAll` action which can be used to select all text in the buffer (regardless of whether a selection is present).
## References
#3663 - Mark Mode
#4993 - [Scenario] Keyboard selection
## PR Checklist
* [x] Closes#1469
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
I've made it such that selecting the "entire buffer" really just selects up to the mutable viewport. This seems like a nice QOL improvement since there's generally nothing past that.
When the user selects all, the viewport does not move. This is consistent with CMD behavior and is intended to allow the user to not lose context when selecting everything.
A minor change had to be made to the DxRenderer because this uncovered an underflow issue. Basically, the selection rects were handed to the DxEngine relative to the viewport (which means that some had a negative y-value). At some point, those rects were stored into `size_t`s, resulting in an underflow issue. This caused the renderer to behave strangely when rendering the selection. Generally, these kinds of issues weren't really noticed because selection would always modify a portion of the viewport.
Funny enough, in a way, this satisfies the "mark mode" scenario because the user now has a way to initiate a selection using only the keyboard. Though this isn't ideal, just a fun thing to point out (that's why I'm not closing the mark mode issue).
## Validation Steps Performed
- Verified using DxEngine and AtlasEngine
- select all --> keyboard selection --> start moving the top-left endpoint (and scroll to there)
- select all --> do not scroll automatically
Turns out if you add that Delete handler there, then every time you navigate to the profile, we'll add another Delete handler to the list of handlers. That's bad - that'll cause us to try and delete the profile multiple times.
The repro I had before was 100%, now it's fixed.
* [x] Closes#13017
`TextAttribute` and `TextColor` are commonly used structures in hot paths.
This commit replaces more complex comparisons where each field is compared
independently with a single call to `memcmp`. This compiles down to just
a few instructions. This reduces code and binary size and improves
performance for paths were `TextAttribute`s need to be compared.
## PR Checklist
* [x] Supports #10563
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] 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
## Validation Steps Performed
* termbench still works ✔️
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
## Summary of the Pull Request
As discussed in team sync. Ignore `newTab` actions with a profile index greater than the number of profiles.
## PR Checklist
* [x] Closes#11114
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated - maybe❓
Mike Griese
