Simplify Key - BackTab and upper-case alphas

[tig]

Next, related question:

Keys, ConsoleKeys, and KeyModifiers is muy confused when it comes to Shift.

Specifically:

• BackTab is special cased (in our library I assert it should not be defined in Key at all and should be handled as a Command bound to Key.Tab | Key.Shift).

Specially on Linux the BackTab is specific. Test on both Windows and Linux, before.

• Key has members for both lower and upper case alphas. I believe in the olden days it just had lower (but as Key.A). @BDisp, I think, added Key.a...z. I think he agrees this was an error.

Yes, I really regret implementing it. You can remove them without any problems.

I'd like to simplify this. Any objections or concerns?

For me, go for it. Thanks.

Originally posted by @BDisp in #2972 (comment)

[tig]

@BDisp, going back to the early versions of gui.cs Key never had elements for any alphas.

I actually think the right thing to do is to do that. For Terminal.Gui to fully support internationalization the internal definitions should not be so tied to the English language/keyboard.

It's nice to be able to do this:

AddKeyBinding (Key.A | Key.CtrlMask, Command.SelectAll);

instead of this:

AddKeyBinding ((Key)'a' | Key.CtrlMask, Command.SelectAll);

But, there is so much complexity in all the drivers and tests around dealing with this it doesn't seem worth it.

What do you tihnk?

[tig]

@BDisp, do you remember what this special case for J is for in CursesDriver?

You introduced this in:

Commit 01d4b4f55f68b4a7e17aebf05a2e2633f33422a0
Author: BDisp <[email protected]>
Date: Sunday, October 25, 2020 5:46 PM
Parent: 4729f467

Added wide improvements on keys managements.

[tznind]

AddKeyBinding ((Key)'a' | Key.CtrlMask, Command.SelectAll);

I would not be a fan of this, having to cast a char to a Key as a public API user would be a big red flag for me.

I agree that Key is in a bad state. We can see from just this:

