From 889ac3bf1fce7bfd014810e1aaa160f908209752 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Wed, 19 Aug 2020 21:37:37 +0700 Subject: [PATCH] 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); } }