Terminal.Gui
Terminal.Gui copied to clipboard
`ConfigurationManager` only adds, doesn't remove from `Dictionary`-based items
When I coded this, I failed to consider what happens if an item is removed from a dictionary.
To repro:
- Run UI Catalog
- While running, add a new
Themenamed "UI Catalog Theme 2" to./.tui/config.json - Note Themes menu now also shows "UI Catalog Theme 2"
- Now remove that added Theme and save the config file
Expected Behavior: "UI Catalog Theme 2" disappears
Actual Behavior: "UI Catalog Theme 2" is still in the Themes dictionary.
The problem is in ConfigProperty.cs where Update does a deep memberwise copy:
internal object? UpdateValueFrom (object source)
{
if (source == null) {
return PropertyValue;
}
var ut = Nullable.GetUnderlyingType (PropertyInfo!.PropertyType);
if (source.GetType () != PropertyInfo!.PropertyType && (ut != null && source.GetType () != ut)) {
throw new ArgumentException ($"The source object ({PropertyInfo!.DeclaringType}.{PropertyInfo!.Name}) is not of type {PropertyInfo!.PropertyType}.");
}
if (PropertyValue != null && source != null) {
PropertyValue = ConfigurationManager.DeepMemberwiseCopy (source, PropertyValue);
} else {
PropertyValue = source;
}
return PropertyValue;
}
DeepMemberwiseCopy is really an "append" operation:
/// <summary>
/// System.Text.Json does not support copying a deserialized object to an existing instance.
/// To work around this, we implement a 'deep, memberwise copy' method.
/// </summary>
/// <remarks>
/// TOOD: When System.Text.Json implements `PopulateObject` revisit
/// https://github.com/dotnet/corefx/issues/37627
/// </remarks>
/// <param name="source"></param>
/// <param name="destination"></param>
/// <returns><paramref name="destination"/> updated from <paramref name="source"/></returns>
internal static object? DeepMemberwiseCopy (object? source, object? destination)
{
if (destination == null) {
throw new ArgumentNullException (nameof (destination));
}
if (source == null) {
return null!;
}
if (source.GetType () == typeof (SettingsScope)) {
return ((SettingsScope)destination).Update ((SettingsScope)source);
}
if (source.GetType () == typeof (ThemeScope)) {
return ((ThemeScope)destination).Update ((ThemeScope)source);
}
if (source.GetType () == typeof (AppScope)) {
return ((AppScope)destination).Update ((AppScope)source);
}
// If value type, just use copy constructor.
if (source.GetType ().IsValueType || source.GetType () == typeof (string)) {
return source;
}
// Dictionary
if (source.GetType ().IsGenericType && source.GetType ().GetGenericTypeDefinition ().IsAssignableFrom (typeof (Dictionary<,>))) {
foreach (var srcKey in ((IDictionary)source).Keys) {
if (srcKey is string) {
}
if (((IDictionary)destination).Contains (srcKey))
((IDictionary)destination) [srcKey] = DeepMemberwiseCopy (((IDictionary)source) [srcKey], ((IDictionary)destination) [srcKey]);
else {
((IDictionary)destination).Add (srcKey, ((IDictionary)source) [srcKey]);
}
}
return destination;
}
// ALl other object types
var sourceProps = source?.GetType ().GetProperties ().Where (x => x.CanRead).ToList ();
var destProps = destination?.GetType ().GetProperties ().Where (x => x.CanWrite).ToList ()!;
foreach (var (sourceProp, destProp) in
from sourceProp in sourceProps
where destProps.Any (x => x.Name == sourceProp.Name)
let destProp = destProps.First (x => x.Name == sourceProp.Name)
where destProp.CanWrite
select (sourceProp, destProp)) {
var sourceVal = sourceProp.GetValue (source);
var destVal = destProp.GetValue (destination);
if (sourceVal != null) {
if (destVal != null) {
// Recurse
destProp.SetValue (destination, DeepMemberwiseCopy (sourceVal, destVal));
} else {
destProp.SetValue (destination, sourceVal);
}
}
}
return destination!;
}
It really should be a "merge" operation, but my brain hurts when thinking how to make that work.