Skip to content

Fix OnlyTake token redistribution order #2817

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

Open
marko1olo wants to merge 1 commit into
base: main
Choose a base branch
from
+80 −15
Open

Fix OnlyTake token redistribution order #2817

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
64 changes: 63 additions & 1 deletion src/System.CommandLine.Tests/CustomParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,68 @@ public void When_custom_parser_passes_on_tokens_the_argument_result_tokens_refle
options => options.WithStrictOrdering());
}

[Fact] // https://github.com/dotnet/command-line-api/issues/2734
public void OnlyTake_preserves_token_order_when_multiple_arguments_pass_on_tokens()
{
static string[] TakeTwo(ArgumentResult result)
{
result.OnlyTake(2);

return result.Tokens.Select(t => t.Value).ToArray();
}

var argument1 = new Argument<string[]>("arg1")
{
Arity = new(1, 15),
CustomParser = TakeTwo
};
var argument2 = new Argument<string[]>("arg2")
{
Arity = new(1, 3),
CustomParser = TakeTwo
};
var argument3 = new Argument<string[]>("arg3")
{
Arity = new(1, 3),
CustomParser = TakeTwo
};
var argument4 = new Argument<string[]>("arg4")
{
Arity = new(1, 3),
CustomParser = TakeTwo
};
var argument5 = new Argument<string[]>("arg5")
{
Arity = new(1, 3),
CustomParser = TakeTwo
};
var argument6 = new Argument<string[]>("arg6")
{
Arity = new(1, 10)
};

var command = new RootCommand
{
argument1,
argument2,
argument3,
argument4,
argument5,
argument6
};

var parseResult = command.Parse(Enumerable.Range(1, 20).Select(i => i.ToString()).ToArray());

parseResult.GetRequiredValue(argument1).Should().BeEquivalentSequenceTo("1", "2");
parseResult.GetRequiredValue(argument2).Should().BeEquivalentSequenceTo("3", "4");
parseResult.GetRequiredValue(argument3).Should().BeEquivalentSequenceTo("5", "6");
parseResult.GetRequiredValue(argument4).Should().BeEquivalentSequenceTo("7", "8");
parseResult.GetRequiredValue(argument5).Should().BeEquivalentSequenceTo("9", "10");
parseResult.GetRequiredValue(argument6)
.Should()
.BeEquivalentSequenceTo("11", "12", "13", "14", "15", "16", "17", "18", "19", "20");
}

[Fact]
public void OnlyTake_throws_when_called_with_a_negative_value()
{
Expand Down Expand Up @@ -1003,4 +1065,4 @@ public void GetResult_by_name_can_be_used_recursively_within_custom_option_parse
parseResult.GetValue<string>("--second").Should().Be("two");
parseResult.GetValue<string>("--third").Should().Be("three");
}
}
}
31 changes: 17 additions & 14 deletions src/System.CommandLine/Parsing/ArgumentResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.CommandLine.Binding;
using System.Collections.Generic;
using System.Linq;

namespace System.CommandLine.Parsing
Expand All @@ -28,8 +29,6 @@ internal ArgumentResult(
/// </summary>
public Argument Argument { get; }

internal bool ArgumentLimitReached => Argument.Arity.MaximumNumberOfValues == (_tokens?.Count ?? 0);

public bool Implicit { get; private set; }

internal ArgumentConversionResult GetArgumentConversionResult()
Expand Down Expand Up @@ -98,9 +97,10 @@ public void OnlyTake(int numberOfTokens)
var arguments = parent.Command.Arguments;
int argumentIndex = arguments.IndexOf(Argument);
int nextArgumentIndex = argumentIndex + 1;
int tokensToPass = _tokens.Count - numberOfTokens;
List<Token> tokensToPass = _tokens.GetRange(numberOfTokens, _tokens.Count - numberOfTokens);
_tokens.RemoveRange(numberOfTokens, tokensToPass.Count);

while (tokensToPass > 0 && nextArgumentIndex < arguments.Count)
while (tokensToPass.Count > 0 && nextArgumentIndex < arguments.Count)
{
Argument nextArgument = parent.Command.Arguments[nextArgumentIndex];
ArgumentResult nextArgumentResult;
Expand All @@ -116,25 +116,28 @@ public void OnlyTake(int numberOfTokens)
SymbolResultTree.Add(nextArgument, nextArgumentResult);
}

while (!nextArgumentResult.ArgumentLimitReached && tokensToPass > 0)
if (nextArgumentResult._tokens is not null)
{
Token toPass = _tokens[numberOfTokens];
_tokens.RemoveAt(numberOfTokens);
nextArgumentResult.AddToken(toPass);
--tokensToPass;
tokensToPass.AddRange(nextArgumentResult._tokens);
}

int tokensToTake = Math.Min(nextArgument.Arity.MaximumNumberOfValues, tokensToPass.Count);

nextArgumentResult._tokens = tokensToTake > 0
? tokensToPass.GetRange(0, tokensToTake)
: null;
nextArgumentResult._conversionResult = null;
nextArgumentResult._validatorsHaveBeenRun = false;

tokensToPass.RemoveRange(0, tokensToTake);
nextArgumentIndex++;
}

CommandResult rootCommand = parent;
// When_tokens_are_passed_on_by_custom_parser_on_last_argument_then_they_become_unmatched_tokens
while (tokensToPass > 0)
for (var i = 0; i < tokensToPass.Count; i++)
{
Token unmatched = _tokens[numberOfTokens];
_tokens.RemoveAt(numberOfTokens);
SymbolResultTree.AddUnmatchedToken(unmatched, parent, rootCommand);
--tokensToPass;
SymbolResultTree.AddUnmatchedToken(tokensToPass[i], parent, rootCommand);
}
}

Expand Down