From ebb1076dd00581b45fa18e3f37bf0b4944cf8e63 Mon Sep 17 00:00:00 2001 From: Phil Scott Date: Sun, 6 Jun 2021 22:46:25 -0400 Subject: [PATCH] Check for default value before writing property values Previous version was overwriting values that might have been set via a property initializer. Closes #422 --- ...ptionalArgumentWithDefaultValueSettings.cs | 7 +++ .../Unit/Cli/CommandAppTests.cs | 44 +++++++++++++++++++ .../Internal/Binding/CommandPropertyBinder.cs | 5 ++- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/Spectre.Console.Tests/Data/Settings/OptionalArgumentWithDefaultValueSettings.cs b/src/Spectre.Console.Tests/Data/Settings/OptionalArgumentWithDefaultValueSettings.cs index b5b63e2..2a2b58d 100644 --- a/src/Spectre.Console.Tests/Data/Settings/OptionalArgumentWithDefaultValueSettings.cs +++ b/src/Spectre.Console.Tests/Data/Settings/OptionalArgumentWithDefaultValueSettings.cs @@ -1,3 +1,4 @@ +using System; using System.ComponentModel; using Spectre.Console.Cli; @@ -10,6 +11,12 @@ namespace Spectre.Console.Tests.Data public string Greeting { get; set; } } + public sealed class OptionalArgumentWithPropertyInitializerSettings : CommandSettings + { + [CommandArgument(0, "[NAMES]")] + public string[] Names { get; set; } = Array.Empty(); + } + public sealed class OptionalArgumentWithDefaultValueAndTypeConverterSettings : CommandSettings { [CommandArgument(0, "[GREETING]")] diff --git a/src/Spectre.Console.Tests/Unit/Cli/CommandAppTests.cs b/src/Spectre.Console.Tests/Unit/Cli/CommandAppTests.cs index 63fa00d..8fe01b9 100644 --- a/src/Spectre.Console.Tests/Unit/Cli/CommandAppTests.cs +++ b/src/Spectre.Console.Tests/Unit/Cli/CommandAppTests.cs @@ -244,6 +244,50 @@ namespace Spectre.Console.Tests.Unit.Cli }); } + [Fact] + public void Should_Assign_Property_Initializer_To_Optional_Argument() + { + // Given + var app = new CommandAppTester(); + app.SetDefaultCommand>(); + app.Configure(config => + { + config.PropagateExceptions(); + }); + + // When + var result = app.Run(Array.Empty()); + + // Then + result.ExitCode.ShouldBe(0); + result.Settings + .ShouldBeOfType() + .And(settings => settings.Names.ShouldNotBeNull()) + .And(settings => settings.Names.ShouldBeEmpty()); + } + + [Fact] + public void Should_Overwrite_Property_Initializer_With_Argument_Value() + { + // Given + var app = new CommandAppTester(); + app.SetDefaultCommand>(); + app.Configure(config => + { + config.PropagateExceptions(); + }); + + // When + var result = app.Run("ABBA", "Herreys"); + + // Then + result.ExitCode.ShouldBe(0); + result.Settings + .ShouldBeOfType() + .And(settings => settings.Names.ShouldContain("ABBA")) + .And(settings => settings.Names.ShouldContain("Herreys")); + } + [Fact] public void Should_Assign_Default_Value_To_Optional_Argument_Using_Converter_If_Necessary() { diff --git a/src/Spectre.Console/Cli/Internal/Binding/CommandPropertyBinder.cs b/src/Spectre.Console/Cli/Internal/Binding/CommandPropertyBinder.cs index a7f4cbb..0277e78 100644 --- a/src/Spectre.Console/Cli/Internal/Binding/CommandPropertyBinder.cs +++ b/src/Spectre.Console/Cli/Internal/Binding/CommandPropertyBinder.cs @@ -10,7 +10,10 @@ namespace Spectre.Console.Cli foreach (var (parameter, value) in lookup) { - parameter.Property.SetValue(settings, value); + if (value != default) + { + parameter.Property.SetValue(settings, value); + } } // Validate the settings.