Code	Value
(int)'a'	97
(int)((Key)'a' | Key.ShiftMask	268435553
(int)Key.A	65
(int)(Key.A|Key.ShiftMask)	268435521
(int)'A'	65

From second row onwards should all be the same.

Maybe the solution would be to seperate completely the modifier keys from the char

AddKeyBinding ('a',Key.CtrlMask, Command.SelectAll);

To go further we could change Key from an enum to a class with modifiers?

new Key('a', Modifier.Shift);

[BDisp]

@BDisp, going back to the early versions of gui.cs Key never had elements for any alphas.

I actually think the right thing to do is to do that. For Terminal.Gui to fully support internationalization the internal definitions should not be so tied to the English language/keyboard.

It's nice to be able to do this:

AddKeyBinding (Key.A | Key.CtrlMask, Command.SelectAll);

instead of this:

AddKeyBinding ((Key)'a' | Key.CtrlMask, Command.SelectAll);

Some Linux shortcuts are tied to lower case and for that cases is needed the (Key)'a' that is different from the Key.A, as @tznind said above. So, the upper case should exist in the Key and the lower cases the user must use the char 'alpha'.

But, there is so much complexity in all the drivers and tests around dealing with this it doesn't seem worth it.

Remove all the lower cases it worth.

What do you tihnk?

For the lower case shortcuts that Linux/mac users are used we must maintain them, I think. And I'm a Windows user :-)

[BDisp]

@BDisp, do you remember what this special case for J is for in CursesDriver?

Ctrl + J represent a newline shortcut on Linux.

[BDisp]

To go further we could change Key from an enum to a class with modifiers?

new Key('a', Modifier.Shift);

That a possibility. For example the TextField use the new KeyEvent (ShortcutHelper.GetModifiersKey (kb) to get the modifiers keys to include them on Key to perform comparison more easer, instead of managing them separately.

[tznind]

We want to make life as simple as possible for end user of API.

I would say that we should standardize input at the point of the driver (i.e. as early as possible).

This means no views ever need to worry about platform or ShortcutHelper etc. If possible there should be a uniform cannonical class which clearly shows what keys were struck and the modifiers held at that time. It should be immutable and work the same on all OS.

If a modifier key or combination is known to not work on a given OS the xml class docs should indicate that.

[tig]

@tznind when you get a chance, I'd like you to opine on how #2927 looks to you in this regard.

E.g. this is now typical:

AddKeyBinding ((Key)' ', Command.ToggleChecked);
AddKeyBinding (Key.Space, Command.ToggleChecked);

We want to make life as simple as possible for end user of API.

I would say that we should standardize input at the point of the driver (i.e. as early as possible).

This means no views ever need to worry about platform or ShortcutHelper etc. If possible there should be a uniform cannonical class which clearly shows what keys were struck and the modifiers held at that time. It should be immutable and work the same on all OS.

If a modifier key or combination is known to not work on a given OS the xml class docs should indicate that.

I'd like to propose an API on CursesDriver that let's us spew diagnostics if a key is used that isn't supported by the platform.

[tznind]

AddKeyBinding ((Key)' ', Command.ToggleChecked);
AddKeyBinding (Key.Space, Command.ToggleChecked);

I am confused. This looks like the same thing twice. Are you sure this is needed? The following test passes so the above code is literally just doing the same thing twice.

[Fact]
public void EqualRight()
{
	Assert.Equal ((Key)' ', Key.Space);
}

[tznind]

I'd like to propose an API on CursesDriver that let's us spew diagnostics if a key is used that isn't supported by the platform.

I think this is a great idea. Some low level logs of events in general would be really nice. Especially when debugging user issues. Starting with the most unpredictable areas of the codebase makes sense (i.e. curses driver ;) ).

[tznind]

@tznind when you get a chance, I'd like you to opine on how #2927 looks to you in this regard.

I love your work on this PR. It is pretty epic! (+/- 24,000) and definetly a big improvement and standardization.

I am still uneasy with the way we have an Enum for which not all values are defined and which seems to be inconsistent. To illustrate (see below). But I want to be practical about it. I think we should merge the key binding changes and worry about this later and/or investigate it seperately on its own.

[Fact]
public void TestIfEqual()
{

	// Test fails
	Assert.Equal ((Key)'a' | Key.ShiftMask, Key.A);
}

[BDisp]

AddKeyBinding ((Key)' ', Command.ToggleChecked);
AddKeyBinding (Key.Space, Command.ToggleChecked);

I am confused. This looks like the same thing twice. Are you sure this is needed? The following test passes so the above code is literally just doing the same thing twice.

[Fact]
public void EqualRight()
{
	Assert.Equal ((Key)' ', Key.Space);
}

I think that's only need one of them (Key.Space) because the are the same.

[BDisp]

@tznind when you get a chance, I'd like you to opine on how #2927 looks to you in this regard.

I love your work on this PR. It is pretty epic! (+/- 24,000) and definetly a big improvement and standardization.

I am still uneasy with the way we have an Enum for which not all values are defined and which seems to be inconsistent. To illustrate (see below). But I want to be practical about it. I think we should merge the key binding changes and worry about this later and/or investigate it seperately on its own.

[Fact]
public void TestIfEqual()
{

	// Test fails
	Assert.Equal ((Key)'a' | Key.ShiftMask, Key.A);
}

@tznind even the ConsoleKey enum doesn't have the Shift, Ctrl and Alt because they aren't keys with visual representation but only modifiers and each keyboard and drivers has it own representation dependent of the culture. Thus Key, now ConsoleDriverKey, is a flagged enum which values of (Key)'a' | Key.ShiftMask cannot be translated to Key.A.
e.g. On the Portuguese keyboard the Key.ShiftMask with the keys from Key.D1 to Key.D0 are ! " # $ % & / ( ) =, respectively. So, it's only a information of the composed key and the AsRune return the representation of the key and the BareKey return the Key's without the modifiers keys (although I don't know what value it return if the Key have the capslock, numlock and scrolllock keys) which can be different from the AsRune. e.g. AsRune may have 'a' and BareKey may have Key.Space | Key.A.