[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.
## 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
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
## 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
## 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.
## 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 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
#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
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.
