Skip to content

Commit

Permalink
Merge pull request #798 from batzen/fix/issue-776
Browse files Browse the repository at this point in the history
Fix for #776 and #797
  • Loading branch information
ericnewton76 authored May 12, 2022
2 parents 9ba45a7 + ff825e5 commit 40a4104
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 17 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,7 @@ artifacts/*
*.DotSettings.user
# Visual Studio 2015 cache/options directory
.vs/
# Rider
.idea/

[R|r]elease/**
29 changes: 16 additions & 13 deletions src/CommandLine/Core/Tokenizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,29 +86,32 @@ public static Result<IEnumerable<Token>, Error> ExplodeOptionList(
return Result.Succeed(exploded as IEnumerable<Token>, tokenizerResult.SuccessMessages());
}

/// <summary>
/// Normalizes the given <paramref name="tokens"/>.
/// </summary>
/// <returns>The given <paramref name="tokens"/> minus all names, and their value if one was present, that are not found using <paramref name="nameLookup"/>.</returns>
public static IEnumerable<Token> Normalize(
IEnumerable<Token> tokens, Func<string, bool> 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<int>();
if (t.IsName() == false
|| nameLookup(t.Text))
{
return Maybe.Nothing<Tuple<Token, Token>>();
}
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<Token, Token>(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<Token>())
.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;
}
Expand Down
36 changes: 32 additions & 4 deletions tests/CommandLine.Tests/Unit/Core/TokenizerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, bool> nameLookup =
name => name.Equals("x") || name.Equals("string-seq") || name.Equals("switch");

Expand All @@ -78,7 +78,7 @@ public void Normalize_should_remove_all_value_with_explicit_assignment_of_existi
Enumerable.Empty<Token>()
.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<Error>()),
, nameLookup);
Expand All @@ -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<string, bool> nameLookup =
name => name.Equals("x") || name.Equals("string-seq") || name.Equals("switch");

// Exercize system
var result =
Tokenizer.Normalize(
//Result.Succeed(
Enumerable.Empty<Token>()
.Concat(
new[] {
Token.Name("x"), Token.Name("string-seq"), Token.Value("value0", true), Token.Value("bb"),
Token.Name("unknown"), Token.Name("switch") })
//,Enumerable.Empty<Error>()),
, nameLookup);

// Verify outcome
result.Should().BeEquivalentTo(expectedTokens);

// Teardown
}

[Fact]
public void Should_properly_parse_option_with_equals_in_value()
{
Expand Down
36 changes: 36 additions & 0 deletions tests/CommandLine.Tests/Unit/Issue776Tests.cs
Original file line number Diff line number Diff line change
@@ -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<Options>(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; }
}
}
}

0 comments on commit 40a4104

Please sign in to comment.