Add support for setting a tab's color at runtime w/ context menu #3789
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I was having code formatting problems too. @greg904 pointed out a tool for code formatting in the repo. |
|
@mkitzan Thanks, mate, yeah, I should've read the docs. |
|
(Warning: we may be a little slow here. We're trying to get through our pull request backlog 😄 thanks for understanding!) |
DHowett-MSFT
reviewed
Dec 4, 2019
No probs, thank you for getting back to me, most kind of you :) |
|
Okay so this is incredible. I'm having a lot of fun playing with this already. The live updating with the color picker is also awesome Looking forward to getting into the code :) |
zadjii-msft
requested changes
Dec 12, 2019
src/cascadia/TerminalApp/Tab.cpp
Outdated
|
|
||
| chooseColorMenuItem.Click([this](auto&&, auto&&) { | ||
| _tabColorPickup.ShowAt(_tabViewItem); | ||
| }); |
There was a problem hiding this comment.
There was a problem hiding this comment.
(though we might need to do this work outside the ctor - if you pull, the Tab::BindEventHandlers method is called at the right time
There was a problem hiding this comment.
I will change it. I'm not 100% what do you mean with your last sentence about Tab::BindEventHandlers but I'm going to check the code.
There was a problem hiding this comment.
Oh you definitely got it right. Thanks!
src/cascadia/TerminalApp/Tab.cpp
Outdated
| closeTabMenuItem.Icon(closeSymbol); | ||
|
|
||
| closeTabMenuItem.Click([this](auto&&, auto&&) { | ||
| ClosePane(); |
There was a problem hiding this comment.
If you have multiple panes in a tab, this only closes the focused pane, right? We probably need a better way to close all the panes
There was a problem hiding this comment.
Yes, this closes the focused pane. I thought it was ok, if I can close a single pane. What do you mean precisely?
There was a problem hiding this comment.
So we can probably leave this for a follow up, but I'd presume that "Close" means "Close Tab" and not "Close Pane". "Close Tab" would close all the panes in the tab, while "Close Pane" only closes the active one. Maybe we could have it just display "Close" if there's only one Pane, and display both "Close Tab" and "Close Pane" if there are multiple tabs?
I think this is fine for now, and something we can address in a follow up.
ghost
added
the
Needs-Author-Feedback
zadjii-msft
added
Area-User Interface
ghost
added
the
No-Recent-Activity
|
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment. |
ghost
removed
Needs-Author-Feedback
|
Hello, guys, I tried to fix the things noted above but this lead to Terminal crashing at startup, when the window title is set. I'm not sure what I've managed to screw up exactly, but currently I'm on a vacation and won't be able to check everything thoroughly this year. Just wanted to let you know. Merry Christmas or whatever you are celebrating and enjoy the holidays. |
|
Could the profile JSON get hooked into this? |
|
@schorrm That's not a terrible idea, but I think it's honestly better suited for #3327. I've got a spec in progress for that issue which should be coming later this month, which should address specifically that request. @gbaychev I think I know what's cause - I ran into something similar in #4032 (or one of my other branches before the new year). I think you need to change terminal/src/cascadia/TerminalApp/TerminalPage.cpp Lines 914 to 919 in 23f2cd2 If you want to push your updates, I can take a look myself and make sure. |
- remove redundant files - disable the alpha settings in the color picker - move the winrt/Windows.UI.Popups.h to pch.h - move the VisualState switching code to a helper method - use weak ptr in the color pickup flyout - use weak_ptr in lambdas instead of [this]
|
Alright, most of the stuff is addressed. The weird crash was because I was trying to get a weak ptr in the constructor (what @zadjii-msft mentioned in a comment above). I made the What is left is the updating of the status bar, will do it ASAP. |
Co-authored-by: Josh Soref <[email protected]>
New misspellings found, please review:
To accept these changes, run the following commands✏️ Contributor please read this
See the
|
…into HEAD # Conflicts: # src/cascadia/TerminalApp/ColorHelper.cpp
New misspellings found, please review:
To accept these changes, run the following commands✏️ Contributor please read this
See the
|
zadjii-msft
added
the
AutoMerge
ghost
merged commit
2 tasks
zadjii-msft
added a commit
that referenced
this pull request
May 4, 2020
This was referenced May 5, 2020
DHowett-MSFT
pushed a commit
that referenced
this pull request
May 5, 2020
This is actually related to another issue we have, #3917. I think if the system is set to "Dark" theme, but the app is set to light theme, then the brush lookup in `_ClearNewTabButtonColor` still returns to us the dark theme brushes. Fortunately, since we're not actually setting the color of the new tab button anymore, we can just remove the call to that method now, and loop back on it later. ## References * regressed in #3789 * related to #3917 ## PR Checklist * [x] Closes #5741
This was referenced May 6, 2020
jelster
pushed a commit
to jelster/terminal
that referenced
this pull request
May 28, 2020
This is actually related to another issue we have, microsoft#3917. I think if the system is set to "Dark" theme, but the app is set to light theme, then the brush lookup in `_ClearNewTabButtonColor` still returns to us the dark theme brushes. Fortunately, since we're not actually setting the color of the new tab button anymore, we can just remove the call to that method now, and loop back on it later. ## References * regressed in microsoft#3789 * related to microsoft#3917 ## PR Checklist * [x] Closes microsoft#5741
ghost
pushed a commit
that referenced
this pull request
May 28, 2020
## Summary of the Pull Request Adds support for setting, from the UI, a runtime override for the tab title text. The user can use this to effectively "rename" a tab. If set, the tab will _always_ use the runtime override string. If the user has multiple panes with different titles in a pane, then the tab's override text will be used _regardless_ of which pane was focused when the tab was renamed. The override text can be removed by just deleting the entire contents of the box. Then, the tab will revert to using the terminal's usual title. ## References * Wouldn't be possible without the context menu from #3789 * Focus doesn't return to the active terminal after hitting <kbd>enter</kbd>/<kbd>esc</kbd>, but that's tracked by #5750 ## PR Checklist * [x] Closes #1079 * [x] I work here * [ ] Tests added/passed * [ ] Requires documentation to be updated ## TODO * [x] `Tab::SetTabText` might be able to be greatly simplified/removed? * [x] I'm _pretty sure_ if they set an override title, we won't bubble that up to set the window title. * [x] I'm unsure how this behaves when the terminal's title changes _while_ the TextBox is visible. I don't think it should change the current contents of the box, but it might currently. * [ ] **for discussion**: If the user doesn't actually change the text of the tab, then we probably shouldn't set the override text, right? - EX: if they open the box and the text is "cmd", and immediately hit <kbd>enter</kbd>, then run `title foo`, should the text change to "foo" or stay "cmd"? ## Detailed Description of the Pull Request / Additional comments        
ghost
pushed a commit
that referenced
this pull request
Jun 5, 2020
## Summary of the Pull Request When we select a color for the tab, we update the foreground color of the text so that it maintains acceptable contrast with the new tab color. However, we weren't also updating the foreground color of the close button. This is understandable though, because apparently this wasn't fixable until MUX 2.4 arrived. I'm not a XAML expert, but I know that setting this key only works when we're using MUX 2.4, so I'm assuming something about the TabView implementation changed in that release. _This PR is marked as a draft until #5778 is merged, then I'll re-target to master._ ## References * #5778 - PR to move to MUX 2.4 * This bug was introduced with the tab color picker in #3789 ## PR Checklist * [x] Closes #5780 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed A light tab color:  A dark tab color: 
This was referenced Jun 12, 2020
|
🎉 Handy links: |
ghost
mentioned this pull request
Closed
ThunderPower
added a commit
to ThunderPower/terminal
that referenced
this pull request
Sep 5, 2020
* Draw the cursor underneath text, and above the background (#6337) ## Summary of the Pull Request  This is the plan that @miniksa suggested to me. Instead of trying to do lots of work in all the renderers to do backgrounds as one pass, and foregrounds as another, we can localize this change to basically just the DX renderer. 1. First, we give the DX engine a "heads up" on where the cursor is going to be drawn during the frame, in `PrepareRenderInfo`. - This function is left unimplemented in the other render engines. 2. While printing runs of text, the DX renderer will try to paint the cursor in `CustomTextRenderer::DrawGlyphRun` INSTEAD of `DxEngine::PaintCursor`. This lets us weave the cursor background between the text background and the text. ## References * #6151 was a spec in this general area. I should probably go back and update it, and we should probably approve that first. * #6193 is also right up in this mess ## PR Checklist * [x] Closes #1203 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments * This is essentially `"cursorTextColor": "textForeground"` from #6151. * A follow up work item is needed to add support for the current behavior, (`"cursorTextColor": null`), and hooking up that setting to the renderer. * Add a test to ensure support for trailing commas in `settings.json` (#6312) ## Summary of the Pull Request Adds support for trailing commas in our json files. ## References * Enabled due to the excellent work over in https://github.com/open-source-parsers/jsoncpp/pull/1098 ## PR Checklist * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated * Improve support for VT character sets (#4496) This PR improves our VT character set support, enabling the [`SCS`] escape sequences to designate into all four G-sets with both 94- and 96-character sets, and supports invoking those G-sets into both the GL and GR areas of the code table, with [locking shifts] and [single shifts]. It also adds [`DOCS`] sequences to switch between UTF-8 and the ISO-2022 coding system (which is what the VT character sets require), and adds support for a lot more characters sets, up to around the level of a VT510. [`SCS`]: https://vt100.net/docs/vt510-rm/SCS.html [locking shifts]: https://vt100.net/docs/vt510-rm/LS.html [single shifts]: https://vt100.net/docs/vt510-rm/SS.html [`DOCS`]: https://en.wikipedia.org/wiki/ISO/IEC_2022#Interaction_with_other_coding_systems ## Detailed Description of the Pull Request / Additional comments To make it easier for us to declare a bunch of character sets, I've made a little `constexpr` class that can build up a mapping table from a base character set (ASCII or Latin1), along with a collection of mappings for the characters the deviate from the base set. Many of the character sets are simple variations of ASCII, so they're easy to define this way. This class then casts directly to a `wstring_view` which is how the translation tables are represented in most of the code. We have an array of four of these tables representing the four G-sets, two instances for the active left and right tables, and one instance for the single shift table. Initially we had just one `DesignateCharset` method, which could select the active character set. We now have two designate methods (for 94- and 96- character sets), and each takes a G-set number specifying the target of the designation, and a pair of characters identifying the character set that will be designated (at the higher VT levels, character sets are often identified by more than one character). There are then two new `LockingShift` methods to invoke these G-sets into either the GL or GR area of the code table, and a `SingleShift` method which invokes a G-set temporarily (for just the next character that is output). I should mention here that I had to make some changes to the state machine to make these single shift sequences work. The problem is that the input state machine treats `SS3` as the start of a control sequence, while the output state machine needs it to be dispatched immediately (it's literally the _Single Shift 3_ escape sequence). To make that work, I've added a `ParseControlSequenceAfterSs3` callback in the `IStateMachineEngine` interface to decide which behavior is appropriate. When it comes to mapping a character, it's simply an array reference into the appropriate `wstring_view` table. If the single shift table is set, that takes preference. Otherwise the GL table is used for characters in the range 0x20 to 0x7F, and the GR table for characters 0xA0 to 0xFF (technically some character sets will only map up to 0x7E and 0xFE, but that's easily controlled by the length of the `wstring_view`). The `DEL` character is a bit of a special case. By default it's meant to be ignored like the `NUL` character (it's essentially a time-fill character). However, it's possible that it could be remapped to a printable character in a 96-character set, so we need to check for that after the translation. This is handled in the `AdaptDispatch::Print` method, so it doesn't interfere with the primary `PrintString` code path. The biggest problem with this whole process, though, is that the GR mappings only really make sense if you have access to the raw output, but by the time the output gets to us, it would already have been translated to Unicode by the active code page. And in the case of UTF-8, the characters we eventually receive may originally have been composed from two or more code points. The way I've dealt with this was to disable the GR translations by default, and then added support for a pair of ISO-2022 `DOCS` sequences, which can switch the code page between UTF-8 and ISO-8859-1. When the code page is ISO-8859-1, we're essentially receiving the raw output bytes, so it's safe to enable the GR translations. This is not strictly correct ISO-2022 behavior, and there are edge cases where it's not going to work, but it's the best solution I could come up with. ## Validation Steps Performed As a result of the `SS3` changes in the state machine engine, I've had to move the existing `SS3` tests from the `OutputEngineTest` to the `InputEngineTest`, otherwise they would now fail (technically they should never have been output tests). I've added no additional unit tests, but I have done a lot of manual testing, and made sure we passed all the character set tests in Vttest (at least for the character sets we currently support). Note that this required a slightly hacked version of the app, since by default it doesn't expose a lot of the test to low-level terminals, and we currently identify as a VT100. Closes #3377 Closes #3487 * Reflect OS build fixes on 7b489128ac back to inbox Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp 677d15a4e298a0e1e3ce093bc1dff8d832e3c2b1 Related work items: #26765368 * Allow Ctrl+Alt <> AltGr aliasing to be disabled (#6212) ## Summary of the Pull Request Some people wish to use Ctrl+Alt combinations without Windows treating those as an alias for AltGr combinations. This PR adds a new `altGrAliasing` setting allowing one to control this behavior. ## PR Checklist * [x] Closes #6211 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [x] Manual testing * [x] Requires documentation to be updated: https://github.com/MicrosoftDocs/terminal/issues/50 * [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: #xxx ## Validation Steps Performed * Choose a German keyboard layout * Using `showkey -a` ensured that both `Ctrl+Alt+Q/E` and `AltGr+Q/E` produce `@/€` * Added `"altGrAliasing": false` to the WSL profile * Using `showkey -a` ensured `Ctrl+Alt+Q/E` now produces `^[^Q/E` while `AltGr+Q/E` continues to produce `@/€` * Move to Microsoft.UI.Xaml 2.4.0 (#5778) This brings support for "Compact" tab sizing, which compresses all inactive tabs to just the size of their icons plus the close button. Neat! It also just keeps us generally up-to-date and good citizens. * Spec for tab tear off and default app (#2080) docs have no build, cleaning out draft spec from PR queue * First draft of a spec for panes with non-terminal content (#1080) Co-authored-by: Michael Niksa <[email protected]> putting outdated draft spec into drafts folder and closing PR. CI has no function on docs. * fix spell things I forced into master. * Add support for `compact` tab sizing (#5787) ## Summary of the Pull Request Really couldn't be more starightforward. MUX 2.4 added support for "compact" sized tabs. This PR (targeting the 2.4 PR currently, will move to `master` when that merges) enables users to specify `"tabWidthMode": "compact"` in their global settings to get this behavior. ## References * #5778 - PR to move to MUX 2.4 * [microsoft-ui-xaml#2016](https://github.com/microsoft/microsoft-ui-xaml/pull/2016) - the MUX PR for compact tab sizing. * #597 - Tab sizing options? ## PR Checklist * [x] I don't think we have an issue for this, though I could be wrong. * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments In this screenshot, I'm hovering over tab 2, but the ubuntu tab is focused:  In this screenshot, tab 2 is focused:  * spelling I * spelling II * Last round of master spelling fixes * Update the tab's close button color to match the tab text color (#5789) ## Summary of the Pull Request When we select a color for the tab, we update the foreground color of the text so that it maintains acceptable contrast with the new tab color. However, we weren't also updating the foreground color of the close button. This is understandable though, because apparently this wasn't fixable until MUX 2.4 arrived. I'm not a XAML expert, but I know that setting this key only works when we're using MUX 2.4, so I'm assuming something about the TabView implementation changed in that release. _This PR is marked as a draft until #5778 is merged, then I'll re-target to master._ ## References * #5778 - PR to move to MUX 2.4 * This bug was introduced with the tab color picker in #3789 ## PR Checklist * [x] Closes #5780 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed A light tab color:  A dark tab color:  * Make the conversion from WORD to TextAttribute explicit (#6380) In Windows, we build with /Zc:wchar_t- (which makes wchar_t an unsigned short typedef.) This causes build breaks when we compare two wchar_t values (or a wchar_t and an enum class that's of type wchar_t) and the compiler decides that it might want to _promote them to TextAttribute_ before doing the comparison. * Reflect OS build fixes back from f90f3bf99 Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp 272dfa1c4ad5e4202c4c56f3db7a445dc0b003cf * Improve the legacy color conversions (#6358) This PR provides a faster algorithm for converting 8-bit and 24-bit colors into the 4-bit legacy values that are required by the Win32 console APIs. It also fixes areas of the code that were incorrectly using a simple 16-color conversion that didn't handle 8-bit and 24-bit values. The faster conversion algorithm should be an improvement for issues #783 and #3950. One of the main points of this PR was to fix the `ReadConsoleOutputAttribute` API, which was using a simplified legacy color conversion (the original `TextAttribute:GetLegacyAttributes` method), which could only handle values from the 16-color table. RGB values, and colors from the 256-color table, would be mapped to completely nonsensical values. This API has now been updated to use the more correct `Settings::GenerateLegacyAttributes` method. But there were also a couple of other places in the code that were using `GetLegacyAttributes` when they really had no reason to be working with legacy attributes at all. This could result in colors being downgraded to 4-bit values (often badly, as explained above), when the code was already perfectly capable of displaying the full 24-bits. This included the fill colors in the IME composer (in `ConsoleImeInfo`), and the construction of the highlighting colors in the color search/selection handler (`Selection::_HandleColorSelection`). I also got rid of some legacy attribute code in the `Popup` class, which was originally intended to update colors below the popup when the settings changed, but actually caused more problems than it solved. The other major goal of this PR was to improve the performance of the `GenerateLegacyAttributes` method, since the existing implementation could be quite slow when dealing with RGB values. The simple cases are handled much the same as they were before. For an `IsDefault` color, we get the default index from the `Settings::_wFillAttribute` field. For an `IsIndex16` color, the index can just be returned as is. For an `IsRgb` color, the RGB components are compressed down to 8 bits (3 red, 3 green, 2 blue), simply by dropping the least significant bits. This 8-bit value is then used to lookup a representative 16-color value from a hard-coded table. An `IsIndex256` color is also converted with a lookup table, just using the existing 8-bit index. The RGB mapping table was calculated by taking each compressed 8-bit color, and picking a entry from the _Campbell_ palette that best approximated that color. This was done by looking at a range of 24-bit colors that mapped to the 8-bit value, finding the best _Campbell_ match for each of them (using a [CIEDE2000] color difference calculation), and then the most common match became the index that the 8-bit value would map to. The 256-color table was just a simpler version of this process. For each entry in the table, we take the default RGB palette value, and find it's closest match in the _Campbell_ palette. Because these tables are hard-coded, the results won't adjust to changes in the palette. However, they should still produce reasonable results for palettes that follow the standard ANSI color range. And since they're only a very loose approximation of the colors anyway, the exact value really isn't that important. That said, I have tried to make sure that if you take an RGB value for a particular index in a reasonable color scheme, then the legacy color mapped from that value should ideally match the same index. This will never be possible for all color schemes, but I have tweaked a few of the table entries to improve the results for some of the common schemes. One other point worth making regarding the hard-coded tables: even if we wanted to take the active palette into account, that wouldn't actually be possible over a conpty connection, because we can't easily know what color scheme the client application is using. At least this way the results in conhost are guaranteed to be the same as in the Windows Terminal. [CIEDE2000]: https://en.wikipedia.org/wiki/Color_difference#CIEDE2000 ## Validation Steps Performed This code still passes the `TextAttributeTests` that check the basic `GetLegacyAttribute` behaviour and verify the all legacy attributes roundtrip correctly. However, some of the values in the `RgbColorTests` had to be updated, since we're now intentionally returning different values as a result of the changes to the RGB conversion algorithm. I haven't added additional unit tests, but I have done a lot of manual testing to see how well the new algorithm works with a range of colors and a variety of different color schemes. It's not perfect in every situation, but I think it works well enough for the purpose it serves. I've also confirmed that the issues reported in #5940 and #6247 are now fixed by these changes. Closes #5940 Closes #6247 * Move all wildcards into targets or expand them (#6406) Wildcards are not allowed in toplevel ItemGroups in vcxproj; they must be generated by targets. We mostly use wildcards for pulling in PRI files that are dumped on disk by the translation tool. We don't want to check those in, so we can't expand references to them. To that end, I've introduced a new target that will take a list of folders containing resw files and expand wildcards under them. All[1] other wildcards have been moved into their respective targets _or_ simply expanded. [1]: Nothing has complained about the resource wildcards in CascadiaResources.build.items, so I haven't exploded it yet. Fixes #6214. * Remove the shell extension from directory backgrounds (#6415) We're removing this because of MSFT:24623699, which prevents us from being able to do the right thing when we're called on the background of a directory for a range of OS builds. #6414 will track re-adding this to the Terminal when the original issue is closed. * [x] closes #6245 * I work here * Add support for win32-input-mode to conhost, ConPTY, Terminal (#6309) Adds support for `win32-input-mode` to conhost, conpty, and the Windows Terminal. * The shared `terminalInput` class supports sending these sequences when a VT client application requests this mode. * ConPTY supports synthesizing `INPUT_RECORD`s from the input sent to it from a terminal * ConPTY requests this mode immediately on startup (if started with a new flag, `PSEUDOCONSOLE_WIN32_INPUT_MODE`) * The Terminal now supports sending this input as well, when conpty asks for it. Also adds a new ConPTY flag `PSEUDOCONSOLE_WIN32_INPUT_MODE` which requests this functionality from conpty, and the Terminal requests this by default. Also adds `experimental.input.forceVT` as a global setting to let a user opt-out of this behavior, if they don't want it / this ends up breaking horribly. ## Validation Steps Performed * played with this mode in vtpipeterm * played with this mode in Terminal * checked a bunch of scenarios, as outlined in a [comment] on #4999 [comment]: https://github.com/microsoft/terminal/issues/4999#issuecomment-628718631 References #4999: The megathread References #5887: The spec Closes #879 Closes #2865 Closes #530 Closes #3079 Closes #1119 Closes #1694 Closes #3608 Closes #4334 Closes #4446 * Remove parentheses from Preview and Dev build (#6418) ## Summary of the Pull Request Remove parentheses from the Preview and Dev build. Now they're called Windows Terminal Preview and Windows Terminal Dev Build respectively. Also removed them from other identifiers of Terminal for consistency. ## PR Checklist * [X] Closes #5974 * Upload Windows Terminal 2.0 roadmap (#6419) <!-- 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 Upload the roadmap for Windows Terminal 2.0 and link to it on the README. <!-- 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 * [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: #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 * wpf: add a .NET Core WPF Test project for the WPF Control (#6441) This commit introduces a new project that lets you F5 a working instance of the Wpf Terminal Control. To make the experience as seamless as possible, I've introduced another solution platform called "DotNet_x64Test". It is set to build the WPF projects for "Any CPU" and every project that PublicTerminalCore requires (including itself) for "x64". This is the only way to ensure that when you press F5, all of the native and managed dependencies get updated. It's all quite cool when it works. * Set tab title as early as possible (#6433) When opening a new tab, it takes a few milliseconds before title to appears. This PR makes it instantaneous. * Updated the Terminal so that it can load the title from the settings before it is initialized. * Load terminal settings in TermControl constructor before the terminal is initialized (see above). * Update Tab so that it sets the TabViewItem's title in the constructor (in Tab::_MakeTabViewItem) instead of waiting for the VT sequence to set the title (from what I understand). NOTE 1: there is a similar problem with the tabview icon which is not fixed by this PR. NOTE 2: This is only a problem with animations disabled because otherwise the title fades in so there is enough time for it to be set when it becomes visible. ## Validation I ran the terminal and opened a new tab. The title appears instantly. * Don't snap on input nor dismiss selection for just a modifier key (#6431) Does what it says on the label. Pure modifier keys weren't making it this far at all prior to #6309. This PR changes these methods to make sure that we only dismiss a selection or snap on input when the key pressed isn't a modifier key. ## References * regressed in #6309 ## PR Checklist * [x] Closes #6423 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed * Tried to repro this in the Terminal, couldn't anymore. * Don't send a manual F7 keyup (#6442) ## Summary of the Pull Request When someone asked "Do we need to send a F7 keyup too" in #6309, the right answer was actually _no_. Turns out that while XAML will eat the F7 key**down**, it _won't_ eat the F7 key**up**. ## References * regressed in #6309 ## PR Checklist * [x] Closes #6438 * [x] I work here * [ ] Tested manually * [n/a] Requires documentation to be updated ## Validation Steps Performed * Checked this with the debug tap * Tie up some A11y loose threads (#6417) This pull request moves WindowUiaProvider back into Win32 interactivity and deletes all mention of it from Windows Terminal. Terminal does not have a single toplevel window that requires Console-like UIA, as each Xaml control inside it is in charge of its own destiny. I've also merged `IUiaWindow` and `IConsoleWindow` back together, as well as `WindowUiaProviderBase` and `WindowUiaProvider`. Things look a lot more like they did before we tore them apart. ## PR Checklist * [x] Closes #3564 * [x] CLA * [x] Tests added/passed (manual) * [ ] Requires documentation to be updated * [x] I've discussed this with core contributors already ## Validation Carlos validated conhost and terminal on this branch. * WpfTest: Add an x86/Win32 build, make DPI aware (#6455) This matches more closely how Visual Studio uses the WPF control. It comes in the form of _another platform_ (sorry), `DotNet_x86Test`. * Improve perf by avoiding vector reallocation in renderer clusters and VT output graphics options (#6420) ## Summary of the Pull Request Caches vectors in the class and uses a new helper to opportunistically shrink/grow as viewport sizes change in order to save performance on alloc/free of commonly used vectors. ## PR Checklist * [x] Scratches a perf itch. * [x] I work here. * [x] wil tests added * [x] No add'l doc. * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments Two fixes: 1. For outputting lots of text, the base renderer class spent a lot of time allocating and freeing and reallocating the `Cluster` vector that adapts the text buffer information into render clusters. I've now cached this vector in the base render class itself and I shrink/grow it based on the viewport update that happens at the top of every frame. To prevent too much thrashing in the downward/shrink direction, I wrote the `til::manage_vector` helper that contains a threshold to only shrink if it asks for small enough of a size relative to the existing one. I used 80% of the existing size as the threshold for this one. 2. For outputting lots of changing colors, the VT graphics output engine spent a bunch of time allocating and reallocating the vector for `GraphicsOptions`. This one doesn't really have a predictable size, but I never expect it to get extremely big. So I just held it in the base class. ## Validation Steps Performed * [x] Ran the til unit test * [x] Checked render cluster vector time before/after against `big.txt` from #1064 * [x] Checked VT graphics output vector time before/after against `cacafire` Case | Before | After ---|---|---| `big.txt` |  |  `cacafire` |  |  * Reduce latency with DXGI 1.3 GetFrameLatencyWaitableObject (#6435) This pull request reduces input lag, especially with selection, by using `IDXGISwapChain2::GetFrameLatencyWaitableObject`. This is based on the [DXGI 1.3 documentation]. Excerpt from the [DXGI 1.3 improvement list]: > The following functionality has been added in Microsoft DirectX Graphics Infrastructure (DXGI) 1.3, which is included starting in Windows 8.1. Before, during rendering: 1. render frame 2. call `Present` on swap chain: 2.a. blocks until it can present 2.b. meanwhile, selection/text in terminal might have changed, but we're still using the frame that we rendered before blocking 2.c. presents After, during rendering: 1. block until we can present 2. render frame with latest data 3. call `Present` on swap chain: 3.a. present without blocking [DXGI 1.3 documentation]: https://docs.microsoft.com/en-us/windows/uwp/gaming/reduce-latency-with-dxgi-1-3-swap-chains [DXGI 1.3 improvement list]: https://docs.microsoft.com/en-us/windows/win32/direct3ddxgi/dxgi-1-3-improvements: * Open the system menu when user right clicks the drag bar (#6443) Related to #1375 ("Click/Right click icon should display Minimize/Maximize/Close menu") * Deps: Bump Newtonsoft.Json from 10.0.3 to 12.0.3 (#6369) Bump Newtonsoft.Json from 10.0.3 to 12.0.3 ## References Part of #5297 ## PR Checklist * [ ] Closes (none) * [x] CLA signed * [ ] Tests added/passed N/A * [ ] Requires documentation to be updated N/A * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Validation Steps Performed CI builds successfully * Fix the x86 build and re-enable x86 CI (#6467) This was a miss. * Add Mini-Spec for openSettings (#5915) The spec introduces a keybinding argument of 'target' to be able to open a specific settings file. When the Settings UI gets implemented, it will also become an option. Alternative designs were presented but the 'target' model was decided on. * Fix 3 different bugs in the WPF control (#6464) * [wpf] WM_KEYUP crashes on x64 #6444 - Turns out that doing the `(uint)lParam` cast worked fine for the keydowns, because the value of lParam usually didn't have super high-order bits set. That's not the case for keyups, where the 30th bit is _always_ set. This is fixed by explicitly getting the byte with the scancode in it. * [wpf] WM_KEYUP generates wrong value in Win32 input mode #6445 - This was fixed by basically the same thing as the above. * [wpf] WPF control crashes on startup trying to render cursor #6446 - This was a regression from #6337. I forgot to initialize the brush used to paint the cursor, because the UWP version always uses color (but the WPF one relies on the text foreground color). * Also adds a minor change to the WPF test app, so that the user can actually exit `win32-input-mode`. * #6337 regressed #6446 * #6309 regressed the other two. Closes #6444 Closes #6445 Closes #6446 * Finalize Command Palette Spec (#5674) ## Summary of the Pull Request This PR aims to move the command palette spec out of the draft state and into a finalized state for inclusion in the 2.0 version of the Windows Terminal. Notably, I've added sections regarding the ability to run `wt` commandlines using the Command Palette UI, something we hadn't considered in the original draft, because `wt` commandlines didn't land for like _4 months_ after this first draft. ## References * #2046 - the original command palette thread * #2193 - the original draft PR * #5400 - the new command palette megathread for WT 2.0, which I'll be updating with follow-up tasks as we work on implementing this. ## PR Checklist * [x] Specs #2046 * [x] I work here * [x] Is documentation ## Detailed Description of the Pull Request / Additional comments _read the spec_ * Pass <Alt> to the application (#6461) For mysterious reasons lost to the sands of time, XAML will _never_ pass us a VK_MENU event. This is something that'll probably get fixed in WinUI 3, but considering we're stuck on system XAML for the time being, the only way to work around this bug is to pass the event through manually. This change generalizes the F7 handler into a "direct key event" handler that uses the same focus and tunneling method to send different key events, and then uses it to send VK_MENU. ## Validation Steps Performed Opened the debug tap, verified that I was seeing alt key ups. Also used some alt keybindings to make sure I didn't break them. Closes #6421 * Throttle scrollbar updates in TermControl to ~one per 8ms (#4608) In addition to the below (original) description, this commit introduces a ThrottledFunc template that can throttle _any_ function. It applies that type to muffle updates to the scrollbar. --- Redo #3531 but without the bug that it caused (#3622) which is why it was reverted. I'm sorry if I explain this badly. If you don't understand a part, make sure to let me know and I will explain it better. ### Explanation How it worked before: `Terminal` signals that viewport changed -> `TermControl::_TerminalScrollPositionChanged` gets called on the terminal thread -> it dispatches work for later to be ran the UI thread to updates the scrollbar's values Why it's bad: * If we have many viewport changes, it will create a long stack of operations to run. Instead, we should just update the scroll bar with the most recent information that we know. * Imagine if the rate that the work gets pushed on the UI thread is greater than the rate that it can handle: it might freeze? * No need to be real time, we can wait just a little bit (8ms) to accumulate viewport changes before we actually change the scroll bar's value because it appears to be expensive (see perf below). Now: `Terminal` signals that viewport changed -> `TermControl::_TerminalScrollPositionChanged` gets called on the terminal thread -> it tells the `ScrollBarUpdater` about a new update -> the `ScrollBarUpdater` only runs one job (I don't know if that's the right term) on the UI thread at a time. If a job is already running but hasn't updated the scroll bar yet, it changes the setting in the already existing job to update the scroll bar with the new values. A job "waits" some time before doing the update to throttle updates because we don't need real time scroll bar updates. -> eventually, it updates the scroll bar If the user scrolls when a scroll bar update is pending, we keep the scroll bar's Maximum and Minimum but let the user choose its new Value with the `CancelPendingValueChange` method. ### Note Also I changed a little bit the code from the Terminal to notify the TermControl less often when possible. I tried to scroll with the scroll bar, with the mouse wheel. I tried to scroll while content is being outputted. I tried to reproduce the crash from #2248 without success (good). Co-authored-by: Leonard Hecker <[email protected]> Closes #3622 * Add keybinding arg to openSettings (#6299) ## Summary of the Pull Request Adds the `target` keybinding arg to `openSettings`. Possible values include: `defaultsFile`, `settingsFile`, and `allFiles`. ## References #5915 - mini-spec ## PR Checklist * [x] Closes #2557 * [x] Tests added/passed ## Detailed Description of the Pull Request / Additional comments Implemented as discussed in the attached spec. A new enum will be added for the SettingsUI when it becomes available. ## Validation Steps Performed Added the following to my settings.json: ```json { "command": "openSettings", "keys":... }, { "command": { "action": "openSettings" }, "keys":... }, { "command": { "action": "openSettings", "target": "settingsFile" }, "keys":... }, { "command": { "action": "openSettings", "target": "defaultsFile" }, "keys":... }, { "command": { "action": "openSettings", "target": "allFiles" }, "keys":... } ``` * Add fast path to til::bitmap::translate using bitshifts (#6493) This commit adds a fast path to `til::bitmap::translate`: use bit shifts when the delta is vertical. Performance while printing the content of a big file, with the patch from #6492 which hasn't been merged yet, in Release mode: Before: * translate represents 13.08% of samples in InvalidateScroll After: * translate represents 0.32% of samples in InvalidateScroll ## Validation Tests passed. * Clear cached runs after translate_y (#6501) "While re-reading the code, I found out that I forgot to do clear cached runs after translate_y in c360b7588ff8d389b49a4ed60cdee51401a5e172." * Reintroduce the check for VT_INPUT_MODE in AdaptDispatch (#6485) This commit reverts the removal of the "SSH hack" in #5383. It was originally added as a solution to #4911, when we realized that SSH would request the SS3 cursor key encoding but we weren't equipped to handle it. A number of folks have filed issues that, in summary, say "when I use SSH, I can't select/copy/paste text". It turns out that SSH will _also_ pass through requests for mouse input. Terminal dutifully responds to those requests, of course, by disabling mouse selection/copy/paste. SSH is **NOT** actually in VT_INPUT_MODE, so it will never receive the mouse messages. It's important to note that even with #376 fixed, we are still required to keep this check. With the closure of #376, we'll be able to convert VT mouse input back into Win32 mouse input for Win32 applications . . . but SSH also doesn't know how to handle Win32 mouse input. Fixes #6476. Fixes #6196. Fixes #5704. Fixes #5608. * Extract `ActionAndArgs::FromJson` into its own class (#6351) ## Summary of the Pull Request Pulls the `ActionAndArgs` deserializing into its own class, separate from `AppKeyBindings`. Some 2.0 features are going to need to re-use these actions in their json, so we'll want one unified way of deserializing them. ## References * Done primarily as part of the work on #2046/#5400/#5674 * Also related: #1571/#5888 * Will aggressively conflict with any open PRs that introduced keybindings (looking at #6299) ## PR Checklist * [x] Closes nothing, this is code refactoring * [x] I work here * [x] Current tests passed * [n/a] Requires documentation to be updated * Fix sending a NUL on alt key up (#6516) ## Summary of the Pull Request Make sure to set the scancode for the manual alt-up's we're sending. If you don't, then terminalInput in the conpty is going to treat that keypress as an actual NUL, and send that to the connected client. ## References * regressed in #6421 ## PR Checklist * [x] Closes #6513 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Tested `showkeys -a` * Only snap on key _downs_ (#6517) ## Summary of the Pull Request Prior to #6309, we'd only snap on input for non-modifier key_down_ events. #6423 fixed this for modifier keys, but didn't fix this for keyups. ## References * #6423 was an incomplete fix to this problem, which caused this regression ## PR Checklist * [x] Closes #6481 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated * Fix openSettings misspelling in defaults (#6520) Fixes #6486. * Remove the WinTelnetEngine (#6526) Nobody was using it. Discussed in #2661. * Spec: Advanced Tab Switcher (#3753) <!-- 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 is the spec for the Advanced Tab Switcher. This would allow the user to navigate through a vertical list of tabs through a UI, similar to those found in VSCode and Visual Studio. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References #1502: Feature Request: Advanced Tab Switcher #973: Ctrl+Tab toggling between two tabs <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Spec for #1502 * [x] CLA signed. * Introduce JsonUtilsNew as documented in #5875 (#6355) Read the [JsonUtils Spec] for more details. This pull request introduces the next version of JsonUtils. It is in a separate file for ease of review and testing. JsonUtilsNew will be renamed in a subsequent commit that rewrites our JSON deserializers. ### Implementer's Notes I went with telescoping exceptions for the key parsing code, because it's totally possible that you can be five keys deep and encounter a type error. This lets us encode information about all failures in the chain instead of just the topmost one. The original JsonUtilsNew code changed to use `decay` everywhere because the tests wouldn't compile. We want to treat `GetValue<const guid>` _the same as_ `GetValue<guid>`, and this lets us do so. `decay` is awesome. I've been developing this with a shim that redirects `JsonUtils.h` to `JsonUtilsNew.h`. I am not comfortable deleting the original until we've moved off of it, and that _will_ be the subject of a followup PR. ## Validation Steps Performed So many tests. [JsonUtils Spec]: https://github.com/microsoft/terminal/blob/master/doc/cascadia/Json-Utility-API.md Refs #2550 * Fixed #6377: TerminalCore::_altGrAliasing is undefined by default (#6571) ## Summary of the Pull Request Fixes #6377. `TerminalCore` does not initialize `_altGrAliasing`. The impact is minimized in WT because it defaults to `true` in higher layers. It's not initialized when WPF is driving. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes #6377 * [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 * Use early returns in TermControl::_KeyHandler (#6575) ## Summary of the Pull Request This PR changes `TermControl::_KeyHandler` to use early returns, which you can think of as "guard clauses". This has the benefit of a reduced nesting level, easier to understand control flow and opens op the way to more complex conditions. ## PR Checklist * [ ] Closes #xxx * [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 ## Validation Steps Performed Everything still works as expected. * doc: copyedit the roadmap a little bit (#6587) * Add 1.1 blog post to 2.0 roadmap (#6581) * Update PR template with link to docs (#6583) * Improved ATTR_ROW::ReplaceAttrs performance (#6573) ## Summary of the Pull Request Improve `ATTR_ROW::ReplaceAttrs` performance by only reserving the necessary capacity instead of resizing the new run. That way `TextAttributeRun`s are only instantiated once instead of twice. ## PR Checklist * [ ] Closes #xxx * [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 Performance could be further improved by directly moving `TextAttributeRun`s into the new vector, but I considered this out of scope for this PR. ## Validation Steps Performed CPU usage when running `cacafire` is slightly reduced. * Add schema check to PR template (#6599) This adds a check for updating the schema, and rewords the documentation checkbox to match the wording of the others. * Enable hot reload of renderer settings that aren't already hot reload capable (#6551) ## Summary of the Pull Request ## PR Checklist * [x] Closes #3927 * [x] I work here. * [x] Tested manually. * [x] Requires documentation to be updated: (generate doc bug here) * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments - I found four settings that weren't hot reloadable with the 3927 comment above them: 1. Experimental retro terminal effect 2. Experimental software rendering 3. Experimental full repaint rendering 4. Antialiasing settings for text I made them all hot reloadable by telling the `TermControl` to propagate them on settings change to the `DxEngine`. Then I set up the `DxEngine` inside the setters to only set them if they changed. And if they do change, to trigger a full repaint and/or a complete drop and recreate of the entire DX device chain (as would happen if it were lost for another reason like a user-mode graphics failure, disconnected display, etc.) I made the boolean an atomic because the settings can be coming in off of another thread (the XAML eventing one) and the renderer is picking the status up on its thread at the top of the BeginPaint frame. ## Validation Steps Performed - [x] Opened it up and toggled all the settings while staring at PowerShell - [x] Opened it up and toggled all the settings while staring at something intensive like a `cacafire` fire * Improve bitmap::_calculateArea performance (#6572) `bitmap::_calculateArea` performance can be improved by leveraging the optimized `find_first`/`find_next` methods instead of iterating through the bitmap manually. * Use D2DDeviceContext and friends over D2DRenderTarget (#6527) I was told that the DeviceContext version supercedes the RenderTarget one. This moves us to it so we can gain access to a higher level of control over the various pieces in our pipeline as we continue to evolve the renderer. The underlying motivation here is to potentially use a `ID2D1CommandList` to batch our commands and run them all later outside the lock. That can only really be done with this more granular level of control over the pipeline. So this moves to that in a single step that is easily findable in history should we have problems I discussed this with @NiklasBorson of the Direct2D/DirectWrite team as well as with @DHowett before doing it. ## Validation - [x] Checked docs to make sure that these work on Windows 7 with Platform Update - [x] Manual smoke test real quick - [ ] Try running on Win7 + Platform Update after change - [x] Probably do more than just a smoke test manually or otherwise Closes #6525 * Always use the dark window borders (#6624) * Recycle assorted rendering components to accelerate drawing (#6483) This saves an awful lot of construction/destruction and memory allocation, especially with text that changes a lot (see: cacafire). Three things: 1. Recycling the text layouts. This holds onto the `CustomTextLayout` so all the things that don't change related to drawing targets and whatnot aren't freed and recreated every frame. 2. Reordering the runs in place. This saves a vector allocation/copy/delete every time OrderRuns is called. They can be rearranged in place. 3. Only clip once per row. This reduces the clip push/pop to only one time per row. Since we're always redrawing an entire row at a time, this saves a lot of alloc/free of the clip frame, dramatically reduces queued commands, and makes less work on the flush since clipping requires staging the drawing and then bringing it back to the main surface. * Double-click a tab to rename it (#6628) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request When the user double clicks on a tab, show the tab rename box as if they right clicked on the tab and clicked on "Rename". <!-- 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 #6600 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments I added a handler for the `DoubleTapped` event on the tab view item when we are constructing it for the tab (in `Tab::_MakeTabViewItem`). The code for that handler was copied the "rename tab menu item" click handler. I did not extract the code into a member function because it is very short (only 2 lines of code) and only used twice so it is not worth it IMO. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed * Spec for unified keybindings and commands, and synthesized action names (#6532) ## Summary of the Pull Request This is another iteration on the Command Palette spec, from #5674. These were some ideas that were tossed around by @DHowett, @cinnamon-msft and myself, formalized here. I proposed this as an addendum to the original spec, since I think the first made sense atomically, and this only makes sense as a set of changes to the original. I didn't want to go hacking up the original doc to add this set of changes. **There are two proposals in this spec - they should be viewed as two atomic units. They can be accepted or rejected independently. I'm suggesting we approve both. They work _together_. I'm realizing now that this is worded confusingly, and it's on me to fix that.** ## PR Checklist * [x] Another spec in the #2046 / #5400 saga * [x] I work here * [x] _is a doc_ > ## Abstract > > This document is intended to serve as an addition to the [Command Palette Spec]. > While that spec is complete in it's own right, subsequent discussion revealed > additional ways to improve the functionality and usability of the command > palette. This document builds largely on the topics already introduced in the > original spec, so readers should first familiarize themselves with that > document. > > One point of note from the original document was that the original specification > was entirely too verbose when defining both keybindings and commands for > actions. Consider, for instance, a user that wants to bind the action "duplicate > the current pane". In that spec, they need to add both a keybinding and a > command: > > ```json > { > "keybindings": [ > { "keys": [ "ctrl+alt+t" ], "command": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" } }, > ], > "commands": [ > { "name": "Duplicate Pane", "action": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" }, "icon": null }, > ] > } > ``` > > These two entries are practically the same, except for two key differentiators: > * the keybinding has a `keys` property, indicating which key chord activates the > action. > * The command has a `name` property, indicating what name to display for the > command in the Command Palette. > > What if the user didn't have to duplicate this action? What if the user could > just add this action once, in their `keybindings` or `commands`, and have it > work both as a keybinding AND a command? > * Optimize booleans (#6548) <!-- 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 Many places in this codebase has an equality comparison to the boolean FALSE. This adds unneeded complexity as C and C++ has a NOT operand for use of these in if statements. This makes the code more readable in those areas. <!-- 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] 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 <!-- 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 One boolean being compared to FALSE was only used once, with the boolean name being "b", so it is better off not existing at all. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Unit Testing passed, compiler refactoring * Replace std::map with std::unordered_map (#6640) Replace std::map with std::unordered_map when the order doesn't matter and hash functions are provided. Simple optimizations, but I expect the performance should be strictly better, especially for CodepointWidthDetector.hpp. * Update _TerminalCursorPositionChanged to use ThrottledFunc (#6492) * Update _TerminalCursorPositionChanged to use ThrottledFunc. * Rename previous ThrottledFunc to ThrottledArgFunc because now ThrottledFunc is for functions that do not take an argument. * Update ThrottledFunc and ThrottledArgFunc to accept a CoreDispatcher on which the function should be called for convenience. * Don't use coroutines/winrt::fire_and_forget in ThrottledFunc/ThrottledArgFunc because they are too slow (see PR). _AdjustCursorPosition went from 17% of samples to 3% in performance testing. * version: bump to 1.2 on master * Add keybinding to rename tab (#6557) <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request Add keybinding for renaming a tab <!-- 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] Fulfills format requirements set by #6567 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [X] Tests passed * [X] 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: #6567 and here (#6557) This no longer c loses #6256, as the spec changed. <!-- 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 * Revert "Skip ... analysis when the ... text is simple (6206)" (#6665) This reverts commit 94eab6e391a099814ce8f9a08c8cccf9f588b7d1. We'll reintroduce this again after making sure it plays nicely with recycling and box drawing glyphs. Fixes #6488 Fixes #6664 * Implement Shift+MultiClick Selection Expansion (#6322) This pull request implements shift+double/triple click. Proper behavior (as described in #4557) is to only expand one selection point, not both. Adding the `bool targetStart` was a bit weird. I decided on this being the cleanest approach though because I still want `PivotSelection` to be its own helper function. Otherwise, the concept of "pivoting" gets kinda messy. ## Validation Steps Performed Manual testing as described on attached issue. Tests were added for Shift+Click and pivoting the selection too. Closes #4557 * Add `setTabColor` and `openTabColorPicker` actions (#6567) ## Summary of the Pull Request Adds a pair of `ShortcutAction`s for setting the tab color. * `setTabColor`: This changes the color of the current tab to the provided color, or can be used to clear the color. * `openTabColorPicker`: This keybinding immediately activates the tab color picker for the currently focused tab. ## References ## PR Checklist * [x] scratches my own itch * [x] I work here * [x] Tests added/passed * [x] https://github.com/MicrosoftDocs/terminal/pull/69 ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed * hey look there are tests * Tested with the following: ```json // { "command": "setTabColor", "keys": [ "alt+c" ] }, { "keys": "ctrl+alt+c", "command": { "action": "setTabColor", "color": "#123456" } }, { "keys": "alt+shift+c", "command": { "action": "setTabColor", "color": null} }, { "keys": "alt+c", "command": "openTabColorPicker" }, ``` * When we add a new tab in compact mode, re-enforce Compact mode (#6670) This workaround was suggested by @chingucoding in microsoft/microsoft-ui-xaml#2711 Fixes #6570 ## References microsoft/microsoft-ui-xaml#2711 #6570 ## PR Checklist * [x] Closes an issue * [x] CLA * [x] Tested * [x] Docs not required * [x] Schema not required * Hardcode the paths to Windows PowerShell and CMD (#6684) Occasionally, we get users with corrupt PATH environment variables: they can't lauch PowerShell, because for some reason it's dropped off their PATH. We also get users who have stray applications named `powershell.exe` just lying around in random system directories. We can combat both of these issues by simply hardcoding where we expect PowerShell and CMD to live. %SystemRoot% was chosen over %WINDIR% because apparently (according to Stack Overflow), SystemPath is read-only and WINDIR isn't. Refs #6039, #4390, #4228 (powershell was not found) Refs #4682, Fixes #6082 (stray powershell.exe) * Add support for the Command Palette (#6635) ## Summary of the Pull Request  This adds a first iteration on the command palette. Notable missing features are: * Commandline mode: This will be a follow-up PR, following the merge of #6537 * nested and iterable commands: These will additionally be a follow-up PR. This is also additionally based off the addenda in #6532. This does not bind a key for the palette by default. That will be done when the above follow-ups are completed. ## References * #2046 - The original command palette thread * #5400 - This is the megathread for all command palette issues, which is tracking a bunch of additional follow up work * #5674 and #6532 - specs * #6537 - related ## PR Checklist * [x] Closes #2046 - incidentally also closes #6645 * [x] I work here * [x] Tests added/passed * [ ] Requires documentation to be updated - delaying this until it's more polished. ## Detailed Description of the Pull Request / Additional comments * There's a lot of code for autogenerating command names. That's all in `ActionArgs.cpp`, because each case is so _not_ boilerplate, unlike the rest of the code in `ActionArgs.h`. ## Validation Steps Performed * I've been playing with this for months. * Tests * Selfhost with the team * Restore simple text runs, correct for crashes (#6695) Restores the simple text run analysis and skipping of most of the shaping/layout steps. Corrects one of the fast-path steps to ensure that offsets and clusters are assigned. ## References - Bug #6488 - Bug #6664 - Simple run PR #6206 - Simple run revert PR #6665 - Recycle glyph runs PR #6483 The "correction" functions, by which box drawing analysis is one of them, is dependent on the steps coming before it properly assigning the four main vectors of the text layout glyphs: indices, advances, offsets, and clusters. When the fast path is identified by the code from #6206, only two of those are fully updated: indices and advances. The offsets doesn't tend to cause a problem because offsets are rarely used so they're pretty much always 0 already (but this PR enforces that they're zero for the simple/fast path.) The clusters, however, were not mapped for the fast path. This showed itself in one of two ways: 1. Before the recycled runs PR #6483, the cluster map had a 0 in every field for the stock initialized vector. 2. After the recycled runs PR #6483, the cluster map had the previous run's mapping in it. This meant that when we reached the steps where glyph runs were potentially split during the correction phase for box drawing characters, unexpected values were present to map the glyph indices to clusters and were corrected, adjusted, or split in an unexpected fashion. For instance, the index out of range bug could appear when the default 0 values appended to the end of the clusters vector were decremented down to a negative value during the run splitter as the true DWrite cluster mapper doesn't generate that sort of pattern in the slow path case without also breaking the run itself. The resolution here is therefore to ensure that all of the fields related to glyph layout are populated even in the fast path. This doesn't affect the slow path because that one always populated all fields by asking DWrite to do it. The fast path just skips a bunch of DWrite steps because it can implicitly identify patterns and save a bunch of time. I've also identified a few vectors that weren't cleared on reset/reuse of the layout. I'm clearing those now so the `.resize()` operations performed on them to get to the correct lengths will fill them with fresh and empty values instead of hanging on to ones that may have been from the previous. This should be OK memory perf wise because the act of `.clear()` on a vector shouldn't free anything, just mark it invalid. And doing `.resize()` from an empty one should just default construct them into already allocated space (which ought to be super quick). ## Validation * [x] Far.exe doesn't crash and looks fine * [x] "\e[30;47m\u{2500} What \u{2500}\e[m" from #6488 appears appropriately antialiased * [x] Validate the "\e[30;47m\u{2500} What \u{2500}\e[m" still works when `FillGeometry` is nerfed as a quick test that the runs are split correctly. * [x] Put `u{fffd} into Powershell Core to make a replacement char in the output. Then press enter a few times and see that shrunken initial characters on random rows. Verify this is gone. Closes #6668 Closes #6669 Co-Authored-By: Chester Liu <[email protected]> * Update Cascadia Code to 2007.01 (#6721) * Add support for OSC 52 (copy-to-clipboard) (#5823) With this commit, terminal will be able to copy text to the system clipboard by using OSC 52 MANIPULATE SELECTION DAATA. We chose not to implement the clipboard querying functionality offered by OSC 52, as sending the clipboard text to an application without the user's knowledge or consent is an immense security hole. We do not currently support the clipboard specifier Pc to specify which clipboard buffer should be filled # Base64 encoded `foo` $ echo -en "\e]52;;Zm9v\a" # Multiple lines # Base64 encoded `foo\r\nbar` $ echo -en "\e]52;;Zm9vDQpiYXI=\a" Closes #2946. * Replace old C headers (xxx.h) with modern ones (cxxx) (#5080) * Improve conpty rendering of default colors in legacy apps (#6698) Essentially what this does is map the default legacy foreground and background attributes (typically white on black) to the `IsDefault` color type in the `TextColor` class. As a result, we can now initialize the buffer for "legacy" shells (like PowerShell and cmd.exe) with default colors, instead of white on black. This fixes the startup rendering in conpty clients, which expect an initial default background color. It also makes these colors update appropriately when the default palette values change. One complication in getting this to work, is that the console permits users to change which color indices are designated as defaults, so we can't assume they'll always be white on black. This means that the legacy-to-`TextAttribute` conversion will need access to those default values. Unfortunately the defaults are stored in the conhost `Settings` class (the `_wFillAttribute` field), which isn't easily accessible to all the code that needs to construct a `TextAttribute` from a legacy value. The `OutputCellIterator` is particularly problematic, because some iterator types need to generate a new `TextAttribute` on every iteration. So after trying a couple of different approaches, I decided that the least worst option would be to add a pair of static properties for the legacy defaults in the `TextAttribute` class itself, then refresh those values from the `Settings` class whenever the defaults changed (this only happens on startup, or when the conhost _Properties_ dialog is edited). And once the `TextAttribute` class had access to those defaults, it was fairly easy to adapt the constructor to handle the conversion of default values …
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit introduces a context menu for Tab and a new item,
"Color...", which will display a color picker.
A flyout menu, containing a custom flyout, is attached to each tab. The
flyout displays a palette of 16 preset colors and includes a color
picker. When the user selects or clears color, an event is fired, which
is intercepted by the tab to which the flyout belongs.
The changing of the color is achieved by putting the selected color in
the resource dictionary of the tab, using well-defined dictionary keys
(e.g. TabViewItemHeaderBackground). Afterwards the visual state of the
tab is toggled, so that the color change is visible immediately.
Custom-colored tabs will be desaturated (somewhat) by alpha blending
them with the tab bar background.
The flyout menu also contains a 'Close' flyout item.
Validation Steps Performed
I've validated the behavior manually: start the program via the start
menu. Right click on the tab -> Choose a tab color.
The color flyout is going to be shown. Click a color swatch or click
'Select a custom color' to use the color picker. Use the 'Clear the
current color' to remove the custom color.
Closes #2994. References #3327.