VkeyPacketSimulator scenario is broken

[tig]

Originally posted by @BDisp in #2927 (comment)

It's been broken in v2_develop for a while. It is made both better and worse in #2927.

This issue is to track fixing it after #2927 is merged.

[BDisp]

Here a prove that the keyboard implementation still failing to invoke events before the derived class. The original unit test is asserting for the Assert.True (view.NewKeyDownEvent (Key.A));. In the TextView there is no key bindings that handles directly the Key.A and thus it return false from the OnInvokingKeyBindings overridden method and the TextView insert the 'a' char when the OnProcessKeyDown method is called, so the ProcessKeyDown is invoked. If we use the Key.Backspace the TextView will handle it on the OnInvokingKeyBindings and return true, so the InvokingKeyBindings event will never be called.
In this way is impossible to code a flow where we can catch the keys in a event before sending to the derived class and the VkeyPacketSimulator scenario will be hard to fix. When we press a command like Ctrl+Shit+Alt+A, which isn't handled by TextView, a message box will opened with the information of the keys pressed.
Try run the follow changed unit test and it will fail on TextView or create a global command that handle the Key.A and probably all views will fail.

	[Fact]
	public void AllViews_KeyDown_All_EventsFire ()
	{
		foreach (var view in TestHelpers.GetAllViews ()) {
			if (view == null) {
				_output.WriteLine ($"ERROR: null view from {nameof (TestHelpers.GetAllViews)}");
				continue;
			}
			_output.WriteLine ($"Testing {view.GetType ().Name}");

			bool keyDown = false;
			view.KeyDown += (s, a) => {
				a.Handled = false; // don't handle it so the other events are called
				keyDown = true;
			};

			bool invokingKeyBindings = false;
			view.InvokingKeyBindings += (s, a) => {
				a.Handled = false; // don't handle it so the other events are called
				invokingKeyBindings = true;
			};

			bool keyDownProcessed = false;
			view.ProcessKeyDown += (s, a) => {
				a.Handled = true;
				keyDownProcessed = true;
			};

			if (view is TextView) {
				Assert.True (view.NewKeyDownEvent (Key.Backspace));
			} else {
				Assert.True (view.NewKeyDownEvent (Key.A)); // this will be true because the ProcessKeyDown event handled it
			}
			Assert.True (keyDown);
			Assert.True (invokingKeyBindings);
			Assert.True (keyDownProcessed);
			view.Dispose ();
		}
	}

[tig]

Please submit this as a new Issue.

[BDisp]

There is no kind of magic to get the right ConsoleKey from a KeyChar because there are the same value on both. For e.g. ConsoleKey.F2 = 113 and the KeyChar = 113 is equal to 'q'. So it isn't easy to sat if the KeyChar is a ConsoleKey or a unicode character. To pass to ConsoleKey.Packet we need to encode both, in the case the KeyChar is also a byte length, on a char length. So the lower bits is the KeyChar and the upper bits is the ConsoleKey. If the KeyChar is a char length then we can't encode the ConsoleKey and on decoding we have to inspect the database to verify if the KeyChar is mapped to a ConsoleKey to get the corresponded value.

[tig]

If going from a KeyCode to a ConsoleKey is required, shouldn't we adjust the definition of KeyCode so it can be done easily?

[BDisp]

If going from a KeyCode to a ConsoleKey is required, shouldn't we adjust the definition of KeyCode so it can be done easily?

That it's only needed in the case the ConsoleKey.Packet is used but we can add to the Key class more properties, like the ConsoleKey, ScanCode. For the KeyChar there is already the AsRune. Another interest property will the the ColumnWidth of a rune :-)

[tig]

Instead of ConsoleKey and Scancode, what about making it a generic "platform specific data" as in 'object DriverData'?

I don't see the need for NumColumns. People shouldn't be using Key as a substitute for Rune (or Cell).