Skip to content

WIP - Test with real ConsoleKeyInfo values - #838

Merged
Jason Shirk (lzybkr) merged 15 commits into
string_keymapsfrom
kblayout_test
Mar 20, 2019
Merged

Jason Shirk (lzybkr) merged 15 commits into
string_keymapsfrom
kblayout_test

Conversation

@lzybkr

Copy link
Copy Markdown
Contributor

The biggest weakness in testing was the reuse of the internal values in the class Keys - it worked well for testing en-US on Windows because that's how I've done most of the development, but there have been too many problems to continue with that approach.

The new approach is to collect the actual ConsoleKeyInfo values to test against, once for each keyboard layout that we care to test. The script Get-KeyInfoData.ps1 can be used, it's a bit tedious, takes maybe 5-15 minutes depending on how careful you are and how familiar you are with the keyboard layout, but it's a one-time task to add a new layout.

I've collected the data for en-US ensuring all tests pass, then added fr-FR and some tests fail. Also note that testing both fr-FR with en-US also causes some en-US tests to fail, that's why this is WIP.

If anyone cares to help out collecting the keys for other layouts (e.g. pt-BR, nb-NO, es-ES, sv-SE, it-IT, any Mac/Linux) - I'd appreciate the help.

Note that the Get-KeyInfoData.ps1 script isn't totally user friendly, e.g. if it asks you to enter a key that the system consumes, the script never sees any input, so you're forced to enter something random, and you'll have to do it twice to get past the typo detection that can't work perfectly, but works well enough. I suggest using F12 but it doesn't matter too much, after adding the tests I'll figure out which keys don't make sense.

@MVKozlov

Max Kozlov (MVKozlov) commented Dec 25, 2018 •

Copy link
Copy Markdown
Contributor

A couple of questions:
when it say

Enter <w>:
Enter <V>:

CAPS, V or Shift+v ?

Enter <Ctrl+C>:

CAPS, Ctrl+C or Ctrl+c or Shift+Ctrl+c ?

there is

Enter Ctrl+c
Enter Ctrl+C

should I use the same combination or press Shift for second ? and many others that exists only with shift

Enter is a gray Enter ? on primary it say "double check', and for <Ctrl+Alt+?> too (may be because ? is a Shift+/ )

for /, *, -, + should I use Shift combinations or gray keyboard ?

btw, [CmdletBindingAttribute()] is a typo or something latest PSv6 specific ?

@lzybkr

Copy link
Copy Markdown
Contributor Author

Max Kozlov (@MVKozlov) - thanks for giving this a go.

It looks like for regular keys, Caps Lock and Shift should work identically, so either is fine and it will be good to test without Shift.

For control keys, it looks Caps Lock,Ctrl+a does not turn into Ctrl+A, so avoid Caps Lock for modified keys.

If you get the message double check, don't worry too much about that. The intent is to catch typos, but that logic is dependent on possibly buggy logic in PSReadLine. If you enter the same key twice, it accepts it as correct and moves on.

For symbols, I suggest using the most natural keys. If you're not too sure, then I suggest not using the numeric keypad as laptops often don't have those keys.

And regarding CmdletBindingAttribute, that's always been supported, I didn't realize I typed out the full type name. By convention, attributes use the suffix Attribute and languages like C# or PowerShell can resolve the type name when used as an attribute with or without the suffix. PowerShell goes a bit further since V3 an resolves the name when used as a type (e.g. 'CmdletBinding' -as [Type]), whereas in C# you'd have to write typeof(CmdletBindingAttribute).

@MVKozlov

Copy link
Copy Markdown
Contributor

You do not say a word about duplicates - Ctrl+C and Ctrl+c - should I use Shift+Ctrl+c for first. I think I should

Caps Lock and Shift can work differently at least at PSRealine :), see #798
I try to use CAPS.

So, I make PR when i finished with it.

and about attribute:

PS D:\> D:\Work\PSReadLine\test\Get-KeyInfoData.ps1
Cannot find the type for custom attribute 'CmdletBindingAttribute'. Make sure that the assembly that contains this type is loaded
.
At D:\Work\PSReadLine\test\Get-KeyInfoData.ps1:1 char:1
+ [CmdletBindingAttribute()]
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: ([CmdletBindingAttribute()]:AttributeAst) [], RuntimeException
    + FullyQualifiedErrorId : CustomAttributeTypeNotFound

PS D:\> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.14409.1012
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14409.1012
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

but with [CmdletBinding()] its OK

@MVKozlov

Copy link
Copy Markdown
Contributor

PR #839

All impossible without Shift shortcuts labeled as "Investigate: true"
Alt+Spacebar tracked by windows itself
@lzybkr

Jason Shirk (lzybkr) commented Dec 26, 2018 •

Copy link
Copy Markdown
Contributor Author

Thanks Max Kozlov (@MVKozlov). [CmdetBindingAttribute()] worked for me because of something I have in my profile. I fixed the script so nobody else will hit the same thing.

Can I ask how you entered the English characters? Alt+numpad? Switch layouts? Or is there another way?

After I work out how to best run the tests with different layouts, we'll start adding language specific tests, so for example we can make sure Cyrillic characters work properly, and make sure you can enter other characters like ã or à or â correctly.

There is a bug in a Win32 api that I'm using that doesn't correctly handle keyboard layout changes. It doesn't affect too many key bindings, but it definitely affects running the tests, so I need to figure out a way to run the tests where the keyboard layout is changed before launching the test runner and the test runner only tests a single language.

@MVKozlov

Copy link
Copy Markdown
Contributor

I switched to US layout but if need I can collect Cyrillic key info on the same keys with Cyrillic layout (but seems I will need to enter every key twice :)

@MVKozlov

Copy link
Copy Markdown
Contributor

Jason Shirk (@lzybkr), so there is need to repeat collecting console info with cyrillic layout switched on ?

@kborowinski

Copy link
Copy Markdown

PR #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
@lzybkr
Jason Shirk (lzybkr) merged commit 0276f9a into string_keymaps Mar 20, 2019
@lzybkr
Jason Shirk (lzybkr) deleted the kblayout_test branch March 20, 2019 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants