From 78171b080e04e1054a19ed2c72de05635c07ebc9 Mon Sep 17 00:00:00 2001 From: Tyler Dunkel <40210514+tydunkel@users.noreply.github.com> Date: Sun, 26 May 2019 21:32:44 -0700 Subject: [PATCH 1/8] Add multi-instance option support --- src/CommandLine/Core/InstanceBuilder.cs | 31 +++- src/CommandLine/Core/InstanceChooser.cs | 29 +++- src/CommandLine/Core/OptionMapper.cs | 34 ++-- src/CommandLine/Core/Sequence.cs | 153 +++++++++++++++--- .../Core/SpecificationPropertyRules.cs | 17 +- src/CommandLine/Core/TokenPartitioner.cs | 5 +- src/CommandLine/Core/TypeConverter.cs | 2 +- src/CommandLine/Parser.cs | 5 +- src/CommandLine/ParserSettings.cs | 10 ++ .../Unit/Core/InstanceBuilderTests.cs | 14 +- .../Unit/Core/InstanceChooserTests.cs | 17 +- .../Unit/Core/OptionMapperTests.cs | 62 +++++++ .../Unit/Core/SequenceTests.cs | 64 +++++++- .../Core/SpecificationPropertyRulesTests.cs | 58 +++++++ .../Unit/Core/TypeConverterTests.cs | 10 ++ tests/CommandLine.Tests/Unit/ParserTests.cs | 66 +++++--- 16 files changed, 505 insertions(+), 72 deletions(-) create mode 100644 tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index dce377f1..788ce187 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -23,6 +23,31 @@ public static ParserResult Build( bool autoHelp, bool autoVersion, IEnumerable nonFatalErrors) + { + return Build( + factory, + tokenizer, + arguments, + nameComparer, + ignoreValueCase, + parsingCulture, + autoHelp, + autoVersion, + false, + nonFatalErrors); + } + + public static ParserResult Build( + Maybe> factory, + Func, IEnumerable, Result, Error>> tokenizer, + IEnumerable arguments, + StringComparer nameComparer, + bool ignoreValueCase, + CultureInfo parsingCulture, + bool autoHelp, + bool autoVersion, + bool allowMultiInstance, + IEnumerable nonFatalErrors) { var typeInfo = factory.MapValueOrDefault(f => f().GetType(), typeof(T)); @@ -70,7 +95,7 @@ public static ParserResult Build( var valueSpecPropsResult = ValueMapper.MapValues( (from pt in specProps where pt.Specification.IsValue() orderby ((ValueSpecification)pt.Specification).Index select pt), - valuesPartition, + valuesPartition, (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, parsingCulture, ignoreValueCase)); var missingValueErrors = from token in errorsPartition @@ -86,7 +111,7 @@ public static ParserResult Build( //build the instance, determining if the type is mutable or not. T instance; - if(typeInfo.IsMutable() == true) + if (typeInfo.IsMutable() == true) { instance = BuildMutable(factory, specPropsWithValue, setPropertyErrors); } @@ -95,7 +120,7 @@ public static ParserResult Build( instance = BuildImmutable(typeInfo, factory, specProps, specPropsWithValue, setPropertyErrors); } - var validationErrors = specPropsWithValue.Validate(SpecificationPropertyRules.Lookup(tokens)); + var validationErrors = specPropsWithValue.Validate(SpecificationPropertyRules.Lookup(tokens, allowMultiInstance)); var allErrors = tokenizerResult.SuccessMessages() diff --git a/src/CommandLine/Core/InstanceChooser.cs b/src/CommandLine/Core/InstanceChooser.cs index f3ab9b99..2b868f7c 100644 --- a/src/CommandLine/Core/InstanceChooser.cs +++ b/src/CommandLine/Core/InstanceChooser.cs @@ -22,6 +22,31 @@ public static ParserResult Choose( bool autoHelp, bool autoVersion, IEnumerable nonFatalErrors) + { + return Choose( + tokenizer, + types, + arguments, + nameComparer, + ignoreValueCase, + parsingCulture, + autoHelp, + autoVersion, + false, + nonFatalErrors); + } + + public static ParserResult Choose( + Func, IEnumerable, Result, Error>> tokenizer, + IEnumerable types, + IEnumerable arguments, + StringComparer nameComparer, + bool ignoreValueCase, + CultureInfo parsingCulture, + bool autoHelp, + bool autoVersion, + bool allowMultiInstance, + IEnumerable nonFatalErrors) { var verbs = Verb.SelectFromTypes(types); var defaultVerbs = verbs.Where(t => t.Item1.IsDefault); @@ -46,7 +71,7 @@ public static ParserResult Choose( arguments.Skip(1).FirstOrDefault() ?? string.Empty, nameComparer)) : (autoVersion && preprocCompare("version")) ? MakeNotParsed(types, new VersionRequestedError()) - : MatchVerb(tokenizer, verbs, defaultVerb, arguments, nameComparer, ignoreValueCase, parsingCulture, autoHelp, autoVersion, nonFatalErrors); + : MatchVerb(tokenizer, verbs, defaultVerb, arguments, nameComparer, ignoreValueCase, parsingCulture, autoHelp, autoVersion, allowMultiInstance, nonFatalErrors); }; return arguments.Any() @@ -92,6 +117,7 @@ private static ParserResult MatchVerb( CultureInfo parsingCulture, bool autoHelp, bool autoVersion, + bool allowMultiInstance, IEnumerable nonFatalErrors) { return verbs.Any(a => nameComparer.Equals(a.Item1.Name, arguments.First())) @@ -106,6 +132,7 @@ private static ParserResult MatchVerb( parsingCulture, autoHelp, autoVersion, + allowMultiInstance, nonFatalErrors) : MatchDefaultVerb(tokenizer, verbs, defaultVerb, arguments, nameComparer, ignoreValueCase, parsingCulture, autoHelp, autoVersion, nonFatalErrors); } diff --git a/src/CommandLine/Core/OptionMapper.cs b/src/CommandLine/Core/OptionMapper.cs index 18349b40..f01f14ee 100644 --- a/src/CommandLine/Core/OptionMapper.cs +++ b/src/CommandLine/Core/OptionMapper.cs @@ -22,26 +22,32 @@ public static Result< .Select( pt => { - var matched = options.FirstOrDefault(s => + var matched = options.Where(s => s.Key.MatchName(((OptionSpecification)pt.Specification).ShortName, ((OptionSpecification)pt.Specification).LongName, comparer)).ToMaybe(); - return matched.IsJust() - ? ( - from sequence in matched - from converted in - converter( - sequence.Value, - pt.Property.PropertyType, - pt.Specification.TargetType != TargetType.Sequence) - select Tuple.Create( - pt.WithValue(Maybe.Just(converted)), Maybe.Nothing()) - ) + + if (matched.IsJust()) + { + var matches = matched.GetValueOrDefault(Enumerable.Empty>>()); + var values = new HashSet(); + foreach (var kvp in matches) + { + foreach (var value in kvp.Value) + { + values.Add(value); + } + } + + return converter(values, pt.Property.PropertyType, pt.Specification.TargetType != TargetType.Sequence) + .Select(value => Tuple.Create(pt.WithValue(Maybe.Just(value)), Maybe.Nothing())) .GetValueOrDefault( Tuple.Create>( pt, Maybe.Just( new BadFormatConversionError( - ((OptionSpecification)pt.Specification).FromOptionSpecification())))) - : Tuple.Create(pt, Maybe.Nothing()); + ((OptionSpecification)pt.Specification).FromOptionSpecification())))); + } + + return Tuple.Create(pt, Maybe.Nothing()); } ).Memoize(); return Result.Succeed( diff --git a/src/CommandLine/Core/Sequence.cs b/src/CommandLine/Core/Sequence.cs index 04d1b4ae..10b9c600 100644 --- a/src/CommandLine/Core/Sequence.cs +++ b/src/CommandLine/Core/Sequence.cs @@ -14,30 +14,141 @@ public static IEnumerable Partition( IEnumerable tokens, Func> typeLookup) { - return from tseq in tokens.Pairwise( - (f, s) => - f.IsName() && s.IsValue() - ? typeLookup(f.Text).MapValueOrDefault(info => - info.TargetType == TargetType.Sequence - ? new[] { f }.Concat(tokens.OfSequence(f, info)) - : new Token[] { }, new Token[] { }) - : new Token[] { }) - from t in tseq - select t; - } + var sequences = new Dictionary>(); + var state = SequenceState.TokenSearch; + Token nameToken = default; + foreach (var token in tokens) + { + switch (state) + { + case SequenceState.TokenSearch: + if (token.IsName()) + { + if (typeLookup(token.Text).MatchJust(out var info) && info.TargetType == TargetType.Sequence) + { + nameToken = token; + state = SequenceState.TokenFound; + } + } + break; - private static IEnumerable OfSequence(this IEnumerable tokens, Token nameToken, TypeDescriptor info) - { - var nameIndex = tokens.IndexOf(t => t.Equals(nameToken)); - if (nameIndex >= 0) + case SequenceState.TokenFound: + if (token.IsValue()) + { + if (sequences.TryGetValue(nameToken, out var sequence)) + { + sequence.Add(token); + } + else + { + sequences[nameToken] = new List(new[] { token }); + } + } + else if (token.IsName()) + { + if (typeLookup(token.Text).MatchJust(out var info) && info.TargetType == TargetType.Sequence) + { + nameToken = token; + state = SequenceState.TokenFound; + } + else + { + state = SequenceState.TokenSearch; + } + } + else + { + state = SequenceState.TokenSearch; + } + break; + } + } + + foreach (var kvp in sequences) { - return info.NextValue.MapValueOrDefault( - _ => info.MaxItems.MapValueOrDefault( - n => tokens.Skip(nameIndex + 1).Take(n), - tokens.Skip(nameIndex + 1).TakeWhile(v => v.IsValue())), - tokens.Skip(nameIndex + 1).TakeWhile(v => v.IsValue())); + yield return kvp.Key; + foreach (var value in kvp.Value) + { + yield return value; + } } - return new Token[] { }; + + //return from tseq in tokens.Pairwise( + //(f, s) => + // f.IsName() && s.IsValue() + // ? typeLookup(f.Text).MapValueOrDefault(info => + // info.TargetType == TargetType.Sequence + // ? new[] { f }.Concat(tokens.OfSequence(f, info)) + // : new Token[] { }, new Token[] { }) + // : new Token[] { }) + // from t in tseq + // select t; + } + + //private static IEnumerable OfSequence(this IEnumerable tokens, Token nameToken, TypeDescriptor info) + //{ + // var state = SequenceState.TokenSearch; + // var count = 0; + // var max = info.MaxItems.GetValueOrDefault(int.MaxValue); + // var values = max != int.MaxValue + // ? new List(max) + // : new List(); + + // foreach (var token in tokens) + // { + // if (count == max) + // { + // break; + // } + + // switch (state) + // { + // case SequenceState.TokenSearch: + // if (token.IsName() && token.Text.Equals(nameToken.Text)) + // { + // state = SequenceState.TokenFound; + // } + // break; + + // case SequenceState.TokenFound: + // if (token.IsValue()) + // { + // state = SequenceState.ValueFound; + // count++; + // values.Add(token); + // } + // else + // { + // // Invalid to provide option without value + // return Enumerable.Empty(); + // } + // break; + + // case SequenceState.ValueFound: + // if (token.IsValue()) + // { + // count++; + // values.Add(token); + // } + // else if (token.IsName() && token.Text.Equals(nameToken.Text)) + // { + // state = SequenceState.TokenFound; + // } + // else + // { + // state = SequenceState.TokenSearch; + // } + // break; + // } + // } + + // return values; + //} + + private enum SequenceState + { + TokenSearch, + TokenFound, } } } diff --git a/src/CommandLine/Core/SpecificationPropertyRules.cs b/src/CommandLine/Core/SpecificationPropertyRules.cs index 5dc1a406..4f8b78a9 100644 --- a/src/CommandLine/Core/SpecificationPropertyRules.cs +++ b/src/CommandLine/Core/SpecificationPropertyRules.cs @@ -13,6 +13,14 @@ static class SpecificationPropertyRules public static IEnumerable, IEnumerable>> Lookup( IEnumerable tokens) + { + return Lookup(tokens, false); + } + + public static IEnumerable, IEnumerable>> + Lookup( + IEnumerable tokens, + bool allowMultiInstance) { return new List, IEnumerable>> { @@ -21,7 +29,7 @@ public static IEnumerable, IEnumerable, IEnumerable> EnforceSingle(IEnumerable tokens) + private static Func, IEnumerable> EnforceSingle(IEnumerable tokens, bool allowMultiInstance) { return specProps => { + if (allowMultiInstance) + { + return Enumerable.Empty(); + } + var specs = from sp in specProps where sp.Specification.IsOption() where sp.Value.IsJust() diff --git a/src/CommandLine/Core/TokenPartitioner.cs b/src/CommandLine/Core/TokenPartitioner.cs index be38a6d0..608ae0e8 100644 --- a/src/CommandLine/Core/TokenPartitioner.cs +++ b/src/CommandLine/Core/TokenPartitioner.cs @@ -21,10 +21,11 @@ Tuple>>, IEnumerable(Switch.Partition(tokenList, typeLookup), tokenComparer); var scalars = new HashSet(Scalar.Partition(tokenList, typeLookup), tokenComparer); var sequences = new HashSet(Sequence.Partition(tokenList, typeLookup), tokenComparer); + var dedupedSequences = new HashSet(sequences); var nonOptions = tokenList .Where(t => !switches.Contains(t)) .Where(t => !scalars.Contains(t)) - .Where(t => !sequences.Contains(t)).Memoize(); + .Where(t => !dedupedSequences.Contains(t)).Memoize(); var values = nonOptions.Where(v => v.IsValue()).Memoize(); var errors = nonOptions.Except(values, (IEqualityComparer)ReferenceEqualityComparer.Default).Memoize(); @@ -36,4 +37,4 @@ Tuple>>, IEnumerable ChangeType(IEnumerable values, Type conversionType, bool scalar, CultureInfo conversionCulture, bool ignoreValueCase) { return scalar - ? ChangeTypeScalar(values.Single(), conversionType, conversionCulture, ignoreValueCase) + ? ChangeTypeScalar(values.Last(), conversionType, conversionCulture, ignoreValueCase) : ChangeTypeSequence(values, conversionType, conversionCulture, ignoreValueCase); } diff --git a/src/CommandLine/Parser.cs b/src/CommandLine/Parser.cs index f801c0f7..10c9b4e1 100644 --- a/src/CommandLine/Parser.cs +++ b/src/CommandLine/Parser.cs @@ -101,6 +101,7 @@ public ParserResult ParseArguments(IEnumerable args) settings.ParsingCulture, settings.AutoHelp, settings.AutoVersion, + settings.AllowMultiInstance, HandleUnknownArguments(settings.IgnoreUnknownArguments)), settings); } @@ -131,6 +132,7 @@ public ParserResult ParseArguments(Func factory, IEnumerable ar settings.ParsingCulture, settings.AutoHelp, settings.AutoVersion, + settings.AllowMultiInstance, HandleUnknownArguments(settings.IgnoreUnknownArguments)), settings); } @@ -163,6 +165,7 @@ public ParserResult ParseArguments(IEnumerable args, params Type settings.ParsingCulture, settings.AutoHelp, settings.AutoVersion, + settings.AllowMultiInstance, HandleUnknownArguments(settings.IgnoreUnknownArguments)), settings); } @@ -228,4 +231,4 @@ private void Dispose(bool disposing) } } } -} \ No newline at end of file +} diff --git a/src/CommandLine/ParserSettings.cs b/src/CommandLine/ParserSettings.cs index 07c10c4c..95a4cd81 100644 --- a/src/CommandLine/ParserSettings.cs +++ b/src/CommandLine/ParserSettings.cs @@ -25,6 +25,7 @@ public class ParserSettings : IDisposable private CultureInfo parsingCulture; private bool enableDashDash; private int maximumDisplayWidth; + private bool allowMultiInstance; /// /// Initializes a new instance of the class. @@ -174,6 +175,15 @@ public int MaximumDisplayWidth set { maximumDisplayWidth = value; } } + /// + /// Gets or sets a value indicating whether options are allowed to be specified multiple times. + /// + public bool AllowMultiInstance + { + get => allowMultiInstance; + set => PopsicleSetter.Set(Consumed, ref allowMultiInstance, value); + } + internal StringComparer NameComparer { get diff --git a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs index 8dd7371c..8b95af1c 100644 --- a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs @@ -19,7 +19,7 @@ namespace CommandLine.Tests.Unit.Core { public class InstanceBuilderTests { - private static ParserResult InvokeBuild(string[] arguments, bool autoHelp = true, bool autoVersion = true) + private static ParserResult InvokeBuild(string[] arguments, bool autoHelp = true, bool autoVersion = true, bool multiInstance = false) where T : new() { return InstanceBuilder.Build( @@ -31,6 +31,7 @@ private static ParserResult InvokeBuild(string[] arguments, bool autoHelp CultureInfo.InvariantCulture, autoHelp, autoVersion, + multiInstance, Enumerable.Empty()); } @@ -1235,6 +1236,17 @@ public void Options_In_Group_Do_Not_Allow_Mutually_Exclusive_Set() errors.Should().BeEquivalentTo(expectedResult); } + [Fact] + public void Parse_int_sequence_with_multi_instance() + { + var expected = new[] { 1, 2, 3 }; + var result = InvokeBuild( + new[] { "--int-seq", "1", "2", "--int-seq", "3" }, + multiInstance: true); + + ((Parsed)result).Value.IntSequence.Should().BeEquivalentTo(expected); + } + #region custom types diff --git a/tests/CommandLine.Tests/Unit/Core/InstanceChooserTests.cs b/tests/CommandLine.Tests/Unit/Core/InstanceChooserTests.cs index c9dae5fb..d5cb9a21 100644 --- a/tests/CommandLine.Tests/Unit/Core/InstanceChooserTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/InstanceChooserTests.cs @@ -15,7 +15,8 @@ public class InstanceChooserTests { private static ParserResult InvokeChoose( IEnumerable types, - IEnumerable arguments) + IEnumerable arguments, + bool multiInstance = false) { return InstanceChooser.Choose( (args, optionSpecs) => Tokenizer.ConfigureTokenizer(StringComparer.Ordinal, false, false)(args, optionSpecs), @@ -26,6 +27,7 @@ private static ParserResult InvokeChoose( CultureInfo.InvariantCulture, true, true, + multiInstance, Enumerable.Empty()); } @@ -168,5 +170,18 @@ public void Parse_sequence_verb_with_separator_returns_verb_instance(string[] ar expected.Should().BeEquivalentTo(((Parsed)result).Value); // Teardown } + + [Fact] + public void Parse_sequence_verb_with_multi_instance_returns_verb_instance() + { + var expected = new SequenceOptions { LongSequence = new long[] { }, StringSequence = new[] { "s1", "s2" } }; + var result = InvokeChoose( + new[] { typeof(Add_Verb), typeof(Commit_Verb), typeof(Clone_Verb), typeof(SequenceOptions) }, + new[] { "sequence", "-s", "s1", "-s", "s2" }, + true); + + Assert.IsType(((Parsed)result).Value); + expected.Should().BeEquivalentTo(((Parsed)result).Value); + } } } diff --git a/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs b/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs index b2219683..63bf22f3 100644 --- a/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs @@ -49,5 +49,67 @@ public void Map_boolean_switch_creates_boolean_value() // Teardown } + + [Fact] + public void Map_with_multi_instance_scalar() + { + var tokenPartitions = new[] + { + new KeyValuePair>("s", new[] { "string1" }), + new KeyValuePair>("shortandlong", new[] { "string2" }), + new KeyValuePair>("shortandlong", new[] { "string3" }), + new KeyValuePair>("s", new[] { "string4" }), + }; + + var specProps = new[] + { + SpecificationProperty.Create( + new OptionSpecification("s", "shortandlong", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(string), TargetType.Scalar, string.Empty), + typeof(Simple_Options).GetProperties().Single(p => p.Name.Equals(nameof(Simple_Options.ShortAndLong), StringComparison.Ordinal)), + Maybe.Nothing()), + }; + + var result = OptionMapper.MapValues( + specProps.Where(pt => pt.Specification.IsOption()), + tokenPartitions, + (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, CultureInfo.InvariantCulture, false), + StringComparer.Ordinal); + + var property = result.SucceededWith().Single(); + Assert.True(property.Specification.IsOption()); + Assert.True(property.Value.MatchJust(out var stringVal)); + Assert.Equal(tokenPartitions.Last().Value.Last(), stringVal); + } + + [Fact] + public void Map_with_multi_instance_sequence() + { + var tokenPartitions = new[] + { + new KeyValuePair>("i", new [] { "1", "2" }), + new KeyValuePair>("i", new [] { "3" }), + new KeyValuePair>("i", new [] { "4", "5" }), + }; + var specProps = new[] + { + SpecificationProperty.Create( + new OptionSpecification("i", string.Empty, false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty), + typeof(Simple_Options).GetProperties().Single(p => p.Name.Equals(nameof(Simple_Options.IntSequence), StringComparison.Ordinal)), + Maybe.Nothing()) + }; + + var result = OptionMapper.MapValues( + specProps.Where(pt => pt.Specification.IsOption()), + tokenPartitions, + (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, CultureInfo.InvariantCulture, false), + StringComparer.Ordinal); + + var property = result.SucceededWith().Single(); + Assert.True(property.Specification.IsOption()); + Assert.True(property.Value.MatchJust(out var sequence)); + + var expected = tokenPartitions.Aggregate(Enumerable.Empty(), (prev, part) => prev.Concat(part.Value.Select(i => int.Parse(i)))); + Assert.Equal(expected, sequence); + } } } diff --git a/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs b/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs index b26575b8..65d3dd3e 100644 --- a/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs @@ -49,7 +49,7 @@ public void Partition_sequence_values() } [Fact] - public void Partition_sequence_values_from_two_sequneces() + public void Partition_sequence_values_from_two_sequences() { var expected = new[] { @@ -93,5 +93,67 @@ public void Partition_sequence_values_only() expected.Should().BeEquivalentTo(result); } + + [Fact] + public void Partition_sequence_multi_instance() + { + var expected = new[] + { + Token.Name("seq"), + Token.Value("seqval0"), + Token.Value("seqval1"), + Token.Value("seqval2"), + Token.Value("seqval3"), + Token.Value("seqval4"), + }; + + var result = Sequence.Partition( + new[] + { + Token.Name("str"), Token.Value("strvalue"), Token.Value("freevalue"), + Token.Name("seq"), Token.Value("seqval0"), Token.Value("seqval1"), + Token.Name("x"), Token.Value("freevalue2"), + Token.Name("seq"), Token.Value("seqval2"), Token.Value("seqval3"), + Token.Name("seq"), Token.Value("seqval4") + }, + name => + new[] { "seq" }.Contains(name) + ? Maybe.Just(TypeDescriptor.Create(TargetType.Sequence, Maybe.Nothing())) + : Maybe.Nothing()); + + var actual = result.ToArray(); + Assert.Equal(expected, actual); + } + + [Fact] + public void Partition_sequence_multi_instance_with_max() + { + var expected = new[] + { + Token.Name("seq"), + Token.Value("seqval0"), + Token.Value("seqval1"), + Token.Value("seqval2"), + Token.Value("seqval3"), + Token.Value("seqval4"), + Token.Value("seqval5"), + }; + + var result = Sequence.Partition( + new[] + { + Token.Name("str"), Token.Value("strvalue"), Token.Value("freevalue"), + Token.Name("seq"), Token.Value("seqval0"), Token.Value("seqval1"), + Token.Name("x"), Token.Value("freevalue2"), + Token.Name("seq"), Token.Value("seqval2"), Token.Value("seqval3"), + Token.Name("seq"), Token.Value("seqval4"), Token.Value("seqval5"), + }, + name => + new[] { "seq" }.Contains(name) + ? Maybe.Just(TypeDescriptor.Create(TargetType.Sequence, Maybe.Just(3))) + : Maybe.Nothing()); + + Assert.Equal(expected, result); + } } } diff --git a/tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs b/tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs new file mode 100644 index 00000000..6e055c55 --- /dev/null +++ b/tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs @@ -0,0 +1,58 @@ +using CommandLine.Core; +using CommandLine.Tests.Fakes; +using CSharpx; +using System.Collections.Generic; +using Xunit; + +namespace CommandLine.Tests.Unit.Core +{ + + public class SpecificationPropertyRulesTests + { + [Fact] + public void Lookup_allows_multi_instance() + { + var tokens = new[] + { + Token.Name("name"), + Token.Value("value"), + Token.Name("name"), + Token.Value("value2"), + }; + + var specProps = new[] + { + SpecificationProperty.Create( + new OptionSpecification(string.Empty, "name", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty), + typeof(SequenceOptions).GetProperty(nameof(SequenceOptions.StringSequence)), + Maybe.Just(new object())), + }; + + var results = specProps.Validate(SpecificationPropertyRules.Lookup(tokens, true)); + Assert.Empty(results); + } + + [Fact] + public void Lookup_fails_with_repeated_options_false_multi_instance() + { + var tokens = new[] + { + Token.Name("name"), + Token.Value("value"), + Token.Name("name"), + Token.Value("value2"), + }; + + var specProps = new[] + { + SpecificationProperty.Create( + new OptionSpecification(string.Empty, "name", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty), + typeof(SequenceOptions).GetProperty(nameof(SequenceOptions.StringSequence)), + Maybe.Just(new object())), + }; + + var results = specProps.Validate(SpecificationPropertyRules.Lookup(tokens, false)); + Assert.Contains(results, r => r.GetType() == typeof(RepeatedOptionError)); + } + } +} diff --git a/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs b/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs index d9f3988c..ed9d6015 100644 --- a/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs @@ -33,6 +33,16 @@ public void ChangeType_scalars(string testValue, Type destinationType, bool expe } } + [Fact] + public void ChangeType_Scalar_LastOneWins() + { + var values = new[] { "100", "200", "300", "400", "500" }; + var result = TypeConverter.ChangeType(values, typeof(int), true, CultureInfo.InvariantCulture, true); + result.MatchJust(out var matchedValue).Should().BeTrue("should parse successfully"); + Assert.Equal(500, matchedValue); + + } + public static IEnumerable ChangeType_scalars_source { get diff --git a/tests/CommandLine.Tests/Unit/ParserTests.cs b/tests/CommandLine.Tests/Unit/ParserTests.cs index 90147ba6..46aad457 100644 --- a/tests/CommandLine.Tests/Unit/ParserTests.cs +++ b/tests/CommandLine.Tests/Unit/ParserTests.cs @@ -847,40 +847,58 @@ public void Blank_lines_are_inserted_between_verbs() // Teardown } - [Fact] - public void Parse_default_verb_implicit() + public void Parse_repeated_options_in_verbs_scenario_with_multi_instance() { - var parser = Parser.Default; - parser.ParseArguments(new[] { "-t" }) - .WithNotParsed(errors => throw new InvalidOperationException("Must be parsed.")) - .WithParsed(args => + using (var sut = new Parser(settings => settings.AllowMultiInstance = true)) + { + var longVal1 = 100; + var longVal2 = 200; + var longVal3 = 300; + var stringVal = "shortSeq1"; + + var result = sut.ParseArguments( + new[] { "sequence", "--long-seq", $"{longVal1}", "-s", stringVal, "--long-seq", $"{longVal2};{longVal3}" }, + typeof(Add_Verb), typeof(Commit_Verb), typeof(SequenceOptions)); + + Assert.IsType>(result); + Assert.IsType(((Parsed)result).Value); + result.WithParsed(verb => { - Assert.True(args.TestValueOne); + Assert.Equal(new long[] { longVal1, longVal2, longVal3 }, verb.LongSequence); + Assert.Equal(new[] { stringVal }, verb.StringSequence); }); + } } [Fact] - public void Parse_default_verb_explicit() + public void Parse_repeated_options_in_verbs_scenario_without_multi_instance() { - var parser = Parser.Default; - parser.ParseArguments(new[] { "default1", "-t" }) - .WithNotParsed(errors => throw new InvalidOperationException("Must be parsed.")) - .WithParsed(args => - { - Assert.True(args.TestValueOne); - }); - } + using (var sut = new Parser(settings => settings.AllowMultiInstance = false)) + { + var longVal1 = 100; + var longVal2 = 200; + var longVal3 = 300; + var stringVal = "shortSeq1"; - [Fact] - public void Parse_multiple_default_verbs() - { - var parser = Parser.Default; - parser.ParseArguments(new string[] { }) - .WithNotParsed(errors => Assert.IsType(errors.First())) - .WithParsed(args => throw new InvalidOperationException("Should not be parsed.")); - } + var result = sut.ParseArguments( + new[] { "sequence", "--long-seq", $"{longVal1}", "-s", stringVal, "--long-seq", $"{longVal2};{longVal3}" }, + typeof(Add_Verb), typeof(Commit_Verb), typeof(SequenceOptions)); + Assert.IsType>(result); + result.WithNotParsed(errors => Assert.All(errors, e => + { + if (e is RepeatedOptionError) + { + // expected + } + else + { + throw new Exception($"{nameof(RepeatedOptionError)} expected"); + } + })); + } + } [Fact] public void Parse_default_verb_with_empty_name() { From 85cf9d3ded6841028b31ca0ad5a4492f065a816e Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Tue, 24 Mar 2020 10:16:23 +0700 Subject: [PATCH 2/8] Add FlagCounter flag to OptionAttribute This will let us parse -vvv and turn that into "Verbose=3". --- src/CommandLine/Core/NameLookup.cs | 6 +- src/CommandLine/Core/OptionSpecification.cs | 14 ++- .../Core/SpecificationExtensions.cs | 1 + src/CommandLine/OptionAttribute.cs | 11 ++ src/CommandLine/Text/HelpText.cs | 3 + .../Unit/Core/NameLookupTests.cs | 4 +- .../Unit/Core/OptionMapperTests.cs | 6 +- .../Core/SpecificationPropertyRulesTests.cs | 116 +++++++++--------- .../Unit/Core/TokenPartitionerTests.cs | 8 +- .../Unit/Core/TokenizerTests.cs | 4 +- 10 files changed, 98 insertions(+), 75 deletions(-) diff --git a/src/CommandLine/Core/NameLookup.cs b/src/CommandLine/Core/NameLookup.cs index 3605d1a3..78d2e11a 100644 --- a/src/CommandLine/Core/NameLookup.cs +++ b/src/CommandLine/Core/NameLookup.cs @@ -10,7 +10,7 @@ namespace CommandLine.Core enum NameLookupResult { NoOptionFound, - BooleanOptionFound, + FlagOptionFound, OtherOptionFound } @@ -20,8 +20,8 @@ public static NameLookupResult Contains(string name, IEnumerable name.MatchName(a.ShortName, a.LongName, comparer)); if (option == null) return NameLookupResult.NoOptionFound; - return option.ConversionType == typeof(bool) - ? NameLookupResult.BooleanOptionFound + return option.ConversionType == typeof(bool) || option.FlagCounter + ? NameLookupResult.FlagOptionFound : NameLookupResult.OtherOptionFound; } diff --git a/src/CommandLine/Core/OptionSpecification.cs b/src/CommandLine/Core/OptionSpecification.cs index 77e7977f..725808c9 100644 --- a/src/CommandLine/Core/OptionSpecification.cs +++ b/src/CommandLine/Core/OptionSpecification.cs @@ -14,10 +14,11 @@ sealed class OptionSpecification : Specification private readonly char separator; private readonly string setName; private readonly string group; + private readonly bool flagCounter; public OptionSpecification(string shortName, string longName, bool required, string setName, Maybe min, Maybe max, char separator, Maybe defaultValue, string helpText, string metaValue, IEnumerable enumValues, - Type conversionType, TargetType targetType, string group, bool hidden = false) + Type conversionType, TargetType targetType, string group, bool flagCounter, bool hidden) : base(SpecificationType.Option, required, min, max, defaultValue, helpText, metaValue, enumValues, conversionType, targetType, hidden) { @@ -26,6 +27,7 @@ public OptionSpecification(string shortName, string longName, bool required, str this.separator = separator; this.setName = setName; this.group = group; + this.flagCounter = flagCounter; } public static OptionSpecification FromAttribute(OptionAttribute attribute, Type conversionType, IEnumerable enumValues) @@ -45,13 +47,14 @@ public static OptionSpecification FromAttribute(OptionAttribute attribute, Type conversionType, conversionType.ToTargetType(), attribute.Group, + attribute.FlagCounter, attribute.Hidden); } - public static OptionSpecification NewSwitch(string shortName, string longName, bool required, string helpText, string metaValue, bool hidden = false) + public static OptionSpecification NewSwitch(string shortName, string longName, bool required, string helpText, string metaValue, bool flagCounter, bool hidden) { return new OptionSpecification(shortName, longName, required, string.Empty, Maybe.Nothing(), Maybe.Nothing(), - '\0', Maybe.Nothing(), helpText, metaValue, Enumerable.Empty(), typeof(bool), TargetType.Switch, string.Empty, hidden); + '\0', Maybe.Nothing(), helpText, metaValue, Enumerable.Empty(), typeof(bool), TargetType.Switch, string.Empty, flagCounter, hidden); } public string ShortName @@ -78,5 +81,10 @@ public string Group { get { return group; } } + + public bool FlagCounter + { + get { return flagCounter; } + } } } diff --git a/src/CommandLine/Core/SpecificationExtensions.cs b/src/CommandLine/Core/SpecificationExtensions.cs index e223e987..c080e983 100644 --- a/src/CommandLine/Core/SpecificationExtensions.cs +++ b/src/CommandLine/Core/SpecificationExtensions.cs @@ -35,6 +35,7 @@ public static OptionSpecification WithLongName(this OptionSpecification specific specification.ConversionType, specification.TargetType, specification.Group, + specification.FlagCounter, specification.Hidden); } diff --git a/src/CommandLine/OptionAttribute.cs b/src/CommandLine/OptionAttribute.cs index 7448b697..1d8011b2 100644 --- a/src/CommandLine/OptionAttribute.cs +++ b/src/CommandLine/OptionAttribute.cs @@ -17,6 +17,7 @@ public sealed class OptionAttribute : BaseAttribute private string setName; private char separator; private string group=string.Empty; + private bool flagCounter; private OptionAttribute(string shortName, string longName) : base() { @@ -27,6 +28,7 @@ private OptionAttribute(string shortName, string longName) : base() this.longName = longName; setName = string.Empty; separator = '\0'; + flagCounter = false; } /// @@ -114,5 +116,14 @@ public string Group get { return group; } set { group = value; } } + + /// + /// When applied to an int property, turns that property into a count of how many times a boolean flag was applied (e.g., -vvv would become 3) + /// + public bool FlagCounter + { + get { return flagCounter; } + set { flagCounter = value; } + } } } diff --git a/src/CommandLine/Text/HelpText.cs b/src/CommandLine/Text/HelpText.cs index e9ce218d..cf1e325f 100644 --- a/src/CommandLine/Text/HelpText.cs +++ b/src/CommandLine/Text/HelpText.cs @@ -856,6 +856,7 @@ private IEnumerable AdaptVerbsToSpecifications(IEnumerable false, verbTuple.Item1.IsDefault? "(Default Verb) "+verbTuple.Item1.HelpText: verbTuple.Item1.HelpText, //Default verb string.Empty, + false, verbTuple.Item1.Hidden); if (autoHelp) optionSpecs = optionSpecs.Concat(new[] { MakeHelpEntry() }); @@ -914,6 +915,7 @@ private OptionSpecification MakeHelpEntry() false, sentenceBuilder.HelpCommandText(AddDashesToOption), string.Empty, + false, false); } @@ -925,6 +927,7 @@ private OptionSpecification MakeVersionEntry() false, sentenceBuilder.VersionCommandText(AddDashesToOption), string.Empty, + false, false); } diff --git a/tests/CommandLine.Tests/Unit/Core/NameLookupTests.cs b/tests/CommandLine.Tests/Unit/Core/NameLookupTests.cs index f009c49e..a00c8f57 100644 --- a/tests/CommandLine.Tests/Unit/Core/NameLookupTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/NameLookupTests.cs @@ -17,7 +17,7 @@ public void Lookup_name_of_sequence_option_with_separator() // Fixture setup var expected = Maybe.Just("."); var specs = new[] { new OptionSpecification(string.Empty, "string-seq", - false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '.', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty)}; + false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '.', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter:false, hidden:false)}; // Exercize system var result = NameLookup.HavingSeparator("string-seq", specs, StringComparer.Ordinal); @@ -35,7 +35,7 @@ public void Get_name_from_option_specification() // Fixture setup var expected = new NameInfo(ShortName, LongName); - var spec = new OptionSpecification(ShortName, LongName, false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '.', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty); + var spec = new OptionSpecification(ShortName, LongName, false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '.', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter:false, hidden:false); // Exercize system var result = spec.FromOptionSpecification(); diff --git a/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs b/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs index 63bf22f3..2a23351a 100644 --- a/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs @@ -28,7 +28,7 @@ public void Map_boolean_switch_creates_boolean_value() var specProps = new[] { SpecificationProperty.Create( - new OptionSpecification("x", string.Empty, false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(bool), TargetType.Switch, string.Empty), + new OptionSpecification("x", string.Empty, false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(bool), TargetType.Switch, string.Empty, flagCounter: false, hidden:false), typeof(Simple_Options).GetProperties().Single(p => p.Name.Equals("BoolValue", StringComparison.Ordinal)), Maybe.Nothing()) }; @@ -64,7 +64,7 @@ public void Map_with_multi_instance_scalar() var specProps = new[] { SpecificationProperty.Create( - new OptionSpecification("s", "shortandlong", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(string), TargetType.Scalar, string.Empty), + new OptionSpecification("s", "shortandlong", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(string), TargetType.Scalar, string.Empty, flagCounter: false, hidden:false), typeof(Simple_Options).GetProperties().Single(p => p.Name.Equals(nameof(Simple_Options.ShortAndLong), StringComparison.Ordinal)), Maybe.Nothing()), }; @@ -93,7 +93,7 @@ public void Map_with_multi_instance_sequence() var specProps = new[] { SpecificationProperty.Create( - new OptionSpecification("i", string.Empty, false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty), + new OptionSpecification("i", string.Empty, false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter: false, hidden:false), typeof(Simple_Options).GetProperties().Single(p => p.Name.Equals(nameof(Simple_Options.IntSequence), StringComparison.Ordinal)), Maybe.Nothing()) }; diff --git a/tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs b/tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs index 6e055c55..6c565056 100644 --- a/tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/SpecificationPropertyRulesTests.cs @@ -1,58 +1,58 @@ -using CommandLine.Core; -using CommandLine.Tests.Fakes; -using CSharpx; -using System.Collections.Generic; -using Xunit; - -namespace CommandLine.Tests.Unit.Core -{ - - public class SpecificationPropertyRulesTests - { - [Fact] - public void Lookup_allows_multi_instance() - { - var tokens = new[] - { - Token.Name("name"), - Token.Value("value"), - Token.Name("name"), - Token.Value("value2"), - }; - - var specProps = new[] - { - SpecificationProperty.Create( - new OptionSpecification(string.Empty, "name", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty), - typeof(SequenceOptions).GetProperty(nameof(SequenceOptions.StringSequence)), - Maybe.Just(new object())), - }; - - var results = specProps.Validate(SpecificationPropertyRules.Lookup(tokens, true)); - Assert.Empty(results); - } - - [Fact] - public void Lookup_fails_with_repeated_options_false_multi_instance() - { - var tokens = new[] - { - Token.Name("name"), - Token.Value("value"), - Token.Name("name"), - Token.Value("value2"), - }; - - var specProps = new[] - { - SpecificationProperty.Create( - new OptionSpecification(string.Empty, "name", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty), - typeof(SequenceOptions).GetProperty(nameof(SequenceOptions.StringSequence)), - Maybe.Just(new object())), - }; - - var results = specProps.Validate(SpecificationPropertyRules.Lookup(tokens, false)); - Assert.Contains(results, r => r.GetType() == typeof(RepeatedOptionError)); - } - } -} +using CommandLine.Core; +using CommandLine.Tests.Fakes; +using CSharpx; +using System.Collections.Generic; +using Xunit; + +namespace CommandLine.Tests.Unit.Core +{ + + public class SpecificationPropertyRulesTests + { + [Fact] + public void Lookup_allows_multi_instance() + { + var tokens = new[] + { + Token.Name("name"), + Token.Value("value"), + Token.Name("name"), + Token.Value("value2"), + }; + + var specProps = new[] + { + SpecificationProperty.Create( + new OptionSpecification(string.Empty, "name", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter: false, hidden:false), + typeof(SequenceOptions).GetProperty(nameof(SequenceOptions.StringSequence)), + Maybe.Just(new object())), + }; + + var results = specProps.Validate(SpecificationPropertyRules.Lookup(tokens, true)); + Assert.Empty(results); + } + + [Fact] + public void Lookup_fails_with_repeated_options_false_multi_instance() + { + var tokens = new[] + { + Token.Name("name"), + Token.Value("value"), + Token.Name("name"), + Token.Value("value2"), + }; + + var specProps = new[] + { + SpecificationProperty.Create( + new OptionSpecification(string.Empty, "name", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', Maybe.Nothing(), string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter: false, hidden:false), + typeof(SequenceOptions).GetProperty(nameof(SequenceOptions.StringSequence)), + Maybe.Just(new object())), + }; + + var results = specProps.Validate(SpecificationPropertyRules.Lookup(tokens, false)); + Assert.Contains(results, r => r.GetType() == typeof(RepeatedOptionError)); + } + } +} diff --git a/tests/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs b/tests/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs index 20006e59..7ec8301a 100644 --- a/tests/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TokenPartitionerTests.cs @@ -21,8 +21,8 @@ public void Partition_sequence_returns_sequence() }; var specs = new[] { - new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', null, string.Empty, string.Empty, new List(), typeof(string), TargetType.Scalar, string.Empty), - new OptionSpecification("i", string.Empty, false, string.Empty, Maybe.Just(3), Maybe.Just(4), '\0', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty) + new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', null, string.Empty, string.Empty, new List(), typeof(string), TargetType.Scalar, string.Empty, flagCounter: false, hidden:false), + new OptionSpecification("i", string.Empty, false, string.Empty, Maybe.Just(3), Maybe.Just(4), '\0', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter: false, hidden:false) }; // Exercize system @@ -48,8 +48,8 @@ public void Partition_sequence_returns_sequence_with_duplicates() }; var specs = new[] { - new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', null, string.Empty, string.Empty, new List(), typeof(string), TargetType.Scalar, string.Empty), - new OptionSpecification("i", string.Empty, false, string.Empty, Maybe.Just(3), Maybe.Just(4), '\0', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty) + new OptionSpecification(string.Empty, "stringvalue", false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), '\0', null, string.Empty, string.Empty, new List(), typeof(string), TargetType.Scalar, string.Empty, flagCounter: false, hidden:false), + new OptionSpecification("i", string.Empty, false, string.Empty, Maybe.Just(3), Maybe.Just(4), '\0', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter:false, hidden:false) }; // Exercize system diff --git a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs index 32d79b4f..2109d482 100644 --- a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs @@ -21,7 +21,7 @@ public void Explode_scalar_with_separator_in_odd_args_input_returns_sequence() var expectedTokens = new[] { Token.Name("i"), Token.Value("10"), Token.Name("string-seq"), Token.Value("aaa"), Token.Value("bb"), Token.Value("cccc"), Token.Name("switch") }; var specs = new[] { new OptionSpecification(string.Empty, "string-seq", - false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), ',', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty)}; + false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), ',', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter: false, hidden:false)}; // Exercize system var result = @@ -44,7 +44,7 @@ public void Explode_scalar_with_separator_in_even_args_input_returns_sequence() var expectedTokens = new[] { Token.Name("x"), Token.Name("string-seq"), Token.Value("aaa"), Token.Value("bb"), Token.Value("cccc"), Token.Name("switch") }; var specs = new[] { new OptionSpecification(string.Empty, "string-seq", - false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), ',', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty)}; + false, string.Empty, Maybe.Nothing(), Maybe.Nothing(), ',', null, string.Empty, string.Empty, new List(), typeof(IEnumerable), TargetType.Sequence, string.Empty, flagCounter: false, hidden:false)}; // Exercize system var result = From 1c86eea90c351babbd1143f73fd1918c9cbe1acb Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Tue, 24 Mar 2020 14:12:30 +0700 Subject: [PATCH 3/8] New Tokenizer implementation Most tests pass; a couple of tests were either not needed, or were testing semantics that change with the new implementation (e.g., spaces in option values are allowed now). --- src/CommandLine/Core/Tokenizer.cs | 295 ++++++++++-------- .../Unit/Core/InstanceBuilderTests.cs | 6 +- .../Unit/Core/TokenizerTests.cs | 47 +-- 3 files changed, 174 insertions(+), 174 deletions(-) diff --git a/src/CommandLine/Core/Tokenizer.cs b/src/CommandLine/Core/Tokenizer.cs index ba6f1ef5..e2a44177 100644 --- a/src/CommandLine/Core/Tokenizer.cs +++ b/src/CommandLine/Core/Tokenizer.cs @@ -16,50 +16,184 @@ public static Result, Error> Tokenize( IEnumerable arguments, Func nameLookup) { - return Tokenizer.Tokenize(arguments, nameLookup, tokens => tokens); + return Tokenizer.Tokenize(arguments, nameLookup, ignoreUnknownArguments:false, allowDashDash:true); } public static Result, Error> Tokenize( IEnumerable arguments, Func nameLookup, - Func, IEnumerable> normalize) + bool ignoreUnknownArguments, + bool allowDashDash) { var errors = new List(); Action onError = errors.Add; - var tokens = (from arg in arguments - from token in !arg.StartsWith("-", StringComparison.Ordinal) - ? new[] { Token.Value(arg) } - : arg.StartsWith("--", StringComparison.Ordinal) - ? TokenizeLongName(arg, onError) - : TokenizeShortName(arg, nameLookup) - select token) - .Memoize(); + int consumeNext = 0; + var tokens = new List(); + Action addValue = (s => tokens.Add(new Value(s))); + Action addName = (s => tokens.Add(new Name(s))); - var normalized = normalize(tokens).Memoize(); + var enumerator = arguments.GetEnumerator(); + while (enumerator.MoveNext()) + { + string arg = enumerator.Current; + // TODO: Turn this into a switch statement with pattern matching + if (arg == null) + { + continue; + } - var unkTokens = (from t in normalized where t.IsName() && nameLookup(t.Text) == NameLookupResult.NoOptionFound select t).Memoize(); + if (consumeNext > 0) + { + addValue(arg); + consumeNext = consumeNext - 1; + continue; + } - return Result.Succeed(normalized.Where(x => !unkTokens.Contains(x)), errors.Concat(from t in unkTokens select new UnknownOptionError(t.Text))); - } + if (arg == "--") + { + if (allowDashDash) + { + consumeNext = System.Int32.MaxValue; + continue; + } + else + { + addValue(arg); + continue; + } + } - public static Result, Error> PreprocessDashDash( - IEnumerable arguments, - Func, Result, Error>> tokenizer) - { - if (arguments.Any(arg => arg.EqualsOrdinal("--"))) - { - var tokenizerResult = tokenizer(arguments.TakeWhile(arg => !arg.EqualsOrdinal("--"))); - var values = arguments.SkipWhile(arg => !arg.EqualsOrdinal("--")).Skip(1).Select(Token.Value); - return tokenizerResult.Map(tokens => tokens.Concat(values)); + if (arg.StartsWith("--")) + { + if (arg.Contains("=")) + { + string[] parts = arg.Substring(2).Split(new char[] { '=' }, 2); + if (String.IsNullOrWhiteSpace(parts[0]) || parts[0].Contains(" ")) + { + onError(new BadFormatTokenError(arg)); + continue; + } + else + { + var name = parts[0]; + var tokenType = nameLookup(name); + if (tokenType == NameLookupResult.NoOptionFound) + { + if (ignoreUnknownArguments) + { + continue; + } + else + { + onError(new UnknownOptionError(name)); + continue; + } + } + addName(parts[0]); + addValue(parts[1]); + continue; + } + } + else + { + var name = arg.Substring(2); + var tokenType = nameLookup(name); + if (tokenType == NameLookupResult.OtherOptionFound) + { + addName(name); + consumeNext = 1; + continue; + } + else if (tokenType == NameLookupResult.NoOptionFound) + { + if (ignoreUnknownArguments) + { + // When ignoreUnknownArguments is true and AutoHelp is true, calling code is responsible for + // setting up nameLookup so that it will return a known name for --help, so that we don't skip it here + continue; + } + else + { + onError(new UnknownOptionError(name)); + continue; + } + } + else + { + addName(name); + continue; + } + } + } + + if (arg == "-") + { + // A single hyphen is always a value (it usually means "read from stdin" or "write to stdout") + addValue(arg); + continue; + } + + if (arg.StartsWith("-")) + { + // First option char that requires a value means we swallow the rest of the string as the value + // But if there is no rest of the string, then instead we swallow the next argument + string chars = arg.Substring(1); + int len = chars.Length; + if (len > 0 && Char.IsDigit(chars[0])) + { + // Assume it's a negative number + addValue(arg); + continue; + } + for (int i = 0; i < len; i++) + { + var s = new String(chars[i], 1); + var tokenType = nameLookup(s); + if (tokenType == NameLookupResult.OtherOptionFound) + { + addName(s); + if (i+1 < len) + { + addValue(chars.Substring(i+1)); + break; + } + else + { + consumeNext = 1; + } + } + else if (tokenType == NameLookupResult.NoOptionFound) + { + if (ignoreUnknownArguments) + { + continue; + } + else + { + onError(new UnknownOptionError(s)); + } + } + else + { + addName(s); + } + } + continue; + } + + // If we get this far, it's a plain value + addValue(arg); } - return tokenizer(arguments); + + return Result.Succeed, Error>(tokens.AsEnumerable(), errors.AsEnumerable()); } public static Result, Error> ExplodeOptionList( Result, Error> tokenizerResult, Func> optionSequenceWithSeparatorLookup) { + // TODO: I don't like how this works. I don't want "-s foo;bar baz" to put three values into -s. Let's instead have a third token type, List, besides Name and Value. var tokens = tokenizerResult.SucceededWith().Memoize(); var replaces = tokens.Select((t, i) => @@ -77,33 +211,6 @@ public static Result, Error> ExplodeOptionList( return Result.Succeed(flattened, tokenizerResult.SuccessMessages()); } - public static IEnumerable Normalize( - IEnumerable tokens, Func nameLookup) - { - var indexes = - from i in - tokens.Select( - (t, i) => - { - var prev = tokens.ElementAtOrDefault(i - 1).ToMaybe(); - return t.IsValue() && ((Value)t).ExplicitlyAssigned - && prev.MapValueOrDefault(p => p.IsName() && !nameLookup(p.Text), false) - ? Maybe.Just(i) - : Maybe.Nothing(); - }).Where(i => i.IsJust()) - select i.FromJustOrFail(); - - var toExclude = - from t in - tokens.Select((t, i) => indexes.Contains(i) ? Maybe.Just(t) : Maybe.Nothing()) - .Where(t => t.IsJust()) - select t.FromJustOrFail(); - - var normalized = tokens.Where(t => toExclude.Contains(t) == false); - - return normalized; - } - public static Func< IEnumerable, IEnumerable, @@ -115,94 +222,10 @@ public static Func< { return (arguments, optionSpecs) => { - var normalize = ignoreUnknownArguments - ? toks => Tokenizer.Normalize(toks, - name => NameLookup.Contains(name, optionSpecs, nameComparer) != NameLookupResult.NoOptionFound) - : new Func, IEnumerable>(toks => toks); - - var tokens = enableDashDash - ? Tokenizer.PreprocessDashDash( - arguments, - args => - Tokenizer.Tokenize(args, name => NameLookup.Contains(name, optionSpecs, nameComparer), normalize)) - : Tokenizer.Tokenize(arguments, name => NameLookup.Contains(name, optionSpecs, nameComparer), normalize); + var tokens = Tokenizer.Tokenize(arguments, name => NameLookup.Contains(name, optionSpecs, nameComparer), ignoreUnknownArguments, enableDashDash); var explodedTokens = Tokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer)); return explodedTokens; }; } - - private static IEnumerable TokenizeShortName( - string value, - Func nameLookup) - { - if (value.Length > 1 && value[0] == '-' && value[1] != '-') - { - var text = value.Substring(1); - - if (char.IsDigit(text[0])) - { - yield return Token.Value(value); - yield break; - } - - if (value.Length == 2) - { - yield return Token.Name(text); - yield break; - } - - var i = 0; - foreach (var c in text) - { - var n = new string(c, 1); - var r = nameLookup(n); - // Assume first char is an option - if (i > 0 && r == NameLookupResult.NoOptionFound) break; - i++; - yield return Token.Name(n); - // If option expects a value (other than a boolean), assume following chars are that value - if (r == NameLookupResult.OtherOptionFound) break; - } - - if (i < text.Length) - { - yield return Token.Value(text.Substring(i)); - } - } - } - - private static IEnumerable TokenizeLongName( - string value, - Action onError) - { - if (value.Length > 2 && value.StartsWith("--", StringComparison.Ordinal)) - { - var text = value.Substring(2); - var equalIndex = text.IndexOf('='); - if (equalIndex <= 0) - { - yield return Token.Name(text); - yield break; - } - if (equalIndex == 1) // "--=" - { - onError(new BadFormatTokenError(value)); - yield break; - } - - var tokenMatch = Regex.Match(text, "^([^=]+)=([^ ].*)$"); - - if (tokenMatch.Success) - { - yield return Token.Name(tokenMatch.Groups[1].Value); - yield return Token.Value(tokenMatch.Groups[2].Value, true); - } - else - { - onError(new BadFormatTokenError(value)); - yield break; - } - } - } } } diff --git a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs index 8b95af1c..e6151215 100644 --- a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs @@ -408,12 +408,10 @@ public void Double_dash_force_subsequent_arguments_as_values() }; var arguments = new[] { "--stringvalue", "str1", "--", "10", "-a", "--bee", "-c", "20" }; - // Exercize system + // Exercize system var result = InstanceBuilder.Build( Maybe.Just>(() => new Simple_Options_With_Values()), - (a, optionSpecs) => - Tokenizer.PreprocessDashDash(a, - args => Tokenizer.Tokenize(args, name => NameLookup.Contains(name, optionSpecs, StringComparer.Ordinal))), + (args, optionSpecs) => Tokenizer.ConfigureTokenizer(StringComparer.Ordinal, false, true)(args, optionSpecs), arguments, StringComparer.Ordinal, false, diff --git a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs index 2109d482..f3c5f58e 100644 --- a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs @@ -61,34 +61,6 @@ public void Explode_scalar_with_separator_in_even_args_input_returns_sequence() // Teardown } - [Fact] - public void Normalize_should_remove_all_value_with_explicit_assignment_of_existing_name() - { - // Fixture setup - var expectedTokens = new[] { - Token.Name("x"), Token.Name("string-seq"), Token.Value("aaa"), Token.Value("bb"), - Token.Name("unknown"), Token.Name("switch") }; - Func nameLookup = - name => name.Equals("x") || name.Equals("string-seq") || name.Equals("switch"); - - // Exercize system - var result = - Tokenizer.Normalize( - //Result.Succeed( - Enumerable.Empty() - .Concat( - new[] { - Token.Name("x"), Token.Name("string-seq"), Token.Value("aaa"), Token.Value("bb"), - Token.Name("unknown"), Token.Value("value0", true), Token.Name("switch") }) - //,Enumerable.Empty()), - , nameLookup); - - // Verify outcome - result.Should().BeEquivalentTo(expectedTokens); - - // Teardown - } - [Fact] public void Should_properly_parse_option_with_equals_in_value() { @@ -99,7 +71,7 @@ public void Should_properly_parse_option_with_equals_in_value() */ var args = new[] { "--connectionString=Server=localhost;Data Source=(LocalDB)\v12.0;Initial Catalog=temp;" }; - var result = Tokenizer.Tokenize(args, name => NameLookupResult.OtherOptionFound, token => token); + var result = Tokenizer.Tokenize(args, name => NameLookupResult.OtherOptionFound); var tokens = result.SucceededWith(); @@ -112,16 +84,23 @@ public void Should_properly_parse_option_with_equals_in_value() [Fact] public void Should_return_error_if_option_format_with_equals_is_not_correct() { - var args = new[] { "--option1 = fail", "--option2= fail" }; + var args = new[] { "--option1 = fail", "--option2= succeed" }; + + var result = Tokenizer.Tokenize(args, name => NameLookupResult.OtherOptionFound); - var result = Tokenizer.Tokenize(args, name => NameLookupResult.OtherOptionFound, token => token); + var errors = result.SuccessMessages(); - var tokens = result.SuccessMessages(); + Assert.NotNull(errors); + Assert.Equal(1, errors.Count()); + Assert.Equal(ErrorType.BadFormatTokenError, errors.First().Tag); + var tokens = result.SucceededWith(); Assert.NotNull(tokens); Assert.Equal(2, tokens.Count()); - Assert.Equal(ErrorType.BadFormatTokenError, tokens.First().Tag); - Assert.Equal(ErrorType.BadFormatTokenError, tokens.Last().Tag); + Assert.Equal(TokenType.Name, tokens.First().Tag); + Assert.Equal(TokenType.Value, tokens.Last().Tag); + Assert.Equal("option2", tokens.First().Text); + Assert.Equal(" succeed", tokens.Last().Text); } } From 3bed6b47583e294f74e5483258341a3b6ff4980e Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Tue, 24 Mar 2020 23:15:41 +0700 Subject: [PATCH 4/8] Use pattern matching in tokenizer Pattern matching is a particularly good fit here, as each case is data-driven and the flow is easy to read top-to-bottom. --- src/CommandLine/Core/Tokenizer.cs | 211 +++++++++++++----------------- 1 file changed, 88 insertions(+), 123 deletions(-) diff --git a/src/CommandLine/Core/Tokenizer.cs b/src/CommandLine/Core/Tokenizer.cs index e2a44177..5e1e282d 100644 --- a/src/CommandLine/Core/Tokenizer.cs +++ b/src/CommandLine/Core/Tokenizer.cs @@ -26,7 +26,10 @@ public static Result, Error> Tokenize( bool allowDashDash) { var errors = new List(); - Action onError = errors.Add; + Action onBadFormatToken = arg => errors.Add(new BadFormatTokenError(arg)); + Action unknownOptionError = name => errors.Add(new UnknownOptionError(name)); + Action doNothing = name => {}; + Action onUnknownOption = ignoreUnknownArguments ? doNothing : unknownOptionError; int consumeNext = 0; var tokens = new List(); @@ -36,154 +39,116 @@ public static Result, Error> Tokenize( var enumerator = arguments.GetEnumerator(); while (enumerator.MoveNext()) { - string arg = enumerator.Current; - // TODO: Turn this into a switch statement with pattern matching - if (arg == null) - { - continue; - } + switch (enumerator.Current) { + case null: + break; - if (consumeNext > 0) - { - addValue(arg); - consumeNext = consumeNext - 1; - continue; - } + case string arg when consumeNext > 0: + addValue(arg); + consumeNext = consumeNext - 1; + break; - if (arg == "--") - { - if (allowDashDash) - { + case "--" when allowDashDash: consumeNext = System.Int32.MaxValue; - continue; - } - else - { - addValue(arg); - continue; - } - } + break; - if (arg.StartsWith("--")) - { - if (arg.Contains("=")) - { + case "--": + addValue("--"); + break; + + case "-": + // A single hyphen is always a value (it usually means "read from stdin" or "write to stdout") + addValue("-"); + break; + + case string arg when arg.StartsWith("--") && arg.Contains("="): string[] parts = arg.Substring(2).Split(new char[] { '=' }, 2); if (String.IsNullOrWhiteSpace(parts[0]) || parts[0].Contains(" ")) { - onError(new BadFormatTokenError(arg)); - continue; + onBadFormatToken(arg); } else { - var name = parts[0]; - var tokenType = nameLookup(name); - if (tokenType == NameLookupResult.NoOptionFound) + switch(nameLookup(parts[0])) { - if (ignoreUnknownArguments) - { - continue; - } - else - { - onError(new UnknownOptionError(name)); - continue; - } + case NameLookupResult.NoOptionFound: + onUnknownOption(parts[0]); + break; + + default: + addName(parts[0]); + addValue(parts[1]); + break; } - addName(parts[0]); - addValue(parts[1]); - continue; } - } - else - { + break; + + case string arg when arg.StartsWith("--"): var name = arg.Substring(2); - var tokenType = nameLookup(name); - if (tokenType == NameLookupResult.OtherOptionFound) - { - addName(name); - consumeNext = 1; - continue; - } - else if (tokenType == NameLookupResult.NoOptionFound) + switch (nameLookup(name)) { - if (ignoreUnknownArguments) - { + case NameLookupResult.OtherOptionFound: + addName(name); + consumeNext = 1; + break; + + case NameLookupResult.NoOptionFound: // When ignoreUnknownArguments is true and AutoHelp is true, calling code is responsible for // setting up nameLookup so that it will return a known name for --help, so that we don't skip it here - continue; - } - else - { - onError(new UnknownOptionError(name)); - continue; - } + onUnknownOption(name); + break; + + default: + addName(name); + break; } - else + break; + + case string arg when arg.StartsWith("-"): + // First option char that requires a value means we swallow the rest of the string as the value + // But if there is no rest of the string, then instead we swallow the next argument + string chars = arg.Substring(1); + int len = chars.Length; + if (len > 0 && Char.IsDigit(chars[0])) { - addName(name); + // Assume it's a negative number + addValue(arg); continue; } - } - } - - if (arg == "-") - { - // A single hyphen is always a value (it usually means "read from stdin" or "write to stdout") - addValue(arg); - continue; - } - - if (arg.StartsWith("-")) - { - // First option char that requires a value means we swallow the rest of the string as the value - // But if there is no rest of the string, then instead we swallow the next argument - string chars = arg.Substring(1); - int len = chars.Length; - if (len > 0 && Char.IsDigit(chars[0])) - { - // Assume it's a negative number - addValue(arg); - continue; - } - for (int i = 0; i < len; i++) - { - var s = new String(chars[i], 1); - var tokenType = nameLookup(s); - if (tokenType == NameLookupResult.OtherOptionFound) + for (int i = 0; i < len; i++) { - addName(s); - if (i+1 < len) - { - addValue(chars.Substring(i+1)); - break; - } - else + var s = new String(chars[i], 1); + switch(nameLookup(s)) { - consumeNext = 1; + case NameLookupResult.OtherOptionFound: + addName(s); + if (i+1 < len) + { + addValue(chars.Substring(i+1)); + i = len; // Can't use "break" inside a switch, so this breaks out of the loop + } + else + { + consumeNext = 1; + } + break; + + case NameLookupResult.NoOptionFound: + onUnknownOption(s); + break; + + default: + addName(s); + break; } } - else if (tokenType == NameLookupResult.NoOptionFound) - { - if (ignoreUnknownArguments) - { - continue; - } - else - { - onError(new UnknownOptionError(s)); - } - } - else - { - addName(s); - } - } - continue; - } + break; - // If we get this far, it's a plain value - addValue(arg); + case string arg: + // If we get this far, it's a plain value + addValue(arg); + break; + } } return Result.Succeed, Error>(tokens.AsEnumerable(), errors.AsEnumerable()); From ee77b8435dfc357c2123cb28e5048dfe9d14347e Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 25 Mar 2020 16:56:31 +0700 Subject: [PATCH 5/8] Simplify tokenizer's switch statement The short and long option code can move into separate functions, which makes the big switch statement much easier to read. Also remove one TODO comment that's not relevant to the current feature. --- src/CommandLine/Core/Tokenizer.cs | 181 ++++++++++++++++-------------- 1 file changed, 97 insertions(+), 84 deletions(-) diff --git a/src/CommandLine/Core/Tokenizer.cs b/src/CommandLine/Core/Tokenizer.cs index 5e1e282d..25a14cdc 100644 --- a/src/CommandLine/Core/Tokenizer.cs +++ b/src/CommandLine/Core/Tokenizer.cs @@ -32,9 +32,9 @@ public static Result, Error> Tokenize( Action onUnknownOption = ignoreUnknownArguments ? doNothing : unknownOptionError; int consumeNext = 0; + Action onConsumeNext = (n => consumeNext = consumeNext + n); + var tokens = new List(); - Action addValue = (s => tokens.Add(new Value(s))); - Action addName = (s => tokens.Add(new Name(s))); var enumerator = arguments.GetEnumerator(); while (enumerator.MoveNext()) @@ -44,7 +44,7 @@ public static Result, Error> Tokenize( break; case string arg when consumeNext > 0: - addValue(arg); + tokens.Add(new Value(arg)); consumeNext = consumeNext - 1; break; @@ -53,100 +53,25 @@ public static Result, Error> Tokenize( break; case "--": - addValue("--"); + tokens.Add(new Value("--")); break; case "-": // A single hyphen is always a value (it usually means "read from stdin" or "write to stdout") - addValue("-"); - break; - - case string arg when arg.StartsWith("--") && arg.Contains("="): - string[] parts = arg.Substring(2).Split(new char[] { '=' }, 2); - if (String.IsNullOrWhiteSpace(parts[0]) || parts[0].Contains(" ")) - { - onBadFormatToken(arg); - } - else - { - switch(nameLookup(parts[0])) - { - case NameLookupResult.NoOptionFound: - onUnknownOption(parts[0]); - break; - - default: - addName(parts[0]); - addValue(parts[1]); - break; - } - } + tokens.Add(new Value("-")); break; case string arg when arg.StartsWith("--"): - var name = arg.Substring(2); - switch (nameLookup(name)) - { - case NameLookupResult.OtherOptionFound: - addName(name); - consumeNext = 1; - break; - - case NameLookupResult.NoOptionFound: - // When ignoreUnknownArguments is true and AutoHelp is true, calling code is responsible for - // setting up nameLookup so that it will return a known name for --help, so that we don't skip it here - onUnknownOption(name); - break; - - default: - addName(name); - break; - } + tokens.AddRange(TokenizeLongName(arg, nameLookup, onBadFormatToken, onUnknownOption, onConsumeNext)); break; case string arg when arg.StartsWith("-"): - // First option char that requires a value means we swallow the rest of the string as the value - // But if there is no rest of the string, then instead we swallow the next argument - string chars = arg.Substring(1); - int len = chars.Length; - if (len > 0 && Char.IsDigit(chars[0])) - { - // Assume it's a negative number - addValue(arg); - continue; - } - for (int i = 0; i < len; i++) - { - var s = new String(chars[i], 1); - switch(nameLookup(s)) - { - case NameLookupResult.OtherOptionFound: - addName(s); - if (i+1 < len) - { - addValue(chars.Substring(i+1)); - i = len; // Can't use "break" inside a switch, so this breaks out of the loop - } - else - { - consumeNext = 1; - } - break; - - case NameLookupResult.NoOptionFound: - onUnknownOption(s); - break; - - default: - addName(s); - break; - } - } + tokens.AddRange(TokenizeShortName(arg, nameLookup, onUnknownOption, onConsumeNext)); break; case string arg: // If we get this far, it's a plain value - addValue(arg); + tokens.Add(new Value(arg)); break; } } @@ -158,7 +83,6 @@ public static Result, Error> ExplodeOptionList( Result, Error> tokenizerResult, Func> optionSequenceWithSeparatorLookup) { - // TODO: I don't like how this works. I don't want "-s foo;bar baz" to put three values into -s. Let's instead have a third token type, List, besides Name and Value. var tokens = tokenizerResult.SucceededWith().Memoize(); var replaces = tokens.Select((t, i) => @@ -192,5 +116,94 @@ public static Func< return explodedTokens; }; } + + private static IEnumerable TokenizeShortName( + string arg, + Func nameLookup, + Action onUnknownOption, + Action onConsumeNext) + { + + // First option char that requires a value means we swallow the rest of the string as the value + // But if there is no rest of the string, then instead we swallow the next argument + string chars = arg.Substring(1); + int len = chars.Length; + if (len > 0 && Char.IsDigit(chars[0])) + { + // Assume it's a negative number + yield return Token.Value(arg); + yield break; + } + for (int i = 0; i < len; i++) + { + var s = new String(chars[i], 1); + switch(nameLookup(s)) + { + case NameLookupResult.OtherOptionFound: + yield return Token.Name(s); + + if (i+1 < len) + { + // Rest of this is the value (e.g. "-sfoo" where "-s" is a string-consuming arg) + yield return Token.Value(chars.Substring(i+1)); + yield break; + } + else + { + // Value is in next param (e.g., "-s foo") + onConsumeNext(1); + } + break; + + case NameLookupResult.NoOptionFound: + onUnknownOption(s); + break; + + default: + yield return Token.Name(s); + break; + } + } + } + + private static IEnumerable TokenizeLongName( + string arg, + Func nameLookup, + Action onBadFormatToken, + Action onUnknownOption, + Action onConsumeNext) + { + string[] parts = arg.Substring(2).Split(new char[] { '=' }, 2); + string name = parts[0]; + string value = (parts.Length > 1) ? parts[1] : null; + // A parameter like "--stringvalue=" is acceptable, and makes stringvalue be the empty string + if (String.IsNullOrWhiteSpace(name) || name.Contains(" ")) + { + onBadFormatToken(arg); + yield break; + } + switch(nameLookup(name)) + { + case NameLookupResult.NoOptionFound: + onUnknownOption(name); + yield break; + + case NameLookupResult.OtherOptionFound: + yield return Token.Name(name); + if (value == null) // NOT String.IsNullOrEmpty + { + onConsumeNext(1); + } + else + { + yield return Token.Value(value); + } + break; + + default: + yield return Token.Name(name); + break; + } + } } } From b6d3d472f035e56dd3b1d4d302d7119a61a1a1dd Mon Sep 17 00:00:00 2001 From: Rob Nasby Date: Tue, 31 Mar 2020 09:33:18 -0500 Subject: [PATCH 6/8] Properly assign arguments after a double dash to values, rather than options. --- src/CommandLine/Core/Scalar.cs | 2 +- src/CommandLine/Core/Sequence.cs | 2 +- src/CommandLine/Core/Token.cs | 35 +++++++++++++------ src/CommandLine/Core/Tokenizer.cs | 11 +++--- ...With_Option_Sequence_And_Value_Sequence.cs | 13 +++++++ tests/CommandLine.Tests/Unit/ParserTests.cs | 20 +++++++++++ 6 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 tests/CommandLine.Tests/Fakes/Options_With_Option_Sequence_And_Value_Sequence.cs diff --git a/src/CommandLine/Core/Scalar.cs b/src/CommandLine/Core/Scalar.cs index 215ca2d2..e1541bd3 100644 --- a/src/CommandLine/Core/Scalar.cs +++ b/src/CommandLine/Core/Scalar.cs @@ -16,7 +16,7 @@ public static IEnumerable Partition( { return from tseq in tokens.Pairwise( (f, s) => - f.IsName() && s.IsValue() + f.IsName() && s.IsValueUnforced() ? typeLookup(f.Text).MapValueOrDefault(info => info.TargetType == TargetType.Scalar ? new[] { f, s } : new Token[] { }, new Token[] { }) : new Token[] { }) diff --git a/src/CommandLine/Core/Sequence.cs b/src/CommandLine/Core/Sequence.cs index 10b9c600..95602458 100644 --- a/src/CommandLine/Core/Sequence.cs +++ b/src/CommandLine/Core/Sequence.cs @@ -33,7 +33,7 @@ public static IEnumerable Partition( break; case SequenceState.TokenFound: - if (token.IsValue()) + if (token.IsValueUnforced()) { if (sequences.TryGetValue(nameToken, out var sequence)) { diff --git a/src/CommandLine/Core/Token.cs b/src/CommandLine/Core/Token.cs index 2afee98f..4e9bb847 100644 --- a/src/CommandLine/Core/Token.cs +++ b/src/CommandLine/Core/Token.cs @@ -27,9 +27,14 @@ public static Token Value(string text) return new Value(text); } - public static Token Value(string text, bool explicitlyAssigned) + public static Token Value(string text, bool forced) { - return new Value(text, explicitlyAssigned); + return new Value(text, forced); + } + + public static Token ValueForced(string text) + { + return new Value(text, true); } public TokenType Tag @@ -79,22 +84,22 @@ public bool Equals(Name other) class Value : Token, IEquatable { - private readonly bool explicitlyAssigned; + private readonly bool forced; public Value(string text) : this(text, false) { } - public Value(string text, bool explicitlyAssigned) + public Value(string text, bool forced) : base(TokenType.Value, text) { - this.explicitlyAssigned = explicitlyAssigned; + this.forced = forced; } - public bool ExplicitlyAssigned + public bool Forced { - get { return explicitlyAssigned; } + get { return forced; } } public override bool Equals(object obj) @@ -110,7 +115,7 @@ public override bool Equals(object obj) public override int GetHashCode() { - return new { Tag, Text }.GetHashCode(); + return new { Tag, Text, Forced }.GetHashCode(); } public bool Equals(Value other) @@ -120,7 +125,7 @@ public bool Equals(Value other) return false; } - return Tag.Equals(other.Tag) && Text.Equals(other.Text); + return Tag.Equals(other.Tag) && Text.Equals(other.Text) && this.Forced == other.Forced; } } @@ -135,5 +140,15 @@ public static bool IsValue(this Token token) { return token.Tag == TokenType.Value; } + + public static bool IsValueForced(this Token token) + { + return token.IsValue() && ((Value)token).Forced; + } + + public static bool IsValueUnforced(this Token token) + { + return token.IsValue() && ! ((Value)token).Forced; + } } -} \ No newline at end of file +} diff --git a/src/CommandLine/Core/Tokenizer.cs b/src/CommandLine/Core/Tokenizer.cs index 25a14cdc..a35a0d28 100644 --- a/src/CommandLine/Core/Tokenizer.cs +++ b/src/CommandLine/Core/Tokenizer.cs @@ -34,6 +34,8 @@ public static Result, Error> Tokenize( int consumeNext = 0; Action onConsumeNext = (n => consumeNext = consumeNext + n); + bool isForced = false; + var tokens = new List(); var enumerator = arguments.GetEnumerator(); @@ -44,21 +46,22 @@ public static Result, Error> Tokenize( break; case string arg when consumeNext > 0: - tokens.Add(new Value(arg)); + tokens.Add(new Value(arg, isForced)); consumeNext = consumeNext - 1; break; case "--" when allowDashDash: consumeNext = System.Int32.MaxValue; + isForced = true; break; case "--": - tokens.Add(new Value("--")); + tokens.Add(new Value("--", isForced)); break; case "-": // A single hyphen is always a value (it usually means "read from stdin" or "write to stdout") - tokens.Add(new Value("-")); + tokens.Add(new Value("-", isForced)); break; case string arg when arg.StartsWith("--"): @@ -71,7 +74,7 @@ public static Result, Error> Tokenize( case string arg: // If we get this far, it's a plain value - tokens.Add(new Value(arg)); + tokens.Add(new Value(arg, isForced)); break; } } diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Option_Sequence_And_Value_Sequence.cs b/tests/CommandLine.Tests/Fakes/Options_With_Option_Sequence_And_Value_Sequence.cs new file mode 100644 index 00000000..c0ce7cdf --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_Option_Sequence_And_Value_Sequence.cs @@ -0,0 +1,13 @@ +using System.Collections.Generic; + +namespace CommandLine.Tests.Fakes +{ + public class Options_With_Option_Sequence_And_Value_Sequence + { + [Option('o', "option-seq")] + public IEnumerable OptionSequence { get; set; } + + [Value(0)] + public IEnumerable ValueSequence { get; set; } + } +} diff --git a/tests/CommandLine.Tests/Unit/ParserTests.cs b/tests/CommandLine.Tests/Unit/ParserTests.cs index 46aad457..081121bc 100644 --- a/tests/CommandLine.Tests/Unit/ParserTests.cs +++ b/tests/CommandLine.Tests/Unit/ParserTests.cs @@ -132,6 +132,26 @@ public void Parse_options_with_double_dash() // Teardown } + [Fact] + public void Parse_options_with_double_dash_and_option_sequence() + { + var expectedOptions = new Options_With_Option_Sequence_And_Value_Sequence + { + OptionSequence = new[] { "option1", "option2", "option3" }, + ValueSequence = new[] { "value1", "value2", "value3" } + }; + + var sut = new Parser(with => with.EnableDashDash = true); + + // Exercize system + var result = + sut.ParseArguments( + new[] { "--option-seq", "option1", "option2", "option3", "--", "value1", "value2", "value3" }); + + // Verify outcome + ((Parsed)result).Value.Should().BeEquivalentTo(expectedOptions); + } + [Fact] public void Parse_options_with_double_dash_in_verbs_scenario() { From 418f6e2f98ca8fb16746b30cf675f55489e14afe Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Mon, 1 Jun 2020 17:15:28 +0700 Subject: [PATCH 7/8] Add tests for several double-dash scenarios Double dash (--) should end option sequences, but should have no effect on value sequences. These unit tests reflect that design. --- ...ith_Value_Sequence_And_Subsequent_Value.cs | 15 ++++ ..._Sequence_With_Max_And_Subsequent_Value.cs | 15 ++++ tests/CommandLine.Tests/Unit/ParserTests.cs | 71 +++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_And_Subsequent_Value.cs create mode 100644 tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_With_Max_And_Subsequent_Value.cs diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_And_Subsequent_Value.cs b/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_And_Subsequent_Value.cs new file mode 100644 index 00000000..85f04d32 --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_And_Subsequent_Value.cs @@ -0,0 +1,15 @@ +// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. + +using System.Collections.Generic; + +namespace CommandLine.Tests.Fakes +{ + class Options_With_Value_Sequence_And_Subsequent_Value + { + [Value(0)] + public IEnumerable StringSequence { get; set; } + + [Value(1)] + public string NeverReachedValue { get; set; } + } +} diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_With_Max_And_Subsequent_Value.cs b/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_With_Max_And_Subsequent_Value.cs new file mode 100644 index 00000000..8af7ddf2 --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_With_Max_And_Subsequent_Value.cs @@ -0,0 +1,15 @@ +// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. + +using System.Collections.Generic; + +namespace CommandLine.Tests.Fakes +{ + class Options_With_Value_Sequence_With_Max_And_Subsequent_Value + { + [Value(0, Max=2)] + public IEnumerable StringSequence { get; set; } + + [Value(1)] + public string NeverReachedValue { get; set; } + } +} diff --git a/tests/CommandLine.Tests/Unit/ParserTests.cs b/tests/CommandLine.Tests/Unit/ParserTests.cs index 081121bc..d11ec46d 100644 --- a/tests/CommandLine.Tests/Unit/ParserTests.cs +++ b/tests/CommandLine.Tests/Unit/ParserTests.cs @@ -152,6 +152,77 @@ public void Parse_options_with_double_dash_and_option_sequence() ((Parsed)result).Value.Should().BeEquivalentTo(expectedOptions); } + [Theory] + [InlineData("value1", "value2", "value3")] + [InlineData("--", "value1", "value2", "value3")] + [InlineData("value1", "--", "value2", "value3")] + [InlineData("value1", "value2", "--", "value3")] + [InlineData("value1", "value2", "value3", "--")] + public void Parse_options_with_double_dash_in_various_positions(params string[] args) + { + var expectedOptions = new Options_With_Sequence_And_Only_Max_Constraint_For_Value + { + StringSequence = new[] { "value1", "value2", "value3" } + }; + + var sut = new Parser(with => with.EnableDashDash = true); + + // Exercize system + var result = + sut.ParseArguments(args); + + // Verify outcome + ((Parsed)result).Value.Should().BeEquivalentTo(expectedOptions); + } + + [Theory] + [InlineData("value1", "value2", "value3")] + [InlineData("--", "value1", "value2", "value3")] + [InlineData("value1", "--", "value2", "value3")] + [InlineData("value1", "value2", "--", "value3")] + [InlineData("value1", "value2", "value3", "--")] + public void Parse_options_with_double_dash_and_all_consuming_sequence_leaves_nothing_for_later_values(params string[] args) + { + var expectedOptions = new Options_With_Value_Sequence_And_Subsequent_Value + { + StringSequence = new[] { "value1", "value2", "value3" }, + NeverReachedValue = null + }; + + var sut = new Parser(with => with.EnableDashDash = true); + + // Exercize system + var result = + sut.ParseArguments(args); + + // Verify outcome + ((Parsed)result).Value.Should().BeEquivalentTo(expectedOptions); + } + + [Theory] + [InlineData("value1", "value2", "value3")] + [InlineData("--", "value1", "value2", "value3")] + [InlineData("value1", "--", "value2", "value3")] + [InlineData("value1", "value2", "--", "value3")] + [InlineData("value1", "value2", "value3", "--")] + public void Parse_options_with_double_dash_and_limited_sequence_leaves_something_for_later_values(params string[] args) + { + var expectedOptions = new Options_With_Value_Sequence_With_Max_And_Subsequent_Value + { + StringSequence = new[] { "value1", "value2" }, + NeverReachedValue = "value3" + }; + + var sut = new Parser(with => with.EnableDashDash = true); + + // Exercize system + var result = + sut.ParseArguments(args); + + // Verify outcome + ((Parsed)result).Value.Should().BeEquivalentTo(expectedOptions); + } + [Fact] public void Parse_options_with_double_dash_in_verbs_scenario() { From 1e791b592783f12855df0ead798169c43881228d Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Mon, 1 Jun 2020 20:31:06 +0700 Subject: [PATCH 8/8] Add unit tests for FlagCounter and implement it Now things like -vvv for triple-verbose can be done in user code. --- src/CommandLine/Core/InstanceBuilder.cs | 4 +- src/CommandLine/Core/OptionMapper.cs | 8 +- src/CommandLine/Core/OptionSpecification.cs | 2 +- src/CommandLine/Core/TypeConverter.cs | 18 +- .../Options_With_FlagCounter_Switches.cs | 13 + .../Unit/Core/OptionMapperTests.cs | 6 +- .../Unit/Core/TypeConverterTests.cs | 230 +++++++++--------- tests/CommandLine.Tests/Unit/ParserTests.cs | 30 +++ 8 files changed, 184 insertions(+), 127 deletions(-) create mode 100644 tests/CommandLine.Tests/Fakes/Options_With_FlagCounter_Switches.cs diff --git a/src/CommandLine/Core/InstanceBuilder.cs b/src/CommandLine/Core/InstanceBuilder.cs index 788ce187..60bce814 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -89,14 +89,14 @@ public static ParserResult Build( OptionMapper.MapValues( (from pt in specProps where pt.Specification.IsOption() select pt), optionsPartition, - (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, parsingCulture, ignoreValueCase), + (vals, type, isScalar, isFlag) => TypeConverter.ChangeType(vals, type, isScalar, isFlag, parsingCulture, ignoreValueCase), nameComparer); var valueSpecPropsResult = ValueMapper.MapValues( (from pt in specProps where pt.Specification.IsValue() orderby ((ValueSpecification)pt.Specification).Index select pt), valuesPartition, - (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, parsingCulture, ignoreValueCase)); + (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, false, parsingCulture, ignoreValueCase)); var missingValueErrors = from token in errorsPartition select diff --git a/src/CommandLine/Core/OptionMapper.cs b/src/CommandLine/Core/OptionMapper.cs index f01f14ee..e57ec04e 100644 --- a/src/CommandLine/Core/OptionMapper.cs +++ b/src/CommandLine/Core/OptionMapper.cs @@ -15,7 +15,7 @@ public static Result< MapValues( IEnumerable propertyTuples, IEnumerable>> options, - Func, Type, bool, Maybe> converter, + Func, Type, bool, bool, Maybe> converter, StringComparer comparer) { var sequencesAndErrors = propertyTuples @@ -28,7 +28,7 @@ public static Result< if (matched.IsJust()) { var matches = matched.GetValueOrDefault(Enumerable.Empty>>()); - var values = new HashSet(); + var values = new List(); foreach (var kvp in matches) { foreach (var value in kvp.Value) @@ -37,7 +37,9 @@ public static Result< } } - return converter(values, pt.Property.PropertyType, pt.Specification.TargetType != TargetType.Sequence) + bool isFlag = pt.Specification.Tag == SpecificationType.Option && ((OptionSpecification)pt.Specification).FlagCounter; + + return converter(values, isFlag ? typeof(bool) : pt.Property.PropertyType, pt.Specification.TargetType != TargetType.Sequence, isFlag) .Select(value => Tuple.Create(pt.WithValue(Maybe.Just(value)), Maybe.Nothing())) .GetValueOrDefault( Tuple.Create>( diff --git a/src/CommandLine/Core/OptionSpecification.cs b/src/CommandLine/Core/OptionSpecification.cs index 725808c9..80364544 100644 --- a/src/CommandLine/Core/OptionSpecification.cs +++ b/src/CommandLine/Core/OptionSpecification.cs @@ -20,7 +20,7 @@ public OptionSpecification(string shortName, string longName, bool required, str char separator, Maybe defaultValue, string helpText, string metaValue, IEnumerable enumValues, Type conversionType, TargetType targetType, string group, bool flagCounter, bool hidden) : base(SpecificationType.Option, - required, min, max, defaultValue, helpText, metaValue, enumValues, conversionType, targetType, hidden) + required, min, max, defaultValue, helpText, metaValue, enumValues, conversionType, conversionType == typeof(int) && flagCounter ? TargetType.Switch : targetType, hidden) { this.shortName = shortName; this.longName = longName; diff --git a/src/CommandLine/Core/TypeConverter.cs b/src/CommandLine/Core/TypeConverter.cs index 2ff75e9f..ec1189b1 100644 --- a/src/CommandLine/Core/TypeConverter.cs +++ b/src/CommandLine/Core/TypeConverter.cs @@ -13,11 +13,13 @@ namespace CommandLine.Core { static class TypeConverter { - public static Maybe ChangeType(IEnumerable values, Type conversionType, bool scalar, CultureInfo conversionCulture, bool ignoreValueCase) + public static Maybe ChangeType(IEnumerable values, Type conversionType, bool scalar, bool isFlag, CultureInfo conversionCulture, bool ignoreValueCase) { - return scalar - ? ChangeTypeScalar(values.Last(), conversionType, conversionCulture, ignoreValueCase) - : ChangeTypeSequence(values, conversionType, conversionCulture, ignoreValueCase); + return isFlag + ? ChangeTypeFlagCounter(values, conversionType, conversionCulture, ignoreValueCase) + : scalar + ? ChangeTypeScalar(values.Last(), conversionType, conversionCulture, ignoreValueCase) + : ChangeTypeSequence(values, conversionType, conversionCulture, ignoreValueCase); } private static Maybe ChangeTypeSequence(IEnumerable values, Type conversionType, CultureInfo conversionCulture, bool ignoreValueCase) @@ -46,6 +48,14 @@ private static Maybe ChangeTypeScalar(string value, Type conversionType, return result.ToMaybe(); } + private static Maybe ChangeTypeFlagCounter(IEnumerable values, Type conversionType, CultureInfo conversionCulture, bool ignoreValueCase) + { + var converted = values.Select(value => ChangeTypeScalar(value, typeof(bool), conversionCulture, ignoreValueCase)); + return converted.Any(maybe => maybe.MatchNothing()) + ? Maybe.Nothing() + : Maybe.Just((object)converted.Count(value => value.IsJust())); + } + private static object ConvertString(string value, Type type, CultureInfo conversionCulture) { try diff --git a/tests/CommandLine.Tests/Fakes/Options_With_FlagCounter_Switches.cs b/tests/CommandLine.Tests/Fakes/Options_With_FlagCounter_Switches.cs new file mode 100644 index 00000000..2ba932cb --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_FlagCounter_Switches.cs @@ -0,0 +1,13 @@ +// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. + +namespace CommandLine.Tests.Fakes +{ + public class Options_With_FlagCounter_Switches + { + [Option('v', FlagCounter=true)] + public int Verbose { get; set; } + + [Option('s', FlagCounter=true)] + public int Silent { get; set; } + } +} diff --git a/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs b/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs index 2a23351a..9d58297e 100644 --- a/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/OptionMapperTests.cs @@ -37,7 +37,7 @@ public void Map_boolean_switch_creates_boolean_value() var result = OptionMapper.MapValues( specProps.Where(pt => pt.Specification.IsOption()), tokenPartitions, - (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, CultureInfo.InvariantCulture, false), + (vals, type, isScalar, isFlag) => TypeConverter.ChangeType(vals, type, isScalar, isFlag, CultureInfo.InvariantCulture, false), StringComparer.Ordinal ); @@ -72,7 +72,7 @@ public void Map_with_multi_instance_scalar() var result = OptionMapper.MapValues( specProps.Where(pt => pt.Specification.IsOption()), tokenPartitions, - (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, CultureInfo.InvariantCulture, false), + (vals, type, isScalar, isFlag) => TypeConverter.ChangeType(vals, type, isScalar, isFlag, CultureInfo.InvariantCulture, false), StringComparer.Ordinal); var property = result.SucceededWith().Single(); @@ -101,7 +101,7 @@ public void Map_with_multi_instance_sequence() var result = OptionMapper.MapValues( specProps.Where(pt => pt.Specification.IsOption()), tokenPartitions, - (vals, type, isScalar) => TypeConverter.ChangeType(vals, type, isScalar, CultureInfo.InvariantCulture, false), + (vals, type, isScalar, isFlag) => TypeConverter.ChangeType(vals, type, isScalar, isFlag, CultureInfo.InvariantCulture, false), StringComparer.Ordinal); var property = result.SucceededWith().Single(); diff --git a/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs b/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs index ed9d6015..22cef6f6 100644 --- a/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs @@ -1,114 +1,116 @@ -using System; -using System.Collections.Generic; -using System.Globalization; -using Xunit; -using FluentAssertions; -using CSharpx; -using CommandLine.Core; - -namespace CommandLine.Tests.Unit.Core -{ - public class TypeConverterTests - { - enum TestEnum - { - ValueA = 1, - ValueB = 2 - } - - [Theory] - [MemberData(nameof(ChangeType_scalars_source))] - public void ChangeType_scalars(string testValue, Type destinationType, bool expectFail, object expectedResult) - { - Maybe result = TypeConverter.ChangeType(new[] {testValue}, destinationType, true, CultureInfo.InvariantCulture, true); - - if (expectFail) - { - result.MatchNothing().Should().BeTrue("should fail parsing"); - } - else - { - result.MatchJust(out object matchedValue).Should().BeTrue("should parse successfully"); - Assert.Equal(matchedValue, expectedResult); - } - } - - [Fact] - public void ChangeType_Scalar_LastOneWins() - { - var values = new[] { "100", "200", "300", "400", "500" }; - var result = TypeConverter.ChangeType(values, typeof(int), true, CultureInfo.InvariantCulture, true); - result.MatchJust(out var matchedValue).Should().BeTrue("should parse successfully"); - Assert.Equal(500, matchedValue); - - } - - public static IEnumerable ChangeType_scalars_source - { - get - { - return new[] - { - new object[] {"1", typeof (int), false, 1}, - new object[] {"0", typeof (int), false, 0}, - new object[] {"-1", typeof (int), false, -1}, - new object[] {"abcd", typeof (int), true, null}, - new object[] {"1.0", typeof (int), true, null}, - new object[] {int.MaxValue.ToString(), typeof (int), false, int.MaxValue}, - new object[] {int.MinValue.ToString(), typeof (int), false, int.MinValue}, - new object[] {((long) int.MaxValue + 1).ToString(), typeof (int), true, null}, - new object[] {((long) int.MinValue - 1).ToString(), typeof (int), true, null}, - - new object[] {"1", typeof (uint), false, (uint) 1}, - // new object[] {"0", typeof (uint), false, (uint) 0}, //cause warning: Skipping test case with duplicate ID - // new object[] {"-1", typeof (uint), true, null}, //cause warning: Skipping test case with duplicate ID - new object[] {uint.MaxValue.ToString(), typeof (uint), false, uint.MaxValue}, - new object[] {uint.MinValue.ToString(), typeof (uint), false, uint.MinValue}, - new object[] {((long) uint.MaxValue + 1).ToString(), typeof (uint), true, null}, - new object[] {((long) uint.MinValue - 1).ToString(), typeof (uint), true, null}, - - new object[] {"true", typeof (bool), false, true}, - new object[] {"True", typeof (bool), false, true}, - new object[] {"TRUE", typeof (bool), false, true}, - new object[] {"false", typeof (bool), false, false}, - new object[] {"False", typeof (bool), false, false}, - new object[] {"FALSE", typeof (bool), false, false}, - new object[] {"abcd", typeof (bool), true, null}, - new object[] {"0", typeof (bool), true, null}, - new object[] {"1", typeof (bool), true, null}, - - new object[] {"1.0", typeof (float), false, 1.0f}, - new object[] {"0.0", typeof (float), false, 0.0f}, - new object[] {"-1.0", typeof (float), false, -1.0f}, - new object[] {"abcd", typeof (float), true, null}, - - new object[] {"1.0", typeof (double), false, 1.0}, - new object[] {"0.0", typeof (double), false, 0.0}, - new object[] {"-1.0", typeof (double), false, -1.0}, - new object[] {"abcd", typeof (double), true, null}, - - new object[] {"1.0", typeof (decimal), false, 1.0m}, - new object[] {"0.0", typeof (decimal), false, 0.0m}, - new object[] {"-1.0", typeof (decimal), false, -1.0m}, - new object[] {"-1.123456", typeof (decimal), false, -1.123456m}, - new object[] {"abcd", typeof (decimal), true, null}, - - new object[] {"", typeof (string), false, ""}, - new object[] {"abcd", typeof (string), false, "abcd"}, - - new object[] {"ValueA", typeof (TestEnum), false, TestEnum.ValueA}, - new object[] {"VALUEA", typeof (TestEnum), false, TestEnum.ValueA}, - new object[] {"ValueB", typeof(TestEnum), false, TestEnum.ValueB}, - new object[] {((int) TestEnum.ValueA).ToString(), typeof (TestEnum), false, TestEnum.ValueA}, - new object[] {((int) TestEnum.ValueB).ToString(), typeof (TestEnum), false, TestEnum.ValueB}, - new object[] {((int) TestEnum.ValueB + 1).ToString(), typeof (TestEnum), true, null}, - new object[] {((int) TestEnum.ValueA - 1).ToString(), typeof (TestEnum), true, null}, - - // Failed before #339 - new object[] {"false", typeof (int), true, 0}, - new object[] {"true", typeof (int), true, 0} - }; - } - } - } -} +using System; +using System.Collections.Generic; +using System.Globalization; +using Xunit; +using FluentAssertions; +using CSharpx; +using CommandLine.Core; + +namespace CommandLine.Tests.Unit.Core +{ + public class TypeConverterTests + { + enum TestEnum + { + ValueA = 1, + ValueB = 2 + } + + [Theory] + [MemberData(nameof(ChangeType_scalars_source))] + public void ChangeType_scalars(string testValue, Type destinationType, bool expectFail, object expectedResult) + { + Maybe result = TypeConverter.ChangeType(new[] {testValue}, destinationType, true, false, CultureInfo.InvariantCulture, true); + + if (expectFail) + { + result.MatchNothing().Should().BeTrue("should fail parsing"); + } + else + { + result.MatchJust(out object matchedValue).Should().BeTrue("should parse successfully"); + Assert.Equal(matchedValue, expectedResult); + } + } + + [Fact] + public void ChangeType_Scalar_LastOneWins() + { + var values = new[] { "100", "200", "300", "400", "500" }; + var result = TypeConverter.ChangeType(values, typeof(int), true, false, CultureInfo.InvariantCulture, true); + result.MatchJust(out var matchedValue).Should().BeTrue("should parse successfully"); + Assert.Equal(500, matchedValue); + + } + + // TODO: Write test for TypeConverter.ChangeType when isFlag = true + + public static IEnumerable ChangeType_scalars_source + { + get + { + return new[] + { + new object[] {"1", typeof (int), false, 1}, + new object[] {"0", typeof (int), false, 0}, + new object[] {"-1", typeof (int), false, -1}, + new object[] {"abcd", typeof (int), true, null}, + new object[] {"1.0", typeof (int), true, null}, + new object[] {int.MaxValue.ToString(), typeof (int), false, int.MaxValue}, + new object[] {int.MinValue.ToString(), typeof (int), false, int.MinValue}, + new object[] {((long) int.MaxValue + 1).ToString(), typeof (int), true, null}, + new object[] {((long) int.MinValue - 1).ToString(), typeof (int), true, null}, + + new object[] {"1", typeof (uint), false, (uint) 1}, + // new object[] {"0", typeof (uint), false, (uint) 0}, //cause warning: Skipping test case with duplicate ID + // new object[] {"-1", typeof (uint), true, null}, //cause warning: Skipping test case with duplicate ID + new object[] {uint.MaxValue.ToString(), typeof (uint), false, uint.MaxValue}, + new object[] {uint.MinValue.ToString(), typeof (uint), false, uint.MinValue}, + new object[] {((long) uint.MaxValue + 1).ToString(), typeof (uint), true, null}, + new object[] {((long) uint.MinValue - 1).ToString(), typeof (uint), true, null}, + + new object[] {"true", typeof (bool), false, true}, + new object[] {"True", typeof (bool), false, true}, + new object[] {"TRUE", typeof (bool), false, true}, + new object[] {"false", typeof (bool), false, false}, + new object[] {"False", typeof (bool), false, false}, + new object[] {"FALSE", typeof (bool), false, false}, + new object[] {"abcd", typeof (bool), true, null}, + new object[] {"0", typeof (bool), true, null}, + new object[] {"1", typeof (bool), true, null}, + + new object[] {"1.0", typeof (float), false, 1.0f}, + new object[] {"0.0", typeof (float), false, 0.0f}, + new object[] {"-1.0", typeof (float), false, -1.0f}, + new object[] {"abcd", typeof (float), true, null}, + + new object[] {"1.0", typeof (double), false, 1.0}, + new object[] {"0.0", typeof (double), false, 0.0}, + new object[] {"-1.0", typeof (double), false, -1.0}, + new object[] {"abcd", typeof (double), true, null}, + + new object[] {"1.0", typeof (decimal), false, 1.0m}, + new object[] {"0.0", typeof (decimal), false, 0.0m}, + new object[] {"-1.0", typeof (decimal), false, -1.0m}, + new object[] {"-1.123456", typeof (decimal), false, -1.123456m}, + new object[] {"abcd", typeof (decimal), true, null}, + + new object[] {"", typeof (string), false, ""}, + new object[] {"abcd", typeof (string), false, "abcd"}, + + new object[] {"ValueA", typeof (TestEnum), false, TestEnum.ValueA}, + new object[] {"VALUEA", typeof (TestEnum), false, TestEnum.ValueA}, + new object[] {"ValueB", typeof(TestEnum), false, TestEnum.ValueB}, + new object[] {((int) TestEnum.ValueA).ToString(), typeof (TestEnum), false, TestEnum.ValueA}, + new object[] {((int) TestEnum.ValueB).ToString(), typeof (TestEnum), false, TestEnum.ValueB}, + new object[] {((int) TestEnum.ValueB + 1).ToString(), typeof (TestEnum), true, null}, + new object[] {((int) TestEnum.ValueA - 1).ToString(), typeof (TestEnum), true, null}, + + // Failed before #339 + new object[] {"false", typeof (int), true, 0}, + new object[] {"true", typeof (int), true, 0} + }; + } + } + } +} diff --git a/tests/CommandLine.Tests/Unit/ParserTests.cs b/tests/CommandLine.Tests/Unit/ParserTests.cs index d11ec46d..608c9cf6 100644 --- a/tests/CommandLine.Tests/Unit/ParserTests.cs +++ b/tests/CommandLine.Tests/Unit/ParserTests.cs @@ -95,6 +95,36 @@ public void Parse_options_with_short_name(string outputFile, string[] args) // Teardown } + [Theory] + [InlineData(new string[0], 0, 0)] + [InlineData(new[] { "-v" }, 1, 0)] + [InlineData(new[] { "-vv" }, 2, 0)] + [InlineData(new[] { "-v", "-v" }, 2, 0)] + [InlineData(new[] { "-v", "-v", "-v" }, 3, 0)] + [InlineData(new[] { "-v", "-vv" }, 3, 0)] + [InlineData(new[] { "-vv", "-v" }, 3, 0)] + [InlineData(new[] { "-vvv" }, 3, 0)] + [InlineData(new[] { "-v", "-s", "-v", "-v" }, 3, 1)] + [InlineData(new[] { "-v", "-ss", "-v", "-v" }, 3, 2)] + [InlineData(new[] { "-v", "-s", "-sv", "-v" }, 3, 2)] + [InlineData(new[] { "-vsvv" }, 3, 1)] + [InlineData(new[] { "-vssvv" }, 3, 2)] + [InlineData(new[] { "-vsvsv" }, 3, 2)] + public void Parse_FlagCounter_options_with_short_name(string[] args, int verboseCount, int silentCount) + { + // Fixture setup + var expectedOptions = new Options_With_FlagCounter_Switches { Verbose = verboseCount, Silent = silentCount }; + var sut = new Parser(with => with.AllowMultiInstance = true); + + // Exercize system + var result = sut.ParseArguments(args); + + // Verify outcome + // ((NotParsed)result).Errors.Should().BeEmpty(); + ((Parsed)result).Value.Should().BeEquivalentTo(expectedOptions); + // Teardown + } + [Fact] public void Parse_repeated_options_with_default_parser() {