Merge pull request #125 from BDisp/vkpacket-fix

Only the Shift key must be handled with the Packet key.

Terminal.Gui/ConsoleDrivers/FakeDriver/FakeDriver.cs

@@ -360,7 +360,7 @@ namespace Terminal.Gui {
 		private Key MapKeyModifiers (ConsoleKeyInfo keyInfo, Key key)
 		{
 			Key keyMod = new Key ();
-			if ((keyInfo.Modifiers & ConsoleModifiers.Shift) != 0)
+			if (CanShiftBeAdded (keyInfo))
 				keyMod = Key.ShiftMask;
 			if ((keyInfo.Modifiers & ConsoleModifiers.Control) != 0)
 				keyMod |= Key.CtrlMask;
@@ -370,6 +370,20 @@ namespace Terminal.Gui {
 			return keyMod != Key.Null ? keyMod | key : key;
 		}
 
+		private bool CanShiftBeAdded (ConsoleKeyInfo keyInfo)
+		{
+			if ((keyInfo.Modifiers & ConsoleModifiers.Shift) == 0) {
+				return false;
+			}
+			if (keyInfo.Key == ConsoleKey.Packet) {
+				var ckiChar = keyInfo.KeyChar;
+				if (char.IsLetter (ckiChar)) {
+					return false;
+				}
+			}
+			return true;
+		}
+
 		Action<KeyEvent> keyHandler;
 		Action<KeyEvent> keyUpHandler;
 		private CursorVisibility savedCursorVisibility;

Terminal.Gui/ConsoleDrivers/WindowsDriver.cs

@@ -1250,15 +1250,6 @@ namespace Terminal.Gui {
 
 		public Key MapKey (WindowsConsole.ConsoleKeyInfoEx keyInfoEx)
 		{
-			// If keystroke is a virtual key
-			if (keyInfoEx.consoleKeyInfo.Key == ConsoleKey.Packet) {
-
-				// try to map the 'char' that came with it into a Key
-				if (TryRemapPacketKey (keyInfoEx.consoleKeyInfo, out var result)) {
-					return result;
-				}
-			}
-
 			var keyInfo = keyInfoEx.consoleKeyInfo;
 			switch (keyInfo.Key) {
 			case ConsoleKey.Escape:
@@ -1381,10 +1372,10 @@ namespace Terminal.Gui {
 			return (Key)(0xffffffff);
 		}
 
-		Key MapKeyModifiers (ConsoleKeyInfo keyInfo, Key key)
+		private Key MapKeyModifiers (ConsoleKeyInfo keyInfo, Key key)
 		{
 			Key keyMod = new Key ();
-			if ((keyInfo.Modifiers & ConsoleModifiers.Shift) != 0)
+			if (CanShiftBeAdded (keyInfo))
 				keyMod = Key.ShiftMask;
 			if ((keyInfo.Modifiers & ConsoleModifiers.Control) != 0)
 				keyMod |= Key.CtrlMask;
@@ -1394,6 +1385,20 @@ namespace Terminal.Gui {
 			return keyMod != Key.Null ? keyMod | key : key;
 		}
 
+		private bool CanShiftBeAdded (ConsoleKeyInfo keyInfo)
+		{
+			if ((keyInfo.Modifiers & ConsoleModifiers.Shift) == 0) {
+				return false;
+			}
+			if (keyInfo.Key == ConsoleKey.Packet) {
+				var ckiChar = keyInfo.KeyChar;
+				if (char.IsLetter (ckiChar)) {
+					return false;
+				}
+			}
+			return true;
+		}
+
 		public override void Init (Action terminalResized)
 		{
 			TerminalResized = terminalResized;
@@ -1738,60 +1743,6 @@ namespace Terminal.Gui {
 		public override void CookMouse ()
 		{
 		}
-
-
-		/// <summary>
-		/// Handles case when the 'key' providied is <see cref="ConsoleKey.Packet"/>
-		/// returning a <see cref="Key"/> that reflects the unicode char that came with
-		/// the OS event.
-		/// </summary>
-		/// <exception cref="ArgumentException">Thrown if passed key was not a <see cref="ConsoleKey.Packet"/></exception>
-		internal static bool TryRemapPacketKey (ConsoleKeyInfo original, out Key result)
-		{
-			result = default (Key);
-			var c = original.KeyChar;
-
-			if (original.Key != ConsoleKey.Packet)
-				throw new ArgumentException ("Expected a ConsoleKeyInfo with a Key of Packet", nameof (original));
-
-			// there is no unicode value passed
-			if (c == '\0') {
-				return false;
-			}
-
-			// do not have a explicit mapping and char is nonzero so 
-			// we can just treat the `Key` as a regular unicode entry
-			result = ApplyModifiers ((Key)c,original.Modifiers);
-
-			return true;
-		}
-
-		/// <summary>
-		/// Applies 
-		/// </summary>
-		/// <param name="c"></param>
-		/// <param name="modifiers"></param>
-		/// <returns></returns>
-		private static Key ApplyModifiers (Key c, ConsoleModifiers modifiers)
-		{
-			if(modifiers.HasFlag(ConsoleModifiers.Control)) {
-				c |= Key.CtrlMask;
-			}
-
-			if (modifiers.HasFlag (ConsoleModifiers.Alt)) {
-				c |= Key.AltMask;
-			}
-
-			/* TODO: Why not this too? I'm a bit confused
-			         ALSO this method looks a lot like `Key MapKeyModifiers (ConsoleKeyInfo keyInfo, Key key)` 
-		             Maybe we should be using that instead?
-			if (modifiers.HasFlag (ConsoleModifiers.Shift)) {
-				c |= Key.ShiftMask;
-			}
-			*/
-
-			return c;
-		}
 		#endregion
 	}
 

UnitTests/ConsoleDriverTests.cs

@@ -610,13 +610,13 @@ namespace Terminal.Gui.ConsoleDrivers {
 		}
 
 		/// <summary>
-		/// Sometimes when using remoting tools EventKeyRecord sends 'virtual keystrokes'.
-		/// These are indicated with the wVirtualKeyCode of 231.  When we see this code
+		/// Sometimes when using remote tools EventKeyRecord sends 'virtual keystrokes'.
+		/// These are indicated with the wVirtualKeyCode of 231. When we see this code
 		/// then we need to look to the unicode character (UnicodeChar) instead of the key
 		/// when telling the rest of the framework what button was pressed. For full details
 		/// see: https://github.com/gui-cs/Terminal.Gui/issues/2008
 		/// </summary>
-		[Theory]
+		[Theory, AutoInitShutdown]
 		[InlineData ('A', false, false, false, Key.A)]
 		[InlineData ('A', true, false, false, Key.A)]
 		[InlineData ('A', true, true, false, Key.A | Key.AltMask)]
@@ -626,36 +626,39 @@ namespace Terminal.Gui.ConsoleDrivers {
 		[InlineData ('z', true, true, false, Key.z | Key.AltMask)]
 		[InlineData ('z', true, true, true, Key.z | Key.AltMask | Key.CtrlMask)]
 		[InlineData ('=', false, false, false, (Key)'=')]
-		[InlineData ('=', true, false, false, (Key)'=')]
-		[InlineData ('=', true, true, false, (Key)'=' | Key.AltMask)]
-		[InlineData ('=', true, true, true, (Key)'=' | Key.AltMask | Key.CtrlMask)]
+		[InlineData ('=', true, false, false, (Key)'=' | Key.ShiftMask)]
+		[InlineData ('=', true, true, false, (Key)'=' | Key.ShiftMask | Key.AltMask)]
+		[InlineData ('=', true, true, true, (Key)'=' | Key.ShiftMask | Key.AltMask | Key.CtrlMask)]
 		[InlineData ('英', false, false, false, (Key)'英')]
 		[InlineData ('英', true, false, false, (Key)'英')]
 		[InlineData ('英', true, true, false, (Key)'英' | Key.AltMask)]
 		[InlineData ('英', true, true, true, (Key)'英' | Key.AltMask | Key.CtrlMask)]
 		[InlineData ('+', false, false, false, (Key)'+')]
-		[InlineData ('+', true, false, false, (Key)'+')]
-		[InlineData ('+', true, true, false, (Key)'+' | Key.AltMask)]
-		[InlineData ('+', true, true, true, (Key)'+' | Key.AltMask | Key.CtrlMask)]
-		public void TestVKPacket (char unicodeCharacter,bool shift, bool alt, bool ctrl, Key expectedRemapping)
+		[InlineData ('+', true, false, false, (Key)'+' | Key.ShiftMask)]
+		[InlineData ('+', true, true, false, (Key)'+' | Key.ShiftMask | Key.AltMask)]
+		[InlineData ('+', true, true, true, (Key)'+' | Key.ShiftMask | Key.AltMask | Key.CtrlMask)]
+		[InlineData ('0', false, false, false, Key.D0)]
+		[InlineData ('0', true, false, false, Key.D0 | Key.ShiftMask)]
+		[InlineData ('0', true, true, false, Key.D0 | Key.ShiftMask | Key.AltMask)]
+		[InlineData ('0', true, true, true, Key.D0 | Key.ShiftMask | Key.AltMask | Key.CtrlMask)]
+		[InlineData ('\0', false, false, false, (Key)'\0')]
+		[InlineData ('\0', true, false, false, (Key)'\0' | Key.ShiftMask)]
+		[InlineData ('\0', true, true, false, (Key)'\0' | Key.ShiftMask | Key.AltMask)]
+		[InlineData ('\0', true, true, true, (Key)'\0' | Key.ShiftMask | Key.AltMask | Key.CtrlMask)]
+		public void TestVKPacket (char unicodeCharacter, bool shift, bool alt, bool control, Key expectedRemapping)
 		{
-			var before = new ConsoleKeyInfo (unicodeCharacter, ConsoleKey.Packet, shift, alt, ctrl);
-			Assert.True (WindowsDriver.TryRemapPacketKey (before, out var after));
+			var before = new ConsoleKeyInfo (unicodeCharacter, ConsoleKey.Packet, shift, alt, control);
+			var top = Application.Top;
 
-			// The thing we are really interested in, did we correctly convert
-			// the input ConsoleKey.Packet to the correct physical key
-			Assert.Equal (expectedRemapping, after);
-		}
-		
-		[Theory]
-		[InlineData (false, false, false)]
-		[InlineData (true, false, false)]
-		[InlineData (true, true, false)]
-		[InlineData (true, true, true)]
-		public void TestVKPacketWithZero (bool shift, bool alt, bool ctrl)
-		{
-			var before = new ConsoleKeyInfo ('\0', ConsoleKey.Packet,shift,alt,ctrl);
-			Assert.False (WindowsDriver.TryRemapPacketKey (before, out var _),"Expected there to be no attempt to map \\0 to a Key");
+			top.KeyPress += (e) => {
+				var after = e.KeyEvent.Key;
+				Assert.Equal (before.KeyChar, (char)after);
+				Assert.Equal (expectedRemapping, after);
+			};
+
+			Application.Begin (top);
+
+			Application.Driver.SendKeys (unicodeCharacter, ConsoleKey.Packet, shift, alt, control);
 		}
 	}
 }