From 41f3fbae82df15fcee87727f0e561cd50aed4b51 Mon Sep 17 00:00:00 2001 From: Luis Marsano Date: Thu, 4 Apr 2019 12:22:12 -0400 Subject: [PATCH 01/15] ignore Visual Studio Code editor data ignore .vscode/ --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 55f900a1..d45e4df4 100644 --- a/.gitignore +++ b/.gitignore @@ -37,5 +37,6 @@ artifacts/* *.DotSettings.user # Visual Studio 2015 cache/options directory .vs/ +.vscode/ -[R|r]elease/** +[R|r]elease/** From f23916f4285caab386bb58ea8f5233921aafb145 Mon Sep 17 00:00:00 2001 From: Luis Marsano Date: Thu, 4 Apr 2019 12:24:33 -0400 Subject: [PATCH 02/15] replace invalid signature key replace CommandLine.snk update strong name in assembly: InternalsVisibleTo attribute --- CommandLine.snk | Bin 595 -> 596 bytes src/CommandLine/Properties/AssemblyInfo.cs | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CommandLine.snk b/CommandLine.snk index 96087a731df6b418f70cfe6df405afeb53af9025..4dbcd829b2c2129e561f0cdb0d1022ccf2759472 100644 GIT binary patch delta 587 zcmV-R0<`_p1k?nO6n|~jz}K(eIfu6#IXaR(XL9J!4UtAaNytDx7&ypp8eOwdC0HX# zX@voJz9@eTiH^584`Htb3Ek0@x@X?vqt5DfAcL%cQA?%}(WGG59>GWmTbRG!g1=M` zHR|%e1|&tfF4KOvsJSDlxu$3Nr3qrA*f>7W6ZeuhjV&Rza(}#AR7R6~jrYBrIvUiY z6hvspK1CSyAOK&BUHz1<$x^h{DKi{=cGWOm)8X3Duus>980vZ}wD=nzbo1WKis$0j zT9}9mQ_VJSzR;dStJAIKoF7ct|Kwvc`_qZXu?i=m38v1yIt~q^YCiFXfB{KNvLs;& zEve*LQmy*z$bX#FH`R6;X9>8a7$rv~k_S&twm!_&vo@`q$=hIi;h%{Q{`-PE^OO~q z(lTO0W`t=))=%*yJruxLoH?5G?2DYW|3XF)Kaxh2+gMvLtbyJhr_gobvPaj(_8heR z5pue`#SeU45&xx~e{>!Iaae*-fRvR4_lZNVS-os^8-Ej$B|&6Ce#Xw}Np}&FXEW^E zrXtyv+u&r*@6g6aSKPQH#n@j*XID+u4WEhcV_mJ-J41W4TK2)fRV(e%q3m$IK@~sD z7s+o$SF51VW{@5s=1jRa#;saMgIc_((NollY Zx|ufn`J!2YF%PUKv~Ht~O@F7Z7=|*ZCa3@a delta 586 zcmV-Q0=50r1k(hN6n_=#b#c~~z;$AHF!@G_X%IWZS7b(e+syn3Q1bickv`%x)z{BE zytr`vLguKNSzz+GKvhPVu0vD2Ya^gx`v<6kYi+V$6dn=DR; z3xyNavf()^Du2$m5!)`rG9pe90Qr(rr`&n6Hg7mO{we0^4%`~!U@yK2eSDpk_-g}d zug^~rk5qh2!QC@_&G;YFDY*E$El3}nIWf4gGLr+t%X1n9fovdLPQ^+})N#3*cyRmr zyuWpl43ire`CXHd2*=^2iP(&JCOBMEBmKCT-q@SNpMR$_HvZf*?{z*zU_|RnE=c)7 z^V$RgVKR}ENDOzv39QEk^RC$p!FH1XR(Z&`kQ4TD$zc`WS@J%T$3?$HvCG>S27yE; zSVgj8M^qyZKp0P%+@LP&0%1ZGa Date: Thu, 4 Apr 2019 17:04:43 -0400 Subject: [PATCH 03/15] test usage property with custom attribute define custom attribute class specify attribute for fake Options_With_Usage_Attribute.Examples property --- tests/CommandLine.Tests/Fakes/CustomAttribute.cs | 4 ++++ tests/CommandLine.Tests/Fakes/Help_Fakes.cs | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 tests/CommandLine.Tests/Fakes/CustomAttribute.cs diff --git a/tests/CommandLine.Tests/Fakes/CustomAttribute.cs b/tests/CommandLine.Tests/Fakes/CustomAttribute.cs new file mode 100644 index 00000000..845fb2dd --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/CustomAttribute.cs @@ -0,0 +1,4 @@ +using System; + +[AttributeUsage(AttributeTargets.All)] +class CustomAttribute: Attribute {} \ No newline at end of file diff --git a/tests/CommandLine.Tests/Fakes/Help_Fakes.cs b/tests/CommandLine.Tests/Fakes/Help_Fakes.cs index cceb5331..4b9fe83e 100644 --- a/tests/CommandLine.Tests/Fakes/Help_Fakes.cs +++ b/tests/CommandLine.Tests/Fakes/Help_Fakes.cs @@ -79,7 +79,9 @@ class Options_With_Usage_Attribute [Option("secert-option", Hidden = true, HelpText = "This is a description for a secert hidden option that should never be visibile to the user via help text.")] public string SecertOption { get; set; } - [Usage(ApplicationAlias = "mono testapp.exe")] + [ Custom + , Usage(ApplicationAlias = "mono testapp.exe") + ] public static IEnumerable Examples { get From 196a83096aa46c7e18392105523e15d5cfa8e3d6 Mon Sep 17 00:00:00 2001 From: Luis Marsano Date: Thu, 4 Apr 2019 17:12:52 -0400 Subject: [PATCH 04/15] pass custom attribute test in ReflectionExtensions.GetUsageData, filter attributes by type before casting to that type --- src/CommandLine/Core/ReflectionExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CommandLine/Core/ReflectionExtensions.cs b/src/CommandLine/Core/ReflectionExtensions.cs index c82ebefc..ef940d95 100644 --- a/src/CommandLine/Core/ReflectionExtensions.cs +++ b/src/CommandLine/Core/ReflectionExtensions.cs @@ -40,8 +40,8 @@ public static Maybe> GetUsageData(this Type { return (from pi in type.FlattenHierarchy().SelectMany(x => x.GetTypeInfo().GetProperties()) - let attrs = pi.GetCustomAttributes(true) - where attrs.OfType().Any() + let attrs = pi.GetCustomAttributes(typeof(UsageAttribute), true) + where attrs.Any() select Tuple.Create(pi, (UsageAttribute)attrs.First())) .SingleOrDefault() .ToMaybe(); From 690136eaed700cb873d1f750076da0360e11dd1e Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Tue, 18 Aug 2020 04:23:20 +0700 Subject: [PATCH 05/15] Add multi-instance option support (#678) * Add multi-instance option support Fixes #357 * Fix Appveyor build It seems Appveyor's C# compiler is old enough that it doesn't know how to do tuple deconstruction, so we'll avoid doing that. It makes TokenPartitioner uglier, but no getting around that if we're dealing with an old C# compiler. --- src/CommandLine/Core/InstanceBuilder.cs | 26 +++- src/CommandLine/Core/InstanceChooser.cs | 31 +++- src/CommandLine/Core/OptionMapper.cs | 33 ++-- src/CommandLine/Core/PartitionExtensions.cs | 25 +++ src/CommandLine/Core/Scalar.cs | 27 ---- src/CommandLine/Core/Sequence.cs | 43 ------ .../Core/SpecificationPropertyRules.cs | 17 +- src/CommandLine/Core/Switch.cs | 21 --- src/CommandLine/Core/TokenPartitioner.cs | 145 ++++++++++++++++-- src/CommandLine/Core/TypeConverter.cs | 2 +- src/CommandLine/Parser.cs | 5 +- src/CommandLine/ParserSettings.cs | 10 ++ ...s_With_Value_Sequence_And_Normal_Option.cs | 28 ++++ .../Unit/Core/InstanceBuilderTests.cs | 14 +- .../Unit/Core/InstanceChooserTests.cs | 17 +- .../Unit/Core/OptionMapperTests.cs | 62 ++++++++ .../Unit/Core/ScalarTests.cs | 6 +- .../Unit/Core/SequenceTests.cs | 76 ++++++++- .../Core/SpecificationPropertyRulesTests.cs | 58 +++++++ .../Unit/Core/SwitchTests.cs | 6 +- .../Unit/Core/TypeConverterTests.cs | 10 ++ tests/CommandLine.Tests/Unit/ParserTests.cs | 68 ++++++++ 22 files changed, 598 insertions(+), 132 deletions(-) create mode 100644 src/CommandLine/Core/PartitionExtensions.cs delete mode 100644 src/CommandLine/Core/Scalar.cs delete mode 100644 src/CommandLine/Core/Sequence.cs delete mode 100644 src/CommandLine/Core/Switch.cs create mode 100644 tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_And_Normal_Option.cs 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..ffd6250b 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -24,6 +24,30 @@ public static ParserResult Build( 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)); var specProps = typeInfo.GetSpecifications(pi => SpecificationProperty.Create( @@ -95,7 +119,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 0593a2b2..72307bf2 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 @@ bool preprocCompare(string command) => 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) { string firstArg = arguments.First(); @@ -114,7 +140,8 @@ private static ParserResult MatchVerb( ignoreValueCase, parsingCulture, autoHelp, - autoVersion, + autoVersion, + allowMultiInstance, nonFatalErrors); } diff --git a/src/CommandLine/Core/OptionMapper.cs b/src/CommandLine/Core/OptionMapper.cs index 18349b40..d222c100 100644 --- a/src/CommandLine/Core/OptionMapper.cs +++ b/src/CommandLine/Core/OptionMapper.cs @@ -22,26 +22,31 @@ 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/PartitionExtensions.cs b/src/CommandLine/Core/PartitionExtensions.cs new file mode 100644 index 00000000..47cc397e --- /dev/null +++ b/src/CommandLine/Core/PartitionExtensions.cs @@ -0,0 +1,25 @@ +// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using CSharpx; + +namespace CommandLine.Core +{ + static class PartitionExtensions + { + public static Tuple,IEnumerable> PartitionByPredicate( + this IEnumerable items, + Func pred) + { + List yes = new List(); + List no = new List(); + foreach (T item in items) { + List list = pred(item) ? yes : no; + list.Add(item); + } + return Tuple.Create,IEnumerable>(yes, no); + } + } +} diff --git a/src/CommandLine/Core/Scalar.cs b/src/CommandLine/Core/Scalar.cs deleted file mode 100644 index 215ca2d2..00000000 --- a/src/CommandLine/Core/Scalar.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Linq; -using CommandLine.Infrastructure; -using CSharpx; - -namespace CommandLine.Core -{ - static class Scalar - { - 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.Scalar ? new[] { f, s } : new Token[] { }, new Token[] { }) - : new Token[] { }) - from t in tseq - select t; - } - } -} diff --git a/src/CommandLine/Core/Sequence.cs b/src/CommandLine/Core/Sequence.cs deleted file mode 100644 index 3a6147b2..00000000 --- a/src/CommandLine/Core/Sequence.cs +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Linq; -using CommandLine.Infrastructure; -using CSharpx; - -namespace CommandLine.Core -{ - static class Sequence - { - 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; - } - - private static IEnumerable OfSequence(this IEnumerable tokens, Token nameToken, TypeDescriptor info) - { - var nameIndex = tokens.IndexOf(t => t.Equals(nameToken)); - if (nameIndex >= 0) - { - return info.NextValue.MapValueOrDefault( - _ => info.MaxItems.MapValueOrDefault( - n => tokens.Skip(nameIndex + 1).Take(n), - tokens.Skip(nameIndex + 1).TakeWhile(v => v.IsValue() && !v.IsValueForced())), - tokens.Skip(nameIndex + 1).TakeWhile(v => v.IsValue() && !v.IsValueForced())); - } - return new Token[] { }; - } - } -} 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/Switch.cs b/src/CommandLine/Core/Switch.cs deleted file mode 100644 index 96e62443..00000000 --- a/src/CommandLine/Core/Switch.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Linq; -using CSharpx; - -namespace CommandLine.Core -{ - static class Switch - { - public static IEnumerable Partition( - IEnumerable tokens, - Func> typeLookup) - { - return from t in tokens - where typeLookup(t.Text).MapValueOrDefault(info => t.IsName() && info.TargetType == TargetType.Switch, false) - select t; - } - } -} diff --git a/src/CommandLine/Core/TokenPartitioner.cs b/src/CommandLine/Core/TokenPartitioner.cs index be38a6d0..a735a762 100644 --- a/src/CommandLine/Core/TokenPartitioner.cs +++ b/src/CommandLine/Core/TokenPartitioner.cs @@ -18,15 +18,14 @@ Tuple>>, IEnumerable tokenComparer = ReferenceEqualityComparer.Default; var tokenList = tokens.Memoize(); - var switches = new HashSet(Switch.Partition(tokenList, typeLookup), tokenComparer); - var scalars = new HashSet(Scalar.Partition(tokenList, typeLookup), tokenComparer); - var sequences = new HashSet(Sequence.Partition(tokenList, typeLookup), tokenComparer); - var nonOptions = tokenList - .Where(t => !switches.Contains(t)) - .Where(t => !scalars.Contains(t)) - .Where(t => !sequences.Contains(t)).Memoize(); - var values = nonOptions.Where(v => v.IsValue()).Memoize(); - var errors = nonOptions.Except(values, (IEqualityComparer)ReferenceEqualityComparer.Default).Memoize(); + var partitioned = PartitionTokensByType(tokenList, typeLookup); + var switches = partitioned.Item1; + var scalars = partitioned.Item2; + var sequences = partitioned.Item3; + var nonOptions = partitioned.Item4; + var valuesAndErrors = nonOptions.PartitionByPredicate(v => v.IsValue()); + var values = valuesAndErrors.Item1; + var errors = valuesAndErrors.Item2; return Tuple.Create( KeyValuePairHelper.ForSwitch(switches) @@ -35,5 +34,131 @@ Tuple>>, IEnumerable t.Text), errors); } + + public static Tuple, IEnumerable, IEnumerable, IEnumerable> PartitionTokensByType( + IEnumerable tokens, + Func> typeLookup) + { + var switchTokens = new List(); + var scalarTokens = new List(); + var sequenceTokens = new List(); + var nonOptionTokens = new List(); + var sequences = new Dictionary>(); + var count = new Dictionary(); + var max = new Dictionary>(); + var state = SequenceState.TokenSearch; + Token nameToken = null; + foreach (var token in tokens) + { + if (token.IsValueForced()) + { + nonOptionTokens.Add(token); + } + else if (token.IsName()) + { + if (typeLookup(token.Text).MatchJust(out var info)) + { + switch (info.TargetType) + { + case TargetType.Switch: + nameToken = null; + switchTokens.Add(token); + state = SequenceState.TokenSearch; + break; + case TargetType.Scalar: + nameToken = token; + scalarTokens.Add(nameToken); + state = SequenceState.ScalarTokenFound; + break; + case TargetType.Sequence: + nameToken = token; + if (! sequences.ContainsKey(nameToken)) + { + sequences[nameToken] = new List(); + count[nameToken] = 0; + max[nameToken] = info.MaxItems; + } + state = SequenceState.SequenceTokenFound; + break; + } + } + else + { + nameToken = null; + nonOptionTokens.Add(token); + state = SequenceState.TokenSearch; + } + } + else + { + switch (state) + { + case SequenceState.TokenSearch: + case SequenceState.ScalarTokenFound when nameToken == null: + case SequenceState.SequenceTokenFound when nameToken == null: + // if (nameToken == null) Console.WriteLine($" (because there was no nameToken)"); + nameToken = null; + nonOptionTokens.Add(token); + state = SequenceState.TokenSearch; + break; + + case SequenceState.ScalarTokenFound: + nameToken = null; + scalarTokens.Add(token); + state = SequenceState.TokenSearch; + break; + + case SequenceState.SequenceTokenFound: + if (sequences.TryGetValue(nameToken, out var sequence)) { + // if (max[nameToken].MatchJust(out int m) && count[nameToken] >= m) + // { + // // This sequence is completed, so this and any further values are non-option values + // nameToken = null; + // nonOptionTokens.Add(token); + // state = SequenceState.TokenSearch; + // } + // else + { + sequence.Add(token); + count[nameToken]++; + } + } + else + { + Console.WriteLine("***BUG!!!***"); + throw new NullReferenceException($"Sequence for name {nameToken} doesn't exist, and it should"); + // sequences[nameToken] = new List(new[] { token }); + } + break; + } + } + } + + foreach (var kvp in sequences) + { + if (kvp.Value.Empty()) { + nonOptionTokens.Add(kvp.Key); + } + else + { + sequenceTokens.Add(kvp.Key); + sequenceTokens.AddRange(kvp.Value); + } + } + return Tuple.Create( + (IEnumerable)switchTokens, + (IEnumerable)scalarTokens, + (IEnumerable)sequenceTokens, + (IEnumerable)nonOptionTokens + ); + } + + private enum SequenceState + { + TokenSearch, + SequenceTokenFound, + ScalarTokenFound, + } + } -} \ No newline at end of file +} diff --git a/src/CommandLine/Core/TypeConverter.cs b/src/CommandLine/Core/TypeConverter.cs index 8f193c46..ee69373e 100644 --- a/src/CommandLine/Core/TypeConverter.cs +++ b/src/CommandLine/Core/TypeConverter.cs @@ -16,7 +16,7 @@ static class TypeConverter public static Maybe 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/Fakes/Options_With_Value_Sequence_And_Normal_Option.cs b/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_And_Normal_Option.cs new file mode 100644 index 00000000..e8e7bf47 --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_Value_Sequence_And_Normal_Option.cs @@ -0,0 +1,28 @@ +// 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 +{ + public class Options_With_Value_Sequence_And_Normal_Option + { + [Option('c', "compress", + HelpText = "Compress Match Pattern, Pipe Separated (|) ", + Separator = '|', + Default = new[] + { + "*.txt", "*.log", "*.ini" + })] + public IEnumerable Compress { get; set; } + + [Value(0, + HelpText = "Input Directories.", + Required = true)] + public IEnumerable InputDirs { get; set; } + + + [Option('n', "name", + HelpText = "Metadata Name.", + Default = "WILDCARD")] + public string Name { get; set; } + } +} diff --git a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs index 3ef33261..be09f375 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()); } @@ -1251,6 +1252,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/ScalarTests.cs b/tests/CommandLine.Tests/Unit/Core/ScalarTests.cs index 9b1028f0..2d08bbcb 100644 --- a/tests/CommandLine.Tests/Unit/Core/ScalarTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/ScalarTests.cs @@ -15,12 +15,13 @@ public void Partition_scalar_values_from_empty_token_sequence() { var expected = new Token[] { }; - var result = Scalar.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new Token[] { }, name => new[] { "str", "int" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Scalar, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item2; // Switch, *Scalar*, Sequence, NonOption expected.Should().BeEquivalentTo(result); } @@ -30,7 +31,7 @@ public void Partition_scalar_values() { var expected = new [] { Token.Name("str"), Token.Value("strvalue") }; - var result = Scalar.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new [] { Token.Name("str"), Token.Value("strvalue"), Token.Value("freevalue"), @@ -40,6 +41,7 @@ public void Partition_scalar_values() new[] { "str", "int" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Scalar, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item2; // Switch, *Scalar*, Sequence, NonOption expected.Should().BeEquivalentTo(result); } diff --git a/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs b/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs index b26575b8..cd17f592 100644 --- a/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs @@ -15,12 +15,13 @@ public void Partition_sequence_values_from_empty_token_sequence() { var expected = new Token[] { }; - var result = Sequence.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new Token[] { }, name => new[] { "seq" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Sequence, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item3; // Switch, Scalar, *Sequence*, NonOption expected.Should().AllBeEquivalentTo(result); } @@ -33,7 +34,7 @@ public void Partition_sequence_values() Token.Name("seq"), Token.Value("seqval0"), Token.Value("seqval1") }; - var result = Sequence.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new[] { Token.Name("str"), Token.Value("strvalue"), Token.Value("freevalue"), @@ -44,6 +45,7 @@ public void Partition_sequence_values() new[] { "seq" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Sequence, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item3; // Switch, Scalar, *Sequence*, NonOption expected.Should().BeEquivalentTo(result); } @@ -57,7 +59,7 @@ public void Partition_sequence_values_from_two_sequneces() Token.Name("seqb"), Token.Value("seqbval0") }; - var result = Sequence.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new[] { Token.Name("str"), Token.Value("strvalue"), Token.Value("freevalue"), @@ -69,6 +71,7 @@ public void Partition_sequence_values_from_two_sequneces() new[] { "seq", "seqb" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Sequence, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item3; // Switch, Scalar, *Sequence*, NonOption expected.Should().BeEquivalentTo(result); } @@ -81,7 +84,7 @@ public void Partition_sequence_values_only() Token.Name("seq"), Token.Value("seqval0"), Token.Value("seqval1") }; - var result = Sequence.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new[] { Token.Name("seq"), Token.Value("seqval0"), Token.Value("seqval1") @@ -90,8 +93,73 @@ public void Partition_sequence_values_only() new[] { "seq" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Sequence, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item3; // Switch, Scalar, *Sequence*, NonOption 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 tokens = TokenPartitioner.PartitionTokensByType( + 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 result = tokens.Item3; // Switch, Scalar, *Sequence*, NonOption + + 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 tokens = TokenPartitioner.PartitionTokensByType( + 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()); + var result = tokens.Item3; // Switch, Scalar, *Sequence*, NonOption + + 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/SwitchTests.cs b/tests/CommandLine.Tests/Unit/Core/SwitchTests.cs index 82edb635..0fc6db70 100644 --- a/tests/CommandLine.Tests/Unit/Core/SwitchTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/SwitchTests.cs @@ -15,12 +15,13 @@ public void Partition_switch_values_from_empty_token_sequence() { var expected = new Token[] { }; - var result = Switch.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new Token[] { }, name => new[] { "x", "switch" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Switch, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item1; // *Switch*, Scalar, Sequence, NonOption expected.Should().BeEquivalentTo(result); } @@ -30,7 +31,7 @@ public void Partition_switch_values() { var expected = new [] { Token.Name("x") }; - var result = Switch.Partition( + var tokens = TokenPartitioner.PartitionTokensByType( new [] { Token.Name("str"), Token.Value("strvalue"), Token.Value("freevalue"), @@ -40,6 +41,7 @@ public void Partition_switch_values() new[] { "x", "switch" }.Contains(name) ? Maybe.Just(TypeDescriptor.Create(TargetType.Switch, Maybe.Nothing())) : Maybe.Nothing()); + var result = tokens.Item1; // *Switch*, Scalar, Sequence, NonOption expected.Should().BeEquivalentTo(result); } diff --git a/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs b/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs index c62a7836..dc16dfd0 100644 --- a/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs @@ -120,5 +120,15 @@ public static IEnumerable ChangeType_scalars_source }; } } + + [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); + + } } } diff --git a/tests/CommandLine.Tests/Unit/ParserTests.cs b/tests/CommandLine.Tests/Unit/ParserTests.cs index bc6d77a8..58ce7d3f 100644 --- a/tests/CommandLine.Tests/Unit/ParserTests.cs +++ b/tests/CommandLine.Tests/Unit/ParserTests.cs @@ -132,6 +132,21 @@ public void Parse_options_with_double_dash() // Teardown } + [Fact] + public void Parse_options_with_repeated_value_in_values_sequence_and_option() + { + var text = "x1 x2 x3 -c x1"; // x1 is the same in -c option and first value + var args = text.Split(); + var parser = new Parser(with => + { + with.HelpWriter = Console.Out; + }); + var result = parser.ParseArguments(args); + var options= (result as Parsed).Value; + options.Compress.Should().BeEquivalentTo(new[] { "x1" }); + options.InputDirs.Should().BeEquivalentTo(new[] { "x1","x2","x3" }); + } + [Fact] public void Parse_options_with_double_dash_and_option_sequence() { @@ -901,6 +916,59 @@ public void Parse_multiple_default_verbs() .WithParsed(args => throw new InvalidOperationException("Should not be parsed.")); } + [Fact] + public void Parse_repeated_options_in_verbs_scenario_with_multi_instance() + { + 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.Equal(new long[] { longVal1, longVal2, longVal3 }, verb.LongSequence); + Assert.Equal(new[] { stringVal }, verb.StringSequence); + }); + } + } + + [Fact] + public void Parse_repeated_options_in_verbs_scenario_without_multi_instance() + { + using (var sut = new Parser(settings => settings.AllowMultiInstance = false)) + { + 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); + 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 889ac3bf1fce7bfd014810e1aaa160f908209752 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 19 Aug 2020 21:37:37 +0700 Subject: [PATCH 06/15] Sequences that hit max stop grabbing values (#682) If a sequence option (one with IEnumerable or similar type) has a Max=N assigned, then once it has hit N values it will stop grabbing values from the command line, and any remaining values will be able to be assigned to other properties with the Value attribute. --- src/CommandLine/Core/TokenPartitioner.cs | 24 ++++++++--------- .../Unit/Core/InstanceBuilderTests.cs | 27 ++++++++++--------- .../Unit/Core/SequenceTests.cs | 12 ++++++++- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/CommandLine/Core/TokenPartitioner.cs b/src/CommandLine/Core/TokenPartitioner.cs index a735a762..d6fe97e1 100644 --- a/src/CommandLine/Core/TokenPartitioner.cs +++ b/src/CommandLine/Core/TokenPartitioner.cs @@ -96,7 +96,6 @@ public static Tuple, IEnumerable, IEnumerable, case SequenceState.TokenSearch: case SequenceState.ScalarTokenFound when nameToken == null: case SequenceState.SequenceTokenFound when nameToken == null: - // if (nameToken == null) Console.WriteLine($" (because there was no nameToken)"); nameToken = null; nonOptionTokens.Add(token); state = SequenceState.TokenSearch; @@ -110,14 +109,14 @@ public static Tuple, IEnumerable, IEnumerable, case SequenceState.SequenceTokenFound: if (sequences.TryGetValue(nameToken, out var sequence)) { - // if (max[nameToken].MatchJust(out int m) && count[nameToken] >= m) - // { - // // This sequence is completed, so this and any further values are non-option values - // nameToken = null; - // nonOptionTokens.Add(token); - // state = SequenceState.TokenSearch; - // } - // else + if (max[nameToken].MatchJust(out int m) && count[nameToken] >= m) + { + // This sequence is completed, so this and any further values are non-option values + nameToken = null; + nonOptionTokens.Add(token); + state = SequenceState.TokenSearch; + } + else { sequence.Add(token); count[nameToken]++; @@ -125,9 +124,10 @@ public static Tuple, IEnumerable, IEnumerable, } else { - Console.WriteLine("***BUG!!!***"); - throw new NullReferenceException($"Sequence for name {nameToken} doesn't exist, and it should"); - // sequences[nameToken] = new List(new[] { token }); + // Should never get here, but just in case: + sequences[nameToken] = new List(new[] { token }); + count[nameToken] = 0; + max[nameToken] = Maybe.Nothing(); } break; } diff --git a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs index be09f375..2f8d02b7 100644 --- a/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs @@ -185,7 +185,7 @@ public void Parse_string_sequence_with_only_max_constraint(string[] arguments, s } [Fact] - public void Breaking_min_constraint_in_string_sequence_gererates_MissingValueOptionError() + public void Breaking_min_constraint_in_string_sequence_generates_MissingValueOptionError() { // Fixture setup var expectedResult = new[] { new MissingValueOptionError(new NameInfo("s", "string-seq")) }; @@ -199,7 +199,7 @@ public void Breaking_min_constraint_in_string_sequence_gererates_MissingValueOpt } [Fact] - public void Breaking_min_constraint_in_string_sequence_as_value_gererates_SequenceOutOfRangeError() + public void Breaking_min_constraint_in_string_sequence_as_value_generates_SequenceOutOfRangeError() { // Fixture setup var expectedResult = new[] { new SequenceOutOfRangeError(NameInfo.EmptyName) }; @@ -213,21 +213,22 @@ public void Breaking_min_constraint_in_string_sequence_as_value_gererates_Sequen } [Fact] - public void Breaking_max_constraint_in_string_sequence_gererates_SequenceOutOfRangeError() + public void Breaking_max_constraint_in_string_sequence_does_not_generate_SequenceOutOfRangeError() { // Fixture setup - var expectedResult = new[] { new SequenceOutOfRangeError(new NameInfo("s", "string-seq")) }; + var expectedResult = new[] { "one", "two", "three" }; // Exercize system var result = InvokeBuild( new[] { "--string-seq=one", "two", "three", "this-is-too-much" }); // Verify outcome - ((NotParsed)result).Errors.Should().BeEquivalentTo(expectedResult); + ((Parsed)result).Value.StringSequence.Should().BeEquivalentTo(expectedResult); + // The "this-is-too-much" arg would end up assigned to a Value; since there is no Value, it is silently dropped } [Fact] - public void Breaking_max_constraint_in_string_sequence_as_value_gererates_SequenceOutOfRangeError() + public void Breaking_max_constraint_in_string_sequence_as_value_generates_SequenceOutOfRangeError() { // Fixture setup var expectedResult = new[] { new SequenceOutOfRangeError(NameInfo.EmptyName) }; @@ -427,7 +428,7 @@ public void Double_dash_force_subsequent_arguments_as_values() } [Fact] - public void Parse_option_from_different_sets_gererates_MutuallyExclusiveSetError() + public void Parse_option_from_different_sets_generates_MutuallyExclusiveSetError() { // Fixture setup var expectedResult = new[] @@ -480,7 +481,7 @@ public void Two_required_options_at_the_same_set_and_none_are_true() } [Fact] - public void Omitting_required_option_gererates_MissingRequiredOptionError() + public void Omitting_required_option_generates_MissingRequiredOptionError() { // Fixture setup var expectedResult = new[] { new MissingRequiredOptionError(new NameInfo("", "str")) }; @@ -494,7 +495,7 @@ public void Omitting_required_option_gererates_MissingRequiredOptionError() } [Fact] - public void Wrong_range_in_sequence_gererates_SequenceOutOfRangeError() + public void Wrong_range_in_sequence_generates_SequenceOutOfRangeError() { // Fixture setup var expectedResult = new[] { new SequenceOutOfRangeError(new NameInfo("i", "")) }; @@ -508,7 +509,7 @@ public void Wrong_range_in_sequence_gererates_SequenceOutOfRangeError() } [Fact] - public void Parse_unknown_long_option_gererates_UnknownOptionError() + public void Parse_unknown_long_option_generates_UnknownOptionError() { // Fixture setup var expectedResult = new[] { new UnknownOptionError("xyz") }; @@ -522,7 +523,7 @@ public void Parse_unknown_long_option_gererates_UnknownOptionError() } [Fact] - public void Parse_unknown_short_option_gererates_UnknownOptionError() + public void Parse_unknown_short_option_generates_UnknownOptionError() { // Fixture setup var expectedResult = new[] { new UnknownOptionError("z") }; @@ -536,7 +537,7 @@ public void Parse_unknown_short_option_gererates_UnknownOptionError() } [Fact] - public void Parse_unknown_short_option_in_option_group_gererates_UnknownOptionError() + public void Parse_unknown_short_option_in_option_group_generates_UnknownOptionError() { // Fixture setup var expectedResult = new[] { new UnknownOptionError("z") }; @@ -596,7 +597,7 @@ public void Parse_utf8_string_correctly(string[] arguments, string expected) } [Fact] - public void Breaking_equal_min_max_constraint_in_string_sequence_as_value_gererates_SequenceOutOfRangeError() + public void Breaking_equal_min_max_constraint_in_string_sequence_as_value_generates_SequenceOutOfRangeError() { // Fixture setup var expectedResult = new[] { new SequenceOutOfRangeError(NameInfo.EmptyName) }; diff --git a/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs b/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs index cd17f592..74a3d877 100644 --- a/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/SequenceTests.cs @@ -133,7 +133,7 @@ public void Partition_sequence_multi_instance() [Fact] public void Partition_sequence_multi_instance_with_max() { - var expected = new[] + var incorrect = new[] { Token.Name("seq"), Token.Value("seqval0"), @@ -144,6 +144,14 @@ public void Partition_sequence_multi_instance_with_max() Token.Value("seqval5"), }; + var expected = new[] + { + Token.Name("seq"), + Token.Value("seqval0"), + Token.Value("seqval1"), + Token.Value("seqval2"), + }; + var tokens = TokenPartitioner.PartitionTokensByType( new[] { @@ -159,6 +167,8 @@ public void Partition_sequence_multi_instance_with_max() : Maybe.Nothing()); var result = tokens.Item3; // Switch, Scalar, *Sequence*, NonOption + // Max of 3 will apply to the total values, so there should only be 3 values, not 6 + Assert.NotEqual(incorrect, result); Assert.Equal(expected, result); } } From 3354ffbdf5970745a7262c60db867c7409286e13 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Tue, 18 Aug 2020 16:41:04 +0700 Subject: [PATCH 07/15] Add FlagCounter property to OptionSpecification FlagCounter lets an int property count the number of times a flag appears, e.g. "-v -v -v" would produce the value 3 in an int property decorated with [Option('v', FlagCounter=true)]. --- src/CommandLine/Core/InstanceBuilder.cs | 4 +-- src/CommandLine/Core/NameLookup.cs | 2 +- src/CommandLine/Core/OptionMapper.cs | 8 +++-- src/CommandLine/Core/OptionSpecification.cs | 17 +++++++-- .../Core/SpecificationExtensions.cs | 1 + src/CommandLine/Core/TypeConverter.cs | 18 +++++++--- src/CommandLine/OptionAttribute.cs | 11 ++++++ src/CommandLine/UnParserExtensions.cs | 18 +++++++--- .../Options_With_FlagCounter_Switches.cs | 13 +++++++ .../Unit/Core/OptionMapperTests.cs | 6 ++-- .../Unit/Core/TypeConverterTests.cs | 36 +++++++++++++++++-- tests/CommandLine.Tests/Unit/ParserTests.cs | 30 ++++++++++++++++ .../Unit/UnParserExtensionsTests.cs | 22 ++++++++++++ 13 files changed, 163 insertions(+), 23 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 ffd6250b..f48127b1 100644 --- a/src/CommandLine/Core/InstanceBuilder.cs +++ b/src/CommandLine/Core/InstanceBuilder.cs @@ -88,14 +88,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/NameLookup.cs b/src/CommandLine/Core/NameLookup.cs index 3605d1a3..ccb24ea5 100644 --- a/src/CommandLine/Core/NameLookup.cs +++ b/src/CommandLine/Core/NameLookup.cs @@ -20,7 +20,7 @@ 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) + return option.ConversionType == typeof(bool) || (option.ConversionType == typeof(int) && option.FlagCounter) ? NameLookupResult.BooleanOptionFound : NameLookupResult.OtherOptionFound; } diff --git a/src/CommandLine/Core/OptionMapper.cs b/src/CommandLine/Core/OptionMapper.cs index d222c100..ded42c4f 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 @@ -27,7 +27,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) @@ -36,7 +36,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 77e7977f..1c2e4f88 100644 --- a/src/CommandLine/Core/OptionSpecification.cs +++ b/src/CommandLine/Core/OptionSpecification.cs @@ -14,18 +14,20 @@ 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 = false, bool hidden = false) : 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; 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) { 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, false, hidden); } public string ShortName @@ -78,5 +81,13 @@ public string Group { get { return group; } } + + /// + /// Whether this is an int option that counts how many times a flag was set rather than taking a value on the command line + /// + 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/Core/TypeConverter.cs b/src/CommandLine/Core/TypeConverter.cs index ee69373e..2e27af40 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/src/CommandLine/OptionAttribute.cs b/src/CommandLine/OptionAttribute.cs index 7448b697..6ae51dac 100644 --- a/src/CommandLine/OptionAttribute.cs +++ b/src/CommandLine/OptionAttribute.cs @@ -15,6 +15,7 @@ public sealed class OptionAttribute : BaseAttribute private readonly string longName; private readonly string shortName; private string setName; + private bool flagCounter; private char separator; private string group=string.Empty; @@ -96,6 +97,16 @@ public string SetName } } + /// + /// If true, this is an int option that counts how many times a flag was set (e.g. "-v -v -v" or "-vvv" would return 3). + /// The property must be of type int (signed 32-bit integer). + /// + public bool FlagCounter + { + get { return flagCounter; } + set { flagCounter = value; } + } + /// /// When applying attribute to target properties, /// it allows you to split an argument and consume its content as a sequence. diff --git a/src/CommandLine/UnParserExtensions.cs b/src/CommandLine/UnParserExtensions.cs index 51811ef9..e823a7fa 100644 --- a/src/CommandLine/UnParserExtensions.cs +++ b/src/CommandLine/UnParserExtensions.cs @@ -153,7 +153,9 @@ public static string FormatCommandLine(this Parser parser, T options, Action< var allOptSpecs = from info in specs.Where(i => i.Specification.Tag == SpecificationType.Option) let o = (OptionSpecification)info.Specification - where o.TargetType != TargetType.Switch || (o.TargetType == TargetType.Switch && ((bool)info.Value)) + where o.TargetType != TargetType.Switch || + (o.TargetType == TargetType.Switch && o.FlagCounter && ((int)info.Value > 0)) || + (o.TargetType == TargetType.Switch && ((bool)info.Value)) where !o.Hidden || settings.ShowHidden orderby o.UniqueName() select info; @@ -176,7 +178,12 @@ orderby v.Index builder = settings.GroupSwitches && shortSwitches.Any() ? builder.Append('-').Append(string.Join(string.Empty, shortSwitches.Select( - info => ((OptionSpecification)info.Specification).ShortName).ToArray())).Append(' ') + info => { + var o = (OptionSpecification)info.Specification; + return o.FlagCounter + ? string.Concat(Enumerable.Repeat(o.ShortName, (int)info.Value)) + : o.ShortName; + }).ToArray())).Append(' ') : builder; optSpecs.ForEach( opt => @@ -250,24 +257,25 @@ private static char SeperatorOrSpace(this Specification spec) private static string FormatOption(OptionSpecification spec, object value, UnParserSettings settings) { return new StringBuilder() - .Append(spec.FormatName(settings)) + .Append(spec.FormatName(value, settings)) .AppendWhen(spec.TargetType != TargetType.Switch, FormatValue(spec, value)) .ToString(); } - private static string FormatName(this OptionSpecification optionSpec, UnParserSettings settings) + private static string FormatName(this OptionSpecification optionSpec, object value, UnParserSettings settings) { // Have a long name and short name not preferred? Go with long! // No short name? Has to be long! var longName = (optionSpec.LongName.Length > 0 && !settings.PreferShortName) || optionSpec.ShortName.Length == 0; - return + var formattedName = new StringBuilder(longName ? "--".JoinTo(optionSpec.LongName) : "-".JoinTo(optionSpec.ShortName)) .AppendWhen(optionSpec.TargetType != TargetType.Switch, longName && settings.UseEqualToken ? "=" : " ") .ToString(); + return optionSpec.FlagCounter ? String.Join(" ", Enumerable.Repeat(formattedName, (int)value)) : formattedName; } private static object NormalizeValue(this object value) 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..06787b31 --- /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 63bf22f3..0a948cea 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 dc16dfd0..c3e93781 100644 --- a/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TypeConverterTests.cs @@ -27,7 +27,7 @@ enum TestFlagEnum [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); + Maybe result = TypeConverter.ChangeType(new[] {testValue}, destinationType, true, false, CultureInfo.InvariantCulture, true); if (expectFail) { @@ -121,11 +121,43 @@ public static IEnumerable ChangeType_scalars_source } } + [Theory] + [MemberData(nameof(ChangeType_flagCounters_source))] + public void ChangeType_flagCounters(string[] testValue, Type destinationType, bool expectFail, object expectedResult) + { + Maybe result = TypeConverter.ChangeType(testValue, destinationType, true, 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); + } + } + + public static IEnumerable ChangeType_flagCounters_source + { + get + { + return new[] + { + new object[] {new string[0], typeof (int), false, 0}, + new object[] {new[] {"true"}, typeof (int), false, 1}, + new object[] {new[] {"true", "true"}, typeof (int), false, 2}, + new object[] {new[] {"true", "true", "true"}, typeof (int), false, 3}, + new object[] {new[] {"true", "x"}, typeof (int), true, 0}, + }; + } + } + [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); + 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); diff --git a/tests/CommandLine.Tests/Unit/ParserTests.cs b/tests/CommandLine.Tests/Unit/ParserTests.cs index 58ce7d3f..f8593cf3 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() { diff --git a/tests/CommandLine.Tests/Unit/UnParserExtensionsTests.cs b/tests/CommandLine.Tests/Unit/UnParserExtensionsTests.cs index 50cb3290..7e878f32 100644 --- a/tests/CommandLine.Tests/Unit/UnParserExtensionsTests.cs +++ b/tests/CommandLine.Tests/Unit/UnParserExtensionsTests.cs @@ -254,6 +254,23 @@ public static void UnParsing_instance_with_int_nullable(bool skipDefault, int? v .Should().BeEquivalentTo(expected); } + + [Theory] + [InlineData(false, false, 0, "")] + [InlineData(false, false, 1, "-v")] // default but not skipped + [InlineData(false, false, 2, "-v -v")] + [InlineData(false, true, 2, "-vv")] + [InlineData(false, false, 3, "-v -v -v")] + [InlineData(false, true, 3, "-vvv")] + [InlineData(true, false, 1, "")] // default, skipped + public static void UnParsing_instance_with_flag_counter(bool skipDefault, bool groupSwitches, int value, string expected) + { + var options = new Option_FlagCounter { VerboseLevel = value }; + var result = new Parser() + .FormatCommandLine(options, x => { x.SkipDefault = skipDefault; x.GroupSwitches = groupSwitches; }) + .Should().BeEquivalentTo(expected); + } + [Theory] [InlineData(Shapes.Circle, "--shape Circle")] [InlineData(Shapes.Square, "--shape Square")] @@ -311,6 +328,11 @@ class Option_Int [Option('v', Default = 1)] public int VerboseLevel { get; set; } } + class Option_FlagCounter + { + [Option('v', Default = 1, FlagCounter=true)] + public int VerboseLevel { get; set; } + } class Option_Nullable_Bool { [Option('v')] From 570d7b77695b0506e15fe9d5d723f3ffcf03728b Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 19 Aug 2020 10:02:06 +0700 Subject: [PATCH 08/15] Add GetoptMode parser setting and implementation Turning on Getopt mode automatically turns on the EnableDashDash and AllowMultiInstance settings as well, but they can be disabled by explicitly setting them to false in the parser settings. --- src/CommandLine/Core/GetoptTokenizer.cs | 219 ++++++++++++++ .../Infrastructure/StringExtensions.cs | 20 +- src/CommandLine/Parser.cs | 9 +- src/CommandLine/ParserSettings.cs | 40 ++- .../Fakes/Simple_Options_With_ExtraArgs.cs | 27 ++ .../Unit/Core/GetoptTokenizerTests.cs | 126 ++++++++ .../Unit/Core/TokenizerTests.cs | 3 +- .../Unit/GetoptParserTests.cs | 284 ++++++++++++++++++ tests/CommandLine.Tests/Unit/ParserTests.cs | 3 + 9 files changed, 720 insertions(+), 11 deletions(-) create mode 100644 src/CommandLine/Core/GetoptTokenizer.cs create mode 100644 tests/CommandLine.Tests/Fakes/Simple_Options_With_ExtraArgs.cs create mode 100644 tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs create mode 100644 tests/CommandLine.Tests/Unit/GetoptParserTests.cs diff --git a/src/CommandLine/Core/GetoptTokenizer.cs b/src/CommandLine/Core/GetoptTokenizer.cs new file mode 100644 index 00000000..d9fe63fc --- /dev/null +++ b/src/CommandLine/Core/GetoptTokenizer.cs @@ -0,0 +1,219 @@ +// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using CommandLine.Infrastructure; +using CSharpx; +using RailwaySharp.ErrorHandling; +using System.Text.RegularExpressions; + +namespace CommandLine.Core +{ + static class GetoptTokenizer + { + public static Result, Error> Tokenize( + IEnumerable arguments, + Func nameLookup) + { + return GetoptTokenizer.Tokenize(arguments, nameLookup, ignoreUnknownArguments:false, allowDashDash:true, posixlyCorrect:false); + } + + public static Result, Error> Tokenize( + IEnumerable arguments, + Func nameLookup, + bool ignoreUnknownArguments, + bool allowDashDash, + bool posixlyCorrect) + { + var errors = new List(); + 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; + Action onConsumeNext = (n => consumeNext = consumeNext + n); + bool forceValues = false; + + var tokens = new List(); + + var enumerator = arguments.GetEnumerator(); + while (enumerator.MoveNext()) + { + switch (enumerator.Current) { + case null: + break; + + case string arg when forceValues: + tokens.Add(Token.ValueForced(arg)); + break; + + case string arg when consumeNext > 0: + tokens.Add(Token.Value(arg)); + consumeNext = consumeNext - 1; + break; + + case "--" when allowDashDash: + forceValues = true; + break; + + case "--": + tokens.Add(Token.Value("--")); + if (posixlyCorrect) forceValues = true; + break; + + case "-": + // A single hyphen is always a value (it usually means "read from stdin" or "write to stdout") + tokens.Add(Token.Value("-")); + if (posixlyCorrect) forceValues = true; + break; + + case string arg when arg.StartsWith("--"): + tokens.AddRange(TokenizeLongName(arg, nameLookup, onBadFormatToken, onUnknownOption, onConsumeNext)); + break; + + case string arg when arg.StartsWith("-"): + tokens.AddRange(TokenizeShortName(arg, nameLookup, onUnknownOption, onConsumeNext)); + break; + + case string arg: + // If we get this far, it's a plain value + tokens.Add(Token.Value(arg)); + if (posixlyCorrect) forceValues = true; + break; + } + } + + return Result.Succeed, Error>(tokens.AsEnumerable(), errors.AsEnumerable()); + } + + public static Result, Error> ExplodeOptionList( + Result, Error> tokenizerResult, + Func> optionSequenceWithSeparatorLookup) + { + var tokens = tokenizerResult.SucceededWith().Memoize(); + + var replaces = tokens.Select((t, i) => + optionSequenceWithSeparatorLookup(t.Text) + .MapValueOrDefault(sep => Tuple.Create(i + 1, sep), + Tuple.Create(-1, '\0'))).SkipWhile(x => x.Item1 < 0).Memoize(); + + var exploded = tokens.Select((t, i) => + replaces.FirstOrDefault(x => x.Item1 == i).ToMaybe() + .MapValueOrDefault(r => t.Text.Split(r.Item2).Select(Token.Value), + Enumerable.Empty().Concat(new[] { t }))); + + var flattened = exploded.SelectMany(x => x); + + return Result.Succeed(flattened, tokenizerResult.SuccessMessages()); + } + + public static Func< + IEnumerable, + IEnumerable, + Result, Error>> + ConfigureTokenizer( + StringComparer nameComparer, + bool ignoreUnknownArguments, + bool enableDashDash, + bool posixlyCorrect) + { + return (arguments, optionSpecs) => + { + var tokens = GetoptTokenizer.Tokenize(arguments, name => NameLookup.Contains(name, optionSpecs, nameComparer), ignoreUnknownArguments, enableDashDash, posixlyCorrect); + var explodedTokens = GetoptTokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer)); + 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; + } + } + } +} diff --git a/src/CommandLine/Infrastructure/StringExtensions.cs b/src/CommandLine/Infrastructure/StringExtensions.cs index 7bfab66a..db8aa0bd 100644 --- a/src/CommandLine/Infrastructure/StringExtensions.cs +++ b/src/CommandLine/Infrastructure/StringExtensions.cs @@ -73,5 +73,23 @@ public static bool ToBoolean(this string value) { return value.Equals("true", StringComparison.OrdinalIgnoreCase); } + + public static bool ToBooleanLoose(this string value) + { + if ((string.IsNullOrEmpty(value)) || + (value == "0") || + (value.Equals("f", StringComparison.OrdinalIgnoreCase)) || + (value.Equals("n", StringComparison.OrdinalIgnoreCase)) || + (value.Equals("no", StringComparison.OrdinalIgnoreCase)) || + (value.Equals("off", StringComparison.OrdinalIgnoreCase)) || + (value.Equals("false", StringComparison.OrdinalIgnoreCase))) + { + return false; + } + else + { + return true; + } + } } -} \ No newline at end of file +} diff --git a/src/CommandLine/Parser.cs b/src/CommandLine/Parser.cs index 10c9b4e1..4301aa52 100644 --- a/src/CommandLine/Parser.cs +++ b/src/CommandLine/Parser.cs @@ -185,8 +185,13 @@ private static Result, Error> Tokenize( IEnumerable optionSpecs, ParserSettings settings) { - return - Tokenizer.ConfigureTokenizer( + return settings.GetoptMode + ? GetoptTokenizer.ConfigureTokenizer( + settings.NameComparer, + settings.IgnoreUnknownArguments, + settings.EnableDashDash, + settings.PosixlyCorrect)(arguments, optionSpecs) + : Tokenizer.ConfigureTokenizer( settings.NameComparer, settings.IgnoreUnknownArguments, settings.EnableDashDash)(arguments, optionSpecs); diff --git a/src/CommandLine/ParserSettings.cs b/src/CommandLine/ParserSettings.cs index 95a4cd81..5ed73f30 100644 --- a/src/CommandLine/ParserSettings.cs +++ b/src/CommandLine/ParserSettings.cs @@ -5,6 +5,7 @@ using System.IO; using CommandLine.Infrastructure; +using CSharpx; namespace CommandLine { @@ -23,9 +24,11 @@ public class ParserSettings : IDisposable private bool autoHelp; private bool autoVersion; private CultureInfo parsingCulture; - private bool enableDashDash; + private Maybe enableDashDash; private int maximumDisplayWidth; - private bool allowMultiInstance; + private Maybe allowMultiInstance; + private bool getoptMode; + private Maybe posixlyCorrect; /// /// Initializes a new instance of the class. @@ -38,6 +41,10 @@ public ParserSettings() autoVersion = true; parsingCulture = CultureInfo.InvariantCulture; maximumDisplayWidth = GetWindowWidth(); + getoptMode = false; + enableDashDash = Maybe.Nothing(); + allowMultiInstance = Maybe.Nothing(); + posixlyCorrect = Maybe.Nothing(); } private int GetWindowWidth() @@ -159,11 +166,12 @@ public bool AutoVersion /// /// Gets or sets a value indicating whether enable double dash '--' syntax, /// that forces parsing of all subsequent tokens as values. + /// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false. /// public bool EnableDashDash { - get { return enableDashDash; } - set { PopsicleSetter.Set(Consumed, ref enableDashDash, value); } + get => enableDashDash.MatchJust(out bool value) ? value : getoptMode; + set => PopsicleSetter.Set(Consumed, ref enableDashDash, Maybe.Just(value)); } /// @@ -177,11 +185,31 @@ public int MaximumDisplayWidth /// /// Gets or sets a value indicating whether options are allowed to be specified multiple times. + /// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying AllowMultiInstance = false. /// public bool AllowMultiInstance { - get => allowMultiInstance; - set => PopsicleSetter.Set(Consumed, ref allowMultiInstance, value); + get => allowMultiInstance.MatchJust(out bool value) ? value : getoptMode; + set => PopsicleSetter.Set(Consumed, ref allowMultiInstance, Maybe.Just(value)); + } + + /// + /// Whether strict getopt-like processing is applied to option values; if true, AllowMultiInstance and EnableDashDash will default to true as well. + /// + public bool GetoptMode + { + get => getoptMode; + set => PopsicleSetter.Set(Consumed, ref getoptMode, value); + } + + /// + /// Whether getopt-like processing should follow the POSIX rules (the equivalent of using the "+" prefix in the C getopt() call). + /// If not explicitly set, will default to false unless the POSIXLY_CORRECT environment variable is set, in which case it will default to true. + /// + public bool PosixlyCorrect + { + get => posixlyCorrect.MapValueOrDefault(val => val, () => Environment.GetEnvironmentVariable("POSIXLY_CORRECT").ToBooleanLoose()); + set => PopsicleSetter.Set(Consumed, ref posixlyCorrect, Maybe.Just(value)); } internal StringComparer NameComparer diff --git a/tests/CommandLine.Tests/Fakes/Simple_Options_With_ExtraArgs.cs b/tests/CommandLine.Tests/Fakes/Simple_Options_With_ExtraArgs.cs new file mode 100644 index 00000000..bb276fa5 --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Simple_Options_With_ExtraArgs.cs @@ -0,0 +1,27 @@ +// 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 +{ + public class Simple_Options_WithExtraArgs + { + [Option(HelpText = "Define a string value here.")] + public string StringValue { get; set; } + + [Option('s', "shortandlong", HelpText = "Example with both short and long name.")] + public string ShortAndLong { get; set; } + + [Option('i', Min = 3, Max = 4, Separator = ',', HelpText = "Define a int sequence here.")] + public IEnumerable IntSequence { get; set; } + + [Option('x', HelpText = "Define a boolean or switch value here.")] + public bool BoolValue { get; set; } + + [Value(0, HelpText = "Define a long value here.")] + public long LongValue { get; set; } + + [Value(1, HelpText = "Extra args get collected here.")] + public IEnumerable ExtraArgs { get; set; } + } +} diff --git a/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs b/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs new file mode 100644 index 00000000..337a9a3f --- /dev/null +++ b/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs @@ -0,0 +1,126 @@ +// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using Xunit; +using FluentAssertions; +using CSharpx; +using RailwaySharp.ErrorHandling; +using CommandLine.Core; + +namespace CommandLine.Tests.Unit.Core +{ + public class GetoptTokenizerTests + { + [Fact] + public void Explode_scalar_with_separator_in_odd_args_input_returns_sequence() + { + // Fixture setup + 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)}; + + // Exercize system + var result = + GetoptTokenizer.ExplodeOptionList( + Result.Succeed( + Enumerable.Empty().Concat(new[] { Token.Name("i"), Token.Value("10"), + Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }), + Enumerable.Empty()), + optionName => NameLookup.HavingSeparator(optionName, specs, StringComparer.Ordinal)); + // Verify outcome + ((Ok, Error>)result).Success.Should().BeEquivalentTo(expectedTokens); + + // Teardown + } + + [Fact] + public void Explode_scalar_with_separator_in_even_args_input_returns_sequence() + { + // Fixture setup + 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)}; + + // Exercize system + var result = + GetoptTokenizer.ExplodeOptionList( + Result.Succeed( + Enumerable.Empty().Concat(new[] { Token.Name("x"), + Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }), + Enumerable.Empty()), + optionName => NameLookup.HavingSeparator(optionName, specs, StringComparer.Ordinal)); + + // Verify outcome + ((Ok, Error>)result).Success.Should().BeEquivalentTo(expectedTokens); + + // Teardown + } + + [Fact] + public void Should_properly_parse_option_with_equals_in_value() + { + /** + * This is how the arg. would look in `static void Main(string[] args)` + * if passed from the command-line and the option-value wrapped in quotes. + * Ex.) ./app --connectionString="Server=localhost;Data Source..." + */ + var args = new[] { "--connectionString=Server=localhost;Data Source=(LocalDB)\v12.0;Initial Catalog=temp;" }; + + var result = GetoptTokenizer.Tokenize(args, name => NameLookupResult.OtherOptionFound); + + var tokens = result.SucceededWith(); + + Assert.NotNull(tokens); + Assert.Equal(2, tokens.Count()); + Assert.Equal("connectionString", tokens.First().Text); + Assert.Equal("Server=localhost;Data Source=(LocalDB)\v12.0;Initial Catalog=temp;", tokens.Last().Text); + } + + [Fact] + public void Should_return_error_if_option_format_with_equals_is_not_correct() + { + var args = new[] { "--option1 = fail", "--option2= succeed" }; + + var result = GetoptTokenizer.Tokenize(args, name => NameLookupResult.OtherOptionFound); + + var errors = 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(TokenType.Name, tokens.First().Tag); + Assert.Equal(TokenType.Value, tokens.Last().Tag); + Assert.Equal("option2", tokens.First().Text); + Assert.Equal(" succeed", tokens.Last().Text); + } + + + [Theory] + [InlineData(new[] { "-a", "-" }, 2,"a","-")] + [InlineData(new[] { "--file", "-" }, 2,"file","-")] + [InlineData(new[] { "-f-" }, 2,"f", "-")] + [InlineData(new[] { "--file=-" }, 2, "file", "-")] + [InlineData(new[] { "-a", "--" }, 2, "a", "--")] + public void Single_dash_as_a_value(string[] args, int countExcepted,string first,string last) + { + //Arrange + //Act + var result = GetoptTokenizer.Tokenize(args, name => NameLookupResult.OtherOptionFound); + var tokens = result.SucceededWith().ToList(); + //Assert + tokens.Should().NotBeNull(); + tokens.Count.Should().Be(countExcepted); + tokens.First().Text.Should().Be(first); + tokens.Last().Text.Should().Be(last); + } + } + +} diff --git a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs index f14eea51..a489b741 100644 --- a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs @@ -124,14 +124,13 @@ public void Should_return_error_if_option_format_with_equals_is_not_correct() Assert.Equal(ErrorType.BadFormatTokenError, tokens.Last().Tag); } - [Theory] [InlineData(new[] { "-a", "-" }, 2,"a","-")] [InlineData(new[] { "--file", "-" }, 2,"file","-")] [InlineData(new[] { "-f-" }, 2,"f", "-")] [InlineData(new[] { "--file=-" }, 2, "file", "-")] [InlineData(new[] { "-a", "--" }, 1, "a", "a")] - public void single_dash_as_a_value(string[] args, int countExcepted,string first,string last) + public void Single_dash_as_a_value(string[] args, int countExcepted,string first,string last) { //Arrange //Act diff --git a/tests/CommandLine.Tests/Unit/GetoptParserTests.cs b/tests/CommandLine.Tests/Unit/GetoptParserTests.cs new file mode 100644 index 00000000..cd2a1577 --- /dev/null +++ b/tests/CommandLine.Tests/Unit/GetoptParserTests.cs @@ -0,0 +1,284 @@ +using System; +using Xunit; +using FluentAssertions; +using CommandLine.Core; +using CommandLine.Tests.Fakes; +using System.IO; +using System.Linq; +using System.Collections.Generic; + +namespace CommandLine.Tests.Unit +{ + public class GetoptParserTests + { + public GetoptParserTests() + { + } + + public class SimpleArgsData : TheoryData + { + public SimpleArgsData() + { + // Options and values can be mixed by default + Add(new string [] { "--stringvalue=foo", "-x", "256" }, + new Simple_Options_WithExtraArgs { + IntSequence = Enumerable.Empty(), + ShortAndLong = null, + StringValue = "foo", + BoolValue = true, + LongValue = 256, + ExtraArgs = Enumerable.Empty(), + }); + Add(new string [] { "256", "--stringvalue=foo", "-x" }, + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + IntSequence = Enumerable.Empty(), + BoolValue = true, + LongValue = 256, + ExtraArgs = Enumerable.Empty(), + }); + Add(new string [] {"--stringvalue=foo", "256", "-x" }, + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + IntSequence = Enumerable.Empty(), + BoolValue = true, + LongValue = 256, + ExtraArgs = Enumerable.Empty(), + }); + + // Sequences end at first non-value arg even if they haven't yet consumed their max + Add(new string [] {"--stringvalue=foo", "-i1", "2", "3", "-x", "256" }, + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + IntSequence = new[] { 1, 2, 3 }, + BoolValue = true, + LongValue = 256, + ExtraArgs = Enumerable.Empty(), + }); + // Sequences also end after consuming their max even if there would be more parameters + Add(new string [] {"--stringvalue=foo", "-i1", "2", "3", "4", "256", "-x" }, + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + IntSequence = new[] { 1, 2, 3, 4 }, + BoolValue = true, + LongValue = 256, + ExtraArgs = Enumerable.Empty(), + }); + + // The special -- option, if not consumed, turns off further option processing + Add(new string [] {"--stringvalue", "foo", "256", "-x", "-sbar" }, + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = "bar", + BoolValue = true, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = Enumerable.Empty(), + }); + Add(new string [] {"--stringvalue", "foo", "--", "256", "-x", "-sbar" }, + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + BoolValue = false, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = new [] { "-x", "-sbar" }, + }); + + // But if -- is specified as a value following an equal sign, it has no special meaning + Add(new string [] {"--stringvalue=--", "256", "-x", "-sbar" }, + new Simple_Options_WithExtraArgs { + StringValue = "--", + ShortAndLong = "bar", + BoolValue = true, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = Enumerable.Empty(), + }); + + // Options that take values will take the next arg whatever it looks like + Add(new string [] {"--stringvalue", "-x", "256" }, + new Simple_Options_WithExtraArgs { + StringValue = "-x", + BoolValue = false, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = Enumerable.Empty(), + }); + Add(new string [] {"--stringvalue", "-x", "-x", "256" }, + new Simple_Options_WithExtraArgs { + StringValue = "-x", + BoolValue = true, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = Enumerable.Empty(), + }); + + // That applies even if the next arg is -- which would normally stop option processing: if it's after an option that takes a value, it's consumed as the value + Add(new string [] {"--stringvalue", "--", "256", "-x", "-sbar" }, + new Simple_Options_WithExtraArgs { + StringValue = "--", + ShortAndLong = "bar", + BoolValue = true, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = Enumerable.Empty(), + }); + + // Options that take values will not swallow the next arg if a value was specified with = + Add(new string [] {"--stringvalue=-x", "256" }, + new Simple_Options_WithExtraArgs { + StringValue = "-x", + BoolValue = false, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = Enumerable.Empty(), + }); + Add(new string [] {"--stringvalue=-x", "-x", "256" }, + new Simple_Options_WithExtraArgs { + StringValue = "-x", + BoolValue = true, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = Enumerable.Empty(), + }); + } + } + + [Theory] + [ClassData(typeof(SimpleArgsData))] + public void Getopt_parser_without_posixly_correct_allows_mixed_options_and_nonoptions(string[] args, Simple_Options_WithExtraArgs expected) + { + // Arrange + var sut = new Parser(config => { + config.GetoptMode = true; + config.PosixlyCorrect = false; + }); + + // Act + var result = sut.ParseArguments(args); + + // Assert + if (result is Parsed parsed) { + parsed.Value.Should().BeEquivalentTo(expected); + } else if (result is NotParsed notParsed) { + Console.WriteLine(String.Join(", ", notParsed.Errors.Select(err => err.Tag.ToString()))); + } + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + public class SimpleArgsDataWithPosixlyCorrect : TheoryData + { + public SimpleArgsDataWithPosixlyCorrect() + { + Add(new string [] { "--stringvalue=foo", "-x", "256" }, + // Parses all options + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + IntSequence = Enumerable.Empty(), + BoolValue = true, + LongValue = 256, + ExtraArgs = Enumerable.Empty(), + }); + Add(new string [] { "256", "--stringvalue=foo", "-x" }, + // Stops parsing after "256", so StringValue and BoolValue not set + new Simple_Options_WithExtraArgs { + StringValue = null, + ShortAndLong = null, + IntSequence = Enumerable.Empty(), + BoolValue = false, + LongValue = 256, + ExtraArgs = new string[] { "--stringvalue=foo", "-x" }, + }); + Add(new string [] {"--stringvalue=foo", "256", "-x" }, + // Stops parsing after "256", so StringValue is set but BoolValue is not + new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + IntSequence = Enumerable.Empty(), + BoolValue = false, + LongValue = 256, + ExtraArgs = new string[] { "-x" }, + }); + } + } + + [Theory] + [ClassData(typeof(SimpleArgsDataWithPosixlyCorrect))] + public void Getopt_parser_with_posixly_correct_stops_parsing_at_first_nonoption(string[] args, Simple_Options_WithExtraArgs expected) + { + // Arrange + var sut = new Parser(config => { + config.GetoptMode = true; + config.PosixlyCorrect = true; + config.EnableDashDash = true; + }); + + // Act + var result = sut.ParseArguments(args); + + // Assert + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + [Fact] + public void Getopt_mode_defaults_to_EnableDashDash_being_true() + { + // Arrange + var sut = new Parser(config => { + config.GetoptMode = true; + config.PosixlyCorrect = false; + }); + var args = new string [] {"--stringvalue", "foo", "256", "--", "-x", "-sbar" }; + var expected = new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = null, + BoolValue = false, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = new [] { "-x", "-sbar" }, + }; + + // Act + var result = sut.ParseArguments(args); + + // Assert + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + [Fact] + public void Getopt_mode_can_have_EnableDashDash_expicitly_disabled() + { + // Arrange + var sut = new Parser(config => { + config.GetoptMode = true; + config.PosixlyCorrect = false; + config.EnableDashDash = false; + }); + var args = new string [] {"--stringvalue", "foo", "256", "--", "-x", "-sbar" }; + var expected = new Simple_Options_WithExtraArgs { + StringValue = "foo", + ShortAndLong = "bar", + BoolValue = true, + LongValue = 256, + IntSequence = Enumerable.Empty(), + ExtraArgs = new [] { "--" }, + }; + + // Act + var result = sut.ParseArguments(args); + + // Assert + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + } +} diff --git a/tests/CommandLine.Tests/Unit/ParserTests.cs b/tests/CommandLine.Tests/Unit/ParserTests.cs index f8593cf3..b079ce0f 100644 --- a/tests/CommandLine.Tests/Unit/ParserTests.cs +++ b/tests/CommandLine.Tests/Unit/ParserTests.cs @@ -136,6 +136,7 @@ public void Parse_repeated_options_with_default_parser() // Verify outcome Assert.IsType>(result); + // NOTE: Once GetoptMode becomes the default, it will imply MultiInstance and the above check will fail because it will be Parsed. // Teardown } @@ -298,6 +299,7 @@ public void Parse_repeated_options_with_default_parser_in_verbs_scenario() // Verify outcome Assert.IsType>(result); + // NOTE: Once GetoptMode becomes the default, it will imply MultiInstance and the above check will fail because it will be Parsed. // Teardown } @@ -973,6 +975,7 @@ public void Parse_repeated_options_in_verbs_scenario_with_multi_instance() [Fact] public void Parse_repeated_options_in_verbs_scenario_without_multi_instance() { + // NOTE: Once GetoptMode becomes the default, it will imply MultiInstance and this test will fail because the parser result will be Parsed. using (var sut = new Parser(settings => settings.AllowMultiInstance = false)) { var longVal1 = 100; From 47618e0ddf43a380baaa54484b188118b30d8a6c Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Thu, 20 Aug 2020 17:43:18 +0700 Subject: [PATCH 09/15] Fix enumerable options grabbing too many values Fixes #687 Fixes #619 Fixes #617 Fixes #510 Fixes #454 Fixes #420 Fixes #396 Fixes #91 --- src/CommandLine/Core/GetoptTokenizer.cs | 35 +++-- src/CommandLine/Core/Token.cs | 34 ++++- src/CommandLine/Core/TokenPartitioner.cs | 20 +++ src/CommandLine/Core/Tokenizer.cs | 35 +++-- ...th_Sequence_Having_Separator_And_Values.cs | 74 +++++++++ .../Fakes/Options_With_Similar_Names.cs | 31 ++++ .../Unit/SequenceParsingTests.cs | 140 ++++++++++++++++++ 7 files changed, 339 insertions(+), 30 deletions(-) create mode 100644 tests/CommandLine.Tests/Fakes/Options_With_Sequence_Having_Separator_And_Values.cs create mode 100644 tests/CommandLine.Tests/Fakes/Options_With_Similar_Names.cs create mode 100644 tests/CommandLine.Tests/Unit/SequenceParsingTests.cs diff --git a/src/CommandLine/Core/GetoptTokenizer.cs b/src/CommandLine/Core/GetoptTokenizer.cs index d9fe63fc..b8c97fc2 100644 --- a/src/CommandLine/Core/GetoptTokenizer.cs +++ b/src/CommandLine/Core/GetoptTokenizer.cs @@ -94,19 +94,28 @@ public static Result, Error> ExplodeOptionList( { var tokens = tokenizerResult.SucceededWith().Memoize(); - var replaces = tokens.Select((t, i) => - optionSequenceWithSeparatorLookup(t.Text) - .MapValueOrDefault(sep => Tuple.Create(i + 1, sep), - Tuple.Create(-1, '\0'))).SkipWhile(x => x.Item1 < 0).Memoize(); - - var exploded = tokens.Select((t, i) => - replaces.FirstOrDefault(x => x.Item1 == i).ToMaybe() - .MapValueOrDefault(r => t.Text.Split(r.Item2).Select(Token.Value), - Enumerable.Empty().Concat(new[] { t }))); - - var flattened = exploded.SelectMany(x => x); - - return Result.Succeed(flattened, tokenizerResult.SuccessMessages()); + var exploded = new List(tokens is ICollection coll ? coll.Count : tokens.Count()); + var nothing = Maybe.Nothing(); // Re-use same Nothing instance for efficiency + var separator = nothing; + foreach (var token in tokens) { + if (token.IsName()) { + separator = optionSequenceWithSeparatorLookup(token.Text); + exploded.Add(token); + } else { + // Forced values are never considered option values, so they should not be split + if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) { + if (token.Text.Contains(sep)) { + exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator)); + } else { + exploded.Add(token); + } + } else { + exploded.Add(token); + } + separator = nothing; // Only first value after a separator can possibly be split + } + } + return Result.Succeed(exploded as IEnumerable, tokenizerResult.SuccessMessages()); } public static Func< diff --git a/src/CommandLine/Core/Token.cs b/src/CommandLine/Core/Token.cs index c8641bdd..90bbe54d 100644 --- a/src/CommandLine/Core/Token.cs +++ b/src/CommandLine/Core/Token.cs @@ -34,7 +34,12 @@ public static Token Value(string text, bool explicitlyAssigned) public static Token ValueForced(string text) { - return new Value(text, false, true); + return new Value(text, false, true, false); + } + + public static Token ValueFromSeparator(string text) + { + return new Value(text, false, false, true); } public TokenType Tag @@ -86,29 +91,45 @@ class Value : Token, IEquatable { private readonly bool explicitlyAssigned; private readonly bool forced; + private readonly bool fromSeparator; public Value(string text) - : this(text, false, false) + : this(text, false, false, false) { } public Value(string text, bool explicitlyAssigned) - : this(text, explicitlyAssigned, false) + : this(text, explicitlyAssigned, false, false) { } - public Value(string text, bool explicitlyAssigned, bool forced) + public Value(string text, bool explicitlyAssigned, bool forced, bool fromSeparator) : base(TokenType.Value, text) { this.explicitlyAssigned = explicitlyAssigned; this.forced = forced; + this.fromSeparator = fromSeparator; } + /// + /// Whether this value came from a long option with "=" separating the name from the value + /// public bool ExplicitlyAssigned { get { return explicitlyAssigned; } } + /// + /// Whether this value came from a sequence specified with a separator (e.g., "--files a.txt,b.txt,c.txt") + /// + public bool FromSeparator + { + get { return fromSeparator; } + } + + /// + /// Whether this value came from args after the -- separator (when EnableDashDash = true) + /// public bool Forced { get { return forced; } @@ -153,6 +174,11 @@ public static bool IsValue(this Token token) return token.Tag == TokenType.Value; } + public static bool IsValueFromSeparator(this Token token) + { + return token.IsValue() && ((Value)token).FromSeparator; + } + public static bool IsValueForced(this Token token) { return token.IsValue() && ((Value)token).Forced; diff --git a/src/CommandLine/Core/TokenPartitioner.cs b/src/CommandLine/Core/TokenPartitioner.cs index d6fe97e1..4dc25f7f 100644 --- a/src/CommandLine/Core/TokenPartitioner.cs +++ b/src/CommandLine/Core/TokenPartitioner.cs @@ -47,15 +47,18 @@ public static Tuple, IEnumerable, IEnumerable, var count = new Dictionary(); var max = new Dictionary>(); var state = SequenceState.TokenSearch; + var separatorSeen = false; Token nameToken = null; foreach (var token in tokens) { if (token.IsValueForced()) { + separatorSeen = false; nonOptionTokens.Add(token); } else if (token.IsName()) { + separatorSeen = false; if (typeLookup(token.Text).MatchJust(out var info)) { switch (info.TargetType) @@ -96,12 +99,14 @@ public static Tuple, IEnumerable, IEnumerable, case SequenceState.TokenSearch: case SequenceState.ScalarTokenFound when nameToken == null: case SequenceState.SequenceTokenFound when nameToken == null: + separatorSeen = false; nameToken = null; nonOptionTokens.Add(token); state = SequenceState.TokenSearch; break; case SequenceState.ScalarTokenFound: + separatorSeen = false; nameToken = null; scalarTokens.Add(token); state = SequenceState.TokenSearch; @@ -116,6 +121,20 @@ public static Tuple, IEnumerable, IEnumerable, nonOptionTokens.Add(token); state = SequenceState.TokenSearch; } + else if (token.IsValueFromSeparator()) + { + separatorSeen = true; + sequence.Add(token); + count[nameToken]++; + } + else if (separatorSeen) + { + // Previous token came from a separator but this one didn't: sequence is completed + separatorSeen = false; + nameToken = null; + nonOptionTokens.Add(token); + state = SequenceState.TokenSearch; + } else { sequence.Add(token); @@ -125,6 +144,7 @@ public static Tuple, IEnumerable, IEnumerable, else { // Should never get here, but just in case: + separatorSeen = false; sequences[nameToken] = new List(new[] { token }); count[nameToken] = 0; max[nameToken] = Maybe.Nothing(); diff --git a/src/CommandLine/Core/Tokenizer.cs b/src/CommandLine/Core/Tokenizer.cs index e35edc9d..b71ec7e2 100644 --- a/src/CommandLine/Core/Tokenizer.cs +++ b/src/CommandLine/Core/Tokenizer.cs @@ -62,19 +62,28 @@ public static Result, Error> ExplodeOptionList( { var tokens = tokenizerResult.SucceededWith().Memoize(); - var replaces = tokens.Select((t, i) => - optionSequenceWithSeparatorLookup(t.Text) - .MapValueOrDefault(sep => Tuple.Create(i + 1, sep), - Tuple.Create(-1, '\0'))).SkipWhile(x => x.Item1 < 0).Memoize(); - - var exploded = tokens.Select((t, i) => - replaces.FirstOrDefault(x => x.Item1 == i).ToMaybe() - .MapValueOrDefault(r => t.Text.Split(r.Item2).Select(Token.Value), - Enumerable.Empty().Concat(new[] { t }))); - - var flattened = exploded.SelectMany(x => x); - - return Result.Succeed(flattened, tokenizerResult.SuccessMessages()); + var exploded = new List(tokens is ICollection coll ? coll.Count : tokens.Count()); + var nothing = Maybe.Nothing(); // Re-use same Nothing instance for efficiency + var separator = nothing; + foreach (var token in tokens) { + if (token.IsName()) { + separator = optionSequenceWithSeparatorLookup(token.Text); + exploded.Add(token); + } else { + // Forced values are never considered option values, so they should not be split + if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) { + if (token.Text.Contains(sep)) { + exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator)); + } else { + exploded.Add(token); + } + } else { + exploded.Add(token); + } + separator = nothing; // Only first value after a separator can possibly be split + } + } + return Result.Succeed(exploded as IEnumerable, tokenizerResult.SuccessMessages()); } public static IEnumerable Normalize( diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Sequence_Having_Separator_And_Values.cs b/tests/CommandLine.Tests/Fakes/Options_With_Sequence_Having_Separator_And_Values.cs new file mode 100644 index 00000000..6099b5b5 --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_Sequence_Having_Separator_And_Values.cs @@ -0,0 +1,74 @@ +// 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 +{ + public class Options_For_Issue_91 + { + [Value(0, Required = true)] + public string InputFileName { get; set; } + + [Option('o', "output")] + public string OutputFileName { get; set; } + + [Option('i', "include", Separator = ',')] + public IEnumerable Included { get; set; } + + [Option('e', "exclude", Separator = ',')] + public IEnumerable Excluded { get; set; } + } + + public class Options_For_Issue_454 + { + [Option('c', "channels", Required = true, Separator = ':', HelpText = "Channel names")] + public IEnumerable Channels { get; set; } + + [Value(0, Required = true, MetaName = "file_path", HelpText = "Path of archive to be processed")] + public string ArchivePath { get; set; } + } + + public class Options_For_Issue_510 + { + [Option('a', "aa", Required = false, Separator = ',')] + public IEnumerable A { get; set; } + + [Option('b', "bb", Required = false)] + public string B { get; set; } + + [Value(0, Required = true)] + public string C { get; set; } + } + + public enum FMode { C, D, S }; + + public class Options_For_Issue_617 + { + [Option("fm", Separator=',', Default = new[] { FMode.S })] + public IEnumerable Mode { get; set; } + + [Option('q')] + public bool q { get;set; } + + [Value(0)] + public IList Files { get; set; } + } + + public class Options_For_Issue_619 + { + [Option("verbose", Required = false, Default = false, HelpText = "Generate process tracing information")] + public bool Verbose { get; set; } + + [Option("outdir", Required = false, Default = ".", HelpText = "Directory to look for object file")] + public string OutDir { get; set; } + + [Option("modules", Required = true, Separator = ',', HelpText = "Directories to look for module file")] + public IEnumerable ModuleDirs { get; set; } + + [Option("ignore", Required = false, Separator = ' ', HelpText = "List of additional module name references to ignore")] + public IEnumerable Ignores { get; set; } + + [Value(0, Required = true, HelpText = "List of source files to process")] + public IEnumerable Srcs { get; set; } + } +} diff --git a/tests/CommandLine.Tests/Fakes/Options_With_Similar_Names.cs b/tests/CommandLine.Tests/Fakes/Options_With_Similar_Names.cs new file mode 100644 index 00000000..781d29cb --- /dev/null +++ b/tests/CommandLine.Tests/Fakes/Options_With_Similar_Names.cs @@ -0,0 +1,31 @@ +// 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 +{ + public class Options_With_Similar_Names + { + [Option("deploy", Separator = ',', HelpText= "Projects to deploy")] + public IEnumerable Deploys { get; set; } + + [Option("profile", Required = true, HelpText = "Profile to use when restoring and publishing")] + public string Profile { get; set; } + + [Option("configure-profile", Required = true, HelpText = "Profile to use for Configure")] + public string ConfigureProfile { get; set; } + } + + public class Options_With_Similar_Names_And_Separator + { + [Option('f', "flag", HelpText = "Flag")] + public bool Flag { get; set; } + + [Option('c', "categories", Required = false, Separator = ',', HelpText = "Categories")] + public IEnumerable Categories { get; set; } + + [Option('j', "jobId", Required = true, HelpText = "Texts.ExplainJob")] + public int JobId { get; set; } + } + +} diff --git a/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs b/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs new file mode 100644 index 00000000..cb65e42f --- /dev/null +++ b/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs @@ -0,0 +1,140 @@ +using System.Collections.Generic; +using System.Linq; +using System; +using Xunit; +using CommandLine.Text; +using CommandLine.Tests.Fakes; +using FluentAssertions; +using CommandLine.Core; +using System.Reflection; +using CSharpx; +using RailwaySharp.ErrorHandling; + +namespace CommandLine.Tests.Unit +{ + // Reference: PR #684 + public class SequenceParsingTests + { + // Issue #91 + [Theory] + [InlineData(false)] + [InlineData(true)] + public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + { + var args = "--exclude=a,b InputFile.txt".Split(); + var expected = new Options_For_Issue_91 { + Excluded = new[] { "a", "b" }, + Included = Enumerable.Empty(), + InputFileName = "InputFile.txt", + }; + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var result = sut.ParseArguments(args); + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + // Issue #396 + [Theory] + [InlineData(false)] + [InlineData(true)] + public static void Options_with_similar_names_are_not_ambiguous(bool useGetoptMode) + { + var args = new[] { "--configure-profile", "deploy", "--profile", "local" }; + var expected = new Options_With_Similar_Names { ConfigureProfile = "deploy", Profile = "local", Deploys = Enumerable.Empty() }; + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var result = sut.ParseArguments(args); + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + // Issue #420 + [Fact] + + public static void Values_with_same_name_as_sequence_option_do_not_cause_later_values_to_split_on_separators() + { + var args = new[] { "c", "x,y" }; + var tokensExpected = new[] { Token.Value("c"), Token.Value("x,y") }; + var typeInfo = typeof(Options_With_Similar_Names_And_Separator); + + var specProps = typeInfo.GetSpecifications(pi => SpecificationProperty.Create( + Specification.FromProperty(pi), pi, Maybe.Nothing())) + .Select(sp => sp.Specification) + .OfType(); + + var tokenizerResult = Tokenizer.ConfigureTokenizer(StringComparer.InvariantCulture, false, false)(args, specProps); + var tokens = tokenizerResult.SucceededWith(); + tokens.Should().BeEquivalentTo(tokensExpected); + } + + // Issue #454 + [Theory] + [InlineData(false)] + [InlineData(true)] + + public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + { + var args = "-c chanA:chanB file.hdf5".Split(); + var expected = new Options_For_Issue_454 { + Channels = new[] { "chanA", "chanB" }, + ArchivePath = "file.hdf5", + }; + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var result = sut.ParseArguments(args); + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + // Issue #510 + [Theory] + [InlineData(false)] + [InlineData(true)] + + public static void Enumerable_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + { + var args = new[] { "-a", "1,2", "c" }; + var expected = new Options_For_Issue_510 { A = new[] { "1", "2" }, C = "c" }; + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var result = sut.ParseArguments(args); + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + // Issue #617 + [Theory] + [InlineData(false)] + [InlineData(true)] + + public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + { + var args = "--fm D,C a.txt".Split(); + var expected = new Options_For_Issue_617 { + Mode = new[] { FMode.D, FMode.C }, + Files = new[] { "a.txt" }, + }; + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var result = sut.ParseArguments(args); + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + + // Issue #619 + [Theory] + [InlineData(false)] + [InlineData(true)] + + public static void Separator_just_before_values_does_not_try_to_parse_values(bool useGetoptMode) + { + var args = "--outdir ./x64/Debug --modules ../utilities/x64/Debug,../auxtool/x64/Debug m_xfunit.f03 m_xfunit_assertion.f03".Split(); + var expected = new Options_For_Issue_619 { + OutDir = "./x64/Debug", + ModuleDirs = new[] { "../utilities/x64/Debug", "../auxtool/x64/Debug" }, + Ignores = Enumerable.Empty(), + Srcs = new[] { "m_xfunit.f03", "m_xfunit_assertion.f03" }, + }; + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var result = sut.ParseArguments(args); + result.Should().BeOfType>(); + result.As>().Value.Should().BeEquivalentTo(expected); + } + } +} From b7102d89a90acd2d9356637ead5a0a64b58b131a Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Fri, 21 Aug 2020 08:10:39 +0700 Subject: [PATCH 10/15] Address review comments * Share common code between Tokenizer and GetoptTokenizer * Use enum for ParserMode as we might add more in the future --- src/CommandLine/Core/GetoptTokenizer.cs | 32 +------------ src/CommandLine/Parser.cs | 20 ++++++-- src/CommandLine/ParserSettings.cs | 45 +++++++++++++---- .../Unit/Core/GetoptTokenizerTests.cs | 6 +-- .../Unit/GetoptParserTests.cs | 8 ++-- .../Unit/SequenceParsingTests.cs | 48 +++++++++---------- 6 files changed, 83 insertions(+), 76 deletions(-) diff --git a/src/CommandLine/Core/GetoptTokenizer.cs b/src/CommandLine/Core/GetoptTokenizer.cs index b8c97fc2..1d97125f 100644 --- a/src/CommandLine/Core/GetoptTokenizer.cs +++ b/src/CommandLine/Core/GetoptTokenizer.cs @@ -88,36 +88,6 @@ public static Result, Error> Tokenize( return Result.Succeed, Error>(tokens.AsEnumerable(), errors.AsEnumerable()); } - public static Result, Error> ExplodeOptionList( - Result, Error> tokenizerResult, - Func> optionSequenceWithSeparatorLookup) - { - var tokens = tokenizerResult.SucceededWith().Memoize(); - - var exploded = new List(tokens is ICollection coll ? coll.Count : tokens.Count()); - var nothing = Maybe.Nothing(); // Re-use same Nothing instance for efficiency - var separator = nothing; - foreach (var token in tokens) { - if (token.IsName()) { - separator = optionSequenceWithSeparatorLookup(token.Text); - exploded.Add(token); - } else { - // Forced values are never considered option values, so they should not be split - if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) { - if (token.Text.Contains(sep)) { - exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator)); - } else { - exploded.Add(token); - } - } else { - exploded.Add(token); - } - separator = nothing; // Only first value after a separator can possibly be split - } - } - return Result.Succeed(exploded as IEnumerable, tokenizerResult.SuccessMessages()); - } - public static Func< IEnumerable, IEnumerable, @@ -131,7 +101,7 @@ public static Func< return (arguments, optionSpecs) => { var tokens = GetoptTokenizer.Tokenize(arguments, name => NameLookup.Contains(name, optionSpecs, nameComparer), ignoreUnknownArguments, enableDashDash, posixlyCorrect); - var explodedTokens = GetoptTokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer)); + var explodedTokens = Tokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer)); return explodedTokens; }; } diff --git a/src/CommandLine/Parser.cs b/src/CommandLine/Parser.cs index 4301aa52..44953c58 100644 --- a/src/CommandLine/Parser.cs +++ b/src/CommandLine/Parser.cs @@ -185,16 +185,28 @@ private static Result, Error> Tokenize( IEnumerable optionSpecs, ParserSettings settings) { - return settings.GetoptMode - ? GetoptTokenizer.ConfigureTokenizer( + switch (settings.ParserMode) + { + case ParserMode.Legacy: + return Tokenizer.ConfigureTokenizer( + settings.NameComparer, + settings.IgnoreUnknownArguments, + settings.EnableDashDash)(arguments, optionSpecs); + + case ParserMode.Getopt: + return GetoptTokenizer.ConfigureTokenizer( settings.NameComparer, settings.IgnoreUnknownArguments, settings.EnableDashDash, - settings.PosixlyCorrect)(arguments, optionSpecs) - : Tokenizer.ConfigureTokenizer( + settings.PosixlyCorrect)(arguments, optionSpecs); + + // No need to test ParserMode.Default, as it should always be one of the above modes + default: + return Tokenizer.ConfigureTokenizer( settings.NameComparer, settings.IgnoreUnknownArguments, settings.EnableDashDash)(arguments, optionSpecs); + } } private static ParserResult MakeParserResult(ParserResult parserResult, ParserSettings settings) diff --git a/src/CommandLine/ParserSettings.cs b/src/CommandLine/ParserSettings.cs index 5ed73f30..c1b1ca77 100644 --- a/src/CommandLine/ParserSettings.cs +++ b/src/CommandLine/ParserSettings.cs @@ -9,6 +9,14 @@ namespace CommandLine { + public enum ParserMode + { + Legacy, + Getopt, + + Default = Legacy + } + /// /// Provides settings for . Once consumed cannot be reused. /// @@ -27,7 +35,7 @@ public class ParserSettings : IDisposable private Maybe enableDashDash; private int maximumDisplayWidth; private Maybe allowMultiInstance; - private bool getoptMode; + private ParserMode parserMode; private Maybe posixlyCorrect; /// @@ -41,7 +49,7 @@ public ParserSettings() autoVersion = true; parsingCulture = CultureInfo.InvariantCulture; maximumDisplayWidth = GetWindowWidth(); - getoptMode = false; + parserMode = ParserMode.Default; enableDashDash = Maybe.Nothing(); allowMultiInstance = Maybe.Nothing(); posixlyCorrect = Maybe.Nothing(); @@ -166,11 +174,11 @@ public bool AutoVersion /// /// Gets or sets a value indicating whether enable double dash '--' syntax, /// that forces parsing of all subsequent tokens as values. - /// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false. + /// Normally defaults to false. If ParserMode = ParserMode.Getopt, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false. /// public bool EnableDashDash { - get => enableDashDash.MatchJust(out bool value) ? value : getoptMode; + get => enableDashDash.MatchJust(out bool value) ? value : (parserMode == ParserMode.Getopt); set => PopsicleSetter.Set(Consumed, ref enableDashDash, Maybe.Just(value)); } @@ -185,21 +193,38 @@ public int MaximumDisplayWidth /// /// Gets or sets a value indicating whether options are allowed to be specified multiple times. - /// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying AllowMultiInstance = false. + /// If ParserMode = ParserMode.Getopt, this defaults to true, but can be turned off by explicitly specifying AllowMultiInstance = false. /// public bool AllowMultiInstance { - get => allowMultiInstance.MatchJust(out bool value) ? value : getoptMode; + get => allowMultiInstance.MatchJust(out bool value) ? value : (parserMode == ParserMode.Getopt); set => PopsicleSetter.Set(Consumed, ref allowMultiInstance, Maybe.Just(value)); } /// - /// Whether strict getopt-like processing is applied to option values; if true, AllowMultiInstance and EnableDashDash will default to true as well. + /// Set this to change how the parser processes command-line arguments. Currently valid values are: + /// + /// + /// Legacy + /// Uses - for short options and -- for long options. + /// Values of long options can only start with a - character if the = syntax is used. + /// E.g., "--string-option -x" will consider "-x" to be an option, not the value of "--string-option", + /// but "--string-option=-x" will consider "-x" to be the value of "--string-option". + /// + /// + /// Getopt + /// Strict getopt-like processing is applied to option values. + /// Mostly like legacy mode, except that option values with = and with space are more consistent. + /// After an option that takes a value, and whose value was not specified with "=", the next argument will be considered the value even if it starts with "-". + /// E.g., both "--string-option=-x" and "--string-option -x" will consider "-x" to be the value of "--string-option". + /// If this mode is chosen, AllowMultiInstance and EnableDashDash will default to true as well, though they can be explicitly turned off if desired. + /// + /// /// - public bool GetoptMode + public ParserMode ParserMode { - get => getoptMode; - set => PopsicleSetter.Set(Consumed, ref getoptMode, value); + get => parserMode; + set => PopsicleSetter.Set(Consumed, ref parserMode, value); } /// diff --git a/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs b/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs index 337a9a3f..d4669a66 100644 --- a/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs @@ -24,7 +24,7 @@ public void Explode_scalar_with_separator_in_odd_args_input_returns_sequence() // Exercize system var result = - GetoptTokenizer.ExplodeOptionList( + Tokenizer.ExplodeOptionList( Result.Succeed( Enumerable.Empty().Concat(new[] { Token.Name("i"), Token.Value("10"), Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }), @@ -47,7 +47,7 @@ public void Explode_scalar_with_separator_in_even_args_input_returns_sequence() // Exercize system var result = - GetoptTokenizer.ExplodeOptionList( + Tokenizer.ExplodeOptionList( Result.Succeed( Enumerable.Empty().Concat(new[] { Token.Name("x"), Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }), @@ -90,7 +90,7 @@ public void Should_return_error_if_option_format_with_equals_is_not_correct() var errors = result.SuccessMessages(); Assert.NotNull(errors); - Assert.Equal(1, errors.Count()); + Assert.NotEmpty(errors); Assert.Equal(ErrorType.BadFormatTokenError, errors.First().Tag); var tokens = result.SucceededWith(); diff --git a/tests/CommandLine.Tests/Unit/GetoptParserTests.cs b/tests/CommandLine.Tests/Unit/GetoptParserTests.cs index cd2a1577..94bc94df 100644 --- a/tests/CommandLine.Tests/Unit/GetoptParserTests.cs +++ b/tests/CommandLine.Tests/Unit/GetoptParserTests.cs @@ -155,7 +155,7 @@ public void Getopt_parser_without_posixly_correct_allows_mixed_options_and_nonop { // Arrange var sut = new Parser(config => { - config.GetoptMode = true; + config.ParserMode = ParserMode.Getopt; config.PosixlyCorrect = false; }); @@ -215,7 +215,7 @@ public void Getopt_parser_with_posixly_correct_stops_parsing_at_first_nonoption( { // Arrange var sut = new Parser(config => { - config.GetoptMode = true; + config.ParserMode = ParserMode.Getopt; config.PosixlyCorrect = true; config.EnableDashDash = true; }); @@ -233,7 +233,7 @@ public void Getopt_mode_defaults_to_EnableDashDash_being_true() { // Arrange var sut = new Parser(config => { - config.GetoptMode = true; + config.ParserMode = ParserMode.Getopt; config.PosixlyCorrect = false; }); var args = new string [] {"--stringvalue", "foo", "256", "--", "-x", "-sbar" }; @@ -259,7 +259,7 @@ public void Getopt_mode_can_have_EnableDashDash_expicitly_disabled() { // Arrange var sut = new Parser(config => { - config.GetoptMode = true; + config.ParserMode = ParserMode.Getopt; config.PosixlyCorrect = false; config.EnableDashDash = false; }); diff --git a/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs b/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs index cb65e42f..a498af6f 100644 --- a/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs +++ b/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs @@ -17,9 +17,9 @@ public class SequenceParsingTests { // Issue #91 [Theory] - [InlineData(false)] - [InlineData(true)] - public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + [InlineData(ParserMode.Legacy)] + [InlineData(ParserMode.Getopt)] + public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) { var args = "--exclude=a,b InputFile.txt".Split(); var expected = new Options_For_Issue_91 { @@ -27,7 +27,7 @@ public static void Enumerable_with_separator_before_values_does_not_try_to_parse Included = Enumerable.Empty(), InputFileName = "InputFile.txt", }; - var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -35,13 +35,13 @@ public static void Enumerable_with_separator_before_values_does_not_try_to_parse // Issue #396 [Theory] - [InlineData(false)] - [InlineData(true)] - public static void Options_with_similar_names_are_not_ambiguous(bool useGetoptMode) + [InlineData(ParserMode.Legacy)] + [InlineData(ParserMode.Getopt)] + public static void Options_with_similar_names_are_not_ambiguous(ParserMode parserMode) { var args = new[] { "--configure-profile", "deploy", "--profile", "local" }; var expected = new Options_With_Similar_Names { ConfigureProfile = "deploy", Profile = "local", Deploys = Enumerable.Empty() }; - var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -68,17 +68,17 @@ public static void Values_with_same_name_as_sequence_option_do_not_cause_later_v // Issue #454 [Theory] - [InlineData(false)] - [InlineData(true)] + [InlineData(ParserMode.Legacy)] + [InlineData(ParserMode.Getopt)] - public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) { var args = "-c chanA:chanB file.hdf5".Split(); var expected = new Options_For_Issue_454 { Channels = new[] { "chanA", "chanB" }, ArchivePath = "file.hdf5", }; - var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -86,14 +86,14 @@ public static void Enumerable_with_colon_separator_before_values_does_not_try_to // Issue #510 [Theory] - [InlineData(false)] - [InlineData(true)] + [InlineData(ParserMode.Legacy)] + [InlineData(ParserMode.Getopt)] - public static void Enumerable_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + public static void Enumerable_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) { var args = new[] { "-a", "1,2", "c" }; var expected = new Options_For_Issue_510 { A = new[] { "1", "2" }, C = "c" }; - var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -101,17 +101,17 @@ public static void Enumerable_before_values_does_not_try_to_parse_too_much(bool // Issue #617 [Theory] - [InlineData(false)] - [InlineData(true)] + [InlineData(ParserMode.Legacy)] + [InlineData(ParserMode.Getopt)] - public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) + public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) { var args = "--fm D,C a.txt".Split(); var expected = new Options_For_Issue_617 { Mode = new[] { FMode.D, FMode.C }, Files = new[] { "a.txt" }, }; - var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -119,10 +119,10 @@ public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_ // Issue #619 [Theory] - [InlineData(false)] - [InlineData(true)] + [InlineData(ParserMode.Legacy)] + [InlineData(ParserMode.Getopt)] - public static void Separator_just_before_values_does_not_try_to_parse_values(bool useGetoptMode) + public static void Separator_just_before_values_does_not_try_to_parse_values(ParserMode parserMode) { var args = "--outdir ./x64/Debug --modules ../utilities/x64/Debug,../auxtool/x64/Debug m_xfunit.f03 m_xfunit_assertion.f03".Split(); var expected = new Options_For_Issue_619 { @@ -131,7 +131,7 @@ public static void Separator_just_before_values_does_not_try_to_parse_values(boo Ignores = Enumerable.Empty(), Srcs = new[] { "m_xfunit.f03", "m_xfunit_assertion.f03" }, }; - var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); + var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); From 34ab560ba5af72e1124bd2e75840f69f9548a38a Mon Sep 17 00:00:00 2001 From: moh-hassan Date: Tue, 25 Aug 2020 17:20:21 +0300 Subject: [PATCH 11/15] Revert "Address review comments" This reverts commit b7102d89a90acd2d9356637ead5a0a64b58b131a. --- src/CommandLine/Core/GetoptTokenizer.cs | 32 ++++++++++++- src/CommandLine/Parser.cs | 20 ++------ src/CommandLine/ParserSettings.cs | 45 ++++------------- .../Unit/Core/GetoptTokenizerTests.cs | 6 +-- .../Unit/GetoptParserTests.cs | 8 ++-- .../Unit/SequenceParsingTests.cs | 48 +++++++++---------- 6 files changed, 76 insertions(+), 83 deletions(-) diff --git a/src/CommandLine/Core/GetoptTokenizer.cs b/src/CommandLine/Core/GetoptTokenizer.cs index 1d97125f..b8c97fc2 100644 --- a/src/CommandLine/Core/GetoptTokenizer.cs +++ b/src/CommandLine/Core/GetoptTokenizer.cs @@ -88,6 +88,36 @@ public static Result, Error> Tokenize( return Result.Succeed, Error>(tokens.AsEnumerable(), errors.AsEnumerable()); } + public static Result, Error> ExplodeOptionList( + Result, Error> tokenizerResult, + Func> optionSequenceWithSeparatorLookup) + { + var tokens = tokenizerResult.SucceededWith().Memoize(); + + var exploded = new List(tokens is ICollection coll ? coll.Count : tokens.Count()); + var nothing = Maybe.Nothing(); // Re-use same Nothing instance for efficiency + var separator = nothing; + foreach (var token in tokens) { + if (token.IsName()) { + separator = optionSequenceWithSeparatorLookup(token.Text); + exploded.Add(token); + } else { + // Forced values are never considered option values, so they should not be split + if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) { + if (token.Text.Contains(sep)) { + exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator)); + } else { + exploded.Add(token); + } + } else { + exploded.Add(token); + } + separator = nothing; // Only first value after a separator can possibly be split + } + } + return Result.Succeed(exploded as IEnumerable, tokenizerResult.SuccessMessages()); + } + public static Func< IEnumerable, IEnumerable, @@ -101,7 +131,7 @@ public static Func< return (arguments, optionSpecs) => { var tokens = GetoptTokenizer.Tokenize(arguments, name => NameLookup.Contains(name, optionSpecs, nameComparer), ignoreUnknownArguments, enableDashDash, posixlyCorrect); - var explodedTokens = Tokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer)); + var explodedTokens = GetoptTokenizer.ExplodeOptionList(tokens, name => NameLookup.HavingSeparator(name, optionSpecs, nameComparer)); return explodedTokens; }; } diff --git a/src/CommandLine/Parser.cs b/src/CommandLine/Parser.cs index 44953c58..4301aa52 100644 --- a/src/CommandLine/Parser.cs +++ b/src/CommandLine/Parser.cs @@ -185,28 +185,16 @@ private static Result, Error> Tokenize( IEnumerable optionSpecs, ParserSettings settings) { - switch (settings.ParserMode) - { - case ParserMode.Legacy: - return Tokenizer.ConfigureTokenizer( - settings.NameComparer, - settings.IgnoreUnknownArguments, - settings.EnableDashDash)(arguments, optionSpecs); - - case ParserMode.Getopt: - return GetoptTokenizer.ConfigureTokenizer( + return settings.GetoptMode + ? GetoptTokenizer.ConfigureTokenizer( settings.NameComparer, settings.IgnoreUnknownArguments, settings.EnableDashDash, - settings.PosixlyCorrect)(arguments, optionSpecs); - - // No need to test ParserMode.Default, as it should always be one of the above modes - default: - return Tokenizer.ConfigureTokenizer( + settings.PosixlyCorrect)(arguments, optionSpecs) + : Tokenizer.ConfigureTokenizer( settings.NameComparer, settings.IgnoreUnknownArguments, settings.EnableDashDash)(arguments, optionSpecs); - } } private static ParserResult MakeParserResult(ParserResult parserResult, ParserSettings settings) diff --git a/src/CommandLine/ParserSettings.cs b/src/CommandLine/ParserSettings.cs index c1b1ca77..5ed73f30 100644 --- a/src/CommandLine/ParserSettings.cs +++ b/src/CommandLine/ParserSettings.cs @@ -9,14 +9,6 @@ namespace CommandLine { - public enum ParserMode - { - Legacy, - Getopt, - - Default = Legacy - } - /// /// Provides settings for . Once consumed cannot be reused. /// @@ -35,7 +27,7 @@ public class ParserSettings : IDisposable private Maybe enableDashDash; private int maximumDisplayWidth; private Maybe allowMultiInstance; - private ParserMode parserMode; + private bool getoptMode; private Maybe posixlyCorrect; /// @@ -49,7 +41,7 @@ public ParserSettings() autoVersion = true; parsingCulture = CultureInfo.InvariantCulture; maximumDisplayWidth = GetWindowWidth(); - parserMode = ParserMode.Default; + getoptMode = false; enableDashDash = Maybe.Nothing(); allowMultiInstance = Maybe.Nothing(); posixlyCorrect = Maybe.Nothing(); @@ -174,11 +166,11 @@ public bool AutoVersion /// /// Gets or sets a value indicating whether enable double dash '--' syntax, /// that forces parsing of all subsequent tokens as values. - /// Normally defaults to false. If ParserMode = ParserMode.Getopt, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false. + /// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying EnableDashDash = false. /// public bool EnableDashDash { - get => enableDashDash.MatchJust(out bool value) ? value : (parserMode == ParserMode.Getopt); + get => enableDashDash.MatchJust(out bool value) ? value : getoptMode; set => PopsicleSetter.Set(Consumed, ref enableDashDash, Maybe.Just(value)); } @@ -193,38 +185,21 @@ public int MaximumDisplayWidth /// /// Gets or sets a value indicating whether options are allowed to be specified multiple times. - /// If ParserMode = ParserMode.Getopt, this defaults to true, but can be turned off by explicitly specifying AllowMultiInstance = false. + /// If GetoptMode is true, this defaults to true, but can be turned off by explicitly specifying AllowMultiInstance = false. /// public bool AllowMultiInstance { - get => allowMultiInstance.MatchJust(out bool value) ? value : (parserMode == ParserMode.Getopt); + get => allowMultiInstance.MatchJust(out bool value) ? value : getoptMode; set => PopsicleSetter.Set(Consumed, ref allowMultiInstance, Maybe.Just(value)); } /// - /// Set this to change how the parser processes command-line arguments. Currently valid values are: - /// - /// - /// Legacy - /// Uses - for short options and -- for long options. - /// Values of long options can only start with a - character if the = syntax is used. - /// E.g., "--string-option -x" will consider "-x" to be an option, not the value of "--string-option", - /// but "--string-option=-x" will consider "-x" to be the value of "--string-option". - /// - /// - /// Getopt - /// Strict getopt-like processing is applied to option values. - /// Mostly like legacy mode, except that option values with = and with space are more consistent. - /// After an option that takes a value, and whose value was not specified with "=", the next argument will be considered the value even if it starts with "-". - /// E.g., both "--string-option=-x" and "--string-option -x" will consider "-x" to be the value of "--string-option". - /// If this mode is chosen, AllowMultiInstance and EnableDashDash will default to true as well, though they can be explicitly turned off if desired. - /// - /// + /// Whether strict getopt-like processing is applied to option values; if true, AllowMultiInstance and EnableDashDash will default to true as well. /// - public ParserMode ParserMode + public bool GetoptMode { - get => parserMode; - set => PopsicleSetter.Set(Consumed, ref parserMode, value); + get => getoptMode; + set => PopsicleSetter.Set(Consumed, ref getoptMode, value); } /// diff --git a/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs b/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs index d4669a66..337a9a3f 100644 --- a/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/GetoptTokenizerTests.cs @@ -24,7 +24,7 @@ public void Explode_scalar_with_separator_in_odd_args_input_returns_sequence() // Exercize system var result = - Tokenizer.ExplodeOptionList( + GetoptTokenizer.ExplodeOptionList( Result.Succeed( Enumerable.Empty().Concat(new[] { Token.Name("i"), Token.Value("10"), Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }), @@ -47,7 +47,7 @@ public void Explode_scalar_with_separator_in_even_args_input_returns_sequence() // Exercize system var result = - Tokenizer.ExplodeOptionList( + GetoptTokenizer.ExplodeOptionList( Result.Succeed( Enumerable.Empty().Concat(new[] { Token.Name("x"), Token.Name("string-seq"), Token.Value("aaa,bb,cccc"), Token.Name("switch") }), @@ -90,7 +90,7 @@ public void Should_return_error_if_option_format_with_equals_is_not_correct() var errors = result.SuccessMessages(); Assert.NotNull(errors); - Assert.NotEmpty(errors); + Assert.Equal(1, errors.Count()); Assert.Equal(ErrorType.BadFormatTokenError, errors.First().Tag); var tokens = result.SucceededWith(); diff --git a/tests/CommandLine.Tests/Unit/GetoptParserTests.cs b/tests/CommandLine.Tests/Unit/GetoptParserTests.cs index 94bc94df..cd2a1577 100644 --- a/tests/CommandLine.Tests/Unit/GetoptParserTests.cs +++ b/tests/CommandLine.Tests/Unit/GetoptParserTests.cs @@ -155,7 +155,7 @@ public void Getopt_parser_without_posixly_correct_allows_mixed_options_and_nonop { // Arrange var sut = new Parser(config => { - config.ParserMode = ParserMode.Getopt; + config.GetoptMode = true; config.PosixlyCorrect = false; }); @@ -215,7 +215,7 @@ public void Getopt_parser_with_posixly_correct_stops_parsing_at_first_nonoption( { // Arrange var sut = new Parser(config => { - config.ParserMode = ParserMode.Getopt; + config.GetoptMode = true; config.PosixlyCorrect = true; config.EnableDashDash = true; }); @@ -233,7 +233,7 @@ public void Getopt_mode_defaults_to_EnableDashDash_being_true() { // Arrange var sut = new Parser(config => { - config.ParserMode = ParserMode.Getopt; + config.GetoptMode = true; config.PosixlyCorrect = false; }); var args = new string [] {"--stringvalue", "foo", "256", "--", "-x", "-sbar" }; @@ -259,7 +259,7 @@ public void Getopt_mode_can_have_EnableDashDash_expicitly_disabled() { // Arrange var sut = new Parser(config => { - config.ParserMode = ParserMode.Getopt; + config.GetoptMode = true; config.PosixlyCorrect = false; config.EnableDashDash = false; }); diff --git a/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs b/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs index a498af6f..cb65e42f 100644 --- a/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs +++ b/tests/CommandLine.Tests/Unit/SequenceParsingTests.cs @@ -17,9 +17,9 @@ public class SequenceParsingTests { // Issue #91 [Theory] - [InlineData(ParserMode.Legacy)] - [InlineData(ParserMode.Getopt)] - public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) + [InlineData(false)] + [InlineData(true)] + public static void Enumerable_with_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) { var args = "--exclude=a,b InputFile.txt".Split(); var expected = new Options_For_Issue_91 { @@ -27,7 +27,7 @@ public static void Enumerable_with_separator_before_values_does_not_try_to_parse Included = Enumerable.Empty(), InputFileName = "InputFile.txt", }; - var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -35,13 +35,13 @@ public static void Enumerable_with_separator_before_values_does_not_try_to_parse // Issue #396 [Theory] - [InlineData(ParserMode.Legacy)] - [InlineData(ParserMode.Getopt)] - public static void Options_with_similar_names_are_not_ambiguous(ParserMode parserMode) + [InlineData(false)] + [InlineData(true)] + public static void Options_with_similar_names_are_not_ambiguous(bool useGetoptMode) { var args = new[] { "--configure-profile", "deploy", "--profile", "local" }; var expected = new Options_With_Similar_Names { ConfigureProfile = "deploy", Profile = "local", Deploys = Enumerable.Empty() }; - var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -68,17 +68,17 @@ public static void Values_with_same_name_as_sequence_option_do_not_cause_later_v // Issue #454 [Theory] - [InlineData(ParserMode.Legacy)] - [InlineData(ParserMode.Getopt)] + [InlineData(false)] + [InlineData(true)] - public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) + public static void Enumerable_with_colon_separator_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) { var args = "-c chanA:chanB file.hdf5".Split(); var expected = new Options_For_Issue_454 { Channels = new[] { "chanA", "chanB" }, ArchivePath = "file.hdf5", }; - var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -86,14 +86,14 @@ public static void Enumerable_with_colon_separator_before_values_does_not_try_to // Issue #510 [Theory] - [InlineData(ParserMode.Legacy)] - [InlineData(ParserMode.Getopt)] + [InlineData(false)] + [InlineData(true)] - public static void Enumerable_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) + public static void Enumerable_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) { var args = new[] { "-a", "1,2", "c" }; var expected = new Options_For_Issue_510 { A = new[] { "1", "2" }, C = "c" }; - var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -101,17 +101,17 @@ public static void Enumerable_before_values_does_not_try_to_parse_too_much(Parse // Issue #617 [Theory] - [InlineData(ParserMode.Legacy)] - [InlineData(ParserMode.Getopt)] + [InlineData(false)] + [InlineData(true)] - public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(ParserMode parserMode) + public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_much(bool useGetoptMode) { var args = "--fm D,C a.txt".Split(); var expected = new Options_For_Issue_617 { Mode = new[] { FMode.D, FMode.C }, Files = new[] { "a.txt" }, }; - var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); @@ -119,10 +119,10 @@ public static void Enumerable_with_enum_before_values_does_not_try_to_parse_too_ // Issue #619 [Theory] - [InlineData(ParserMode.Legacy)] - [InlineData(ParserMode.Getopt)] + [InlineData(false)] + [InlineData(true)] - public static void Separator_just_before_values_does_not_try_to_parse_values(ParserMode parserMode) + public static void Separator_just_before_values_does_not_try_to_parse_values(bool useGetoptMode) { var args = "--outdir ./x64/Debug --modules ../utilities/x64/Debug,../auxtool/x64/Debug m_xfunit.f03 m_xfunit_assertion.f03".Split(); var expected = new Options_For_Issue_619 { @@ -131,7 +131,7 @@ public static void Separator_just_before_values_does_not_try_to_parse_values(Par Ignores = Enumerable.Empty(), Srcs = new[] { "m_xfunit.f03", "m_xfunit_assertion.f03" }, }; - var sut = new Parser(parserSettings => { parserSettings.ParserMode = parserMode; }); + var sut = new Parser(parserSettings => { parserSettings.GetoptMode = useGetoptMode; }); var result = sut.ParseArguments(args); result.Should().BeOfType>(); result.As>().Value.Should().BeEquivalentTo(expected); From 31895f1693234476297ad79841456caea751d6fc Mon Sep 17 00:00:00 2001 From: NN Date: Sat, 26 Jun 2021 13:19:42 +0300 Subject: [PATCH 12/15] Remove System.Linq.Expressions dependency. --- src/CommandLine/Core/ReflectionExtensions.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/CommandLine/Core/ReflectionExtensions.cs b/src/CommandLine/Core/ReflectionExtensions.cs index a4529871..622e1e6e 100644 --- a/src/CommandLine/Core/ReflectionExtensions.cs +++ b/src/CommandLine/Core/ReflectionExtensions.cs @@ -4,7 +4,6 @@ using System.Collections; using System.Collections.Generic; using System.Linq; -using System.Linq.Expressions; using System.Reflection; using CommandLine.Infrastructure; using CommandLine.Text; @@ -121,11 +120,7 @@ public static object CreateEmptyArray(this Type type) public static object GetDefaultValue(this Type type) { - var e = Expression.Lambda>( - Expression.Convert( - Expression.Default(type), - typeof(object))); - return e.Compile()(); + return type.IsValueType ? Activator.CreateInstance(type) : null; } public static bool IsMutable(this Type type) From efc1e740529af8c99840474da125e00c25ca6bd0 Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Thu, 30 Dec 2021 10:06:53 +0100 Subject: [PATCH 13/15] Add Rider cache folder to gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 55f900a1..4969d4de 100644 --- a/.gitignore +++ b/.gitignore @@ -37,5 +37,7 @@ artifacts/* *.DotSettings.user # Visual Studio 2015 cache/options directory .vs/ +# Rider +.idea/ [R|r]elease/** From ff825e52d032c72014ff3d0dd8c5e938ab34e1be Mon Sep 17 00:00:00 2001 From: Bastian Schmidt Date: Thu, 30 Dec 2021 10:24:40 +0100 Subject: [PATCH 14/15] Fixes #776 --- src/CommandLine/Core/Tokenizer.cs | 29 ++++++++------- .../Unit/Core/TokenizerTests.cs | 36 ++++++++++++++++--- tests/CommandLine.Tests/Unit/Issue776Tests.cs | 36 +++++++++++++++++++ 3 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 tests/CommandLine.Tests/Unit/Issue776Tests.cs diff --git a/src/CommandLine/Core/Tokenizer.cs b/src/CommandLine/Core/Tokenizer.cs index b71ec7e2..fe94fc61 100644 --- a/src/CommandLine/Core/Tokenizer.cs +++ b/src/CommandLine/Core/Tokenizer.cs @@ -86,29 +86,32 @@ public static Result, Error> ExplodeOptionList( return Result.Succeed(exploded as IEnumerable, tokenizerResult.SuccessMessages()); } + /// + /// Normalizes the given . + /// + /// The given minus all names, and their value if one was present, that are not found using . public static IEnumerable Normalize( IEnumerable tokens, Func nameLookup) { - var indexes = + var toExclude = 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(); + if (t.IsName() == false + || nameLookup(t.Text)) + { + return Maybe.Nothing>(); + } + + var next = tokens.ElementAtOrDefault(i + 1).ToMaybe(); + var removeValue = next.MatchJust(out var nextValue) + && next.MapValueOrDefault(p => p.IsValue() && ((Value)p).ExplicitlyAssigned, false); + return Maybe.Just(new Tuple(t, removeValue ? nextValue : null)); }).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); + var normalized = tokens.Where(t => toExclude.Any(e => ReferenceEquals(e.Item1, t) || ReferenceEquals(e.Item2, t)) == false); return normalized; } diff --git a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs index a489b741..ea7268be 100644 --- a/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs +++ b/tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs @@ -62,12 +62,12 @@ public void Explode_scalar_with_separator_in_even_args_input_returns_sequence() } [Fact] - public void Normalize_should_remove_all_value_with_explicit_assignment_of_existing_name() + public void Normalize_should_remove_all_names_and_values_with_explicit_assignment_of_non_existing_names() { // Fixture setup var expectedTokens = new[] { - Token.Name("x"), Token.Name("string-seq"), Token.Value("aaa"), Token.Value("bb"), - Token.Name("unknown"), Token.Name("switch") }; + Token.Name("x"), Token.Name("string-seq"), Token.Value("value0", true), Token.Value("bb"), + Token.Name("switch") }; Func nameLookup = name => name.Equals("x") || name.Equals("string-seq") || name.Equals("switch"); @@ -78,7 +78,7 @@ public void Normalize_should_remove_all_value_with_explicit_assignment_of_existi Enumerable.Empty() .Concat( new[] { - Token.Name("x"), Token.Name("string-seq"), Token.Value("aaa"), Token.Value("bb"), + Token.Name("x"), Token.Name("string-seq"), Token.Value("value0", true), Token.Value("bb"), Token.Name("unknown"), Token.Value("value0", true), Token.Name("switch") }) //,Enumerable.Empty()), , nameLookup); @@ -89,6 +89,34 @@ public void Normalize_should_remove_all_value_with_explicit_assignment_of_existi // Teardown } + [Fact] + public void Normalize_should_remove_all_names_of_non_existing_names() + { + // Fixture setup + var expectedTokens = new[] { + Token.Name("x"), Token.Name("string-seq"), Token.Value("value0", true), Token.Value("bb"), + 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("value0", true), Token.Value("bb"), + Token.Name("unknown"), Token.Name("switch") }) + //,Enumerable.Empty()), + , nameLookup); + + // Verify outcome + result.Should().BeEquivalentTo(expectedTokens); + + // Teardown + } + [Fact] public void Should_properly_parse_option_with_equals_in_value() { diff --git a/tests/CommandLine.Tests/Unit/Issue776Tests.cs b/tests/CommandLine.Tests/Unit/Issue776Tests.cs new file mode 100644 index 00000000..2e247f9b --- /dev/null +++ b/tests/CommandLine.Tests/Unit/Issue776Tests.cs @@ -0,0 +1,36 @@ +using FluentAssertions; +using Xunit; + +// Issue #776 and #797 +// When IgnoreUnknownArguments is used and there are unknown arguments with explicitly assigned values, other arguments with explicit assigned values should not be influenced. +// The bug only occured when the value was the same for a known and an unknown argument. + +namespace CommandLine.Tests.Unit +{ + public class Issue776Tests + { + [Theory] + [InlineData("3")] + [InlineData("4")] + public void IgnoreUnknownArguments_should_work_for_all_values(string dummyValue) + { + var arguments = new[] { "--cols=4", $"--dummy={dummyValue}" }; + var result = new Parser(with => { with.IgnoreUnknownArguments = true; }) + .ParseArguments(arguments); + + Assert.Empty(result.Errors); + Assert.Equal(ParserResultType.Parsed, result.Tag); + + result.WithParsed(options => + { + options.Cols.Should().Be(4); + }); + } + + private class Options + { + [Option("cols", Required = false)] + public int Cols { get; set; } + } + } +} From 87316e978b08fe6ab8ce48f1838a57eac093bad6 Mon Sep 17 00:00:00 2001 From: Marcin Otorowski Date: Mon, 7 Feb 2022 22:42:11 +0100 Subject: [PATCH 15/15] Add missing interpolation character This fixes unhelpful exception message, if localizable text cannot be found in the associated resource dictionary. --- src/CommandLine/Infrastructure/LocalizableAttributeProperty.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CommandLine/Infrastructure/LocalizableAttributeProperty.cs b/src/CommandLine/Infrastructure/LocalizableAttributeProperty.cs index 9edd621b..33db2640 100644 --- a/src/CommandLine/Infrastructure/LocalizableAttributeProperty.cs +++ b/src/CommandLine/Infrastructure/LocalizableAttributeProperty.cs @@ -48,7 +48,7 @@ private string GetLocalizedValue() throw new ArgumentException($"Invalid resource type '{_type.FullName}'! {_type.Name} is not visible for the parser! Change resources 'Access Modifier' to 'Public'", _propertyName); PropertyInfo propertyInfo = _type.GetProperty(_value, BindingFlags.Public | BindingFlags.GetProperty | BindingFlags.Static); if (propertyInfo == null || !propertyInfo.CanRead || propertyInfo.PropertyType != typeof(string)) - throw new ArgumentException("Invalid resource property name! Localized value: {_value}", _propertyName); + throw new ArgumentException($"Invalid resource property name! Localized value: {_value}", _propertyName); _localizationPropertyInfo = propertyInfo; } return (string)_localizationPropertyInfo.GetValue(null, null);