Merge branch 'v2_3841-ConfigManager' of tig:tig/Terminal.Gui into v2_3841-ConfigManager

Example/Example.cs

@@ -6,6 +6,12 @@
 using System;
 using Terminal.Gui;
 
+ConfigurationManager.Memory = """
+                              {
+                              "Application.QuitKey" : "Ctrl+Q"
+                              }
+                              """;
+
 Application.Run<ExampleWindow> ().Dispose ();
 
 // Before the application exits, reset Terminal.Gui for clean shutdown

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);
+            }
         }
     }
 

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;

Terminal.Gui/Configuration/ConfigProperty.cs

@@ -35,40 +35,38 @@ public class ConfigProperty
     /// <returns></returns>
     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); }
 
     /// <summary>
-    ///     Updates (using reflection) <see cref="PropertyValue"/> with
-    ///     the value in <paramref name="source"/>.
+    ///     Updates (using reflection) <see cref="PropertyValue"/> with the value in <paramref name="source"/> using a deep memberwise copy.
     /// </summary>
     /// <param name="source"></param>
     /// <returns></returns>

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;
     }
 
+
     /// <summary>
     ///     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 =>

Terminal.Gui/Input/Key.cs

@@ -405,13 +405,13 @@ public class Key : EventArgs, IEquatable<Key>
     bool IEquatable<Key>.Equals (Key other) { return Equals (other); }
 
     /// <inheritdoc/>
-    public override int GetHashCode () { return (int)_keyCode; }
+    public override int GetHashCode () { return _keyCode.GetHashCode (); }
 
     /// <summary>Compares two <see cref="Key"/>s for equality.</summary>
     /// <param name="a"></param>
     /// <param name="b"></param>
     /// <returns></returns>
-    public static bool operator == (Key a, Key b) { return a!.Equals(b); }
+    public static bool operator == (Key a, Key b) { return a!.Equals (b); }
 
     /// <summary>Compares two <see cref="Key"/>s for not equality.</summary>
     /// <param name="a"></param>

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);
     }
 
     /// <summary>
@@ -213,7 +217,7 @@ public class KeyBindings
     // TODO: Add a dictionary comparer that ignores Scope
     // TODO: This should not be public!
     /// <summary>The collection of <see cref="KeyBinding"/> objects.</summary>
-    public Dictionary<Key, KeyBinding> Bindings { get; } = new ();
+    public Dictionary<Key, KeyBinding> Bindings { get; } = new (new KeyEqualityComparer ());
 
     /// <summary>
     ///     The view that the <see cref="KeyBindings"/> are bound to.

Terminal.Gui/Input/KeyEqualityComparer.cs

@@ -0,0 +1,35 @@
+#nullable enable
+using Terminal.Gui;
+
+/// <summary>
+/// 
+/// </summary>
+public class KeyEqualityComparer : IEqualityComparer<Key>
+{
+    /// <inheritdoc />
+    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;
+    }
+
+    /// <inheritdoc />
+    public int GetHashCode (Key? obj)
+    {
+        if (obj is null)
+        {
+            return 0;
+        }
+
+        return obj.KeyCode.GetHashCode ();
+    }
+}

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;
     }

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<ArgumentException>(() => 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<JsonException> (() => 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;
+    }
+}

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<Key, string> dict = new Dictionary<Key, string> (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 ()
     {

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<Key> (json, ConfigurationManager._serializerOptions);
 
         // Assert