This commit upgrades C++/WinRT to 2.0.200316.3 and fixes a couple of the
transitive dependency issues we had in the process.
Because the latest version has better dependency resolution, we're able
to properly depend on Microsoft.UI.Xaml and the Toolkit in TerminalApp
and TerminalAppLib so we no longer need to manually include .dll and
.pri files.
Because of nebulous _other_ changes in dependency resolution,
WindowsTerminalUniversal isn't picking up transitive .winmd dependencies
from TerminalApp, and needs to include them as ProjectReferences
directly. This was already happening transitively, so now it's explicit.
I've also taken the time to upgrade GSL to v2.1.0, the last release
before they removed span::at and blew up our world.
## Summary of the Pull Request
Moves the ConPTY drawing mechanism (`VtRenderer`) to use the fine-grained `til::bitmap` individual-dirty-bit tracking mechanism instead of coarse-grained rectangle unions to improve drawing performance by dramatically reducing the total area redrawn.
## PR Checklist
* [x] Part of #778 and #1064
* [x] I work here
* [x] Tests added and updated.
* [x] I'm a core contributor
## Detailed Description of the Pull Request / Additional comments
- Converted `GetDirtyArea()` interface from `IRenderEngine` to use a vector of `til::rectangle` instead of the `SMALL_RECT` to banhammer inclusive rectangles.
- `VtEngine` now holds and operates on the `til::bitmap` for invalidation regions. All invalidation operation functions that used to be embedded inside `VtEngine` are deleted in favor of using the ones in `til::bitmap`.
- Updated `VtEngine` tracing to use new `til::bitmap` on trace and the new `to_string()` methods detailed below.
- Comparison operators for `til::bitmap` and complementary tests.
- Fixed an issue where the dirty rectangle shortcut in `til::bitmap` was set to 0,0,0,0 by default which means that `|=` on it with each `set()` operation was stretching the rectangle from 0,0. Now it's a `std::optional` so it has no value after just being cleared and will build from whatever the first invalidated rectangle is. Complementary tests added.
- Optional run caching for `til::bitmap` in the `runs()` method since both VT and DX renderers will likely want to generate the set of runs at the beginning of a frame and refer to them over and over through that frame. Saves the iteration and creation and caches inside `til::bitmap` where the chance of invalidation of the underlying data is known best. It is still possible to iterate manually with `begin()` and `end()` from the outside without caching, if desired. Complementary tests added.
- WEX templates added for `til::bitmap` and used in tests.
- `translate()` method for `til::bitmap` which will slide the dirty points in the direction specified by a `til::point` and optionally back-fill the uncovered area as dirty. Complementary tests added.
- Moves all string generation for `til` types `size`, `point`, `rectangle`, and `some` into a `to_string` method on each object such that it can be used in both ETW tracing scenarios AND in the TAEF templates uniformly. Adds a similar method for `bitmap`.
- Add tagging to `_bitmap_const_iterator` such that it appears as a valid **Input Iterator** to STL collections and can be used in a `std::vector` constructor as a range. Adds and cleans up operators on this iterator to match the theoretical requirements for an **Input Iterator**. Complementary tests added.
- Add loose operators to `til` which will allow some basic math operations (+, -, *, /) between `til::size` and `til::point` and vice versa. Complementary tests added. Complementary tests added.
- Adds operators to `til::rectangle` to allow scaling with basic math operations (+, -, *) versus `til::size` and translation with basic math operations (+, -) against `til::point`. Complementary tests added.
- In-place variants of some operations added to assorted `til` objects. Complementary tests added.
- Update VT tests to compare invalidation against the new map structure instead of raw rectangles where possible.
## Validation Steps Performed
- Wrote additional til Unit Tests for all additional operators and functions added to the project to support this operation
- Updated the existing VT renderer tests
- Ran perf check
## Summary of the Pull Request
Seriously just read the code on this one, it's so incredibly obvious what I did wrong
## References
Regressed with #4741
## PR Checklist
* [x] Closes#5029
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Summary of the Pull Request
Currently, if the Terminal attempts to parse a setting that _should_ be a `bool`
and the user provided a string, then we'll throw an exception while parsing the
settings, and display an error message that's pretty unrelated to the actual
problem.
The same goes for `bool`s as `int`s, `float`s as `int`s, etc.
This PR instead updates our settings parsing to ensure that we check the type of
a json value before actually trying to get its parsed value.
## References
## PR Checklist
* [x] Closes#4299
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I made a bunch of `JsonUtils` helpers for this in the same vein as the
`GetOptionalValue` ones.
Notably, any other value type can safely be treated as a string value.
## Validation Steps Performed
* added tests
* ran the Terminal and verified we can parse settings with the wrong types
These projects load Xaml components that have Uids, but those Uids just
weren't working because Xaml components are, by default, loaded in
"Application" scope. Application scope is great if the resource producer
is the EXE project.
Application scope means that resources are looked up at the resource
root, but DLLs with resources don't produce resources at the root. They
produce resources at a root named after the DLL.
Setting the Xaml component resource location to Nested makes sure the
Xaml resource loader loads resources from the right places.
## Summary of the Pull Request
This PR turns on TextWrapping on `TSFInputControl::TextBlock`. Once the TextBlock hits the end of the Terminal window, it will wrap downwards, but the TextBlock will have as much width as it had when composition started. Unfortunately, this means if composition starts right at the end of the Terminal with enough width for just one character, there will be a vertical line of characters down the right side of the Terminal 😅. It's definitely not ideal, and I imagine this won't be the last time we visit this issue, but for now users can see what they're typing.
## PR Checklist
* [x] Closes#3657
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Played around with IME towards the edge of the Terminal window.
There's an issue in the UAC consent dialog where it cannot read an
application's name if it's stored in a resource. When it fails, it deems
us an "Unknown Program" and that looks pretty silly.
Fixes#2289.
## Summary of the Pull Request
This is 100% on me. Even after mucking around in this function for the last 3
months, I missed that there was a single addition where we weren't doing a
clamped addition. This would lead to us creating a buffer with negative height,
and all sorts of badness.
Clamping this addition was enough to fix the bug.
## PR Checklist
* [x] Closes#2815
* [x] Closes#4972
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
* ran tests
* Created a profile with `"historySize" : 32728`, then filled the viewport with
text, then maximized, and saw that the viewport indeed did resize to the new
size of the window.
## Summary of the Pull Request
Block selections were always read and displayed as line selections in UIA. This fixes that.
## PR Checklist
* [x] Closes#4509
## Detailed Description of the Pull Request / Additional comments
1. Expose `IsBlockSelection()` via IUiaData
2. Update the constructor to be able to take in a block selection parameter
3. Make ScreenInfoUiaProviders pass step 1 output into step 2 constructor
4. Update all instances of `UiaTextRange::GetTextRects()` to include this new flag
## Validation Steps Performed
Manually tested.
Additional tests would be redundant as GetTextRects() is tested in the text buffer.
## Summary of the Pull Request
This notifies automation clients (i.e.: NVDA, narrator, etc...) of new output being rendered to the screen.
## References
Close#2447 - Signaling for new output and cursor
Close#3791 - fixed by signaling cursor changes
## Detailed Description of the Pull Request / Additional comments
- Added tracing for UiaRenderer. This makes it easier to debug issues with notifying an automation client.
- Fire TextChanged automation events when new content is output to the screen.
## Validation Steps Performed
Verified with NVDA [1]
## Narrator
Narrator works _better_, but is unable to detect new output consistently. There is no harm for narrator when this change goes in.
[1] https://github.com/microsoft/terminal/issues/2447#issuecomment-595879890
## Summary of the Pull Request
Alt-Numpad# input would be escaping each numkey before sending it through. This would result in some weird behavior, for example, in powershell, where the first alt-numpad# would start digit argument and once the user releases alt, a character is sent through and digit argument would repeat that character X times. To resolve this, we simply need to ignore KeyDowns where Alt is held and a Numpad# is pressed.
Once Alt is released, we'll receive a character through `TSFInputControl`, not `TermControl::CharacterHandler`. It seems that the `CoreTextEditContext` in `TSFInputControl` intercepts the character before it gets to `TermControl`. TSF will then send the received character through as normal.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#1401
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Testing various combinations of Alt-Numpad# consistently sends through only one instance of the expected symbols.
## Summary of the Pull Request
We (the royal "we") broke key unbinding in #4746. We didn't run the local tests after this, which actually would have caught this. The comment even suggests what we should have done here. We need to make sure that when we bail, it's because there's a parsing function that returned nothing. `null`, `"unbound"`, etc actually don't even have a parsing function at all, so they should just keep on keepin' on.
## References
Source of this regression: #4746
## PR Checklist
* [x] Closes#3729
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
This is a great example of why your unittests should run in CI always
## Validation Steps Performed
* **ran the tests**
* tested the following unbindings:
```json
{ "command": null, "keys": [ "ctrl+shift+t" ] },
{ "command": "unbound", "keys": [ "ctrl+shift+t" ] },
{ "command": "null", "keys": [ "ctrl+shift+t" ] },
```
and they each individually worked.
Add the option to set the cursor color as part of the color scheme.
This is very useful for light themes, where the cursor disappears unless its color
is set in the profile.
Related to issue #764, but doesn't fully resolve it.
## Validation
I tested this manually by creating a light color scheme, setting the cursor color
to black and setting the profile color scheme to the newly created color scheme.
I validated the cursor is black, then set the cursor color in the profile (to red)
and saw it trumps the cursor color from the color scheme.
When WSL.exe would hang for users, WslDistroGenerator would also hang
while waiting for its `wsl.exe --list` call to return. The timeout was
`INFINITE` in the `WaitForSingleObject` call, but we should slap a
timeout on it instead (here we choose 2 seconds). In addition, if it
times out, we should also just return profiles and let the Terminal
continue to start up without the WSL distro profiles loaded.
# Validation Steps Performed
Made a sleep 30 executable as the command instead, made sure it hit the
`WAIT_TIMEOUT` and continued to start up without loading my Ubuntu
profile.
Closes#3987
## Summary of the Pull Request
This _actually_ implements `\033c`
([RIS](https://vt100.net/docs/vt220-rm/chapter4.html)) for the Windows Terminal.
I thought I had done this in #4433, but that PR actually only passthrough'd
`\x1b[3J`. I didn't realize at the time that #2715 was mostly about hard reset,
not erase scrollback.
Not only should conpty pass through RIS, but the Terminal should also be
prepared to actually handle that sequence. So this PR adds that support as well.
## References
* #4433: original PR I thought fixed this.
## PR Checklist
* [x] Closes#2715 for real this time
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
Actually tested `printf \033c` in the Terminal this time
## Summary of the Pull Request
Currently, when the user resizes the Terminal, we'll snap the visible viewport back to the bottom of the buffer. This PR changes the visible viewport of the Terminal to instead remain in the same relative location it was before the resize.
## References
Made possible by our sponsors at #4741, and listeners like you.
## PR Checklist
* [x] Closes#3494
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
We already hated the `std::optional<short>&` thing I yeet'd into #4741 right at the end to replace a `short*`. So I was already going to change that to a `std::optional<std::reference_wrapper<short>>`, which is more idomatic. But then I was looking through the list of bugs and #3494 caught my eye. I realized it would be trivial to not only track the top of the `mutableViewport` during a resize, but we could use the same code path to track the _visible_ viewport's start as well.
So basically I'm re-using that bit of code in `Reflow` to calculate the visible viewport's position too.
## Validation Steps Performed
Gotta love just resizing things all day, errday
## Summary of the Pull Request
When the auto-hide taskbar setting is enabled, then we don't
always get another window message to trigger us to remove the drag bar.
So, make sure to update the size of the drag region here, so that it
_definitely_ goes away.
## References
## PR Checklist
* [x] Closes#4224
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
Manually tested it
Adds support for setting the cursor visibility in Terminal. Visibility
is a property entirely independent from whether the cursor is "on" or
not. The cursor blinker _should_ change the "IsOn" property. It was
actually changing the "Visible" property, which was incorrect. This PR
additionally corrects the naming of the method used by the cursor
blinker, and makes it do the right thing.
I added a pair of tests, one taken straight from conhost. In
copy-pasting that test, I took it a step further and implemented
`^[[?12h`, `^[[?12l`, which enables/disables cursor blinking, for the
`TerminalCore`. THIS DOES NOT ADD SUPPORT FOR DISABLING BLINKING IN THE
APP. Conpty doesn't emit the blinking on/off sequences quite yet, but
when it _does_, the Terminal will be ready.
## References
* I'd bet this conflicts with #2892
* This isn't a solution for #1379
* There shockingly isn't an issue for cursor blink state via conpty...?
## PR Checklist
* [x] Closes#3093
* [x] Closes#3499
* [x] Closes#4644
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
cmatrix is somewhat of a pathological case for our infrastructure: it
prints out a bunch of green and white characters and then updates them a
million times a second.
It also maintains a column of space between every green character. When
it prints this column, it prints it in "default" or "white". This ends
up making runs of text that look like this:
(def: G=green B=bright white W=white *=matrix char =space)
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
G W G W G W G W G W G W G W G W
As characters trickle in:
G*W G*W G*W G*W G*W G*W G*W B*W
G*W G*W G*W G*W G*W G*W G*W G W
G*W G*W G*W B*W G*W G*W G*W G W
G*W B*W G*W G W G*W G*W G*W G*W
G*W G W G*W G W G*W B*W G*W G*W
B*W G W G*W G W G*W G W B*W G*W
G W G W G*W G W G*W G W G W B*W
G W G W B*W G W G*W G W G W G W
Every one of those color transitions causes us to break up the run of
text and start rendering it again. This impacts GDI, Direct2D *and*
ConPTY. In the example above, there are 120 runs.
The problem is, printing a space doesn't **use** the foreground color!
This commit introduces an optimization. When we're about to break a text
cluster because its attributes changed, we make sure that it's not just
filled with spaces and doesn't differ in any visually-meaningful way
(like underline or strikethrough, considering global invert state).
This lets us optimize both the rendering _and_ the PTY output to look
like this:
G* * * * * * * B*G
G* * * * * * *
G* * * B*G * * *
G* B*G * * * * *
G* * * B*G * *
B*G * * B*G *
G * * B*G
G B*G *
Text will be printed at best line-by-line and at worst only when the
visible properties of the screen actually change. In the example
above, there are only 21 runs.
This speeds up cmatrix remarkably.
Refs #1064
## Summary of the Pull Request
Make TerminalControl synthesize mouse events and Terminal send them to
the TerminalInput's MouseInput module.
The implementation here takes significant inspiration from how we handle
KeyEvents.
## References
Closes#545 - VT Mouse Mode (Terminal)
References #376 - VT Mouse Mode (ConPty)
### TerminalControl
- `_TrySendMouseEvent` attempts to send a mouse event via TermInput.
Similar to `_TrySendKeyEvent`
- Use the above function to try and send the mouse event _before_
deciding to modify the selection
### TerminalApi
- Hookup (re)setting the various modes to handle VT Input
- Terminal is _always_ in VT Input mode (important for #4856)
### TerminalDispatch
- Hookup (re)setting the various modes to handle VT Input
### TerminalInput
- Convert the mouse input position from viewport position to buffer
position
- Then send it over to the MouseInput in TerminalInput to actually do it
(#4848)
## Validation Steps Performed
Tests should still pass.
This PR adds support for "Resize with Reflow" to the Terminal. In
conhost, `ResizeWithReflow` is the function that's responsible for
reflowing wrapped lines of text as the buffer gets resized. Now that
#4415 has merged, we can also implement this in the Terminal. Now, when
the Terminal is resized, it will reflow the lines of it's buffer in the
same way that conhost does. This means, the terminal will no longer chop
off the ends of lines as the buffer is too small to represent them.
As a happy side effect of this PR, it also fixed#3490. This was a bug
that plagued me during the investigation into this functionality. The
original #3490 PR, #4354, tried to fix this bug with some heavy conpty
changes. Turns out, that only made things worse, and far more
complicated. When I really got to thinking about it, I realized "conhost
can handle this right, why can't the Terminal?". Turns out, by adding
resize with reflow, I was also able to fix this at the same time.
Conhost does a little bit of math after reflowing to attempt to keep the
viewport in the same relative place after a reflow. By re-using that
logic in the Terminal, I was able to fix#3490.
I also included that big ole test from #3490, because everyone likes
adding 60 test cases in a PR.
## References
* #4200 - this scenario
* #405/#4415 - conpty emits wrapped lines, which was needed for this PR
* #4403 - delayed EOL wrapping via conpty, which was also needed for
this
* #4354 - we don't speak of this PR anymore
## PR Checklist
* [x] Closes#1465
* [x] Closes#3490
* [x] Closes#4771
* [x] Tests added/passed
## EDIT: Changes to this PR on 5 March 2020
I learned more since my original version of this PR. I wrote that in
January, and despite my notes that say it was totally working, it
_really_ wasn't.
Part of the hard problem, as mentioned in #3490, is that the Terminal
might request a resize to (W, H-1), and while conpty is preparing that
frame, or before the terminal has received that frame, the Terminal
resizes to (W, H-2). Now, there aren't enough lines in the terminal
buffer to catch all the lines that conpty is about to emit. When that
happens, lines get duplicated in the buffer. From a UX perspective, this
certainly looks a lot worse than a couple lost lines. It looks like
utter chaos.
So I've introduced a new mode to conpty to try and counteract this
behavior. This behavior I'm calling "quirky resize". The **TL;DR** of
quirky resize mode is that conpty won't emit the entire buffer on a
resize, and will trust that the terminal is prepared to reflow it's
buffer on it's own.
This will enable the quirky resize behavior for applications that are
prepared for it. The "quirky resize" is "don't `InvalidateAll` when the
terminal resizes". This is added as a quirk as to not regress other
terminal applications that aren't prepared for this behavior
(gnome-terminal, conhost in particular). For those kinds of terminals,
when the buffer is resized, it's just going to lose lines. That's what
currently happens for them.
When the quirk is enabled, conpty won't repaint the entire buffer. This
gets around the "duplicated lines" issue that requesting multiple
resizes in a row can cause. However, for these terminals that are
unprepared, the conpty cursor might end up in the wrong position after a
quirky resize.
The case in point is maximizing the terminal. For maximizing
(height->50) from a buffer that's 30 lines tall, with the cursor on
y=30, this is what happens:
* With the quirk disabled, conpty reprints the entire buffer. This is
60 lines that get printed. This ends up blowing away about 20 lines
of scrollback history, as the terminal app would have tried to keep
the text pinned to the bottom of the window. The term. app moved the
viewport up 20 lines, and then the 50 lines of conpty output (30
lines of text, and 20 blank lines at the bottom) overwrote the lines
from the scrollback. This is bad, but not immediately obvious, and
is **what currently happens**.
* With the quirk enabled, conpty doesn't emit any lines, but the
actual content of the window is still only in the top 30 lines.
However, the terminal app has still moved 20 lines down from the
scrollback back into the viewport. So the terminal's cursor is at
y=50 now, but conpty's is at 30. This means that the terminal and
conpty are out of sync, and there's not a good way of re-syncing
these. It's very possible (trivial in `powershell`) that the new
output will jump up to y=30 override the existing output in the
terminal buffer.
The Windows Terminal is already prepared for this quirky behavior, so it
doesn't keep the output at the bottom of the window. It shifts it's
viewport down to match what conpty things the buffer looks like.
What happens when we have passthrough mode and WT is like "I would like
quirky resize"? I guess things will just work fine, cause there won't be
a buffer behind the passthrough app that the terminal cares about. Sure,
in the passthrough case the Terminal could _not_ quirky resize, but the
quirky resize won't be wrong.
When ConPTY encounters a string we don't understand, immediately flush the frame.
## References
This PR supersedes #2665. This solution is much simpler than what was proposed in that PR.
As mentioned in #2665: "This might have some long-term consequences for #1173."
## PR Checklist
* [x] Closes#2011
* [x] Closes#4106
* [x] I work here
* [x] Tests added/passed
## Summary of the Pull Request
This fixes our calculation for the initial size of the window. WE weren't accounting for the height of the tabs, so the `initialRows` was consistently wrong.
## PR Checklist
* [x] Closes#2061
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
For the tabs below the titlebar case, there's 6px (unscaled) of space that I cannot account for. I seriously have no idea where it's coming from. When we end up creating the first `TermControl` after startup, there's an inexplicable `6*scale` difference between the height of the `tabContent` and the `SwapChainPanel`'s size.
## Validation Steps Performed
Checked all six of the following cases:
* 1.0 DPI scaling, Tabs in Titlebar
* 1.25 DPI scaling, Tabs in Titlebar
* 1.0 DPI scaling, Tabs NOT in Titlebar, always show tabs
* 1.0 DPI scaling, Tabs NOT in Titlebar, DON'T always show tabs
* 1.25 DPI scaling, Tabs NOT in Titlebar, always show tabs
* 1.25 DPI scaling, Tabs NOT in Titlebar, DON'T always show tabs
If UseAcrylic is disabled, CTRL+SHIFT+SCROLL would enable it, without
having to change the setting in profile.json manually.
1. Set "useAcrylic" to false for the any profile in profile.json
2. Open terminal window for that profile.
3. CTRL+SHIFT+MouseScroll
Acrylic background opacity should change according to mouse scroll
## PR Checklist
* [x] CLA signed.
* [x] Tested manually
* [x] Updated documentation
Closes#661
There's a platform limitation that causes us to crash when we rearrange
tabs. Xaml tries to send a drag visual (to wit: a screenshot) to the
drag hosting process, but that process is running at a different IL than
us.
For now, we're disabling elevated drag.
Fixes#3581
I noticed a crash in debug builds when a connected application terminates;
we get its exit code, then we destruct, and the u16state is used after it's
destructed by us parsing the process's last words.
We should have been doing this all along.
<!-- 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
I originally thought that setting `TSFInputControl::_editContext.InputPaneDisplayPolicy` to be Automatic would allow the InputPanel to show and hide automatically when `TSFInputControl` gains and loses focus. It doesn't seem to behave that way, so we'll show the InputPanel manually.
I'll show the panel during `PointerPressedHandler` and during `GotFocusHandler`. A user will have the on-screen keyboard pop up when getting focus, but if they close the keyboard, they can simply re-tap on the terminal to bring it back up.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#3639
* [x] CLA signed.
* [x] Tests added/passed
## Validation Steps Performed
Played on my surfaces book with the on screen keyboard by closing/tapping on the terminal and using the search box.
## Summary of the Pull Request
When we are maximized or fullscreened, check for the presence of the taskbar in auto-hide mode. If the Terminal finds the taskbar on any side of the monitor, adjust our window rect by just a little bit, so that the taskbar can still be revealed by the user mousing over that edge.
## References
## PR Checklist
* [x] Closes#1438
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Note to future code archeologists:
This doesn't seem to work for fullscreen on the primary display. However, testing a bunch of other apps with fullscreen modes and an auto-hiding taskbar has shown that _none_ of them reveal the taskbar from fullscreen mode. This includes Edge, Firefox, Chrome, Sublime Text, Powerpoint - none seemed to support this.
This does however work fine for maximized.
## Validation Steps Performed
I'm maximized and fullscreened the Terminal a lot in the last two days.
<!-- 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
Right clicking on a focused tab while Copy On Select is active currently copies any active selection. This is because `PointerReleasedHandler` doesn't check for the mouse button that was released.
During a mouse button release, only the left mouse button release should be doing anything.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#4740
* [x] CLA signed.
* [x] Tests added/passed
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
These are the scenarios I've tested. They're a combination of in focus/out of focus, Copy On Select on/off, left/right click pressed and their move and release variants.
From Out of Focus:
- Left Click = Focus
- Left Click Move = Focus + Selection
- Left Click Release
- CoS on = Copy
- CoS off = Nothing
- Shift Left Click = Focus
- Right Click
- Focus
- CoS on = Paste
- CoS off = Copy if Active Selection, Paste if not.
- Right Click Move = Nothing
- Right Click Release = Nothing
From In Focus
- Left Click = Selection if CoS off
- Left Click Move = Selection
- Left Click Release
- CoS on = Copy
- CoS off = Nothing
- Shift Left Click = Set Selection Anchor
- Right Click
- CoS on = Paste
- CoS off = Copy if Active Selection, Paste if not.
- Right Click Move = Nothing
- Right Click Release = Nothing
checkpointing this since it's so close. It works for everything but fullscreen on my primary, 125% display which has the taskbar on top, and autohides. Every other case works fine.
## Summary of the Pull Request
Add a `SizeChanged` handler to the titlebar content UI element. It's possible that this element's size will change after the dragbar's. When that happens, the drag bar won't send another `SizeChanged` event, because the dragbar's _size_ didn't change, only it's position.
## References
We originally duped this issue to #4166, but after #4829 fixed that issue, this one persisted. They're all related, and _look_ like dupes, but they weren't.
## PR Checklist
* [x] Closes#4288
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
## Validation Steps Performed
I had a solid 100% repro that doesn't repro anymore.
I've maximized, restored, resized, and generally played with the window a bunch.
<!-- 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
Emoji composition was only being shown one letter at a time. This is because of the way I expected `CoreTextTextUpdatingEventArgs.Range` to be provided to TSFInputControl during composition (for Chinese/Japanese IME). Emoji IME composition gives the `StartCaretPosition` as the same as `EndCaretPosition`, unlike how for Chinese/Japanese IME, `StartCaretPosition` is usually the start of the composition and `EndCaretPosition` is the latest character in the composition. The solution is to change the `_inputBuffer.substr()` call to simply grab all of the buffer starting from `_activeTextStart`. This way I can ensure that I grab all of the "active text", instead of trusting the given `args.Range` to tell me the active text.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#4828
* [x] CLA signed.
* [x] Tests added/passed
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Chinese, Japanese, Korean, Emoji composition performed. Emoji selection through pointer also performed.
## Summary of the Pull Request
`GetTextForClipboard` already exists in the TextBuffer. It makes sense to use that for UIA as well. This changes the behavior or `GetText()` such that it does not remove leading/trailing whitespace anymore. That is more of an expected behavior.
## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection anchors are an accurate representation of what is selected. We just need to move GetText to the TextBuffer. Then we can have those three issues just rely on code from the text buffer. This also means ConHost gets some of this stuff for free 😀
## Detailed Description of the Pull Request / Additional comments
- `TextBuffer::GetTextForClipboard()` --> `GetText()`
- `TextBuffer::GetText()` no longer requires GetForegroundColor/GetBackgroundColor. If either of these are not defined, we return a `TextAndColor` with only the `text` field populated.
- renamed a few parameters for copying text to the clipboard for clarity
- Updated `UiaTextRange::GetText()` to use `TextBuffer::GetText()`
## Validation Steps Performed
Manual tests for UIA using accessibility insights and Windows Terminal's copy action (w/ and w/out shift)
Added tests as well.
## Summary of the Pull Request
We're deref'ing a null `_terminal`. Don't do that. This is a _okay_ fix, mostly to stem the bleeding. @DHowett-MSFT's got a mind for a real fix to #4166, but this isn't it.
## PR Checklist
* [x] related to #4166
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Summary of the Pull Request
Pretty straightforward. When we get a `WM_DISPLAYCHANGE`, that means the
display's DPI changed. When that happens, resize the drag bar, so that
it'll reflect the new scaling.
Unblocks #4778Closes#4166
## Validation
Man I've changed the DPI of my displays so many times in the last 30
minutes. I dragged the window across a bunch of DPI boundaries too.
AutomationProperties of interest in this PR include...
- Name: the name of a UI element (generally used as the main identifier
for it)
- HelpText: an additional description for a more complex UI element
- AccessibilityView[1]
- Raw: hide from the UIA tree. Only navigate to this if you know what
you're doing
- Control: a control without any content in it. Basically, a point at
which the user can make a decision as to how to navigate the tree or
invoke an action.
- Content: a control that also has content to present to the user.
I set a few more AutomationProperties throughout Windows Terminal...
- MinMaxClose Control: hidden (we can/should rely on the true buttons
that we are hiding)
- SplitButton: Name and Help text (currently ignored due to #4804, but
having it in the resource file won't cause any problems)
- SearchBox: added a more specific name to the close button
- BackgroundImage: hide it
## References
A few additional work items have been created for tracking...
- SplitButton: https://github.com/microsoft/terminal/issues/4804
## PR Checklist
* [X] Closes#2099
* [X] Closes#2102
## Validation Steps Performed
Verified using Accessibility Insights and Inspect.exe
[1] https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-treeoverview
## Summary of the Pull Request
Adds warning messages for a pair of keybindings-related scenarios. This covers the following two bugs:
* #4239 - If the user has supplied more than one key chord in their `"keys"` array.
* #3522 - If a keybinding has a _required_ argument, then we'll display a message to the user
- currently, the only required parameter is the `direction` parameter for both `resizePane` and `moveFocus`
## References
When we get to #1334, we'll want to remove the `TooManyKeysForChord` warning.
## PR Checklist
* [x] Closes#4239
* [x] Closes#3522
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
Tested manually, added tests.
The Xaml input stack doesn't allow an application to suppress the "caret
browsing" dialog experience triggered when you press F7.
The official recommendation from the Xaml team is to catch F7 before we
hand it off.
This commit introduces a special F7 handler and an ad-hoc implementation of event bubbling.
Runtime classes implementing a custom IF7Listener interface are
considered during a modified focus parent walk to determine who can
handle F7 specifically.
If the recipient control handles F7, we suppress the message completely.
This event bubbler has some minor issues -- the search box will not be
able to receive F7 because its parent control implements the handler.
Since search is already mostly a text box, it doesn't _need_ special
caret browsing functionality as far as I can tell.
TermControl implements its OnF7Pressed handler by synthesizing a
keybindings event and an event to feed into Terminal Core directly.
It's not possible to create a synthetic KeyPressRoutedEvent; if it were,
I would have just popped one into the traditional input queue. :)
Fixes#638.
## Summary of the Pull Request
Korean IME was not working correctly due to way we were clearing the input buffer inside of `TSFInputControl`. We wanted to clear our input buffer and tell TSF to clear its input buffer as well when we receive a `CompositionCompleted` event. This works fine in some IME languages such as Chinese and Japanese. However, Korean IME composes characters differently in such a way where we can't tell TSF to clear their buffer during a `CompositionCompleted` event because it would clear the character that triggered the `CompositionCompleted` event in the first place.
The solution in this PR is to keep our `_inputBuffer` intact until the user presses <kbd>Enter</kbd> or <kbd>Esc</kbd>, in which case we clear our buffer and the TSF buffer. I've chosen these two keys because it seems to make sense to clear the buffer after text is sent to the terminal with <kbd>Enter</kbd>, and <kbd>Esc</kbd> usually means to cancel a current composition anyway.
This means we need to keep track of our last known "Composition Start Point", which is represented by `_activeTextStart`. Whenever we complete a composition, we'll send the portion of the input buffer between `_activeTextStart` and the end of the input buffer to the terminal. Then, we'll update `_activeTextStart` to be the end of the input buffer so that the next time we send text to the terminal, we'll only send the portion of our buffer that's "active".
## PR Checklist
* [x] Closes#4226
* [x] CLA signed
* [x] Tests added/passed
## Validation Steps Performed
Manual testing with Chinese, Japanese, and Korean IME.
* Azure: rewrite user input handler
This commit replaces the AzureConnection's input handler with one that
acts more like "getline()". Instead of the Read thread setting a state
and WriteInput filling in the right member variable, the reader blocks
on the user's input and receives it in an optional<string>.
This moves the input number parsing and error case handling closer to
the point where those inputs are used, as opposed to where they're
collected.
It also switches our input to be "line-based", which is a huge boon for
typing tenant numbers >9. This fixes#3233. A simple line editor
(supporting only backspace and CR) is included.
It also enables echo on user input, and prints it in a nice pretty green
color.
It also enables input queueing: if the user types anything before the
connection is established, it'll be sent once it is.
Fixes#3233.
* Azure: display the user's options and additional information in color
This commit colorizes parts of the AzCon's strings that include "user
options" -- things the user can type -- in yellow. This is to help with
accessibility.
The implementation here is based on a discussion with the team.
Alternative options for coloration were investigated, such as:
* Embedding escape sequences in the resource file.
This would have been confusing for translators.
The RESW file format doesn't support  escapes, so we would need
some magic post-processing.
* Embedding "markup" in the resource file (like #{93m}, ...)
This still would have been annoying for translators.
We settled on an implementation that takes resource names, colorizes
them, and string-formats them into other resources.
* Azure: follow the user's shell choice from the online portal
Fixes#2266.
* Azure: remove all credentials instead of just the first one
## Summary of the Pull Request
Match conhost behavior and clear selection on viewport/font resize.
## PR Checklist
* [X] Closes#1165
## Validation Steps Performed
Retried attached bug repro steps
PR #4548 inadvertantly broke mouse button input in the WPF control. This happened due to the extra layer of HWND indirection. The fix is to move the mouse button handling down into the native control where the window messages are now being sent.
- When performing chunk selection, the expansion now occurs at the time
of the selection, not the rendering of the selection
- `GetSelectionRects()` was moved to the `TextBuffer` and is now shared
between ConHost and Windows Terminal
- Some of the selection variables were renamed for clarity
- Selection COORDs are now in the Text Buffer coordinate space
- Fixes an issue with Shift+Click after performing a Multi-Click
Selection
## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection
anchors are an accurate representation of what is selected. We just need
to move `GetText` to the `TextBuffer`. Then we can have those three
issues just rely on code from the text buffer. This also means ConHost
gets some of this stuff for free 😀
### TextBuffer
- `GetTextRects` is the abstracted form of `GetSelectionRects`
- `_ExpandTextRow` is still needed to handle wide glyphs properly
### Terminal
- Rename...
- `_boxSelection` --> `_blockSelection` for consistency with ConHost
- `_selectionAnchor` --> `_selectionStart` for consistency with UIA
- `_endSelectionPosition` --> `_selectionEnd` for consistency with
UIA
- Selection anchors are in Text Buffer coordinates now
- Really rely on `SetSelectionEnd` to accomplish appropriate chunk
selection and shift+click actions
## Validation Steps Performed
- Shift+Click
- Multi-Click --> Shift+Click
- Chunk Selection at...
- top of buffer
- bottom of buffer
- random region in scrollback
Closes#4465Closes#4547
Defines the following automation properties for a Terminal Control:
- [**Orientation**](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.peers.automationpeer.getorientationcore):
- The orientation of the control
- None --> Vertical
- [**Name**](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.peers.automationpeer.getnamecore):
- The name as used by assistive technology and other Microsoft UI
Automation clients. Generally presented by automation clients as the
primary way to identify an element (along with the control type)
- "" --> <profile name>
- [**HelpText**](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.peers.automationpeer.gethelptextcore):
- The help text. Generally presented by automation clients if
requested by the user. This would be something that you would normally
expect to appear from tooltips.
- "" --> <tab title>
- [**LiveSetting**](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.peers.automationpeer.getlivesettingcore):
- reports the live setting notification behavior. A representation of
how assertive this control should be when content changes.
- none --> Polite
## Detailed Description of the Pull Request / Additional comments
ProfileName had to be added to the TerminalSettings (IControlSettings)
to pass that information along to the automation peer. In the rare event
that somebody purposefully decided to make their ProfileName empty, we
fallback to the tab title.
## Validation Steps Performed
Verified using Accessibility Insights and inspect.exe
This is are some examples of the information a general user can expect
to receive about a Terminal Control.
- Type: Terminal Control
- Name: Command Prompt
- Help Text (if requested): Command Prompt - ping bing.com
- Type: Terminal Control
- Name: Ubuntu
- Help Text (if requested): cazamor@PC-cazamor:/mnt/c/Users/cazamor$
Note, it is generally read by an automation client as follows:
"<type>, <name>"
References #2099 - Automation Properties for TerminalControl, Search Box
References #2142 - Localization
Closes#2142
## Summary of the Pull Request
`keys` in `keybindings` now accepts a string value. This assumes that you wanted a keychord of size 1. The schema and user docs were properly updated too.
This means that the following keybinding is now accepted in your profiles.json:
```json
{ "command": "copy", "keys": "ctrl+c" }
```
as opposed to...
```json
{ "command": "copy", "keys": [ "ctrl+c" ] }
```
## PR Checklist
* [X] Closes#4713
* [X] CLA signed.
* [X] Tests added/passed
* [X] Requires documentation to be updated
## Validation Steps Performed
- [X] tested the new schema
- [X] added test
## Summary of the Pull Request
Changes how conpty emits text to preserve line-wrap state, and additionally adds rudimentary support to the Windows Terminal for wrapped lines.
## References
* Does _not_ fix (!) #3088, but that might be lower down in conhost. This makes wt behave like conhost, so at least there's that
* Still needs a proper deferred EOL wrap implementation in #780, which is left as a todo
* #4200 is the mega bucket with all this work
* MSFT:16485846 was the first attempt at this task, which caused the regression MSFT:18123777 so we backed it out.
* #4403 - I made sure this worked with that PR before I even sent #4403
## PR Checklist
* [x] Closes#405
* [x] Closes#3367
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
I started with the following implementation:
When conpty is about to write the last column, note that we wrapped this line here. If the next character the vt renderer is told to paint get is supposed to be at the start of the following line, then we know that the previous line had wrapped, so we _won't_ emit the usual `\r\n` here, and we'll just continue emitting text.
However, this isn't _exactly_ right - if someone fills the row _exactly_ with text, the information that's available to the vt renderer isn't enough to know for sure if this line broke or not. It is possible for the client to write a full line of text, with a `\n` at the end, to manually break the line. So, I had to also add the `lineWrapped` param to the `IRenderEngine` interface, which is about half the files in this changelist.
## Validation Steps Performed
* Ran tests
* Checked how the Windows Terminal behaves with these changes
* Made sure that conhost/inception and gnome-terminal both act as you'd expect with wrapped lines from conpty
This commit removes all of the custom UI initialization code in
TermControl and replaces it with a xaml file. Some dead or reundant code
was removed as part of this refactoring.
It also fixes two (quasi-related) issues:
* The search box, on first launch, was offset by the scrollbar even if
the scrollbar was invisible.
* The scrollbar state wasn't hot-reloadable.
The new UIA code in PublicTerminalCore broke windows 7 support by referring
to API set dlls for UIA. This PR changes the link line to link to
UIAutomationcore.dll directly.
## Summary of the Pull Request
Currently, when the user attempts to type using IME while the Search Box is focused, the input goes to the terminal instead. This is due to the fact that the `TSFInputControl` assumes it's in control whenever TermControl gets focus. So, it'll intercept IME input before the Search Box receives it. We simply need to modify `TermControl::GotFocus` to check if the SearchBox has focus. If it does, `TSFInputControl::NotifyFocusEnter` shouldn't be called.
As a small side fix, I've also disabled the terminal cursor blinking when the Search Box has focus.
Thinking a little further, if we have more features in the future that behave like search box (i.e. advanced tab switcher, or any other XAML controls that pop up) and require text input, we might need to create a sort of "AnyOtherTextControlInFocus" function to see if TSFInputControl should receive focus or not.
## PR Checklist
* [x] Closes#4434
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Search works as expected with IME. Composition picker appears underneath Search Box when typing IME in the Search Box. Clicking outside of the Search Box still returns control to TSFInputControl/TermControl.
Terminal Cursor blinks when it has focus, and doesn't when the Search Box has focus.
## Summary of the Pull Request
Currently, while in IME mode, selections with the Emoji/Kaomoji/Symbol Picker (which is brought up with <kbd>win+.</kbd>) are not displayed until the user starts a new composition. This is due to the fact that we hide the TextBlock when we receive a CompositionCompleted event, and we only show the TextBlock when we receive a CompositionStarted event. Input from the picker does not count as a composition, so we were never showing the text box, even if the symbols were thrown into the inputBuffer. In addition, we weren't receiving CompositionStarted events when we expected to.
We should be showing the TextBlock when we receive _any_ text, so we should make the TextBlock visible inside of `TextUpdatingHandler`. Furthermore, some really helpful discussion in #3745 around wrapping the `NotifyTextChanged` call with a `NotifyFocusLeave` and a `NotifyFocusEnter` allowed the control to much more consistently determine when a CompositionStarted and a CompositionEnded.
I've also went around and replaced casts with saturating casts, and have removed the line that sets the `textBlock.Width()` so that it would automatically set its width. This resolves the issue where while composing a sentence, the textBlock would be too small to contain all the text, so it would be cut off, but the composition is still valid and still able to continue.
## PR Checklist
* [x] Closes#4148
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Tested picking emojis, kaomojis, and symbols with numerous different languages.
This commit introduces two fixes for C5205 (delete of an abtract class
without a virtual dtor) and one fix for a very hopeful VS version gating
that didn't pan out.
This PR hooks up the existing UIA implementation to the WPF control. Some existing code that was specific to the UWP terminal control could be shared so that has been refactored to a common location as well.
## Validation Steps Performed
WPF control was brought up in UISpy and the UIA tree was verified. NVDA was then used to check that screen readers were operating properly.
This commit introduces a small console-subsystem application whose sole
job is to consume TerminalConnection.dll and hook it up to something
other than Terminal. It is 99% of the way to a generic solution.
I've introduced a stopgap in TerminalPage that makes sure we launch
TerminalAzBridge using ConptyConnection instead of AzureConnection.
As a bonus, this commit includes a class whose sole job it is to make
reading VT input off a console handle not terrible. It returns you a
string and dispatches window size change callbacks.
Fixes#2267.
Fixes#4589.
Related to #2266 (since pwsh needs better VT).
This unifies the rest of the projects around the resource structure laid out in WindowsTerminalUniversal. Now we'll have a single flat structure for resource files and keep the qualifiers in their filenames. It's easier to manage this way.
## Summary of the Pull Request
We used to return multiple text ranges to represent one selection. We only support one selection at a time, so we should only return one range.
Additionally, I moved all TriggerSelection() calls to the renderer from Terminal to TermControl for consistency. This ensures we only call it _once_ when we make a change to our selection state.
## References
#2447 - helps polish Signaling for Selection
#4465 - This is more apparent as the problem holding back Signaling for Selection
## PR Checklist
* [x] Closes#4452
Tested using Accessibility Insights.
## Summary of the Pull Request
Currently, clicking on an unfocused terminal with a selection active will trigger `copyOnSelect`. This is because the check for `copyOnSelect` and copying to the clipboard is bound to when the Pointer is released. This works fine for when a user performs a click-drag selection, but it inadvertently also triggers when the user performs a single click on an unfocused terminal. We expect `copyOnSelect` to trigger only on the first time a selection is completed.
This PR will allow the user to single click on an unfocused terminal that has a selection active without triggering a copyOnSelect. It also ensures that any click-drag selection, whether it's on an unfocused or focused terminal, will trigger copyOnSelect.
## PR Checklist
* [x] Closes#4255
## Validation Steps Performed
Performed manual testing involving permutations of multiple panes, tabs, in focus, and out of focus.
Fixed inconsistent scrolling when using both touchscreen and precision
touchpad.
Scrolling jumpiness is caused by rounding errors. Instead of retrieving
the current scrolling value from `GetScrollOffset`, which is already
rounded in `int`, let the scrolling operation to operate on
`_scrollBar`'s `Value` directly (which uses `double`) in
`TermControl.cpp`.
TermControl now also respects WHEEL_DELTA, which it was previously
ignoring, to determine how many scroll wheel detents were used.
Testing scrolling on the following scenario manually:
- nonscrollable terminal (e.g. the window is large enough to contain the
current buffer).
- scrolling to the topmost and bottom-most.
- scrolling TUI apps such as `nano` and `more` in WSL.
- after clearing the terminal, both in cmd and WSL.
... has the same behavior between using touchscreen or precision trackpad
and regular mouse wheel.
Closes#4554 (original pull request)
Closes#1066Closes#4542
## Summary of the Pull Request
The issue seems to be how `SwapChainScaleChanged` gets fired and attempts to tell the renderer
to `UpdateDPI` when the renderer is gone. So, as a quick bandaid, we'll put a quick check to only do the thing if the renderer is alive.
## PR Checklist
* [x] Closes#4539
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Held my new tab button for about thirty seconds then held the close tab button until all tabs closed without a crash.
## Summary of the Pull Request
This PR tries to address some of the weird interactions with pointer pressed events when the Terminal isn't in focus. Here's the four things that have changed as part of this PR;
1. This PR will allow the user to be able to make a selection with a click-drag without having to first perform a single click on a tab/pane to bring it to focus.
2. Another weird bug that's fixed in this PR is where trying to make a selection on an unfocused tab when it already has a selection active will simply extend the existing selection instead of making a new one.
3. Not related to the issue that his PR closes: a right click will now focus the tab/pane.
I've made sure that we still have the existing functionality where a single click on an unfocused tab/pane does not make a single-cell selection and just focuses the tab/pane.
## PR Checklist
* [x] Closes#4282
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Played around with all sorts of selection when in-focus and out of focus with multiple panes and tabs.
Unit tests still pass as well.
This will attempt to match the style of the user's JSON.
Caveats:
1. If the user has no profiles, it'll explode. This isn't new.
2. If the user's indentation style if `{profile}, {profile}, {profile}` (that is: no indentation), you'll get this:
```
{profile}, {profile}, {profile},
{
new profile content
}
```
There may be something better we can do by copying their newline (or lack thereof) and using it in our generator or detecting the indentation of their members as well.
That's an exercise for later.
Ref #2805
## Summary of the Pull Request
This will collect some user choices related to profiles and tab settings to help us understand if and how we should change the in-built defaults.
## PR Checklist
* [x] Closes#3855
* [x] I work here.
* [x] Manual test only.
* [x] Meh, no doc update.
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
The following data is collected with examples of the types of questions we intend to answer:
1. What is the name of the executable attached to the PTY? (What shells are popular? Should we focus our testing on them? Are there any common ones we are blind to that we should know about?)
- "Microsoft.Windows.Terminal.Connection" {e912fe7b-eeb6-52a5-c628-abe388e5f792}
- "ConPtyConnected" event
- "SessionGuid" value = WT_SESSION
- "Client" value = Name of EXE
2. Is Acrylic used on a tab? And with what opacity? (Do people really want acrylic? Should it be default? What opacity is most pleasing in our context?)
- "Microsoft.Windows.Terminal.App" {24a1622f-7da7-5c77-3303-d850bd1ab2ed}
- "TabInformation" event
- "EventVer" value is now 1u
- "UseAcrylic" value is now TRUE/FALSE on the setting choice
- "TintOpacity" value is now Float on the setting choice
3. What font are people choosing? (Do people move away from Cascadia Code? Which ones are the most popular for us to validate when updating the renderer?)
- "Microsoft.Windows.Terminal.App" {24a1622f-7da7-5c77-3303-d850bd1ab2ed}
- "TabInformation" event
- "FontFace" value is now string font from settings
4. What keybindings do people choose to customize (Add or Remove)? (Are there extremely common keys that folks bind or unbind that we should have adjusted by default in a fresh install?)
- "Microsoft.Windows.Terminal.App" {24a1622f-7da7-5c77-3303-d850bd1ab2ed}
- "CustomKeybindings" event
- "Keybindings" value is the entire JSON segment that describes the user keybindings from `settings.json`.
5. Do people change their default profile from the PowerShell one we set? If so, to what? (Should we not set PowerShell as the default? Should we adjust the ranking of our dynamic generators to favor the most popular ones to bubble to the top?)
- "Microsoft.Windows.Terminal.App" {24a1622f-7da7-5c77-3303-d850bd1ab2ed}
- "CustomDefaultProfile" event
- "DefaultProfile" value is the GUID of the chosen profile
## Validation Steps Performed
1. Implemented the events
2. Launched the ETL channel viewer
3. Triggered the events
4. Saw the data come out
The Terminal would crash when closing it when there are multiple tabs
open. This was due to `TerminalPage` attempting to select a nonexistent
tab.
The block of code that was removed was causing issues when trying to
close all tabs at once. The way we close all our tabs in
`_CloseAllTabs()` was by repeatedly calling
`_RemoveTabViewItemByIndex(0)` until `_tabs.Size() == 0`. The problem
was that `_RemoveTabViewItemByIndex` would eventually call a coroutine
to set the next tab as the `SelectedItem` after removing a tab. The
coroutine would then pass control back to `_CloseAllTabs()` to finish
its loop, and by the time the coroutine resumes control, `_tabs` and
`TabView().TabItems()` would both be empty and it would crash attempting
to focus a tab.
Luckily, the functionality that this block of code provided is really no
longer needed . This code was used to focus on the next tab after
closing a tab. This might have been written way back when TabView
didn't have this functionality built in. It seems now that after
removing a `TabItem` from the `TabView`, the `SelectedItem` of the
TabView automatically updates, making this block of code unnecessary.
## Validation Steps Performed
Did a lot of multiple tab open and closings and closing the window after
opening a ton of tabs. No crashes seem to occur anymore!
Test cases still pass.
Closes#4482
The UIA Provider now scrolls the viewport when necessary. This just fills in the missing virtual function in Terminal to have the same behavior as what it does in ConHost.
* [X] Closes#2361
* [X] CLA signed.
`ChangeViewport` is now a virtual function at the `ScreenInfoUiaProvider` layer to have access to the TermControl.
In ConHost, we pass this call up to the WindowUiaProvider layer. In Terminal, we don't need to do that because the concept of updating the viewport is handled at the TermControl layer. So we just call that function and _voila_!
## Summary of the Pull Request
Forgot to include the scaling factor. Also went ahead and used chromium math for this portion.
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#2551
* [x] CLA signed.
## Validation Steps Performed
Tested on 200% display and 100% display. Rects are aligned on both.
## Summary of the Pull Request
This is a fix that technically was caused by #357, though we didn't have the Terminal at the time, so I only fixed conhost then. When a client app prints the very last column in the buffer, the cursor is often not _actually_ moved to the next row quite yet. The cursor usually just "floats" on the last character of the row, until something happens. This could be a printable character, which will print it on the next line, or a newline, which will move the cursor to the next line manually, or it could be a backspace, which might take the cursor back a character.
Conhost and gnome-terminal behave slightly differently here, and wt behaves differently all together. Heck, conhost behaves differently depending on what output mode you're in.
The scenario in question is typing a full row of text, then hitting backspace to erase the last char of the row.
What we were emitting before in this case was definitely wrong - we'd emit a space at that last row, but then not increment our internal tracker of where the cursor is, so the cursor in conpty and the terminal would be misaligned. The easy fix for this is to make sure to always update the `_lastText` member appropriately. This is the `RightExclusive` change.
The second part of this change is to not be so tricksy immediately following a "delayed eol wrap". When we have just printed the last char like that, always use the VT sequence CUP the next time the cursor moves. Depending on the terminal emulator and it's flags, performing a BS in this state might not bring the cursor to the correct position.
## References
#405, #780, #357
## PR Checklist
* [x] Closes#1245
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
With the impending #405 PR I have, this still works, but the sequences that are emitted change, so I didn't write a test for this currently.
## Validation Steps Performed
Tried the scenario for both #357 and #1245 in inception, `gnome-temrinal` and `wt` all, and they all display the cursor correctly.
<!-- 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 a bug where scrolling up/down doesn't update the viewport after the window is resized and in other cases. Also changes other things, please read the detailed description.
<!-- 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#1494
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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
There are two ways scroll can happen:
- the user scrolls using the scroll bar and the `Terminal` is notified
- the `Terminal` changed the viewport and the scroll bar is updated to reflect the change
The code to notify the `Terminal` that the user scrolled is in the event handler for when the scroll bar's value changes. However this poses a problem because it means that when the `Terminal` changes the viewport, the scroll bar is updated so it would then also notify the `Terminal` that the scroll changed. But it already knows because it's coming from itself!
To fix this, the `TermControl` class had a member called `_lastScrollOffset` that would be set when the `Terminal` decides to change the viewport so that the event handler for the scroll bar could check the new scroll value against `_lastScrollOffset` and if it matches, then everything is fine and there is nothing to update.
This is what happens when the `Terminal` changes the viewport:
1. set `_lastScrollOffset`
2. dispatch job on the UI thread: update the scrollbar which is going to call the event handler which is going to check for `_lastScrollOffset` and clear it
There are two bugs introduced by this approach:
1. (I am not sure about this.) The dispatcher appears to store jobs in a LIFO stack so it sometimes reorders the "update the scrollbar" jobs when there are too many. When I run `1..10000` on PowerShell, then I get this from the event handler (format: `_lastScrollOffset newValue`):
```
8988 8988
8989 8989
8990 8990
8992 8991
8993 8992
...
9001 8997
9001 8998
9001 8999
9001 9000
9001 9001
9001 8985
9001 8968
9001 8953
...
9001 7242
9001 7226
9001 7210
```
This causes the following issues:
1. `_lastScrollOffset` wouldn't be reset because it wouldn't be equal to the current scroll bar value (see example above) so the next scrolls wouldn't do anything as the event handler would still be waiting for an event with the good scroll bar value which would never happen because it happened earlier
2. the `TermControl` would notify the `Terminal` about its own scroll
2. If the `Terminal` didn't actually changed its viewport but still called the `TermControl::_TerminalScrollPositionChanged` method, then it would set the `_lastScrollOffset` member as usual but the scroll bar value change event handler would not be called because it is only called when the value actually changes so the `_lastScrollOffset` member wouldn't be cleared and subsequent scroll bar value change events would be ignored because again the event handler would still be waiting for an event with the good scroll bar value which would never happen. This is actually the reason for #1494: when the window is resized, the `Terminal` will call `TermControl::_TerminalScrollPositionChanged` even if the scroll position didn't actually change (444de5b166/src/cascadia/TerminalCore/Terminal.cpp (L183)). Maybe this should also be fixed in another PR?
I replaced `_lastScrollOffset` by a flag `_isTerminalInitiatedScroll`. I set the flag just before and unset it just after the terminal changes the scrollbar on the UI thread to eliminate the race conditions and the bug when the scroll bar's value doesn't actually change.
Other changes:
- I also fixed a potential bug where if the user scrolls just after the terminal updates the viewport, it would en up ignoring the user scroll. To do this, when the user scrolls, I cancel any update with `_willUpdateScrollBarToMatchViewport`.
- I also removed the original `ScrollViewport` method because it was not used anywhere and I think it can potentially create confusion (and therefore bugs) because this method updates the viewport but not the scroll bar unlike `KeyboardScrollViewport` which functions as you would expect. I then renamed `KeyboardScrollViewport` into `ScrollViewport`. So, now, there is only one method to scroll the viewport from the `TermControl`. Please, tell me if this shouldn't be in this PR.
- I also removed `_terminal->UserScrollViewport(viewTop);` in the `KeyboardScrollViewport` method because it will be updated later anyways in the scroll bar's value change event handler because of the `_scrollBar.Value(viewTop);`.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
I tested manually by doing this:
- For bug 1:
1. Start the terminal
2. Run the `1..30000` command in PowerShell and wait for it to end (maybe more if you have a fast computer?)
3. Hold left click on the scrollbar slider and start moving it
- For bug 2:
1. Start the terminal
2. Run the `1..100` command in PowerShell and wait for it to end
3. Resize the window horizontally
4. Hold left click on the scrollbar slider and start moving it
Without this patch, the viewport doesn't update.
With the patch, the viewport updates correctly.
Generated by https://github.com/jsoref/spelling `f`; to maintain your repo, please consider `fchurn`
I generally try to ignore upstream bits. I've accidentally included some items from the `deps/` directory. I expect someone will give me a list of items to drop, I'm happy to drop whole files/directories, or to split the PR into multiple items (E.g. comments/locals/public).
Closes#4294
In debug builds that haven't been LTO'd or had unused refs removed,
there will still be a spurious reference to api-ms-win-winrt-core (or
something similar.)
In release builds, that reference is gone.
Fixes#4519.
## Summary of the Pull Request
Conpty doesn't need `CSI 3 J`, it doesn't have a scrollback. The terminal that's connected should use that. This makes conpty pass it through, like other sequences that conpty has no need for.
## References
## PR Checklist
* [x] Closes#2715
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
This fixes a crash caused by Narrator starting *before* terminal.
Fixes#2907.
For context,
```
// We must initialize the main thread as a single-threaded apartment before
// constructing any Xaml objects. Failing to do so will cause some issues
// in accessibility somewhere down the line when a UIAutomation object will
// be queried on the wrong thread at the wrong time.
// We used to initialize as STA only _after_ initializing the application
// host, which loaded the settings. The settings needed to be loaded in MTA
// because we were using the Windows.Storage APIs. Since we're no longer
// doing that, we can safely init as STA before any WinRT dispatches.
```
## Summary of the Pull Request
This PR will make the existing `Tab` class into a WinRT type. This will allow any XAML to simply bind to the `ObservableVector` of Tabs.
This PR will be followed up with a future PR to change our TabView to use the ObservableVector, which will in turn eliminate the need for maintaining two vectors of Tabs. (We currently maintain `_tabs` in `TerminalPage` and we also maintain `TabView().TabViewItems()` at the same time as described here: #2740)
## References
#3922
## PR Checklist
* [x] CLA signed.
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
I've currently only exposed a Tab's Title and IconPath to keep things simple. I foresee XAML elements that bind to Tabs to only really need these two properties for displaying.
I've also converted `TerminalPage`'s `std::vector<std::shared_ptr> _tabs` into a `IObservableVector<winrt::TerminalPage::Tab> _tabs` just so that future PRs will have the ground set for binding to this vector of tabs.
## Validation Steps Performed
Played around with Tabs and Panes and all sorts of combinations of keybindings for interacting with tabs and dragging and whatnot, it all seemed fine! Tab Tests also all pass.
Moved `FindText` to `UiaTextRangeBase`. Now that Search is a shared component (thanks #3279), I can just reuse it basically as-is.
#3279 - Make Search a shared component
#4018 - UiaTextRange Refactor
I removed it from the two different kinds of UiaTextRange and put it in the base class.
I needed a very minor change to ensure we convert from an inclusive end (from Search) to an exclusive end (in UTR).
Worked with `FindText` was globally messed with in windows.h. So we had to do a few weird things there (thanks Michael).
No need for additional tests because it _literally_ just sets up a Searcher and calls it.
## Summary of the Pull Request
This pull request is intended to achieve the following goals...
1) reduce duplicate code
2) remove static functions
3) improve readability
4) improve reliability
5) improve code-coverage for testing
6) establish functioning text buffer navigation in Narrator and NVDA
This also required a change to the wrapper class `XamlUiaTextRange` that has been causing issues with Narrator and NVDA.
See below for additional context.
## References
#3976 - I believe this might have been a result of improperly handling degenerate ranges. Fixed here.
#3895 - reduced the duplicate code. No need to separate into different files
#2160 - same as #3976 above
#1993 - I think just about everything is no longer static
## PR Checklist
* [x] Closes#3895, Closes#1993, Closes#3976, Closes#2160
* [x] CLA signed
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
### UiaTextRange
- converted endpoints into the COORD system in the TextBuffer coordinate space
- `start` is inclusive, `end` is exclusive. A degenerate range is when start == end.
- all functions are no longer static
- `MoveByUnit()` functions now rely on `MoveEndpointByUnit()` functions
- removed unnecessary typedefs like `Endpoint`, `ScreenInfoRow`, etc..
- relied more heavily on existing functionality from `TextBuffer` and `Viewport`
### XamlUiaTextRange
- `GetAttributeValue()` must return a special HRESULT that signifies that the requested attribute is not supported. This was the cause of a number of inconsistencies between Narrator and NVDA.
- `FindText()` should return `nullptr` if nothing was found. #4373 properly fixes this functionality now that Search is a shared module
### TextBuffer
- Word navigation functionality is entirely in `TextBuffer` for proper abstraction
- a total of 6 functions are now dedicated to word navigation to get a good understanding of the differences between a "word" in Accessibility and a "word" in selection
As an example, consider a buffer with this text in it:
" word other "
In selection, a "word" is defined as the range between two delimiters, so the words in the example include [" ", "word", " ", "other", " "].
In accessibility , a "word" includes the delimiters after a range of readable characters, so the words in the example include ["word ", "other "].
Additionally, accessibility word navigation must be able to detect if it is on the first or last word. This resulted in a slight variant of word navigation functions that return a boolean instead of a COORD.
Ideally, these functions can be consolidated, but that is too risky for a PR of this size as it can have an effect on selection.
### Viewport
- the concept of `EndExclusive` is added. This is used by UiaTextRange's `end` anchor as it is exclusive. To signify that the last character in the buffer is included in this buffer, `end` must be one past the end of the buffer. This is `EndExclusive`
- Since many functions check if the given `COORD` is in bounds, a flag must be set to allow `EndExclusive` as a valid `COORD` that is in bounds.
### Testing
- word navigation testing relies more heavily on TextBuffer tests
- additional testing was created for non-movement focused functions of UiaTextRange
- The results have been compared to Microsoft Word and some have been verified by UiAutomation/Narrator contacts as expected results.
## Validation Steps Performed
Tests pass
Narrator works
NVDA works
This pull request teaches the PowerShell Core generator about a bunch of different locations in which it might find a PowerShell.
These instances will be sorted, a leader will be elected, and that leader will be promoted and given the vaunted title of "PowerShell".
Names will be generated for the rest.
The sort order is documented in the comments, but that comment will be replicated here:
```
// <-- Less Valued .................................... More Valued -->
// | All instances of PS 6 | All PS7 |
// | Preview | Stable | ~~~ |
// | Non-Native | Native | Non-Native | Native | ~~~ |
// | Trd | Pack | Trd | Pack | Trd | Pack | Trd | Pack | ~~~ |
// (where Pack is a stand-in for store, scoop, dotnet, though they have their own orders,
// and Trd is a stand-in for "Traditional" (Program Files))
```
Closes#2300
This PR addresses the following two issues:
#4203: If a selection is active, a <kbd>shift</kbd>-LeftClick will set the SelectionEnd to where the pointer is.
#3911: Currently, any keypress will clear selection, and will pass through to the terminal. This PR will make it so that if a selection is active, _any_ keypress will clear the selection and then any keypress _except_ <kbd>esc</kbd> will pass through to the terminal.
## PR Checklist
* [x] Closes#4203; Closes#3911
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
## Validation Steps Performed
Played around a whole bunch with shift-clicking selections and regular clicking selections.
Also played around with selections and dismissing with all sorts of keypresses and keychords.
Tests all pass still!
Fixes#4155.
## Validation steps
```
Summary: Total=23, Passed=22, Failed=1, Blocked=0, Not Run=0, Skipped=0
```
The failing test is the same one as before. It is not germane to this pull request.
<!-- 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
So this PR adds a profile setting called "confirmCloseAllTabs", that allows one to enable or disable the "Do you want close all tabs?" dialog that appears when you close a window with multiple open tabs. It current defaults to "true". Also adds a checkbox to that dialog that also sets "confirmCloseAllTabs"
<!-- 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#3883
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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 added a checkbox to the close dialog to set this setting, but I'm not sure how to best go about actually changing the setting from code; am open to suggestions, as to how it should be done, or if I should also just remove it and stick with the profile setting.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
1. Set "confirmCloseAllTabs" to false in my profile.json file.
2. Opened a 2nd tab.
3. Closed the window
4. Observed that there was no confirmation before the window closed.
5. Set "confirmCloseAllTabs" to true
6. Repeat steps 2 and 3
7. Observe that there was a confirmation before the window closed.
This commit also fixes default buttons and default button styling
in all other dialogs and properly hooks up ESC and Enter for the
Close dialog.
Closes#4307.
Closes#3379.
We were overriding the button foreground and the placeholder foreground using our
own custom resource names. They didn't exist in HC.
Instead of making them exist in HC, I made us use and override the real resource
names. Those ones have HC colors defined by the platform!
Fixes#4393.
## Summary of the Pull Request
I took the code from conhost that handles this and just copy-pasted it into the terminal codebase.
## References
Original conhost code:
027f1228cb/src/interactivity/win32/windowproc.cpp (L854-L889)
## PR Checklist
* [x] Closes#904
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Okay it was a little more complicated than that. I had `IslandWindow` handle the drop, which then raises a generic event for `AppHost` to handle. `AppHost` handles this by writing the path as input to the terminal, traversing `AppLogic`, `TerminalPage` and finally landing in `TermControl`
## Validation Steps Performed
Tested manually with both paths with and without spaces.
This commit fixes an issue where "wt -d C: wsl -d Alpine" would be
parsed as "wt -d C: -d Alpine wsl" and rejected as invalid due to the
repeated -d. It also fixes support for the option parsing terminator,
--, in all command lines.
Fixes#4277.
## Summary of the Pull Request
This PR removes all of the VT-specific functionality from the `WriteCharsLegacy` function that dealt with control characters, since those controls are now handled in the state machine when in VT mode. It also removes most of the control character handling from the `Terminal::_WriteBuffer` method for the same reason.
## References
This is a followup to PR #4171
## PR Checklist
* [x] Closes#3971
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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: https://github.com/microsoft/terminal/issues/780#issuecomment-570287435
## Detailed Description of the Pull Request / Additional comments
There are four changes to the `WriteCharsLegacy` implementation:
1. The `TAB` character had special case handling in VT mode which is now no longer required. This fixes a bug in the Python REPL editor (when run from a cmd shell in Windows Terminal), which would prevent you tabbing past the end of the line. It also fixes#3971.
2. Following on from point 1, the `WC_NONDESTRUCTIVE_TAB` flag could also now be removed. It only ever applied in VT mode, in which case the `TAB` character isn't handled in `WriteCharsLegacy`, so there isn't a need for a non-destructive version.
3. There used to be special case handling for a `BS` character at the beginning of the line when in VT mode, and that is also no longer required. This fixes an edge-case bug which would prevent a glyph being output for code point 8 when `ENABLE_PROCESSED_OUTPUT` was disabled.
4. There was quite a lot of special case handling for control characters in the "end-of-line wrap" implementation, which is no longer required. This fixes a bug which would prevent "low ASCII" characters from wrapping when output at the end of a line.
Then in the `Terminal::_WriteBuffer` implementation, I've simply removed all control character handling, except for `LF`. The Terminal is always in VT mode, so the control characters are always handled by the state machine. The exception for the `LF` character is simply because it doesn't have a proper implementation yet, so it still passes the character through to `_WriteBuffer`. That will get cleaned up eventually, but I thought that could wait for a later PR.
Finally, with the removal of the VT mode handling in `WriteCharsLegacy`, there was no longer a need for the `SCREEN_INFORMATION::InVTMode` method to be publicly accessible. That has now been made private.
## Validation Steps Performed
I've only tested manually, making sure the conhost and Windows Terminal still basically work, and confirming that the above-mentioned bugs are fixed by these changes.
## Summary of the Pull Request
#4354 is a pretty complicated PR. It's got a bunch of conpty changes, but what it also has was some critical improvements to the roundtrip test suite. I'm working on some other bugfixes in the same area currently, and need these tests enhancements in those branches _now_. The rest of #4354 is complex enough that I don't trust it will get merged soon (if ever). However, these fixes _should_ be in regardless.
## PR Checklist
* [x] Taken directly from #4354
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
This is four main changes:
* Enable conpty to be fully enabled in unittests. Just setting up a VT renderer isn't enough to trick the host into being in conpty mode - it also needs to have some other flags set.
* Some minor changes to `CommonState` to better configure the common test state for conpty
* Move some of the verify helpers from `ConptyRoundtripTests` into their own helper class, to be shared in multiple tests
* Add a `TerminalBufferTests` class, for testing the Terminal buffer directly (without conpty).
This change is really easier than
would suggest, I promise.
* Remove unneeded c_str() calls when converting an hstring to a wstring_view.
* Remove unneeded c_str() calls when constructing a FontInfo class with a wstring face name.
* Remove unneeded winrt::to_hstring calls when passing a wstring to a method that expects an hstring.
* Remove unneeded c_str() calls when passing an hstring to a method that already accepts hstrings without conversion.
* Remove unneeded c_str() and data() calls when explicitly constructing an hstring from a wstring.
## Summary of the Pull Request
Adds support for commandline arguments to the Windows Terminal, in accordance with the spec in #3495
## References
* Original issue: #607
* Original spec: #3495
## PR Checklist
* [x] Closes#607
* [x] I work here
* [x] Tests added/passed
* [ ] We should probably add some docs on these commands
* [x] The spec (#3495) needs to be merged first!
## Detailed Description of the Pull Request / Additional comments
🛑 **STOP** 🛑 - have you read #3495 yet? If you haven't, go do that now.
This PR adds support for three initial sub-commands to the `wt.exe` application:
* `new-tab`: Used to create a new tab.
* `split-pane`: Used to create a new split.
* `focus-tab`: Moves focus to another tab.
These commands are largely POC to prove that the commandlines work. They're not totally finished, but they work well enough. Follow up work items will be filed to track adding support for additional parameters and subcommands
Important scenarios added:
* `wt -d .`: Open a new wt instance in the current working directory #878
* `wt -p <profile name>`: Create a wt instance running the given profile, to unblock #576, #1357, #2339
* `wt ; new-tab ; split-pane -V`: Launch the terminal with multiple tabs, splits, to unblock #756
## Validation Steps Performed
* Ran tests
* Played with it a bunch
This commit introduces a new recursive pane shutdown that will give all
controls under a tab a chance to clean up their state before beign
detached from the UI. It also reorders the call to LastTabClosed() so
that the application does not exit before the final connections are
terminated.
It also teaches TSFInputControl how to shut down to avoid a dramatic
platform bug.
Fixes#4159.
Fixes#4336.
## PR Checklist
* [x] CLA signed
* [x] I've discussed this with core contributors already.
## Validation Steps Performed
Validated through manual terminal teardown within and without the debugger, given a crazy number of panes and tabs.
## Summary of the Pull Request
In #4213 I added a dependency to the `UnitTests_TerminalCore` project on basically all of conhost. This _worked on my machine_, but it's consistently not working on other machines. This should fix those issues.
## References
## PR Checklist
* [x] Closes#4285
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
Made a fresh clone and built it.
## Summary of the Pull Request
This PR adds two tests:
* First, I started by writing a test where I could write output to the console host and inspect what output came out of conpty. This is the `ConptyOutputTests` in the host unit tests.
* Then I got crazy and thought _"what if I could take that output and dump it straight into the `Terminal`"_? Hence, the `ConptyRoundtripTests` were born, into the TerminalCore unit tests.
## References
Done in pursuit of #4200, but I felt this warranted it's own atomic PR
## PR Checklist
* [x] Doesn't close anything on it's own.
* [x] I work here
* [x] you better believe this adds tests
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
From the comment in `ConptyRoundtripTests`:
> This test class creates an in-proc conpty host as well as a Terminal, to
> validate that strings written to the conpty create the same resopnse on the
> terminal end. Tests can be written that validate both the contents of the
> host buffer as well as the terminal buffer. Everytime that
> `renderer.PaintFrame()` is called, the tests will validate the expected
> output, and then flush the output of the VtEngine straight to th
Also, some other bits had to be updated:
* The renderer needed to be able to survive without a thread, so I hadded a simple check that it actually had a thread before calling `pThread->NotifyPaint`
* Bits in `CommonState` used `NTSTATUS_FROM_HRESULT` which did _not_ work outside the host project. Since the `NTSTATUS` didn't seem that important, I replaced that with a `HRESULT`
* `CommonState` likes to initialize the console to some _weird_ defaults. I added an optional param to let us just use the defaults.
WT crashes when an unparseable/invalid `backgroundImage` or `icon`
resource path is provided in `profiles.json`. This PR averts the crash
by the validating and correcting resource paths as a part of the
`_ValidateSettings()` function in `CascadiaSettings`.
`_ValidateSettings()` is run on start up and any time `profiles.json` is
changed, so a user can not change a file path and avoid the validation
step.
When a bad `backgroundImage` or `icon` resource path is detected, a
warning screen will be presented.
References #4002, which identified a consistent repro for the crash.
To validate the resource, a `Windows::Foundation::Uri` object is
constructed with the path. The ctor will throw if the resource path is
invalid. Whether or not this validation method is robust enough is a
subject worth review. The correction method for when a bad resource path
is detected is to reset the `std::optional<winrt::hstring>` holding the
file path.
The text in the warning display was cribbed from the text used when an
invalid `colorScheme` is used. Whether or not the case of a bad
background image file path warrants a warning display is a subject worth
review.
Ensured the repro steps in #4002 did not trigger a crash. Additionally,
some potential backdoor paths to a crash were tested:
- Deleting the file of a validated background image file path
- Changing the actual file name of a validated background image file
path
- Replacing the file of a validated background image file path with a
non-image file (of the same name)
- Using a non-image file as a background image
In all the above cases WT does not crash, and instead defaults to the
background color specified in the profile's `colorScheme`. This PR does
not implement this recovery behavior (existing error catching code
does).
Closes#2329
This commit moves the handling of the `BEL`, `BS`, `TAB`, and `CR`
controls characters into the state machine (when in VT mode), instead of
forwarding them on to the default string writer, which would otherwise
have to parse them out all over again.
This doesn't cover all the control characters, but `ESC`, `SUB`, and
`CAN` are already an integral part of the `StateMachine` itself; `NUL`
is filtered out by the `OutputStateMachineEngine`; and `LF`, `FF`, and
`VT` are due to be implemented as part of PR #3271.
Once all of these controls are handled at the state machine level, we
can strip out all the VT-specific code from the `WriteCharsLegacy`
function, which should simplify it considerably. This would also let us
simplify the `Terminal::_WriteBuffer` implementation, and the planned
replacement stream writer for issue #780.
On the conhost side, the implementation is handled as follows:
* The `BS` control is dispatched to the existing `CursorBackward`
method, with a distance of 1.
* The `TAB` control is dispatched to the existing `ForwardTab` method,
with a tab count of 1.
* The `CR` control required a new dispatch method, but the
implementation was a simple call to the new `_CursorMovePosition` method
from PR #3628.
* The `BEL` control also required a new dispatch method, as well as an
additional private API in the `ConGetSet` interface. But that's mostly
boilerplate code - ultimately it just calls the `SendNotifyBeep` method.
On the Windows Terminal side, not all dispatch methods are implemented.
* There is an existing `CursorBackward` implementation, so `BS` works
OK.
* There isn't a `ForwardTab` implementation, but `TAB` isn't currently
required by the conpty protocol.
* I had to implement the `CarriageReturn` dispatch method, but that was
a simple call to `Terminal::SetCursorPosition`.
* The `WarningBell` method I've left unimplemented, because that
functionality wasn't previously supported anyway, and there's an
existing issue for that (#4046).
## Validation Steps Performed
I've added a state machine test to confirm that the updated control
characters are now forwarded to the appropriate dispatch handlers. But
since the actual implementation is mostly relying on existing
functionality, I'm assuming that code is already adequately tested
elsewhere. That said, I have also run various manual tests of my own,
and confirmed that everything still worked as well as before.
References #3271
References #780
References #3628
References #4046
## Summary of the Pull Request
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#4013
* [x] I work here.
* [x] Existing tests should be OK. Real changes, just adding a lib to use.
* [x] Couldn't find any existing docs about intsafe.
* [x] Am core contributor.
## Detailed Description of the Pull Request / Additional comments
* [x] Can we remove min/max completely or rename it in the two projects where it had to be reintroduced? This is now moved into #4152
* [x] How many usages of the old safe math are there? **79**
* [x] If not a ton, can we migrate them here or in a follow on PR? This is now moved into #4153
Files with old safe math:
- TerminalControl: TSFInputControl.cpp
- TerminalCore: TerminalDispatch.cpp
- TerminalCore: TerminalSelection.cpp
- Host: directio.cpp
- RendererGdi: invalidate.cpp
- RendererGdi: math.cpp
- RendererGdi: paint.cpp
- RendererVt: paint.cpp
- TerminalAdapter: adaptDispatch.cpp
- Types: viewport.cpp
- Types: WindowUiaProviderBase.cpp
## Validation Steps Performed
## Summary of the Pull Request
This adds support for the `FF` (form feed) and `VT` (vertical tab) [control characters](https://vt100.net/docs/vt510-rm/chapter4.html#T4-1), as well as the [`NEL` (Next Line)](https://vt100.net/docs/vt510-rm/NEL.html) and [`IND` (Index)](https://vt100.net/docs/vt510-rm/IND.html) escape sequences.
## References
#976 discusses the conflict between VT100 Index sequence and the VT52 cursor back sequence.
## PR Checklist
* [x] Closes#3189
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be 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: #3189
## Detailed Description of the Pull Request / Additional comments
I've added a `LineFeed` method to the `ITermDispatch` interface, with an enum parameter specifying the required line feed type (i.e. with carriage return, without carriage return, or dependent on the [`LNM` mode](https://vt100.net/docs/vt510-rm/LNM.html)). The output state machine can then call that method to handle the various line feed control characters (parsed in the `ActionExecute` method), as well the `NEL` and `IND` escape sequences (parsed in the `ActionEscDispatch` method).
The `AdaptDispatch` implementation of `LineFeed` then forwards the call to a new `PrivateLineFeed` method in the `ConGetSet` interface, which simply takes a bool parameter specifying whether a carriage return is required or not. In the case of mode-dependent line feeds, the `AdaptDispatch` implementation determines whether the return is necessary or not, based on the existing _AutoReturnOnNewLine_ setting (which I'm obtaining via another new `PrivateGetLineFeedMode` method).
Ultimately we'll want to support changing the mode via the [`LNM` escape sequence](https://vt100.net/docs/vt510-rm/LNM.html), but there's no urgent need for that now. And using the existing _AutoReturnOnNewLine_ setting as a substitute for the mode gives us backwards compatible behaviour, since that will be true for the Windows shells (which expect a linefeed to also generate a carriage return), and false in a WSL bash shell (which won't want the carriage return by default).
As for the actual `PrivateLineFeed` implementation, that is just a simplified version of how the line feed would previously have been executed in the `WriteCharsLegacy` function. This includes setting the cursor to "On" (with `Cursor::SetIsOn`), potentially clearing the wrap property of the line being left (with `CharRow::SetWrapForced` false), and then setting the new position using `AdjustCursorPosition` with the _fKeepCursorVisible_ parameter set to false.
I'm unsure whether the `SetIsOn` call is really necessary, and I think the way the forced wrap is handled needs a rethink in general, but for now this should at least be compatible with the existing behaviour.
Finally, in order to make this all work in the _Windows Terminal_ app, I also had to add a basic implementation of the `ITermDispatch::LineFeed` method in the `TerminalDispatch` class. There is currently no need to support mode-specific line feeds here, so this simply forwards a `\n` or `\r\n` to the `Execute` method, which is ultimately handled by the `Terminal::_WriteBuffer` implementation.
## Validation Steps Performed
I've added output engine tests which confirm that the various control characters and escape sequences trigger the dispatch method correctly. Then I've added adapter tests which confirm the various dispatch options trigger the `PrivateLineFeed` API correctly. And finally I added some screen buffer tests that check the actual results of the `NEL` and `IND` sequences, which covers both forms of the `PrivateLineFeed` API (i.e. with and without a carriage return).
I've also run the _Test of cursor movements_ in the [Vttest](https://invisible-island.net/vttest/) utility, and confirmed that screens 1, 2, and 5 are now working correctly. The first two depend on `NEL` and `IND` being supported, and screen 5 requires the `VT` control character.
## Summary of the Pull Request
See [my code comment](https://github.com/microsoft/terminal/pull/4150#discussion_r364392640) below for technical details of the issue that caused #4145.
## PR Checklist
* [x] Closes#1360, Closes#4145.
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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
TBH I kinda hope this project could migrate to an internal use of UTF-8 in the future. 😶
## Validation Steps Performed
Followed the "Steps to reproduce" in #4145 and ensured the "Expected behavior" happens.
## Summary of the Pull Request
New year, new unittests.
This PR introduces a new project, `TestHostApp`. This project is largely taken from the TAEF samples, and allows us to easily construct a helper executable and `resources.pri` for running TerminalApp unittests.
## References
## PR Checklist
* [x] Closes#3986
* [x] I work here
* [x] is Tests
* [n/a] Requires documentation to be updated
* [x] **Waiting for an updated version of TAEF to be available**
## Detailed Description of the Pull Request / Additional comments
Unittesting for the TerminalApp project has been a horrifying process to try getting everything pieced together just right. Dependencies need to get added to manifests, binplaced correctly, and XAML resources need to get compiled together as well. In addition, using a MUX `Application` (as opposed to the Windows.UI.Xaml `Application`) has led to additional problems.
This was always a horrifying house of cards for us. Turns out, the reason this was so horrible is that the test infrastructure for doing what we're doing _literally didn't exist_ when I started doing all that work last year.
So, with help from the TAEF team, I was able to get rid of our entire house of cards, and use a much simpler project to build and run the tests.
Unfortunately, the latest TAEF release has a minor bug in it's build rules, and only publishes the x86 version of a dll we need from them. But, the rest of this PR works for x86, and I'll bump this when that updated version is available. We should be able to review this even in the state it's in.
## Validation Steps Performed
ran the tests yo
<!-- 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 turns all* instances of `Dispatcher().RunAsync` to WinRT coroutines 👌.
This was good coding fodder to fill my plane ride ✈️. Enjoy your holidays everyone!
*With the exception of three functions whose signatures cannot be changed due to inheritance and function overriding in `TermControlAutomationPeer` [`L44`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp#L44), [`L58`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp#L58), [`L72`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp#L72).
<!-- 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#3919
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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: #3919
<!-- 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
My thought pattern here was to minimally disturb the existing code where possible. So where I could, I converted existing functions into coroutine using functions (like in the [core example](https://github.com/microsoft/terminal/issues/3919#issue-536598706)). For ~the most part~ all instances, I used the format where [`this` is accessed safely within a locked scope](https://github.com/microsoft/terminal/issues/3919#issuecomment-564730620). Some function signatures were changed to take objects by value instead of reference, so the coroutines don't crash when the objects are accessed past their original lifetime. The [copy](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/TerminalPage.cpp#L1132) and [paste](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/TerminalPage.cpp#L1170) event handler entry points were originally set to a high priority; however, the WinRT coroutines don't appear to support a priority scheme so this priority setting was not preserved in the translation.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Compiles and runs, and for every event with a clear trigger repro, I triggered it to ensure crashes weren't introduced.
These utility distributions are used by Docker for Windows' WSL2
integration. They are not intended for user consumption.
* [x] Closes#3556
* [x] CLA signed.
* [x] Tests added/passed
* [x] Requires documentation to be updated
* [x] I've discussed this with core contributors already.
There is a minimal chance that a user _has_ a distribution named `docker-desktop` or some superstring thereof, but this is taken as an acceptable level of risk.
We were eating an exception on exit in debug mode because we were using
wil to clean up half of a named pipe we'd already handed ownership of
over to the CRT. The CRT likes to tie up all its loose ends when it gets
unloaded, so it was coming along at exit and closing the handle again.
Big no-no.
## Summary of the Pull Request
Adds a setting `snapToGridOnResize` to disable snapping the window on resize, and defaults it to `false`.
## References
Introduced by pr #3181
## PR Checklist
* [x] Closes#3995
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
When user resizes window, snap the size to align with the character grid
(like e.g. putty, mintty and most unix terminals). Properly resolves
arbitrary pane configuration (even with different font sizes and
padding) trying to align each pane as close as possible.
It also fixes terminal minimum size enforcement which was not quite well
handled, especially with multiple panes.
This PR does not however try to keep the terminals aligned at other user
actions (e.g. font change or pane split). That is to be tracked by some
other activity.
Snapping is resolved in the pane tree, recursively, so it (hopefully)
works for any possible layout.
Along the way I had to clean up some things as so to make the resulting
code not so cumbersome:
1. Pane.cpp: Replaced _firstPercent and _secondPercent with single
_desiredSplitPosition to reduce invariants - these had to be kept in
sync so their sum always gives 1 (and were not really a percent). The
desired part refers to fact that since panes are aligned, there is
usually some deviation from that ratio.
2. Pane.cpp: Fixed _GetMinSize() - it was improperly accounting for
split direction
3. TerminalControl: Made dedicated member for padding instead of
reading it from a control itself. This is because the winrt property
functions turned out to be slow and this algorithm needs to access it
many times. I also cached scrollbar width for the same reason.
4. AppHost: Moved window to client size resolution to virtual method,
where IslandWindow and NonClientIslandWindow have their own
implementations (as opposite to pointer casting).
One problem with current implementation is I had to make a long call
chain from the window that requests snapping to the (root) pane that
implements it: IslandWindow -> AppHost's callback -> App ->
TerminalPage -> Tab -> Pane. I don't know if this can be done better.
## Validation Steps Performed
Spam split pane buttons, randomly change font sizes with ctrl+mouse
wheel and drag the window back and forth.
Closes#2834Closes#2277
The first argument to `NotifyTextChanged` incorrectly was `[0,0]`
instead of the length of the text to be removed from the
`CoreTextEditContext`.
Best source of documentation for `NotifyTextChanged`:
https://docs.microsoft.com/en-us/windows/uwp/design/input/custom-text-input#overriding-text-updates
FYI @DHowett-MSFT (just in case): C++/WinRT uses `winrt::param::hstring`
for string parameters which intelligently borrows strings. As such you
can simply pass a `std::wstring` to most WinRT methods without the need
of having to allocate an intermediate `hstring`. 🙂
## Validation Steps Performed
I followed the reproduction instructions of #3706 and #3745 and ensured
the issue doesn't happen anymore.
Closes#3645Closes#3706Closes#3745
The terminal will use the system setting to determine the number of lines to scroll at a time.
This can be overridden by adding rowsToScroll to app global settings file.
terminal will use the system setting if the app setting is 0, or not specified. No restart is needed to reflect setting changes in system or the settings file.
The default was hardcoded to 4 in the code with a todo comment. 1 works better on precision touchpads, where 4 scrolls too fast.
Co-authored-by: Hannes Nel <hannesne@microsoft.com>
## Summary of the Pull Request
- Enables auditing of some Terminal libraries (Connection, Core, Settings)
- Also audit WinConPTY.LIB since Connection depends on it
## PR Checklist
* [x] Rolls audit out to more things
* [x] I work here
* [x] Tests should still pass
* [x] Am core contributor
## Detailed Description of the Pull Request / Additional comments
This is turning on the auditing of these projects (as enabled by the heavier lifting in the other refactor) and then cleaning up the remaining warnings.
## Validation Steps Performed
- [x] Built it
- [x] Ran the tests
<!-- 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
- Enables auditing of Virtual Terminal libraries (input, adapter, parser)
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Rolls audit out to more things
* [x] I work here
* [x] Tests should still pass
* [x] Am core contributor
* [x] Closes#3957
<!-- 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
This is turning on the auditing of these projects (as enabled by the heavier lifting in the other refactor) and then cleaning up the remaining warnings.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
- [x] Built it
- [x] Ran the tests
<!-- 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
Before, when a terminal window was focused, the blinking cursor would initially be hidden. This PR will immediately show the cursor when the window is focused, making it easier to keep track of the cursor.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
#3761
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#3761
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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 guess I'm the cursor guy now
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
* Switched rapidly between different panes, different tabs and focused and unfocused the main window repeatedly.
* search box text localization and search parameters refactoring
* format fix
* remvove unecessary spaces
* Tooltips text localization, CR chanegs
* Move ESC handling to SearchBoxControl
* format check
* mark Esc key input as handled in SearchBoxControl
## Summary of the Pull Request
Refactors parsing/adapting libraries and consumers to use safer and/or more consistent mechanisms for passing information.
## PR Checklist
* [x] I work here
* [x] Tests still pass
* [x] Am a core contributor.
## Detailed Description of the Pull Request / Additional comments
This is in support of hopefully turning audit mode on to more projects. If I turned it on, it would immediately complain about certain classes of issues like pointer and size, pointer math, etc. The changes in this refactoring will eliminate those off the top.
Additionally, this has caught a bunch of comments all over the VT classes that weren't updated to match the parameters lists.
Additionally, this has caught a handful of member variables on classes that were completely unused (and now gone).
Additionally, I'm killing almost all hungarian and shortening variable names. I'm only really leaving 'p' for pointers.
Additionally, this is vaguely in support of a future where we can have "infinite scrollback" in that I'm moving things to size_t across the board. I know it's a bit of a memory cost, but all the casting and moving between types is error prone and unfun to save a couple bytes.
## Validation Steps Performed
- [x] build it
- [x] run all the tests
- [x] everyone looked real hard at it
## Summary of the Pull Request
Adds support for `auto` as a potential value for a `splitPane` keybinding's `split` argument. For example:
```json
{ "keys": [ "ctrl+shift+z" ], "command": { "action": "splitPane", "profile": "matrix", "commandline": "cmd.exe", "split":"auto" } },
```
When set to `auto`, Panes will decide which direction to split based on the available space within the terminal. If the pane is wider than it is tall, the pane will introduce a new vertical split (and vice-versa).
## References
## PR Checklist
* [x] Closes#3960
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Validation Steps Performed
Ran tests, played with it.
## Summary of the Pull Request
This latest MUX prerelease fixes the issue where the tab row wouldn't expand to fill the width of the window after shrinking the window size.
## PR Checklist
* [x] Closes#3300
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Thanks again @teaP for help fixing this
## Validation Steps Performed
Launched the terminal, played with it a bit
<!-- 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)? -->
This is the PR for feature Search: #605
This PR includes the newly introduced SearchBoxControl in TermControl dir, which is the search bar for the search experience. And the codes that enable Search in Windows Terminal.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
The PR that migrates the Conhost search module: https://github.com/microsoft/terminal/pull/3279
Spec (still actively updating): https://github.com/microsoft/terminal/pull/3299
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#605
* [ ] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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 -->
These functionalities are included in the search experience.
1. Search in Terminal text buffer.
2. Automatic wrap-around.
3. Search up or down switch by clicking different buttons.
4. Search case sensitively/insensitively by clicking a button. S. Move the search box to the top/bottom by clicking a button.
6. Close by clicking 'X'.
7. Open search by ctrl + F.
When the searchbox is open, the user could still interact with the terminal by clicking the terminal input area.
While I already have the search functionalities, currently there are still some known to-do works and I will keep updating my PR:
1. Optimize the search box UI, this includes:
1) Theme adaptation. The search box background and font color
should change according to the theme,
2) Add background. Currently the elements in search box are all
transparent. However, we need a background.
3) Move button should be highlighted once clicked.
2. Accessibility: search process should be able to performed without mouse. Once the search box is focused, the user should be able to navigate between all interactive elements on the searchbox using keyboard.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
To test:
1. checkout this branch.
2. Build the project.
3. Start Windows Terminal and press Ctrl+F
4. The search box should appear on the top right corner.
This PR fixes the ability to move between already-split panes
## Summary of the Pull Request
## PR Checklist
* [x] Closes#3045
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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
Fixes the sides disappearing when entering full screen mode when the window is maximized.
However, now, a Vista-style frame briefly appears when entering/exiting full screen.
<!-- 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#3709
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [x] Requires documentation to be updated (no)
* [ ] 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
When the non-client island window is maximized and has the WS_OVERLAPPEDWINDOW style, SetWindowPos is "lying": the position ends up being offset compared to the one we gave it (found by debugging). So I changed it to use WS_POPUP like the client island window was already doing. But now it has the Vista frame that appears briefly when entering/exiting full screen like the client island window.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
<!-- 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
Uses the verification in `at` to ensure the index is correct (as @j4james suggests). If `at` throws, then returns false.
<!-- 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#3720
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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: #3720
<!-- 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
Can no longer repro the issue after the fix.
<!-- 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
An asynchronous event handler capturing raw `this` in `TermControl` was causing an exception to be thrown when a scroll update event occurred after closing the active tab. This PR replaces all non-auto_revoke lambda captures in `TermControl` to capture (and validate) a `winrt::weak_ref` instead of using raw `this`.
<!-- 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#2947
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Requires documentation to be 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: #2947
<!-- 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
`TermControl` is already a WinRT type so no changes were required to enable the `winrt::weak_ref` functionality. There was only one strange change I had to make. In the destructor's helper function `Close`, I had to remove two calls to [`Stop`](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.dispatchertimer.stop#Windows_UI_Xaml_DispatcherTimer_Stop) which were throwing under [some circumstances](https://github.com/microsoft/terminal/issues/2947#issuecomment-562914135). Fortunately, these calls don't appear to be critical, but definitely a spot to look into when reviewing this PR.
Beyond scrolling, any anomalous crash related to the following functionality while closing a tab or WT may be fixed by this PR:
- Settings updating
- Changing background color
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Before these changes I was able to consistently repro the issue in #2947. Now, I can no longer repro the issue.
Enables support for word navigation when using an automation client (i.e.: Narrator, etc...). Specifically, adds this functionality to the UiaTextRange class. The only delimiter used is whitespace because that's how words are separated in English.
# General "Word Movement" Expectations
The resulting text range should include any word break characters that are present at the end of the word, but before the start of the next word. (Source)
If you already are on a word, getting the "next word" means you skip the word you are on, and highlight the upcoming word appropriately. (similar idea when moving backwards)
# Word Expansion
Since word selection is supposed to detect word delimiters already, I figured I'd reuse that code. I moved it from TerminalCore to the TextBuffer.
Then I built on top of it by adding an optional additional parameter that decides if you want to include...
- the delimiter run when moving forward
- the character run when moving backwards
It defaults to false so that we don't have to care when using it in selection. But we change it to true when using it in our UiaTextRange
# UiaTextRange
The code is based on character movement. This allows us to actually work with boundary conditions.
The main thing to remember here is that each text range is recorded as a MoveState. The text range is most easily defined when you think about the start Endpoint and the end Endpoint. An Endpoint is just a linear 1-dimensional indexing of the text buffer. Examples:
- Endpoint 0 --> (0,0)
- Endpoint 79 --> (79,0) (when the buffer width is 80)
- Endpoint 80 -->(0,1) (when the buffer width is 80)
- When moving forward, the strategy is to focus on moving the end Endpoint. That way, we properly get the indexing for the "next" word (this also fixes a wrapping issue). Then, we update the start Endpoint. (This is reversed for moving backwards).
- When moving a specific Endpoint, we just have a few extra if statements to properly adjust for moving start vs end.
# Hooking it up
All we really had to do is add an enum. This part was super easy :)
I originally wanted the delimiters to be able to be defined. I'm not so sure about that anymore. Either way, I hardcoded our delimiter into a variable so if we ever want to expand on it or make that customizable, we just modify that variable.
# Defining your own word delimiters
- Import a word delimiter into the constructor of the ScreenInfoUiaProvider (SIUP)
- This defines a word delimiter for all the UiaTextRanges (UTR) created by in this context
- import a word delimiter into the UTR directly
- this provides more control over what a "word" is
- this can be useful if you have an idea of what text a particular UTR will encounter and you want to customize the word navigation for it (i.e consider adding / or \\ for file paths)
The default param of " " is scattered throughout because this is the word delimiter used in the English language.
## Summary of the Pull Request
The original PR had a few TODOs in it without issue numbers. IMO, this wasn't important enough to block the PR over. _Also I'm impatient and wanted that setting_.
After I merged the PR I created the issues and added the numbers myself.
## References
## PR Checklist
* [x] Closes nothing, this just adds a couple TODOs
* [x] I work here
* [x] this _really_ doesn't need tests
* [x] This _is_ a docs update
<!-- 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
Cool retro terminal effects
- glow
- scan lines
- cool
- will make terminal competitive with iterm2
<!-- 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
* [ ] Requires documentation to be 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
## Summary of the Pull Request
Turns on Audit for WinRTUtils, fixes audit failures.
## PR Checklist
* [x] I work here
* [x] Still builds
* [x] Am core contributor.
## Validation Steps Performed
Built it.
This PR makes use of the UiaRenderer by attaching it to the TerminalControl and setting up selectionChanged events for accessibility.
Part 1: attaching the UiaRenderer
The uiaRenderer is treated very similarly to the dxRenderer. We have a unique_ptr ref to it in the TermControl. This gets populated when the TermControlAutomationPeer is created (thus enabling accessibility).
To prevent every TermControl from sending signals simultaneously, we specifically only enable whichever one is in an active pane.
The UiaRenderer needs to send encoded events to the automation provider (in this case, TermControlAutomationPeer). We needed our own automation events so that we can reuse this model for ConHost. This is the purpose of IUiaEventDispatcher.
We need a dispatcher for the UiaRenderer. Otherwise, we would do a lot of work to find out when to fire an event, but we wouldn't have a way of doing that.
Part 2: hooking up selection events
This provides a little bit of polish to hooking it up before. Primarily to actually make it work. This includes returning S_FALSE instead of E_NOTIMPL.
The main thing here really is just how to detect if a selection has changed. This also shows how clean adding more events will be in the future!
## Summary of the Pull Request
On this month's episode of "Mike accidentally breaks this":
Mike forgets that `==` is defined on a pair of winrt objects as "do these point to the _SAME_ object", not "do they have the same values".
This just slipped right through code review.
## References
Broken in #3825
## PR Checklist
* [x] Closes#3896
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
## Validation Steps Performed
Manually checked that these are still there
## Summary of the Pull Request
I accidentally did the wrong check here to see if the value exists. For an `IReference`, you need to do `variable != nullptr`. I did `variable.Value()`.
## References
Introduced in #3825
## PR Checklist
* [x] Closes#3897
* [x] I work here
* [ ] Tests added/passed - wow this was a lot harder than I expected
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
This includes a maybe unrelated fix to make `TerminalPage`'s `ShortcutActionDispatch` a `com_ptr`. While I was messing with the tests for this, I caught that we're not supposed to direct allocate winrt types like that. Ofc, the `TerminalAppLib` project doesn't catch this.
## Validation Steps Performed
Ran the terminal manually, instead of just running the tests
## Summary of the Pull Request
_This is attempt 2 at this feature_. The original PR can be found at #3369.
These are settings that apply to _every_ profile, before user customizations.
If the user wants to add "default profile settings", they can make the `"profiles"` property an _object_, instead of a list, and add `"defaults"` key underneath that object. The users list of profiles should then be under the `list` property of the `profiles` object.
## References
#2515, #2603, #3369, #3569
## PR Checklist
* [x] Closes#2325
* [x] I work here
* [x] Tests added/passed
* [x] schema, docs updated
## Detailed Description of the Pull Request / Additional comments
~~Discussion in #2325 itself serves as the "spec" for this task. I thought we'd need more discussion on the topic, but it ended up being pretty straightforward.~~
I should not have said that in the original PR. We've had a better spec review now that I think we're happier with.
## Validation Steps Performed
_ran the tests_
For our Universal terminal for development purposes, we will use telnet to escape the universal application container and empower developers to debug/diagnose issues with their own machine on loopback to the already-elevated telnet context.
## Summary of the Pull Request
This enables the user to set a number of extra settings in the `NewTab` and `SplitPane` `ShortcutAction`s, that enable customizing how a new terminal is created at runtime. The following four properties were added:
* `profile`
* `commandline`
* `tabTitle`
* `startingDirectory`
`profile` can be used with either a GUID or the name of a profile, and the action will launch that profile instead of the default.
`commandline`, `tabTitle`, and `startingDirectory` can all be used to override the profile's values of those settings. This will be more useful for #607.
With this PR, you can make bindings like the following:
```json
{ "keys": ["ctrl+a"], "command": { "action": "splitPane", "split": "vertical" } },
{ "keys": ["ctrl+b"], "command": { "action": "splitPane", "split": "vertical", "profile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}" } },
{ "keys": ["ctrl+c"], "command": { "action": "splitPane", "split": "vertical", "profile": "profile1" } },
{ "keys": ["ctrl+d"], "command": { "action": "splitPane", "split": "vertical", "profile": "profile2" } },
{ "keys": ["ctrl+e"], "command": { "action": "splitPane", "split": "horizontal", "commandline": "foo.exe" } },
{ "keys": ["ctrl+f"], "command": { "action": "splitPane", "split": "horizontal", "profile": "profile1", "commandline": "foo.exe" } },
{ "keys": ["ctrl+g"], "command": { "action": "newTab" } },
{ "keys": ["ctrl+h"], "command": { "action": "newTab", "startingDirectory": "c:\\foo" } },
{ "keys": ["ctrl+i"], "command": { "action": "newTab", "profile": "profile2", "startingDirectory": "c:\\foo" } },
{ "keys": ["ctrl+j"], "command": { "action": "newTab", "tabTitle": "bar" } },
{ "keys": ["ctrl+k"], "command": { "action": "newTab", "profile": "profile2", "tabTitle": "bar" } },
{ "keys": ["ctrl+l"], "command": { "action": "newTab", "profile": "profile1", "tabTitle": "bar", "startingDirectory": "c:\\foo", "commandline":"foo.exe" } }
```
## References
This is a lot of work that was largely started in pursuit of #607. We want people to be able to override these properties straight from the commandline. While they may not make as much sense as keybindings like this, they'll make more sense as commandline arguments.
## PR Checklist
* [x] Closes#998
* [x] I work here
* [x] Tests added/passed
* [x] Requires documentation to be updated
## Validation Steps Performed
There are tests 🎉
Manually added some bindings, they opened the correct profiles in panes/tabs
