## Summary of the Pull Request
This PR causes the Terminal to combine taskbar states at the tab and window level, according to the [MSDN docs for `SetProgressState`](https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate#how-the-taskbar-button-chooses-the-progress-indicator-for-a-group).
This allows the Terminal's taskbar icon to continue showing progress information, even if you're in a pane/tab that _doesn't_ have progress state. This is helpful for cases where the user may be running a build in one tab, and working on something else in another.
## References
* [`SetProgressState`](https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-itaskbarlist3-setprogressstate#how-the-taskbar-button-chooses-the-progress-indicator-for-a-group)
* Progress mega: #6700
## PR Checklist
* [x] Closes#10090
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
This also fixes a related bug where transitioning from the "error" or "warning" state directly to the "indeterminate" state would cause the taskbar icon to get stuck in a bad state.
## Validation Steps Performed
<details>
<summary><code>progress.cmd</code></summary>
```cmd
@echo off
setlocal enabledelayedexpansion
set _type=3
if (%1) == () (
set _type=3
) else (
set _type=%1
)
if (%_type%) == (0) (
<NUL set /p =]9;4
echo Cleared progress
)
if (%_type%) == (1) (
<NUL set /p =]9;4;1;25
echo Started progress (normal, 25^)
)
if (%_type%) == (2) (
<NUL set /p =]9;4;2;50
echo Started progress (error, 50^)
)
if (%_type%) == (3) (
@rem start indeterminate progress in the taskbar
@rem this `<NUL set /p =` magic will output the text _without a newline_
<NUL set /p =]9;4;3
echo Started progress (indeterminate, {omitted})
)
if (%_type%) == (4) (
<NUL set /p =]9;4;4;75
echo Started progress (warning, 75^)
)
```
</details>
## Summary of the Pull Request
Apparently the exception handler in TerminalApi is far too talkative. We're apparently throwing in `TerminalApi::CursorLineFeed` way too often, and that's caused an internal bug to be filed on us.
This represents making the event less talkative, but doesn't actually fix the bug. It's just easier to get the OS bug cleared out quick this way.
## References
* MSFT:33310649
## PR Checklist
* [x] Fixes the **A** portion of #10882, which closes MSFT:33310649
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated
## Summary of the Pull Request
Turns out, we'd only ever use the non-client size to calculate the size of the window, but not the actual position. As we learned in #10676, the nonclient area extends a few pixels past the visible borders of the window.
## PR Checklist
* [x] Closes#10583
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* [x] Works with the `IslandWindow`
* [x] Works with the `NonClientIslandWindow`
## Summary of the Pull Request
Do not invoke terminal resize logic if view port dimensions didn't change
## PR Checklist
* [x] Closes#10857
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
Short-circuit `ControlCore::_doResizeUnderLock` if the dimensions of the
required view port are equal to the dimensions of the current view port
#### ⚠️ targets #10051
## Summary of the Pull Request
This updates our `ThrottledFunc`s to take a dispatcher parameter. This means that we can use the `Windows::UI::Core::CoreDispatcher` in the `TermControl`, where there's always a `CoreDispatcher`, and use a `Windows::System::DispatcherQueue` in `ControlCore`/`ControlInteractivity`. When running in-proc, these are always the _same thing_. However, out-of-proc, the core needs a dispatcher queue that's not tied to a UI thread (because the content proces _doesn't have a UI thread!_).
This lets us get rid of the output event, because we don't need to bubble that event out to the `TermControl` to let it throttle that update anymore.
## References
* Tear-out: #1256
* Megathread: #5000
* Project: https://github.com/microsoft/terminal/projects/5
## PR Checklist
* [x] This is a part of #1256
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Fortunately, `winrt::resume_foreground` works the same on both a `CoreDispatcher` and a `DispatcherQueue`, so this wasn't too hard!
## Validation Steps Performed
This was validated in `dev/migrie/oop/the-whole-thing` (or `dev/migrie/oop/connection-factory`, I forget which), and I made sure that it worked both in-proc and x-proc. Not only that, _it wasn't any slower_!This reverts commit 04b751faa7.
This PR adds conhost support for downloadable soft fonts - also known as
dynamically redefinable character sets (DRCS) - using the `DECDLD`
escape sequence.
These fonts are typically designed to work on a specific terminal model,
and each model tends to have a different character cell size. So in
order to support as many models as possible, the code attempts to detect
the original target size of the font, and then scale the glyphs to fit
our current cell size.
Once a font has been downloaded to the terminal, it can be designated in
the same way you would a standard character set, using an `SCS` escape
sequence. The identification string for the set is defined by the
`DECDLD` sequence. Internally we map the characters in this set to code
points `U+EF20` to `U+EF7F` in the Unicode private use are (PUA).
Then in the renderer, any characters in that range are split off into
separate runs, which get painted with a special font. The font itself is
dynamically generated as an in-memory resource, constructed from the
downloaded character bitmaps which have been scaled to the appropriate
size.
If no soft fonts are in use, then no mapping of the PUA code points will
take place, so this shouldn't interfere with anyone using those code
points for something else, as along as they aren't also trying to use
soft fonts. I also tried to pick a PUA range that hadn't already been
snatched up by Nerd Fonts, but if we do receive reports of a conflict,
it's easy enough to change.
## Validation Steps Performed
I added an adapter test that runs through a bunch of parameter
variations for the `DECDLD` sequence, to make sure we're correctly
detecting the font sizes for most of the known DEC terminal models.
I've also tested manually on a wide range of existing fonts, of varying
dimensions, and from multiple sources, and made sure they all worked
reasonably well.
Closes#9164
`VkKeyScanW` as well as `MapVirtualKeyW` are used throughout
the project, but are input method sensitive functions.
Since #10666 `win+sc(41)` is used as the quake mode keybinding,
which is then mapped to a virtual key in order to call `RegisterHotKey`.
This mapping is highly dependent on the input method and the quake mode
key binding will fail to work once the input method was changed.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#10729
* [x] I work here
* [ ] Tests added/passed
## Validation Steps Performed
* win+` opens quake window before & after changing keyboard layout ✔️
* keyboard layout changes while WT is minimized trigger reloaded ✔️
- Monarch no longer sets itself up as a `CTerminalHandoff` multi instance server by default
- In fact, `CTerminalHandoff` will only ever be a single instance server
- When COM needs a `CTerminalHandoff`, it launches `wt.exe -embedding`, which gets picked up by the Monarch and then gets handed off to itself/peasant depending on user settings.
- Peasant now recognizes the `-embedding` commandline and will start a `CTerminalHandoff` single instance listener, and receives the connection into a new tab.
Closes#10358
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Adds the Split Tab option to the tab context menu.
Clicking this option will `auto` split the active pane of the tab into a duplicate pane.
Clicking on an unfocused tab and splitting it will bring that tab into focus and split its active pane.
We could make this a flyout from the context menu to let people choose horizontal/vertical split in the future if it's requested.
I'm also wondering if this should be called Split Pane instead of Split Tab?
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#1912
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#5025
* [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.
* [ ] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
https://user-images.githubusercontent.com/48369326/127691919-aae4683a-212a-4525-a0eb-a61c877461ed.mp4
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
## Summary of the Pull Request
When switching from the alt buffer back to the main buffer, we need to copy certain cursor attributes from the one to the other. However, this copying was taking place after the alt buffer had been freed, and thus could result in the app crashing. This PR simply moves that code up a bit so it's prior to the buffer being freed.
## References
PR #10843 added the code that introduced this problem.
## PR Checklist
* [ ] Closes #xxx
* [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
## Validation Steps Performed
I was able to reproduce the crash when using a debug build, and confirmed that the crash no longer occurred after this PR was applied. I also checked that the cursor attributes were still being correctly copied back when returning from the alt buffer.
Shortly before adding the SSE2 variant I "improved" it by using
`_mm_packs_epi32`, but failed to test it again afterwards.
## PR Checklist
* [x] Closes#10866
* [x] I work here
## Validation Steps Performed
* `printf "\e[mNORMAL \e[1mBOLD\n"` results in correct bold white glyphs ✔️
## Validation Steps Performed
Clicked around, validated that settings still behave the same (as far as
I can tell with my limited terminal configuration expertise)
Closes#10387
Fixes dragging and dropping drive letters onto the '+' button.
Manually tested - dragging and dropping the `C:\` drive onto the '+' button works when creating a new tab, splitting or creating a new window. Dragging and dropping a regular directory still works.
Closes#10723
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Add the ability to toggle a pane's split direction
- Switch from horizontal to vertical split (and vice versa)
- Propogate new borders through to children.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#10665
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#10665
* [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
* [x] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Ran terminal, created multiple panes in different orientations, ran command through command palate and verified that they displayed properly in the new orientation.
When the `SetContoleTitle` API is called with a title containing control
characters, we need to filter out those characters before we can forward
the title change over conpty as an escape sequence. If we don't do that,
the receiving terminal will end up executing the control characters
instead of updating the title. We were already filtering out the C0
control characters, but with this PR we're now filtering out C1 controls
characters as well.
I've simply updated the sanitizing routine in `DoSrvSetConsoleTitleW` to
filter our characters in the range `0x80` to `0x9F`. This is in addition
to the C0 range (`0x00` to `0x1F`) that was already excluded.
## Validation Steps Performed
I've added a conpty unit test that calls `DoSrvSetConsoleTitleW` with
titles containing a variety of C0 and C1 controls characters, and which
verifies that those characters are stripped from the title forwarded to
conpty.
I've also confirmed that the test case in issue #10312 is now working
correctly in Windows Terminal.
Closes#10312
The `_invalidMap` size is dependent on both `clientSize` as well
as `glyphCellSize` and must be resized when either changes.
## PR Checklist
* [x] Closes#10855
* [x] I work here
## Validation Steps Performed
* Changing font size with Ctrl+Mousewheel in fullscreen works ✔️
This commit introduces a hack to ConptyConnection for launching WSL.
When we detect that WSL is being launched (either "wsl" or "wsl.exe",
unqialified or _specifically_ from the current OS's System32 directory),
we will promote the startingDirectory specified at launch time into a
commandline argument.
Why do we want to switch to `--cd`?
With the current design of ConptyConnection and WSL, there are some
significant limitations:
* `startingDirectory` cannot be a WSL path, which forces users to
use weird tricks such as setting the starting directory to
`\\wsl$\Distro\home\user`.
* WSL occasionally fails to launch in time to handle a `\\wsl$` path,
which makes us spawn in a strange location (or no location at all).
(This fix will only address the second one until a WSL update is
released that adds support for `--cd $LINUX_PATH`.)
We will not do the promotion if any of the following are true:
* the commandline contains `--cd` already
* the commandline contains a bare `~`
* This was a commonly-used workaround that forced wsl to start in the
user's home directory. It conflicts with --cd.
* wsl is not spelled properly (`WSL` and `WSL.EXE` are unacceptable)
* an absolute path to wsl outside the system32 directory is provided
We chose the do this trick in the connection layer, the latest possible
point, because it captures the most use cases.
We could have done it earlier, but the options were quite limiting.
They are:
* Generate WSL profiles with startingDirectory set to the home folder
* We can't do this because we do not know the user's home folder
path.
* Generate WSL profiles with `--cd` in them.
* This only works for unmodified profiles.
* This only works for generated profiles.
* Users cannot override the commandline without breaking it.
* Users cannot specify a startingDirectory (!) since the one on the
commandline wins.
* Set a flag on generated WSL profiles to request this trick
* This only works for generated profiles. Users who create their own
WSL profiles couldn't set startingDirectory and have it work the
same.
Patching the commandline, hacky though it may be, seemed to be the most
compatible option. Eventually, we can even support `wt -d ~ wsl`!
## Validation Steps Performed
Manual validation for the following cases:
```c++
// MUST MANGLE
auto a01 = _tryMangleStartingDirectoryForWSL(LR"(wsl)", L"SENTINEL");
auto a02 = _tryMangleStartingDirectoryForWSL(LR"(wsl -d X)", L"SENTINEL");
auto a03 = _tryMangleStartingDirectoryForWSL(LR"(wsl -d X ~/bin/sh)", L"SENTINEL");
auto a04 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe)", L"SENTINEL");
auto a05 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe -d X)", L"SENTINEL");
auto a06 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe -d X ~/bin/sh)", L"SENTINEL");
auto a07 = _tryMangleStartingDirectoryForWSL(LR"("wsl")", L"SENTINEL");
auto a08 = _tryMangleStartingDirectoryForWSL(LR"("wsl.exe")", L"SENTINEL");
auto a09 = _tryMangleStartingDirectoryForWSL(LR"("wsl" -d X)", L"SENTINEL");
auto a10 = _tryMangleStartingDirectoryForWSL(LR"("wsl.exe" -d X)", L"SENTINEL");
auto a11 = _tryMangleStartingDirectoryForWSL(LR"("C:\Windows\system32\wsl.exe" -d X)", L"SENTINEL");
auto a12 = _tryMangleStartingDirectoryForWSL(LR"("C:\windows\system32\wsl" -d X)", L"SENTINEL");
auto a13 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~/bin)", L"SENTINEL");
// MUST NOT MANGLE
auto a14 = _tryMangleStartingDirectoryForWSL(LR"("C:\wsl.exe" -d X)", L"SENTINEL");
auto a15 = _tryMangleStartingDirectoryForWSL(LR"(C:\wsl.exe)", L"SENTINEL");
auto a16 = _tryMangleStartingDirectoryForWSL(LR"(wsl --cd C:\)", L"SENTINEL");
auto a17 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~)", L"SENTINEL");
auto a18 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~ -d Ubuntu)", L"SENTINEL");
```
We don't have anywhere to put TerminalConnection unit tests :|
Closes#592.
When switching to the alt buffer, the starting cursor position, style,
and visibility is meant to be inherited from the main buffer. Similarly,
when returning to the main buffer, any changes made to those attributes
should be copied back (with the exception of the cursor position, which
is restored to its original state). This PR makes sure we handle that
cursor state correctly.
At some point I'd like to move the cursor state out of the
`SCREEN_INFORMATION` class, which would make this inheritance problem a
non-issue. For now, though, I've just made it copy the state from the
main buffer when creating the alt buffer, and copy it back when
returning to the main buffer.
## Validation Steps Performed
I've added some unit tests to verify the cursor state is inherited
correctly when switching to the alt buffer and back again. I also had to
make a small change to one of the existing alt buffer test that relied
on the initial cursor position being at 0;0, which is no longer the
case.
I've verified that the test case in issue #3545 is now working
correctly. I've also confirmed that this fixes a problem in the
_notcurses_ demo, where the cursor was showing when it should have been
hidden.
Closes#3545
## Summary of the Pull Request
<kbd>win+shift+arrows</kbd> can be used to move windows to adjacent monitors. When that happens, we'll new re-calculate the size of the window for the new monitor.
## References
* megathread: #8888
## PR Checklist
* [x] Closes#10274
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
In `WM_WINDOWPOSCHANGING`, the OS says "hey, I'm about to do {something} to your window. You cool with that?". We handle that message by:
1. checking if the window was _moved_ as a part of this message
2. getting the monitor that the window will be moved onto
3. If that monitor is different than the monitor the window is currently on, then
* calculate how big the quake window should be on that monitor
* tell the OS that's where we'd like to be.
## Validation Steps Performed
* <kbd>win+shift+arrows</kbd> works right now
* normal quake summoning still works right
I was watching a video about vectorized instructions and I wanted to
try out some new things, as I had never written AVX code before.
This commit is the result of this tiny Thursday morning detour into
AVX land. It improves performance of `TextColor::GetColor` by about 3x.
## Validation Steps Performed
* Default colors are still properly shifted +8 ✔️
Sets the tooltip text on the '+' button based on the keyboard modifiers
when dragging and dropping.
## Validation Steps Performed
Manually tested - dragged a directory onto the '+ button and saw that
* The text changed when `shift` was pressed
* The text changed when `alt` was pressed
* The text changed back when `shift` or `alt` were released
Closes#10722
## Summary of the Pull Request
This fixes two bugs related to dragging into the bounds of the `TermControl`. Although the fixes are fairly small, I'm batching them up, because I don't want to stack 2 more PRs on top of #10051.
* #9109
- This is fixed by only starting an autoscroll if the click&drag actually started within the bounds of the control.
* #4603
- Building on the above change, only modify the selection when the drag started in the control.
## References
* srsly go read #10051.
## PR Checklist
* [x] Closes#9109
* [x] Closes#4603
* [x] I work here
* [x] Test added
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
This is kind of annoying that the auto-scrolling is handled by the TermControl, but it uses a timer that's still a WinUI construct.
We only want to start the auto-scrolling behavior when the drag started _inside_ the control. Otherwise, in the tab drag scenario, dragging into the bounds of the TermControl will trick it into thinking it should start a scroll.
## Summary of the Pull Request
When we're restoring from fullscreen, we do a little adjustment to make sure to clamp the window bounds within the bounds of the active monitor. We unfortunately didn't account for the size of the non-client area (the invisible borders around our 1px border). This didn't matter most of the time, but if the window was within ~8px of the side of the monitor (any side), then restoring from fullscreen would actually move it to the wrong place.
As it turns out, the `_quake` window is within ~8px of the edges of the monitor _very often_.
## References
* regressed in #9737
## PR Checklist
* [x] Closes#10199
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
The repro in the bug was fairly straightforward. It doesn't happen anymore.
For inexplicable reasons, the top row of pixels on our tabs, new tab
button, and caption buttons is totally unclickable. The mouse simply
refuses to interact with them. So when we're maximized, on certain
monitor configurations, this results in the top row of pixels not
reacting to clicks at all.
To obey Fitt's Law, we're gonna hackily shift the entire island up one
pixel. That will result in the top row of pixels in the window actually
being the _second_ row of pixels for those buttons, which will make them
clickable. It's perhaps not the right fix, but it works.
After discussion, we think this is a fine fix for this. We don't think
anyone's going to miss the top row of pixels on the TabView. The original
bug is painful enough for the subset of users it impacts that this is an
acceptable trade. Should a better fix be found, we can absolutely do that
instead.
Closes#7422
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Implementation of #6219 with a small tweak, not just passing the keys when no panes are present, but passing on the keys when there is no other pane to move to. This enables another usecase: 2 panes in terminal split vertically; in one of these panes running tmux with two panes that are split horizontally. This allows the user to still navigate between tmux panes even though they have terminal panes open.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
Not that I know of
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#6219
* [x] CLA signed.
* [x] Tests added/passed
* [x] Documentation updated. I don't think that's necessary
* [x] Schema updated. N/A
* [ ] 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.
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
Implementation by propagating the boolean indicating success of moving focus all the way to the action handler, where this result will determine whether the action will be considered handled or not. When the action is not handled, the keychord will be propagated to the terminal.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Manual testing; all relevant unit tests still work
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Fixes/implements #10058 according to directions in that issue: added support for browser navigation keys to be used in actions.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#10058
* [x] CLA signed.
* [x] Tests added/passed
* [x] Documentation updated: . If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: https://github.com/MicrosoftDocs/terminal/pull/371
* [x] Schema updated.
* [x] I've discussed this with core contributors already. According to instructions in #10058
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
The mouse back/forward keys do not correspond to the keys added here. That would be a nice (but more complicated) addition, I'll add an issue for it.
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
The `+=` operator is an extremely hot path under heavily output load. This PR aims to optimize its speed.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Supports #10563
* [ ] 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.
* [ ] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
- For tabs started from the Terminal, the initial sizing information is
passed into the connection and used to establish the PTY. Those
parameters are given over to the `OpenConsole.exe` acting as PTY to
establish the initial buffer/window size.
- However, for tabs started from outside, the PTY is created with some
default buffer information FIRST as the Terminal hasn't even been
involved yet. As such, when the Terminal gets that connection, it must
tell the PTY to resize just as it connects to match the window size
it's about to use.
- Ongoing resize operations in the Terminal did and still work fine
because they transmitted the updated size with the
`ResizePseudoConsole` API.
## Validation Steps Performed
- [x] Confirmed existing tabs opening have correct initial size in PTY
(like with CMD `mode con` command)
- [x] Confirmed inbound cmd tabs have correct initial size in PTY via
`mode con` command per bug repro
Closes#9811
Turns out, DWrite will automatically turn some features on even if they weren't included in the feature vector passed into it. Remove these features from our default list for easier readability.
This commit fixes the UseDx mode for conhost.
In order to add support for UseDx without calling `SetWindowSize`,
responsibility for resizing `_invalidMap` has been moved to occur
only when the renderer itself recognizes a new size. Furthermore
`InvalidateAll` is now the central point to invalidate `_invalidMap`.
## Validation Steps Performed
* Enabling `UseDx` enables the DxEngine for conhost ✔️
* Resizing windows in conhost works ✔️
* Resizing windows in WT works ✔️Closes#5455
## Summary of the Pull Request
Uses the new logic to find visual neighbors of a pane to find which pane is the target when the move-focus commands are used.
## References
It sounds like this logic will be refined later to meet #4692
## PR Checklist
* [x] Closes#2398
* [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.
* [ ] 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
Created a grid of panes and confirmed that focus movement went to the right quadrant instead of just the first child of the sibling.
Adds support for users to be able to set font features and axes (see the spec for more details!)
## Detailed Description
**CustomTextLayout**
- Asks the `DxFontRenderData` for the font features when getting glyphs
- _If any features have been set/updated, we always skip the "isTextSimple" shortcut_
- Asks the `_formatInUse` for any font axes when mapping characters in `_AnalyzeFontFallback`
**DxFontRenderData**
- Stores a map of font features (initialized to the [standard feature list])
- Stores a map of font axes
- Has methods to add font features/axes to the map or update existing ones
- Has methods to retrieve the font features/axes
- Sets the font axes in the `IDWriteTextFormat` when creating it
## Validation Steps Performed
It works!
[standard feature list]: ac5aef67d1/DrawableObject.ixx (L802)
Specified in #10457
Related to #1790Closes#759Closes#5828
## Summary of the Pull Request
When we perform a `focusTab` action, we currently do nothing if the parameter was greater than the number of tabs. This PR changes that behavior. Now, `focus-tab -t 999999` will always focus the last tab, instead of silently doing nothing.
## PR Checklist
* [x] Closes#9369
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* [x] ran tests
* [x] validated commandline manually
Related work items: MSFT-32957145
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev bdb25dc99dcb2f1ee483dffe883d0178ea9d18dc
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Add functionality to swap a pane with an adjacent (Up/Down/Left/Right) neighbor.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
This work potentially touches on: #1000#2398 and #4922
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes a component of #1000 (partially, comment), #4922 (partially, `SwapPanes` function is added but not hooked up, no detach functionality)
* [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
* [x] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
Its been a while since I've written C++ code, and it is my first time working on a Windows application. I hope that I have not made too many mistakes.
Work currently done:
- Add boilerplate/infrastructure for argument parsing, hotkeys, event handling
- Adds the `MovePane` function that finds the focused pane, and then tries to find
a pane that is visually adjacent to according to direction.
- First pass at the `SwapPanes` function that swaps the tree location of two panes
- First working version of helpers `_FindFocusAndNeighbor` and `_FindNeighborFromFocus`
that search the tree for the currently focused pane, and then climbs back up the tree
to try to find a sibling pane that is adjacent to it.
- An `_IsAdjacent' function that tests whether two panes, given their relative offsets, are adjacent to each other according to the direction.
Next steps:
- Once working these functions (`_FindFocusAndNeighbor`, etc) could be utilized to also solve #2398 by updating the `NavigateFocus` function.
- Do we want default hotkeys for the new actions?
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
At this point, compilation and manual testing of functionality (with hotkeys) by creating panes, adding distinguishers to each pane, and then swapping them around to confirm they went to the right location.
Pass inbound handoff message via heap so it cannot race out of scope by the time it reaches the ConsoleIoThread
## PR Checklist
* [x] Closes#10251
* [x] I work here.
* [x] Manually verified somewhat
## Detailed Description of the Pull Request / Additional comments
- `OpenConsole.exe` is started in response to the OS `conhost.exe` request for a handoff and prepares an Out Of Proc Multithreaded COM server.
- A COM thread from the pool inside `OpenConsole.exe` picks up the inbound message and allocates some stack space for the `CONSOLE_API_MSG` coming in
- That COM thread calls down to set up the I/O thread that will pump the console driver handle and passes a pointer to the stack-allocated `CONSOLE_API_MSG` as the `LPVOID` parameter for starting the thread.
Now one of two things happen:
1. The I/O thread is scheduled pretty much immediately (or soon enough that the COM thread hasn't messed with the stack space), picks up the pointer to the COM thread's stack with `CONSOLE_API_MSG`, and processes the initial message correctly.
2. The COM thread continues and finalizes the handoff message to `conhost.exe` declaring success. It then pops stack and "frees" the memory space. If it doesn't manage to overwrite it, we're still good. If it does, then things go crazy.
This fix changes it so that the `CONSOLE_API_MSG` is sent into the heap before being passed to the other thread so it's in a known location that won't be freed or overwritten unexpectedly.
## Validation Steps Performed
- [x] - Confirmed that many handoffs from the run box seem to work alright on my system after this change.
- [x] - Confirmed that many tab creations/splits seem to work alright on my system after this change.
- [x] - Would prefer if @ianjoneill could try to F5 this branch to build/deploy it, set it as default, and see if it makes it go away completely... but I'm pretty confident it is this based on the dumps provided either way.
## Summary of the Pull Request
We no longer automatically write the 'hidden' field for profile stubs we create
**Note**: This does not retroactively remove the automatically generated hidden fields in current settings files
## PR Checklist
* [x] Closes#10539
* [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
Deleted the ubuntu stub in my settings file, booted up terminal, new created stub did not have the hidden field. Created a fragment that overrides the hidden field and it worked.
As identified by Michael Niksa, our MDE heuristics for understanding relationship between conhost and related processes was incorrect. Exposing trace here to assist in correlation.
Related work items: MSFT-32957145
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 3c886da66d77d1aa36b52794929e388af292539c
The `_CONSOLE_API_MSG` buffer is resized to cover an entire message.
Later on any UTF-8 data is cached in a separate temporary
buffer inside `til::u8state` to prevent lone surrogate pairs.
Both cases are problematic as neither buffer is freed after the read
has finished. Passing a 100MB buffer to conhost once will thus cause it
to continue using ~220MB of physical memory until the conhost process exits.
This change releases unneeded memory as soon as the requested buffer
size has halved. In practice this means that once a command has returned
all buffers will shrink, as the shell commonly sends very small messages.
## PR Checklist
* [x] Closes#10731
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Buffers aren't reallocated during printing ✔️
* Buffers shrink after printing finished ✔️
This commit introduces an alternative to specifying key bindings as a combination of key modifiers and a character. It allows you to specify an explicit virtual key as `vk(nnn)`.
Additionally this commit makes it possible to bind actions to scan codes. As scan code 41 appears to be the button below the Escape key on virtually all keyboards, we'll be able to bind the quake mode hotkey to `win+sc(41)` and have it work consistently across most if not all keyboard layouts.
## PR Checklist
* [x] Closes#7539, Closes#10203
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
The following was tested both on US and DE keyboard layouts:
* Ctrl+, opens settings ✔️
* Win+` opens quake mode window ✔️
* Ctrl+plus/minus increase/decrease font size ✔️
## Summary of the Pull Request
Adjust the y-coordinate of the mouse coordinates we send based on how much the viewport has been scrolled
## Validation Steps Performed
Validated: cannot repro the issue in #10190Closes#10190
## Summary of the Pull Request
We were making the quake window exactly the width of the monitor it was on, but that didn't account for the 1px of border on either side.
## References
* megathread: #8888
## PR Checklist
* [x] Closes#10201
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
It happened before, it doesn't anymore.
## Summary of the Pull Request
Add an explicit background color to part of the settings UI to prevent animation overflow. The previous solution (adding a ScrollViewer) caused problems.
## References
#10619 adds a ScrollViewer for one of the issues in #10609
## PR Checklist
* [x] Closes#10664
* [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.
* [ ] 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
## Validation Steps Performed
Visually confirmed the animation doesn't overflow, changed the theme and confirmed the colors are responsive. Confirmed the extra scrollbar is gone.
Visual Studio 2022 Preview recently released the v143 toolchain.
C4189 is now flagging several unused variables, which breaks our build.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* CascadiaPackage builds ✔️
* All tests build ✔️
## Summary of the Pull Request
When the quake window is moved to another monitor, re-evaluate it's size for that monitor.
## References
* megathread: #8888
* Similar, but not the same: #10274
## PR Checklist
* [x] Closes#10182
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
We'll probably need to do this in a few more places, but I'm breaking PRs into small chunks for easier reviews.
## Validation Steps Performed
Summoned the window to a bunch of different resolutions. Where it would use the wrong size before, it no longer does.
As discussed in team sync. Is this a mysterious dark pattern we didn't know about?
* [x] closes#10721
* [x] I work here
* [x] doesn't need tests
* [x] doesn't need docs
* see also #10160
#### ⚠️ targets #10051
## Summary of the Pull Request
This PR does one big, primary thing. It removes all the constructors from any TerminalConnections, and changes them to use an `Initialize` method that accepts a `ValueSet` of properties.
Why?
For the upcoming window/content process work, we'll need the content process to be able to initialize the connection _in the content process_. However, the window process will be the one that knows what type of connection to make. Enter `ConnectionInformation`. This class will let us specify the class name of the type we want to create, and a set of settings to use when initializing that connection.
**IMPORTANT**: As a part of this, the constructor for a connection must have 0 arguments. `RoActivateInstance` lets you just conjure a WinRT type just by class name, but that class must have a 0 arg ctor. Hence the need for `Initialize`, to actually pass the settings.
We're using a `ValueSet` here because it's basically a json blob, with more steps. In the future, when extension authors want to have custom connections, we can always deserialize the json into a `ValueSet`, pass it to their connection's `Initialize`, and let then get what they need out of it.
## References
* Tear-out: #1256
* Megathread: #5000
* Project: https://github.com/microsoft/terminal/projects/5
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50760298
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
`ConnectionInformation` was included as a part of this PR, to demonstrate how this will eventually be used. `ConnectionInformation` is not _currently_ used.
## Validation Steps Performed
It still builds and runs.
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
This PR implements the ability to drop directories/files on the '+' button which in turn will open the tab/pane/window in the given starting path.
In order to do this, I refactored the click's lambda into a method and re-used it
Sadly I wasn't able to add note about the alt/shift feature (any ideas how to do this?)
Also most of the code is "look-a-like" from other places within the project, as I don't have much experience in windows development.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
implements #10073
## PR Checklist
* [ ] Closes#10073
* [ ] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
** tests were done manually both of the old feature (alt/shift+click) on the '+' and on the profiles
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
** no idea what to add there, if any.
* [ ] 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
## Validation Steps Performed
tested manually.
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] Supports #10563
* [ ] 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.
* [ ] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
When you use the size parameter to WideCharToMultiByte, it only
null-terminates the output string if the input string was
null-terminated within the specified range.
Burned in for 1k runs-
BEFORE
Summary: Total=1000, Passed=997, Failed=3
AFTER
Summary: Total=1000, Passed=1000, Failed=0
Fixes MSFT-34656993
Fix unbound read of cooked read buffer
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 756c8dcd4cf9551f5bf090b98bf3fba5498f8eff
Related work items: MSFT-32957145
An internal change CommandLineToArgVW to an apiset.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev a71b943e06c009085d6a2bb886dd50c2d0d2c276
Related work items: MSFT-32178383
## Summary of the Pull Request
This forces the `TermControl` to only use `ControlCore` and `ControlInteractivity` via their WinRT projections. We want this, because WinRT projections can be used across process boundaries. In the future, `ControlCore` and `ControlInteractivity` are going to be living in a different process entirely from `TermControl`. By enforcing this boundary now, we can make sure that they will work seamlessly in the future.
## References
* Tear-out: #1256
* Megathread: #5000
* Project: https://github.com/microsoft/terminal/projects/5
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50760270
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Most all this was just converting pure c++ types to winrt types when possible. I've added a couple helper projections with `til` converters, which made most of this really easy.
The "`MouseButtonState` needs to be composed of `Int32`s instead of `bool`s" is MENTAL. I have no idea why this is, but when I had the control OOP in the sample, that would crash when trying to de-marshal the bools. BODGY.
The biggest changes are in the way the UIA stuff is hooked up. The UiaEngine needs to be attached directly to the `Renderer`, and it can't be easily projected, so it needs to live next to the `ControlCore`. But the `TermControlAutomationPeer` needed the `UiaEngine` to help implement some interfaces.
Now, there's a new layer we've introduced. `InteractivityAutomationPeer` does the `ITextProvider`, `IControlAccessibilityInfo` and the `IUiaEventDispatcher` thing. `TermControlAutomationPeer` now has a
`InteractivityAutomationPeer` stashed inside itself, so that it can ask the interactivity layer to do the real work. We still need the `TermControlAutomationPeer` though, to be able to attach to the real UI tree.
## Validation Steps Performed
The terminal behaves basically the same as before.
Most importantly, I whipped out Accessibility Insights, and the Terminal looks the same as before.
## Summary of the Pull Request
Replaces the key chord editor in the actions page with a listener instead of a plain text box.
## References
#6900 - Settings UI Epic
## Detailed Description of the Pull Request / Additional comments
- `Actions` page:
- Replace `Keys` with `CurrentKeys` for consistency with `Action`/`CurrentAction`
- `ProposedKeys` is now a `Control::KeyChord`
- removes key chord validation (now we don't need it)
- removes accept/cancel shortcuts (nowhere we could use it now)
- `KeyChordListener`:
- `Keys`: dependency property that hooks us up to a system to the committed setting value
- this is the key binding view model, which propagates the change to the settings model clone on "accept changes"
- We bind to `PreviewKeyDown` to intercept the key event _before_ some special key bindings are handled (i.e. "select all" in the text box)
- `CoreWindow` is used to get the modifier keys because (1) it's easier than updating on each key press and (2) that approach resulted in a strange bug where the <kbd>Alt</kbd> key-up event was not detected
- `LosingFocus` means that we have completed our operation and want to commit our changes to the key binding view model
- `KeyDown` does most of the magic of updating `Keys`. We filter out any key chords that could be problematic (i.e. <kbd>Shift</kbd>+<kbd>Tab</kbd> and <kbd>Tab</kbd> for keyboard navigation)
## Validation Steps Performed
- Tested a few key chords:
- ✅single key: <kbd>X</kbd>
- ✅key with modifier(s): <kbd>Ctrl</kbd>+<kbd>Alt</kbd>+<kbd>X</kbd>
- ❌plain modifier: <kbd>Ctrl</kbd>
- ✅key that is used by text box: <kbd>Ctrl+A</kbd>
- ✅key that is used by Windows Terminal: <kbd>Alt</kbd>+<kbd>F4</kbd>
- ❌key that is taken by Windows OS: <kbd>Windows</kbd>+<kbd>X</kbd>
- ✅key that is not taken by Windows OS: <kbd>Windows</kbd>+<kbd>Shift</kbd>+<kbd>X</kbd>
- Known issue:
- global key taken by Windows Terminal: (i.e. quake mode keybinding)
- Behavior: global key binding executed
- Expected: key chord recorded
## Demo
## Summary of the Pull Request
Sends the additional xaml notification when the user presses the '+' or delete button for unfocused appearances
## PR Checklist
* [x] Closes#10673
* [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
It works now
`SRWLOCK`, as used by `std::shared_mutex`, is a inherently unfair mutex
and makes no guarantee whatsoever whether a thread may acquire the lock
in a timely manner. This is problematic for our renderer which relies on
being able to acquire the lock in a timely and predictable manner.
Drawing stalls of up to one minute have been observed in tests.
This issue can be solved with a primitive ticket lock, which is 10x
slower than a `SRWLOCK` but still sufficiently fast for our use case
(10M locks per second per thread). It's likely that any non-trivial lock
duration will diminish the difference to "negligible".
## Validation Steps Performed
* It still blends ✔️
This commit is a preparation for upcoming changes to
KeyChordSerialization for #7539 and #10203. It introduces several
string helpers to simplify key chord parsing and get rid of our implicit
dependency on locale sensitive functions, which are known to behave
erratically.
Additionally key chord serialization used to depend on iteration order
of a hashmap which caused different strings to be returned for the same
key chord. This commit fixes the iteration order and will always return
the same string.
## Validation Steps Performed
* Key bindings are correctly parsed ✔️
* Key bindings are correctly serialized ❔
## Summary of the Pull Request
Adds unfocused appearance creation/configuration in the SUI
There is now an 'Unfocused Appearance' section at the bottom of the 'Appearance' tab in a profile. There is a '+' button to create an unfocused appearance if one does not exist, or a delete button to delete the unfocused appearance if one exists (only one of these buttons is visible at a time).
## PR Checklist
* [ ] Closes #xxx
* [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
This commit is a preparation for upcoming changes to KeyChordSerialization for #7539 and #10203.
In order to support variadic macros, /Zc:preprocessor was enabled, which required changing unrelated parts of the project.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Project still compiles ✔️
When navigating the settings (or saving/discarding) the animation of the main content overflows the bar with the save and discard buttons. If the main content is encapsulated in a ScrollView the issue goes away.
Fixes one of the issues in #10609
## Validation Steps Performed
Clicked around a whole bunch and have not seen the overflow happen again. Verified that on tabs where scroll is necessary it can still be scrolled, and reflow of elements still functions.
Replaces `KeyModifiers` with the pretty much equivalent
`VirtualKeyModifiers` enum in winrt.
After doing this I noticed #10593 which changes the KeyChords a lot, but
it seems these PRs are still compatible
The issue also mentions replacing Vkey with
`Windows::System::VirtualKey`, but I chose not to because that enum only
includes a subset of the keys terminal supports here (no VK_OEM_* keys)
## Validation Steps Performed
Changed key bind in config, and confirmed it still works after
restarting terminal
Closes#877
## Summary of the Pull Request
RIS resets mouse mode and encoding
## PR Checklist
* [x] Closes#8613
* [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
## Summary of the Pull Request
When discarding or saving settings, the current navigation should be retained.
## References
Issue introduced by #10390
## PR Checklist
* [x] Closes#10617
* [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.
* [ ] 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
`menuItemsSTL` is filled with all _non_ profile navItems, then `menuItemsSTL` fills `menuItems`, then the profile navItems are added to `menuItems`. So to include the profile nav items in the iteration, `menuItems` needs to be used
## Validation Steps Performed
Spam discard and save buttons
## Summary of the Pull Request
This implements `GetAttributeValue` and `FindAttribute` for `UiaTextRangeBase` (the shared `ITextRangeProvider` for Conhost and Windows Terminal). This also updates `UiaTracing` to collect more useful information on these function calls.
## References
#7000 - Epic
[Text Attribute Identifiers](https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids)
[ITextRangeProvider::GetAttributeValue](https://docs.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-getattributevalue)
[ITextRangeProvider::FindAttribute](https://docs.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-findattribute)
## PR Checklist
* [X] Closes#2161
* [X] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
- `TextBuffer`:
- Exposes a new `TextBufferCellIterator` that takes in an end position. This simplifies the logic drastically as we can now use this iterator to navigate through the text buffer. The iterator can also expose the position in the buffer.
- `UiaTextRangeBase`:
- Shared logic & helper functions:
- Most of the text attributes are stored as `TextAttribute`s in the text buffer. To extract them, we generate an attribute verification function via `_getAttrVerificationFn()`, then use that to verify if a given cell has the desired attribute.
- A few attributes are special (i.e. font name, font size, and "is read only"), in that they are (1) acquired differently and (2) consistent across the entire text buffer. These are handled separate from the attribute verification function.
- `GetAttributeValue`: Retrieve the attribute verification of the first cell in the range. Then, verify that the entire range has that attribute by iterating through the text range. If a cell does not have that attribute, return the "reserved mixed attribute value".
- `FindAttribute`: Iterate through the text range and leverage the attribute verification function to find the first contiguous range with that attribute. Then, make the end exclusive and output a `UiaTextRangeBase`. This function must be able to perform a search backwards, so we abstract the "start" and "end" into `resultFirstAnchor` and `resultSecondAnchor`, then perform post processing to output a valid `UiaTextRangeBase`.
- `UiaTracing`:
- `GetAttributeValue`: Log uia text range, desired attribute, resulting attribute metadata, and the type of the result.
- `FindAttribute`: Log uia text range, desired attribute and attribute metadata, if we were searching backwards, the type of the result, and the resulting text range.
- `AttributeType` is a nice way to understand/record if the result was either of the reserved UIA values, a normal result, or an error.
- `UiaTextRangeTests`:
- `GetAttributeValue`:
- verify that we know which attributes we support
- test each of the known text attributes (expecting 100% code coverage for `_getAttrVerificationFn()`)
- `FindAttribute`:
- test each of the known _special_ text attributes
- test `IsItalic`. NOTE: I'm explicitly only testing one of the standard text attributes because the logic is largely the same between all of them and they leverage `_getAttrVerificationFn()`.
## Validation Steps Performed
- @codeofdusk has been testing this Conhost build
- Tests added for Conhost and shared implementation
- Windows Terminal changes were manually verified using accessibility insights and NVDA
This pull request brings back the "Base Layer" page, now renamed to
"Defaults", and the "Reset to inherited value" buttons. The scope of
inheritance for which buttons will display has been widened.
The button will be visible in the following cases:
The user has set a setting for the current profile, and it overrides...
1. ... something in profiles.defaults.
2. ... something in a Fragment Extension profile.
3. ... something from a Dynamic Profile Generator.
4. ... something from the compiled-in defaults.
Compared to the original implementation of reset arrows, cases (1), (3)
and (4) are new. Rationale:
(1) The user can see a setting on the Defaults page, and they need a way
to reset back to it.
(3) Dynamic profiles are not meaningfully different from fragments, and
users may need a way to reset back to the default value generated
for WSL or PowerShell.
(4) The user can see a setting on the Defaults page, **BUT** they are
not the one who created it. They *still* need a way to get back to
it.
To support this, I've introduced another origin tag, "User", and renamed
"Custom" to "None". Due to the way origin/override detection works¹, we
cannot otherwise disambiguate between settings that came from the user
and settings that came from the compiled-in defaults.
Changes were required in TerminalSettings such that we could construct a
settings object with a profile that does not have a GUID. In making this
change, I fixed a bit of silliness where we took a profile, extracted
its guid, and used that guid to look up the same profile object. Oops.
I also fixed the PropertyChanged notifier to include the
XxxOverrideSource property.
The presence of the page and the reset arrows is restricted to
Preview- or Dev-branded builds. Stable builds will retain their current
behavior.
¹ `XxxOverrideSource` returns the profile *above* the current profile
that holds a value for setting `Xxx`. When the value is the
compiled-in value, `XxxOverrideSource` will be `null`. Since it's
supposed to be the profile above the current profile, it will also be
`null` if the profile contains a setting at this layer.
In short, `null` means "user specified" *or* "compiled in". Oops.
Fixes#10430
Validation
----------
* [x] Tested Release build to make sure it's mostly arrow-free (apart from fragments)
Implements an `Appearances` xaml object and an `AppearanceViewModel` in the SettingsEditor project. Updates `Profiles` to use these new objects for its default appearance.
This is the first step towards getting `UnfocusedAppearance` into the SUI.
Adds try-catch blocks to the parts where we layer a fragment onto a profile and create a new profile from a fragment. This allows us to continue looping over the remaining fragments after failing to use a badly-formed one, instead of aborting prematurely.
## PR Checklist
* [x] Closes#10590
## Validation Steps Performed
Non-badly formed fragments get loaded even if there is a badly formed one somewhere
Sets the working directory of the terminal when invoked from the shell extension. This ensures that new tabs opened with a starting directory of `.` open in the directory that the terminal was invoked from.
Closes#8933
## Validation Steps Performed
Manually tested - default PowerShell profile set to use home directory, Windows PowerShell profile set to use current directory. Launched via the shell extension and the default profile opened in the explorer directory, as did a new Windows PowerShell tab.
Change accessibility notifier creation so we do not create one when we're in PTY mode. (Guard all call sites to skip math/event work when the notifier is null.) MSAA events are legacy events that are registered for globally and used by some screen readers to find content in the conhost window. The PTY mode is not responsible for hosting the display content or input window, so it makes sense for it to not broadcast these events and delegate the accessibility requirement to the connected terminal.
## References
- #10537
## PR Checklist
* [x] Closes#10568
* [x] I work here
* [x] Manual test launches passed.
Currently, when the user changes the console codepage (manually or via a script) the GDI engine tries to find and set the "best possible" font. The "best possible" here is charset-wise, it doesn't mean that the font is actually better or more eye-candy for the user.
Example:
- Open cmd
- Set the font to Consolas
- Enter `chcp 932`
- Suddenly, a wild MS Gothic appears!
This kind of makes sense (*"if I'm changing the codepage I probably want to see the national characters"*) but it doesn't happen anywhere else - all other apps just substitute the missing glyphs.
After #10472 / #10478 this magic should finally work here as well. So, do we still need to change the whole font? Terminal doesn't do that after all.
## Validation Steps Performed
Download [932.cmd.txt](https://github.com/microsoft/terminal/files/6697577/932.cmd.txt),
rename to 932.cmd, run it, check if the output is still readable.
Closes#10497
## Summary of the Pull Request
Adds a feature flag `Feature_EditableActionsPage` that controls whether the Actions page in the Settings UI is read-only vs editable. The editable version is disabled for `Release` builds and enabled everywhere else (i.e. Dev, Preview, etc...).
Validated using `<stage>` `AlwaysEnabled` and `AlwaysDisabled`.
## References
#6900 - Actions Page Epic
## PR Checklist
Closes#10578
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Implementation of #10477 - handle surrogate pairs in GDI renderer.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes#10477
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] 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: #10477
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
### Why not let Windows draw surrogate pairs? It can do that.
Basically, the comment says everything:
c90de69250/src/renderer/gdi/paint.cpp (L346-L347)
However, handling things above U+FFFF doesn't really require extra effort. It's enough to:
- Put *all* characters to the output buffer
- Set the first width to cluster width and the rest to 0
- Sit back and relax while Windows does the rest
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
```CMD
@echo off
chcp 65001
echo 𠜎𠝹𠱓𠱸𠲖𠳏𠳕𠴕𠵼𠵿𠸎
echo 👨👩👧👦
```
Save this as a UTF-8 cmd file and run.
### Before the change
### After the change
An example of a third party app working with surrogate pairs in a patched OpenConsole:
As discussed, this change doesn't claim to be the full support for surrogate pairs (there are still corner cases possible), but brings it on par with Terminal with minimal effort.
This PR is a small start in a broader "Minimize to Tray" feature (#5727).
This particular change is scoped only to the scenario when a quake window
is minimized. Currently the only way to bring back the quake window
when it's minimized is to press the global hotkey again. This gives another
option - to press the terminal icon in the tray.
Eventually though, minimize to tray will be available for any window, and
I'd like more time to flesh out the general porpoise scenarios and context
menus. Having just a bit in this PR also helps reviewers by keeping it small!
Delay load call SetThreadDescription to restore WPF renderer on Win7
## PR Checklist
* [x] Closes something @DHowett asked me to do.
* [x] I work here
* [x] I F5'd it on a version with this function and it still works
## Detailed Description of the Pull Request / Additional comments
I keep forgetting that anything in the WPF control needs to keep working on Win7. Or more specifically... I remember this fact for the DX renderer, but not for the render thread base. Oops. Turns out this particular convenience method to set thread descriptions for visibility inside the debugger (to make my life easier) only works down to 1607 (see https://docs.microsoft.com/windows/win32/api/processthreadsapi/nf-processthreadsapi-setthreaddescription). Since it's just a debugging convenience... skipping it entirely when the procedure is not found should be fine. Also I don't try to load `kernel32.dll` and just get the handle of the existing module (which per the remarks at https://docs.microsoft.com/windows/win32/api/libloaderapi/nf-libloaderapi-getmodulehandlew will not increment the module reference count) because `kernel32.dll` pretty much has to be there or we're already in hot water.
## Summary of the Pull Request
This adds the "add new" button to the actions page. It build on the work of #10220 by basically just adding a new list item to the top of the key binding list.
This also makes it so that if you click the "accept changes" button when you have an invalid key chord, we don't do anything.
## References
#6900 - Actions page Epic
#9427 - Actions page design doc
#10220 - Actions page PR - set action
## Detailed Description of the Pull Request / Additional comments
- `ModifyKeyBindingEventArgs` is used to introduce new key bindings. We just ignore `OldKeys` and `OldActionName` because both didn't exist before.
- `IsNewlyAdded` tracks if this is an action that was added, but has not been confirmed to add to the settings model.
- `CancelChanges()` is directly bound to the cancel button. This allows us to delete the key binding when it's clicked on a "newly added" action.
## Validation Steps Performed
- Cancel:
- Deletes the action (because it doesn't truly exist until you confirm changes)
- Accept:
- Adds the new action.
- If you attempt to edit it, the delete button is back.
- Add Action:
- Delete button should not be visible (redundant with 'Cancel')
- Action should be initialized to a value
- Key chord should be empty
- Cannot add another action if a newly added action exists
- Keyboard interaction:
- escape --> cancel
- enter --> accept
- Accessibility:
- "add new" button has a name
- Interaction with other key bindings:
- editing another action --> delete the "newly added" action (it hasn't been added yet)
- only one action can be edited at a time
This commit adds support for bold text in DxRenderer.
For now, bold fonts are always rendered using DWRITE_FONT_WEIGHT_BOLD
regardless of the base weight.
As yet, this behavior is unconfigurable.
References
Previous refactoring PRs: #9096 (DxFontRenderData) #9201 (DxFontInfo)
SGR support tracking issue: #6879Closes#109
The code in this file was adapted from the STL on the 2021-07-05.
It backports the following Windows 8 functions to Windows 7:
* WaitOnAddress
* WakeByAddressSingle
* WakeByAddressAll
These functions are used within `til`. This commit will allow `til` to be used in the conhost source code.
Validation
* [x] correct .dll loads on Windows 7
* [x] correct .dll loads on Windows 10
* [x] link line for PublicTerminalCore prefers this fake apiset over kernel32
Previously `TermControl::Close` destroyed all `ThrottledFunc`s to ensure they're not scheduling any callbacks on the UI thread, as the call to `Close` signals the point at which the `TermControl` isn't part of the UI thread anymore. `_CursorPositionChanged` tried to prevent access to the potentially deallocated `_tsfTryRedrawCanvas` by checking the `std::shared_ptr` for nullability, but since the deallocation happens on the UI thread and the nullability check on a background thread, this check introduced a race condition.
This commit solves the issue by not deallocating any `ThrottledFunc`s anymore and instead checking the `_closing` flag inside the `ThrottledFunc` callback on the UI thread.
Additionally this commit cleans up some antipatterns around the use of `std::optional`.
## PR Checklist
* [x] Closes#10479
* [x] Closes#10302
## Validation Steps Performed
* Opening and closing tabs doesn't crash ✔️
* Printing long text doesn't crash ✔️
* Manual scrolling doesn't crash ✔️
* ^G / the audible bell doesn't crash ✔️
## Summary of the Pull Request
#7960 was caused by `UiaTextRangeBase::_blockRange` not being initialized, thus pointing to random memory. In most cases, we initialize it properly in `RuntimeClassInitialize`, however, the copying version of `RuntimeClassInitialize` doesn't actually copy it over, resulting in it still containing random memory.
NVDA (and other screen readers) occasionally use `Clone` (really just the copy initializer), resulting in this bug occurring randomly.
## PR Checklist
* [X] Closes#7960
* [X] Tests added/passed
## Validation Steps Performed
Test failed before the change, but passes after the change.
This introduces the ability to set the action for a key binding. A combo box is used to let the user select a new action.
## References
#6900 - Actions page Epic
#9427 - Actions page design doc
#9949 - Actions page PR
## Detailed Description of the Pull Request / Additional comments
### Settings Model Changes
- `ActionAndArgs`
- new ctor that just takes a `ShortcutAction`
- `ActionMap`
- `AvailableActions` provides a map of all the "acceptable" actions to choose from. This is a merged list of (1) all `{ "command": X }` style actions and (2) any actions with args that are already defined in the ActionMap (or any parents).
- `RegisterKeyBinding` introduces a new unnamed key binding to the action map.
### Editor Changes
- XAML
- Pretty straightforward, when in edit mode, we replace the text block with a combo box. This combo box just presents the actions you can choose from.
- `RebindKeysEventArgs` --> `ModifyKeyBindingEventArgs`
- `AvailableActionAndArgs`
- stores the list of actions to choose from in the combo box
- _Unfortunately_, `KeyBindingViewModel` needs this so that we can populate the combo box
- `Actions` stores and maintains this though. We populate this from the settings model on navigation.
- `ProposedAction` vs `CurrentAction`
- similar to `ProposedKeys` and `Keys`, we need a way to distinguish the value from the settings model and the value of the control (i.e. combo box).
- `CurrentAction` --> settings model
- `ProposedAction` --> combo box selected item
## Validation Steps Performed
- Cancel:
- ✔️ change action --> cancel button --> begin editing action again --> original action is selected
- Accept:
- ✔️ don't change anything
- ✔️ change action --> OK! --> Save!
- NOTE: The original action is still left as a stub `{ "command": "closePane" }`. This is intentional because we want to prevent all modifications to the command palette.
- ✔️ change action & change key chord --> OK! --> Save!
- ✔️ change action & change key chord (conflicting key chord) --> OK! --> click ok on flyout --> Save!
- NOTE: original action is left as a stub; original key chord explicitly unbound; new command/keys combo added.
Introduces `FontConfig`, an object that isolates font-related settings
in our profiles
Users can now define font settings in their json as so:
```
"font":{
"face": "Consolas",
"size": 12
}
```
Backwards compatible with the currently expected way of defining font
settings in the json, note however that upon hitting 'Save' in the SUI,
these settings **will be rewritten to the font-object style in the json
(as above)**.
## Validation Steps Performed
Existing functionality works, new functionality works
References #1790Closes#6049
This commit adds a "(requires relaunch)" suffix to the header of the language picker control.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
Passing structures larger than the register size is very expensive
due to Microsoft's x64 calling convention. We could reduce the
overhead by passing the string-view by reference, but this forces us
to allocate the parameters as static string-views on the data
segment of our binary. I've found that passing them as classic
C-strings is more ergonomic instead and fits the need for
high performance in this particular code.
This improves performance for VT-heavy output by 15-20%.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
This commit introduces a basic ApplicationState class, without being used for anything yet to aid reviewers. At a later point actual usages of this new class may be added separately.
## References
This commit is an initial step towards implementing #8324.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Creating a `state.json` with `{"generatedProfiles":["{53e75ed9-2b63-4118-856d-0510c4f6b97e}"]}` updates the ApplicationState, as observed through a debugger ✔️
* Deleting the "generatedProfiles" field sets the corresponding field back to nullopt ✔️
Try to throttle the cursor redrawing in the conhost world.
The motivation of this is the high CPU usage of `TriggerRedrawCursor` (#10393).
This can be seen as the conhost version of #2960.
This saves 5%~8% of the CPU time.
Supports #10462.
## Summary of the Pull Request
Updates the `closeTab` action to optionally take an index.
## PR Checklist
* [x] Closes#7180
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [x] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: MicrosoftDocs/terminal#347
* [x] 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
Added the following configuration to `settings.json` and validated both key combinations behaved as expected. Also opened the command palette and ensured that the actions were displayed.
```json
{ "command": "closeTab", "keys": "ctrl+shift+delete" },
{ "command": { "action": "closeTab", "index": 0 }, "keys": "ctrl+shift+end" }
```
This commit introduce three new `til` features:
* "til/latch.h": A std::latch clone, until we're on C++20.
* "til/mutex.h": A safe mutex wrapper, which only allows you access to the protected data after locking it. No more forgetting to lock mutexes!
* "til/throttled_func.h": Function invocation throttling used to be available as the `ThrottledFunc` class already. But this class is vastly more efficient and doesn't rely on any WinRT types.
This PR also adds a `til::ends_with` string helper which is `til::starts_with` counterpart.
## Validation Steps Performed
* Scrollbar throttling still works as it used to ✔️
* No performance regressions when printing big.txt ✔️Closes#10393
This PR Introduces `DxFontInfo` to simplify the logic in
`DxFontRenderData`.
`DxFontInfo` aims to be the DWrite equivalent of `FontInfo` &
`FontInfoBase` in GDI. It encapsulates the needed information to
represent a displayable font face. It also provides the ability to
resolve a font face based on the available fonts on the system.
## References
This is a follow-up of #9096.
Initial Italic support was introduced by #8580.
The motivation behind this is to support bold & bold-italic text in
Windows Terminal.
Fix startup race of resizing ConPTY
- Depending on what the timing and ordering is of the message coming in
from the signal thread, it may be applied to the startup structure
after the I/O thread has begun initializing the console buffer
structures but before it has signaled that it is done and the signal
thread is ready to make changes directly. This likely happens because
the end of the I/O thread setup has a weird unlock/lock jog for the
input thread and the signal thread might have been scheduled in the
middle of it.
- My resolution here is to ensure that the signal thread just keeps
storing the latest resize message until it is told that everything is
initialized. Whomever comes in to tell the signal thread this
information (under lock) will pickup and run the resize if one came in
before everything was ready. This should resolve the race.
## Validation Steps Performed
- o-sdn-o confirms this resolves their issue
Closes#10400
Replace PolyTextOutW with ExtTextOutW to allow substitution of missing
glyphs from other fonts.
Why not let Windows substitute the glyphs that are missing in the
current font? Currently the GDI renderer of conhost/OpenConsole uses
`PolyTextOutW` for drawing. `PolyTextOutW` doesn't try to substitute
any missing glyphs, so the only way to see, say, Hiragana is to change
the _whole font_ to something like MS Gothic (which is eye-bleeding, to
be honest).
A trivial replace `PolyTextOutW` -> `ExtTextOutW` does the trick.
Switch to `PolyTextOutW` happened in Windows 7 along with introduction
of conhost.exe. Substitution worked in previous Windows versions, where
internal NT interfaces were used.
# Before the change:
# After the change:
Closes#10472
The CursorStyle enum is declared as being of type `uint` on the C# side,
but as `size_t` on the C++ side. There's a C# size_t impostor we could
use, System.UIntPtr, but I don't want to risk changing the public API of
TerminalTheme and I don't know if it can be used as a base type for an
enum.
Anyway, since we don't have more than four billion cursor types I chose
to narrow the field to a uint32_t and unpack it in TerminalSetTheme.
Fixes#10485
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 40df712b33a40e76aeeac87f823bbb028d2e3972
Related work items: MSFT-32007459
This update brings some significant changes to the Cascadia family:
* Arabic and Hebrew support
* Italics (the new ones, not the cursive ones)
* Tweaked letterforms and fixed interpolation values for the upright
faces.
Since we now have four font files, this commit also relocates them to a
much more reasonable place (res/fonts/) and tidies up the build and
exclude rules to make them more extensible in the future.
This commit introduces localization for the "Open in Windows Terminal"
menu item and differentiates it based on compile-time branding (rather
than runtime detection!).
@leonMSFT's tray icon pull request had the excellent idea to use the
TerminalApp's resource compartment for auxiliary resources for projects
that can't otherwise be localized the same way. Doing localization in
the shell extension (or WindowsTerminal.exe) would require us to use
MUIRCT and split the build process up to support mui files. That's a
huge amount of work... but this is *not* a huge amount of work.
Fixes#6112
## Summary of the Pull Request
#10297 found a bug in `ActionMap` where the `ToggleCommandPalette` key chord could not be found using `GetKeyBindingForAction`.
This was caused by the following:
- `AddAction`: when adding the action, the `ActionAndArgs` is basically added as `{ToggleCommandPalette, ToggleCommandLineArgs{}}` (Note the default ctor used for the action args)
- `GetKeyBindingForAction`: we're searching for an `ActionAndArgs` structured as `{ToggleCommandPalette, nullptr}`
- Since these are _technically_ two different actions, we are unable to find it.
This issue was fixed by making the `Hash(ActionAndArgs)` function smarter! If the `ActionAndArgs` has no args, but the `ShortcutAction` _supports_ args, generate the args using the default ctor.
By making `Hash()` smarter, everybody benefits from this logic! We can basically now enforce that `ActionAndArgs{ <X>, nullptr } == ActionAndArgs{ <X>, <default_ctor> }`.
## Validation Steps Performed
- Added a test.
- Tested this on #10297's branch and this does fix the bug
## Summary of the Pull Request
`ApplicationLanguages::PrimaryLanguageOverride` requires packaged activation.
This PR prevents any such application crashes, by skipping any calls to `PrimaryLanguageOverride`, as well as hiding the language selector in the settings UI.
## PR Checklist
* [x] I work here
* [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.
## Validation Steps Performed
When WT is run unpackaged:
* Doesn't crash during start ✔️
* SUI doesn't show the language selector ✔️
Persist inbox conhost; delegate control activities to it via a pipe
## PR Checklist
* [x] Closes#10194 - WSL Debug Tap doesn't work
* [x] Closes#10134 - WSL Parameter is Incorrect
* [x] Closes#10413 - Ctrl+C not passed to client
* [x] Closes#10414 - Leftover processes on abrupt termination
* [x] Might help #10251 - Win+X Powershell sometimes fails to attach
* [x] I work here
* [x] Manually tested with assorted launch scenarios
## Detailed Description of the Pull Request / Additional comments
It turns out that there's a bit of ownership that goes on with the original inbox `conhost.exe` and the operating system/driver. The PID of that original `conhost.exe` is stowed when the initial connection is established and it is verified for several activities. This means that the plan of letting it go completely away and having the `OpenConsole.exe` take over all of its activities must be slightly revised.
I have tested the following two alternatives to keeping `conhost.exe` around and they do not work:
1. Replacing the original owner `conhost.exe` with `OpenConsole.exe` - A.) The driver does not allow this. Once the owner is registered, it cannot be replaced. B.) There's no way of updating this information inside the client process space and it is kept there too in the `kernelbase`/`conclnt` data from its initial connection.
2. Attempting to pick up the first packet (to determine headed/headless and other initial connection information that we use to determine whether handoff is appropriate or not) prior to registering any owner at all. - The driver doesn't allow this either. The owner must be registered prior to a packet coming through.
Put this mental model in your head:
CMD --> Conhost (inbox) --> OpenConsole (WT Package) --> Terminal (WT Package)
So since the `conhost.exe` needs to stick around, here's what I'm doing in this PR:
- `conhost.exe` in the OS will receive back the `OpenConsole.exe` process handle on a successful handoff and is expected to remain alive until the `OpenConsole.exe` exits. It's now waiting on that before it terminates itself.
- `conhost.exe` in the OS will establish a signal channel pipe and listen for control commands from `OpenConsole.exe` in a very similar fashion to how the `ConPTY` signal pipe operates between the Terminal and the PTY (provided by `OpenConsole.exe` in this particular example.) When `OpenConsole.exe` needs to do something that would be verified by the OS and rejected... it will instead signal the original `conhost.exe` to do that thing and it will go through.
- `conhost.exe` will give its own handle through to `OpenConsole.exe` so it can monitor its lifetime and cleanup. If the owner is gone, the session should end.
- Assorted handle cleanup that was leading to improper exits. I was confused between `.reset()` and `.release()` for some of the `wil::unique_any<T>` handling and it lead to leaked handles. The leaked handles meant that threads weren't aware of the other sides collapsing and wouldn't cleanup/terminate appropriately.
How does this fix things?
- For the WSL cases... WSL was specifically looking up the owner PID of the console session from the driver. That was the `conhost.exe` PID. If it exits, that PID isn't valid and is recycled. Thus the parameter is incorrect or other inappropriate WSL setup behaviors.
- Ctrl+C not passed... this is a signal the operating system rejects from a PID that is not the owner. This is now relayed through the original owner and it works.
- Leftover processes... I believe I explained this was both not-enough-monitoring of each others' process lifetimes coupled with mishandling of release/resetting handles and leaking them.
- Powershell sometimes fails to attach... my theory on this one is that it's a race that became upset when the `conhost.exe` disappeared while something about Powershell/.NET was still starting, much like the WSL one. I believe now that it is sticking around, it will be fine.
Also, this WILL require an OS update to complete improvement of functionality and I have revised the interface ID. This is considered an acceptable breaking change with no mitigation because we said this feature was an alpha preview.
## Validation Steps Performed
- Launched WSL with defapp set, it works
- Launched WSL with defapp set and the debug tap on, it works and opens in two tabs
- Launched CMD, ran ping, did Ctrl+C, it now receives it
- Launched Win+X powershell a ton of times. It seems fine now
- Launched cmd, powershell, wsl, etc. Killed assorted processes in the chain (client/conhost/openconsole/windowsterminal) and observed in Process Explorer (with a long delta timer so I could see it) that they all successfully tear down now without leftovers.
An exception was introduced for the 'Toggle Command Palette' action to **not** being dispatched. Otherwise the command palette that was just closed will become visible again.
## PR Checklist
* [x] Closes#10240
* [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
- Selecting the `Toggle command palette` item in the command palette will now properly close the command palette.
- Opening and closing the Command Palette through shortcut keys is still working fine.
- Other command palette items are still working fine as well.
## Summary of the Pull Request
This commit introduces a copy constructor/operator for
`_CONSOLE_API_MSG`. The change is not trivial as the struct contains a
union of unnamed structs that cannot be copied using regular language
features. As such a copy operator using `memcpy` was implemented.
Additionally all access specifiers were removed, as those allow a C++
compiler to reorder struct members. This would break message passing.
This commit is a good opportunity to prevent such miscompilations
proactively.
## Validation Steps Performed
* Command prompts of WSL2 fish-shell and pwsh still work ✔️Closes#10076
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Immediately cancelling the preview when the user is navigating back from a nested command.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#10165
* [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.
* [ ] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
Basically 2 changes are done here:
- Allow the click handler to run for the back button when the button has focus and user hits the enter key (similarly as hitting space now).
- Now immediately cancelling the preview when the user is navigating back. Felt nicer to do it immediately at that point then keeping the preview active until the user hits cancel to close the palette. So the preview is already cancelled at step **5** instead of 6 as mentioned in the reproduction steps here https://github.com/microsoft/terminal/issues/10165#issue-899838383. But of course let me know if you're not agreeing here 😀 .
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
- Open 'Color Scheme' and verified preview is still working fine when selecting different schemes.
- After tabbing back to the Back button verified that when hitting enter or space the preview is cancelled and the original color scheme is being used again.
- Then after going back to 'Color Scheme' previews are still working ok.
- After hitting Enter on one of the Color Schemes the scheme still becomes active as before.
## Summary of the Pull Request
Fixes a bug where the edit button in the actions page would have white text when in light theme. Now, we just fallback to XAML's built-in value (black in light theme and white in dark theme).
## References
#6900 - Epic
Closes#10406
This commit makes the feature staging header build in OS razzle.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev cd1428ce557238663b7a748763ea8ce082b88256
Related work items: MSFT-33722503
## Summary of the Pull Request
This commit fixes various race conditions regarding the settings UI. It's unsafe to write to class members from background threads without acquiring mutexes or yielding to the main thread first.
By changing the settings reload code path to yield to the main thread early, we're able to cut down on code complexity and unsafe member accesses.
## PR Checklist
* [x] Closes#9273
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Settings UI reloads without crashing ✔️
This pull request converts four of our existing `#ifdef` (or `#ifndef`)
`INSIDE_WINDOWS` blocks to til::features:
* Attempting to establish a handoff session (inside Windows only)
* The ability to *receive* a handoff session (outside Windows only)
* The DX engine (outside Windows only) and shaders (also outside only)
* Whether we use numpad event synthesis for clipboard/conpty (inside
Windows only)
Most of these are using the preprocessor verison of til::feature, only
because it is more difficult to gate the inclusion of headers on
constant expressions. I'd love to prefer the compile time version.
## Summary of the Pull Request
This PR adds a global "language" setting, which may be set to any supported BCP 47 tag.
Additionally a ComboBox is added to the settings UI under "Appearance", listing all languages with their localized names.
This PR introduces one new issue: If you change the language while the app is running, the UI will be in a torn state, as not all UI elements refresh automatically if the `PrimaryLanguageOverride` is changed.
## PR Checklist
* [x] Closes#5497
* [x] I work here
* [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
* [x] Schema updated
## Validation Steps Performed
* UI language changes when changing the "language" in settings.json before starting WT / while WT is running. ✔️
* "language" field is removed from settings.json if "Use system default" is selected. ✔️
* "language" field is added or updated in settings.json if any other language is selected. ✔️
* Removes qps- languages if debugFeatures is false. ✔️
* Correctly refreshes all UI elements with the new language. ❌
This pull request implements a "feature flagging" system that will let
us turn Terminal and conhost features on/off by branch, "release" status
or branding (Dev, Preview, etc.).
It's loosely modelled after the Windows OS concept of "Velocity," but
only insofar as it is driven by an XML document and there's a tool that
emits a header file for you to include.
It only supports toggling features at compile time, and the feature flag
evaluators are intended to be fully constant expressions.
Features are added to `src\features.xml` and marked with a "stage". For
now, the only stages available are `AlwaysDisabled` and `AlwaysEnabled`.
Features can be toggled to different states using branch and branding
tokens, as documented in the included feature flag docs.
For a given feature Feature_XYZ, we will emit two fixtures visible to
the compiler:
1. A preprocessor define `TIL_FEATURE_XYZ_ENABLED` (usable from MIDL,
C++ and C)
2. A feature class type `Feature_XYZ` with a static constexpr member
`IsEnabled()` (usable from C++, designed for `if constexpr()`).
Like Velocity, we rely on the compiler to eliminate dead code caused by
things that compile down to `if constexpr (false)`. :)
Michael suggested that we could use `WindowsInbox` as a branding to
determine when we were being built inside Windows to supplant our use of
the `__INSIDE_WINDOWS` preprocessor token. It was brilliant.
Design Decisions
----------------
* Emitting the header as part of an MSBuild project
* WHY: This allows the MSBuild engine to ensure that the build is
only run once, even in a parallel build situation.
* Only having one feature flag document for the entire project
* WHY: Ease.
* Forcibly including `TilFeatureStaging` with `/FI` for all CL compiler
invocations.
* WHY: If this is a project-wide feature system, we should make it as
easy as possible to use.
* Emitting preprocessor definitions instead of constexpr/consteval
* WHY: Removing entire functions/includes is impossible with `if
constexpr`.
* WHY: MIDL cannot use a `static constexpr bool`, but it can rely on
the C preprocessor to remove text.
* Using MSBuild to emit the text instead of PowerShell
* WHY: This allows us to leverage MSBuild's `WriteOnlyWhenDifferent`
task parameter to avoid changing the file's modification time when
it would have resulted in the same contents. This lets us use the
same FeatureStaging header across multiple builds and multiple
branches and brandings _assuming that they do not result in a
feature flag change_.
* The risk in using a force-include is always that it, for some
reason, determines that the entire project is out of date. We've
gone to great lengths to make sure that it only does so if the
features _actually materially changed_.
## Summary of the Pull Request
Fixes a bug where top-level iterable commands were not serialized.
## PR Checklist
* [X] Closes#10365
* [X] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
- `Command::ToJson`:
- iterable commands deserve the same treatment as nested commands
- `ActionMap`:
- Similar to how we store nested commands, iterable commands need to be handled separately from standard commands. Then, when generating the name map, we make sure we export the iterable commands at the same time we export the nested commands.
Summon, not toggle visibility, window on command line dispatch
## PR Checklist
* [x] Closes#10292
* [x] I work here
* [x] Manual test
## Detailed Description of the Pull Request / Additional comments
- This is the same as #10389, just a different route. I didn't realize it at the time.
## Validation Steps Performed
- Opened a window. Dispatched the `wt -w 0 - p <profile>` and watched it join/summon instead of minimize the active WT.
The flyout wasn't very polished, so I did some adjustments.
It's all visual changes, functionality should be the same.
* made the flyout use OverlayCornerRadius and 16px padding (to match WinUI 2.6)
* changed ColorPicker to muxc:ColorPicker for new styles (the color schemes picker too)
* changed "Custom" Button into a ToggleButton
* no longer needs ellipsis - localization files should be updated
* OK button was moved to the right and uses accent color
* adjusted margins and padding
* tweaked the color boxes to _look_ like the ones in color schemes
 
## Validation Steps Performed
* Color picker in settings UI still works ✔️
* Color picker for tabs still works ✔️
Activate window only (no toggle) when inbound connection arrives
## PR Checklist
* [x] Closes#10386
* [x] I work here
* [x] Manual test passed.
## Detailed Description of the Pull Request / Additional comments
The default for the `SummonWindowBehavior` is a toggle of the visibility state. I didn't realize that. We do not want that for inbound connections. We want always-brought-to-front.
## Validation Steps Performed
- Made the change. Launched Terminal as default as active window. Runbox'd another command. It didn't hide itself like it used to. Stays visible.
This PR adds a new PercentageSignConverter that appends the percentage sign to a number. The new converter is being used by the Acrylic opacity slider label and the Background image opacity slider label.
* [x] Closes#10289
## PR Checklist
* [x] Closes random crash that @lhecker sent me on Teams
* [x] I work here.
## Detailed Description of the Pull Request / Additional comments
- Any change to the renderer engine has to be done under lock. Leonard gave me a crash where the dirty rectangles changed out from under the renderer thread. By inspection, only one spot in `ControlCore` is modifying the engine outside of lock.... here. The dump is too far along to definitively prove the issue and it's sort of a race so its difficult to repro. But the theory is sound that all writes to the dirty regions must be done under lock. So here's a fix.
Restore embedded manifests to say 18362 or unpackaged activation won't work (for helix testing.)
## PR Checklist
* [x] Closes#10265
* [x] I work here
* [x] Tests now pass
## Detailed Description of the Pull Request / Additional comments
- Unpackaged activation uses the embedded manifest inside the exe. We use unpackaged activation to run our tests in Helix as it's easier that way. Turns out the 1903/19h1 OS thinks 19041 isn't greater than the minimum XAML islands version of 18226 and blocks the load of `TerminalApp.dll` causing a crash (fail fast) on launch. For **REASONS**, 18362 is considered greater than 18226.
- Packaged activation will use the value in the .appxmanifest and everything is somehow still fine there even with it saying 19041 now.
## Validation Steps Performed
- Kicking a Helix-run off on this branch: https://dev.azure.com/ms/terminal/_build/results?buildId=177336&view=results
## Summary of the Pull Request
Incremental linking was disabled for debug builds only by mistake in the past.
It can't hurt to have it enabled for debug builds.
## PR Checklist
* [x] I work here
* [x] Project still builds
Applies feedback from https://github.com/microsoft/terminal/pull/9949#pullrequestreview-662590658
Highlights include:
- bugfix: make all edit buttons stay visible if the user is using assistive technology
- rename a few functions and resources to match the correct naming scheme
- update the localized text for a conflicting key chord being assigned
- provide better comments throughout the actions page code
## References
#9949 - Original PR
Closes#10168
Just come cleanup I did not manage to get to before #9270 merged.
Specifically:
- We only initialize the animation and timer if we need them
- We don't repeatedly destroy/create the timer
## Validation Steps Performed
It still works
C++/WinRT has a way to ensure that we use `make<>` instead of allocating
WinRT objects on the stack, but until 10.0.19041 the XAML compiler
generated code that violated that rule.
Because of how make detection is implemented, it must create a derived
type (and so WinRT implementation types can't be `final`).
I cannot for the life of me repro the original bug. I've got fonts with bad permissions SxS, I've tried installing a font twice, I've tried stopping the font cache service. No idea how to manually repro the original bug.
BUT theoretically, this function should never throw. So lets just switch this to a `LOG_IF_FAILED`, and hope that this goes away?
* [x] Fixes#10211?
* [x] built & ran manually.
Unclear if this can get cherry-picked trivially to 1.8. Code's pretty trivial though so if we need another PR for that, it can be arranged.
Stop startup crash by logging when monarch fails to register inbound connections, but still crash when COM attempted to start us
## References
- See also #10243
## PR Checklist
* [x] Closes#10233
* [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
- This should stop the crash on launch until we can get the internal teams to resolve the catalog issue
- I left the COM -Embedding start fail fast though so it won't take forever to time out (as default timeout is 3-5 minutes). I will change that if it becomes necessary.
## Validation Steps Performed
- I basically have to guess at this one based on the crash dump and Watson logs because it happens sporadically when the platform messes up on us.
ConptyConnection has two different failure modes:
1. We failed to initialize the pseudoconsole or create the process
2. The process exited with an error code.
Until this commit, they were treated the same way: closeOnExit=always
would force the pane/tab to be destroyed. This was very bad in case 1,
where we would display a (possibly useful) error message and then
immediately close the window.
This was made even worse by the change in #10045. We removed
startingDirectory validation and promoted it to an error message (so
that we could eventually let the connection handle startingDirectory in
its own way.) This of course revealed that a number of users had set
invalid starting directories… and those users included some who set
closeOnExit to always. Boom: instant "terminal opens and crashes"¹
In this commit, we introduce detection for a connection that fails
before it's been established. When that happens, we will ignore the
user's closeOnExit mode.
¹ It only looks like a crash; it's actually _technically_ functioning
properly.
Closes#10225.
Prevent crashes in Settings UI launch on OS versions before package management extensions
## PR Checklist
* [x] Closes#10106
* [x] I work here
* [x] Manual tests passed.
## Detailed Description of the Pull Request / Additional comments
- On older OS versions like 18363, some of the COM interfaces we use to look up information from the OS application package management catalog (to find default terminals) are unavailable. This returns `E_NOINTERFACE`. This then ends up returning an empty list of items and null as a selected item.
- I had intended for that to not return that particular error all the way up and just log it because the console and terminal lookup functions always return at least one element: the one representing the `conhost.exe` that is already on the machine.
- I have changed the "default packages" lookup to log instead of return failures like E_NOINTERFACE such that it can continue processing and make the "package" of the hardcoded `conhost.exe` default no matter what. (It will still return an error if there are somehow 0 packages because that code changed or some other catastrophic event happened...)
- I have also changed the Model to have a nulled DefaultTerminal model object (as all winrt objects are nullable) instead of using an optional. I did this because XAML is perfectly happy receiving a `nullptr` for a selected item and will just not select anything. By contrast, if it has an exception occur... it will just bubble that out and crash.
## Validation Steps Performed
- Simulated no items returned from list and nullptr returned to XAML on Current() method of Model. Validated XAML will happily select no item from list (and is fine with an empty list of items... that is it doesn't crash).
- Simulated downlevel OS returning package management errors in lookup catalog functions after the hardcoded default is added to the list. Ensured that this error is only logged, the remainder of the package identification functions make the hardcoded default package, and it is presented as your one and only option in the XAML.
Summon the listening window when it receives an inbound connection
## PR Checklist
* [x] Closes#9460
* [x] I work here.
* [x] Manual test.
## Detailed Description of the Pull Request / Additional comments
- We cannot just send our window to foreground by simply calling user32 on the window handle. But fortunately, the remoting behavior already has a summon window function with a workaround for the Quake functionality.
- This bubbles up an event from the TerminalApp's Page to the WindowsTerminal's Apphost so it can call the same window summoning behavior in IslandWindow as is triggered when the Monarch dictates this out of the Microsoft.Terminal.Remoting project.
## Validation Steps Performed
- Opened the Terminal with it registered as DefTerm. Activated some other windows to the foreground. Start > Run > Cmd. Tab connects and opens in existing Terminal and it is brought to foreground.
- With no running Terminal and registered as DefTerm, do Start > Run > Cmd. New Terminal is spawned and it is brought to foreground
## Summary of the Pull Request
VS16.10 and later contain two regressions:
* Marking the use of `pshpack*.h` in system headers with C4103
* The newly included, builtin `AssemblyReference.xaml` is missing the `AssemblyReferences` project capability
## PR Checklist
* [x] I work here
## Validation Steps Performed
Built the project with VS16.10 and VS17.0.
Correct Default Application Selector styles for high contrast and to change with OS theme dark/light toggle
## References
- https://docs.microsoft.com/windows/uwp/design/controls-and-patterns/xaml-theme-resources
## PR Checklist
* [x] Closes#10181
* [x] I work here
* [x] Manual tests passed
## Detailed Description of the Pull Request / Additional comments
1. If I'm going to override colors, I need to define styles in a resource dictionary with Light, Dark, and HighContrast variants so it can be appropriate for each of those.
2. For HighContrast, I need to not mess with text colors and let them follow the default settings.
3. For using System Brushes, I need to use a `ThemeResource` binding not a `StaticResource` binding. The former lets it change when you flip the OS toggle Light/Dark. The latter is stuck to whatever it was when the page loaded.
## Validation Steps Performed
- Loaded in light mode. Flipped to dark. Watched it change live. Checked both unselected and rollover/selected to ensure it was fine.
- Loaded in dark mode. Flipped to light. Watched it change live. Checked both unselected and rollover/selected to ensure it was fine.
- Flipped to HC. Watched it change live. Confirmed that unselected is black/white contrast and the roll over has the cyan/black. (No longer uses special second-line brush for HC, matches the controls I modeled this one on from OS Settings).
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Adds a new bellStyle called `window`. When `window` is set and a BEL is emitted, we flash the pane that emitted it.
Additionally, changes bellStyle in the SUI to a list of checkboxes instead of radio buttons, to match bellStyle being a flag-enum. Deprecates 'BellStyle::Visual' in the schema, but still allows it to be set in the json (it maps to `Window | Taskbar`)
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#6700
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [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
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
GIF in Teams
[Defapp] Use real HPCON for PTY management; Have Monarch always listen for connections
## PR Checklist
* [x] Closes#9464
* [x] Related to #9475 - incomplete fix
* [x] I work here.
* [x] Manual test
## Detailed Description of the Pull Request / Additional comments
- Sometimes peasants can't manage to accept a connection appropriately because I wrote defterm before @zadjii-msft's monarch/peasant architecture. The simple solution here is to just make the monarch always be listening for inbound connections. Then COM won't start a peasant with -Embedding just to ask the monarch where it should go. It'll just join the active window. I didn't close 9475 because it should follow monarch policies on which window to join... and it doesn't yet.
- A lot of interesting things are happening because this didn't have a real HPCON. So I passed through the remaining handles (and re-GUID-ed the interface) that made it possible for me to pack the right process handles and such into an HPCON on the inbound connection and monitor that like any other ConptyConnection. This should resolve some of the process exit behaviors and signal channel things like resizing.
Added trace to conhost to instrument buffers that are cooked prior to being passed to the console.
* [x] I've discussed this with core contributors already (internal)
VALIDATION
- Ensured trace is correctly logged in ETL (via TraceLog)
## Summary of the Pull Request
This is a redux of #8882.
From the original:
> This is really similar to what we're doing with the `CommandPalette`. We're adding a ~~Preview~~`KeyDown` handler to the SUI `MainPage`, that connects to `TerminalPage::_HandleKey`. That allows the SUI a chance to search the keymap to dispatch actions for keybindings, similar to how the command palette does it.
>
> This also means it's now possible for the SUI to invoke _all_ the actions available to the Terminal. This includes the ones like `IncreaseFontSize`, which require a _Terminal_ to actually do something. So we have to make sure all the calls to `_GetActiveControl` actually check that the result is non-null before using it.
>
> A bunch of the actions do nothing now from a SUI tab, others behave _weird_. Like "Rename tab" / "Open Tab Renamer" do nothing. "Duplicate Tab" again does nothing - we try making a new settings tab, which just focuses the settings tab again. "Copy text" definitely does nothing, same with paste.
I don't know why I thought this wouldn't work. I thought we'd have to do this in `PreviewKeyDown` or something, which led to [weirdness](https://github.com/microsoft/terminal/pull/8882#issuecomment-767088554). Turns out, we don't need it to be in `PreviewKeyDown`. It can just be in the SUI's `KeyDown`.
## References
* Original: #8882
* Workaround was in #8885
## PR Checklist
* [x] Closes#8767
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
The special case handler from #8885 is no longer needed
## Validation Steps Performed
* Switching tabs with Ctrl+Tab works
* Command palette works
* fullscreen, focus mode works
* close window works
* copy paste on Ctrl+C/V works, even when bound
* Select all text in textboxes works
* tab navigation through UI elements works
Each mouse-down event's time and position is now stored, and if we
process a left-mouse-down event at the same position as the previous one
and within the double click time we set the double click flag.
Also adds a case statement to `_UpdateSGRMouseButtonState` so that we
send hover events instead of ignoring them.
Note: The 'right-click menu in far manager shows up at the wrong
location' bug still exists with this, as it seems to use the cursor
position as told by user32.
Related to #376
## Validation Steps Performed
Double click in far manager works, hover in far manager works (hovering
over items in the right-click menu correctly highlights them)
Went for option 2 proposed here:
https://github.com/microsoft/terminal/issues/10140#issuecomment-845193132.
Disabling back space in the nested entry didn't felt as the nicest
solution. Instead now all state that keeps track of nested commands is
cleared when switching beteen modes.
## Validation Steps Performed
- Validated the specified issue is fixed by this change:. now after
entering a sub command and hitting backspace the palette no longer
shows the sub command item (here `< Select color scheme...` ).
- Validated that switching between all modes (command line, actions, tab
search & tab switch) still work as expected.
- Validated as well that all modes still work as expected.
Didn't add unit tests, but happy to try that out if this would be
required.
Closes#10140
## Summary of the Pull Request
Adds support for the `focusPane` action, and the `focus-pane` subcommand. These allow the user to focus a pane by it's ID.
* `focusPane` accepts an `id`, identifying the id of the pane to focus.
* `focus-pane`, `fp` requires the parameter `--target,-t` to ID the pane it's going to focus.
## PR Checklist
* [x] Closes#5803
* [x] Closes#5464
* [x] I work here
* [x] Tests added/passed
* [ ] Requires documentation to be updated - oh no
## Detailed Description of the Pull Request / Additional comments
The ID isn't _totally_ useful right now, since users can't see them. But they're there, and used in-order. This is just slightly more ergonomic for complicated commandlines than `mf up; mf left`
## Validation Steps Performed
Tested in command palette
Tested a variety of commandlines. `wtd -w 0 mf down ; sp` and `wtd -w 0 fp -t 1 ; sp` gave me special difficulty.
Now it just launches in focus mode, but you can _leave_ focus mode just fine.
I can't iterate on this more today - VS decided that it _needed_ an update ☹️
* [x] I work here
* [x] Is polish
* [x] @cinnamon-msft: We'll need to update the docs to reflect this.
see also: #8888, comments in that thread
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/8881
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
* Preserve profile GUID upon item Tag creation.
* Use this GUID rather than the current profile GUID to select an item
upon settings update.
So even if the profile was renamed and the GUID has changed,
the GUID in the tag remains unchanged and can be found
upon discarding.
## Summary of the Pull Request
Check for null before serializing the default terminal. Because the _currentDefaultTerminal is only initialized when the `Launch` page is navigated to, this _could_ be null if you navigate to another page, save, then save again.
## PR Checklist
* [x] Closes#10003
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
It crashed consistently before, it doesn't now.
The bug that caused #9962 resulted in folks getting profiles written to
their settings that didn't contain any identifying information (name or
guid), sometimes multiple times.
These profiles look (somewhat) like this:
```json
{ "colorScheme": "Campbell" },
{},
```
An empty profile serves no purpose -- it shows up in the list as being
named "Default", and it only launches CMD (unless the commandline is the
thing that the user successfully changed.)
We can heal the settings file by simply ignoring those profiles that
have *no identifying information* (a guid or a name that can be
converted into a guid).
Validation
----------
I created a number of profiles that fit this format and made sure that
they were ignored on load and destroyed on save.
## PR Checklist
* [x] Closes an annoyance we discovered after 9962.
## Summary of the Pull Request
This PR builds on the `ActionMap` PR (#6900) by leveraging `ActionMap` to serialize actions. From the top down, the process is now as follows:
- `CascadiaSettings`: remove the hack of copying whatever we had for actions before.
- `GlobalAppSettings`: serialize the `ActionMap` to `"actions": []`
- `ActionMap`: iterate over the internal `_ActionMap` (list of actions) and serialize each `Command`
- `Command`: **THIS IS WHERE THE MAGIC HAPPENS!** For _each_ key mapping, serialize an action. Only the first one needs to include the name and icon.
- `ActionAndArgs`: Find the relevant `IActionArgs` parser and serialize the `ActionAndArgs`.
- `ActionArgs`: **ANNOYING CHANGE** Serialize any args that are set. We _need_ each setting to be saved as a `std::optional`. As with inheritance, this allows us to distinguish an explicit setting to the default value (sometimes `null`) vs an implicit "give me the default value". This allows us to serialize only the relevant details of each action, rather than writing _all_ of the args.
## References
- #8100: Inheritance/Layering for lists
- This tracks layering and better serialization for color schemes _and_ actions. This PR resolves half of that issue. The next step is to apply the concepts used in this PR (and #9621) to solve the similar problem for color schemes.
- #6900: Actions page
## Validation Steps Performed
Tests added!
## Summary of the Pull Request
This replaces `ThrottledFunc` with two variants:
* `ThrottledFuncLeading` invokes the callback immediately and blocks further calls for the given duration
* `ThrottledFuncTrailing` blocks calls for the given duration and then invokes the callback
## References
* #9270 - `ThrottledFuncLeading` will allow the pane to flash immediately for a BEL, but block further BELs until the animation finished
## PR Checklist
* [x] I work here
* [ ] Tests added/passed
## Validation Steps Performed
* [x] Ensured scrolling still works
## Summary of the Pull Request
Introduces `til::rle`, a vector-like container which stores elements of
type T in a run length encoded format. This allows efficient compaction
of repeated elements within the vector.
## References
* #8000 - Supports buffer rewrite work. A re-use of `til::rle` will be
useful as a column counter as we pursue NxM storage and presentation.
* #3075 - The new iterators allow skipping forward by multiple units,
which wasn't possible under `TextBuffer-/OutputCellIterator`.
Additionally it also allows a bulk insertions.
* #8787 and #410 - High probability this should be `pmr`-ified
like `bitmap` for things like `chafa` and `cacafire`
which are changing the run length frequently.
## PR Checklist
* [x] Closes#8741
* [x] I work here.
* [x] Tests added.
* [x] Tests passed.
## Validation Steps Performed
* [x] Ran `cacafire` in `OpenConsole.exe` and it looked beautiful
* [x] Ran new suite of `RunLengthEncodingTests.cpp`
Co-authored-by: Michael Niksa <miniksa@microsoft.com>
## Summary of the Pull Request
Upgrade the Windows SDK to 19041 by setting `WindowsTargetPlatformMinVersion` to 17763 and `WindowsTargetPlatformVersion` to 19041.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
General usage of the Windows Terminal application appears fine.
## Summary of the Pull Request
This fixes a bug where if you had the `_quake` window open, and you tried to `globalSummon` it (not with the `quakeMode` action, but just with a plain-old `globalSummon` to activate the MRU window), we'd _create a new window_ instead of just summoning the `_quake` window.
## References
* regressed in #9956
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-60325142
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
It's basically a one-line fix, I just had to update the function signature for `_getMostRecentPeasantID` to allow us to use it differently for glomming vs summoning. When glomming, `ignoreQuakeWindow` should be true. When summoning, `ignoreQuakeWindow` should be false.
I'd personally chose to just bind `globalSummon` to <kbd>win+`</kbd>, but _I do as I'm told_.
* [x] I work here
* [x] @cinnamon-msft mentioned this
* [x] Docs
1. The TSFInputControl may get a layout event after it has been removed
from service (and no longer has a XAML tree)
* Two fixes:
* first, guard the layour updater from accessing detached xaml
objects
* second, shut down all pending throttled functions during close
(not destruction!¹)
2. The TermControlAutomationPeer may be destructed before its events
fire.
3. The TermControlAutomationPeer may receive a notification after it has
been detached from XAML (and therefore has no dispatcher).
¹ Close happens before the control is removed from the XAML tree;
destruction happens some time later. We must detach all UI-bound events
in Close so that they don't fire between when we detach and when we
destruct.
Fixes MSFT-32496693
Fixes MSFT-32496158
Fixes MSFT-32509759
Fixes MSFT-32871913
(cherry picked from commit 661fde5937)
This reverts commit a3a2a4102d.
#10046 causes a crash on save. MainPage::UpdateSettings() is unable to update the navigation view's selected item due to an "incorrect parameter". This is particularly strange to see because #10046 only modifies the navigation view's header, not the menu items themselves. Reverting this change fixes that crash (verified).
Reopens#9694
## Summary of the Pull Request
This PR lays the foundation for a new Actions page in the Settings UI as designed in #6900. The Actions page now leverages the `ActionMap` to display all of the key bindings and allow the user to modify the associated key chord or delete the key binding entirely.
## References
#9621 - ActionMap
#9926 - ActionMap serialization
#9428 - ActionMap Spec
#6900 - Actions page
#9427 - Actions page design doc
## Detailed Description of the Pull Request / Additional comments
### Settings Model Changes
- `Command::Copy()` now copies the `ActionAndArgs`
- `ActionMap::RebindKeys()` handles changing the key chord of a key binding. If a conflict occurs, the conflicting key chord is overwritten.
- `ActionMap::DeleteKeyBinding()` "deletes" a key binding by binding "unbound" to the given key chord.
- `ActionMap::KeyBindings()` presents another view (similar to `NameMap`) of the `ActionMap`. It specifically presents a map of key chords to commands. It is generated similar to how `NameMap` is generated.
### Editor Changes
- `Actions.xaml` is mainly split into two parts:
- `ListView` (as before) holds the list of key bindings. We _could_ explore the idea of an items repeater, but the `ListView` seems to provide some niceties with regards to navigating the list via the key board (though none are selectable).
- `DataTemplate` is used to represent each key binding inside the `ListView`. This is tricky because it is bound to a `KeyBindingViewModel` which must provide _all_ context necessary to modify the UI and the settings model. We cannot use names to target UI elements inside this template, so we must make the view model smart and force updates to the UI via changes in the view model.
- `KeyBindingViewModel` is a view model object that controls the UI and the settings model.
There are a number of TODOs in Actions.cpp will be long-term follow-ups and would be nice to have. This includes...
- a binary search by name on `Actions::KeyBindingList`
- presenting an error when the provided key chord is invalid.
## Demo
This is a hotfix to #10048. When the tab renamer is opened, we need to make sure to not immediately steal focus from it.
* [x] closes#10112
* [x] I work here
* [x] tested manually
## Summary of the Pull Request
This is a scoped implementation of "hide on minimize", only to the `_quake` window. When minimized, the `_quake` window won't appear in the taskbar. IT ALSO WON'T APPEAR IN THE TRAY, BECAUSE WE DON'T HAVE ONE YET.
I talked about this with @DHowett, and it seemed cool. Other windows will still minimize normally.
## References
* Original thread: #653
* Spec: #9274
* megathread: #8888
* minimize to tray: #5727
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-61246940
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated - probably yea, but something <sub>something <sub>something</sub></sub>
## Detailed Description of the Pull Request / Additional comments
After playing with it, it is in fact, cool.
ALSO `LOG_IF_WIN32_BOOL_FALSE` should DEFINITELY not be used with `ShowWindow`. [`ShowWindow`](https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-showwindow#return-value) returns false if the window was previously hidden, but doesn't `SetLastError`, so that macro will _throw_.
## Validation Steps Performed
```jsonc
{ "keys": "ctrl+`", "command": { "action": "quakeMode" } },
{ "keys": "ctrl+1", "command": { "action": "globalSummon", "name": "_quake" } },
// { "keys": "ctrl+1", "command": { "action": "globalSummon" } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "desktop": "toCurrent" } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "toggleVisibility": false } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "dropdownDuration": 2000 } },
{ "keys": "ctrl+2", "command": { "action": "globalSummon", "monitor": "any" } },
// { "keys": "ctrl+3", "command": { "action": "globalSummon", "desktop": "onCurrent" } },
{ "keys": "ctrl+3", "command": { "action": "globalSummon", "monitor": "toMouse" } },
// { "keys": "ctrl+4", "command": { "action": "globalSummon", "desktop": "any" } },
{ "keys": "ctrl+4", "command": { "action": "globalSummon", "monitor": "toMouse", "dropdownDuration": 500 } },
{ "keys": "ctrl+5", "command": { "action": "globalSummon", "dropdownDuration": 500 } },
```
#### ⚠️ this pr targets #9977
## Summary of the Pull Request
This adds support for part of the `monitor` property for `globalSummon`. It also goes a little off-spec:
```json
"monitor": "any"|"toCurrent"|"toMouse"
```
* `monitor`: This controls the monitor that the window will be summoned from/to
- `"any"`: Summon the MRU window, regardless of which monitor it's currently on.
- `"toCurrent"`/omitted: (_default_): Summon the MRU window **TO** the monitor with the current **foreground** window.
- [**NEW**] `"toMouse"`: Summon the MRU window **TO** the monitor where the **mouse** cursor is.
When I was playing with this, It felt like `toMouse` was always what I wanted, not `toCurrent`. We can always just comment that out if we think that's contentious - I'm aware I didn't originally spec that.
## References
* Original thread: #653
* Spec: #9274
* megathread: #8888
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-60325291
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated 😢
## Detailed Description of the Pull Request / Additional comments
I made `toMouse` the default because it felt better. fite-me.jpg
## Validation Steps Performed
my ever evolving blob:
```jsonc
{ "keys": "ctrl+`", "command": { "action": "quakeMode" } },
{ "keys": "ctrl+1", "command": { "action": "globalSummon" } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "desktop": "toCurrent" } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "toggleVisibility": false } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "dropdownDuration": 2000 } },
{ "keys": "ctrl+2", "command": { "action": "globalSummon", "monitor": "any" } },
// { "keys": "ctrl+3", "command": { "action": "globalSummon", "desktop": "onCurrent" } },
{ "keys": "ctrl+3", "command": { "action": "globalSummon", "monitor": "toMouse" } },
// { "keys": "ctrl+4", "command": { "action": "globalSummon", "desktop": "any" } },
{ "keys": "ctrl+4", "command": { "action": "globalSummon", "monitor": "toMouse", "dropdownDuration": 500 } },
{ "keys": "ctrl+5", "command": { "action": "globalSummon", "dropdownDuration": 500 } },
```
## Summary of the Pull Request
Adds the `dropdownDuration` property to `globalSummon`. This controls how fast the window appears on the screen when summoned from minimized. It similarly controls the speed for sliding out of view when the window is dismissed with `"toggleVisibility": true`.
`dropdownDuration` specifies the duration in **milliseconds**. This defaults to `0` for `globalSummon`, and defaults to `200` for `quakeMode`. 200 was picked because, according to [`AnimateWindow`](https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-animatewindow):
> Typically, an animation takes 200 milliseconds to play.
Do note that you won't be able to interact with the window during the animation! Input sent during the dropdown will arrive at the end of the animation, but input sent during the slide-up _won't_. Avoid setting this to large values!
The gifs are in Teams.
## References
* Original thread: #653
* Spec: #9274
* megathread: #8888
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-59030824
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I had the following previously in the doc comments, but it feels better in the PR body:
- This was chosen because it was easier to implement and generally nicer than:
* `AnimateWindow`, which would show the window borders for the duration of
the animation, and occasionally just plain not work. Additionally, for
`AnimateWindow` to work, the window much not be visible, so we'd need to
first restore the window, then hide it, then animate it. That would flash
the taskbar.
* `SetWindowRgn` on the root HWND, which caused the xaml content to shift to
the left, and caused a black bar to be drawn on the right of the window.
Presumably, `SetWindowRgn` and `DwmExtendFrameIntoClientArea` did not play
well with each other.
* `SetWindowPos(..., SWP_NOSENDCHANGING)`, which worked the absolute best for
longer animations, and is the closest to the actual implementation of
`AnimateWindow`. This would resize the ROOT window, without sending resizes
to the XAML island, allowing the content to _not_ reflow. but for a
duration of 200ms, would only ever display ~2 frames. That's basically
not even animation anymore, it's now just an "appear". Since that's how
long the default animation is, if felt silly to have it basically not
work by default.
- If a future reader would like to implement this better, **they should feel
free to**, and not mistake my notes here as expertise. These are research
notes into the dark and terrible land that is Win32 programming. I'm no expert.
## Validation Steps Performed
This is the blob of json I'm testing with these days:
```jsonc
{ "keys": "ctrl+`", "command": { "action": "quakeMode" } },
{ "keys": "ctrl+1", "command": { "action": "globalSummon" } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "desktop": "toCurrent" } },
// { "keys": "ctrl+2", "command": { "action": "globalSummon", "toggleVisibility": false } },
{ "keys": "ctrl+2", "command": { "action": "globalSummon", "dropdownDuration": 2000 } },
{ "keys": "ctrl+3", "command": { "action": "globalSummon", "desktop": "onCurrent" } },
{ "keys": "ctrl+4", "command": { "action": "globalSummon", "desktop": "any" } },
```
* <kbd>ctrl+\`</kbd> will summon the quake window with a _quick_ animation
* <kbd>ctrl+2</kbd> will summon the window with a s l o w animation
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Adds a global setting, `experimental.detectHyperlinks`, that controls whether we automatically detect links and make them clickable. Default is set to true.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#9981
* [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
* [x] Schema updated.
* [x] I work here
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
When `detectHyperlinks` is set to false, links do not underline on hover and are not clickable.
While a user is formulating their hex string for a `tabColor` arg
in the CommandPalette, we try to parse the string one char at
a time as it comes in. `ColorFromHexString` doesn't like anything
except a well formed hex string so it'll throw. We can probably eat
any error that comes out of this because we should only care to set
the TabColor once the string provided is a valid hex str.
Closes#10053
Set keyword flags on all events so those sharing a provider with
telemetry do not fire unless tracing is enabled
## PR Checklist
* [x] Closes#10093
* [x] I work here
* [x] Tests passed
* [x] Documentation added in `til.h` about how keywords work and at the
only other site of keywords we define in the Host project tracing
files.
## Detailed Description of the Pull Request / Additional comments
I initially thought that we would need to split providers here to
accomplish this... but @DHowett helped me realize that might be a lot of
additional metadata and bloat binary size. So with help from a friend
from fundamentals, I realized that we could use Keywords to
differentiate here. We can no longer define 0 keywords as that
represents an any/all scenario. Every `TraceLoggingWrite` event now
needs a keyword. When our events have a keyword, they're not included in
any trace. Additionally, when we have an explicit keyword to check that
is different from the ones used for the telemetry pipeline, we can
ensure that we only do "hard work" to generate debug trace data when an
"ALL" type listener like TraceView or Windows Performance Recorder with
our profiles is listening to these providers for ALL keyworded events.
## Validation Steps Performed
- [x] - Built with full release build config to confirm performance is
worse than dev builds BECAUSE of the telemetry event collector camping
our provider and triggering full trace event generation on shared
providers.
- [x] - Built with full release build config to enable statistics
collection and validated trace event collection is excluded and trace
event short-circuits work with this change.
- [x] - Checked that TraceView still sees both telemetry and tracing
events
- [x] - Checked that WPR with our .wprp profile sees both telemetry and
tracing events
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Upgrade check-spelling to [v0.0.18](https://github.com/check-spelling/check-spelling/releases/tag/v0.0.18)
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [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.
* [ ] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
I've replaced the `dictionary` directory with `allow` and `reject`. When terminal got check-spelling, I didn't have a way to do `allow`/`reject` (but they were added a while ago). With this release, the bot will complain about items that are in user managed files that wouldn't be valid, this is mostly `-`s in dictionary files, but it also includes numbers `0`..`9` and `_`. If a specific token needs to be accepted but not its sub-elements, the item should be added to `patterns.txt` instead (`D2DERR_SHADER_COMPILE_FAILED` is an example).
With this version, check-spelling defaults to only considering tokens with at least 3 letters. It's possible to tune it back to 2 (or even 1), but in testing, the 2 character tokens have ended up not being worthwhile. (This can be [adjusted](https://github.com/check-spelling/check-spelling/wiki/Configuration#shortest_word) if it turns out that people manage to misspell two character tokens often enough to justify checking them.)
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
I ran a number of passes of the spell checker in https://github.com/check-spelling/terminal/actions (note: I tend to delete this repository, so this link may be dead at some point, and action run logs expire).
Implement PGO in pipelines for AMD64 architecture; supply training test scenarios
## References
- #3075 - Relevant to speed interests there and other linked issues.
## PR Checklist
* [x] Closes#6963
* [x] I work here.
* [x] New UIA Tests added and passed. Manual build runs also tested.
## Detailed Description of the Pull Request / Additional comments
- Creates a new pipeline run for creating instrumented binaries for Profile Guided Optimization (PGO).
- Creates a new suite of UIA tests on the full Windows Terminal app to run PGO training scenarios on instrumented binaries (and incidentally can be used to write other UIA tests later for the full Terminal app.)
- Creates a new NuGet artifact to store trained PGO databases (PGD files) at `Microsoft.Internal.Windows.Terminal.PGODatabase`
- Creates a new NuGet artifact to supply large-scale test content for automated tests at `Microsoft.Internal.Windows.Terminal.TestContent`
- Adjusts the release pipeline to run binaries in PGO optimized mode where content from PGO databases is leveraged at link time to optimize the final release build
The following binaries are trained:
- OpenConsole.exe
- WindowsTerminal.exe
- TerminalApp.dll
- TerminalConnection.dll
- Microsoft.Terminal.Control.dll
- Microsoft.Terminal.Remoting.dll
- Microsoft.Terminal.Settings.Editor.dll
- Microsoft.Terminal.Settings.Model.dll
In the future, adding `<PgoTarget>true</PgoTarget>` to a new `vcxproj` file will automatically enroll the DLL/EXE for PGO instrumentation and optimization going forward.
Two training test scenarios are implemented:
- Smoke test the Terminal by just opening it and typing a bit of text then exiting. (Should help focus on the standard launch path.)
- Optimize bulk text output by launching terminal, outputting `big.txt`, then exiting.
Additional scenarios can be contributed to the `WindowsTerminal_UIATests` project with the `[TestProperty("IsPGO", "true")]` annotation to add them to the suite of scenarios for PGO.
**NOTE:** There are currently no weights applied to the various test scenarios. We will revisit that in the future when/if necessary.
## Validation Steps Performed
- [x] - Training run completed at https://dev.azure.com/ms/terminal/_build?definitionId=492&_a=summary
- [x] - Optimization run completed locally (by forcing `PGOBuildMode` to `Optimize` on my local machine, manually retrieving the databases with NuGet, and building).
- [x] - Validated locally that x86 and ARM64 do not get trained and automatically skip optimization as databases are not present for them.
- [x] - Smoke tested optimized binary versus latest releases. `big.txt` output through CMD is ~11-12seconds prior to PGO and just over 8 seconds with PGO.
## Summary of the Pull Request
Adds the profile icons to the page header. I had to manually create the header, and manually bind it to the `Icon` and `Content` of each `NavViewItem`.
It's important that each `NavViewItem`'s icon is set as an `IconSource`, so that we can bind to it. If it's just a plain old `FontIcon`, then we can't re-use it.
Additionally, I removed the manual sizing of all font icons to font size 12. That would make font icons _tiny_ in the header. Now, they'll properly re-use the size of the `NavigationViewTitleHeaderContentControlTextStyle` in the nav view header. This involved also manually making the icons smaller on the `AddProfile` page and in the `CommandPalette`.
As per usual, images are in Teams
## PR Checklist
* [x] Closes#9694
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* Checked (bitmap|font) icons in tabs
* (bitmap|font) icons in the flyout
* (bitmap|font) icons in command palette
* (bitmap|font) icons in the nav view
* (bitmap|font) icons in the header
* (bitmap|font) icons in the add profile page
This is primarily being done to unblock #9223.
Prior to this, we'd validate that the user's `startingDirectory` existed
here. If it was invalid, we'd gracefully fall back to `%USERPROFILE%`.
However, that could cause hangs when combined with WSL. When the WSL
filesystem is slow to respond, we'll end up waiting indefinitely for
their filesystem driver to respond. This can result in the whole terminal
becoming unresponsive.
Similarly, with #9223 we want users to be able to specify WSL paths in a
profile, but this bit of validation logic totally prevents that from working,
because it'll just replace the path with `%USERPROFILE%`.
If the path is eventually invalid, we'll display warning in the
`ConptyConnection`, when the process fails to launch.
Closes#9541Closes#9114
A redo of #6290. That PR was overkill. In that one, we'd toss focus back to the active control any time that the tab view item got focus. That's maybe not the _best_ solution.
Instead, this PR is precision strikes. We're re-using a lot of what we already have from #9260.
* When the context menu is closed, yeet focus to the control.
* When the renamer is dismissed, yeet focus to the control.
* When the TabViewItem is tapped (meaning no one else handled it), yeet focus to the control.
### checklist
* [x] I work here
* [ ] This is UI so it doesn't have tests
* [x] Closes#3609
* [x] Closes#5750
* [x] Closes#6680
### scenarios:
* [x] focus the window by clicking on the tab -> Control is focused.
* [x] Open the color picker with the context menu, can move the focus inside the picker with the arrow keys.
* [x] Dismiss the picker with esc -> Control is focused.
* [x] Dismiss the picker with enter -> Control is focused.
* [x] Dismiss the renamer with esc -> Control is focused.
* [x] Dismiss the renamer with enter -> Control is focused.
* [x] Dismiss the context menu with esc -> Control is focused.
* [x] Start renaming, then click on the tab -> Rename is committed, Control is focused.
* [x] Start renaming, then click on the text box -> focus is still in the text box
This test has some load bearing specifics in it.
In short, it must trigger multiple boost::small_vector growths. The
vector is reused for the lifetime of the console (!) so it must be
forced to act the way the repro requires.
Oh, and the test needs to be run with Application Verifier +Heap.
This validates the fix in c0ab9cb5b5.
## Summary of the Pull Request
ControlCore::AttachUiaEngine receives a IRenderEngine as a raw pointer,
which TermControl owns. We must ensure that we first destroy the
ControlCore before the UiaEngine instance (both owned by TermControl).
Otherwise a deallocated IRenderEngine is accessed when
ControlCore calls Renderer::TriggerTeardown.
## References
This crash was introduced in #10031.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Run accevent.exe to cause a UiaEngine to be attached to a TermControl.
* Close the current tab
* Ensured no crashes occur
Until there's a "Wait", there's usually only one API message inflight at
a time.
In our quest for performance, we put that single API message in charge
of its own buffer management: instead of allocating buffers on the heap
and deleting them later (storing pointers to them at the far corners of
the earth), it would instead allocate them from small internal pools (if
possible) and only heap allocate (transparently) if necessary. The
pointers flung to the corners of the earth would be pointers (1) back
into the API_MSG or (2) to a heap block owned by boost::small_vector.
It took us months to realize that those bare pointers were being held by
COOKED_READ and RAW_READ and not actually being updated when the API
message was _copied_ as it was shuffled off to the background to become
a "Wait" message.
It turns out that it's trivially possible to crash the console by
sending two API calls--one that waits and one that completes
immediately--when the waiting message or the "wait completer" has a
bunch of dangling pointers in it. It further turns out that some
accessibility software (like JAWS) attaches directly to the console
session, much like winpty and ConEmu and friends. They're trying to read
out the buffer (API call!) and sometimes there's a shell waiting for
input (API call!). Oops.
In this commit, we fix up the message's internal pointers (in lieu of
giving it a proper copy constructor; see GH-10076) and then tell the
wait completion routine (which is going to be a COOKED_READ, RAW_READ,
DirectRead or WriteData) about the new buffer location.
This is a scoped fix that should be replaced (TODO GH-10076) with a
final one after Ask mode.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev eca0875950fd3a9735662474613405e2dc06f485
References GH-10076
Fixes MSFT-33127449
Fixes GH-9692
The ES folks removed support for implicit binary dependencies in test projects, which required our testmd to change to include its dependency on testnetv5.
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev a2a74747feda018c347ba787e166e8385bb17e47
Related work items: MSFT-31480819
## PR Checklist
* [x] Closes#9920
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
## Validation Steps Performed
- When opening a profile page and selecting a random pivot, when then navigating away from the profile page to another item and navigating back to the same profile the last selected pivot was still active.
- When opening a profile page and selecting a random pivot, when then navigating away from the profile page to a second profile the last selected pivot was immediately active on the second profile.
- When selecting random pivots and navigating only between other profile pages the last selected pivot was always active on the opened profile page.
What felt a bit strange at first sight was:
- open a profile page, select a random pivot
- start adding a new profile
- the last selected pivot is immediately active and not the "General" pivot what might be the obvious pivot to show when a user wants to add a new profile.
After playing a bit more with this this started to feel ok. Since as a user you might go to "Add new profile" immediately anyway instead of opening other profiles first.
Just explicitly highlighting this last point since maybe someone prefers that we need to make sure to always start with the 'General' pivot when adding a new profile and then this fix is not correct.
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
When the client application sets the console input mode with the `ENABLE_MOUSE_INPUT` flag, we send along the appropriate VT sequences to the connected terminal (if there is one).
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#376#6859
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [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
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Far manager works
## Summary of the Pull Request
When we parse OSC 9;4, allow a trailing semicolon (i.e. allow `9;4;` or something like `9;4;3;`).
## PR Checklist
* [x] Closes#9960
* [X] Tests added/passed
## Validation Steps Performed
OSC 9;4 sequences with or without trailing semicolons work
This entirely removes `KeyMapping` from the settings model, and builds on the work done in #9543 to consolidate all actions (key bindings and commands) into a unified data structure (`ActionMap`).
## References
#9428 - Spec
#6900 - Actions page
Closes#7441
## Detailed Description of the Pull Request / Additional comments
The important thing here is to remember that we're shifting our philosophy of how to interact/represent actions. Prior to this, the actions arrays in the JSON would be deserialized twice: once for key bindings, and again for commands. By thinking of every entry in the relevant JSON as a `Command`, we can remove a lot of the context switching between working with a key binding vs a command palette item.
#9543 allows us to make that shift. Given the work in that PR, we can now deserialize all of the relevant information from each JSON action item. This allows us to simplify `ActionMap::FromJson` to simply iterate over each JSON action item, deserialize it, and add it to our `ActionMap`.
Internally, our `ActionMap` operates as discussed in #9428 by maintaining a `_KeyMap` that points to an action ID, and using that action ID to retrieve the `Command` from the `_ActionMap`. Adding actions to the `ActionMap` automatically accounts for name/key-chord collisions. A `NameMap` can be constructed when requested; this is for the Command Palette.
Querying the `ActionMap` is fairly straightforward. Helper functions were needed to be able to distinguish an explicit unbinding vs the command not being found in the current layer. Internally, we store explicitly unbound names/key-chords as `ShortcutAction::Invalid` commands. However, we return `nullptr` when a query points to an unbound command. This is done to hide this complexity away from any caller.
The command palette still needs special handling for nested and iterable commands. Thankfully, the expansion of iterable commands is performed on an `IMapView`, so we can just expose `NameMap` as a consolidation of `ActionMap`'s `NameMap` with its parents. The same can be said for exposing key chords in nested commands.
## Validation Steps Performed
All local tests pass.
- Whenever we add a new profile setting from now on we have to update
`Profile::CopySettings` _and_ `CascadiaSettings::DuplicateProfile` 👎
Notes from bug bash (checked bugs have been resolved):
- [ ] The duplicate list can be very long if you have profiles
- [x] DH: "Create new" seems too vague. "New empty profile" or something
seems a little clearer to me.
- [x] There is no deduplication counter for name
- [x] Crash when your settings file is corrupt and we had to fall back
to the defaults and you duplicate a profile
- [x] Crash due to #10003
## PR Checklist
* [x] Closes#9121
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/9941
* [x] CLA signed.
## Detailed Description of the Pull Request / Additional comments
The bug is due to us using std::tolower, while the default locale is not user's locale.
The fix here is to use the same approach as upon sorting: lstrcmpi.
While there are additional methods to do locale aware comparison,
here we convert chars to string and call lstrcmpi.
While this approach seems somewhat inefficient it ensures consistency
(with the order of locales that lstrcmi tries to apply internally).
## Summary of the Pull Request
This PR encompasses the first three bugs we found post-#9820.
### A: Mousedown, select, SCROLL does a weird thing with endpoints that doesn't happen in stable
We were using the terminal position to set the selection anchor, when we should have used the pixel position.
This is fixed in 4f4df01.
### B: Trackpad scrolling down with small increments seems buggy
This one's the most complicated. The touchpad sends very many small scroll deltas, less than one row at a time. The control scrollbar can store a `double`, so small deltas can accumulate. Originally, these would accumulate in the scrollbar, and we'd only read that out as an `int` in the scrollbar updater, which is throttled.
In the interactivity split, there's no place for us to store that double. We immediately narrow to an `int` for `ControlInteractivity::_updateScrollbar`.
So this introduces a double inside `ControlInteractivity` as a fake scrollbar, with which to accumulate to.
This is fixed in 33d29fa...0fefc5b
### C: Looks like there's a selection issue when you click and drag too quickly.
The diff for this one is:
<table>
<tr><td>1.8</td><td>main</td></tr>
<tr>
<td>
```c++
if (_singleClickTouchdownPos)
{
// Figure out if the user's moved a quarter of a cell's smaller axis away from the clickdown point
auto& touchdownPoint{ *_singleClickTouchdownPos };
auto distance{ std::sqrtf(std::powf(cursorPosition.X - touchdownPoint.X, 2) + std::powf(cursorPosition.Y - touchdownPoint.Y, 2)) };
const til::size fontSize{ _actualFont.GetSize() };
const auto fontSizeInDips = fontSize.scale(til::math::rounding, 1.0f / _renderEngine->GetScaling());
if (distance >= (std::min(fontSizeInDips.width(), fontSizeInDips.height()) / 4.f))
{
_terminal->SetSelectionAnchor(_GetTerminalPosition(touchdownPoint));
// stop tracking the touchdown point
_singleClickTouchdownPos = std::nullopt;
}
}
```
</td>
<td>
```c++
if (_singleClickTouchdownPos)
{
// Figure out if the user's moved a quarter of a cell's smaller axis away from the clickdown point
auto& touchdownPoint{ *_singleClickTouchdownPos };
float dx = ::base::saturated_cast<float>(pixelPosition.x() - touchdownPoint.x());
float dy = ::base::saturated_cast<float>(pixelPosition.y() - touchdownPoint.y());
auto distance{ std::sqrtf(std::powf(dx, 2) +
std::powf(dy, 2)) };
const auto fontSizeInDips{ _core->FontSizeInDips() };
if (distance >= (std::min(fontSizeInDips.width(), fontSizeInDips.height()) / 4.f))
{
_core->SetSelectionAnchor(terminalPosition);
// stop tracking the touchdown point
_singleClickTouchdownPos = std::nullopt;
}
}
```
</td>
</tr>
</table>
```c++
_terminal->SetSelectionAnchor(_GetTerminalPosition(touchdownPoint));
```
vs
```c++
_core->SetSelectionAnchor(terminalPosition);
```
We're now using the location of the drag event as the selection anchor, instead of the location that the user initially clicked. Oops.
## PR Checklist
* [x] Checks three boxes, though I'll be shocked if they're the last.
* [x] I work here
* [x] Tests added/passed 🎉🎉🎉
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
All three have tests, 🙌🙌🙌🙌
## Validation Steps Performed
Manual, and automated via tests
## Summary of the Pull Request
Let the dropdown menu open downwards if there's enough space, when clicking on the down arrow.
## PR Checklist
* [X] Closes#8924
* [X] CLA signed.
## Detailed Description of the Pull Request / Additional comments
Set the placement of the flyout to BottomEdgeAlignedLeft, as was done when opening the menu from the key binding.
## Validation Steps Performed
Manual tests
## Summary of the Pull Request
This is to mitigate MSFT:33035972. If you call `MoveWindowToDesktop` while an app is set to "Show windows from this app on all desktops", the OS will clear that "Show windows from this app on all desktops" state. But it _won't_ clear that state from the task view, so it'll just plain look broken.
We can mitigate this just by checking if we're already on the current desktop first. "Show windows from this app on all desktops" windows will _always_ be on every desktop, so that API will return true, and we can avoid tearing the state.
## References
* added in #9954
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-60325102
* [x] I work here
* [ ] Tests aren't possible
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* it works again
When syncing terminals across users (i.e. Liveshare shared terminals),
the terminal size is synced. This leads to having unused space around
the terminal which is the same color as the terminal's background
causing confusion as to what space is usable within the terminal.
Instead this change allows consumers to set the background color of the
control, separate from the terminal renderer's background, which makes
it easier to identify the edges of the terminal.
## Summary of the Pull Request
ControlCore's _renderer (IRenderTarget) is allocated as std::unique_ptr,
but is given to Terminal::CreateFromSettings as a reference.
ControlCore::Close deallocates the _renderer, but if ThrottledFuncs
are still scheduled to call ControlCore::UpdatePatternLocations
it'll cause Terminal::UpdatePatterns to be called, which in turn ends up
accessing the deallocated IRenderTarget reference and lead to a crash.
A proper solution with shared pointers is nontrivial and should be
attempted at a later point in time. This solution moves the teardown of
the _renderer into ControlCore::~ControlCore, where we can be certain
that no further strong references are held by ThrottledFuncs.
## PR Checklist
* [x] Closes#9910
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
The crash is a race condition and inherently hard to reproduce.
During validation this PR didn't appear to introduce new crashes.
This PR introduces a mechanism via which DCS data strings can be passed
through directly to the dispatch method that will be handling them, so
the data can be processed as it is received, rather than being buffered
in the state machine. This also simplifies the way string termination is
handled, so it now more closely matches the behaviour of the original
DEC terminals.
* Initial support for DCS sequences was introduced in PR #6328.
* Handling of DCS (and other) C1 controls was added in PR #7340.
* This is a prerequisite for Sixel (#448) and Soft Font (#9164) support.
The way this now works, a `DCS` sequence is dispatched as soon as the
final character of the `VTID` is received. Based on that ID, the
`OutputStateMachineEngine` should forward the call to the corresponding
dispatch method, and its the responsibility of that method to return an
appropriate handler function for the sequence.
From then on, the `StateMachine` will pass on all of the remaining bytes
in the data string to the handler function. When a data string is
terminated (with `CAN`, `SUB`, or `ESC`), the `StateMachine` will pass
on one final `ESC` character to let the handler know that the sequence
is finished. The handler can also end a sequence prematurely by
returning false, and then all remaining data bytes will be ignored.
Note that once a `DCS` sequence has been dispatched, it's not possible
to abort the data string. Both `CAN` and `SUB` are considered valid
forms of termination, and an `ESC` doesn't necessarily have to be
followed by a `\` for the string terminator. This is because the data
string is typically processed as it's received. For example, when
outputting a Sixel image, you wouldn't erase the parts that had already
been displayed if the data string is terminated early.
With this new way of handling the string termination, I was also able to
simplify some of the `StateMachine` processing, and get rid of a few
states that are no longer necessary. These changes don't apply to the
`OSC` sequences, though, since we're more likely to want to match the
XTerm behavior for those cases (which requires a valid `ST` control for
the sequence to be accepted).
## Validation Steps Performed
For the unit tests, I've had to make a few changes to some of the
`OutputEngineTests` to account for the updated `StateMachine`
processing. I've also added a new `StateMachineTest` to confirm that the
data strings are correctly passed through to the string handler under
all forms of termination.
To test whether the framework is actually usable, I've been working on
DRCS Soft Font support branched off of this PR, and haven't encountered
any problems. To test the throughput speed, I also hacked together a
basic Sixel parser, and that seemed to perform reasonably well.
Closes#7316
This adds a `toggleVisibility` parameter to `globalSummon`.
* When `true` (default): when you press the global summon keybinding, and the window is currently the foreground window, we'll minimize the window.
* When `false`, we'll just do nothing.
## References
* Original thread: #653
* Spec: #9274
* megathread: #8888
## PR Checklist
* [x] Checks a box in #8888
* [x] closes https://github.com/microsoft/terminal/projects/5#card-59030814
* [x] I work here
* [ ] No tests for this one.
* [ ] yes yes eventually I'll come back on the docs
## Detailed Description of the Pull Request / Additional comments
I've got nothing extra to add here. This one's pretty simple. I'm only targeting #9954 since that one laid so much foundation to build on, with the `SummonBehavior`
## Validation Steps Performed
Played with this for a while, and it's amazing.
This adds support for the `desktop` param to the `globalSummon` action. It accepts 3 values:
* `toCurrent` (default): The window moves to the current desktop when it's summoned
* `any`: We don't care what desktop the window is on. We'll go to the desktop the window is on when we summon it.
* `onCurrent`: We'll only try to summon the MRU window on this desktop when summoning a window.
* When combined with `name`, if there's a window matching `name`, we'll move it to this desktop.
* If there's not a window on this desktop, and `name` is omitted, then we'll make a new window.
`quakeMode` was also updated to use `toCurrent` behavior by default.
## References
* Original thread: #653
* Spec: #9274
* megathread: #8888
## PR Checklist
* [x] Checks some boxes in #8888
* [x] closes https://github.com/microsoft/terminal/projects/5#card-59030845
* [x] I work here
* [x] Tests added
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
S/O to https://github.com/microsoft/PowerToys, who graciously let us use `VirtualDesktopUtils` for figuring out what desktop is the current desktop. Yea, that's all we needed that entire file for. No, there isn't an API for this (_surprised-pikachu.png_)
## Validation Steps Performed
Played with this for a while, and it's amazing.
Adds support for two new actions:
* `globalSummon`, which can be used to activate a window using a _global_ (READ: OS-level) hotkey.
- accepts an optional `name` argument. When provided, this will attempt to summon with the given name. When omitted, we'll try to summon the most recent window.
* `quakeMode` which is `globalSummon` for the `_quake` window.
These actions are stored in the actions array, but are read by the `WindowsTerminal` level and bound to the OS in `IslandWindow`. The monarch registers for these keybindings with the OS. When one is pressed, the monarch will recieve a `WM_HOTKEY` message. It'll use that to look up the corresponding action args. It'll use those to try and summon the right window.
## References
* #8888: Quake mode megathread
* #9274: Spec (**guys seriously i just need one more ✔️**)
* #9785: The start of granting "\_quake" super powers
## PR Checklist
* [x] Closes#653 - I'm gonna say this closes it for now, though we have _many_ follow-ups in #8888
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* Validated that it works with `win` keys
* Validated that it works without `win` keys
* Validated that it hot-reloads
* Validated that it moves to the new monarch
* Validated that you can bind both `globalSummon` and `quakeMode` at the same time and do different things
* Validated that you can bind `globalSummon` with a name and it creates that name if it doesn't already exist
This PR aims to optimize the text analysis process by breaking the text
into simple & complex runs according to the result of
`GetTextComplexity`. For simple runs, we can skip certain processing
steps to improve the analysis performance.
Previous to this PR, we rely on the result of `AnalyzeBidi`,
`AnalyzeScript` and `AnalyzeNumberSubstitution` to both break the text
into different runs and attach the corresponding
bidi/script/number_substitution information to the run. Thanks to #6695
we have the chance to skip the expensive analysis process when we found
the *entire text* is determined to be simple.
Inspired by https://github.com/microsoft/cascadia-code/issues/411 and
discussions in #9156, I found that the "entire text simplicity" is often
hard to meet. In order to fully utilize the complexity information of
the text, we need to first break the text into simple & complex ranges.
These ranges are also the initial runs prior to the
bidi/script/number_substitution analysis. This way we can skip the text
analysis for simple runs to speed up the process.
VALIDATION
Build & run cmatrix, cacafire, cat big.txt with it.
Initial simple run PR: #6695Closes#9156
#9962 was caused by a serialization bug. _Technically_, `ToJson` works
as intended: if the current layer has any values set, write them out to
the json. However, on first load, the dynamic profile `Profile` objects
are actually empty (because they inherit from base layer, then the
dynamic profile generator). This means that `ToJson` writes the dynamic
profiles as empty objects `{}`. Then, on reload, we see that the dynamic
profiles aren't in the JSON, and we write them again.
To get around this issue, we added a simple check to `Profile::ToJson`:
if we have a source, make sure we write out the name, guid, hidden, and
source. This is intended to align with `Profile::GenerateStub`.
Closes#9962
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Fixes code health failure since 66b9b9d6f1
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #xxx
* [ ] 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.
* [ ] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Implement dropdown menu for choosing a default terminal application from inside the Windows Terminal Settings UI
## PR Checklist
* [x] Closes#9463
* [x] I work here.
* [x] Manual tests passed
* [x] https://github.com/MicrosoftDocs/terminal/issues/314 (and cross reference #9462)
## Detailed Description of the Pull Request / Additional comments
- Adds dropdown menu and a template card for displaying the available default applications (using the same lookup code as the console property sheet `console.dll`)
- Adds model to TSM for adapting the data for display and binding on XAML
- Lookup occurs on every page reload. Persistence only happens on Save Changes.
- Manifest changed for Terminal to add capability to opt-out of registry redirection so we can edit this setting
## Validation Steps Performed
- [x] Flipped the menu and pressed Save Changes and launched cmd from run box... it moved between the two.
- [x] Flipped system theme from light to dark and ensured secondary color looked good
- [x] Flipped the status with a different mechanism (conhost propsheet) and then reopened settings page and confirmed it loaded the updated status
## Summary of the Pull Request
I came across a few build system bug fixes, which served their purpose now that VS 16.9 has been released.
## PR Checklist
* [x] I work here
* [x] Project still compiles
## Summary of the Pull Request
We don't want it acting as the "most recent window" for windowing behavior.
The most recent window should always be some other window.
This is being made as an atomic commit because we're probably 50% sure on this
one. Maybe people do want new tabs to open up in the quake window! If they're
running from the commandline, that's easy. If they're running from the shell
context menu, that's **H**ard / impossible currently. $20 someone asks for
that if we ship this. That of course might just fall into "explorer context
menu settings" though.
## References
* Original thread: #653
* Spec: #9274
* megathread: #8888
## PR Checklist
* [x] Checks a box in #8888
* [x] closes https://github.com/microsoft/terminal/projects/5#card-59030791
* [x] I work here
* [x] Tests added
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I mean, this one's super straightforward, not sure what else there is to add.
## Validation Steps Performed
Played with this, it works exactly as you'd think.
When we encounter clipboard input that cannot be mapped to a keyboard
key, we try our best to map it to an event. If if is an alphanumeric
character or a wide glyph (per the old width algorithm), we will pass it
through as the UnicodeChar of a key event. If it's *not* wide and *not*
alphanumeric, we will synthesize a set of Alt+NumPad events.
Those events comprise a set containing...
1. Alt Down (modifiers = LAlt, UnicodeChar = 0)
2. Numpad 0 Down/Up (modifiers = LAlt, UnicodeChar = 0)
3. Numpad 1 Down/Up (modifiers = LAlt, UnicodeChar = 0)
4. Numpad 2 Down/Up (modifiers = LAlt, UnicodeChar = 0)
5. Alt Up (modifiers = 0, UnicodeChar = [THE CHARACTER])
Because of event group 5, application developers seem to have taken a
dependency on receiving Alt Up + Character and don't seek to recompose
the original character from its numpad representation.
In pull request GH-8035 (!5394370), we stripped the old width algorithm
out and replaced it with a lookup table (finally!). Unfortunately, that
broke clipboard input for Chinese text as it was no longer considered
"Wide" for the purposes of detecting whether we should use numpad
events.
This commit introduces a version of GetQuickCharWidth that fulfills the
exact contract CharToKeyEvents needs, and doesn't answer for a codepoint
more. We'll use it in Windows to fix MSFT-32901370.
The Terminal analogue of this bug, GH-9052, is fixed by *never emitting
numpad events again.* We think this is okay because it looks like nobody
was ever handling numpad events... and that we were only using them as a
way to communicate within conhost (which we're *also* not using) and any
public exposition of event 5 as a contract was unintended.
VALIDATION
----------
I took this new implementation (with an early return) and the old
implementation and compared whether they would yield a numpad event or a
key event over the entire supported codepoint space [0000..FFFF].
They matched.
Fixes MSFT-32901370
Fixes GH-9052
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 224751bbb061f5f59d794c2c9bdac5a9674ebde6
## Summary of the Pull Request
Brace yourselves, it's finally here. This PR does the dirty work of splitting the monolithic `TermControl` into three components. These components are:
* `ControlCore`: This encapsulates the `Terminal` instance, the `DxEngine` and `Renderer`, and the `Connection`. This is intended to everything that someone might need to stand up a terminal instance in a control, but without any regard for how the UX works.
* `ControlInteractivity`: This is a wrapper for the `ControlCore`, which holds the logic for things like double-click, right click copy/paste, selection, etc. This is intended to be a UI framework-independent abstraction. The methods this layer exposes can be called the same from both the WinUI TermControl and the WPF control.
* `TermControl`: This is the UWP control. It's got a Core and Interactivity inside it, which it uses for the actual logic of the terminal itself. TermControl's main responsibility is now
By splitting into smaller pieces, it will enable us to
* write unit tests for the `Core` and `Interactivity` bits, which we desparately need
* Combine `ControlCore` and `ControlInteractivity` in an out-of-proc core process in the future, to enable tab tearout.
However, we're not doing that work quite yet. There's still lots of work to be done to enable that, thought this is likely the biggest portion.
Ideally, this would just be methods moved wholesale from one file to another. Unfortunately, there are a bunch of cases where that didn't work as well as expected. Especially when trying to better enforce the boundary between the classes.
We've got a couple tests here that I've added. These are partially examples, and partially things I ran into while implementing this. A bunch of things from #7001 can go in now that we have this.
This PR is gonna be a huge pain to review - 38 files with 3,730 additions and 1,661 deletions is nothing to scoff at. It will also conflict 100% with anything that's targeting `TermControl`. I'm hoping we can review this over the course of the next week and just be done with it, and leave plenty of runway for 1.9 bugs in post.
## References
* In pursuit of #1256
* Proc Model: #5000
* https://github.com/microsoft/terminal/projects/5
## PR Checklist
* [x] Closes#6842
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50760249
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50760258
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
* I don't love the names `ControlCore` and `ControlInteractivity`. Open to other names.
* I added a `ICoreState` interface for "properties that come from the `ControlCore`, but consumers of the `TermControl` need to know". In the future, these will all need to be handled specially, because they might involve an RPC call to retrieve the info from the core (or cache it) in the window process.
* I've added more `EventArgs` to make more events proper `TypedEvent`s.
* I've changed how the TerminalApp layer requests updated TaskbarProgress state. It doesn't need to pump TermControl to raise a new event anymore.
* ~~Something that snuck into this branch in the very long history is the switch to `DCompositionCreateSurfaceHandle` for the `DxEngine`. @miniksa wrote this originally in 30b8335, I'm just finally committing it here. We'll need that in the future for the out-of-proc stuff.~~
* I reverted this in c113b65d9. We can revert _that_ commit when we want to come back to it.
* I've changed the acrylic handler a decent amount. But added tests!
* All the `ThrottledFunc` things are left in `TermControl`. Some might be able to move down into core/interactivity, but once we figure out how to use a different kind of Dispatcher (because a UI thread won't necessarily exist for those components).
* I've undoubtably messed up the merging of the locking around the appearance config stuff recently
## Validation Steps Performed
I've got a rolling list in https://github.com/microsoft/terminal/issues/6842#issuecomment-810990460 that I'm updating as I go.
## Summary of the Pull Request
This PR adds some special behavior to the window named "\_quake".
* When creating the quake window, it ignores "initialRows" and "initialCols" and opens on the top half of the monitor.
- It uses `initialPosition` to determine which monitor this is
* It cannot be moved
* It can only be vertically resized on the bottom border.
* It's always in focus mode.
- We should probably have an issue tracking "Allow showing tabs in focus mode"? Maybe?
- This one element is maybe the one I'm least attached to
When renaming a window to "\_quake", it adopts all those behaviors as well. It does not exit focus mode when leaving QM, nor does it resize back. That seemed unnecessary.
## References
* As spec'ed in #9274
* See also #8888
## PR Checklist
* [x] In the pursuit of #653
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated, but I'm not gonna do any of that till quake mode is totally done.
## Detailed Description of the Pull Request / Additional comments
Note that this doesn't do things like:
* dropdown
* global hotkey summon
* summon to the current monitor
* summon to the current desktop
I'm doing #653 _very_ piecemeal, to try and make the PRs less egregious.
## Validation Steps Performed
* validated that center on launch still works
* validated that QM works on different monitors based on `initialPosition`
* validated entering/exiting QM behaves as expected
## TODO!
* [ ] When snapping the quake window between desktops with <kbd>win+shift+arrow</kbd>, the window doesn't horizontally re-size to the new monitor dimensions. It should.
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/9706
* [x] CLA signed.
* [ ] Tests added/passed
* [x] Documentation updated here: https://github.com/MicrosoftDocs/terminal/pull/313
* [x] Schema updated.
* [ ] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
Added global flag named `trimBlockSelection` set to `false` by default.
The setting was added to Interactions menu of the SUI.
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/8374
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
The majority of the work was already done earlier.
The fix is only in _SetFocusedTab, that runs asynchronously
and thus might result in a race or even overflow.
All other changes are decorative.
## Validation Steps Performed
UT and manual tests
I added a `RenameSucceededText` property to the `TerminalPage` which returns the
formatted message `Successfully renamed window to "{WindowNameForDisplay()}"`
This _doesn't_ pop the dialog when you `wt -w foo` for the first time. Only
_subsequent_ renames.
## References
* Added in #9662
* Closes#9804
When we resize the text buffer, initialize the buffer with the
_default_¹ attributes, not the _current_ ones. If we use the current
attributes, then we can get into scenarios where something like `vim` is
running, and left the attributes set to something other than the
defaults, and when we resized the buffer, we'd fill it up with color, as
opposed to whatever the default would be.
This PR instead initializes the buffers with the default colors. It also
makes sure to set the active attributes of the newly created buffers
back to whatever the current attributes of the old buffer were.
[1]: For the Terminal, the default attributes are "default on default".
For conhost, the default attributes are whatever the result of
`Settings::GetDefaultAttributes` is, which could be any combo of the
legacy indices and the default color.
## PR Checklist
* [x] Closes#3848
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* ran tests
## Summary of the Pull Request
Allow schemes to be previewed as the user hovers over them in the Command Palette.
## References
* Branched off of #8392, which is why the commit history is so polluted. 330a8e8 : 544b2fd has the interesting commits
* #5400: cmdpal megathread
### Potential follow-ups
* changing the font size
* changing the font face
* changing the opacity of acrylic
## PR Checklist
* [x] Closes#6689, a last straggling FHL PR
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated - I don't think so
## Detailed Description of the Pull Request / Additional comments
This works by inserting a "preview" `TerminalSettings` into the settings hierarchy, before the `TermControl`'s runtime settings, and after the ones from the actual `CascadiaSettings`. This allows us to modify that preview settings object, then discard it when we're done with the preview.
This could also be used for other settings in the future - I built it to be extensible to other `ShortcutAction`s, though I haven't implemented those yet.
## Validation Steps Performed
* Select a colorscheme - it becomes the active one
* `colortool -x <scheme>` after selecting a scheme - colortool overrides the selected scheme
* Select a colorscheme after a `colortool -x <scheme>` after selecting a scheme - the scheme in the palette becomes the active one
* Pressing <kbd>esc</kbd> at any point to dismiss the command palette - scheme returns to the previous one
* reloading the settings - returns to the scheme in the settings
## Summary of the Pull Request
Remove an unnecessary check in `Profiles.cpp` that was preventing us from enabling the text box and browse button when the user unchecks 'use parent process directory'
## PR Checklist
* [x] Closes#9847
* [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
Played around with it and it works.
## Summary of the Pull Request
Add the "Close other tabs"/"Close tabs to the right" menu items straight to the tab context menu to work around #8238.
We can't add them into a dedicated sub-menu until the upstream crash is fixed.
## References
#8238
## PR Checklist
* [X] Closes#8238
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] 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.
## Detailed Description of the Pull Request / Additional comments
Moved the creation of the close menu items to a single function. Once the originating crash is fixed, the sub-menu can be restored by just replacing a few lines of code.
## Validation Steps Performed
There is a bug in the compiler that we trip over when we handle the
exception generated by Package::Current inside a coroutine. It appears
to destruct an invalid instance of winrt::factory_guard_count.
Learned from the compiler folks: "coroutine frame pointer wasn't being
stored ... properly".
Fixes#9821
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/9836
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
Not sure what is the reason for handling right button.
But delaying it to PointerReleased seems not to regress anything.
## Summary of the Pull Request
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. This PR attempts to fix that.
## PR Checklist
* [x] Closes#9754
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] 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: #9754
## Detailed Description of the Pull Request / Additional comments
When a call is made to `UpdateBottom`, we now clamp the value so it's at least as low as the current viewport bottom (i.e. if the viewport has moved down, we want the virtual bottom to move down too), but no lower than the bottom of the buffer (we don't want it to be out of range).
There is one special case where we do actually want the virtual bottom to move up - when the scrollback has been cleared with an `ED3` escape sequence. So in that case we needed a new `ConGetSet` API (`ResetBottom`) to reset the virtual bottom to the top of the buffer (essentially one less than the viewport height, since the virtual bottom points to the last line of the viewport).
## Validation Steps Performed
I had to reset the virtual bottom manually in some parts of the `ScreenBufferTests`, since some of the tests were relying on the virtual bottom being automatically reset when the viewport was reset, which is no longer the case.
I've also added a new test to verify that the virtual bottom doesn't move upwards if an update is triggered while the visible viewport is scrolled up. This essentially reproduces the test case from issue #9754, which I've also manually confirmed is fixed.
Add flag that will ensure we do not handoff to a registered default console. Also for a bonus, allow double-click launches or explicit command line launches of conhost.exe with a binary argument to open with the inbox one.
## PR Checklist
* [x] Closes#9791
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
* [x] Automated tests for parsing
* [x] Enable defapp, double click conhost.exe, opens as itself
* [x] Enable defapp, run conhost.exe runbox, opens as itself
* [x] Enable defapp, run conhost.exe powershell.exe runbox, opens as itself
* [x] Runbox cmd.exe/pwsh.exe trigger defapp handoff to Terminal
* [x] Shortcut to cmd.exe/pwsh.exe trigger defapp handoff to Terminal
* [x] Use CHOP tool to launch conhost with a server handle triggers handoff to Terminal
* [x] Use CHOP tool to launch conhost with a server handle and -ForceNoHandoff opens as itself
## Summary of the Pull Request
Does what it says on the can. People can now use `win` in a keybinding to
indicate that the chord needs <kbd>win</kbd>.
## References
* Done for #653
* See also #8888
## PR Checklist
* [x] Closes#3184
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
For the record, I hate this. But it's great for quake mode, so _meh_. There's
shockingly more win keys claimed then you think - many more than the shortcut
guide even shows.
* `win+b`: Focus the tray?
* `win+t`: Focus the taskbar
* `win+p`: Project...
* `win+c`: The powertoys color picker
* `win+v`: cloud clipboard
So the list of valid combos is vanishingly small. It's all about that <kbd>win+~</kbd>
## Validation Steps Performed
Bound
```json
{ "keys": [ "win+`" ], "command": "commandPalette" },
```
and yea, it works as expected
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/9714
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
Attempts to generate a name Profile X, where X is the index of the new profile (1-based).
As long as name is already taken, generates new name by incrementing X by 1
## Summary of the Pull Request
Clearly, I didn't run these tests on my last commit where I made the toasts lazy-load.
## References
* broken in in #9662
*
## PR Checklist
* [x] Closes#9769
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
For whatever reason, these tests are unhappy running back to back, but are just fine running isolated.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/9776
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
## Detailed Description of the Pull Request / Additional comments
Use `ThrottledFunc` in `TermControl` to limit bell emission callback to one per second.
Add names to threads to make debugging a slight bit easier.
## PR Checklist
* [x] Closes personal todo item.
* [x] I work here.
* [x] Tested manually.
## Detailed Description of the Pull Request / Additional comments
Thread descriptions show up as names in both the Visual Studio debugger, WinDBG debugger, and Windows Performance Analyzer. This makes it faster and easier to identify threads of interest in our processes.
## Validation Steps Performed
* [x] Checked threads were named in OpenConsole.exe running in classic conhost window mode under VS debug
* [x] Checked threads were named in OpenConsole.exe running in conpty mode under VS debug
* [x] Checked threads were named in WindowsTerminal.exe (for a few of the threads around connections)
* [x] Checked that we could also see it in WinDBG
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
This change cleans up the Fullscreen implementation for both conhost and Terminal, improving the restore position (where the window goes when exiting fullscreen).
Prior to this change the window wasn't guaranteed to restore somewhere on the window's current monitor when exiting fullscreen. With this change the window will restore always to its current monitor, at a reasonable location (and will 'double restore' (to fullscreen->maximize->restore) after monitor changes while fullscreen, which is the expected user behavior.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#9746
* [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.
* [ ] 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
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
A fullscreen window's monitor can change.
- Win+Shift+left/right migrates a window between monitors.
- User could open settings, display, and move the monitor or change its DPI.
- The monitor could be unplugged.
- The session could be remote and be disconnected.
A fullscreen window stores a 'restore position' when entering fullscreen, used to move the window back 'where it was'. BUT, its unexpected for the window to exit fullscreen and jump to another monitor. This means its previous position must be migrated from the old monitor's work area to the new monitor's work area.
If a window is maximized, it is sized to the work area. Like with fullscreen, a maximized window has a 'restore position', though unlike with fullscreen the restore position for maximized is stored by the system itself. Migration in cases where a maximized (or fullscreen) window's monitor changes is also taken care of by the system. To restore 'safely' to maximized (after changing window styles) a window must only `SetWindowPos(SWP_FRAMECHANGED)`. While technically a maximized window that becomes fullscreen 'is still maximized' (from Win32's perspective), its prudent to also `ShowWindow(SW_MAXIMIZED)` prior to `SWP_FRAMECHANGED` (to explicitly make the window maximized).
If not restoring to maximized, the restore position is adjusted by the new/ old work area. Additionally, the new/ old window DPI is used to adjust the size of the window by the DPI change (keeping the window's logical size the same).
- The work area origin is checked first (shifting window rect by the change in origin)
- The DPI is checked next, changing right/ bottom (size only)
- Each edge of the window is compared against the corresponding edge of the work area, nudging the window back on-screen if hanging offscreen. By shifting right before left, bottom before top, the top-left is guaranteed on-screen.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Tried it out. Seemed to work on my machine.
Jk, ran conhost/ terminal on mixed DPI system, max (or not), fullscreen, win+shift+left/ exit fullscreen/ maximize. Monitor unplug, etc.
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/issues/9787
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
## Validation Steps Performed
* [x] single click = no selection
* [x] single click and drag = selection starting from first point
* [x] single click in unfocused pane and drag = focus pane, selection starting from first point
* [x] double-click = selects a whole word
* [x] triple-click = selects a whole line
* [x] double-click and drag = selects a whole word, drag selects whole words
* [x] triple-click and drag = selects a whole line, drag selects whole lines
* [x] Shift single-click = defines start point
* [x] second Shift single-click = defines end point
* [x] Shift double-click = selects entire word
* [x] Shift triple-click = selects entire line
* [x] Shift double-click and drag = selects entire word, drag selects whole words
* [x] Mouse mode: Shift single-click = defines start point
* [x] Mouse mode: second Shift single-click = defines end point
* [x] Mouse mode: Shift double-click = selects entire word
* [x] Mouse mode: Shift triple-click = selects entire line
* [x] Mouse mode: Shift double-click and drag = selects entire word, drag selects whole words
* [x] With existing selection: single-click outside the selection and drag = establishes a new selection starting from the click point
* [x] Click-drag to set selection, shift-click-drag outside of selection = extend selection while dragging
This is required to maintain the modality of the dialogs, which we lost
when we moved from Pickers to IFileDialog. The HWND hosting Window API
we dreamed up is incompatible with IModalDialog, because IModalDialog
requires the HWND immediately upon `Show`. We're smuggling it in a
uint64, as is tradition.
zadjii-msft noticed this in #9760.
This PR removes the `GetConsoleCursorInfo` and `SetConsoleCursorInfo`
methods from the `ConGetSet` interface, and the `GetScrollingRegion`
method from the `SCREEN_INFORMATION` class. None of these methods are
used anymore.
PR #2764 removed the last usage of `GetScrollingRegion`.
The `Get/SetConsoleCursorInfo` methods don't seem to have ever been used
in the time that the terminal code has been open source, so whatever
purpose they originally served must have been replaced a long time ago.
The `GetScrollingRegion` method was originally called from the
`ScrollRegion` function, but that usage was removed in PR #2764 when the
margin handling was moved to a higher level.
I've checked that the code still compiles.
Closes#9771
"ctrl+numpad_plus" command now increases font size and
"ctrl+numpad_minus" command now decreases font size.
Before this only "ctrl+=" and "ctrl+-" controlled font size. Increase in
font size follows previous convention where zooms in arbitrarily large,
but decrease in font size is capped.
## Validation Steps Performed
I first ran "ctrl+=" and "ctrl+-" in my terminal to verify its behavior,
then compared that against "ctrl+numpad_plus" and "ctrl+"numpad_minus".
Both increased and decreased the font size by the same amount, and both
appeared to have a cap for how small they could get, but did not appear
to have a cap for how big they could get.
Closes#7518
