Skip to content

Sequences that hit max should stop grabbing values #682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 19, 2020
Merged

Sequences that hit max should stop grabbing values #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions src/CommandLine/Core/TokenPartitioner.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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