Skip to content

Commit e292617

Browse files
jonsequiturjonsequitur
authored
Fix #2128 (#2656)
* improve preaction testing * fix #2128
1 parent 1045e6c commit e292617

File tree

7 files changed

+161
-49
lines changed

7 files changed

+161
-49
lines changed

‎src/System.CommandLine.Tests/Help/Approvals/HelpBuilderTests.Help_layout_has_not_changed.approved.txt

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ Arguments:
1212
<Read|ReadWrite|Write> the-root-arg-enum-default-description [default: Read]
1313

1414
Options:
15-
-trna, --the-root-option-no-arg (REQUIRED) the-root-option-no-arg-description
16-
-trondda, --the-root-option-no-description-default-arg [default: the-root-option--no-description-default-arg-value]
17-
-tronda, --the-root-option-no-default-arg <the-root-option-arg-no-default-arg> (REQUIRED) the-root-option-no-default-description
18-
-troda, --the-root-option-default-arg <the-root-option-arg> the-root-option-default-arg-description [default: the-root-option-arg-value]
19-
-troea, --the-root-option-enum-arg <Read|ReadWrite|Write> the-root-option-description [default: Read]
20-
-trorea, --the-root-option-required-enum-arg <Read|ReadWrite|Write> (REQUIRED) the-root-option-description [default: Read]
21-
-tromld, --the-root-option-multi-line-description the-root-option
22-
multi-line
23-
description
15+
-trna, --the-root-option-no-arg (REQUIRED) the-root-option-no-arg-description
16+
-trondda, --the-root-option-no-description-default-arg <the-root-option-no-description-default-arg> [default: the-root-option--no-description-default-arg-value]
17+
-tronda, --the-root-option-no-default-arg <the-root-option-arg-no-default-arg> (REQUIRED) the-root-option-no-default-description
18+
-troda, --the-root-option-default-arg <the-root-option-arg> the-root-option-default-arg-description [default: the-root-option-arg-value]
19+
-troea, --the-root-option-enum-arg <Read|ReadWrite|Write> the-root-option-description [default: Read]
20+
-trorea, --the-root-option-required-enum-arg <Read|ReadWrite|Write> (REQUIRED) the-root-option-description [default: Read]
21+
-tromld, --the-root-option-multi-line-description the-root-option
22+
multi-line
23+
description
2424

‎src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public void Option_can_customize_displayed_default_value()
4545
_helpBuilder.Write(command, _console);
4646
var expected =
4747
$"Options:{NewLine}" +
48-
$"{_indentation}--the-option{_columnPadding}[default: 42]{NewLine}{NewLine}";
48+
$"{_indentation}--the-option <the-option>{_columnPadding}[default: 42]{NewLine}{NewLine}";
4949

5050
_console.ToString().Should().Contain(expected);
5151
}
@@ -245,9 +245,9 @@ public void Customize_throws_when_symbol_is_null()
245245

246246

247247
[Theory]
248-
[InlineData(false, false, "--option \\s*description")]
248+
[InlineData(false, false, "--option <option>\\s*description")]
249249
[InlineData(true, false, "custom 1st\\s*description")]
250-
[InlineData(false, true, "--option \\s*custom 2nd")]
250+
[InlineData(false, true, "--option <option>\\s*custom 2nd")]
251251
[InlineData(true, true, "custom 1st\\s*custom 2nd")]
252252
public void Option_can_fallback_to_default_when_customizing(bool conditionA, bool conditionB, string expected)
253253
{

‎src/System.CommandLine.Tests/Help/HelpBuilderTests.cs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,7 @@ public void Options_section_properly_wraps_description_when_long_default_value_i
11541154
new Option<string>("--aaa", "-a")
11551155
{
11561156
Description = longOptionText,
1157-
DefaultValueFactory = (_) => "the quick brown fox jumps over the lazy dog"
1157+
DefaultValueFactory = _ => "the quick brown fox jumps over the lazy dog"
11581158
},
11591159
new Option<string>("-y") { Description = "Option with a short description" },
11601160
};
@@ -1163,9 +1163,9 @@ public void Options_section_properly_wraps_description_when_long_default_value_i
11631163
helpBuilder.Write(command, _console);
11641164

