diff --git a/Terminal.Gui/Application/Application.Keyboard.cs b/Terminal.Gui/Application/Application.Keyboard.cs index 0981bbd2d..48170eb22 100644 --- a/Terminal.Gui/Application/Application.Keyboard.cs +++ b/Terminal.Gui/Application/Application.Keyboard.cs @@ -5,7 +5,7 @@ namespace Terminal.Gui; public static partial class Application // Keyboard handling { - private static Key _nextTabKey = Key.Empty; // Defined in config.json + private static Key _nextTabKey = Key.Tab; // Resources/config.json overrrides /// Alternative key to navigate forwards through views. Ctrl+Tab is the primary key. [SerializableConfigurationProperty (Scope = typeof (SettingsScope))] @@ -17,22 +17,13 @@ public static partial class Application // Keyboard handling { if (_nextTabKey != value) { - Key oldKey = _nextTabKey; + ReplaceKey (_nextTabKey, value); _nextTabKey = value; - - if (_nextTabKey == Key.Empty) - { - KeyBindings.Remove (_nextTabKey); - } - else - { - KeyBindings.ReplaceKey (oldKey, _nextTabKey); - } } } } - private static Key _prevTabKey = Key.Empty; // Defined in config.json + private static Key _prevTabKey = Key.Tab.WithShift; // Resources/config.json overrrides /// Alternative key to navigate backwards through views. Shift+Ctrl+Tab is the primary key. [SerializableConfigurationProperty (Scope = typeof (SettingsScope))] @@ -44,22 +35,13 @@ public static partial class Application // Keyboard handling { if (_prevTabKey != value) { - Key oldKey = _prevTabKey; + ReplaceKey (_prevTabKey, value); _prevTabKey = value; - - if (_prevTabKey == Key.Empty) - { - KeyBindings.Remove (_prevTabKey); - } - else - { - KeyBindings.ReplaceKey (oldKey, _prevTabKey); - } } } } - private static Key _nextTabGroupKey = Key.Empty; // Defined in config.json + private static Key _nextTabGroupKey = Key.F6; // Resources/config.json overrrides /// Alternative key to navigate forwards through views. Ctrl+Tab is the primary key. [SerializableConfigurationProperty (Scope = typeof (SettingsScope))] @@ -71,22 +53,13 @@ public static partial class Application // Keyboard handling { if (_nextTabGroupKey != value) { - Key oldKey = _nextTabGroupKey; + ReplaceKey (_nextTabGroupKey, value); _nextTabGroupKey = value; - - if (_nextTabGroupKey == Key.Empty) - { - KeyBindings.Remove (_nextTabGroupKey); - } - else - { - KeyBindings.ReplaceKey (oldKey, _nextTabGroupKey); - } } } } - private static Key _prevTabGroupKey = Key.Empty; // Defined in config.json + private static Key _prevTabGroupKey = Key.F6.WithShift; // Resources/config.json overrrides /// Alternative key to navigate backwards through views. Shift+Ctrl+Tab is the primary key. [SerializableConfigurationProperty (Scope = typeof (SettingsScope))] @@ -98,22 +71,13 @@ public static partial class Application // Keyboard handling { if (_prevTabGroupKey != value) { - Key oldKey = _prevTabGroupKey; + ReplaceKey (_prevTabGroupKey, value); _prevTabGroupKey = value; - - if (_prevTabGroupKey == Key.Empty) - { - KeyBindings.Remove (_prevTabGroupKey); - } - else - { - KeyBindings.ReplaceKey (oldKey, _prevTabGroupKey); - } } } } - private static Key _quitKey = Key.Empty; // Defined in config.json + private static Key _quitKey = Key.Esc; // Resources/config.json overrrides /// Gets or sets the key to quit the application. [SerializableConfigurationProperty (Scope = typeof (SettingsScope))] @@ -125,21 +89,29 @@ public static partial class Application // Keyboard handling { if (_quitKey != value) { - Key oldKey = _quitKey; + ReplaceKey (_quitKey, value); _quitKey = value; - - if (_quitKey == Key.Empty) - { - KeyBindings.Remove (_quitKey); - } - else - { - KeyBindings.ReplaceKey (oldKey, _quitKey); - } } } } + private static void ReplaceKey (Key oldKey, Key newKey) + { + if (KeyBindings.Bindings.Count == 0) + { + return; + } + + if (newKey == Key.Empty) + { + KeyBindings.Remove (oldKey); + } + else + { + KeyBindings.ReplaceKey (oldKey, newKey); + } + } + /// /// Event fired when the user presses a key. Fired by . /// @@ -413,6 +385,13 @@ public static partial class Application // Keyboard handling KeyBindings.Clear (); + // Resources/config.json overrrides + NextTabKey = Key.Tab; + PrevTabKey = Key.Tab.WithShift; + NextTabGroupKey = Key.F6; + PrevTabGroupKey = Key.F6.WithShift; + QuitKey = Key.Esc; + KeyBindings.Add (QuitKey, KeyBindingScope.Application, Command.QuitToplevel); KeyBindings.Add (Key.CursorRight, KeyBindingScope.Application, Command.NextView); diff --git a/Terminal.Gui/Application/Application.cs b/Terminal.Gui/Application/Application.cs index e5332ee7f..e3912475f 100644 --- a/Terminal.Gui/Application/Application.cs +++ b/Terminal.Gui/Application/Application.cs @@ -142,14 +142,12 @@ public static partial class Application UnGrabbedMouse = null; // Keyboard - PrevTabGroupKey = Key.Empty; - NextTabGroupKey = Key.Empty; - QuitKey = Key.Empty; KeyDown = null; KeyUp = null; SizeChanging = null; Navigation = null; + AddApplicationKeyBindings (); Colors.Reset (); diff --git a/Terminal.Gui/Input/KeyBindings.cs b/Terminal.Gui/Input/KeyBindings.cs index 89ffde7ad..5dd69a260 100644 --- a/Terminal.Gui/Input/KeyBindings.cs +++ b/Terminal.Gui/Input/KeyBindings.cs @@ -136,10 +136,9 @@ public class KeyBindings throw new ArgumentException ("Application scoped KeyBindings must be added via Application.KeyBindings.Add"); } - if (key is null || !key.IsValid) + if (key == Key.Empty || !key.IsValid) { - //throw new ArgumentException ("Invalid Key", nameof (commands)); - return; + throw new ArgumentException (@"Invalid Key", nameof (commands)); } if (commands.Length == 0) @@ -150,7 +149,6 @@ public class KeyBindings if (TryGet (key, out KeyBinding binding)) { throw new InvalidOperationException (@$"A key binding for {key} exists ({binding})."); - //Bindings [key] = new (commands, scope, BoundView); } else { @@ -313,12 +311,17 @@ public class KeyBindings /// Replaces a key combination already bound to a set of s. /// /// The key to be replaced. - /// The new key to be used. + /// The new key to be used. If no action will be taken. public void ReplaceKey (Key oldKey, Key newKey) { if (!TryGet (oldKey, out KeyBinding _)) { - return; + throw new InvalidOperationException ($"Key {oldKey} is not bound."); + } + + if (!newKey.IsValid) + { + throw new InvalidOperationException ($"Key {newKey} is is not valid."); } KeyBinding value = Bindings [oldKey]; diff --git a/Terminal.Gui/Views/Shortcut.cs b/Terminal.Gui/Views/Shortcut.cs index c5e023ee7..6530d71bc 100644 --- a/Terminal.Gui/Views/Shortcut.cs +++ b/Terminal.Gui/Views/Shortcut.cs @@ -681,7 +681,7 @@ public class Shortcut : View, IOrientation, IDesignable private void UpdateKeyBinding (Key oldKey) { - if (Key != null) + if (Key != null && Key.IsValid) { // Disable the command view key bindings CommandView.KeyBindings.Remove (Key); diff --git a/Terminal.Gui/Views/TableView/TableView.cs b/Terminal.Gui/Views/TableView/TableView.cs index a1b69cb65..10c53513d 100644 --- a/Terminal.Gui/Views/TableView/TableView.cs +++ b/Terminal.Gui/Views/TableView/TableView.cs @@ -324,11 +324,15 @@ public class TableView : View { if (cellActivationKey != value) { - KeyBindings.ReplaceKey (cellActivationKey, value); + if (KeyBindings.TryGet (cellActivationKey, out _)) + { + KeyBindings.ReplaceKey (cellActivationKey, value); + } + else + { + KeyBindings.Add (value, Command.Accept); + } - // of API user is mixing and matching old and new methods of keybinding then they may have lost - // the old binding (e.g. with ClearKeybindings) so KeyBindings.Replace alone will fail - KeyBindings.Add (value, Command.Accept); cellActivationKey = value; } } @@ -792,7 +796,7 @@ public class TableView : View } /// - protected internal override bool OnMouseEvent (MouseEvent me) + protected internal override bool OnMouseEvent (MouseEvent me) { if (!me.Flags.HasFlag (MouseFlags.Button1Clicked) && !me.Flags.HasFlag (MouseFlags.Button1DoubleClicked) diff --git a/UnitTests/Application/ApplicationTests.cs b/UnitTests/Application/ApplicationTests.cs index ce03ac11f..b84dc8ae8 100644 --- a/UnitTests/Application/ApplicationTests.cs +++ b/UnitTests/Application/ApplicationTests.cs @@ -183,9 +183,11 @@ public class ApplicationTests Assert.Null (Application.Driver); Assert.Null (Application.MainLoop); Assert.False (Application.EndAfterFirstIteration); - Assert.Equal (Key.Empty, Application.PrevTabGroupKey); - Assert.Equal (Key.Empty, Application.NextTabGroupKey); - Assert.Equal (Key.Empty, Application.QuitKey); + Assert.Equal (Key.Tab.WithShift, Application.PrevTabKey); + Assert.Equal (Key.Tab, Application.NextTabKey); + Assert.Equal (Key.F6.WithShift, Application.PrevTabGroupKey); + Assert.Equal (Key.F6, Application.NextTabGroupKey); + Assert.Equal (Key.Esc, Application.QuitKey); Assert.Null (ApplicationOverlapped.OverlappedChildren); Assert.Null (ApplicationOverlapped.OverlappedTop); @@ -236,7 +238,7 @@ public class ApplicationTests Application.PrevTabGroupKey = Key.A; Application.NextTabGroupKey = Key.B; Application.QuitKey = Key.C; - Application.KeyBindings.Add (Key.A, KeyBindingScope.Application, Command.Cancel); + Application.KeyBindings.Add (Key.D, KeyBindingScope.Application, Command.Cancel); //ApplicationOverlapped.OverlappedChildren = new List (); //ApplicationOverlapped.OverlappedTop = diff --git a/UnitTests/Application/KeyboardTests.cs b/UnitTests/Application/KeyboardTests.cs index 899420d94..2f6d85d00 100644 --- a/UnitTests/Application/KeyboardTests.cs +++ b/UnitTests/Application/KeyboardTests.cs @@ -65,7 +65,7 @@ public class KeyboardTests { Application.ResetState (true); // Before Init - Assert.Equal (Key.Empty, Application.QuitKey); + Assert.Equal (Key.Esc, Application.QuitKey); Application.Init (new FakeDriver ()); // After Init diff --git a/UnitTests/Configuration/SettingsScopeTests.cs b/UnitTests/Configuration/SettingsScopeTests.cs index 5525e530a..ca4f6b830 100644 --- a/UnitTests/Configuration/SettingsScopeTests.cs +++ b/UnitTests/Configuration/SettingsScopeTests.cs @@ -29,9 +29,9 @@ public class SettingsScopeTests Settings.Apply (); // assert - Assert.Equal (KeyCode.Q, Application.QuitKey.KeyCode); - Assert.Equal (KeyCode.F, Application.NextTabGroupKey.KeyCode); - Assert.Equal (KeyCode.B, Application.PrevTabGroupKey.KeyCode); + Assert.Equal (Key.Q, Application.QuitKey); + Assert.Equal (Key.F, Application.NextTabGroupKey); + Assert.Equal (Key.B, Application.PrevTabGroupKey); } [Fact] diff --git a/UnitTests/Input/KeyBindingTests.cs b/UnitTests/Input/KeyBindingTests.cs index bf3b007fd..077fc38f1 100644 --- a/UnitTests/Input/KeyBindingTests.cs +++ b/UnitTests/Input/KeyBindingTests.cs @@ -8,11 +8,20 @@ public class KeyBindingTests public KeyBindingTests (ITestOutputHelper output) { _output = output; } [Fact] - public void Add_Empty_Throws () + public void Add_No_Commands_Throws () { var keyBindings = new KeyBindings (); List commands = new (); Assert.Throws (() => keyBindings.Add (Key.A, commands.ToArray ())); + + } + + [Fact] + public void Add_Invalid_Key_Throws () + { + var keyBindings = new KeyBindings (); + List commands = new (); + Assert.Throws (() => keyBindings.Add (Key.Empty, KeyBindingScope.HotKey, Command.Accept)); } [Fact] @@ -193,7 +202,7 @@ public class KeyBindingTests } [Fact] - public void Replace_Key () + public void ReplaceKey_Replaces () { var keyBindings = new KeyBindings (); keyBindings.Add (Key.A, KeyBindingScope.Application, Command.HotKey); @@ -218,6 +227,21 @@ public class KeyBindingTests Assert.Contains (Command.HotKey, keyBindings.GetCommands (Key.H)); } + [Fact] + public void ReplaceKey_Throws_If_DoesNotContain_Old () + { + var keyBindings = new KeyBindings (); + Assert.Throws (() => keyBindings.ReplaceKey (Key.A, Key.B)); + } + + [Fact] + public void ReplaceKey_Throws_If_New_Is_Empty () + { + var keyBindings = new KeyBindings (); + keyBindings.Add (Key.A, KeyBindingScope.Application, Command.HotKey); + Assert.Throws (() => keyBindings.ReplaceKey (Key.A, Key.Empty)); + } + // Add with scope does the right things [Theory] [InlineData (KeyBindingScope.Focused)]