Repository navigation
Stop dispatching on ConsoleKeyInfo - #831
Conversation
Also minimized the number of calls to this method somewhat.
|
Also worth noting - the Windows build on Azure Pipelines was failing before this PR for reasons I never got to the bottom of, but I investigated enough to suspect this change might fix it, and indeed, it does pass on Windows now, so that's enabled. This also means we could stop building on AppVeyor. |
|
/cc TylerLeonhardt Rob Holt (@rjmholt) for Mac/Linux testing if y'all have bandwidth. Would also be good to ensure the vscode-powershell insiders build starts pulling this after it is merged. |
Steve Lee (SteveL-MSFT)
left a comment
Steve Lee (SteveL-MSFT)
left a comment
There was a problem hiding this comment.
I'll selfhost this on macOS
|
on macOS, |
|
Steve Lee (@SteveL-MSFT) it's probably Set-PSReadLineKeyHandler -Chord "Ctrl+@" -Function MenuComplete |
|
Jason Shirk (@lzybkr) any chance you can look at this again this week? |
* Change tests to use actual ConsoleKeyInfo data * wip - Add fr-FR keyboard layout tests * Handle AltGr * Get fr-FR tests passing * Add KeyInfo-ru-RU-windows.json (#839) All impossible without Shift shortcuts labeled as "Investigate: true" Alt+Spacebar tracked by windows itself * Add manifest so OS version check is correct * Use CmdletBinding instead of CmdletBindingAttribute * Treat Ctrl+c as input in data collection script * Add linux key info * Automate keyboard layout changes for CI * Upload test results * Log keyboard layouts * Don't set LASTEXITCODE after running tests * Fix publish test results task * Add KeyInfo for Polish (Programmers) layout (#870) Add KeyInfo for Polish (Programmers) layout on Windows and Linux Notes: On Windows *Alt+Spacebar* is hijacked by Windows On Linux any *Alt+F1-12* combination was not detected
4 participants
This should fix all keyboard layout issues that I introduced last year, but it might cause some regressions.
The simple idea is to stop using
ConsoleKeyInfoas the key of the dispatch tables because for many keys, theConsoleKeyInfodiffers between keyboard layouts (differences in modifiers or scan codes) or operating systems (differences in the CLR).Instead, the dispatch tables now use simple strings as the key - the same string used by
Get-PSReadLineKeyHandlerandSet-PSReadLineKeyHandler. Layout/os differences are handled in converting theConsoleKeyInfoto this simpler representation.Note that in this PR, key handlers continue to accept a
ConsoleKeyInfo, so there are multiple conversions on each key press. I'm not totally happy with this, but it does avoid a minor breaking change.For now, the dispatch key isn't actually a string, but the wrapper type
PSKeyInfothat holds the string and adds some convenience properties. That type is not public for now. It might be useful as a platform/layout neutral representation of a key, but I'm not committing to that.Not yet implemented, but as part of this work, I realized testing needs much improvement to avoid platform/layout issues. I started writing a script to help capture key presses. I intend to clean that up at some point and crowd source the data collection so the tests can easily test the exact
ConsoleKeyInfovalues we see depending on the keyboard layout or OS.This is untested as a user on non-Windows. Azure Pipelines is testing Linux/Mac, but isn't as useful as I'd like for this test because the
ConsoleKeyInfovalues are generated from the code being tested instead of real values, I'd appreciate some testing from folks using Linux/Mac regularly before this PR is merged if possible, but definitely before the next release.Fix #824 #826