11651165
var expected =
1166-
$"{_indentation}-a, --aaa{_columnPadding}The option whose description is long enough that it {NewLine}" +
1167-
$"{_indentation} {_columnPadding}wraps to a new line [default: the quick brown fox jumps {NewLine}" +
1168-
$"{_indentation} {_columnPadding}over the lazy dog]{NewLine}";
1166+
$"{_indentation}-a, --aaa <aaa>{_columnPadding}The option whose description is long enough that {NewLine}" +
1167+
$"{_indentation} {_columnPadding}it wraps to a new line [default: the quick brown {NewLine}" +
1168+
$"{_indentation} {_columnPadding}fox jumps over the lazy dog]{NewLine}";
11691169

11701170
_console.ToString().Should().Contain(expected);
11711171
}
@@ -1313,6 +1313,22 @@ public void Option_aliases_are_shown_before_long_names_regardless_of_alphabetica
13131313
_console.ToString().Should().Contain("-a, -z, --aaa, --zzz");
13141314
}
13151315

1316+
[Fact] // https://github.com/dotnet/command-line-api/issues/2128
1317+
public void Option_argument_name_uses_option_name_when_help_name_is_not_specified()
1318+
{
1319+
var command = new RootCommand
1320+
{
1321+
new Option<string>("--name", "-n")
1322+
{
1323+
Description = "The description"
1324+
}
1325+
};
1326+
1327+
_helpBuilder.Write(command, _console);
1328+
1329+
_console.ToString().Should().Contain("-n, --name <name> The description");
1330+
}
1331+
13161332
[Fact]
13171333
public void Help_describes_default_value_for_option_with_argument_having_default_value()
13181334
{
@@ -1341,14 +1357,14 @@ public void Option_arguments_with_default_values_that_are_enumerable_display_pip
13411357
{
13421358
new Option<List<int>>("--filter-size")
13431359
{
1344-
DefaultValueFactory = (_) => new List<int> { 0, 2, 4 }
1360+
DefaultValueFactory = _ => [0, 2, 4]
13451361
}
13461362
};
13471363

13481364
_helpBuilder.Write(command, _console);
13491365
var expected =
13501366
$"Options:{NewLine}" +
1351-
$"{_indentation}--filter-size{_columnPadding}[default: 0|2|4]{NewLine}{NewLine}";
1367+
$"{_indentation}--filter-size <filter-size>{_columnPadding}[default: 0|2|4]{NewLine}{NewLine}";
13521368

13531369
_console.ToString().Should().Contain(expected);
13541370
}
@@ -1360,14 +1376,14 @@ public void Option_arguments_with_default_values_that_are_array_display_pipe_del
13601376
{
13611377
new Option<string[]>("--prefixes")
13621378
{
1363-
DefaultValueFactory = (_) => new[]{ "^(TODO|BUG)", "^HACK" }
1379+
DefaultValueFactory = _ => new[]{ "^(TODO|BUG)", "^HACK" }
13641380
}
13651381
};
13661382

13671383
_helpBuilder.Write(command, _console);
13681384
var expected =
13691385
$"Options:{NewLine}" +
1370-
$"{_indentation}--prefixes{_columnPadding}[default: ^(TODO|BUG)|^HACK]{NewLine}{NewLine}";
1386+
$"{_indentation}--prefixes <prefixes>{_columnPadding}[default: ^(TODO|BUG)|^HACK]{NewLine}{NewLine}";
13711387

13721388
_console.ToString().Should().Contain(expected);
13731389
}

‎src/System.CommandLine.Tests/HelpOptionTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ public void Help_and_version_options_are_displayed_after_other_options_on_root_c
319319
.Should()
320320
.ContainInOrder([
321321
"Options:",
322-
"-i The option",
322+
"-i <i> The option",
323323
"-?, -h, --help Show help and usage information",
324324
"--version Show version information",
325325
"Commands:"
@@ -356,7 +356,7 @@ public void Help_and_version_options_are_displayed_after_other_options_on_subcom
356356
.Should()
357357
.ContainInOrder([
358358
"Options:",
359-
"-i The option",
359+
"-i <i> The option",
360360
"-?, -h, --help Show help and usage information",
361361
]);
362362
}

‎src/System.CommandLine.Tests/Invocation/InvocationTests.cs

Lines changed: 93 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -265,35 +265,112 @@ public void Nonterminating_option_action_does_not_short_circuit_command_action()
265265
commandActionWasCalled.Should().BeTrue();
266266
}
267267

268-
[Fact]
269-
public void When_multiple_options_with_actions_are_present_then_only_the_last_one_is_invoked()
268+
[Theory]
269+
[InlineData(true)]
270+
[InlineData(false)]
271+
public async Task When_multiple_options_with_terminating_actions_are_present_then_only_the_last_one_is_invoked(bool invokeAsync)
270272
{
271273
bool optionAction1WasCalled = false;
272274
bool optionAction2WasCalled = false;
273275
bool optionAction3WasCalled = false;
274276

275-
SynchronousTestAction optionAction1 = new(_ => optionAction1WasCalled = true);
276-
SynchronousTestAction optionAction2 = new(_ => optionAction2WasCalled = true);
277-
SynchronousTestAction optionAction3 = new(_ => optionAction3WasCalled = true);
277+
SynchronousTestAction optionAction1 = new(_ =>
278+
{
279+
optionAction1WasCalled = true;
280+
}, terminating: true);
281+
SynchronousTestAction optionAction2 = new(_ =>
282+
{
283+
optionAction2WasCalled = true;
284+
}, terminating: true);
285+
SynchronousTestAction optionAction3 = new(_ =>
286+
{
287+
optionAction3WasCalled = true;
288+
}, terminating: true);
278289

279-
Command command = new Command("cmd")
290+
var command = new RootCommand
280291
{
281-
new Option<bool>("--1") { Action = optionAction1 },
282-
new Option<bool>("--2") { Action = optionAction2 },
283-
new Option<bool>("--3") { Action = optionAction3 }
292+
Action = new AsynchronousTestAction(_ => {}),
293+
Options =
294+
{
295+
new Option<bool>("--1") { Action = optionAction1 },
296+
new Option<bool>("--2") { Action = optionAction2 },
297+
new Option<bool>("--3") { Action = optionAction3 },
298+
}
284299
};
285300

286-
ParseResult parseResult = command.Parse("cmd --1 true --3 false --2 true");
301+
ParseResult parseResult = command.Parse("--1 --3 --2");
287302

288303
using var _ = new AssertionScope();
289304

290305
parseResult.Action.Should().Be(optionAction2);
291-
parseResult.Invoke().Should().Be(0);
306+
307+
if (invokeAsync)
308+
{
309+
(await parseResult.InvokeAsync()).Should().Be(0);
310+
}
311+
else
312+
{
313+
parseResult.Invoke().Should().Be(0);
314+
}
315+
292316
optionAction1WasCalled.Should().BeFalse();
293317
optionAction2WasCalled.Should().BeTrue();
294318
optionAction3WasCalled.Should().BeFalse();
295319
}
296320

321+
[Theory]
322+
[InlineData(true)]
323+
[InlineData(false)]
324+
public async Task When_multiple_options_with_nonterminating_actions_are_present_then_all_are_invoked(bool invokeAsync)
325+
{
326+
bool optionAction1WasCalled = false;
327+
bool optionAction2WasCalled = false;
328+
bool optionAction3WasCalled = false;
329+
bool commandActionWasCalled = false;
330+
331+
SynchronousTestAction optionAction1 = new(_ =>
332+
{
333+
optionAction1WasCalled = true;
334+
}, terminating: false);
335+
SynchronousTestAction optionAction2 = new(_ =>
336+
{
337+
optionAction2WasCalled = true;
338+
}, terminating: false);
339+
SynchronousTestAction optionAction3 = new(_ =>
340+
{
341+
optionAction3WasCalled = true;
342+
}, terminating: false);
343+
344+
var command = new RootCommand
345+
{
346+
Action = new AsynchronousTestAction(_ => commandActionWasCalled = true),
347+
Options =
348+
{
349+
new Option<bool>("--1") { Action = optionAction1 },
350+
new Option<bool>("--2") { Action = optionAction2 },
351+
new Option<bool>("--3") { Action = optionAction3 },
352+
}
353+
};
354+
355+
ParseResult parseResult = command.Parse("--1 true --3 false --2 true");
356+
357+
using var _ = new AssertionScope();
358+
359+
if (invokeAsync)
360+
{
361+
(await parseResult.InvokeAsync()).Should().Be(0);
362+
}
363+
else
364+
{
365+
parseResult.Invoke().Should().Be(0);
366+
}
367+
368+
optionAction1WasCalled.Should().BeTrue();
369+
optionAction2WasCalled.Should().BeTrue();
370+
optionAction3WasCalled.Should().BeTrue();
371+
commandActionWasCalled.Should().BeTrue();
372+
}
373+
297374
[Fact]
298375
public void Directive_action_takes_precedence_over_option_action()
299376
{
@@ -327,9 +404,12 @@ public void Directive_action_takes_precedence_over_option_action()
327404
[Theory]
328405
[InlineData(true)]
329406
[InlineData(false)]
330-
public async Task Nontermninating_option_actions_handle_exceptions_and_return_an_error_return_code(bool invokeAsync)
407+
public async Task Nonterminating_option_actions_handle_exceptions_and_return_an_error_return_code(bool invokeAsync)
331408
{
332-
var nonexclusiveAction = new SynchronousTestAction(_ => throw new Exception("oops!"), terminating: false);
409+
var nonexclusiveAction = new SynchronousTestAction(_ =>
410+
{
411+
throw new Exception("oops!");
412+
}, terminating: false);
333413

334414
var command = new RootCommand
335415
{

‎src/System.CommandLine/Help/HelpBuilder.Default.cs

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,28 +75,44 @@ public static string GetArgumentUsageLabel(Symbol parameter)
7575
_ => throw new InvalidOperationException()
7676
};
7777

78-
static string? GetUsageLabel(string? helpName, Type valueType, List<Func<CompletionContext, IEnumerable<CompletionItem>>> completionSources, Symbol symbol, ArgumentArity arity)
78+
static string? GetUsageLabel(
79+
string? helpName,
80+
Type valueType,
81+
List<Func<CompletionContext, IEnumerable<CompletionItem>>> completionSources,
82+
Symbol symbol,
83+
ArgumentArity arity)
7984
{
80-
// Argument.HelpName is always first choice
8185
if (!string.IsNullOrWhiteSpace(helpName))
8286
{
8387
return $"<{helpName}>";
8488
}
85-
else if (
86-
!(valueType == typeof(bool) || valueType == typeof(bool?))
87-
&& arity.MaximumNumberOfValues > 0 // allowing zero arguments means we don't need to show usage
88-
&& completionSources.Count > 0)
89-
{
90-
IEnumerable<string> completions = symbol
91-
.GetCompletions(CompletionContext.Empty)
92-
.Select(item => item.Label);
9389

94-
string joined = string.Join("|", completions);
90+
if (valueType == typeof(bool) ||
91+
valueType == typeof(bool?) ||
92+
arity.MaximumNumberOfValues <= 0) // allowing zero arguments means we don't need to show usage
93+
{
94+
return null;
95+
}
9596

96-
if (!string.IsNullOrEmpty(joined))
97+
if (completionSources.Count <= 0)
98+
{
99+
if (symbol is Option)
97100
{
98-
return $"<{joined}>";
101+
return $"<{symbol.Name.TrimStart('-', '/')}>";
99102
}
103+
104+
return null;
105+
}
106+
107+
IEnumerable<string> completions = symbol
108+
.GetCompletions(CompletionContext.Empty)
109+
.Select(item => item.Label);
110+
111+
string joined = string.Join("|", completions);
112+
113+
if (!string.IsNullOrEmpty(joined))
114+
{
115+
return $"<{joined}>";
100116
}
101117

102118
return null;

‎src/System.CommandLine/ParseResult.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ internal ParseResult(
5151
}
5252
else
5353
{
54-
Tokens = Array.Empty<Token>();
54+
Tokens = [];
5555
}
5656

5757
CommandLineText = commandLineText;

0 commit comments

Comments
 (0)