Fixes gui-cs#3055: Remove Key.Unknown (gui-cs#3057)

* Simplifies Key.IsValid

* Updated unit tests

* Fixed menu

Terminal.Gui/Configuration/KeyCodeJsonConverter.cs

@@ -4,18 +4,19 @@
 using System.Text.Json.Serialization;
 
 namespace Terminal.Gui;
+
 class KeyCodeJsonConverter : JsonConverter<KeyCode> {
 	public override KeyCode Read (ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
 	{
 		if (reader.TokenType == JsonTokenType.StartObject) {
-			KeyCode key = KeyCode.Unknown;
-			Dictionary<string, KeyCode> modifierDict = new Dictionary<string, KeyCode> (comparer: StringComparer.InvariantCultureIgnoreCase) {
-					{ "Shift", KeyCode.ShiftMask },
-					{ "Ctrl", KeyCode.CtrlMask },
-					{ "Alt", KeyCode.AltMask }
-				};
+			KeyCode key = KeyCode.Null;
+			var modifierDict = new Dictionary<string, KeyCode> (comparer: StringComparer.InvariantCultureIgnoreCase) {
+				{ "Shift", KeyCode.ShiftMask },
+				{ "Ctrl", KeyCode.CtrlMask },
+				{ "Alt", KeyCode.AltMask }
+			};
 
-			List<KeyCode> modifiers = new List<KeyCode> ();
+			var modifiers = new List<KeyCode> ();
 
 			while (reader.Read ()) {
 				if (reader.TokenType == JsonTokenType.EndObject) {
@@ -38,7 +39,7 @@ public override KeyCode Read (ref Utf8JsonReader reader, Type typeToConvert, Jso
 								break;
 							}
 
-							if (key == KeyCode.Unknown || key == KeyCode.Null) {
+							if (key == KeyCode.Null) {
 								throw new JsonException ($"The value \"{reader.GetString ()}\" is not a valid Key.");
 							}
 
@@ -60,7 +61,7 @@ public override KeyCode Read (ref Utf8JsonReader reader, Type typeToConvert, Jso
 								if (reader.TokenType == JsonTokenType.EndArray) {
 									break;
 								}
-								var mod = reader.GetString ();
+								string mod = reader.GetString ();
 								try {
 									modifiers.Add (modifierDict [mod]);
 								} catch (KeyNotFoundException e) {
@@ -91,21 +92,20 @@ public override void Write (Utf8JsonWriter writer, KeyCode value, JsonSerializer
 	{
 		writer.WriteStartObject ();
 
-		var keyName = (value & ~KeyCode.CtrlMask & ~KeyCode.ShiftMask & ~KeyCode.AltMask).ToString ();
+		string keyName = (value & ~KeyCode.CtrlMask & ~KeyCode.ShiftMask & ~KeyCode.AltMask).ToString ();
 		if (keyName != null) {
 			writer.WriteString ("Key", keyName);
 		} else {
 			writer.WriteNumber ("Key", (uint)(value & ~KeyCode.CtrlMask & ~KeyCode.ShiftMask & ~KeyCode.AltMask));
 		}
 
-		Dictionary<string, KeyCode> modifierDict = new Dictionary<string, KeyCode>
-		{
-				{ "Shift", KeyCode.ShiftMask },
-				{ "Ctrl", KeyCode.CtrlMask },
-				{ "Alt", KeyCode.AltMask }
-			    };
+		var modifierDict = new Dictionary<string, KeyCode> {
+			{ "Shift", KeyCode.ShiftMask },
+			{ "Ctrl", KeyCode.CtrlMask },
+			{ "Alt", KeyCode.AltMask }
+		};
 
-		List<string> modifiers = new List<string> ();
+		var modifiers = new List<string> ();
 		foreach (var pair in modifierDict) {
 			if ((value & pair.Value) == pair.Value) {
 				modifiers.Add (pair.Key);
@@ -115,12 +115,12 @@ public override void Write (Utf8JsonWriter writer, KeyCode value, JsonSerializer
 		if (modifiers.Count > 0) {
 			writer.WritePropertyName ("Modifiers");
 			writer.WriteStartArray ();
-			foreach (var modifier in modifiers) {
+			foreach (string modifier in modifiers) {
 				writer.WriteStringValue (modifier);
 			}
 			writer.WriteEndArray ();
 		}
 
 		writer.WriteEndObject ();
 	}
-}
+}

Terminal.Gui/Configuration/KeyJsonConverter.cs

@@ -8,7 +8,7 @@ class KeyJsonConverter : JsonConverter<Key> {
 	public override Key Read (ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
 	{
 		if (reader.TokenType == JsonTokenType.StartObject) {
-			Key key = KeyCode.Unknown;
+			Key key = Key.Empty;
 			while (reader.Read ()) {
 				if (reader.TokenType == JsonTokenType.EndObject) {
 					break;

Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs

@@ -1060,10 +1060,5 @@ public enum KeyCode : uint {
 	/// F24 key.
 	/// </summary>
 	F24,
-
-	/// <summary>
-	/// A key with an unknown mapping was raised.
-	/// </summary>
-	Unknown
 }
 

Terminal.Gui/ConsoleDrivers/ConsoleKeyMapping.cs

@@ -263,9 +263,6 @@ public static ConsoleKey MapKeyToConsoleKey (KeyCode keyValue, ConsoleModifiers
 				return ConsoleKey.F24;
 			case KeyCode.Tab | KeyCode.ShiftMask:
 				return ConsoleKey.Tab;
-			case KeyCode.Unknown:
-				isMappable = true;
-				return 0;
 			}
 
 			isMappable = true;

Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs

@@ -416,7 +416,7 @@ static KeyCode MapCursesKey (int cursesKey)
 		case Curses.AltCtrlKeyPPage: return KeyCode.PageUp | KeyCode.AltMask | KeyCode.CtrlMask;
 		case Curses.AltCtrlKeyHome: return KeyCode.Home | KeyCode.AltMask | KeyCode.CtrlMask;
 		case Curses.AltCtrlKeyEnd: return KeyCode.End | KeyCode.AltMask | KeyCode.CtrlMask;
-		default: return KeyCode.Unknown;
+		default: return KeyCode.Null;
 		}
 	}
 

Terminal.Gui/ConsoleDrivers/FakeDriver/FakeDriver.cs

@@ -253,7 +253,7 @@ KeyCode MapKey (ConsoleKeyInfo keyInfo)
 		case ConsoleKey.OemPlus:
 		case ConsoleKey.OemMinus:
 			if (keyInfo.KeyChar == 0) {
-				return KeyCode.Unknown;
+				return KeyCode.Null;
 			}
 
 			return ConsoleKeyMapping.MapKeyModifiers (keyInfo, (KeyCode)((uint)keyInfo.KeyChar));

Terminal.Gui/Input/Key.cs

@@ -95,7 +95,7 @@ public Key () : this (KeyCode.Null) { }
 	/// This property is the backing data for the <see cref="Key"/>. It is a <see cref="KeyCode"/> enum value.
 	/// </remarks>
 	[JsonInclude] [JsonConverter (typeof (KeyCodeJsonConverter))]
-	public KeyCode KeyCode { get; set; }
+	public KeyCode KeyCode { get; init; }
 
 	/// <summary>
 	/// Enables passing the key binding scope with the event. Default is <see cref="KeyBindingScope.Focused"/>.
@@ -195,9 +195,10 @@ public static bool GetIsKeyCodeAtoZ (KeyCode keyCode)
 	}
 
 	/// <summary>
-	/// Indicates whether the <see cref="Key"/> is valid or not. 
+	/// Indicates whether the <see cref="Key"/> is valid or not. Invalid keys are <see cref="Key.Empty"/>,
+	/// and keys with only shift modifiers.
 	/// </summary>
-	public bool IsValid => KeyCode is not (KeyCode.Null or KeyCode.Unknown);
+	public bool IsValid => this != Empty && (NoAlt.NoShift.NoCtrl != Empty);
 
 	/// <summary>
 	/// Helper for specifying a shifted <see cref="Key"/>.
@@ -383,8 +384,9 @@ static string GetKeyString (KeyCode key)
 	/// <returns>The formatted string. If the key is a printable character, it will be returned as a string. Otherwise, the key name will be returned.</returns>
 	public static string ToString (KeyCode key, Rune separator)
 	{
-		if (key is KeyCode.Null) {
-			return string.Empty;
+		if (key is KeyCode.Null || (key & ~KeyCode.CtrlMask & ~KeyCode.AltMask & ~KeyCode.ShiftMask) == 0) {
+			// Same as Key.IsValid
+			return @"Null";
 		}
 
 		var sb = new StringBuilder ();

Terminal.Gui/Input/ShortcutHelper.cs

@@ -17,7 +17,7 @@ public class ShortcutHelper {
 	public virtual KeyCode Shortcut {
 		get => shortcut;
 		set {
-			if (shortcut != value && (PostShortcutValidation (value) || value is KeyCode.Null or KeyCode.Unknown)) {
+			if (shortcut != value && (PostShortcutValidation (value) || value is KeyCode.Null)) {
 				shortcut = value;
 			}
 		}
@@ -142,7 +142,7 @@ public static bool PostShortcutValidation (KeyCode key)
 		GetKeyToString (key, out KeyCode knm);
 
 		if (CheckKeysFlagRange (key, KeyCode.F1, KeyCode.F12) ||
-			((key & (KeyCode.CtrlMask | KeyCode.ShiftMask | KeyCode.AltMask)) != 0 && knm != KeyCode.Null && knm != KeyCode.Unknown)) {
+			((key & (KeyCode.CtrlMask | KeyCode.ShiftMask | KeyCode.AltMask)) != 0 && knm != KeyCode.Null)) {
 			return true;
 		}
 		Debug.WriteLine ($"WARNING: {Key.ToString (key)} is not a valid shortcut key.");

Terminal.Gui/Text/TextFormatter.cs

@@ -984,7 +984,7 @@ public static bool FindHotKey (string text, Rune hotKeySpecifier, bool firstUppe
 				hotPos = hot_pos;
 
 				var newHotKey = (KeyCode)hot_key.Value;
-				if (newHotKey != KeyCode.Unknown && newHotKey != KeyCode.Null && !(newHotKey == KeyCode.Space || Rune.IsControl (hot_key))) {
+				if (newHotKey != KeyCode.Null && !(newHotKey == KeyCode.Space || Rune.IsControl (hot_key))) {
 					if ((newHotKey & ~KeyCode.Space) is >= KeyCode.A and <= KeyCode.Z) {
 						newHotKey &= ~KeyCode.Space;
 					}

Terminal.Gui/View/ViewKeyboard.cs

@@ -76,7 +76,7 @@ void TextFormatter_HotKeyChanged (object sender, KeyChangedEventArgs e)
 	public virtual Key HotKey {
 		get => _hotKey;
 		set {
-			if (value is null || value.KeyCode == KeyCode.Unknown) {
+			if (value is null) {
 				throw new ArgumentException (@"HotKey must not be null. Use Key.Empty to clear the HotKey.", nameof (value));
 			}
 			if (AddKeyBindingsForHotKey (_hotKey, value)) {
@@ -115,7 +115,7 @@ public virtual bool AddKeyBindingsForHotKey (Key prevHotKey, Key hotKey)
 			return false;
 		}
 
-		var newKey = hotKey == KeyCode.Unknown ? KeyCode.Null : hotKey;
+		var newKey = hotKey;
 
 		var baseKey = newKey.NoAlt.NoShift.NoCtrl;
 		if (newKey != Key.Empty && (baseKey == Key.Space || Rune.IsControl (baseKey.AsRune))) {
@@ -195,7 +195,7 @@ void SetHotKey ()
 			return; // throw new InvalidOperationException ("Can't set HotKey unless a TextFormatter has been created");
 		}
 		if (TextFormatter.FindHotKey (_text, HotKeySpecifier, true, out _, out var hk)) {
-			if (_hotKey.KeyCode != hk && hk != KeyCode.Unknown) {
+			if (_hotKey.KeyCode != hk) {
 				HotKey = hk;
 			}
 		} else {

Terminal.Gui/Views/CheckBox.cs

@@ -107,24 +107,23 @@ void SetInitialProperties (string s, bool is_checked)
 	public override Key HotKey {
 		get => base.HotKey;
 		set {
-			if (value is null || value.KeyCode is KeyCode.Unknown) {
+			if (value is null) {
 				throw new ArgumentException (nameof (value));
 			}
 
 			var prev = base.HotKey;
 			if (prev != value) {
-				var v = value == KeyCode.Unknown ? Key.Empty: value;
-				base.HotKey = TextFormatter.HotKey = v;
+				base.HotKey = TextFormatter.HotKey = value;
 
 				// Also add Alt+HotKey
-				if (prev != (Key)KeyCode.Null && KeyBindings.TryGet (prev.WithAlt, out _)) {
-					if (v.KeyCode == KeyCode.Null) {
+				if (prev != Key.Empty && KeyBindings.TryGet (prev.WithAlt, out _)) {
+					if (value.KeyCode == KeyCode.Null) {
 						KeyBindings.Remove (prev.WithAlt);
 					} else {
-						KeyBindings.Replace (prev.WithAlt, v.WithAlt);
+						KeyBindings.Replace (prev.WithAlt, value.WithAlt);
 					}
-				} else if (v.KeyCode != KeyCode.Null) {
-					KeyBindings.Add (v.WithAlt, Command.Accept);
+				} else if (value != Key.Empty) {
+					KeyBindings.Add (value.WithAlt, Command.Accept);
 				}
 			}
 		}

Terminal.Gui/Views/Menu/Menu.cs

@@ -133,7 +133,7 @@ public KeyCode Shortcut {
 	/// <summary>
 	/// Gets the text describing the keystroke combination defined by <see cref="Shortcut"/>.
 	/// </summary>
-	public string ShortcutTag => Key.ToString (_shortcutHelper.Shortcut, MenuBar.ShortcutDelimiter);
+	public string ShortcutTag => _shortcutHelper.Shortcut == KeyCode.Null ? string.Empty : Key.ToString (_shortcutHelper.Shortcut, MenuBar.ShortcutDelimiter);
 	#endregion Keyboard Handling
 
 	/// <summary>
@@ -264,15 +264,15 @@ public void ToggleChecked ()
 		bool? previousChecked = Checked;
 		if (AllowNullChecked) {
 			switch (previousChecked) {
-				case null:
-					Checked = true;
-					break;
-				case true:
-					Checked = false;
-					break;
-				case false:
-					Checked = null;
-					break;
+			case null:
+				Checked = true;
+				break;
+			case true:
+				Checked = false;
+				break;
+			case false:
+				Checked = null;
+				break;
 			}
 		} else {
 			Checked = !Checked;
@@ -331,8 +331,8 @@ public Menu (MenuBar host, int x, int y, MenuBarItem barItems, Menu parent = nul
 
 		if (barItems == null) {
 			throw new ArgumentNullException (nameof (barItems));
-		} 
-		
+		}
+
 		_host = host;
 		_barItems = barItems;
 
@@ -432,7 +432,7 @@ void AddKeyBindings (MenuBarItem menuBarItem)
 		foreach (var menuItem in menuBarItem.Children.Where (m => m != null)) {
 			KeyBindings.Add ((KeyCode)menuItem.HotKey.Value, Command.ToggleExpandCollapse);
 			KeyBindings.Add ((KeyCode)menuItem.HotKey.Value | KeyCode.AltMask, Command.ToggleExpandCollapse);
-			if (menuItem.Shortcut != KeyCode.Unknown) {
+			if (menuItem.Shortcut != KeyCode.Null) {
 				KeyBindings.Add (menuItem.Shortcut, KeyBindingScope.HotKey, Command.Select);
 			}
 			var subMenu = menuBarItem.SubMenu (menuItem);
@@ -494,7 +494,7 @@ bool SelectOrRun ()
 
 		var key = keyEvent.KeyCode;
 
-		if (KeyBindings.TryGet(key, out _)) {
+		if (KeyBindings.TryGet (key, out _)) {
 			_menuBarItemToActivate = -1;
 			_menuItemToSelect = null;
 

Terminal.Gui/Views/Menu/MenuBar.cs

@@ -162,7 +162,7 @@ internal void AddKeyBindings (MenuBar menuBar)
 				menuBar.KeyBindings.Add ((KeyCode)menuItem.HotKey.Value, Command.ToggleExpandCollapse);
 				menuBar.KeyBindings.Add ((KeyCode)menuItem.HotKey.Value | KeyCode.AltMask, KeyBindingScope.HotKey, Command.ToggleExpandCollapse);
 			}
-			if (menuItem.Shortcut != KeyCode.Unknown && menuItem.Shortcut != KeyCode.Null) {
+			if (menuItem.Shortcut != KeyCode.Null) {
 				menuBar.KeyBindings.Add (menuItem.Shortcut, KeyBindingScope.HotKey, Command.Select);
 			}
 			SubMenu (menuItem)?.AddKeyBindings (menuBar);
@@ -323,7 +323,7 @@ public MenuBar (MenuBarItem [] menus) : base ()
 					KeyBindings.Add ((KeyCode)menuBarItem.HotKey.Value, Command.ToggleExpandCollapse);
 					KeyBindings.Add ((KeyCode)menuBarItem.HotKey.Value | KeyCode.AltMask, KeyBindingScope.HotKey, Command.ToggleExpandCollapse);
 				}
-				if (menuBarItem.Shortcut != KeyCode.Unknown && menuBarItem.Shortcut != KeyCode.Null) {
+				if (menuBarItem.Shortcut != KeyCode.Null) {
 					// Technically this will will never run because MenuBarItems don't have shortcuts
 					KeyBindings.Add (menuBarItem.Shortcut, KeyBindingScope.HotKey, Command.Select);
 				}

Terminal.Gui/Views/RadioGroup.cs

@@ -244,7 +244,7 @@ public override void OnDrawContent (Rect contentArea)
 			Driver.SetAttribute (GetNormalColor ());
 			Driver.AddStr ($"{(i == _selected ? CM.Glyphs.Selected : CM.Glyphs.UnSelected)} ");
 			TextFormatter.FindHotKey (rl, HotKeySpecifier, true, out int hotPos, out var hotKey);
-			if (hotPos != -1 && (hotKey != KeyCode.Null || hotKey != KeyCode.Unknown)) {
+			if (hotPos != -1 && (hotKey != KeyCode.Null)) {
 				var rlRunes = rl.ToRunes ();
 				for (int j = 0; j < rlRunes.Length; j++) {
 					Rune rune = rlRunes [j];