-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some keybindings used by PSReadLine aren't getting through #879
Comments
This is likely a duplicate of #530 but with more information |
TL;DR: There are actually some things that just cannot be represented via VT sequences, (Shift-Ctrl-Enter, Shift-[anything that isn't a letter/number]), and for which we'll need to devise or extend a standard. |
@DHowett-MSFT -- I was thinking about this before and there is a VT200+ command, |
In my opinion, the right way to fix this is to add support for full The other potential trick here is how do we get full |
@zadjii-msft As long as it doesn't become a nest of windows-specific magic. One of the goals of ConPTY is to allow anyone to write a drop-in replacement terminal that speaks VT. If it has to speak WinAPI then I think we've failed somewhere along the way. |
Oh I'm very aware of that. I think this is primarily a case of making sure that Windows-specific programs running attached to a conpty terminal can still behave the way they used to, if the terminal is capable of acting as a Windows-specific terminal. The alternative is driving with PSReadline to stop using those keychords, because they won't work on *nix. Kinda curious that this is still a problem at all, with all the cross-platform stuff powershell is working on. I woulda thought they changed those keybindings. I guess them changing the defaults won't prevent people from setting them back to what they like, even if they don't work cross-platform. |
The problem with pushing it back to PSReadLine to change its default keybindings is that the whole reason these particular bindings were used was to make the text editing experience in PowerShell consistent with other text editors (like Visual Studio). That's the case for keybindings like Ctrl+Backspace, Ctrl+Space, PageUp, and PageDown especially. |
Alacritty has an interesting solution to this problem. They support remapping keys (from the OS point of view) to arbitrary character (VT) sequences. See their default config file. This would still require custom key bindings in PSReadLine or similar applications. For example, in my
Then in my PowerShell profile I might add: Set-PSReadLineKeyHandler -Chord 'F12,a' -Function MenuComplete |
It turns out that the xterm guys may have a more generalized approach for dealing with modifiers:
I know we're trying not to step too far outside DEC VT stuff, but xterm is the de-facto standard. I don't think we should aim to emulate xterm, but we definitely need to take on enough to solve our open problems. Anyway, this seems to imply that xterm has a scheme for passing modifiers for all keys, not just shifted f5-f20 on vanilla DEC hardware. Thoughts? |
Oh, that's a really good idea. We'd need to add support to conpty for parsing them, but that shouldn't be impossible. |
Just to add more to the mix, here's Kitty's approach: https://sw.kovidgoyal.net/kitty/protocol-extensions.html#keyboard-handling
As you can see, they've got a pretty good analog of how we already encode things with INPUT_RECORD. I'm curious if xterm does the same. |
I generally prefer the xterm way. xterm is the de facto standard for terminal communication. Generally any valid VT parser should discard those particular CSIs if they don't understand them. And if we can get away with solving the problem without going fully toward the However, Kitty's approach does seem pretty decent if we do indeed end up having to do something like I think this won't become clear until we choose a path and start down it by trying it out. At which point, I'd encourage the xterm-style way first until proven otherwise. If anything, I think we should probably accept both methods as input to our system but only actively try to emit the xterm way out of the pty interface until proven otherwise. |
Agree @miniksa -- the Kitty guy's protocol using Application Program Commands (APC) is pretty neat, but it's de-facto making anything that parses them a "KittyApp"(tm) and not a VT app. I guess Xterm are doing the same thing, but they are the Gold Standard. APC is meant for VT apps to signal each other, and should be ignored (never echoed) by terminals (although DEC does use some APCs itself, namely DECEKBD -- extended keyboard report.) That said, it's a open, well-known way to do internal signalling, if needed. |
Just want to point out that legacy tools like ZtreeWin are very hard to impossible to use under Windows Terminal without some kind of support for direct raw input. |
Here's a thought: could this instead just be a profile setting? "I want to run a legacy thing like ZTW" => "do the raw input dance" |
Just throwing my vote in for the xterm way. xterm.js doesn't implement |
When there are updates, we'll be sure to post here. We agree that this is a highly important feature, but it's one that unfortunately is going to take a lot of work to get quite right. It's a harder problem than just "add this keychord to the terminal" - it involves implementing an entire other input encoding to the conpty backend, and then also implementing a similar synthesis of input from the Terminal. I've got this crazy thought that some of the work in #4856 might enable this keybinding for WSL applications, but it still won't work for Win32 console apps, like |
@zadjii-msft - PSReadLine supports VT input mode if you set |
@lzybkr Oh I didn't know that existed! That's certainly an interesting idea. Looks like with #4856 merged, we do pipe a NUL all the way through conpty to the client app for Ctrl+Space, but PSReadline doesn't accept it as the key for Ctrl+Space. It looks like it's being treated by PsReadline as Ctrl+@. That's perfectly understandable - they both have the same encoding in VT. Weirdly enough though, binding Ctrl-@ to MenuComplete still doesn't trigger it, event with #4856 merged and |
It's probably an easy fix here. |
Not sure it's already fixed in development, but CTRL-H doesn't work in Vim in my environment. It works in Command prompt, but not work in Windows Terminal when I use Vim. |
When I use Far Manager under the Terminal, RightCtrl+N (N is numeric key) keys don't work. Is it a manifestation of this issue? (if yes, it's very sad, since makes Terminal unusable to host Far Manager) |
Unfortunately, yes. Anything that requires modifiers alone or specific modifiers (right versus left) is covered by this issue. |
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]: #4999 (comment) 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
🎉This issue was addressed in #6309, which has now been successfully released as Handy links: |
* Draw the cursor underneath text, and above the background (#6337) ## Summary of the Pull Request ![textAboveCursor003](https://user-images.githubusercontent.com/18356694/83681722-67a24d00-a5a8-11ea-8d9b-2d294065e4e4.gif) 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 <miniksa@microsoft.com> 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: ![image](https://user-images.githubusercontent.com/18356694/81302365-e6ef4000-903f-11ea-9ce3-5f5ce92e5ba4.png) In this screenshot, tab 2 is focused: ![image](https://user-images.githubusercontent.com/18356694/81302383-ea82c700-903f-11ea-9820-92348d5adc64.png) * 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: ![image](https://user-images.githubusercontent.com/18356694/81303943-00918700-9042-11ea-86e6-7bdfe343c4ca.png) A dark tab color: ![image](https://user-images.githubusercontent.com/18356694/81303953-04250e00-9042-11ea-8db2-be97af519fae.png) * 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` | ![image](https://user-images.githubusercontent.com/18221333/84088632-cbaa8400-a9a1-11ea-8932-04b2e12a0477.png) | ![image](https://user-images.githubusercontent.com/18221333/84088996-b6822500-a9a2-11ea-837c-5e32a110156e.png) `cacafire` | ![image](https://user-images.githubusercontent.com/18221333/84089153-22648d80-a9a3-11ea-8567-c3d80efa16a6.png) | ![image](https://user-images.githubusercontent.com/18221333/84089190-34463080-a9a3-11ea-98e5-a236b12330d6.png) * 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 <leonard@hecker.io> 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 ![command-palette-001](https://user-images.githubusercontent.com/18356694/85313480-b6dbef00-b47d-11ea-8a8f-a802d26c2f9b.gif) 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 <skyline75489@outlook.com> * 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 …
Environment
Steps to reproduce
Open a PowerShell tab in Windows Terminal. Attempt to use these PSReadLine key bindings (use the Get-PSReadLineKeyHandler command to see a list of all key bindings):
Expected behavior
Key executes the specified PSReadLine function.
Actual behavior
Nothing happens (or the current tab changes in the case of Alt+digit)
The text was updated successfully, but these errors were encountered: