Skip to content

Commit f37d8df

Browse files
committed
address code review feedback
1 parent ea619bf commit f37d8df

File tree

3 files changed

+22
-37
lines changed

3 files changed

+22
-37
lines changed

src/System.CommandLine.Tests/DirectiveTests.cs

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,13 @@ namespace System.CommandLine.Tests
1111
public class DirectiveTests
1212
{
1313
[Fact]
14-
public void Directives_should_not_be_considered_as_unmatched_tokens_when_they_are_enabled()
14+
public void Directives_should_be_considered_as_unmatched_tokens_when_they_are_not_matched()
1515
{
16-
RootCommand root = new () { new Option<bool>("-y") };
17-
CommandLineBuilder builder = new (root);
18-
builder.Directives.Add(new ("some"));
16+
Directive directive = new("parse");
1917

20-
var result = root.Parse($"{RootCommand.ExecutableName} [parse] -y", builder.Build());
18+
ParseResult result = Parse(new Option<bool>("-y"), directive, $"{RootCommand.ExecutableName} [nonExisting] -y");
2119

22-
result.UnmatchedTokens.Should().BeEmpty();
20+
result.UnmatchedTokens.Should().ContainSingle("[nonExisting]");
2321
}
2422

2523
[Fact]
@@ -99,14 +97,19 @@ public void Directives_must_have_a_non_empty_key(string directive)
9997
}
10098

10199
[Theory]
102-
[InlineData("[par se]")]
103-
[InlineData("[ parse]")]
104-
[InlineData("[parse ]")]
105-
public void Directives_cannot_contain_spaces(string value)
100+
[InlineData("[par se]", "[par", "se]")]
101+
[InlineData("[ parse]", "[", "parse]")]
102+
[InlineData("[parse ]", "[parse", "]")]
103+
public void Directives_cannot_contain_spaces(string value, string firstUnmatchedToken, string secondUnmatchedToken)
106104
{
107105
Action create = () => new Directive(value);
108-
109106
create.Should().Throw<ArgumentException>();
107+
108+
Directive directive = new("parse");
109+
ParseResult result = Parse(new Option<bool>("-y"), directive, $"{value} -y");
110+
result.FindResultFor(directive).Should().BeNull();
111+
112+
result.UnmatchedTokens.Should().BeEquivalentTo(firstUnmatchedToken, secondUnmatchedToken);
110113
}
111114

112115
[Fact]
@@ -119,24 +122,6 @@ public void When_a_directive_is_specified_more_than_once_then_its_values_are_agg
119122
result.FindResultFor(directive).Values.Should().BeEquivalentTo("one", "two");
120123
}
121124

122-
[Fact]
123-
public void When_directives_are_not_enabled_they_are_treated_as_regular_tokens()
124-
{
125-
var config = new CommandLineConfiguration(
126-
new RootCommand
127-
{
128-
new Argument<List<string>>()
129-
});
130-
131-
var result = config.RootCommand.Parse("[hello]", config);
132-
133-
result.CommandResult
134-
.Tokens
135-
.Select(t => t.Value)
136-
.Should()
137-
.BeEquivalentTo("[hello]");
138-
}
139-
140125
private static ParseResult Parse(Option option, Directive directive, string commandLine)
141126
{
142127
RootCommand root = new() { option };

src/System.CommandLine/Parsing/ParseOperation.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ void ParseDirective()
297297

298298
if (token.Symbol is not Directive directive)
299299
{
300-
// Directives_should_not_be_considered_as_unmatched_tokens
300+
AddCurrentTokenToUnmatched();
301301
return;
302302
}
303303

src/System.CommandLine/Parsing/StringExtensions.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ internal static void Tokenize(
8282

8383
var currentCommand = configuration.RootCommand;
8484
var foundDoubleDash = false;
85-
var foundEndOfDirectives = configuration.Directives.Count == 0;
85+
var foundEndOfDirectives = false;
8686

8787
var tokenList = new List<Token>(args.Count);
8888

89-
var knownTokens = configuration.RootCommand.ValidTokens(configuration);
89+
var knownTokens = configuration.RootCommand.ValidTokens(configuration.Directives);
9090

9191
int i = FirstArgumentIsRootCommand(args, configuration.RootCommand, inferRootCommand)
9292
? 0
@@ -185,7 +185,7 @@ configuration.TokenReplacer is { } replacer &&
185185
if (cmd != configuration.RootCommand)
186186
{
187187
knownTokens = cmd.ValidTokens(
188-
configuration: null); // config contains Directives, they are allowed only for RootCommand
188+
directives: null); // config contains Directives, they are allowed only for RootCommand
189189
}
190190
currentCommand = cmd;
191191
tokenList.Add(Command(arg, cmd));
@@ -431,15 +431,15 @@ static IEnumerable<string> SplitLine(string line)
431431
}
432432
}
433433

434-
private static Dictionary<string, Token> ValidTokens(this Command command, CommandLineConfiguration? configuration)
434+
private static Dictionary<string, Token> ValidTokens(this Command command, IReadOnlyList<Directive>? directives)
435435
{
436436
Dictionary<string, Token> tokens = new(StringComparer.Ordinal);
437437

438-
if (configuration?.Directives is not null)
438+
if (directives is not null)
439439
{
440-
for (int directiveIndex = 0; directiveIndex < configuration.Directives.Count; directiveIndex++)
440+
for (int directiveIndex = 0; directiveIndex < directives.Count; directiveIndex++)
441441
{
442-
Directive directive = configuration.Directives[directiveIndex];
442+
Directive directive = directives[directiveIndex];
443443
tokens.Add(
444444
directive.Name,
445445
new Token(directive.Name, TokenType.Directive, directive, Token.ImplicitPosition));

0 commit comments

Comments
 (0)