Skip to content

Commit

Permalink
Sequences that hit max stop grabbing values (#682)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@rmunn
rmunn authored Aug 19, 2020
1 parent 074d050 commit 889ac3b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 26 deletions.
24 changes: 12 additions & 12 deletions src/CommandLine/Core/TokenPartitioner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ public static Tuple<IEnumerable<Token>, IEnumerable<Token>, IEnumerable<Token>,
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;
Expand All @@ -110,24 +109,25 @@ public static Tuple<IEnumerable<Token>, IEnumerable<Token>, IEnumerable<Token>,

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]++;
}
}
else
{
Console.WriteLine("***BUG!!!***");
throw new NullReferenceException($"Sequence for name {nameToken} doesn't exist, and it should");
// sequences[nameToken] = new List<Token>(new[] { token });
// Should never get here, but just in case:
sequences[nameToken] = new List<Token>(new[] { token });
count[nameToken] = 0;
max[nameToken] = Maybe.Nothing<int>();
}
break;
}
Expand Down
27 changes: 14 additions & 13 deletions tests/CommandLine.Tests/Unit/Core/InstanceBuilderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")) };
Expand All @@ -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) };
Expand All @@ -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<Options_With_Sequence_And_Only_Max_Constraint>(
new[] { "--string-seq=one", "two", "three", "this-is-too-much" });

// Verify outcome
((NotParsed<Options_With_Sequence_And_Only_Max_Constraint>)result).Errors.Should().BeEquivalentTo(expectedResult);
((Parsed<Options_With_Sequence_And_Only_Max_Constraint>)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) };
Expand Down Expand Up @@ -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[]
Expand Down Expand Up @@ -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")) };
Expand All @@ -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", "")) };
Expand All @@ -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") };
Expand All @@ -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") };
Expand All @@ -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") };
Expand Down Expand Up @@ -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) };
Expand Down
12 changes: 11 additions & 1 deletion tests/CommandLine.Tests/Unit/Core/SequenceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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[]
{
Expand All @@ -159,6 +167,8 @@ public void Partition_sequence_multi_instance_with_max()
: Maybe.Nothing<TypeDescriptor>());
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);
}
}
Expand Down

0 comments on commit 889ac3b

Please sign in to comment.