## Summary of the Pull Request
This adds support for the [`DECALN`](https://vt100.net/docs/vt510-rm/DECALN.html) escape sequence, which produces a kind of test pattern, originally used on VT terminals to adjust the screen alignment. It's needed to pass several of the tests in the [Vttest](https://invisible-island.net/vttest/) suite.
## PR Checklist
* [x] Closes#3671
* [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
* [ ] 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
To start with, the `ActionEscDispatch` method in the `OutputStateMachineEngine` needed to be extended to check for a new intermediate type (`#`). Then when that intermediate is followed by an `8`, it dispatches to a new `ScreenAlignmentPattern` method in the `ITermDispatch` interface.
The implementation of the `ScreenAlignmentPattern` itself is fairly simple. It uses the recently added `PrivateFillRegion` API to fill the screen with the character `E` using default attributes. Then in addition to that, a bunch of VT properties are reset:
* The meta/extended attributes are reset (although the active colors must be left unchanged).
* The origin mode is set to absolute positioning.
* The scrolling margins are cleared.
* The cursor position is moved to home.
## Validation Steps Performed
I've added a screen buffer test that makes sure the `DECALN` sequence fills the screen with the correct character and attributes, and that the above mentioned properties are all updated appropriately.
I've also tested in Vttest, and confirmed that the first two pages of the _Test of cursor movements_ are now showing the frame of E's that are expected there.
<!-- 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
## Summary of the Pull Request
Pretty much the title here.
## References
## PR Checklist
* [x] Closes#3780
* [x] I work here
* [x] documentation updated
<!-- 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.
## Summary of the Pull Request
When the `DECSTBM` margins are set, scrolling should only allowed within
those margins. So if the cursor is below the bottom margin, it should
just be clamped when it tries to move down from the bottom line of the
viewport, instead of scrolling the screen up. This PR implements that
restriction, i.e. it prevents scrolling taking place outside the
`DECSTBM` margins, which was previously allowed.
## PR Checklist
* [x] Closes#2657
* [x] CLA signed.
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
This simply adds a condition in the `AdjustCursorPosition` function to
check if the cursor is moving below the bottom of the viewport when the
margins are set. If so, it clamps the y coordinate to the bottom line of
the viewport.
The only time it's acceptable for scrolling to happen when margins are
set, is if the scrolling is taking place within those margins. But those
cases would already have been handled earlier in the function (in the
`fScrollDown` or `scrollDownAtTop` conditions), and thus the y
coordinate would have already been prevented from moving out of the
viewport.
## Validation Steps Performed
I've added some screen buffer tests to confirm this new behaviour, and
I've also checked the test case from the initial bug report (#2657) and
made sure it now matches the results in XTerm.
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!
Refer to the original issue: **Default Profile for Common Profile Settings** #2325
So this is my summary of everything we discussed regarding "default profile settings". The original PR was #3369, but we were _not_ in agreement on the UX, so this PR is for discussion about that.
I put forth 4 proposals that were mentioned in the discussion.
In the discussion that followed, we decided the 3rd proposal was the best. The doc reflects that choice.
## 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_
## Summary of the Pull Request
Operations that erase areas of the screen are typically meant to do so using the current color attributes, but with the rendition attributes reset (what we refer to as meta attributes). This also includes scroll operations that have to clear the area of the screen that has scrolled into view. The only exception is the _Erase Scrollback_ operation, which needs to reset the buffer with the default attributes. This PR updates all of these cases to apply the correct attributes when scrolling and erasing.
## PR Checklist
* [x] Closes#2553
* [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
* [ ] I've not really discussed this with core contributors. I'm ready to accept this work might be rejected in favor of a different grand plan.
## Detailed Description of the Pull Request / Additional comments
My initial plan was to use a special case legacy attribute value to indicate the "standard erase attribute" which could safely be passed through the legacy APIs. But this wouldn't cover the cases that required default attributes to be used. And then with the changes in PR #2668 and #2987, it became clear that our requirements could be better achieved with a couple of new private APIs that wouldn't have to depend on legacy attribute hacks at all.
To that end, I've added the `PrivateFillRegion` and `PrivateScrollRegion` APIs to the `ConGetSet` interface. These are just thin wrappers around the existing `SCREEN_INFORMATION::Write` method and the `ScrollRegion` function respectively, but with a simple boolean parameter to choose between filling with default attributes or the standard erase attributes (i.e the current colors but with meta attributes reset).
With those new APIs in place, I could then update most scroll operations to use `PrivateScrollRegion`, and most erase operations to use `PrivateFillRegion`.
The functions affected by scrolling included:
* `DoSrvPrivateReverseLineFeed` (the RI command)
* `DoSrvPrivateModifyLinesImpl` (the IL and DL commands)
* `AdaptDispatch::_InsertDeleteHelper` (the ICH and DCH commands)
* `AdaptDispatch::_ScrollMovement` (the SU and SD commands)
The functions affected by erasing included:
* `AdaptDispatch::_EraseSingleLineHelper` (the EL command, and most ED variants)
* `AdaptDispatch::EraseCharacters` (the ECH command)
While updating these erase methods, I noticed that both of them also required boundary fixes similar to those in PR #2505 (i.e. the horizontal extent of the erase operation should apply to the full width of the buffer, and not just the current viewport width), so I've addressed that at the same time.
In addition to the changes above, there were also a few special cases, the first being the line feed handling, which required updating in a number of places to use the correct erase attributes:
* `SCREEN_INFORMATION::InitializeCursorRowAttributes` - this is used to initialise the rows that pan into view when the viewport is moved down the buffer.
* `TextBuffer::IncrementCircularBuffer` - this occurs when we scroll passed the very end of the buffer, and a recycled row now needs to be reinitialised.
* `AdjustCursorPosition` - when within margin boundaries, this relies on a couple of direct calls to `ScrollRegion` which needed to be passed the correct fill attributes.
The second special case was the full screen erase sequence (`ESC 2 J`), which is handled separately from the other ED sequences. This required updating the `SCREEN_INFORMATION::VtEraseAll` method to use the standard erase attributes, and also required changes to the horizontal extent of the filled area, since it should have been clearing the full buffer width (the same issue as the other erase operations mentioned above).
Finally, there was the `AdaptDispatch::_EraseScrollback` method, which uses both scroll and fill operations, which could now be handled by the new `PrivateScrollRegion` and `PrivateFillRegion` APIs. But in this case we needed to fill with the default attributes rather than the standard erase attributes. And again this implementation needed some changes to make sure the full width of the active area was retained after the erase, similar to the horizontal boundary issues with the other erase operations.
Once all these changes were made, there were a few areas of the code that could then be simplified quite a bit. The `FillConsoleOutputCharacterW`, `FillConsoleOutputAttribute`, and `ScrollConsoleScreenBufferW` were no longer needed in the `ConGetSet` interface, so all of that code could now be removed. The `_EraseSingleLineDistanceHelper` and `_EraseAreaHelper` methods in the `AdaptDispatch` class were also no longer required and could be removed.
Then there were the hacks to handle legacy default colors in the `FillConsoleOutputAttributeImpl` and `ScrollConsoleScreenBufferWImpl` implementations. Since those hacks were only needed for VT operations, and the VT code no longer calls those methods, there was no longer a need to retain that behaviour (in fact there are probably some edge cases where that behaviour might have been considered a bug when reached via the public console APIs).
## Validation Steps Performed
For most of the scrolling operations there were already existing tests in place, and those could easily be extended to check that the meta attributes were correctly reset when filling the revealed lines of the scrolling region.
In the screen buffer tests, I made updates of that sort to the `ScrollOperations` method (handling SU, SD, IL, DL, and RI), the `InsertChars` and `DeleteChars` methods (ICH and DCH), and the `VtNewlinePastViewport` method (LF). I also added a new `VtNewlinePastEndOfBuffer` test to check the case where the line feed causes the viewport to pan past the end of the buffer.
The erase operations, however, were being covered by adapter tests, and those aren't really suited for this kind of functionality (the same sort of issue came up in PR #2505). As a result I've had to reimplement those tests as screen buffer tests.
Most of the erase operations are covered by the `EraseTests` method, except the for the scrollback erase which has a dedicated `EraseScrollbackTests` method. I've also had to replace the `HardReset` adapter test, but that was already mostly covered by the `HardResetBuffer` screen buffer test, which I've now extended slightly (it could do with some more checks, but I think that can wait for a future PR when we're fixing other RIS issues).
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
## Summary of the Pull Request
Fix the `TabTests`, and enable testing of types with XAML content. The `TabTests` were written many, many moons ago. they were intended to be our tests of XAML-like content within the Terminal app, so we could have unittests of Tabs, Panes, etc. Between their initial authoring and the day they were checked in, we had a bunch of build changes come in and break them irreperably.
We've gotten them fixed now with _one weird trick_ <sup>doctors hate me</sup>. As long as there isn't an `App.xbf` in the test's output directory, then the tests will deploy just fine.
We also needed a bit of magic, cribbed straight from TAEF, to enable running test code synchronously on the UI thread. Hence, `CppwinrtTailored.h`.
## References
## PR Checklist
* [x] Closes#2472
* [x] I work here
* [x] Tests added/passed - you better believe it
* [n/a] Requires documentation to be updated
## Validation Steps Performed
## Summary of the Pull Request
I believe this fixes#3861 but I honestly don't know how to test that part of the code. Just from reading the issue description that @dhowett-msft provided.
## References
## PR Checklist
* [x] Closes#3861
* [x] I work here
* [ ] Are there tests for this?
* [n/a] Requires documentation to be updated
## Validation Steps Performed
Really none, I just built it and :fingers_crossed:
<!-- 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
Every lambda capture in `Tab` and `TerminalPage` has been changed from capturing raw `this` to `std::weak_ptr<Tab>` or `winrt::weak_ref<TerminalPage>`. Lambda bodies have been changed to check the weak reference before use.
Capturing raw `this` in `Tab`'s [title change event handler](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/Tab.cpp#L299) was the root cause of #3776, and is fixed in this PR among other instance of raw `this` capture.
The lambda fixes to `TerminalPage` are unrelated to the core issue addressed in the PR checklist. Because I was already editing `TerminalPage`, figured I'd do a [weak_ref pass](https://github.com/microsoft/terminal/issues/3776#issuecomment-560575575).
<!-- 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#3776, potentially #2248, likely closes others
* [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: #3776
<!-- 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
`Tab` now inherits from `enable_shared_from_this`, which enable accessing `Tab` objects as `std::weak_ptr<Tab>` objects. All instances of lambdas capturing `this` now capture `std::weak_ptr<Tab>` instead. `TerminalPage` is a WinRT type which supports `winrt::weak_ref<TerminalPage>`. All previous instance of `TerminalPage` lambdas capturing `this` has been replaced to capture `winrt::weak_ref<TerminalPage>`. These weak pointers/references can only be created after object construction necessitating for `Tab` a new function called after construction to bind lambdas.
Any anomalous crash related to the following functionality during closing a tab or WT may be fixed by this PR:
- Tab icon updating
- Tab text updating
- Tab dragging
- Clicking new tab button
- Changing active pane
- Closing an active tab
- Clicking on a tab
- Creating the new tab flyout menu
Sorry about all the commits. Will fix my fork after this PR! 😅
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Attempted to repro the steps indicated in issue #3776 with the new changes and failed. When before the changes, the issue could consistently be reproed.
RTF data is now copied to the clipboard. The clipboard format name
used for RTF data is `Rich Text Format`.
Refactored some code in `Clipboard.cpp` so that the code for setting
data to the clipboard is re-used. Also, renamed parameter
`fAlsoCopyHtml` to `fAlsoCopyFormatting` to make it more generic.
Tested by copying text from console to WordPad. Also verified that
HTML copy is not regressed by copying to Word.
Closes#3560.
# Summary of the Pull Request
When a horizontal tab ('\t') is output on the last column of the screen, the current implementation moves the cursor position to the start of the next line. However, the DEC STD 070 manual specifies that a horizontal tab shouldn't move past the last column of the active line (or the right margin, if we supported horizontal margins). This PR updates the forward tab implementation, to prevent it wrapping onto a new line when it reaches the end of a line.
# Detailed Description of the Pull Request / Additional comments
Originally the SCREEN_INFORMATION::GetForwardTab method had a condition which handled a tab at the end of the line as a special case, moving the cursor to the start of the next line. I've simply removed that condition, so an end-of-line tab is handled the same way as any other position (in this case it will just leaves the cursor where it is).
While testing, though, I found that there were circumstances where you could have tab stops greater than the width of the screen, and when that happens, a tab can still end up wrapping onto the next line. To fix that I had to add an additional check to make sure the tab position was always clamped to the width of the buffer.
With these fixes in place, a tab control should now never move off the active line, so I realised that the DoPrivateTabHelper function could be optimized to calculate all of the tab movements in advance, and then only make a single call to AdjustCursorPosition with the final coordinates. This change is not strictly necessary, though, so it can easily be reverted if there are any objections.
Regarding backwards compatibility, note that the GetForwardTab method is only used in two places:
when handling a tab character in the WriteCharsLegacy function, but this only applies in VT mode (see here).
when handling the CHT escape sequence in the DoPrivateTabHelper function, and obviously an escape sequence would also only be applicable in VT mode.
So this change should have no effect on legacy console applications, which wouldn't have VT mode activated.
# Validation Steps Performed
I've added another step to the TestGetForwardTab test which makes sure that a horizontal tab won't wrap at the end of a line.
I've also confirmed that this fixes the last remaining issue in the Test of autowrap in Vttest (pages 3 and 4 of the Test of cursor movements). Although I should note that this only works in conhost.
## Summary of the Pull Request
Updates MUX to the latest pre-release version. This prerelease has a fix for a certain `E_LAYOUTCYCLE` bug in the TabView that was causing an untold number of crashes for us.
Thanks again @teaP!
## PR Checklist
* [x] Closes#3303
* [x] Closes#2277
* [x] I work here
* [n/a] Tests added/passed
* [n/a] Requires documentation to be updated
First, my changes to `build.bat`:
1. `Build.bat` now looks for Visual Studio 2019 too.
2. `Build.bat` now ensures that the linker places ColorTool.exe into
`\debug` or `\release` folders, not `\debug\net471` or
`\release\net471`.
3. `Build.bat` is now smarter in its search. It determines the operating
system's CPU architecture before deciding whether to search in "Program
Files (x86)" or "Program Files".
Second, my changes to the help text displayed to the user:
1. The help text now makes it clear that some switches cannot be used
with certain others.
2. Some typos are fixed. e.g. "ct" to "ColorTool" (it took me two hours
to figure this one out!) and "schemename" to "scheme name".
I've made a minor change to the order of `switch (arg)` in Program.cs
too, to ensure that the terminating switches are analyzed first. This
way, there will be fewer surprises if the user supplies malformed input.
But feel free to reject this one.
# Unresolved issues
`Build.bat` is inherently faulty. On a pristine computer, a user cannot
just install the latest version of Microsoft Build Tool and run
`build.bat` to build ColorTool. The reason is the absence of certain
NuGet packages. Either NuGet or Visual Studio must download the
dependencies first.
# Validation Steps Performed
Since the changes to the code are minor, I don't know what test I can
possibly devise, other than compiling it and seeing that it works.
## Summary of the Pull Request
Fixes the LocalTests that should work. This does _not_ fix the TabTests.
## PR Checklist
* [x] Closes#3536
* [x] I work here
* [x] This is literally making the tests work
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
In a previous PR, we broke unpackaged activation. This had the unintended side-effect of breaking these tests. The solution here is to run these unittests _packaged_.
This also makes things a little bit better for the TabTests, but doesn't fix them (inexplicably). A mail thread internally is tracking the progress of fixing those 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
Fixed the noexcept specifier on `GetGuid`, and corrected `FindProfile` and `FindGuid` so they don't throw. Also, adjusted `SettingsTests` to reflect these changes.
<!-- 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#3763
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed updated a test group in `SettingsTests`
* [ ] 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: #3763
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
The `noexcept` specifier on `GetGuid` was not removed when `Profile` was [updated](https://github.com/microsoft/terminal/issues/3763#issuecomment-559497094) to `std::optional<GUID>`. This PR fixes that and modifies two helper functions `FindProfile` and `FindGuid` in `CascadiaSettings` to work correctly if `GetGuid` does throw.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Updated the `TestHelperFunctions` test group in `SettingsTests` and made sure the tests pass.
<!-- 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
Replaced a `gsl::narrow` call to `gsl::narrow_cast` call. The `gsl::narrow` call used to throw when the user had custom display scaling due to a bad comparison between floating point values.
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
Possible other [startup crashes](https://github.com/microsoft/terminal/issues/3749#issuecomment-559900267). I'll update this as they're found.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#3749, likely #3747
* [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: #3749
<!-- 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
It's a one line fix. If you want more context, here's the [full description](https://github.com/microsoft/terminal/issues/3749#issuecomment-559911062) of the problem.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Set my machine to a custom scaling and opened a fixed build of the WT. My WT started up without crashing and continued to operate without issues (including maximizing, minimizing, and fullscreen toggle).
<!-- 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 updates the TitleBar buttons to be more consistent with other Windows apps.
Current buttons are a tiny bit smaller as compared to Chrome/Credge/Settings:
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] CLA signed
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already.
<!-- 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 PR changes the PointerHover Background of the close button on the TitleBar to match other Windows apps, from "#ff0000" to "#e81123". Also, the button width has been changed to 46 to be the same as other windows apps.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
------------------------------------------
* Fix Close Button Color
Changed the color of the Close Button on mouse hover from Red to "#e81123" which is the color used by other uwp apps.
* Updated Button Width
Changed the button width to be consistent with other uwp apps.
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Adds a simple helper function to look up the GUID associated with a profile name.
<!-- 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#3680
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed. Yes, new test group in `SettingsTests`!
* [ ] 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: #3680
<!-- 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
Very simple function, for-each through profiles checking for a match. Returns the associated GUID if found, else returns the null GUID.
This function is marked as `noexcept` to comply with assumption made by other `CascadiaSettings` functions that [`Profiles::GetGuid`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalApp/Profile.cpp#L141) does not throw, despite it throwing.
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
The new function is simple and can be visually validated, but added tests regardless. A new test group was added in `SettingsTests` called `TestHelperFunctions` to validate the new function `FindGuid` and older function `FindProfile`. This test group could be used to validate more helper functions in `CascadiaSettings` as they're added. The new test group passes after running `te.exe TerminalApp.LocalTests.dll`.
--------------------------------------------
* Added FindGuid helper function
* Style change
* Tests for FindGuid and FindProfile
* Fixed code format?
* Code format guess
No feedback from the Azure pipeline
* optional<GUID> fix
* Updated function desc
## Summary of the Pull Request
We already have "splitHorizontal" and "splitVertical", but those will both be deprecated in favor of "splitPane" with arguments.
Currently, there's one argument: "style", which is one of "vertical" or "horizontal."
## References
This is being done in pursuit of supporting #607 and #998. I don't really want to lob #998 in with this one, since both that and this are hefty enough PRs even as they are. (I have a branch for #998, but it needs this first)
This will probably conflict with #3658
## PR Checklist
* [ ] Doesn't actually close anything, only enables #998
* [x] I work here
* [ ] Tests added/passed - yea okay no excuses here
* [x] Requires documentation to be updated
## Validation Steps Performed
Added new keybindings with the args - works
Tried the old keybindings without the args - still works
---------------------------------------
* Add a 'splitPane' keybinding that can be used for splitting a pane either vertically or horizontally
* Update documentation too
* Good lord this is important
* Add a test too, though I have no idea if it works
* "style" -> "split"
* pr comments from carlos
## Summary of the Pull Request
Moves all the code responsible for dispatching an `ActionAndArgs` to it's own class, `ShortcutActionDispatch`. Now, the `AppKeyBindings` just uses the single instance of a `ShortcutActionDispatch` that the `TerminalPage` owns to dispatch events, without the need to re-attach the event handlers every time we reload the settings.
## References
This is something I originally did as a part of #2046.
I need this now for #607.
It's also a part of work for #3475
## PR Checklist
* [x] This is a bullet point within #3475
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
With this change, we'll be able to have other things dispatch `ShortcutAction`s easily, by constructing an `ActionAndArgs` and just passing it straight to the `ShortcutActionDispatch`.
## Validation Steps Performed
Ran the Terminal, tried out some keybindings, namely <kbd>Ctrl+c</kbd> for copy when there is a selection, or send `^C` when there isn't. That still works.
Reloading settings also still works.
-----------------------------------------------
* Move action handling to it's own class separate from AKB. This is the first checkbox in #3475
(cherry picked from commit 696726b571d3d1fdf1d59844c76e182fc72cb2ea)
* clean up doc comments
