From 851779a68c914d1dcb6ca7e8d8612036fafef606 Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Sun, 31 May 2015 16:44:04 -0500 Subject: [PATCH 1/9] Fixed failed parsing on nullable options Added unit test for nullable types --- .../CommandLine.Tests.csproj | 1 + .../Fakes/FakeOptionsWithNullable.cs | 11 ++++++++++ src/CommandLine.Tests/Unit/ParserTests.cs | 20 +++++++++++++++++++ src/CommandLine/Core/TypeConverter.cs | 2 ++ 4 files changed, 34 insertions(+) create mode 100644 src/CommandLine.Tests/Fakes/FakeOptionsWithNullable.cs diff --git a/src/CommandLine.Tests/CommandLine.Tests.csproj b/src/CommandLine.Tests/CommandLine.Tests.csproj index c6ae4597..6a84be75 100644 --- a/src/CommandLine.Tests/CommandLine.Tests.csproj +++ b/src/CommandLine.Tests/CommandLine.Tests.csproj @@ -58,6 +58,7 @@ Properties\SharedAssemblyInfo.cs + diff --git a/src/CommandLine.Tests/Fakes/FakeOptionsWithNullable.cs b/src/CommandLine.Tests/Fakes/FakeOptionsWithNullable.cs new file mode 100644 index 00000000..92906f2b --- /dev/null +++ b/src/CommandLine.Tests/Fakes/FakeOptionsWithNullable.cs @@ -0,0 +1,11 @@ +namespace CommandLine.Tests.Fakes +{ + class FakeOptionsWithNullable + { + [Option('n')] + public int? NullableIntValue { get; set; } + + [Option('c')] + public Colors? NullableColorsValue { get; set; } + } +} diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index 524e87a4..ccb7ba1a 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -227,5 +227,25 @@ public void Parse_verbs_using_generic_overload() Assert.False(result.Errors.Any()); // Teardown } + + [Fact] + public void Parse_nullable_options() + { + // Fixture setup + var expectedOptions = new FakeOptionsWithNullable + { + NullableIntValue = 60, + NullableColorsValue = Colors.Red + }; + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new[] { "-n", "60", "-c", "Red" }); + + // Verify outcome + result.Value.ShouldHave().AllProperties().EqualTo(expectedOptions); + Assert.False(result.Errors.Any()); + // Teardown + } } } diff --git a/src/CommandLine/Core/TypeConverter.cs b/src/CommandLine/Core/TypeConverter.cs index 65321659..853e976e 100644 --- a/src/CommandLine/Core/TypeConverter.cs +++ b/src/CommandLine/Core/TypeConverter.cs @@ -41,6 +41,8 @@ private static Maybe ChangeType(string value, Type conversionType, Cultu { try { + conversionType = Nullable.GetUnderlyingType(conversionType) ?? conversionType; + return Maybe.Just( MatchBoolString(value) ? ConvertBoolString(value) From e2610bd2022bf5ad55be5e346a807fbdd7f1878d Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Sun, 31 May 2015 18:26:08 -0500 Subject: [PATCH 2/9] Allow null Option.DefaultValue --- src/CommandLine.Tests/CommandLine.Tests.csproj | 1 + .../Fakes/FakeOptionsWithNullDefault.cs | 15 +++++++++++++++ src/CommandLine.Tests/Unit/ParserTests.cs | 14 ++++++++++++++ src/CommandLine/Core/InstanceBuilder.cs | 12 ++++++------ src/CommandLine/OptionAttribute.cs | 10 +--------- 5 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs diff --git a/src/CommandLine.Tests/CommandLine.Tests.csproj b/src/CommandLine.Tests/CommandLine.Tests.csproj index 6a84be75..20e42fd8 100644 --- a/src/CommandLine.Tests/CommandLine.Tests.csproj +++ b/src/CommandLine.Tests/CommandLine.Tests.csproj @@ -59,6 +59,7 @@ Properties\SharedAssemblyInfo.cs + diff --git a/src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs b/src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs new file mode 100644 index 00000000..630f10f5 --- /dev/null +++ b/src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs @@ -0,0 +1,15 @@ +using System.Collections.Generic; + +namespace CommandLine.Tests.Fakes +{ + class FakeOptionsWithNullDefault + { + [Option('i', DefaultValue = null)] + public IEnumerable IntSequence + { + get { return intSequence; } + set { intSequence = value; } + } + private IEnumerable intSequence; + } +} diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index ccb7ba1a..43f67145 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -247,5 +247,19 @@ public void Parse_nullable_options() Assert.False(result.Errors.Any()); // Teardown } + + [Fact] + public void Parse_allow_null_default_value() + { + // Fixture setup + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new string[0]); + + // Verify outcome + Assert.Null(result.Value.IntSequence); + // Teardown + } } } diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index ab794d29..63d143fc 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -80,12 +80,12 @@ public static ParserResult Build( sp => sp.Value.FromJust()) .SetProperties(specPropsWithValue, sp => sp.Value.IsNothing() && sp.Specification.DefaultValue.IsJust(), - sp => sp.Specification.DefaultValue.FromJust()) - .SetProperties(specPropsWithValue, - sp => sp.Value.IsNothing() - && sp.Specification.ConversionType.ToDescriptor() == DescriptorType.Sequence - && sp.Specification.DefaultValue.MatchNothing(), - sp => sp.Property.PropertyType.GetGenericArguments().Single().CreateEmptyArray()); + sp => sp.Specification.DefaultValue.FromJust()); + //.SetProperties(specPropsWithValue, + // sp => sp.Value.IsNothing() + // && sp.Specification.ConversionType.ToDescriptor() == DescriptorType.Sequence + // && sp.Specification.DefaultValue.MatchNothing(), + // sp => sp.Property.PropertyType.GetGenericArguments().Single().CreateEmptyArray()); var validationErrors = specPropsWithValue.Validate(SpecificationPropertyRules.Lookup) .OfType>().Select(e => e.Value); diff --git a/src/CommandLine/OptionAttribute.cs b/src/CommandLine/OptionAttribute.cs index 2d93f452..886e7fe6 100644 --- a/src/CommandLine/OptionAttribute.cs +++ b/src/CommandLine/OptionAttribute.cs @@ -153,15 +153,7 @@ public int Max public object DefaultValue { get { return this.defaultValue; } - set - { - if (value == null) - { - throw new ArgumentNullException("value"); - } - - this.defaultValue = value; - } + set { this.defaultValue = value; } } /// From aed0d87344fab99872e8378a90df6e23eba1b507 Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Sun, 31 May 2015 18:28:39 -0500 Subject: [PATCH 3/9] Fixed unit test: Parameters_names_are_inferred When shortName is specified, longName cannot be used. This appears to be by design so that unit test has been updated to reflect. --- src/CommandLine.Tests/Unit/ParserTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index 43f67145..160bb92b 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -89,7 +89,7 @@ public void Parameters_names_are_inferred() var sut = new Parser(); // Exercize system - var result = sut.ParseArguments(new[] { "--stringvalue", "strvalue", "--intsequence", "1", "2", "3" }); + var result = sut.ParseArguments(new[] { "--stringvalue", "strvalue", "-i", "1", "2", "3" }); // Verify outcome result.Value.ShouldHave().AllProperties().EqualTo(expectedOptions); From dfdb4ec2b2ea4f68644e091400229abfad9c227b Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Sun, 31 May 2015 19:18:24 -0500 Subject: [PATCH 4/9] Cleanup on aisle e2610bd2022bf5ad55be5e346a807fbdd7f1878d --- src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs b/src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs index 630f10f5..f5d61b2d 100644 --- a/src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs +++ b/src/CommandLine.Tests/Fakes/FakeOptionsWithNullDefault.cs @@ -5,11 +5,6 @@ namespace CommandLine.Tests.Fakes class FakeOptionsWithNullDefault { [Option('i', DefaultValue = null)] - public IEnumerable IntSequence - { - get { return intSequence; } - set { intSequence = value; } - } - private IEnumerable intSequence; + public IEnumerable IntSequence { get; set; } } } From 43a868b5fbc9f9f3c426e48e82fd9e28469167f0 Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Mon, 1 Jun 2015 22:56:37 -0500 Subject: [PATCH 5/9] Allow Min = 0 on sequences Added unit test for sequences with Min = 0 --- src/CommandLine.Tests/CommandLine.Tests.csproj | 1 + .../Fakes/FakeOptionsWithSequenceWithMinZero.cs | 10 ++++++++++ src/CommandLine.Tests/Unit/ParserTests.cs | 15 +++++++++++++++ src/CommandLine/Core/InstanceBuilder.cs | 12 ++++++++++++ src/CommandLine/Core/SpecificationGuards.cs | 2 +- src/CommandLine/Core/TokenPartitioner.cs | 2 +- 6 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 src/CommandLine.Tests/Fakes/FakeOptionsWithSequenceWithMinZero.cs diff --git a/src/CommandLine.Tests/CommandLine.Tests.csproj b/src/CommandLine.Tests/CommandLine.Tests.csproj index 20e42fd8..5e39b531 100644 --- a/src/CommandLine.Tests/CommandLine.Tests.csproj +++ b/src/CommandLine.Tests/CommandLine.Tests.csproj @@ -60,6 +60,7 @@ + diff --git a/src/CommandLine.Tests/Fakes/FakeOptionsWithSequenceWithMinZero.cs b/src/CommandLine.Tests/Fakes/FakeOptionsWithSequenceWithMinZero.cs new file mode 100644 index 00000000..ea856a6d --- /dev/null +++ b/src/CommandLine.Tests/Fakes/FakeOptionsWithSequenceWithMinZero.cs @@ -0,0 +1,10 @@ +using System.Collections.Generic; + +namespace CommandLine.Tests.Fakes +{ + class FakeOptionsWithSequenceWithMinZero + { + [Option('i', Min = 0)] + public IEnumerable IntSequence { get; set; } + } +} diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index 160bb92b..3576ba6e 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -261,5 +261,20 @@ public void Parse_allow_null_default_value() Assert.Null(result.Value.IntSequence); // Teardown } + + [Fact] + public void Parse_allow_min_equal_zero() + { + // Fixture setup + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new[] { "-i" }); + + // Verify outcome + Assert.NotNull(result.Value.IntSequence); + Assert.Empty(result.Value.IntSequence); + // Teardown + } } } diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index 63d143fc..5d18077b 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -69,15 +69,27 @@ public static ParserResult Build( (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, parsingCulture)); var missingValueErrors = from token in partitions.Item3 + let optionSpec = optionSpecs.Single(o => token.Text.MatchName(o.ShortName, o.LongName, nameComparer)) + where !(optionSpec.ConversionType.ToDescriptor() == DescriptorType.Sequence && optionSpec.Min == 0) select new MissingValueOptionError( NameInfo.FromOptionSpecification(optionSpecs.Single(o => token.Text.MatchName(o.ShortName, o.LongName, nameComparer)))); + var emptyOptionSpecProps = from sp in optionSpecProps.Value + let o = (OptionSpecification) sp.Specification + where tokens.Count(token => token.Text.MatchName(o.ShortName, o.LongName, nameComparer)) == 1 + && o.ConversionType.ToDescriptor() == DescriptorType.Sequence + && o.Min == 0 + select sp; + var specPropsWithValue = optionSpecProps.Value.Concat(valueSpecProps.Value); instance = instance .SetProperties(specPropsWithValue, sp => sp.Value.IsJust(), sp => sp.Value.FromJust()) + .SetProperties(emptyOptionSpecProps, + sp => sp.Value.IsNothing(), + sp => sp.Property.PropertyType.GetGenericArguments().Single().CreateEmptyArray()) .SetProperties(specPropsWithValue, sp => sp.Value.IsNothing() && sp.Specification.DefaultValue.IsJust(), sp => sp.Specification.DefaultValue.FromJust()); diff --git a/src/CommandLine/Core/SpecificationGuards.cs b/src/CommandLine/Core/SpecificationGuards.cs index df73be5b..19cd1f4e 100644 --- a/src/CommandLine/Core/SpecificationGuards.cs +++ b/src/CommandLine/Core/SpecificationGuards.cs @@ -21,7 +21,7 @@ private static Func GuardAgainstScalarWithRange() private static Func GuardAgainstSequenceWithWrongRange() { - return spec => spec.ConversionType.ToDescriptor() == DescriptorType.Sequence && spec.Min > spec.Max; + return spec => spec.ConversionType.ToDescriptor() == DescriptorType.Sequence && (spec.Min > spec.Max && spec.Min > 0); } private static Func GuardAgainstOneCharLongName() diff --git a/src/CommandLine/Core/TokenPartitioner.cs b/src/CommandLine/Core/TokenPartitioner.cs index f10ff379..a3366ce0 100644 --- a/src/CommandLine/Core/TokenPartitioner.cs +++ b/src/CommandLine/Core/TokenPartitioner.cs @@ -64,7 +64,7 @@ private static IEnumerable PartitionSequences( Func>>> typeLookup) { return from tseq in tokens.Pairwise( - (f, s) => + (f, s) => f.IsName() && s.IsValue() ? typeLookup(f.Text).Return(info => info.Item1 == DescriptorType.Sequence From f3fda62f012c7724accb125ed29ab2f4ffe3a1c2 Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Fri, 26 Jun 2015 22:28:19 -0500 Subject: [PATCH 6/9] Fixed unspecified sequences always being null --- .../CommandLine.Tests.csproj | 1 + .../Fakes/FakeOptionsWithSequence.cs | 10 +++++++ .../Unit/Core/OptionMapperTests.cs | 2 +- .../Unit/Core/TokenPartitionerTests.cs | 8 ++--- src/CommandLine.Tests/Unit/ParserTests.cs | 29 +++++++++++++++++++ src/CommandLine/Core/InstanceBuilder.cs | 13 +++++---- src/CommandLine/Core/OptionSpecification.cs | 5 ++-- src/CommandLine/Core/Specification.cs | 9 +++++- .../Core/SpecificationExtensions.cs | 1 + src/CommandLine/Core/ValueSpecification.cs | 5 ++-- src/CommandLine/OptionAttribute.cs | 12 +++++++- src/CommandLine/Text/HelpText.cs | 3 +- src/CommandLine/ValueAttribute.cs | 11 +++---- 13 files changed, 86 insertions(+), 23 deletions(-) create mode 100644 src/CommandLine.Tests/Fakes/FakeOptionsWithSequence.cs diff --git a/src/CommandLine.Tests/CommandLine.Tests.csproj b/src/CommandLine.Tests/CommandLine.Tests.csproj index 5e39b531..1ab34adc 100644 --- a/src/CommandLine.Tests/CommandLine.Tests.csproj +++ b/src/CommandLine.Tests/CommandLine.Tests.csproj @@ -60,6 +60,7 @@ + diff --git a/src/CommandLine.Tests/Fakes/FakeOptionsWithSequence.cs b/src/CommandLine.Tests/Fakes/FakeOptionsWithSequence.cs new file mode 100644 index 00000000..3d7e73d2 --- /dev/null +++ b/src/CommandLine.Tests/Fakes/FakeOptionsWithSequence.cs @@ -0,0 +1,10 @@ +using System.Collections.Generic; + +namespace CommandLine.Tests.Fakes +{ + class FakeOptionsWithSequence + { + [Option('i')] + public IEnumerable IntSequence { get; set; } + } +} diff --git a/src/CommandLine.Tests/Unit/Core/OptionMapperTests.cs b/src/CommandLine.Tests/Unit/Core/OptionMapperTests.cs index 611423c1..35ae103a 100644 --- a/src/CommandLine.Tests/Unit/Core/OptionMapperTests.cs +++ b/src/CommandLine.Tests/Unit/Core/OptionMapperTests.cs @@ -24,7 +24,7 @@ public void Map_boolean_switch_creates_boolean_value() var specProps = new[] { SpecificationProperty.Create( - new OptionSpecification("x", string.Empty, false, string.Empty, -1, -1, Maybe.Nothing(), typeof(bool), string.Empty, string.Empty, new List()), + new OptionSpecification("x", string.Empty, false, string.Empty, -1, -1, Maybe.Nothing(), false, typeof(bool), string.Empty, string.Empty, new List()), typeof(FakeOptions).GetProperties().Single(p => p.Name.Equals("BoolValue", StringComparison.Ordinal)), Maybe.Nothing()) }; diff --git a/src/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs b/src/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs index 6a133e80..54e58e45 100644 --- a/src/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs +++ b/src/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs @@ -20,8 +20,8 @@ public void Partition_sequence_returns_sequence() }; var specs =new[] { - new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, -1, -1, null, typeof(string), string.Empty, string.Empty, new List()), - new OptionSpecification("i", string.Empty, false, string.Empty, 3, 4, null, typeof(IEnumerable), string.Empty, string.Empty, new List()) + new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, -1, -1, null, false, typeof(string), string.Empty, string.Empty, new List()), + new OptionSpecification("i", string.Empty, false, string.Empty, 3, 4, null, false, typeof(IEnumerable), string.Empty, string.Empty, new List()) }; // Exercize system @@ -46,8 +46,8 @@ public void Partition_sequence_returns_sequence_with_duplicates() }; var specs =new[] { - new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, -1, -1, null, typeof(string), string.Empty, string.Empty, null), - new OptionSpecification("i", string.Empty, false, string.Empty, 3, 4, null, typeof(IEnumerable), string.Empty, string.Empty, null) + new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, -1, -1, null, false, typeof(string), string.Empty, string.Empty, null), + new OptionSpecification("i", string.Empty, false, string.Empty, 3, 4, null, false, typeof(IEnumerable), string.Empty, string.Empty, null) }; // Exercize system diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index 3576ba6e..6acaa796 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -248,6 +248,21 @@ public void Parse_nullable_options() // Teardown } + [Fact] + public void Parse_check_empty_sequence() + { + // Fixture setup + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new string[0]); + + // Verify outcome + Assert.NotNull(result.Value.IntSequence); + Assert.Empty(result.Value.IntSequence); + // Teardown + } + [Fact] public void Parse_allow_null_default_value() { @@ -262,6 +277,20 @@ public void Parse_allow_null_default_value() // Teardown } + [Fact] + public void Parse_reject__sequence_without_values() + { + // Fixture setup + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new[] { "-i" }); + + // Verify outcome + Assert.NotEmpty(result.Errors); + // Teardown + } + [Fact] public void Parse_allow_min_equal_zero() { diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index 5d18077b..9f504337 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -92,12 +92,13 @@ where tokens.Count(token => token.Text.MatchName(o.ShortName, o.LongName, nameCo sp => sp.Property.PropertyType.GetGenericArguments().Single().CreateEmptyArray()) .SetProperties(specPropsWithValue, sp => sp.Value.IsNothing() && sp.Specification.DefaultValue.IsJust(), - sp => sp.Specification.DefaultValue.FromJust()); - //.SetProperties(specPropsWithValue, - // sp => sp.Value.IsNothing() - // && sp.Specification.ConversionType.ToDescriptor() == DescriptorType.Sequence - // && sp.Specification.DefaultValue.MatchNothing(), - // sp => sp.Property.PropertyType.GetGenericArguments().Single().CreateEmptyArray()); + sp => sp.Specification.DefaultValue.FromJust()) + .SetProperties(specPropsWithValue, + sp => sp.Value.IsNothing() + && sp.Specification.ConversionType.ToDescriptor() == DescriptorType.Sequence + && !sp.Specification.DefaulSpecified + && sp.Specification.DefaultValue.MatchNothing(), + sp => sp.Property.PropertyType.GetGenericArguments().Single().CreateEmptyArray()); var validationErrors = specPropsWithValue.Validate(SpecificationPropertyRules.Lookup) .OfType>().Select(e => e.Value); diff --git a/src/CommandLine/Core/OptionSpecification.cs b/src/CommandLine/Core/OptionSpecification.cs index ab3940bc..744dde83 100644 --- a/src/CommandLine/Core/OptionSpecification.cs +++ b/src/CommandLine/Core/OptionSpecification.cs @@ -14,8 +14,8 @@ internal sealed class OptionSpecification : Specification private readonly string metaValue; private readonly System.Collections.Generic.IEnumerable enumValues; - public OptionSpecification(string shortName, string longName, bool required, string setName, int min, int max, Maybe defaultValue, System.Type conversionType, string helpText, string metaValue, System.Collections.Generic.IEnumerable enumValues) - : base(SpecificationType.Option, required, min, max, defaultValue, conversionType) + public OptionSpecification(string shortName, string longName, bool required, string setName, int min, int max, Maybe defaultValue, bool defaultSpecified, System.Type conversionType, string helpText, string metaValue, System.Collections.Generic.IEnumerable enumValues) + : base(SpecificationType.Option, required, min, max, defaultValue, defaultSpecified, conversionType) { this.shortName = shortName; this.longName = longName; @@ -35,6 +35,7 @@ public static OptionSpecification FromAttribute(OptionAttribute attribute, Syste attribute.Min, attribute.Max, attribute.DefaultValue.ToMaybe(), + attribute.DefaultSpecified, conversionType, attribute.HelpText, attribute.MetaValue, diff --git a/src/CommandLine/Core/Specification.cs b/src/CommandLine/Core/Specification.cs index e6846b6c..add30077 100644 --- a/src/CommandLine/Core/Specification.cs +++ b/src/CommandLine/Core/Specification.cs @@ -20,18 +20,20 @@ internal abstract class Specification private readonly int min; private readonly int max; private readonly Maybe defaultValue; + private readonly bool defaultSpecified; /// /// This information is denormalized to decouple Specification from PropertyInfo. /// private readonly System.Type conversionType; - protected Specification(SpecificationType tag, bool required, int min, int max, Maybe defaultValue, System.Type conversionType) + protected Specification(SpecificationType tag, bool required, int min, int max, Maybe defaultValue, bool defaultSpecified, System.Type conversionType) { this.tag = tag; this.required = required; this.min = min; this.max = max; this.defaultValue = defaultValue; + this.defaultSpecified = defaultSpecified; this.conversionType = conversionType; } @@ -60,6 +62,11 @@ public Maybe DefaultValue get { return this.defaultValue; } } + public bool DefaulSpecified + { + get { return defaultSpecified; } + } + public System.Type ConversionType { get { return this.conversionType; } diff --git a/src/CommandLine/Core/SpecificationExtensions.cs b/src/CommandLine/Core/SpecificationExtensions.cs index 9b865f81..fe2ee8eb 100644 --- a/src/CommandLine/Core/SpecificationExtensions.cs +++ b/src/CommandLine/Core/SpecificationExtensions.cs @@ -33,6 +33,7 @@ public static OptionSpecification WithLongName(this OptionSpecification specific specification.Min, specification.Max, specification.DefaultValue, + specification.DefaulSpecified, specification.ConversionType, specification.HelpText, specification.MetaValue, diff --git a/src/CommandLine/Core/ValueSpecification.cs b/src/CommandLine/Core/ValueSpecification.cs index ebc04428..426b265f 100644 --- a/src/CommandLine/Core/ValueSpecification.cs +++ b/src/CommandLine/Core/ValueSpecification.cs @@ -9,8 +9,8 @@ internal sealed class ValueSpecification : Specification { private readonly int index; - public ValueSpecification(int index, bool required, int min, int max, Maybe defaultValue, System.Type conversionType) - : base(SpecificationType.Value, required, min, max, defaultValue, conversionType) + public ValueSpecification(int index, bool required, int min, int max, Maybe defaultValue, bool defaultSpecified, System.Type conversionType) + : base(SpecificationType.Value, required, min, max, defaultValue, defaultSpecified, conversionType) { this.index = index; } @@ -23,6 +23,7 @@ public static ValueSpecification FromAttribute(ValueAttribute attribute, System. attribute.Min, attribute.Max, attribute.DefaultValue.ToMaybe(), + attribute.DefaultSpecified, conversionType); } diff --git a/src/CommandLine/OptionAttribute.cs b/src/CommandLine/OptionAttribute.cs index 886e7fe6..63a2b005 100644 --- a/src/CommandLine/OptionAttribute.cs +++ b/src/CommandLine/OptionAttribute.cs @@ -153,9 +153,19 @@ public int Max public object DefaultValue { get { return this.defaultValue; } - set { this.defaultValue = value; } + + set + { + this.defaultValue = value; + DefaultSpecified = true; + } } + /// + /// Gets whether has actually been set. + /// + public bool DefaultSpecified { get; private set; } + /// /// Gets or sets a short description of this command line option. Usually a sentence summary. /// diff --git a/src/CommandLine/Text/HelpText.cs b/src/CommandLine/Text/HelpText.cs index d4209cfd..95860b5b 100644 --- a/src/CommandLine/Text/HelpText.cs +++ b/src/CommandLine/Text/HelpText.cs @@ -540,6 +540,7 @@ private IEnumerable AdaptVerbListToOptionList(IEnumerable(), + false, typeof(bool), verbTuple.Item1.HelpText, string.Empty, @@ -565,7 +566,7 @@ private HelpText AddOptionsImpl(IEnumerable optionList, str private OptionSpecification CreateHelpEntry() { - return new OptionSpecification(string.Empty, "help", false, string.Empty, -1, -1, Maybe.Nothing(), typeof(bool), + return new OptionSpecification(string.Empty, "help", false, string.Empty, -1, -1, Maybe.Nothing(), false, typeof(bool), this.sentenceBuilder.HelpCommandText(this.AddDashesToOption), string.Empty, new List()); } diff --git a/src/CommandLine/ValueAttribute.cs b/src/CommandLine/ValueAttribute.cs index dfdc9946..e79f2425 100644 --- a/src/CommandLine/ValueAttribute.cs +++ b/src/CommandLine/ValueAttribute.cs @@ -87,13 +87,14 @@ public object DefaultValue get { return this.defaultValue; } set { - if (value == null) - { - throw new ArgumentNullException("value"); - } - this.defaultValue = value; + DefaultSpecified = true; } } + + /// + /// Gets whether has actually been set. + /// + public bool DefaultSpecified { get; private set; } } } \ No newline at end of file From 445503339d4da29491574ac93bd0d548fad881e0 Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Fri, 26 Jun 2015 22:46:27 -0500 Subject: [PATCH 7/9] Fixed errors when using set and non-set options together --- src/CommandLine.Tests/CommandLine.Tests.csproj | 1 + .../Fakes/FakeOptionsWithSetAndNonSet.cs | 11 +++++++++++ src/CommandLine.Tests/Unit/ParserTests.cs | 14 ++++++++++++++ src/CommandLine/Core/SpecificationPropertyRules.cs | 2 +- 4 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 src/CommandLine.Tests/Fakes/FakeOptionsWithSetAndNonSet.cs diff --git a/src/CommandLine.Tests/CommandLine.Tests.csproj b/src/CommandLine.Tests/CommandLine.Tests.csproj index 1ab34adc..1e94ff57 100644 --- a/src/CommandLine.Tests/CommandLine.Tests.csproj +++ b/src/CommandLine.Tests/CommandLine.Tests.csproj @@ -62,6 +62,7 @@ + diff --git a/src/CommandLine.Tests/Fakes/FakeOptionsWithSetAndNonSet.cs b/src/CommandLine.Tests/Fakes/FakeOptionsWithSetAndNonSet.cs new file mode 100644 index 00000000..f7fd21f9 --- /dev/null +++ b/src/CommandLine.Tests/Fakes/FakeOptionsWithSetAndNonSet.cs @@ -0,0 +1,11 @@ +namespace CommandLine.Tests.Fakes +{ + class FakeOptionsWithSetAndNonSet + { + [Option('s', "set", SetName = "Set")] + public bool Set { get; set; } + + [Option('n', "nonset")] + public bool NonSet { get; set; } + } +} diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index 6acaa796..5f260c76 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -305,5 +305,19 @@ public void Parse_allow_min_equal_zero() Assert.Empty(result.Value.IntSequence); // Teardown } + + [Fact] + public void Parse_allow_set_option_with_non_set() + { + // Fixture setup + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new[] { "-s", "-n" }); + + // Verify outcome + Assert.Empty(result.Errors); + // Teardown + } } } diff --git a/src/CommandLine/Core/SpecificationPropertyRules.cs b/src/CommandLine/Core/SpecificationPropertyRules.cs index ba3f31dc..c83cf0be 100644 --- a/src/CommandLine/Core/SpecificationPropertyRules.cs +++ b/src/CommandLine/Core/SpecificationPropertyRules.cs @@ -20,7 +20,7 @@ private static Func, IEnumerable { return specProps => { - var options = specProps.Where(sp => sp.Specification.IsOption() && sp.Value.IsJust()); + var options = specProps.Where(sp => sp.Specification.IsOption() && sp.Value.IsJust() && !string.IsNullOrEmpty(((OptionSpecification)sp.Specification).SetName)); var groups = options.GroupBy(g => ((OptionSpecification)g.Specification).SetName); if (groups.Count() > 1) { From 372099cdf637f88bb887ffbf254433119aefcb14 Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Sat, 27 Jun 2015 10:47:44 -0500 Subject: [PATCH 8/9] Fixed sequences being empty when max is unspecified --- src/CommandLine.Tests/Unit/ParserTests.cs | 21 ++++++++++++++++++++- src/CommandLine/Core/InstanceBuilder.cs | 2 +- src/CommandLine/Core/TokenPartitioner.cs | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index 5f260c76..3a697715 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -243,8 +243,8 @@ public void Parse_nullable_options() var result = sut.ParseArguments(new[] { "-n", "60", "-c", "Red" }); // Verify outcome + Assert.Empty(result.Errors); result.Value.ShouldHave().AllProperties().EqualTo(expectedOptions); - Assert.False(result.Errors.Any()); // Teardown } @@ -263,6 +263,25 @@ public void Parse_check_empty_sequence() // Teardown } + [Fact] + public void Parse_check_nonempty_sequence() + { + // Fixture setup + var expectedOptions = new FakeOptionsWithSequence + { + IntSequence = new[] { 60, 120 } + }; + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new[] { "-i", "60", "120" }); + + // Verify outcome + Assert.Empty(result.Errors); + result.Value.ShouldHave().AllProperties().EqualTo(expectedOptions); + // Teardown + } + [Fact] public void Parse_allow_null_default_value() { diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index 9f504337..aef5b4cf 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -65,7 +65,7 @@ public static ParserResult Build( var valueSpecProps = ValueMapper.MapValues( (from pt in specProps where pt.Specification.IsValue() select pt), - partitions.Item2, + partitions.Item2, (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, parsingCulture)); var missingValueErrors = from token in partitions.Item3 diff --git a/src/CommandLine/Core/TokenPartitioner.cs b/src/CommandLine/Core/TokenPartitioner.cs index a3366ce0..f280f8f8 100644 --- a/src/CommandLine/Core/TokenPartitioner.cs +++ b/src/CommandLine/Core/TokenPartitioner.cs @@ -68,7 +68,7 @@ private static IEnumerable PartitionSequences( f.IsName() && s.IsValue() ? typeLookup(f.Text).Return(info => info.Item1 == DescriptorType.Sequence - ? new[] { f }.Concat(tokens.SkipWhile(t => t.Equals(f)).TakeWhile(v => v.IsValue()).Take(MaybeExtensions.Return(info.Item2, items => items, 0))) + ? new[] { f }.Concat(tokens.SkipWhile(t => t.Equals(f)).TakeWhile(v => v.IsValue()).Take(MaybeExtensions.Return(info.Item2, items => items, int.MaxValue))) : new Token[] { } , new Token[] { }) : new Token[] {}) from t in tseq From d69b6a537a038cf5e089de033e93dc5f84476f63 Mon Sep 17 00:00:00 2001 From: Adam Byrd Date: Sun, 28 Jun 2015 12:17:48 -0500 Subject: [PATCH 9/9] Fixed sequences with Min = 0 always being empty --- src/CommandLine.Tests/Unit/ParserTests.cs | 23 +++++++++++++++++++++-- src/CommandLine/Core/TypeLookup.cs | 2 +- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/CommandLine.Tests/Unit/ParserTests.cs b/src/CommandLine.Tests/Unit/ParserTests.cs index 3a697715..5bba69cf 100644 --- a/src/CommandLine.Tests/Unit/ParserTests.cs +++ b/src/CommandLine.Tests/Unit/ParserTests.cs @@ -297,7 +297,7 @@ public void Parse_allow_null_default_value() } [Fact] - public void Parse_reject__sequence_without_values() + public void Parse_reject_sequence_without_values() { // Fixture setup var sut = new Parser(); @@ -311,7 +311,7 @@ public void Parse_reject__sequence_without_values() } [Fact] - public void Parse_allow_min_equal_zero() + public void Parse_allow_min_equal_zero_empty() { // Fixture setup var sut = new Parser(); @@ -325,6 +325,25 @@ public void Parse_allow_min_equal_zero() // Teardown } + [Fact] + public void Parse_allow_min_equal_zero_nonempty() + { + // Fixture setup + var expectedOptions = new FakeOptionsWithSequence + { + IntSequence = new[] { 60, 120 } + }; + var sut = new Parser(); + + // Exercize system + var result = sut.ParseArguments(new[] { "-i", "60", "120" }); + + // Verify outcome + Assert.Empty(result.Errors); + result.Value.ShouldHave().AllProperties().EqualTo(expectedOptions); + // Teardown + } + [Fact] public void Parse_allow_set_option_with_non_set() { diff --git a/src/CommandLine/Core/TypeLookup.cs b/src/CommandLine/Core/TypeLookup.cs index fccfed39..914c9829 100644 --- a/src/CommandLine/Core/TypeLookup.cs +++ b/src/CommandLine/Core/TypeLookup.cs @@ -22,7 +22,7 @@ public static Maybe>> GetDescriptorInfo( .ToMaybe() .Map( s => Tuple.Create( - s.ConversionType.ToDescriptor(), (s.Min < 0 && s.Max < 0) ? Maybe.Nothing() : Maybe.Just(s.Max))); + s.ConversionType.ToDescriptor(), (s.Max < 0) ? Maybe.Nothing() : Maybe.Just(s.Max))); } } } \ No newline at end of file