From d8446fbaf18e255056d5f3b8965c4e1e69567816 Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 22 Nov 2024 16:24:42 -0700 Subject: [PATCH] hacked CM to special case Key --- Example/Example.cs | 6 + .../Application/Application.Keyboard.cs | 10 +- Terminal.Gui/Application/Application.Run.cs | 3 +- Terminal.Gui/Configuration/ConfigProperty.cs | 53 +++--- .../Configuration/ConfigurationManager.cs | 25 ++- Terminal.Gui/Input/Key.cs | 4 +- Terminal.Gui/Input/KeyBindings.cs | 8 +- Terminal.Gui/Input/KeyEqualityComparer.cs | 35 ++++ UnitTests/Application/ApplicationTests.cs | 25 ++- .../Configuration/ConfigPropertyTests.cs | 174 ++++++++++++++++++ .../Configuration/ConfigurationMangerTests.cs | 47 ++++- .../Configuration/KeyJsonConverterTests.cs | 2 +- 12 files changed, 334 insertions(+), 58 deletions(-) create mode 100644 Terminal.Gui/Input/KeyEqualityComparer.cs create mode 100644 UnitTests/Configuration/ConfigPropertyTests.cs diff --git a/Example/Example.cs b/Example/Example.cs index 39126f4d9..1be90e063 100644 --- a/Example/Example.cs +++ b/Example/Example.cs @@ -6,6 +6,12 @@ using System; using Terminal.Gui; +ConfigurationManager.Memory = """ + { + "Application.QuitKey" : "Ctrl+Q" + } + """; + Application.Run ().Dispose (); // Before the application exits, reset Terminal.Gui for clean shutdown diff --git a/Terminal.Gui/Application/Application.Keyboard.cs b/Terminal.Gui/Application/Application.Keyboard.cs index a883dea8e..18881d6c5 100644 --- a/Terminal.Gui/Application/Application.Keyboard.cs +++ b/Terminal.Gui/Application/Application.Keyboard.cs @@ -290,7 +290,15 @@ public static partial class Application // Keyboard handling } else { - KeyBindings.ReplaceKey (oldKey, newKey); + if (KeyBindings.TryGet(oldKey, out KeyBinding binding)) + { + KeyBindings.Remove (oldKey); + KeyBindings.Add (newKey, binding); + } + else + { + KeyBindings.Add (newKey, binding); + } } } diff --git a/Terminal.Gui/Application/Application.Run.cs b/Terminal.Gui/Application/Application.Run.cs index 5c326e20c..7511f8216 100644 --- a/Terminal.Gui/Application/Application.Run.cs +++ b/Terminal.Gui/Application/Application.Run.cs @@ -1,6 +1,7 @@ #nullable enable using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Text.Json.Serialization; using Microsoft.CodeAnalysis.Diagnostics; namespace Terminal.Gui; @@ -16,7 +17,7 @@ public static partial class Application // Run (Begin, Run, End, Stop) get => _quitKey; set { - //if (_quitKey != value) + if (_quitKey != value) { ReplaceKey (_quitKey, value); _quitKey = value; diff --git a/Terminal.Gui/Configuration/ConfigProperty.cs b/Terminal.Gui/Configuration/ConfigProperty.cs index 272c22d92..5a132b48c 100644 --- a/Terminal.Gui/Configuration/ConfigProperty.cs +++ b/Terminal.Gui/Configuration/ConfigProperty.cs @@ -35,40 +35,38 @@ public class ConfigProperty /// public bool Apply () { - if (PropertyValue is { }) + try { - try + if (PropertyInfo?.GetValue (null) is { }) { - if (PropertyInfo?.GetValue (null) is { }) - { - PropertyInfo?.SetValue (null, DeepMemberWiseCopy (PropertyValue, PropertyInfo?.GetValue (null))); - } + var val = DeepMemberWiseCopy (PropertyValue, PropertyInfo?.GetValue (null)); + PropertyInfo?.SetValue (null, val); } - catch (TargetInvocationException tie) + } + catch (TargetInvocationException tie) + { + // Check if there is an inner exception + if (tie.InnerException is { }) { - // Check if there is an inner exception - if (tie.InnerException is { }) - { - // Handle the inner exception separately without catching the outer exception - Exception? innerException = tie.InnerException; + // Handle the inner exception separately without catching the outer exception + Exception? innerException = tie.InnerException; - // Handle the inner exception here - throw new JsonException ( - $"Error Applying Configuration Change: {innerException.Message}", - innerException - ); - } - - // Handle the outer exception or rethrow it if needed - throw new JsonException ($"Error Applying Configuration Change: {tie.Message}", tie); - } - catch (ArgumentException ae) - { + // Handle the inner exception here throw new JsonException ( - $"Error Applying Configuration Change ({PropertyInfo?.Name}): {ae.Message}", - ae + $"Error Applying Configuration Change: {innerException.Message}", + innerException ); } + + // Handle the outer exception or rethrow it if needed + throw new JsonException ($"Error Applying Configuration Change: {tie.Message}", tie); + } + catch (ArgumentException ae) + { + throw new JsonException ( + $"Error Applying Configuration Change ({PropertyInfo?.Name}): {ae.Message}", + ae + ); } return PropertyValue != null; @@ -95,8 +93,7 @@ public class ConfigProperty public object? RetrieveValue () { return PropertyValue = PropertyInfo!.GetValue (null); } /// - /// Updates (using reflection) with - /// the value in . + /// Updates (using reflection) with the value in using a deep memberwise copy. /// /// /// diff --git a/Terminal.Gui/Configuration/ConfigurationManager.cs b/Terminal.Gui/Configuration/ConfigurationManager.cs index 4735aa53a..e7379d259 100644 --- a/Terminal.Gui/Configuration/ConfigurationManager.cs +++ b/Terminal.Gui/Configuration/ConfigurationManager.cs @@ -291,7 +291,7 @@ public static class ConfigurationManager Settings?.Update ($"~/.tui/{AppName}.{_configFilename}"); } - if (Locations.HasFlag (ConfigLocations.Memory) && !string.IsNullOrEmpty(Memory)) + if (Locations.HasFlag (ConfigLocations.Memory) && !string.IsNullOrEmpty (Memory)) { Settings?.Update (Memory, "ConfigurationManager.Memory"); } @@ -415,7 +415,13 @@ public static class ConfigurationManager } // If value type, just use copy constructor. - if (source.GetType ().IsValueType || source.GetType () == typeof (string)) + if (source.GetType ().IsValueType || source is string) + { + return source; + } + + // HACK: Key is a class, but we want to treat it as a value type so just _keyCode gets copied. + if (source.GetType () == typeof (Key)) { return source; } @@ -426,9 +432,6 @@ public static class ConfigurationManager { foreach (object? srcKey in ((IDictionary)source).Keys) { - if (srcKey is string) - { } - if (((IDictionary)destination).Contains (srcKey)) { ((IDictionary)destination) [srcKey] = @@ -478,9 +481,10 @@ public static class ConfigurationManager } } - return destination!; + return destination; } + /// /// Retrieves the hard coded default settings (static properites) from the Terminal.Gui library implementation. Used in /// development of @@ -553,9 +557,12 @@ public static class ConfigurationManager let props = c.Value .GetProperties ( BindingFlags.Instance - | BindingFlags.Static - | BindingFlags.NonPublic - | BindingFlags.Public + | + BindingFlags.Static + | + BindingFlags.NonPublic + | + BindingFlags.Public ) .Where ( prop => diff --git a/Terminal.Gui/Input/Key.cs b/Terminal.Gui/Input/Key.cs index 204eda17a..70d078e0c 100644 --- a/Terminal.Gui/Input/Key.cs +++ b/Terminal.Gui/Input/Key.cs @@ -405,13 +405,13 @@ public class Key : EventArgs, IEquatable bool IEquatable.Equals (Key other) { return Equals (other); } /// - public override int GetHashCode () { return (int)_keyCode; } + public override int GetHashCode () { return _keyCode.GetHashCode (); } /// Compares two s for equality. /// /// /// - public static bool operator == (Key a, Key b) { return a!.Equals(b); } + public static bool operator == (Key a, Key b) { return a!.Equals (b); } /// Compares two s for not equality. /// diff --git a/Terminal.Gui/Input/KeyBindings.cs b/Terminal.Gui/Input/KeyBindings.cs index 39bcc83af..59a97bfd6 100644 --- a/Terminal.Gui/Input/KeyBindings.cs +++ b/Terminal.Gui/Input/KeyBindings.cs @@ -46,7 +46,11 @@ public class KeyBindings binding.BoundView = boundViewForAppScope; } - Bindings.Add (key, binding); + // IMPORTANT: Add a COPY of the key. This is needed because ConfigurationManager.Apply uses DeepMemberWiseCopy + // IMPORTANT: update the memory referenced by the key, and Dictionary uses caching for performance, and thus + // IMPORTANT: Apply will update the Dictionary with the new key, but the old key will still be in the dictionary. + // IMPORTANT: See the ConfigurationManager.Illustrate_DeepMemberWiseCopy_Breaks_Dictionary test for details. + Bindings.Add (new (key), binding); } /// @@ -213,7 +217,7 @@ public class KeyBindings // TODO: Add a dictionary comparer that ignores Scope // TODO: This should not be public! /// The collection of objects. - public Dictionary Bindings { get; } = new (); + public Dictionary Bindings { get; } = new (new KeyEqualityComparer ()); /// /// The view that the are bound to. diff --git a/Terminal.Gui/Input/KeyEqualityComparer.cs b/Terminal.Gui/Input/KeyEqualityComparer.cs new file mode 100644 index 000000000..fe02f13dc --- /dev/null +++ b/Terminal.Gui/Input/KeyEqualityComparer.cs @@ -0,0 +1,35 @@ +#nullable enable +using Terminal.Gui; + +/// +/// +/// +public class KeyEqualityComparer : IEqualityComparer +{ + /// + public bool Equals (Key? x, Key? y) + { + if (ReferenceEquals (x, y)) + { + return true; + } + + if (x is null || y is null) + { + return false; + } + + return x.KeyCode == y.KeyCode; + } + + /// + public int GetHashCode (Key? obj) + { + if (obj is null) + { + return 0; + } + + return obj.KeyCode.GetHashCode (); + } +} diff --git a/UnitTests/Application/ApplicationTests.cs b/UnitTests/Application/ApplicationTests.cs index 9fe0f8ef1..18689dbb9 100644 --- a/UnitTests/Application/ApplicationTests.cs +++ b/UnitTests/Application/ApplicationTests.cs @@ -11,7 +11,7 @@ public class ApplicationTests { _output = output; ConsoleDriver.RunningUnitTests = true; - ConfigurationManager.Locations = ConfigLocations.Default; + Locations = ConfigLocations.Default; #if DEBUG_IDISPOSABLE View.Instances.Clear (); @@ -273,14 +273,15 @@ public class ApplicationTests [InlineData (typeof (CursesDriver))] public void Init_ResetState_Resets_Properties (Type driverType) { - ConfigurationManager.ThrowOnJsonErrors = true; + ThrowOnJsonErrors = true; // For all the fields/properties of Application, check that they are reset to their default values // Set some values Application.Init (driverName: driverType.Name); - // Application.IsInitialized = true; + + // Application.IsInitialized = true; // Reset Application.ResetState (); @@ -371,7 +372,7 @@ public class ApplicationTests Application.ResetState (); CheckReset (); - ConfigurationManager.ThrowOnJsonErrors = false; + ThrowOnJsonErrors = false; } [Fact] @@ -399,10 +400,7 @@ public class ApplicationTests } [Fact] - public void Shutdown_Alone_Does_Nothing () - { - Application.Shutdown (); - } + public void Shutdown_Alone_Does_Nothing () { Application.Shutdown (); } [Theory] [InlineData (typeof (FakeDriver))] @@ -545,11 +543,10 @@ public class ApplicationTests ThrowOnJsonErrors = true; Memory = """ - - { - "Application.QuitKey": "Ctrl-Q" - } - """; + { + "Application.QuitKey": "Ctrl-Q" + } + """; Assert.Equal (Key.Esc, Application.QuitKey); @@ -558,6 +555,8 @@ public class ApplicationTests Assert.Equal (Key.Q.WithCtrl, Application.QuitKey); + Assert.Contains (Key.Q.WithCtrl, Application.KeyBindings.Bindings); + Application.Shutdown (); Locations = ConfigLocations.Default; } diff --git a/UnitTests/Configuration/ConfigPropertyTests.cs b/UnitTests/Configuration/ConfigPropertyTests.cs new file mode 100644 index 000000000..0bf96dc6e --- /dev/null +++ b/UnitTests/Configuration/ConfigPropertyTests.cs @@ -0,0 +1,174 @@ +using System; +using System.Reflection; +using System.Text.Json; +using System.Text.Json.Serialization; +using Terminal.Gui; +using Xunit; + +public class ConfigPropertyTests +{ + [Fact] + public void Apply_PropertyValueIsAppliedToStatic_String_Property() + { + // Arrange + TestConfiguration.Reset (); + var propertyInfo = typeof(TestConfiguration).GetProperty(nameof(TestConfiguration.TestStringProperty)); + var configProperty = new ConfigProperty + { + PropertyInfo = propertyInfo, + PropertyValue = "UpdatedValue" + }; + + // Act + var result = configProperty.Apply(); + + // Assert + Assert.Equal (1, TestConfiguration.TestStringPropertySetCount); + Assert.True(result); + Assert.Equal("UpdatedValue", TestConfiguration.TestStringProperty); + TestConfiguration.Reset (); + } + + [Fact] + public void Apply_PropertyValueIsAppliedToStatic_Key_Property () + { + // Arrange + TestConfiguration.Reset (); + var propertyInfo = typeof (TestConfiguration).GetProperty (nameof (TestConfiguration.TestKeyProperty)); + var configProperty = new ConfigProperty + { + PropertyInfo = propertyInfo, + PropertyValue = Key.Q.WithCtrl + }; + + // Act + var result = configProperty.Apply (); + + // Assert + Assert.Equal(1, TestConfiguration.TestKeyPropertySetCount); + Assert.True (result); + Assert.Equal (Key.Q.WithCtrl, TestConfiguration.TestKeyProperty); + TestConfiguration.Reset (); + } + + [Fact] + public void RetrieveValue_GetsCurrentValueOfStaticProperty() + { + // Arrange + TestConfiguration.TestStringProperty = "CurrentValue"; + var propertyInfo = typeof(TestConfiguration).GetProperty(nameof(TestConfiguration.TestStringProperty)); + var configProperty = new ConfigProperty + { + PropertyInfo = propertyInfo + }; + + // Act + var value = configProperty.RetrieveValue(); + + // Assert + Assert.Equal("CurrentValue", value); + Assert.Equal("CurrentValue", configProperty.PropertyValue); + } + + [Fact] + public void UpdateValueFrom_Updates_String_Property_Value () + { + // Arrange + TestConfiguration.Reset (); + var propertyInfo = typeof(TestConfiguration).GetProperty(nameof(TestConfiguration.TestStringProperty)); + var configProperty = new ConfigProperty + { + PropertyInfo = propertyInfo, + PropertyValue = "InitialValue" + }; + + // Act + var updatedValue = configProperty.UpdateValueFrom("NewValue"); + + // Assert + Assert.Equal (0, TestConfiguration.TestStringPropertySetCount); + Assert.Equal("NewValue", updatedValue); + Assert.Equal("NewValue", configProperty.PropertyValue); + TestConfiguration.Reset (); + } + + //[Fact] + //public void UpdateValueFrom_InvalidType_ThrowsArgumentException() + //{ + // // Arrange + // var propertyInfo = typeof(TestConfiguration).GetProperty(nameof(TestConfiguration.TestStringProperty)); + // var configProperty = new ConfigProperty + // { + // PropertyInfo = propertyInfo + // }; + + // // Act & Assert + // Assert.Throws(() => configProperty.UpdateValueFrom(123)); + //} + + [Fact] + public void Apply_TargetInvocationException_ThrowsJsonException() + { + // Arrange + var propertyInfo = typeof(TestConfiguration).GetProperty(nameof(TestConfiguration.TestStringProperty)); + var configProperty = new ConfigProperty + { + PropertyInfo = propertyInfo, + PropertyValue = null // This will cause ArgumentNullException in the set accessor + }; + + // Act & Assert + var exception = Assert.Throws (() => configProperty.Apply()); + } + + [Fact] + public void GetJsonPropertyName_ReturnsJsonPropertyNameAttributeValue() + { + // Arrange + var propertyInfo = typeof(TestConfiguration).GetProperty(nameof(TestConfiguration.TestStringProperty)); + + // Act + var jsonPropertyName = ConfigProperty.GetJsonPropertyName(propertyInfo); + + // Assert + Assert.Equal("TestStringProperty", jsonPropertyName); + } +} + +public class TestConfiguration +{ + private static string _testStringProperty = "Default"; + public static int TestStringPropertySetCount { get; set; } + + [SerializableConfigurationProperty] + public static string TestStringProperty + { + get => _testStringProperty; + set + { + TestStringPropertySetCount++; + _testStringProperty = value ?? throw new ArgumentNullException (nameof (value)); + } + } + + private static Key _testKeyProperty = Key.Esc; + + public static int TestKeyPropertySetCount { get; set; } + + [SerializableConfigurationProperty] + public static Key TestKeyProperty + { + get => _testKeyProperty; + set + { + TestKeyPropertySetCount++; + _testKeyProperty = value ?? throw new ArgumentNullException (nameof (value)); + } + } + + public static void Reset () + { + TestStringPropertySetCount = 0; + TestKeyPropertySetCount = 0; + } +} diff --git a/UnitTests/Configuration/ConfigurationMangerTests.cs b/UnitTests/Configuration/ConfigurationMangerTests.cs index 0b3d05601..945782d3f 100644 --- a/UnitTests/Configuration/ConfigurationMangerTests.cs +++ b/UnitTests/Configuration/ConfigurationMangerTests.cs @@ -1,4 +1,5 @@ -using System.Reflection; +using System.Diagnostics; +using System.Reflection; using System.Text.Json; using Xunit.Abstractions; using static Terminal.Gui.ConfigurationManager; @@ -146,6 +147,50 @@ public class ConfigurationManagerTests Assert.Equal (dictDest ["Normal"], dictCopy ["Normal"]); } + public class DeepCopyTest () + { + public static Key key = Key.Esc; + } + + [Fact] + public void Illustrate_DeepMemberWiseCopy_Breaks_Dictionary () + { + Assert.Equal (Key.Esc, DeepCopyTest.key); + + Dictionary dict = new Dictionary (new KeyEqualityComparer ()); + dict.Add (new (DeepCopyTest.key), "Esc"); + Assert.Contains (Key.Esc, dict); + + DeepMemberWiseCopy (Key.Q.WithCtrl, DeepCopyTest.key); + + Assert.Equal (Key.Q.WithCtrl, DeepCopyTest.key); + Assert.Equal (Key.Esc, dict.Keys.ToArray () [0]); + + var eq = new KeyEqualityComparer (); + Assert.True (eq.Equals (Key.Q.WithCtrl, DeepCopyTest.key)); + Assert.Equal (Key.Q.WithCtrl.GetHashCode (), DeepCopyTest.key.GetHashCode ()); + Assert.Equal (eq.GetHashCode (Key.Q.WithCtrl), eq.GetHashCode (DeepCopyTest.key)); + Assert.Equal (Key.Q.WithCtrl.GetHashCode (), eq.GetHashCode (DeepCopyTest.key)); + Assert.True (dict.ContainsKey (Key.Esc)); + + dict.Remove (Key.Esc); + dict.Add (new (DeepCopyTest.key), "Ctrl+Q"); + Assert.True (dict.ContainsKey (Key.Q.WithCtrl)); + } + + //[Fact] + //public void Illustrate_DeepMemberWiseCopy_ () + //{ + // Assert.Equal (Key.Esc, Application.QuitKey); + + // var o = UpdateValueFrom (Application.QuitKey); + // DeepMemberWiseCopy (Key.Q.WithCtrl, Application.QuitKey); + + // Assert.Equal (Key.Q.WithCtrl, Application.QuitKey); + + // Application.ResetState (true); + //} + [Fact] public void Load_Raises_Updated () { diff --git a/UnitTests/Configuration/KeyJsonConverterTests.cs b/UnitTests/Configuration/KeyJsonConverterTests.cs index eba845feb..b30b5a30a 100644 --- a/UnitTests/Configuration/KeyJsonConverterTests.cs +++ b/UnitTests/Configuration/KeyJsonConverterTests.cs @@ -59,7 +59,7 @@ public class KeyJsonConverterTests Key key = Key.Q.WithCtrl; // Act - string json = """Ctrl+Q"""; + string json = "\"Ctrl+Q\""; Key deserializedKey = JsonSerializer.Deserialize (json, ConfigurationManager._serializerOptions); // Assert