From 5e4548d1279cff02fc432d44a0de90c5919cd04d Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Thu, 13 Apr 2023 11:11:01 -0700 Subject: [PATCH 01/19] wip --- .../Help/HelpBuilderTests.Customization.cs | 2 +- .../Invocation/InvocationTests.cs | 153 +++++++++++++++++- src/System.CommandLine/CliAction.cs | 1 + .../Completions/SuggestDirective.cs | 1 + .../Invocation/AnonymousCliAction.cs | 1 + .../Invocation/TypoCorrectionAction.cs | 1 + .../Parsing/ParseOperation.cs | 1 + 7 files changed, 156 insertions(+), 4 deletions(-) diff --git a/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs b/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs index f9065cec9a..5056af2b1c 100644 --- a/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs +++ b/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs @@ -447,7 +447,7 @@ public void Layout_can_be_composed_dynamically_based_on_context() commandWithCustomHelp }; - command.Options.OfType().Single().Action = new HelpAction() + command.Options.OfType().Single().Action = new HelpAction { Builder = helpBuilder }; diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index b3f07344f9..f85142c9d0 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; +using System.CommandLine; using System.CommandLine.Help; using System.CommandLine.Parsing; using System.IO; @@ -134,7 +136,7 @@ public void Custom_RootCommand_Action_can_set_custom_result_code_via_Invoke() { var rootCommand = new CliRootCommand { - Action = new CustomExitCodeAction() + Action = new CustomReturnCodeAction() }; rootCommand.Parse("").Invoke().Should().Be(123); @@ -145,7 +147,7 @@ public async Task Custom_RootCommand_Action_can_set_custom_result_code_via_Invok { var rootCommand = new CliRootCommand { - Action = new CustomExitCodeAction() + Action = new CustomReturnCodeAction() }; (await rootCommand.Parse("").InvokeAsync()).Should().Be(456); @@ -170,6 +172,7 @@ public async Task Anonymous_RootCommand_Task_returning_Action_can_set_custom_res (await rootCommand.Parse("").InvokeAsync()).Should().Be(123); } + [Fact] public void Anonymous_RootCommand_int_returning_Action_can_set_custom_result_code_via_Invoke() { @@ -190,6 +193,74 @@ public async Task Anonymous_RootCommand_int_returning_Action_can_set_custom_resu (await rootCommand.Parse("").InvokeAsync()).Should().Be(123); } + [Fact] + public void Option_action_takes_precedence_over_command_action() + { + OptionAction optionAction = new(); + bool commandActionWasCalled = false; + + CliOption option = new("--test") + { + Action = optionAction + }; + CliCommand command = new CliCommand("cmd") + { + option + }; + command.SetAction(_ => + { + commandActionWasCalled = true; + }); + + ParseResult parseResult = command.Parse("cmd --test true"); + + parseResult.Action.Should().NotBeNull(); + optionAction.WasCalled.Should().BeFalse(); + commandActionWasCalled.Should().BeFalse(); + + parseResult.Invoke().Should().Be(0); + optionAction.WasCalled.Should().BeTrue(); + commandActionWasCalled.Should().BeFalse(); + } + + [Fact] + public void When_multiple_options_with_actions_are_present_then_only_the_last_one_is_invoked() + { + OptionAction optionAction1 = new(); + OptionAction optionAction2 = new(); + OptionAction optionAction3 = new(); + + CliCommand command = new CliCommand("cmd") + { + new CliOption("--1") { Action = optionAction1 }, + new CliOption("--2") { Action = optionAction2 }, + new CliOption("--3") { Action = optionAction3 } + }; + + ParseResult parseResult = command.Parse("cmd --1 true --3 false --2 true "); + + parseResult.Action.Should().Be(optionAction2); + + parseResult.Invoke().Should().Be(0); + optionAction1.WasCalled.Should().BeFalse(); + optionAction2.WasCalled.Should().BeTrue(); + optionAction3.WasCalled.Should().BeFalse(); + } + + internal sealed class OptionAction : CliAction + { + internal bool WasCalled = false; + + public override int Invoke(ParseResult context) + { + WasCalled = true; + return 0; + } + + public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) + => Task.FromResult(Invoke(context)); + } + [Theory] [InlineData(true)] [InlineData(false)] @@ -239,7 +310,7 @@ public async Task Command_InvokeAsync_with_cancelation_token_invokes_command_han await command.Parse("test").InvokeAsync(cancellationToken: cts.Token); } - private class CustomExitCodeAction : CliAction + private class CustomReturnCodeAction : CliAction { public override int Invoke(ParseResult context) => 123; @@ -281,5 +352,81 @@ public override Task InvokeAsync(ParseResult parseResult, CancellationToken return Task.FromResult(0); } } + + [Fact] + public async Task Rich_return_types_example() + { + var command = new CliRootCommand(); + + command.SetAction(async (parseResult, cancellationToken) => + { + return new MyCustomResultType(); + }); + + var config = new MyCustomConfiguration(command); + + await config.InvokeAsync("x -y 123"); + + + + // TODO (Rich_return_types_example) write test + throw new NotImplementedException(); + } + } +} + +public static class RichReturnExtensions +{ + public static void SetAction(this CliCommand command, Func> action) + { + command.Action = new RichReturnAction(action); + } + + private class RichReturnAction : CliAction + { + private readonly Func> _action; + + public RichReturnAction(Func> action) + { + _action = action; + } + + public override int Invoke(ParseResult parseResult) + { + var actionResult = _action(parseResult, CancellationToken.None).GetAwaiter().GetResult(); + + if (parseResult.Configuration is MyCustomConfiguration customConfig) + { + customConfig.ActionResult = actionResult; + } + + return 0; + } + + public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + { + var actionResult = await _action(parseResult, CancellationToken.None); + + if (parseResult.Configuration is MyCustomConfiguration customConfig) + { + customConfig.ActionResult = actionResult; + } + + return 0; + } } } + +public class MyCustomResultType +{ +} + +public class MyCustomConfiguration : CliConfiguration +{ + public MyCustomConfiguration(CliCommand rootCommand) : base(rootCommand) + { + } + + public object ActionResult { get; set; } +} + diff --git a/src/System.CommandLine/CliAction.cs b/src/System.CommandLine/CliAction.cs index c857811287..1f2c41eb43 100644 --- a/src/System.CommandLine/CliAction.cs +++ b/src/System.CommandLine/CliAction.cs @@ -11,6 +11,7 @@ namespace System.CommandLine /// public abstract class CliAction { + // FIX: (CliAction) change this to accommodate the fact that the core action isn't "exclusive" either, and some actions might eventually be created that run after it. public bool Exclusive { get; protected set; } = true; /// diff --git a/src/System.CommandLine/Completions/SuggestDirective.cs b/src/System.CommandLine/Completions/SuggestDirective.cs index 4853194705..1ef76c31dd 100644 --- a/src/System.CommandLine/Completions/SuggestDirective.cs +++ b/src/System.CommandLine/Completions/SuggestDirective.cs @@ -26,6 +26,7 @@ public override CliAction? Action private sealed class SuggestDirectiveAction : CliAction { + // FIX: (SuggestDirectiveAction) rename, make public private readonly SuggestDirective _directive; internal SuggestDirectiveAction(SuggestDirective suggestDirective) => _directive = suggestDirective; diff --git a/src/System.CommandLine/Invocation/AnonymousCliAction.cs b/src/System.CommandLine/Invocation/AnonymousCliAction.cs index 5b0b048792..1fbcc516f5 100644 --- a/src/System.CommandLine/Invocation/AnonymousCliAction.cs +++ b/src/System.CommandLine/Invocation/AnonymousCliAction.cs @@ -8,6 +8,7 @@ namespace System.CommandLine.Invocation { internal sealed class AnonymousCliAction : CliAction { + // FIX: (AnonymousCliAction) rename to CliCommandAction, make public private readonly Func>? _asyncAction; private readonly Func? _syncAction; diff --git a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs b/src/System.CommandLine/Invocation/TypoCorrectionAction.cs index c490f94695..b26e42d3cb 100644 --- a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs +++ b/src/System.CommandLine/Invocation/TypoCorrectionAction.cs @@ -10,6 +10,7 @@ namespace System.CommandLine.Invocation { internal sealed class TypoCorrectionAction : CliAction { + // FIX: (TypoCorrectionAction) make this part of ParseErrorResultAction private const int MaxLevenshteinDistance = 3; public override int Invoke(ParseResult parseResult) diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index b4a4895fd6..8fc19d805c 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -191,6 +191,7 @@ private void ParseOption() if (!_symbolResultTree.TryGetValue(option, out SymbolResult? symbolResult)) { + // FIX: (ParseOption) can this be moved out to a CliAction? // DiagramDirective has a precedence over --help and --version if (!_isDiagramRequested) { From bfaa2cf696432add2691c944463f067850f5070a Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 23 May 2023 15:38:53 -0700 Subject: [PATCH 02/19] wip --- System.CommandLine.sln | 37 +-------- System.CommandLine.v3.ncrunchsolution | 10 +++ .../Invocation/InvocationTests.cs | 76 ------------------- src/System.CommandLine/CliCommand.cs | 8 +- ...nymousCliAction.cs => CliCommandAction.cs} | 9 +-- .../Parsing/ArgumentResult.cs | 1 + .../Parsing/SymbolResult.cs | 1 - 7 files changed, 23 insertions(+), 119 deletions(-) create mode 100644 System.CommandLine.v3.ncrunchsolution rename src/System.CommandLine/Invocation/{AnonymousCliAction.cs => CliCommandAction.cs} (79%) diff --git a/System.CommandLine.sln b/System.CommandLine.sln index 40d52102a2..84234d3f2a 100644 --- a/System.CommandLine.sln +++ b/System.CommandLine.sln @@ -58,16 +58,13 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Hosting. EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "HostingPlayground", "samples\HostingPlayground\HostingPlayground.csproj", "{0BF6958D-9EE3-4623-B3D6-4DA77EAC1906}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Generator", "src\System.CommandLine.Generator\System.CommandLine.Generator.csproj", "{B0D00128-E41B-4648-9D22-9B91F8F6BF0C}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Generator.CommandHandler", "src\System.CommandLine.Generator.CommandHandler\System.CommandLine.Generator.CommandHandler.csproj", "{591EF370-7AD7-4624-8B9D-FD15010CA657}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Generator.Tests", "src\System.CommandLine.Generator.Tests\System.CommandLine.Generator.Tests.csproj", "{70B98293-2F69-4262-AADD-D3EEE12046A8}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.NamingConventionBinder", "src\System.CommandLine.NamingConventionBinder\System.CommandLine.NamingConventionBinder.csproj", "{10DFE204-B027-49DA-BD77-08ECA18DD357}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.CommandLine.Generator.CommandHandler", "src\System.CommandLine.Generator.CommandHandler\System.CommandLine.Generator.CommandHandler.csproj", "{591EF370-7AD7-4624-8B9D-FD15010CA657}" -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.CommandLine.NamingConventionBinder", "src\System.CommandLine.NamingConventionBinder\System.CommandLine.NamingConventionBinder.csproj", "{10DFE204-B027-49DA-BD77-08ECA18DD357}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.NamingConventionBinder.Tests", "src\System.CommandLine.NamingConventionBinder.Tests\System.CommandLine.NamingConventionBinder.Tests.csproj", "{789A05F2-5EF6-4FE8-9609-4706207E047E}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.CommandLine.NamingConventionBinder.Tests", "src\System.CommandLine.NamingConventionBinder.Tests\System.CommandLine.NamingConventionBinder.Tests.csproj", "{789A05F2-5EF6-4FE8-9609-4706207E047E}" -EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.CommandLine.ApiCompatibility.Tests", "src\System.CommandLine.ApiCompatibility.Tests\System.CommandLine.ApiCompatibility.Tests.csproj", "{A54EE328-D456-4BAF-A180-84E77E6409AC}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.ApiCompatibility.Tests", "src\System.CommandLine.ApiCompatibility.Tests\System.CommandLine.ApiCompatibility.Tests.csproj", "{A54EE328-D456-4BAF-A180-84E77E6409AC}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -247,30 +244,6 @@ Global {0BF6958D-9EE3-4623-B3D6-4DA77EAC1906}.Release|x64.Build.0 = Release|Any CPU {0BF6958D-9EE3-4623-B3D6-4DA77EAC1906}.Release|x86.ActiveCfg = Release|Any CPU {0BF6958D-9EE3-4623-B3D6-4DA77EAC1906}.Release|x86.Build.0 = Release|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Debug|Any CPU.Build.0 = Debug|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Debug|x64.ActiveCfg = Debug|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Debug|x64.Build.0 = Debug|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Debug|x86.ActiveCfg = Debug|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Debug|x86.Build.0 = Debug|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Release|Any CPU.ActiveCfg = Release|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Release|Any CPU.Build.0 = Release|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Release|x64.ActiveCfg = Release|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Release|x64.Build.0 = Release|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Release|x86.ActiveCfg = Release|Any CPU - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C}.Release|x86.Build.0 = Release|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Debug|Any CPU.Build.0 = Debug|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Debug|x64.ActiveCfg = Debug|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Debug|x64.Build.0 = Debug|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Debug|x86.ActiveCfg = Debug|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Debug|x86.Build.0 = Debug|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Release|Any CPU.ActiveCfg = Release|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Release|Any CPU.Build.0 = Release|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Release|x64.ActiveCfg = Release|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Release|x64.Build.0 = Release|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Release|x86.ActiveCfg = Release|Any CPU - {70B98293-2F69-4262-AADD-D3EEE12046A8}.Release|x86.Build.0 = Release|Any CPU {591EF370-7AD7-4624-8B9D-FD15010CA657}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {591EF370-7AD7-4624-8B9D-FD15010CA657}.Debug|Any CPU.Build.0 = Debug|Any CPU {591EF370-7AD7-4624-8B9D-FD15010CA657}.Debug|x64.ActiveCfg = Debug|Any CPU @@ -338,8 +311,6 @@ Global {644C4B4A-4A32-4307-9F71-C3BF901FFB66} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {39483140-BC26-4CAD-BBAE-3DC76C2F16CF} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {0BF6958D-9EE3-4623-B3D6-4DA77EAC1906} = {6749FB3E-39DE-4321-A39E-525278E9408D} - {B0D00128-E41B-4648-9D22-9B91F8F6BF0C} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} - {70B98293-2F69-4262-AADD-D3EEE12046A8} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {591EF370-7AD7-4624-8B9D-FD15010CA657} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {10DFE204-B027-49DA-BD77-08ECA18DD357} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {789A05F2-5EF6-4FE8-9609-4706207E047E} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} diff --git a/System.CommandLine.v3.ncrunchsolution b/System.CommandLine.v3.ncrunchsolution new file mode 100644 index 0000000000..7d5820a216 --- /dev/null +++ b/System.CommandLine.v3.ncrunchsolution @@ -0,0 +1,10 @@ + + + True + + TargetFrameworks = net7.0 + TargetFramework = net7.0 + + True + + \ No newline at end of file diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index f85142c9d0..c76f3c0922 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -352,81 +352,5 @@ public override Task InvokeAsync(ParseResult parseResult, CancellationToken return Task.FromResult(0); } } - - [Fact] - public async Task Rich_return_types_example() - { - var command = new CliRootCommand(); - - command.SetAction(async (parseResult, cancellationToken) => - { - return new MyCustomResultType(); - }); - - var config = new MyCustomConfiguration(command); - - await config.InvokeAsync("x -y 123"); - - - - // TODO (Rich_return_types_example) write test - throw new NotImplementedException(); - } } } - -public static class RichReturnExtensions -{ - public static void SetAction(this CliCommand command, Func> action) - { - command.Action = new RichReturnAction(action); - } - - private class RichReturnAction : CliAction - { - private readonly Func> _action; - - public RichReturnAction(Func> action) - { - _action = action; - } - - public override int Invoke(ParseResult parseResult) - { - var actionResult = _action(parseResult, CancellationToken.None).GetAwaiter().GetResult(); - - if (parseResult.Configuration is MyCustomConfiguration customConfig) - { - customConfig.ActionResult = actionResult; - } - - return 0; - } - - public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - { - var actionResult = await _action(parseResult, CancellationToken.None); - - if (parseResult.Configuration is MyCustomConfiguration customConfig) - { - customConfig.ActionResult = actionResult; - } - - return 0; - } - } -} - -public class MyCustomResultType -{ -} - -public class MyCustomConfiguration : CliConfiguration -{ - public MyCustomConfiguration(CliCommand rootCommand) : base(rootCommand) - { - } - - public object ActionResult { get; set; } -} - diff --git a/src/System.CommandLine/CliCommand.cs b/src/System.CommandLine/CliCommand.cs index e5441433b4..04a78c04e4 100644 --- a/src/System.CommandLine/CliCommand.cs +++ b/src/System.CommandLine/CliCommand.cs @@ -110,7 +110,7 @@ public void SetAction(Action action) throw new ArgumentNullException(nameof(action)); } - Action = new AnonymousCliAction(context => + Action = new CliCommandAction(context => { action(context); return 0; @@ -128,7 +128,7 @@ public void SetAction(Func action) throw new ArgumentNullException(nameof(action)); } - Action = new AnonymousCliAction(action); + Action = new CliCommandAction(action); } /// @@ -141,7 +141,7 @@ public void SetAction(Func action) throw new ArgumentNullException(nameof(action)); } - Action = new AnonymousCliAction(async (context, cancellationToken) => + Action = new CliCommandAction(async (context, cancellationToken) => { await action(context, cancellationToken); return 0; @@ -159,7 +159,7 @@ public void SetAction(Func> action) throw new ArgumentNullException(nameof(action)); } - Action = new AnonymousCliAction(action); + Action = new CliCommandAction(action); } /// diff --git a/src/System.CommandLine/Invocation/AnonymousCliAction.cs b/src/System.CommandLine/Invocation/CliCommandAction.cs similarity index 79% rename from src/System.CommandLine/Invocation/AnonymousCliAction.cs rename to src/System.CommandLine/Invocation/CliCommandAction.cs index 1fbcc516f5..57b588c9c3 100644 --- a/src/System.CommandLine/Invocation/AnonymousCliAction.cs +++ b/src/System.CommandLine/Invocation/CliCommandAction.cs @@ -6,16 +6,15 @@ namespace System.CommandLine.Invocation { - internal sealed class AnonymousCliAction : CliAction + public class CliCommandAction : CliAction { - // FIX: (AnonymousCliAction) rename to CliCommandAction, make public private readonly Func>? _asyncAction; private readonly Func? _syncAction; - internal AnonymousCliAction(Func action) + internal CliCommandAction(Func action) => _syncAction = action; - internal AnonymousCliAction(Func> action) + internal CliCommandAction(Func> action) => _asyncAction = action; public override int Invoke(ParseResult parseResult) @@ -41,7 +40,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio } else { - return _syncAction!(parseResult); + return _syncAction!(parseResult); } } } diff --git a/src/System.CommandLine/Parsing/ArgumentResult.cs b/src/System.CommandLine/Parsing/ArgumentResult.cs index efc9100248..f7a793aff1 100644 --- a/src/System.CommandLine/Parsing/ArgumentResult.cs +++ b/src/System.CommandLine/Parsing/ArgumentResult.cs @@ -29,6 +29,7 @@ internal ArgumentResult( internal bool ArgumentLimitReached => Argument.Arity.MaximumNumberOfValues == (_tokens?.Count ?? 0); + // FIX: (ArgumentResult.Implicit) this can probably be removed internal bool Implicit => Argument.HasDefaultValue && Tokens.Count == 0; internal ArgumentConversionResult GetArgumentConversionResult() => diff --git a/src/System.CommandLine/Parsing/SymbolResult.cs b/src/System.CommandLine/Parsing/SymbolResult.cs index cff413b4f9..1be501f06c 100644 --- a/src/System.CommandLine/Parsing/SymbolResult.cs +++ b/src/System.CommandLine/Parsing/SymbolResult.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.Linq; namespace System.CommandLine.Parsing { From 319ac56f379682f8ca36be8c5541f191a1a48b80 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Thu, 1 Jun 2023 18:59:38 -0700 Subject: [PATCH 03/19] split CliAction into sync and async types --- .../CommandLine/Perf_Parser_ParseResult.cs | 5 +- .../HostingHandlerTest.cs | 38 +++----- .../HostingAction.cs | 35 ++++--- .../HostingExtensions.cs | 3 +- .../ModelBindingCommandHandlerTests.cs | 3 +- .../ParameterBindingTests.cs | 13 +-- .../BindingContextExtensions.cs | 4 +- .../BindingHandler.cs | 8 +- .../ModelBindingCommandHandler.cs | 3 - .../DirectiveTests.cs | 37 ++++++-- .../EnvironmentVariableDirectiveTests.cs | 9 +- .../CancelOnProcessTerminationTests.cs | 7 +- .../Invocation/InvocationTests.cs | 30 ++---- .../ParseResultTests.cs | 3 +- .../Utility/ParseResultExtensions.cs | 5 +- src/System.CommandLine/CliAction.cs | 32 ------- src/System.CommandLine/CliCommand.cs | 8 +- src/System.CommandLine/CliDirective.cs | 6 +- src/System.CommandLine/CliOption.cs | 6 +- .../Completions/SuggestDirective.cs | 16 +--- .../EnvironmentVariablesDirective.cs | 17 ++-- src/System.CommandLine/Help/HelpOption.cs | 2 + .../Help/HelpOptionAction.cs | 12 +-- .../Invocation/CliCommandAction.cs | 92 +++++++++++-------- .../Invocation/InvocationPipeline.cs | 45 ++++++++- .../Invocation/ParseErrorAction.cs | 39 +++----- .../Invocation/TypoCorrectionAction.cs | 79 ++++++++-------- .../ParseDiagramDirective.cs | 3 +- src/System.CommandLine/ParseResult.cs | 31 ++++++- .../Parsing/ArgumentResult.cs | 3 - .../Parsing/ParseDiagramAction.cs | 19 +--- src/System.CommandLine/VersionOption.cs | 23 +---- 32 files changed, 306 insertions(+), 330 deletions(-) delete mode 100644 src/System.CommandLine/CliAction.cs diff --git a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs index 9b1e5fbe65..e6e926ae97 100644 --- a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs +++ b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.CommandLine.Benchmarks.Helpers; +using System.CommandLine.Invocation; using System.IO; using System.Linq; using System.Text; @@ -60,7 +61,7 @@ public string ParseResult_Diagram(BdnParam parseResult) // clear the contents, so each benchmark has the same starting state stringBuilder.Clear(); - parseResult.Value.Action!.Invoke(parseResult.Value); + ((SynchronousCliAction)parseResult.Value.Action)!.Invoke(parseResult.Value); return stringBuilder.ToString(); } @@ -74,7 +75,7 @@ public async Task ParseResult_DiagramAsync(BdnParam parseRe // clear the contents, so each benchmark has the same starting state stringBuilder.Clear(); - await parseResult.Value.Action!.InvokeAsync(parseResult.Value); + await ((AsynchronousCliAction)parseResult.Value.Action!).InvokeAsync(parseResult.Value); return stringBuilder.ToString(); } diff --git a/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs b/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs index 76d59d3f8b..559f9143ad 100644 --- a/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs +++ b/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs @@ -1,4 +1,5 @@ -using System.Threading; +using System.CommandLine.Invocation; +using System.Threading; using System.Threading.Tasks; using FluentAssertions; using Microsoft.Extensions.DependencyInjection; @@ -101,8 +102,8 @@ public static async Task Invokes_DerivedClass() var service = new MyService(); var cmd = new CliRootCommand(); - cmd.Subcommands.Add(new MyCommand().UseCommandHandler()); - cmd.Subcommands.Add(new MyOtherCommand().UseCommandHandler()); + cmd.Subcommands.Add(new MyCommand().UseCommandHandler()); + cmd.Subcommands.Add(new MyOtherCommand().UseCommandHandler()); var config = new CliConfiguration(cmd) .UseHost((builder) => { builder.ConfigureServices(services => @@ -118,15 +119,10 @@ public static async Task Invokes_DerivedClass() service.StringValue.Should().Be("TEST"); } - public abstract class MyBaseHandler : CliAction + public abstract class MyBaseCliAction : AsynchronousCliAction { public int IntOption { get; set; } // bound from option - public override int Invoke(ParseResult context) - { - return Act(); - } - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) { return Task.FromResult(Act()); @@ -142,7 +138,7 @@ public MyCommand() : base(name: "mycommand") Options.Add(new CliOption("--int-option")); // or nameof(Handler.IntOption).ToKebabCase() if you don't like the string literal } - public class MyHandler : CliAction + public class MyHandler : AsynchronousCliAction { private readonly MyService service; @@ -152,13 +148,7 @@ public MyHandler(MyService service) } public int IntOption { get; set; } // bound from option - - public override int Invoke(ParseResult context) - { - service.Value = IntOption; - return IntOption; - } - + public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) { service.Value = IntOption; @@ -166,11 +156,11 @@ public override Task InvokeAsync(ParseResult context, CancellationToken can } } - public class MyDerivedHandler : MyBaseHandler + public class MyDerivedCliAction : MyBaseCliAction { private readonly MyService service; - public MyDerivedHandler(MyService service) + public MyDerivedCliAction(MyService service) { this.service = service; } @@ -191,7 +181,7 @@ public MyOtherCommand() : base(name: "myothercommand") Arguments.Add(new CliArgument("One") { Arity = ArgumentArity.ZeroOrOne }); } - public class MyHandler : CliAction + public class MyHandler : AsynchronousCliAction { private readonly MyService service; @@ -203,9 +193,7 @@ public MyHandler(MyService service) public int IntOption { get; set; } // bound from option public string One { get; set; } - - public override int Invoke(ParseResult context) => InvokeAsync(context, CancellationToken.None).GetAwaiter().GetResult(); - + public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) { service.Value = IntOption; @@ -214,11 +202,11 @@ public override Task InvokeAsync(ParseResult context, CancellationToken can } } - public class MyDerivedHandler : MyBaseHandler + public class MyDerivedCliAction : MyBaseCliAction { private readonly MyService service; - public MyDerivedHandler(MyService service) + public MyDerivedCliAction(MyService service) { this.service = service; } diff --git a/src/System.CommandLine.Hosting/HostingAction.cs b/src/System.CommandLine.Hosting/HostingAction.cs index 8f582336e6..2214d97b15 100644 --- a/src/System.CommandLine.Hosting/HostingAction.cs +++ b/src/System.CommandLine.Hosting/HostingAction.cs @@ -1,10 +1,11 @@ -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Hosting; +using System.CommandLine.Binding; +using System.CommandLine.Invocation; +using System.CommandLine.NamingConventionBinder; using System.Linq; using System.Threading; using System.Threading.Tasks; -using System.CommandLine.NamingConventionBinder; -using System.CommandLine.Binding; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; namespace System.CommandLine.Hosting { @@ -13,11 +14,11 @@ internal sealed class HostingAction : BindingHandler { private readonly Func _hostBuilderFactory; private readonly Action _configureHost; - private readonly CliAction _actualAction; + private readonly AsynchronousCliAction _actualAction; internal static void SetHandlers(CliCommand command, Func hostBuilderFactory, Action configureHost) { - command.Action = new HostingAction(hostBuilderFactory, configureHost, command.Action); + command.Action = new HostingAction(hostBuilderFactory, configureHost, (AsynchronousCliAction)command.Action); command.TreatUnmatchedTokensAsErrors = false; // to pass unmatched Tokens to host builder factory foreach (CliCommand subCommand in command.Subcommands) @@ -26,30 +27,27 @@ internal static void SetHandlers(CliCommand command, Func hostBuilderFactory, Action configureHost, CliAction actualAction) + private HostingAction(Func hostBuilderFactory, Action configureHost, AsynchronousCliAction actualAction) { _hostBuilderFactory = hostBuilderFactory; _configureHost = configureHost; _actualAction = actualAction; } - public override BindingContext GetBindingContext(ParseResult parseResult) + public override BindingContext GetBindingContext(ParseResult parseResult) => _actualAction is BindingHandler bindingHandler - ? bindingHandler.GetBindingContext(parseResult) - : base.GetBindingContext(parseResult); + ? bindingHandler.GetBindingContext(parseResult) + : base.GetBindingContext(parseResult); - public async override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { var argsRemaining = parseResult.UnmatchedTokens; var hostBuilder = _hostBuilderFactory?.Invoke(argsRemaining.ToArray()) - ?? new HostBuilder(); + ?? new HostBuilder(); hostBuilder.Properties[typeof(ParseResult)] = parseResult; CliDirective configurationDirective = parseResult.Configuration.Directives.Single(d => d.Name == "config"); - hostBuilder.ConfigureHostConfiguration(config => - { - config.AddCommandLineDirectives(parseResult, configurationDirective); - }); + hostBuilder.ConfigureHostConfiguration(config => { config.AddCommandLineDirectives(parseResult, configurationDirective); }); var bindingContext = GetBindingContext(parseResult); int registeredBefore = 0; hostBuilder.UseInvocationLifetime(); @@ -88,6 +86,7 @@ public async override Task InvokeAsync(ParseResult parseResult, Cancellatio { return await _actualAction.InvokeAsync(parseResult, cancellationToken); } + return 0; } finally @@ -95,7 +94,5 @@ public async override Task InvokeAsync(ParseResult parseResult, Cancellatio await host.StopAsync(cancellationToken); } } - - public override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult).GetAwaiter().GetResult(); } -} +} \ No newline at end of file diff --git a/src/System.CommandLine.Hosting/HostingExtensions.cs b/src/System.CommandLine.Hosting/HostingExtensions.cs index 3444e24c41..713e25b3f2 100644 --- a/src/System.CommandLine.Hosting/HostingExtensions.cs +++ b/src/System.CommandLine.Hosting/HostingExtensions.cs @@ -1,4 +1,5 @@ using System.CommandLine.Binding; +using System.CommandLine.Invocation; using System.CommandLine.NamingConventionBinder; using CommandHandler = System.CommandLine.NamingConventionBinder.CommandHandler; @@ -54,7 +55,7 @@ public static OptionsBuilder BindCommandLine( public static CliCommand UseCommandHandler(this CliCommand command) where THandler : CliAction { - command.Action = CommandHandler.Create(typeof(THandler).GetMethod(nameof(CliAction.InvokeAsync))); + command.Action = CommandHandler.Create(typeof(THandler).GetMethod(nameof(AsynchronousCliCommandAction.InvokeAsync))); return command; } diff --git a/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs b/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs index d1a59df6b0..488e2a1c65 100644 --- a/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs +++ b/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.CommandLine.Invocation; using System.CommandLine.Tests.Binding; using System.CommandLine.Utility; using System.IO; @@ -87,7 +88,7 @@ public async Task Handler_method_receives_option_arguments_bound_to_the_specifie { var testCase = BindingCases[(type, variation)]; - CliAction handler; + AsynchronousCliAction handler; if (!useDelegate) { var captureMethod = GetType() diff --git a/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs b/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs index 471d1efeba..3788ceccb3 100644 --- a/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs +++ b/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs @@ -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.CommandLine.Invocation; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -397,19 +398,17 @@ void Execute(string fullnameOrNickname, int age) public async Task Method_invoked_is_matching_to_the_interface_implementation(Type type, int expectedResult) { var command = new CliCommand("command"); - command.Action = CommandHandler.Create(type.GetMethod(nameof(CliAction.InvokeAsync))); + command.Action = CommandHandler.Create(type.GetMethod(nameof(AsynchronousCliAction.InvokeAsync))); int result = await command.Parse("command").InvokeAsync(); result.Should().Be(expectedResult); } - public abstract class AbstractTestCommandHandler : CliAction + public abstract class AbstractTestCommandHandler : AsynchronousCliAction { public abstract Task DoJobAsync(); - - public override int Invoke(ParseResult context) => InvokeAsync(context, CancellationToken.None).GetAwaiter().GetResult(); - + public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) => DoJobAsync(); } @@ -420,10 +419,8 @@ public override Task DoJobAsync() => Task.FromResult(42); } - public class VirtualTestCommandHandler : CliAction + public class VirtualTestCommandHandler : AsynchronousCliAction { - public override int Invoke(ParseResult context) => 42; - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) => Task.FromResult(42); } diff --git a/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs b/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs index d72b2647bd..fe707c2347 100644 --- a/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs +++ b/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.CommandLine.Binding; @@ -14,8 +14,6 @@ public static class BindingContextExtensions { private sealed class DummyStateHoldingHandler : BindingHandler { - public override int Invoke(ParseResult parseResult) => 0; - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => Task.FromResult(0); } diff --git a/src/System.CommandLine.NamingConventionBinder/BindingHandler.cs b/src/System.CommandLine.NamingConventionBinder/BindingHandler.cs index 5422c44d5d..558842c266 100644 --- a/src/System.CommandLine.NamingConventionBinder/BindingHandler.cs +++ b/src/System.CommandLine.NamingConventionBinder/BindingHandler.cs @@ -1,8 +1,12 @@ -using System.CommandLine.Binding; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.CommandLine.Binding; +using System.CommandLine.Invocation; namespace System.CommandLine.NamingConventionBinder { - public abstract class BindingHandler : CliAction + public abstract class BindingHandler : AsynchronousCliAction { private BindingContext? _bindingContext; diff --git a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs index 7f7aa88180..a95fca3d10 100644 --- a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs +++ b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs @@ -129,7 +129,4 @@ private void BindValueSource(ParameterInfo param, IValueSource valueSource) : _methodDescriptor.ParameterDescriptors .FirstOrDefault(x => x.ValueName == param.Name && x.ValueType == param.ParameterType); - - /// - public override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult, CancellationToken.None).GetAwaiter().GetResult(); } \ No newline at end of file diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index ed25bd359e..93bda02238 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.CommandLine.Invocation; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -67,14 +68,21 @@ public async Task Multiple_instances_of_the_same_directive_can_be_invoked(bool i var commandActionWasCalled = false; var directiveCallCount = 0; + Action incrementCallCount = _ => directiveCallCount++; + Action verifyActionWasCalled = _ => commandActionWasCalled = true; + var testDirective = new TestDirective("test") { - Action = new NonexclusiveTestAction(_ => directiveCallCount++) + Action = invokeAsync + ? new AsynchronousNonexclusiveTestAction(incrementCallCount) + : new SynchronousNonexclusiveTestAction(incrementCallCount) }; var config = new CliConfiguration(new CliRootCommand { - Action = new NonexclusiveTestAction(_ => commandActionWasCalled = true) + Action = invokeAsync + ? new AsynchronousNonexclusiveTestAction(verifyActionWasCalled) + : new SynchronousNonexclusiveTestAction(verifyActionWasCalled) }) { Directives = { testDirective } @@ -106,15 +114,15 @@ public async Task Multiple_different_directives_can_be_invoked(bool invokeAsync) var directiveOne = new TestDirective("one") { - Action = new NonexclusiveTestAction(_ => directiveOneActionWasCalled = true) + Action = new SynchronousNonexclusiveTestAction(_ => directiveOneActionWasCalled = true) }; var directiveTwo = new TestDirective("two") { - Action = new NonexclusiveTestAction(_ => directiveTwoActionWasCalled = true) + Action = new SynchronousNonexclusiveTestAction(_ => directiveTwoActionWasCalled = true) }; var config = new CliConfiguration(new CliRootCommand { - Action = new NonexclusiveTestAction(_ => commandActionWasCalled = true) + Action = new SynchronousNonexclusiveTestAction(_ => commandActionWasCalled = true) }) { Directives = { directiveOne, directiveTwo } @@ -143,11 +151,11 @@ public TestDirective(string name) : base(name) } } - private class NonexclusiveTestAction : CliAction + private class SynchronousNonexclusiveTestAction : SynchronousCliAction { private readonly Action _invoke; - public NonexclusiveTestAction(Action invoke) + public SynchronousNonexclusiveTestAction(Action invoke) { _invoke = invoke; Exclusive = false; @@ -158,11 +166,22 @@ public override int Invoke(ParseResult parseResult) _invoke(parseResult); return 0; } + } + + private class AsynchronousNonexclusiveTestAction : AsynchronousCliAction + { + private readonly Action _invoke; + + public AsynchronousNonexclusiveTestAction(Action invoke) + { + _invoke = invoke; + Exclusive = false; + } public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { - ; - return Task.FromResult(Invoke(parseResult)); + _invoke(parseResult); + return Task.FromResult(0); } } diff --git a/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs b/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs index cb47806f31..77eb55dd4b 100644 --- a/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs +++ b/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs @@ -1,4 +1,5 @@ using System.CommandLine.Help; +using System.CommandLine.Invocation; using FluentAssertions; using System.Linq; using System.Threading; @@ -126,7 +127,7 @@ public void It_does_not_prevent_help_from_being_invoked() Environment.GetEnvironmentVariable(_testVariableName).Should().Be("1"); } - private class CustomHelpAction : CliAction + private class CustomHelpAction : SynchronousCliAction { public bool WasCalled { get; private set; } @@ -135,12 +136,6 @@ public override int Invoke(ParseResult parseResult) WasCalled = true; return 0; } - - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - { - WasCalled = true; - return Task.FromResult(0); - } } } } diff --git a/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs b/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs index 6815736886..84a1b85682 100644 --- a/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.CommandLine.Invocation; using FluentAssertions; using System.CommandLine.Tests.Utility; using System.Diagnostics; @@ -61,11 +62,9 @@ private static Task Program(string[] args) }.InvokeAsync(args); } - private sealed class CustomCliAction : CliAction + private sealed class CustomCliAction : AsynchronousCliAction { - public override int Invoke(ParseResult context) => throw new NotImplementedException(); - - public async override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) + public override async Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) { Console.WriteLine(ChildProcessWaiting); diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index c76f3c0922..4059731180 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -4,6 +4,7 @@ using System; using System.CommandLine; using System.CommandLine.Help; +using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.IO; using System.Threading; @@ -81,7 +82,7 @@ public void RootCommand_Invoke_returns_0_when_handler_is_successful() var wasCalled = false; var rootCommand = new CliRootCommand(); - rootCommand.SetAction((_) => wasCalled = true); + rootCommand.SetAction(_ => wasCalled = true); int result = rootCommand.Parse("").Invoke(); @@ -136,7 +137,7 @@ public void Custom_RootCommand_Action_can_set_custom_result_code_via_Invoke() { var rootCommand = new CliRootCommand { - Action = new CustomReturnCodeAction() + Action = new SynchronousCustomReturnCodeAction() }; rootCommand.Parse("").Invoke().Should().Be(123); @@ -147,7 +148,7 @@ public async Task Custom_RootCommand_Action_can_set_custom_result_code_via_Invok { var rootCommand = new CliRootCommand { - Action = new CustomReturnCodeAction() + Action = new AsynchronousCustomReturnCodeAction() }; (await rootCommand.Parse("").InvokeAsync()).Should().Be(456); @@ -247,7 +248,7 @@ public void When_multiple_options_with_actions_are_present_then_only_the_last_on optionAction3.WasCalled.Should().BeFalse(); } - internal sealed class OptionAction : CliAction + internal sealed class OptionAction : SynchronousCliAction { internal bool WasCalled = false; @@ -256,9 +257,6 @@ public override int Invoke(ParseResult context) WasCalled = true; return 0; } - - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) - => Task.FromResult(Invoke(context)); } [Theory] @@ -310,16 +308,19 @@ public async Task Command_InvokeAsync_with_cancelation_token_invokes_command_han await command.Parse("test").InvokeAsync(cancellationToken: cts.Token); } - private class CustomReturnCodeAction : CliAction + private class SynchronousCustomReturnCodeAction : SynchronousCliAction { public override int Invoke(ParseResult context) => 123; + } + private class AsynchronousCustomReturnCodeAction : AsynchronousCliAction + { public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) => Task.FromResult(456); } - private class NonexclusiveTestAction : CliAction + private class NonexclusiveTestAction : SynchronousCliAction { public NonexclusiveTestAction() { @@ -340,17 +341,6 @@ public override int Invoke(ParseResult parseResult) return 0; } - - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - { - HasBeenInvoked = true; - if (ThrowOnInvoke) - { - throw new Exception("oops!"); - } - - return Task.FromResult(0); - } } } } diff --git a/src/System.CommandLine.Tests/ParseResultTests.cs b/src/System.CommandLine.Tests/ParseResultTests.cs index 73be24267c..b8f9948e93 100644 --- a/src/System.CommandLine.Tests/ParseResultTests.cs +++ b/src/System.CommandLine.Tests/ParseResultTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.CommandLine.Invocation; using System.Linq; using System.CommandLine.Parsing; using FluentAssertions; @@ -156,7 +157,7 @@ public void Handler_is_not_null_when_parsed_command_specified_handler() parseResult.Action.Should().NotBeNull(); handlerWasCalled.Should().BeFalse(); - parseResult.Action.Invoke(null!).Should().Be(0); + ((SynchronousCliAction)parseResult.Action!).Invoke(null!).Should().Be(0); handlerWasCalled.Should().BeTrue(); } } diff --git a/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs b/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs index d423e3b0a0..1d14455333 100644 --- a/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs +++ b/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs @@ -1,4 +1,5 @@ -using System.IO; +using System.CommandLine.Invocation; +using System.IO; namespace System.CommandLine.Tests { @@ -11,7 +12,7 @@ internal static string Diagram(this ParseResult parseResult) try { parseResult.Configuration.Output = new StringWriter(); - new DiagramDirective().Action.Invoke(parseResult); + ((SynchronousCliAction)new DiagramDirective().Action!).Invoke(parseResult); return parseResult.Configuration.Output.ToString() .TrimEnd(); // the directive adds a new line, tests that used to rely on Diagram extension method don't expect it } diff --git a/src/System.CommandLine/CliAction.cs b/src/System.CommandLine/CliAction.cs deleted file mode 100644 index 1f2c41eb43..0000000000 --- a/src/System.CommandLine/CliAction.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Threading; -using System.Threading.Tasks; - -namespace System.CommandLine -{ - /// - /// Defines the behavior of a symbol. - /// - public abstract class CliAction - { - // FIX: (CliAction) change this to accommodate the fact that the core action isn't "exclusive" either, and some actions might eventually be created that run after it. - public bool Exclusive { get; protected set; } = true; - - /// - /// Performs an action when the associated symbol is invoked on the command line. - /// - /// Provides the parse results. - /// A value that can be used as the exit code for the process. - public abstract int Invoke(ParseResult parseResult); - - /// - /// Performs an action when the associated symbol is invoked on the command line. - /// - /// Provides the parse results. - /// The token to monitor for cancellation requests. - /// A value that can be used as the exit code for the process. - public abstract Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default); - } -} diff --git a/src/System.CommandLine/CliCommand.cs b/src/System.CommandLine/CliCommand.cs index 04a78c04e4..26f5b57193 100644 --- a/src/System.CommandLine/CliCommand.cs +++ b/src/System.CommandLine/CliCommand.cs @@ -110,7 +110,7 @@ public void SetAction(Action action) throw new ArgumentNullException(nameof(action)); } - Action = new CliCommandAction(context => + Action = new SynchronousCliCommandAction(context => { action(context); return 0; @@ -128,7 +128,7 @@ public void SetAction(Func action) throw new ArgumentNullException(nameof(action)); } - Action = new CliCommandAction(action); + Action = new SynchronousCliCommandAction(action); } /// @@ -141,7 +141,7 @@ public void SetAction(Func action) throw new ArgumentNullException(nameof(action)); } - Action = new CliCommandAction(async (context, cancellationToken) => + Action = new AsynchronousCliCommandAction(async (context, cancellationToken) => { await action(context, cancellationToken); return 0; @@ -159,7 +159,7 @@ public void SetAction(Func> action) throw new ArgumentNullException(nameof(action)); } - Action = new CliCommandAction(action); + Action = new AsynchronousCliCommandAction(action); } /// diff --git a/src/System.CommandLine/CliDirective.cs b/src/System.CommandLine/CliDirective.cs index 01072c0e2e..04024beca8 100644 --- a/src/System.CommandLine/CliDirective.cs +++ b/src/System.CommandLine/CliDirective.cs @@ -1,5 +1,9 @@ -using System.Collections.Generic; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; using System.CommandLine.Completions; +using System.CommandLine.Invocation; namespace System.CommandLine { diff --git a/src/System.CommandLine/CliOption.cs b/src/System.CommandLine/CliOption.cs index 00151f2ff4..d865411f98 100644 --- a/src/System.CommandLine/CliOption.cs +++ b/src/System.CommandLine/CliOption.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.CommandLine.Completions; +using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.Linq; @@ -116,11 +117,6 @@ internal virtual bool Greedy /// public override IEnumerable GetCompletions(CompletionContext context) { - if (Argument is null) - { - return Array.Empty(); - } - List? completions = null; foreach (var completion in Argument.GetCompletions(context)) diff --git a/src/System.CommandLine/Completions/SuggestDirective.cs b/src/System.CommandLine/Completions/SuggestDirective.cs index 1ef76c31dd..7e6ab2adf4 100644 --- a/src/System.CommandLine/Completions/SuggestDirective.cs +++ b/src/System.CommandLine/Completions/SuggestDirective.cs @@ -1,7 +1,6 @@ -using System.Linq; +using System.CommandLine.Invocation; using System.CommandLine.Parsing; -using System.Threading.Tasks; -using System.Threading; +using System.Linq; namespace System.CommandLine.Completions { @@ -24,9 +23,9 @@ public override CliAction? Action set => _action = value ?? throw new ArgumentNullException(nameof(value)); } - private sealed class SuggestDirectiveAction : CliAction + private sealed class SuggestDirectiveAction : SynchronousCliAction { - // FIX: (SuggestDirectiveAction) rename, make public + // FIX: (SuggestDirectiveAction) rename (CompletionAction?), make public private readonly SuggestDirective _directive; internal SuggestDirectiveAction(SuggestDirective suggestDirective) => _directive = suggestDirective; @@ -51,11 +50,6 @@ public override int Invoke(ParseResult parseResult) return 0; } - - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - => cancellationToken.IsCancellationRequested - ? Task.FromCanceled(cancellationToken) - : Task.FromResult(Invoke(parseResult)); } } -} +} \ No newline at end of file diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index b47aa33509..1b00c53669 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -1,6 +1,8 @@ -using System.CommandLine.Parsing; -using System.Threading; -using System.Threading.Tasks; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; namespace System.CommandLine { @@ -22,7 +24,7 @@ public override CliAction? Action set => _action = value ?? throw new ArgumentNullException(nameof(value)); } - private sealed class EnvironmentVariablesDirectiveAction : CliAction + private sealed class EnvironmentVariablesDirectiveAction : SynchronousCliAction { private readonly EnvironmentVariablesDirective _directive; @@ -39,13 +41,6 @@ public override int Invoke(ParseResult parseResult) return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - { - SetEnvVars(parseResult); - - return Task.FromResult(0); - } - private void SetEnvVars(ParseResult parseResult) { DirectiveResult directiveResult = parseResult.GetResult(_directive)!; diff --git a/src/System.CommandLine/Help/HelpOption.cs b/src/System.CommandLine/Help/HelpOption.cs index 5774120727..03c42859da 100644 --- a/src/System.CommandLine/Help/HelpOption.cs +++ b/src/System.CommandLine/Help/HelpOption.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.CommandLine.Invocation; + namespace System.CommandLine.Help { public sealed class HelpOption : CliOption diff --git a/src/System.CommandLine/Help/HelpOptionAction.cs b/src/System.CommandLine/Help/HelpOptionAction.cs index 185a6fc94f..ec9edeabb3 100644 --- a/src/System.CommandLine/Help/HelpOptionAction.cs +++ b/src/System.CommandLine/Help/HelpOptionAction.cs @@ -1,9 +1,8 @@ -using System.Threading; -using System.Threading.Tasks; +using System.CommandLine.Invocation; namespace System.CommandLine.Help { - public sealed class HelpAction : CliAction + public sealed class HelpAction : SynchronousCliAction { private HelpBuilder? _builder; @@ -29,10 +28,5 @@ public override int Invoke(ParseResult parseResult) return 0; } - - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - => cancellationToken.IsCancellationRequested - ? Task.FromCanceled(cancellationToken) - : Task.FromResult(Invoke(parseResult)); } -} +} \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/CliCommandAction.cs b/src/System.CommandLine/Invocation/CliCommandAction.cs index 57b588c9c3..ce29fc47da 100644 --- a/src/System.CommandLine/Invocation/CliCommandAction.cs +++ b/src/System.CommandLine/Invocation/CliCommandAction.cs @@ -4,44 +4,62 @@ using System.Threading; using System.Threading.Tasks; -namespace System.CommandLine.Invocation +namespace System.CommandLine.Invocation; + +// FIX: separate files + +public abstract class CliAction { - public class CliCommandAction : CliAction + private protected CliAction() { - private readonly Func>? _asyncAction; - private readonly Func? _syncAction; - - internal CliCommandAction(Func action) - => _syncAction = action; - - internal CliCommandAction(Func> action) - => _asyncAction = action; - - public override int Invoke(ParseResult parseResult) - { - if (_syncAction is not null) - { - return _syncAction(parseResult); - } - else - { - return SyncUsingAsync(parseResult); // kept in a separate method to avoid JITting - } - - int SyncUsingAsync(ParseResult parseResult) - => _asyncAction!(parseResult, CancellationToken.None).GetAwaiter().GetResult(); - } - - public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken) - { - if (_asyncAction is not null) - { - return await _asyncAction(parseResult, cancellationToken); - } - else - { - return _syncAction!(parseResult); - } - } } + + // FIX: (CliAction) change this to accommodate the fact that the core action isn't "exclusive" either, and some actions might eventually be created that run after it. + public bool Exclusive { get; protected set; } = true; +} + +/// +/// Defines the behavior of a symbol. +/// +public abstract class SynchronousCliAction : CliAction +{ + /// + /// Performs an action when the associated symbol is invoked on the command line. + /// + /// Provides the parse results. + /// A value that can be used as the exit code for the process. + public abstract int Invoke(ParseResult parseResult); +} + +public abstract class AsynchronousCliAction : CliAction +{ + /// + /// Performs an action when the associated symbol is invoked on the command line. + /// + /// Provides the parse results. + /// The token to monitor for cancellation requests. + /// A value that can be used as the exit code for the process. + public abstract Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default); +} + +public class SynchronousCliCommandAction : SynchronousCliAction +{ + private readonly Func _syncAction; + + internal SynchronousCliCommandAction(Func action) + => _syncAction = action; + + public override int Invoke(ParseResult parseResult) => + _syncAction(parseResult); +} + +public class AsynchronousCliCommandAction : AsynchronousCliAction +{ + private readonly Func> _asyncAction; + + internal AsynchronousCliCommandAction(Func> action) + => _asyncAction = action; + + public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken) => + _asyncAction(parseResult, cancellationToken); } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index a9b6d90feb..4c5b1e9493 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -24,11 +24,32 @@ internal static async Task InvokeAsync(ParseResult parseResult, Cancellatio { for (int i = 0; i < parseResult.NonexclusiveActions.Count; i++) { - await parseResult.NonexclusiveActions[i].InvokeAsync(parseResult, cts.Token); + var action = parseResult.NonexclusiveActions[i]; + + switch (action) + { + case SynchronousCliAction syncAction: + syncAction.Invoke(parseResult); + break; + case AsynchronousCliAction asyncAction: + await asyncAction.InvokeAsync(parseResult, cts.Token); + break; + } } } - Task startedInvocation = parseResult.Action.InvokeAsync(parseResult, cts.Token); + Task startedInvocation; + switch (parseResult.Action) + { + case SynchronousCliAction syncAction: + startedInvocation = Task.FromResult(syncAction.Invoke(parseResult)); + break; + case AsynchronousCliAction asyncAction: + startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token); + break; + default: + throw new ArgumentOutOfRangeException(nameof(parseResult.Action)); + } if (parseResult.Configuration.ProcessTerminationTimeout.HasValue) { @@ -71,11 +92,27 @@ internal static int Invoke(ParseResult parseResult) { for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) { - parseResult.NonexclusiveActions[i].Invoke(parseResult); + var action = parseResult.NonexclusiveActions[i]; + + switch (action) + { + case SynchronousCliAction syncAction: + syncAction.Invoke(parseResult); + break; + case AsynchronousCliAction _: + throw new InvalidOperationException($"{nameof(AsynchronousCliAction)} called within non-async invocation."); + } } } - return parseResult.Action.Invoke(parseResult); + switch (parseResult.Action) + { + case SynchronousCliAction syncAction: + return syncAction.Invoke(parseResult); + break; + default: + throw new InvalidOperationException($"{nameof(AsynchronousCliAction)} called within non-async invocation."); + } } catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) { diff --git a/src/System.CommandLine/Invocation/ParseErrorAction.cs b/src/System.CommandLine/Invocation/ParseErrorAction.cs index 8a13a09958..1a35fcde20 100644 --- a/src/System.CommandLine/Invocation/ParseErrorAction.cs +++ b/src/System.CommandLine/Invocation/ParseErrorAction.cs @@ -2,38 +2,27 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.CommandLine.Help; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -namespace System.CommandLine.Invocation +namespace System.CommandLine.Invocation; + +internal sealed class ParseErrorAction : SynchronousCliAction { - internal sealed class ParseErrorAction : CliAction + public override int Invoke(ParseResult parseResult) { - public override int Invoke(ParseResult parseResult) - { - ConsoleHelpers.ResetTerminalForegroundColor(); - ConsoleHelpers.SetTerminalForegroundRed(); - - foreach (var error in parseResult.Errors) - { - parseResult.Configuration.Error.WriteLine(error.Message); - } - - parseResult.Configuration.Error.WriteLine(); + ConsoleHelpers.ResetTerminalForegroundColor(); + ConsoleHelpers.SetTerminalForegroundRed(); - ConsoleHelpers.ResetTerminalForegroundColor(); + foreach (var error in parseResult.Errors) + { + parseResult.Configuration.Error.WriteLine(error.Message); + } - HelpOption helpOption = parseResult.RootCommandResult.Command.Options.FirstOrDefault(option => option is HelpOption) as HelpOption ?? new HelpOption(); + parseResult.Configuration.Error.WriteLine(); - helpOption.Action!.Invoke(parseResult); + ConsoleHelpers.ResetTerminalForegroundColor(); - return 1; - } + new HelpAction().Invoke(parseResult); - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - => cancellationToken.IsCancellationRequested - ? Task.FromCanceled(cancellationToken) - : Task.FromResult(Invoke(parseResult)); + return 1; } } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs b/src/System.CommandLine/Invocation/TypoCorrectionAction.cs index b26e42d3cb..1f2c1a2473 100644 --- a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs +++ b/src/System.CommandLine/Invocation/TypoCorrectionAction.cs @@ -3,12 +3,10 @@ using System.Collections.Generic; using System.Linq; -using System.Threading; -using System.Threading.Tasks; namespace System.CommandLine.Invocation { - internal sealed class TypoCorrectionAction : CliAction + internal sealed class TypoCorrectionAction : SynchronousCliAction { // FIX: (TypoCorrectionAction) make this part of ParseErrorResultAction private const int MaxLevenshteinDistance = 3; @@ -16,11 +14,6 @@ internal sealed class TypoCorrectionAction : CliAction public override int Invoke(ParseResult parseResult) => ProvideSuggestions(parseResult); - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - => cancellationToken.IsCancellationRequested - ? Task.FromCanceled(cancellationToken) - : Task.FromResult(ProvideSuggestions(parseResult)); - private static int ProvideSuggestions(ParseResult result) { var unmatchedTokens = result.UnmatchedTokens; @@ -52,46 +45,49 @@ private static IEnumerable GetPossibleTokens(CliCommand targetSymbol, st } IEnumerable possibleMatches = targetSymbol - .Children - .Where(x => !x.Hidden && x is CliOption or CliCommand) - .Select(symbol => - { - AliasSet? aliasSet = symbol is CliOption option ? option._aliases : ((CliCommand)symbol)._aliases; + .Children + .Where(x => !x.Hidden && x is CliOption or CliCommand) + .Select(symbol => + { + AliasSet? aliasSet = symbol is CliOption option ? option._aliases : ((CliCommand)symbol)._aliases; + + if (aliasSet is null) + { + return symbol.Name; + } + + return new[] { symbol.Name }.Concat(aliasSet) + .OrderBy(x => GetDistance(token, x)) + .ThenByDescending(x => GetStartsWithDistance(token, x)) + .First(); + }); - if (aliasSet is null) - { - return symbol.Name; - } - - return new[] { symbol.Name }.Concat(aliasSet) - .OrderBy(x => GetDistance(token, x)) - .ThenByDescending(x => GetStartsWithDistance(token, x)) - .First(); - }); - int? bestDistance = null; return possibleMatches - .Select(possibleMatch => (possibleMatch, distance:GetDistance(token, possibleMatch))) - .Where(tuple => tuple.distance <= MaxLevenshteinDistance) - .OrderBy(tuple => tuple.distance) - .ThenByDescending(tuple => GetStartsWithDistance(token, tuple.possibleMatch)) - .TakeWhile(tuple => - { - var (_, distance) = tuple; - if (bestDistance is null) - { - bestDistance = distance; - } - return distance == bestDistance; - }) - .Select(tuple => tuple.possibleMatch); + .Select(possibleMatch => (possibleMatch, distance: GetDistance(token, possibleMatch))) + .Where(tuple => tuple.distance <= MaxLevenshteinDistance) + .OrderBy(tuple => tuple.distance) + .ThenByDescending(tuple => GetStartsWithDistance(token, tuple.possibleMatch)) + .TakeWhile(tuple => + { + var (_, distance) = tuple; + if (bestDistance is null) + { + bestDistance = distance; + } + + return distance == bestDistance; + }) + .Select(tuple => tuple.possibleMatch); } private static int GetStartsWithDistance(string first, string second) { int i; for (i = 0; i < first.Length && i < second.Length && first[i] == second[i]; i++) - { } + { + } + return i; } @@ -109,7 +105,6 @@ private static int GetDistance(string first, string second) throw new ArgumentNullException(nameof(second)); } - // Get the length of both. If either is 0, return // the length of the other, since that number of insertions // would be required. @@ -118,7 +113,6 @@ private static int GetDistance(string first, string second) if (n == 0) return m; if (m == 0) return n; - // Rather than maintain an entire matrix (which would require O(n*m) space), // just store the current row and the next row, each of which has a length m+1, // so just O(m) space. Initialize the current row. @@ -145,7 +139,6 @@ private static int GetDistance(string first, string second) rows[nextRow][j] = Math.Min(dist1, Math.Min(dist2, dist3)); } - // Swap the current and next rows if (curRow == 0) { @@ -163,4 +156,4 @@ private static int GetDistance(string first, string second) return rows[curRow][m]; } } -} +} \ No newline at end of file diff --git a/src/System.CommandLine/ParseDiagramDirective.cs b/src/System.CommandLine/ParseDiagramDirective.cs index b050cb1935..d3f5d61ead 100644 --- a/src/System.CommandLine/ParseDiagramDirective.cs +++ b/src/System.CommandLine/ParseDiagramDirective.cs @@ -1,4 +1,5 @@ -using System.CommandLine.Parsing; +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; namespace System.CommandLine { diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 84d88bccaa..062a44d5b4 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -297,7 +297,36 @@ public Task InvokeAsync(CancellationToken cancellationToken = default) /// Invokes the appropriate command handler for a parsed command line input. /// /// A value that can be used as a process exit code. - public int Invoke() => InvocationPipeline.Invoke(this); + public int Invoke() + { + var useAsync = false; + + if (Action is AsynchronousCliAction) + { + useAsync = true; + } + else if (NonexclusiveActions is not null) + { + for (var i = 0; i < NonexclusiveActions.Count; i++) + { + var action = NonexclusiveActions[i]; + if (action is AsynchronousCliAction) + { + useAsync = true; + break; + } + } + } + + if (useAsync) + { + return InvocationPipeline.InvokeAsync(this, CancellationToken.None).GetAwaiter().GetResult(); + } + else + { + return InvocationPipeline.Invoke(this); + } + } /// /// Gets the for parsed result. The handler represents the action diff --git a/src/System.CommandLine/Parsing/ArgumentResult.cs b/src/System.CommandLine/Parsing/ArgumentResult.cs index f7a793aff1..dec2ea3008 100644 --- a/src/System.CommandLine/Parsing/ArgumentResult.cs +++ b/src/System.CommandLine/Parsing/ArgumentResult.cs @@ -29,9 +29,6 @@ internal ArgumentResult( internal bool ArgumentLimitReached => Argument.Arity.MaximumNumberOfValues == (_tokens?.Count ?? 0); - // FIX: (ArgumentResult.Implicit) this can probably be removed - internal bool Implicit => Argument.HasDefaultValue && Tokens.Count == 0; - internal ArgumentConversionResult GetArgumentConversionResult() => _conversionResult ??= ValidateAndConvert(useValidators: true); diff --git a/src/System.CommandLine/Parsing/ParseDiagramAction.cs b/src/System.CommandLine/Parsing/ParseDiagramAction.cs index ccdf486562..4836ac64b3 100644 --- a/src/System.CommandLine/Parsing/ParseDiagramAction.cs +++ b/src/System.CommandLine/Parsing/ParseDiagramAction.cs @@ -3,6 +3,7 @@ using System.Collections; using System.CommandLine.Binding; +using System.CommandLine.Invocation; using System.Linq; using System.Text; using System.Threading.Tasks; @@ -14,7 +15,7 @@ namespace System.CommandLine.Parsing /// Implements the [diagram] directive action, which when specified on the command line /// will short circuit normal command handling and display a diagram explaining the parse result for the command line input. /// - internal sealed class DiagramAction : CliAction + internal sealed class DiagramAction : SynchronousCliAction { private readonly int _parseErrorReturnValue; @@ -25,21 +26,7 @@ public override int Invoke(ParseResult parseResult) parseResult.Configuration.Output.WriteLine(Diagram(parseResult)); return parseResult.Errors.Count == 0 ? 0 : _parseErrorReturnValue; } - - public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - { - cancellationToken.ThrowIfCancellationRequested(); - - StringBuilder diagram = Diagram(parseResult); - -#if NET7_0_OR_GREATER - await parseResult.Configuration.Output.WriteLineAsync(diagram, cancellationToken); -#else - await parseResult.Configuration.Output.WriteLineAsync(diagram.ToString()); -#endif - return parseResult.Errors.Count == 0 ? 0 : _parseErrorReturnValue; - } - + /// /// Formats a string explaining a parse result. /// diff --git a/src/System.CommandLine/VersionOption.cs b/src/System.CommandLine/VersionOption.cs index d6f35c5b1f..85ece7faae 100644 --- a/src/System.CommandLine/VersionOption.cs +++ b/src/System.CommandLine/VersionOption.cs @@ -1,10 +1,9 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.Linq; -using System.Threading; -using System.Threading.Tasks; namespace System.CommandLine { @@ -41,38 +40,22 @@ private void AddValidators() Validators.Add(static result => { if (result.Parent is CommandResult parent && - parent.Children.Where(r => !(r is OptionResult optionResult && optionResult.Option is VersionOption)) - .Any(NotImplicit)) + parent.Children.Any(r => r is not OptionResult { Option: VersionOption })) { result.AddError(LocalizationResources.VersionOptionCannotBeCombinedWithOtherArguments(result.IdentifierToken?.Value ?? result.Option.Name)); } }); } - private static bool NotImplicit(SymbolResult symbolResult) - { - return symbolResult switch - { - ArgumentResult argumentResult => !argumentResult.Implicit, - OptionResult optionResult => !optionResult.Implicit, - _ => true - }; - } - internal override bool Greedy => false; - private sealed class VersionOptionAction : CliAction + private sealed class VersionOptionAction : SynchronousCliAction { public override int Invoke(ParseResult parseResult) { parseResult.Configuration.Output.WriteLine(CliRootCommand.ExecutableVersion); return 0; } - - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - => cancellationToken.IsCancellationRequested - ? Task.FromCanceled(cancellationToken) - : Task.FromResult(Invoke(parseResult)); } } } \ No newline at end of file From 7cc0dfa5f1b34f2fcadf320797c65dab922799cb Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Fri, 2 Jun 2023 10:27:11 -0700 Subject: [PATCH 04/19] minor cleanups --- src/System.CommandLine.Tests/VersionOptionTests.cs | 6 +++--- src/System.CommandLine/CliConfiguration.cs | 1 + src/System.CommandLine/CliDirective.cs | 1 + src/System.CommandLine/Invocation/CliCommandAction.cs | 2 ++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine.Tests/VersionOptionTests.cs b/src/System.CommandLine.Tests/VersionOptionTests.cs index 2537e873eb..2bf2205fab 100644 --- a/src/System.CommandLine.Tests/VersionOptionTests.cs +++ b/src/System.CommandLine.Tests/VersionOptionTests.cs @@ -111,13 +111,13 @@ public async Task When_the_version_option_is_specified_and_there_are_default_arg public void Version_is_not_valid_with_other_tokens(string commandLine) { var subcommand = new CliCommand("subcommand"); - subcommand.SetAction((_) => { }); + subcommand.SetAction(_ => { }); var rootCommand = new CliRootCommand { subcommand, - new CliOption("-x"), + new CliOption("-x") }; - rootCommand.SetAction((_) => { }); + rootCommand.SetAction(_ => { }); CliConfiguration configuration = new(rootCommand) { diff --git a/src/System.CommandLine/CliConfiguration.cs b/src/System.CommandLine/CliConfiguration.cs index 6a2fc93dfc..e6e3b05891 100644 --- a/src/System.CommandLine/CliConfiguration.cs +++ b/src/System.CommandLine/CliConfiguration.cs @@ -8,6 +8,7 @@ using System.Threading; using System.IO; using System.CommandLine.Completions; +using System.CommandLine.Invocation; namespace System.CommandLine { diff --git a/src/System.CommandLine/CliDirective.cs b/src/System.CommandLine/CliDirective.cs index 04024beca8..cb7930f5fe 100644 --- a/src/System.CommandLine/CliDirective.cs +++ b/src/System.CommandLine/CliDirective.cs @@ -33,6 +33,7 @@ public CliDirective(string name) /// public virtual CliAction? Action { get; set; } + /// public override IEnumerable GetCompletions(CompletionContext context) => Array.Empty(); } diff --git a/src/System.CommandLine/Invocation/CliCommandAction.cs b/src/System.CommandLine/Invocation/CliCommandAction.cs index ce29fc47da..a2d5d4d161 100644 --- a/src/System.CommandLine/Invocation/CliCommandAction.cs +++ b/src/System.CommandLine/Invocation/CliCommandAction.cs @@ -49,6 +49,7 @@ public class SynchronousCliCommandAction : SynchronousCliAction internal SynchronousCliCommandAction(Func action) => _syncAction = action; + /// public override int Invoke(ParseResult parseResult) => _syncAction(parseResult); } @@ -60,6 +61,7 @@ public class AsynchronousCliCommandAction : AsynchronousCliAction internal AsynchronousCliCommandAction(Func> action) => _asyncAction = action; + /// public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken) => _asyncAction(parseResult, cancellationToken); } \ No newline at end of file From f6607662a58bc8bcb0142b8cd3e17110b90f73db Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Fri, 2 Jun 2023 13:21:35 -0700 Subject: [PATCH 05/19] Move typo corrections into ParseErrorAction, make it public This also adds properties to the ParseErrorAction to allow toggling help and typo suggestions when there's a parse error. --- .../Invocation/InvocationTests.cs | 8 +- .../Invocation/TypoCorrectionTests.cs | 34 +++- .../UseParseErrorReportingTests.cs | 25 +-- .../VersionOptionTests.cs | 6 +- src/System.CommandLine/CliConfiguration.cs | 5 - .../Help/HelpOptionAction.cs | 1 + .../Invocation/InvocationPipeline.cs | 59 +++--- .../Invocation/ParseErrorAction.cs | 174 +++++++++++++++++- .../Invocation/ProcessTerminationHandler.cs | 3 + .../Invocation/TypoCorrectionAction.cs | 159 ---------------- .../Parsing/ParseDiagramAction.cs | 30 ++- .../Parsing/ParseOperation.cs | 8 +- 12 files changed, 267 insertions(+), 245 deletions(-) delete mode 100644 src/System.CommandLine/Invocation/TypoCorrectionAction.cs diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index 4059731180..328e0416c8 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System; -using System.CommandLine; using System.CommandLine.Help; using System.CommandLine.Invocation; using System.CommandLine.Parsing; @@ -96,7 +94,7 @@ public async Task RootCommand_InvokeAsync_returns_1_when_handler_throws() var wasCalled = false; var rootCommand = new CliRootCommand(); - rootCommand.SetAction((_, __) => + rootCommand.SetAction((_, _) => { wasCalled = true; @@ -115,7 +113,7 @@ public void RootCommand_Invoke_returns_1_when_handler_throws() var wasCalled = false; var rootCommand = new CliRootCommand(); - rootCommand.SetAction((_, __) => + rootCommand.SetAction((_, _) => { wasCalled = true; throw new Exception("oops!"); @@ -292,7 +290,7 @@ public async Task Nonexclusive_actions_handle_exceptions_and_return_an_error_ret returnCode.Should().Be(1); } - + [Fact] public async Task Command_InvokeAsync_with_cancelation_token_invokes_command_handler() { diff --git a/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs b/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs index 4194497673..6aef6c2ec3 100644 --- a/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs +++ b/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs @@ -1,3 +1,4 @@ +using System.CommandLine.Invocation; using System.IO; using System.Threading.Tasks; using FluentAssertions; @@ -18,7 +19,6 @@ public async Task When_option_is_mistyped_it_is_suggested() CliConfiguration config = new(rootCommand) { - EnableTypoCorrections = true, Output = new StringWriter() }; @@ -29,6 +29,31 @@ public async Task When_option_is_mistyped_it_is_suggested() config.Output.ToString().Should().Contain($"'niof' was not matched. Did you mean one of the following?{NewLine}info"); } + [Fact] + public async Task Typo_corrections_can_be_disabled() + { + CliRootCommand rootCommand = new() + { + new CliOption("info") + }; + + CliConfiguration config = new(rootCommand) + { + Output = new StringWriter() + }; + + var result = rootCommand.Parse("niof", config); + + if (result.Action is ParseErrorAction parseError) + { + parseError.ShowTypoCorrections = false; + } + + await result.InvokeAsync(); + + config.Output.ToString().Should().NotContain("Did you mean"); + } + [Fact] public async Task When_there_are_no_matches_then_nothing_is_suggested() { @@ -37,7 +62,6 @@ public async Task When_there_are_no_matches_then_nothing_is_suggested() CliConfiguration configuration = new(rootCommand) { - EnableTypoCorrections = true, Output = new StringWriter() }; @@ -56,7 +80,6 @@ public async Task When_command_is_mistyped_it_is_suggested() CliConfiguration configuration = new(rootCommand) { - EnableTypoCorrections = true, Output = new StringWriter() }; @@ -83,7 +106,6 @@ public async Task When_there_are_multiple_matches_it_picks_the_best_matches() }; CliConfiguration configuration = new(rootCommand) { - EnableTypoCorrections = true, Output = new StringWriter() }; @@ -109,7 +131,6 @@ public async Task Hidden_commands_are_not_suggested() CliConfiguration configuration = new(rootCommand) { - EnableTypoCorrections = true, Output = new StringWriter() }; @@ -132,7 +153,6 @@ public async Task Arguments_are_not_suggested() }; CliConfiguration configuration = new(rootCommand) { - EnableTypoCorrections = true, EnableParseErrorReporting = false, Output = new StringWriter() }; @@ -158,7 +178,6 @@ public async Task Hidden_options_are_not_suggested() }; CliConfiguration config = new(rootCommand) { - EnableTypoCorrections = true, Output = new StringWriter() }; @@ -179,7 +198,6 @@ public async Task Suggestions_favor_matches_with_prefix() }; CliConfiguration config = new(rootCommand) { - EnableTypoCorrections = true, Output = new StringWriter() }; var result = rootCommand.Parse("-all", config); diff --git a/src/System.CommandLine.Tests/UseParseErrorReportingTests.cs b/src/System.CommandLine.Tests/UseParseErrorReportingTests.cs index b75081609a..1bb5dc295c 100644 --- a/src/System.CommandLine.Tests/UseParseErrorReportingTests.cs +++ b/src/System.CommandLine.Tests/UseParseErrorReportingTests.cs @@ -5,7 +5,8 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.CommandLine.Help; -using System.Linq; +using System.CommandLine.Invocation; +using System.IO; using FluentAssertions; using Xunit; @@ -37,28 +38,28 @@ public void Parse_error_reporting_reports_error_when_help_is_used_and_required_s } [Fact] - public void User_can_customize_parse_error_result_code() + public void Help_display_can_be_disabled() { - var root = new CliRootCommand + CliRootCommand rootCommand = new() { - new CliCommand("inner") + new CliOption("--verbose") }; - CliConfiguration config = new (root) + CliConfiguration config = new(rootCommand) { - EnableParseErrorReporting = true + Output = new StringWriter() }; - ParseResult parseResult = root.Parse("", config); + var result = rootCommand.Parse("oops", config); - int result = parseResult.Invoke(); - - if (parseResult.Errors.Any()) + if (result.Action is ParseErrorAction parseError) { - result = 42; + parseError.ShowHelp = false; } - result.Should().Be(42); + result.Invoke(); + + config.Output.ToString().Should().NotContain("--verbose"); } [Fact] diff --git a/src/System.CommandLine.Tests/VersionOptionTests.cs b/src/System.CommandLine.Tests/VersionOptionTests.cs index 2bf2205fab..e331d72261 100644 --- a/src/System.CommandLine.Tests/VersionOptionTests.cs +++ b/src/System.CommandLine.Tests/VersionOptionTests.cs @@ -128,18 +128,18 @@ public void Version_is_not_valid_with_other_tokens(string commandLine) result.Errors.Should().Contain(e => e.Message == "--version option cannot be combined with other arguments."); } - + [Fact] public void Version_option_is_not_added_to_subcommands() { var childCommand = new CliCommand("subcommand"); - childCommand.SetAction((_) => { }); + childCommand.SetAction(_ => { }); var rootCommand = new CliRootCommand { childCommand }; - rootCommand.SetAction((_) => { }); + rootCommand.SetAction(_ => { }); CliConfiguration configuration = new(rootCommand) { diff --git a/src/System.CommandLine/CliConfiguration.cs b/src/System.CommandLine/CliConfiguration.cs index e6e3b05891..04c7d96c7f 100644 --- a/src/System.CommandLine/CliConfiguration.cs +++ b/src/System.CommandLine/CliConfiguration.cs @@ -70,11 +70,6 @@ public CliConfiguration(CliCommand rootCommand) /// public bool EnableParseErrorReporting { get; set; } = true; - /// - /// Configures the application to provide alternative suggestions when a parse error is detected. Disabled by default. - /// - public bool EnableTypoCorrections { get; set; } = false; - /// /// Enables signaling and handling of process termination (Ctrl+C, SIGINT, SIGTERM) via a /// that can be passed to a during invocation. diff --git a/src/System.CommandLine/Help/HelpOptionAction.cs b/src/System.CommandLine/Help/HelpOptionAction.cs index ec9edeabb3..822296e5a5 100644 --- a/src/System.CommandLine/Help/HelpOptionAction.cs +++ b/src/System.CommandLine/Help/HelpOptionAction.cs @@ -15,6 +15,7 @@ public HelpBuilder Builder set => _builder = value ?? throw new ArgumentNullException(nameof(value)); } + /// public override int Invoke(ParseResult parseResult) { var output = parseResult.Configuration.Output; diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 4c5b1e9493..a61a85b46c 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -81,42 +81,43 @@ internal static async Task InvokeAsync(ParseResult parseResult, Cancellatio internal static int Invoke(ParseResult parseResult) { - if (parseResult.Action is null) + switch (parseResult.Action) { - return ReturnCodeForMissingAction(parseResult); - } + case null: + return ReturnCodeForMissingAction(parseResult); - try - { - if (parseResult.NonexclusiveActions is not null) - { - for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) + case SynchronousCliAction syncAction: + try { - var action = parseResult.NonexclusiveActions[i]; - - switch (action) + if (parseResult.NonexclusiveActions is not null) { - case SynchronousCliAction syncAction: - syncAction.Invoke(parseResult); - break; - case AsynchronousCliAction _: - throw new InvalidOperationException($"{nameof(AsynchronousCliAction)} called within non-async invocation."); + // FIX: (Invoke) Debug.Assert these pre-action checks? + for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) + { + if (parseResult.NonexclusiveActions[i] is not SynchronousCliAction) + { + throw new InvalidOperationException($"{nameof(AsynchronousCliAction)} called within non-async invocation."); + } + } + + for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) + { + if (parseResult.NonexclusiveActions[i] is SynchronousCliAction syncPreaction) + { + syncPreaction.Invoke(parseResult); + } + } } - } - } - switch (parseResult.Action) - { - case SynchronousCliAction syncAction: return syncAction.Invoke(parseResult); - break; - default: - throw new InvalidOperationException($"{nameof(AsynchronousCliAction)} called within non-async invocation."); - } - } - catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) - { - return DefaultExceptionHandler(ex, parseResult.Configuration); + } + catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) + { + return DefaultExceptionHandler(ex, parseResult.Configuration); + } + + default: + throw new InvalidOperationException($"{nameof(AsynchronousCliAction)} called within non-async invocation."); } } diff --git a/src/System.CommandLine/Invocation/ParseErrorAction.cs b/src/System.CommandLine/Invocation/ParseErrorAction.cs index 1a35fcde20..b2e6d80ee4 100644 --- a/src/System.CommandLine/Invocation/ParseErrorAction.cs +++ b/src/System.CommandLine/Invocation/ParseErrorAction.cs @@ -1,13 +1,37 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Collections.Generic; using System.CommandLine.Help; +using System.Linq; namespace System.CommandLine.Invocation; -internal sealed class ParseErrorAction : SynchronousCliAction +public sealed class ParseErrorAction : SynchronousCliAction { + public bool ShowHelp { get; set; } = true; + + public bool ShowTypoCorrections { get; set; } = true; + + /// public override int Invoke(ParseResult parseResult) + { + if (ShowTypoCorrections) + { + WriteTypoCorrectionSuggestions(parseResult); + } + + WriteErrorDetails(parseResult); + + if (ShowHelp) + { + WriteHelp(parseResult); + } + + return 1; + } + + private static void WriteErrorDetails(ParseResult parseResult) { ConsoleHelpers.ResetTerminalForegroundColor(); ConsoleHelpers.SetTerminalForegroundRed(); @@ -20,9 +44,155 @@ public override int Invoke(ParseResult parseResult) parseResult.Configuration.Error.WriteLine(); ConsoleHelpers.ResetTerminalForegroundColor(); + } + private static void WriteHelp(ParseResult parseResult) + { new HelpAction().Invoke(parseResult); + } - return 1; + private static void WriteTypoCorrectionSuggestions(ParseResult parseResult) + { + var unmatchedTokens = parseResult.UnmatchedTokens; + + for (var i = 0; i < unmatchedTokens.Count; i++) + { + var token = unmatchedTokens[i]; + + bool first = true; + foreach (string suggestion in GetPossibleTokens(parseResult.CommandResult.Command, token)) + { + if (first) + { + parseResult.Configuration.Output.WriteLine(LocalizationResources.SuggestionsTokenNotMatched(token)); + first = false; + } + + parseResult.Configuration.Output.WriteLine(suggestion); + } + } + + parseResult.Configuration.Output.WriteLine(); + + static IEnumerable GetPossibleTokens(CliCommand targetSymbol, string token) + { + if (targetSymbol is { HasOptions: false, HasSubcommands: false }) + { + return Array.Empty(); + } + + IEnumerable possibleMatches = targetSymbol + .Children + .Where(x => !x.Hidden && x is CliOption or CliCommand) + .Select(symbol => + { + AliasSet? aliasSet = symbol is CliOption option ? option._aliases : ((CliCommand)symbol)._aliases; + + if (aliasSet is null) + { + return symbol.Name; + } + + return new[] { symbol.Name }.Concat(aliasSet) + .OrderBy(x => GetDistance(token, x)) + .ThenByDescending(x => GetStartsWithDistance(token, x)) + .First(); + }); + + int? bestDistance = null; + return possibleMatches + .Select(possibleMatch => (possibleMatch, distance: GetDistance(token, possibleMatch))) + .Where(tuple => tuple.distance <= MaxLevenshteinDistance) + .OrderBy(tuple => tuple.distance) + .ThenByDescending(tuple => GetStartsWithDistance(token, tuple.possibleMatch)) + .TakeWhile(tuple => + { + var (_, distance) = tuple; + if (bestDistance is null) + { + bestDistance = distance; + } + + return distance == bestDistance; + }) + .Select(tuple => tuple.possibleMatch); + } + + static int GetStartsWithDistance(string first, string second) + { + int i; + for (i = 0; i < first.Length && i < second.Length && first[i] == second[i]; i++) + { + } + + return i; + } + + //Based on https://blogs.msdn.microsoft.com/toub/2006/05/05/generic-levenshtein-edit-distance-with-c/ + static int GetDistance(string first, string second) + { + // Validate parameters + if (first is null) + { + throw new ArgumentNullException(nameof(first)); + } + + if (second is null) + { + throw new ArgumentNullException(nameof(second)); + } + + // Get the length of both. If either is 0, return + // the length of the other, since that number of insertions + // would be required. + + int n = first.Length, m = second.Length; + if (n == 0) return m; + if (m == 0) return n; + + // Rather than maintain an entire matrix (which would require O(n*m) space), + // just store the current row and the next row, each of which has a length m+1, + // so just O(m) space. Initialize the current row. + + int curRow = 0, nextRow = 1; + int[][] rows = { new int[m + 1], new int[m + 1] }; + + for (int j = 0; j <= m; ++j) + { + rows[curRow][j] = j; + } + + // For each virtual row (since we only have physical storage for two) + for (int i = 1; i <= n; ++i) + { + // Fill in the values in the row + rows[nextRow][0] = i; + for (int j = 1; j <= m; ++j) + { + int dist1 = rows[curRow][j] + 1; + int dist2 = rows[nextRow][j - 1] + 1; + int dist3 = rows[curRow][j - 1] + (first[i - 1].Equals(second[j - 1]) ? 0 : 1); + + rows[nextRow][j] = Math.Min(dist1, Math.Min(dist2, dist3)); + } + + // Swap the current and next rows + if (curRow == 0) + { + curRow = 1; + nextRow = 0; + } + else + { + curRow = 0; + nextRow = 1; + } + } + + // Return the computed edit distance + return rows[curRow][m]; + } } + + private const int MaxLevenshteinDistance = 3; } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/ProcessTerminationHandler.cs b/src/System.CommandLine/Invocation/ProcessTerminationHandler.cs index 4fd5543ad1..ead35b4eb0 100644 --- a/src/System.CommandLine/Invocation/ProcessTerminationHandler.cs +++ b/src/System.CommandLine/Invocation/ProcessTerminationHandler.cs @@ -1,3 +1,6 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; diff --git a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs b/src/System.CommandLine/Invocation/TypoCorrectionAction.cs deleted file mode 100644 index 1f2c1a2473..0000000000 --- a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs +++ /dev/null @@ -1,159 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Collections.Generic; -using System.Linq; - -namespace System.CommandLine.Invocation -{ - internal sealed class TypoCorrectionAction : SynchronousCliAction - { - // FIX: (TypoCorrectionAction) make this part of ParseErrorResultAction - private const int MaxLevenshteinDistance = 3; - - public override int Invoke(ParseResult parseResult) - => ProvideSuggestions(parseResult); - - private static int ProvideSuggestions(ParseResult result) - { - var unmatchedTokens = result.UnmatchedTokens; - for (var i = 0; i < unmatchedTokens.Count; i++) - { - var token = unmatchedTokens[i]; - - bool first = true; - foreach (string suggestion in GetPossibleTokens(result.CommandResult.Command, token)) - { - if (first) - { - result.Configuration.Output.WriteLine(LocalizationResources.SuggestionsTokenNotMatched(token)); - first = false; - } - - result.Configuration.Output.WriteLine(suggestion); - } - } - - return 0; - } - - private static IEnumerable GetPossibleTokens(CliCommand targetSymbol, string token) - { - if (!targetSymbol.HasOptions && !targetSymbol.HasSubcommands) - { - return Array.Empty(); - } - - IEnumerable possibleMatches = targetSymbol - .Children - .Where(x => !x.Hidden && x is CliOption or CliCommand) - .Select(symbol => - { - AliasSet? aliasSet = symbol is CliOption option ? option._aliases : ((CliCommand)symbol)._aliases; - - if (aliasSet is null) - { - return symbol.Name; - } - - return new[] { symbol.Name }.Concat(aliasSet) - .OrderBy(x => GetDistance(token, x)) - .ThenByDescending(x => GetStartsWithDistance(token, x)) - .First(); - }); - - int? bestDistance = null; - return possibleMatches - .Select(possibleMatch => (possibleMatch, distance: GetDistance(token, possibleMatch))) - .Where(tuple => tuple.distance <= MaxLevenshteinDistance) - .OrderBy(tuple => tuple.distance) - .ThenByDescending(tuple => GetStartsWithDistance(token, tuple.possibleMatch)) - .TakeWhile(tuple => - { - var (_, distance) = tuple; - if (bestDistance is null) - { - bestDistance = distance; - } - - return distance == bestDistance; - }) - .Select(tuple => tuple.possibleMatch); - } - - private static int GetStartsWithDistance(string first, string second) - { - int i; - for (i = 0; i < first.Length && i < second.Length && first[i] == second[i]; i++) - { - } - - return i; - } - - //Based on https://blogs.msdn.microsoft.com/toub/2006/05/05/generic-levenshtein-edit-distance-with-c/ - private static int GetDistance(string first, string second) - { - // Validate parameters - if (first is null) - { - throw new ArgumentNullException(nameof(first)); - } - - if (second is null) - { - throw new ArgumentNullException(nameof(second)); - } - - // Get the length of both. If either is 0, return - // the length of the other, since that number of insertions - // would be required. - - int n = first.Length, m = second.Length; - if (n == 0) return m; - if (m == 0) return n; - - // Rather than maintain an entire matrix (which would require O(n*m) space), - // just store the current row and the next row, each of which has a length m+1, - // so just O(m) space. Initialize the current row. - - int curRow = 0, nextRow = 1; - int[][] rows = { new int[m + 1], new int[m + 1] }; - - for (int j = 0; j <= m; ++j) - { - rows[curRow][j] = j; - } - - // For each virtual row (since we only have physical storage for two) - for (int i = 1; i <= n; ++i) - { - // Fill in the values in the row - rows[nextRow][0] = i; - for (int j = 1; j <= m; ++j) - { - int dist1 = rows[curRow][j] + 1; - int dist2 = rows[nextRow][j - 1] + 1; - int dist3 = rows[curRow][j - 1] + (first[i - 1].Equals(second[j - 1]) ? 0 : 1); - - rows[nextRow][j] = Math.Min(dist1, Math.Min(dist2, dist3)); - } - - // Swap the current and next rows - if (curRow == 0) - { - curRow = 1; - nextRow = 0; - } - else - { - curRow = 0; - nextRow = 1; - } - } - - // Return the computed edit distance - return rows[curRow][m]; - } - } -} \ No newline at end of file diff --git a/src/System.CommandLine/Parsing/ParseDiagramAction.cs b/src/System.CommandLine/Parsing/ParseDiagramAction.cs index 4836ac64b3..0a8f86bfc0 100644 --- a/src/System.CommandLine/Parsing/ParseDiagramAction.cs +++ b/src/System.CommandLine/Parsing/ParseDiagramAction.cs @@ -6,8 +6,6 @@ using System.CommandLine.Invocation; using System.Linq; using System.Text; -using System.Threading.Tasks; -using System.Threading; namespace System.CommandLine.Parsing { @@ -46,7 +44,7 @@ internal static StringBuilder Diagram(ParseResult parseResult) for (var i = 0; i < unmatchedTokens.Count; i++) { var error = unmatchedTokens[i]; - builder.Append(" "); + builder.Append(' '); builder.Append(error); } } @@ -61,24 +59,24 @@ private static void Diagram( { if (parseResult.Errors.Any(e => e.SymbolResult == symbolResult)) { - builder.Append("!"); + builder.Append('!'); } switch (symbolResult) { - case DirectiveResult directiveResult when directiveResult.Directive is not DiagramDirective: + case DirectiveResult { Directive: not DiagramDirective }: break; + case ArgumentResult argumentResult: { var includeArgumentName = - argumentResult.Argument.FirstParent!.Symbol is CliCommand command && - command.HasArguments && command.Arguments.Count > 1; + argumentResult.Argument.FirstParent!.Symbol is CliCommand { HasArguments: true, Arguments.Count: > 1 }; if (includeArgumentName) { builder.Append("[ "); builder.Append(argumentResult.Argument.Name); - builder.Append(" "); + builder.Append(' '); } if (argumentResult.Argument.Arity.MaximumNumberOfValues > 0) @@ -96,26 +94,26 @@ private static void Diagram( break; case IEnumerable items: - builder.Append("<"); + builder.Append('<'); builder.Append( string.Join("> <", items.Cast().ToArray())); - builder.Append(">"); + builder.Append('>'); break; default: - builder.Append("<"); + builder.Append('<'); builder.Append(conversionResult.Value); - builder.Append(">"); + builder.Append('>'); break; } break; default: // failures - builder.Append("<"); + builder.Append('<'); builder.Append(string.Join("> <", symbolResult.Tokens.Select(t => t.Value))); - builder.Append(">"); + builder.Append('>'); break; } @@ -135,7 +133,7 @@ private static void Diagram( if (optionResult is { Implicit: true }) { - builder.Append("*"); + builder.Append('*'); } builder.Append("[ "); @@ -158,7 +156,7 @@ private static void Diagram( continue; } - builder.Append(" "); + builder.Append(' '); Diagram(builder, child, parseResult); } diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 8fc19d805c..c92924ee00 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -65,12 +65,8 @@ internal ParseResult Parse() if (_primaryAction is null) { - if (_configuration.EnableTypoCorrections && _rootCommandResult.Command.TreatUnmatchedTokensAsErrors - && _symbolResultTree.UnmatchedTokens is not null) - { - _primaryAction = new TypoCorrectionAction(); - } - else if (_configuration.EnableParseErrorReporting && _symbolResultTree.ErrorCount > 0) + if (_configuration.EnableParseErrorReporting && + _symbolResultTree.ErrorCount > 0) { _primaryAction = new ParseErrorAction(); } From 85df88f964787fd91266f0bbe6b705428d389cac Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Fri, 2 Jun 2023 13:28:41 -0700 Subject: [PATCH 06/19] rename test class --- .../ParseErrorReportingTests.cs | 64 +++++++++++ .../UseParseErrorReportingTests.cs | 106 ------------------ 2 files changed, 64 insertions(+), 106 deletions(-) create mode 100644 src/System.CommandLine.Tests/ParseErrorReportingTests.cs delete mode 100644 src/System.CommandLine.Tests/UseParseErrorReportingTests.cs diff --git a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs new file mode 100644 index 0000000000..b9c626b30c --- /dev/null +++ b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs @@ -0,0 +1,64 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.CommandLine.Help; +using System.CommandLine.Invocation; +using System.IO; +using FluentAssertions; +using Xunit; + +namespace System.CommandLine.Tests; + +public class ParseErrorReportingTests +{ + [Fact] // https://github.com/dotnet/command-line-api/issues/817 + public void Parse_error_reporting_reports_error_when_help_is_used_and_required_subcommand_is_missing() + { + var root = new CliRootCommand + { + new CliCommand("inner"), + new HelpOption() + }; + + CliConfiguration config = new (root) + { + EnableParseErrorReporting = true + }; + + var parseResult = root.Parse("", config); + + parseResult.Errors.Should().NotBeEmpty(); + + var result = config.Invoke(""); + + result.Should().Be(1); + } + + [Fact] + public void Help_display_can_be_disabled() + { + CliRootCommand rootCommand = new() + { + new CliOption("--verbose") + }; + + CliConfiguration config = new(rootCommand) + { + Output = new StringWriter() + }; + + var result = rootCommand.Parse("oops", config); + + if (result.Action is ParseErrorAction parseError) + { + parseError.ShowHelp = false; + } + + result.Invoke(); + + config.Output.ToString().Should().NotContain("--verbose"); + } +} \ No newline at end of file diff --git a/src/System.CommandLine.Tests/UseParseErrorReportingTests.cs b/src/System.CommandLine.Tests/UseParseErrorReportingTests.cs deleted file mode 100644 index 1bb5dc295c..0000000000 --- a/src/System.CommandLine.Tests/UseParseErrorReportingTests.cs +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.CommandLine.Help; -using System.CommandLine.Invocation; -using System.IO; -using FluentAssertions; -using Xunit; - -namespace System.CommandLine.Tests -{ - public class UseParseErrorReportingTests - { - [Fact] // https://github.com/dotnet/command-line-api/issues/817 - public void Parse_error_reporting_reports_error_when_help_is_used_and_required_subcommand_is_missing() - { - var root = new CliRootCommand - { - new CliCommand("inner"), - new HelpOption() - }; - - CliConfiguration config = new (root) - { - EnableParseErrorReporting = true - }; - - var parseResult = root.Parse("", config); - - parseResult.Errors.Should().NotBeEmpty(); - - var result = config.Invoke(""); - - result.Should().Be(1); - } - - [Fact] - public void Help_display_can_be_disabled() - { - CliRootCommand rootCommand = new() - { - new CliOption("--verbose") - }; - - CliConfiguration config = new(rootCommand) - { - Output = new StringWriter() - }; - - var result = rootCommand.Parse("oops", config); - - if (result.Action is ParseErrorAction parseError) - { - parseError.ShowHelp = false; - } - - result.Invoke(); - - config.Output.ToString().Should().NotContain("--verbose"); - } - - [Fact] - public void User_can_customize_help_printed_on_parse_error() - { - CustomHelpBuilder customHelpBuilder = new(); - - CliRootCommand root = new (); - root.Options.Clear(); - root.Options.Add(new HelpOption() - { - Action = new HelpAction() - { - Builder = customHelpBuilder - } - }); - - CliConfiguration config = new(root) - { - EnableParseErrorReporting = true - }; - - customHelpBuilder.WasUsed.Should().BeFalse(); - - ParseResult parseResult = root.Parse("-bla", config); - - int result = parseResult.Invoke(); - result.Should().Be(1); - customHelpBuilder.WasUsed.Should().BeTrue(); - } - - private sealed class CustomHelpBuilder : HelpBuilder - { - internal bool WasUsed = false; - - public override void Write(HelpContext context) - { - WasUsed = true; - - base.Write(context); - } - } - } -} \ No newline at end of file From b826e78cdc73e675eee63309b4f7107eb269a3b9 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 5 Jun 2023 11:20:13 -0700 Subject: [PATCH 07/19] bring back (internal) anonymous CliAction types --- .../HostingExtensions.cs | 2 +- src/System.CommandLine/CliCommand.cs | 8 ++-- .../Invocation/CliCommandAction.cs | 8 ++-- .../Invocation/InvocationPipeline.cs | 44 +++++++++---------- .../Parsing/ParseOperation.cs | 2 +- 5 files changed, 31 insertions(+), 33 deletions(-) diff --git a/src/System.CommandLine.Hosting/HostingExtensions.cs b/src/System.CommandLine.Hosting/HostingExtensions.cs index 713e25b3f2..644e43ad30 100644 --- a/src/System.CommandLine.Hosting/HostingExtensions.cs +++ b/src/System.CommandLine.Hosting/HostingExtensions.cs @@ -55,7 +55,7 @@ public static OptionsBuilder BindCommandLine( public static CliCommand UseCommandHandler(this CliCommand command) where THandler : CliAction { - command.Action = CommandHandler.Create(typeof(THandler).GetMethod(nameof(AsynchronousCliCommandAction.InvokeAsync))); + command.Action = CommandHandler.Create(typeof(THandler).GetMethod(nameof(AsynchronousCliAction.InvokeAsync))); return command; } diff --git a/src/System.CommandLine/CliCommand.cs b/src/System.CommandLine/CliCommand.cs index 26f5b57193..a466dfffe2 100644 --- a/src/System.CommandLine/CliCommand.cs +++ b/src/System.CommandLine/CliCommand.cs @@ -110,7 +110,7 @@ public void SetAction(Action action) throw new ArgumentNullException(nameof(action)); } - Action = new SynchronousCliCommandAction(context => + Action = new AnonymousSynchronousCliAction(context => { action(context); return 0; @@ -128,7 +128,7 @@ public void SetAction(Func action) throw new ArgumentNullException(nameof(action)); } - Action = new SynchronousCliCommandAction(action); + Action = new AnonymousSynchronousCliAction(action); } /// @@ -141,7 +141,7 @@ public void SetAction(Func action) throw new ArgumentNullException(nameof(action)); } - Action = new AsynchronousCliCommandAction(async (context, cancellationToken) => + Action = new AnonymousAsynchronousCliAction(async (context, cancellationToken) => { await action(context, cancellationToken); return 0; @@ -159,7 +159,7 @@ public void SetAction(Func> action) throw new ArgumentNullException(nameof(action)); } - Action = new AsynchronousCliCommandAction(action); + Action = new AnonymousAsynchronousCliAction(action); } /// diff --git a/src/System.CommandLine/Invocation/CliCommandAction.cs b/src/System.CommandLine/Invocation/CliCommandAction.cs index a2d5d4d161..1ecbc1b573 100644 --- a/src/System.CommandLine/Invocation/CliCommandAction.cs +++ b/src/System.CommandLine/Invocation/CliCommandAction.cs @@ -42,11 +42,11 @@ public abstract class AsynchronousCliAction : CliAction public abstract Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default); } -public class SynchronousCliCommandAction : SynchronousCliAction +internal class AnonymousSynchronousCliAction : SynchronousCliAction { private readonly Func _syncAction; - internal SynchronousCliCommandAction(Func action) + internal AnonymousSynchronousCliAction(Func action) => _syncAction = action; /// @@ -54,11 +54,11 @@ public override int Invoke(ParseResult parseResult) => _syncAction(parseResult); } -public class AsynchronousCliCommandAction : AsynchronousCliAction +internal class AnonymousAsynchronousCliAction : AsynchronousCliAction { private readonly Func> _asyncAction; - internal AsynchronousCliCommandAction(Func> action) + internal AnonymousAsynchronousCliAction(Func> action) => _asyncAction = action; /// diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index a61a85b46c..48b81fa93d 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -38,35 +38,33 @@ internal static async Task InvokeAsync(ParseResult parseResult, Cancellatio } } - Task startedInvocation; switch (parseResult.Action) { case SynchronousCliAction syncAction: - startedInvocation = Task.FromResult(syncAction.Invoke(parseResult)); - break; + return syncAction.Invoke(parseResult); + case AsynchronousCliAction asyncAction: - startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token); - break; - default: - throw new ArgumentOutOfRangeException(nameof(parseResult.Action)); - } + var startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token); + if (parseResult.Configuration.ProcessTerminationTimeout.HasValue) + { + terminationHandler = new(cts, startedInvocation, parseResult.Configuration.ProcessTerminationTimeout.Value); + } - if (parseResult.Configuration.ProcessTerminationTimeout.HasValue) - { - terminationHandler = new(cts, startedInvocation, parseResult.Configuration.ProcessTerminationTimeout.Value); - } + if (terminationHandler is null) + { + return await startedInvocation; + } + else + { + // Handlers may not implement cancellation. + // In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C, + // ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal. + Task firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task); + return await firstCompletedTask; // return the result or propagate the exception + } - if (terminationHandler is null) - { - return await startedInvocation; - } - else - { - // Handlers may not implement cancellation. - // In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C, - // ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal. - Task firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task); - return await firstCompletedTask; // return the result or propagate the exception + default: + throw new ArgumentOutOfRangeException(nameof(parseResult.Action)); } } catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index c92924ee00..1b5ccf9769 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -333,7 +333,7 @@ void ParseDirective() { _primaryAction = directive.Action; } - else + else { if (_nonexclusiveActions is null) { From 986ef79b29e19e2eb8e738a1addad580dbefdb90 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 5 Jun 2023 11:57:20 -0700 Subject: [PATCH 08/19] Change exception to be a debug-only check --- .../Invocation/InvocationPipeline.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 48b81fa93d..e88608bc32 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -89,14 +89,19 @@ internal static int Invoke(ParseResult parseResult) { if (parseResult.NonexclusiveActions is not null) { - // FIX: (Invoke) Debug.Assert these pre-action checks? +#if DEBUG for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) { - if (parseResult.NonexclusiveActions[i] is not SynchronousCliAction) + var action = parseResult.NonexclusiveActions[i]; + + if (action is not SynchronousCliAction) { - throw new InvalidOperationException($"{nameof(AsynchronousCliAction)} called within non-async invocation."); + parseResult.Configuration.EnableDefaultExceptionHandler = false; + throw new Exception( + $"This should not happen. An instance of {nameof(AsynchronousCliAction)} ({action}) was called within {nameof(InvocationPipeline)}.{nameof(Invoke)}. This is supposed to be detected earlier resulting in a call to {nameof(InvocationPipeline)}{nameof(InvokeAsync)}"); } } +#endif for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) { From 1e2958382e95a3cd81e4ee7b1bf14a055f57d853 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 5 Jun 2023 18:05:47 -0700 Subject: [PATCH 09/19] update API baselines --- ...tionBinder_api_is_not_changed.approved.txt | 3 +- ...ommandLine_api_is_not_changed.approved.txt | 40 +++++++++++-------- .../{CliCommandAction.cs => CliAction.cs} | 0 3 files changed, 24 insertions(+), 19 deletions(-) rename src/System.CommandLine/Invocation/{CliCommandAction.cs => CliAction.cs} (100%) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt index 3b24de558b..2a9e067492 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt @@ -21,7 +21,7 @@ System.CommandLine.NamingConventionBinder public static System.Void AddModelBinder(this System.CommandLine.Binding.BindingContext bindingContext, ModelBinder binder) public static System.CommandLine.Binding.BindingContext GetBindingContext(this System.CommandLine.ParseResult parseResult) public static ModelBinder GetOrCreateModelBinder(this System.CommandLine.Binding.BindingContext bindingContext, System.CommandLine.Binding.IValueDescriptor valueDescriptor) - public abstract class BindingHandler : System.CommandLine.CliAction + public abstract class BindingHandler : System.CommandLine.Invocation.AsynchronousCliAction public System.CommandLine.Binding.BindingContext GetBindingContext(System.CommandLine.ParseResult parseResult) public static class CommandHandler public static BindingHandler Create(System.Delegate delegate) @@ -119,7 +119,6 @@ System.CommandLine.NamingConventionBinder public class ModelBindingCommandHandler : BindingHandler public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.CliArgument argument) public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.CliOption option) - public System.Int32 Invoke(System.CommandLine.ParseResult parseResult) public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) public class ModelDescriptor public static ModelDescriptor FromType() diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 1305e4a18f..5be1c526bd 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -16,11 +16,6 @@ System.CommandLine public static CliArgument AcceptExistingOnly(this CliArgument argument) public static CliArgument AcceptExistingOnly(this CliArgument argument) public static CliArgument AcceptExistingOnly(this CliArgument argument) - public abstract class CliAction - public System.Boolean Exclusive { get; } - public System.Int32 Invoke(ParseResult parseResult) - public System.Threading.Tasks.Task InvokeAsync(ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) - protected System.Void set_Exclusive(System.Boolean value) public abstract class CliArgument : CliSymbol public ArgumentArity Arity { get; set; } public System.Collections.Generic.List>> CompletionSources { get; } @@ -42,7 +37,7 @@ System.CommandLine public System.Void AcceptOnlyFromAmong(System.String[] values) public class CliCommand : CliSymbol, System.Collections.Generic.IEnumerable, System.Collections.IEnumerable .ctor(System.String name, System.String description = null) - public CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } public System.Collections.Generic.ICollection Aliases { get; } public System.Collections.Generic.IList Arguments { get; } public System.Collections.Generic.IEnumerable Children { get; } @@ -65,7 +60,6 @@ System.CommandLine public System.Boolean EnableDefaultExceptionHandler { get; set; } public System.Boolean EnableParseErrorReporting { get; set; } public System.Boolean EnablePosixBundling { get; set; } - public System.Boolean EnableTypoCorrections { get; set; } public System.IO.TextWriter Error { get; set; } public System.IO.TextWriter Output { get; set; } public System.Nullable ProcessTerminationTimeout { get; set; } @@ -82,10 +76,10 @@ System.CommandLine .ctor(System.String message) public class CliDirective : CliSymbol .ctor(System.String name) - public CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) public abstract class CliOption : CliSymbol - public CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } public System.Collections.Generic.ICollection Aliases { get; } public System.Boolean AllowMultipleArgumentsPerToken { get; set; } public ArgumentArity Arity { get; set; } @@ -120,18 +114,18 @@ System.CommandLine public static System.Void Add(this System.Collections.Generic.List>> completionSources, System.String[] completions) public class DiagramDirective : CliDirective .ctor() - public CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } public System.Int32 ParseErrorReturnValue { get; set; } public class EnvironmentVariablesDirective : CliDirective .ctor() - public CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } public static class OptionValidation public static CliOption AcceptExistingOnly(this CliOption option) public static CliOption AcceptExistingOnly(this CliOption option) public static CliOption AcceptExistingOnly(this CliOption option) public static CliOption AcceptExistingOnly(this CliOption option) public class ParseResult - public CliAction Action { get; } + public System.CommandLine.Invocation.CliAction Action { get; } public System.CommandLine.Parsing.CommandResult CommandResult { get; } public CliConfiguration Configuration { get; } public System.Collections.Generic.IReadOnlyList Errors { get; } @@ -154,7 +148,7 @@ System.CommandLine public class VersionOption : CliOption .ctor() .ctor(System.String name, System.String[] aliases) - public CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } System.CommandLine.Completions public class CompletionContext public static CompletionContext Empty { get; } @@ -174,17 +168,16 @@ System.CommandLine.Completions public System.String ToString() public class SuggestDirective : System.CommandLine.CliDirective .ctor() - public System.CommandLine.CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } public class TextCompletionContext : CompletionContext public System.String CommandLineText { get; } public System.Int32 CursorPosition { get; } public TextCompletionContext AtCursorPosition(System.Int32 position) System.CommandLine.Help - public class HelpAction : System.CommandLine.CliAction + public class HelpAction : System.CommandLine.Invocation.SynchronousCliAction .ctor() public HelpBuilder Builder { get; set; } public System.Int32 Invoke(System.CommandLine.ParseResult parseResult) - public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) public class HelpBuilder .ctor(System.Int32 maxWidth = 2147483647) public System.Int32 MaxWidth { get; } @@ -217,7 +210,7 @@ System.CommandLine.Help public class HelpOption : System.CommandLine.CliOption .ctor() .ctor(System.String name, System.String[] aliases) - public System.CommandLine.CliAction Action { get; set; } + public System.CommandLine.Invocation.CliAction Action { get; set; } public class TwoColumnHelpRow, System.IEquatable .ctor(System.String firstColumnText, System.String secondColumnText) public System.String FirstColumnText { get; } @@ -225,6 +218,19 @@ System.CommandLine.Help public System.Boolean Equals(System.Object obj) public System.Boolean Equals(TwoColumnHelpRow other) public System.Int32 GetHashCode() +System.CommandLine.Invocation + public abstract class AsynchronousCliAction : CliAction + public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) + public abstract class CliAction + public System.Boolean Exclusive { get; } + protected System.Void set_Exclusive(System.Boolean value) + public class ParseErrorAction : SynchronousCliAction + .ctor() + public System.Boolean ShowHelp { get; set; } + public System.Boolean ShowTypoCorrections { get; set; } + public System.Int32 Invoke(System.CommandLine.ParseResult parseResult) + public abstract class SynchronousCliAction : CliAction + public System.Int32 Invoke(System.CommandLine.ParseResult parseResult) System.CommandLine.Parsing public class ArgumentResult : SymbolResult public System.CommandLine.CliArgument Argument { get; } diff --git a/src/System.CommandLine/Invocation/CliCommandAction.cs b/src/System.CommandLine/Invocation/CliAction.cs similarity index 100% rename from src/System.CommandLine/Invocation/CliCommandAction.cs rename to src/System.CommandLine/Invocation/CliAction.cs From 8334e6501ea6817d427607c606790034e5252fa3 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 5 Jun 2023 19:16:25 -0700 Subject: [PATCH 10/19] generalize precedence of directive actions over option actions --- .../DirectiveTests.cs | 50 +++------------ .../Invocation/InvocationTests.cs | 62 +++++++++++++------ .../TestCliActions.cs | 42 +++++++++++++ src/System.CommandLine/CliCommand.cs | 40 ++++++------ .../EnvironmentVariablesDirective.cs | 2 +- .../Invocation/CliAction.cs | 2 +- .../Parsing/ParseOperation.cs | 15 ++--- 7 files changed, 118 insertions(+), 95 deletions(-) create mode 100644 src/System.CommandLine.Tests/TestCliActions.cs diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 93bda02238..ce62c50b22 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -1,9 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System.CommandLine.Invocation; using System.Linq; -using System.Threading; using System.Threading.Tasks; using FluentAssertions; using FluentAssertions.Execution; @@ -74,15 +72,15 @@ public async Task Multiple_instances_of_the_same_directive_can_be_invoked(bool i var testDirective = new TestDirective("test") { Action = invokeAsync - ? new AsynchronousNonexclusiveTestAction(incrementCallCount) - : new SynchronousNonexclusiveTestAction(incrementCallCount) + ? new AsynchronousNonterminatingTestAction(incrementCallCount, terminating: false) + : new SynchronousTestAction(incrementCallCount, terminating: false) }; var config = new CliConfiguration(new CliRootCommand { Action = invokeAsync - ? new AsynchronousNonexclusiveTestAction(verifyActionWasCalled) - : new SynchronousNonexclusiveTestAction(verifyActionWasCalled) + ? new AsynchronousNonterminatingTestAction(verifyActionWasCalled, terminating: false) + : new SynchronousTestAction(verifyActionWasCalled, terminating: false) }) { Directives = { testDirective } @@ -114,15 +112,15 @@ public async Task Multiple_different_directives_can_be_invoked(bool invokeAsync) var directiveOne = new TestDirective("one") { - Action = new SynchronousNonexclusiveTestAction(_ => directiveOneActionWasCalled = true) + Action = new SynchronousTestAction(_ => directiveOneActionWasCalled = true, terminating: false) }; var directiveTwo = new TestDirective("two") { - Action = new SynchronousNonexclusiveTestAction(_ => directiveTwoActionWasCalled = true) + Action = new SynchronousTestAction(_ => directiveTwoActionWasCalled = true, terminating: false) }; var config = new CliConfiguration(new CliRootCommand { - Action = new SynchronousNonexclusiveTestAction(_ => commandActionWasCalled = true) + Action = new SynchronousTestAction(_ => commandActionWasCalled = true, terminating: false) }) { Directives = { directiveOne, directiveTwo } @@ -151,40 +149,6 @@ public TestDirective(string name) : base(name) } } - private class SynchronousNonexclusiveTestAction : SynchronousCliAction - { - private readonly Action _invoke; - - public SynchronousNonexclusiveTestAction(Action invoke) - { - _invoke = invoke; - Exclusive = false; - } - - public override int Invoke(ParseResult parseResult) - { - _invoke(parseResult); - return 0; - } - } - - private class AsynchronousNonexclusiveTestAction : AsynchronousCliAction - { - private readonly Action _invoke; - - public AsynchronousNonexclusiveTestAction(Action invoke) - { - _invoke = invoke; - Exclusive = false; - } - - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) - { - _invoke(parseResult); - return Task.FromResult(0); - } - } - [Theory] [InlineData("[key:value]", "key", "value")] [InlineData("[key:value:more]", "key", "value:more")] diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index 328e0416c8..0fdeb85b4e 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -5,9 +5,11 @@ using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.IO; +using System.Runtime.InteropServices.ComTypes; using System.Threading; using System.Threading.Tasks; using FluentAssertions; +using FluentAssertions.Execution; using Xunit; namespace System.CommandLine.Tests.Invocation @@ -195,7 +197,8 @@ public async Task Anonymous_RootCommand_int_returning_Action_can_set_custom_resu [Fact] public void Option_action_takes_precedence_over_command_action() { - OptionAction optionAction = new(); + bool wasCalled = false; + SynchronousTestAction optionAction = new(_ => wasCalled = true); bool commandActionWasCalled = false; CliOption option = new("--test") @@ -214,20 +217,24 @@ public void Option_action_takes_precedence_over_command_action() ParseResult parseResult = command.Parse("cmd --test true"); parseResult.Action.Should().NotBeNull(); - optionAction.WasCalled.Should().BeFalse(); + wasCalled.Should().BeFalse(); commandActionWasCalled.Should().BeFalse(); parseResult.Invoke().Should().Be(0); - optionAction.WasCalled.Should().BeTrue(); + wasCalled.Should().BeTrue(); commandActionWasCalled.Should().BeFalse(); } [Fact] public void When_multiple_options_with_actions_are_present_then_only_the_last_one_is_invoked() { - OptionAction optionAction1 = new(); - OptionAction optionAction2 = new(); - OptionAction optionAction3 = new(); + bool optionAction1WasCalled = false; + bool optionAction2WasCalled = false; + bool optionAction3WasCalled = false; + + SynchronousTestAction optionAction1 = new(_ => optionAction1WasCalled = true); + SynchronousTestAction optionAction2 = new(_ => optionAction2WasCalled = true); + SynchronousTestAction optionAction3 = new(_ => optionAction3WasCalled = true); CliCommand command = new CliCommand("cmd") { @@ -236,25 +243,44 @@ public void When_multiple_options_with_actions_are_present_then_only_the_last_on new CliOption("--3") { Action = optionAction3 } }; - ParseResult parseResult = command.Parse("cmd --1 true --3 false --2 true "); + ParseResult parseResult = command.Parse("cmd --1 true --3 false --2 true"); - parseResult.Action.Should().Be(optionAction2); + using var _ = new AssertionScope(); + parseResult.Action.Should().Be(optionAction2); parseResult.Invoke().Should().Be(0); - optionAction1.WasCalled.Should().BeFalse(); - optionAction2.WasCalled.Should().BeTrue(); - optionAction3.WasCalled.Should().BeFalse(); + optionAction1WasCalled.Should().BeFalse(); + optionAction2WasCalled.Should().BeTrue(); + optionAction3WasCalled.Should().BeFalse(); } - internal sealed class OptionAction : SynchronousCliAction + [Fact] + public void Directive_action_takes_precedence_over_option_action() { - internal bool WasCalled = false; + bool optionActionWasCalled = false; + bool directiveActionWasCalled = false; - public override int Invoke(ParseResult context) + SynchronousTestAction optionAction = new(_ => optionActionWasCalled = true); + SynchronousTestAction directiveAction = new(_ => directiveActionWasCalled = true); + + var directive = new CliDirective("directive") { - WasCalled = true; - return 0; - } + Action = directiveAction + }; + + CliCommand command = new CliCommand("cmd") + { + new CliOption("-x") { Action = optionAction } + }; + + ParseResult parseResult = command.Parse("[directive] cmd -x", new CliConfiguration(command) { Directives = { directive } }); + + using var _ = new AssertionScope(); + + parseResult.Action.Should().Be(directiveAction); + parseResult.Invoke().Should().Be(0); + optionActionWasCalled.Should().BeFalse(); + directiveActionWasCalled.Should().BeTrue(); } [Theory] @@ -322,7 +348,7 @@ private class NonexclusiveTestAction : SynchronousCliAction { public NonexclusiveTestAction() { - Exclusive = false; + Terminating = false; } public bool ThrowOnInvoke { get; set; } diff --git a/src/System.CommandLine.Tests/TestCliActions.cs b/src/System.CommandLine.Tests/TestCliActions.cs new file mode 100644 index 0000000000..b07818d0b8 --- /dev/null +++ b/src/System.CommandLine.Tests/TestCliActions.cs @@ -0,0 +1,42 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.CommandLine.Invocation; +using System.Threading; +using System.Threading.Tasks; + +namespace System.CommandLine.Tests; + +public class SynchronousTestAction : SynchronousCliAction +{ + private readonly Action _invoke; + + public SynchronousTestAction(Action invoke, bool terminating = true) + { + _invoke = invoke; + Terminating = terminating; + } + + public override int Invoke(ParseResult parseResult) + { + _invoke(parseResult); + return 0; + } +} + +public class AsynchronousNonterminatingTestAction : AsynchronousCliAction +{ + private readonly Action _invoke; + + public AsynchronousNonterminatingTestAction(Action invoke, bool terminating = true) + { + _invoke = invoke; + Terminating = terminating; + } + + public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + { + _invoke(parseResult); + return Task.FromResult(0); + } +} \ No newline at end of file diff --git a/src/System.CommandLine/CliCommand.cs b/src/System.CommandLine/CliCommand.cs index a466dfffe2..f282ad1b94 100644 --- a/src/System.CommandLine/CliCommand.cs +++ b/src/System.CommandLine/CliCommand.cs @@ -163,29 +163,25 @@ public void SetAction(Func> action) } /// - /// Adds a to the command. + /// Adds a to the command. /// - /// The symbol to add to the command. - [EditorBrowsable(EditorBrowsableState.Never)] // hide from intellisense, it's public for C# duck typing - public void Add(CliSymbol symbol) - { - // this method exists so users can use C# duck typing for adding symbols to the Command: - // new Command { option }; - switch (symbol) - { - case CliOption option: - Options.Add(option); - break; - case CliArgument argument: - Arguments.Add(argument); - break; - case CliCommand command: - Subcommands.Add(command); - break; - default: - throw new NotSupportedException(); - } - } + /// The option to add to the command. + [EditorBrowsable(EditorBrowsableState.Never)] // hide from intellisense, it's public for C# collection initializer support + public void Add(CliArgument argument) => Arguments.Add(argument); + + /// + /// Adds a to the command. + /// + /// The option to add to the command. + [EditorBrowsable(EditorBrowsableState.Never)] // hide from intellisense, it's public for C# collection initializer support + public void Add(CliOption option) => Options.Add(option); + + /// + /// Adds a to the command. + /// + /// The Command to add to the command. + [EditorBrowsable(EditorBrowsableState.Never)] // hide from intellisense, it's public for C# collection initializer support + public void Add(CliCommand command) => Subcommands.Add(command); /// /// Gets or sets a value that indicates whether unmatched tokens should be treated as errors. For example, diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index 1b00c53669..11a83de541 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -31,7 +31,7 @@ private sealed class EnvironmentVariablesDirectiveAction : SynchronousCliAction internal EnvironmentVariablesDirectiveAction(EnvironmentVariablesDirective directive) { _directive = directive; - Exclusive = false; + Terminating = false; } public override int Invoke(ParseResult parseResult) diff --git a/src/System.CommandLine/Invocation/CliAction.cs b/src/System.CommandLine/Invocation/CliAction.cs index 1ecbc1b573..1bad7b1165 100644 --- a/src/System.CommandLine/Invocation/CliAction.cs +++ b/src/System.CommandLine/Invocation/CliAction.cs @@ -15,7 +15,7 @@ private protected CliAction() } // FIX: (CliAction) change this to accommodate the fact that the core action isn't "exclusive" either, and some actions might eventually be created that run after it. - public bool Exclusive { get; protected set; } = true; + public bool Terminating { get; protected set; } = true; } /// diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 1b5ccf9769..4778b6de3e 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -18,7 +18,7 @@ internal sealed class ParseOperation private int _index; private CommandResult _innermostCommandResult; private bool _isHelpRequested; - private bool _isDiagramRequested; + private bool _isTerminatingDirectiveSpecified; private CliAction? _primaryAction; private List? _nonexclusiveActions; @@ -187,9 +187,8 @@ private void ParseOption() if (!_symbolResultTree.TryGetValue(option, out SymbolResult? symbolResult)) { - // FIX: (ParseOption) can this be moved out to a CliAction? - // DiagramDirective has a precedence over --help and --version - if (!_isDiagramRequested) + // directives have a precedence over --help and --version + if (!_isTerminatingDirectiveSpecified) { if (option.Action is not null) { @@ -329,9 +328,10 @@ void ParseDirective() if (directive.Action is not null) { - if (directive.Action.Exclusive) + if (directive.Action.Terminating) { _primaryAction = directive.Action; + _isTerminatingDirectiveSpecified = true; } else { @@ -343,11 +343,6 @@ void ParseDirective() _nonexclusiveActions.Add(directive.Action); } } - - if (directive is DiagramDirective) - { - _isDiagramRequested = true; - } } } From f25af1440278e03eea7d85d0c1b2576f50229dc2 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 6 Jun 2023 09:42:44 -0700 Subject: [PATCH 11/19] split into separate files, add XML doc comments --- .../Completions/SuggestDirective.cs | 1 + .../EnvironmentVariablesDirective.cs | 1 + src/System.CommandLine/Help/HelpOption.cs | 3 + .../Help/HelpOptionAction.cs | 3 + .../AnonymousAsynchronousCliAction.cs | 19 ++++++ .../AnonymousSynchronousCliAction.cs | 16 +++++ .../Invocation/AsynchronousCliAction.cs | 19 ++++++ .../Invocation/CliAction.cs | 58 ++----------------- .../Invocation/ParseErrorAction.cs | 11 ++++ .../Invocation/SynchronousCliAction.cs | 17 ++++++ src/System.CommandLine/VersionOption.cs | 3 + 11 files changed, 98 insertions(+), 53 deletions(-) create mode 100644 src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs create mode 100644 src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs create mode 100644 src/System.CommandLine/Invocation/AsynchronousCliAction.cs create mode 100644 src/System.CommandLine/Invocation/SynchronousCliAction.cs diff --git a/src/System.CommandLine/Completions/SuggestDirective.cs b/src/System.CommandLine/Completions/SuggestDirective.cs index 7e6ab2adf4..86584baec7 100644 --- a/src/System.CommandLine/Completions/SuggestDirective.cs +++ b/src/System.CommandLine/Completions/SuggestDirective.cs @@ -12,6 +12,7 @@ public sealed class SuggestDirective : CliDirective { private CliAction? _action; + /// public SuggestDirective() : base("suggest") { } diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index 11a83de541..cac0ec7d85 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -13,6 +13,7 @@ public sealed class EnvironmentVariablesDirective : CliDirective { private CliAction? _action; + /// public EnvironmentVariablesDirective() : base("env") { } diff --git a/src/System.CommandLine/Help/HelpOption.cs b/src/System.CommandLine/Help/HelpOption.cs index 03c42859da..c4c53f194c 100644 --- a/src/System.CommandLine/Help/HelpOption.cs +++ b/src/System.CommandLine/Help/HelpOption.cs @@ -5,6 +5,9 @@ namespace System.CommandLine.Help { + /// + /// A standard option that indicates that command line help should be displayed. + /// public sealed class HelpOption : CliOption { private CliAction? _action; diff --git a/src/System.CommandLine/Help/HelpOptionAction.cs b/src/System.CommandLine/Help/HelpOptionAction.cs index 822296e5a5..9700643ae5 100644 --- a/src/System.CommandLine/Help/HelpOptionAction.cs +++ b/src/System.CommandLine/Help/HelpOptionAction.cs @@ -2,6 +2,9 @@ namespace System.CommandLine.Help { + /// + /// Provides command line help. + /// public sealed class HelpAction : SynchronousCliAction { private HelpBuilder? _builder; diff --git a/src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs b/src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs new file mode 100644 index 0000000000..329a6e2312 --- /dev/null +++ b/src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs @@ -0,0 +1,19 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Threading; +using System.Threading.Tasks; + +namespace System.CommandLine.Invocation; + +internal class AnonymousAsynchronousCliAction : AsynchronousCliAction +{ + private readonly Func> _asyncAction; + + internal AnonymousAsynchronousCliAction(Func> action) + => _asyncAction = action; + + /// + public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => + _asyncAction(parseResult, cancellationToken); +} \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs b/src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs new file mode 100644 index 0000000000..f2e2893558 --- /dev/null +++ b/src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs @@ -0,0 +1,16 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace System.CommandLine.Invocation; + +internal class AnonymousSynchronousCliAction : SynchronousCliAction +{ + private readonly Func _syncAction; + + internal AnonymousSynchronousCliAction(Func action) + => _syncAction = action; + + /// + public override int Invoke(ParseResult parseResult) => + _syncAction(parseResult); +} \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/AsynchronousCliAction.cs b/src/System.CommandLine/Invocation/AsynchronousCliAction.cs new file mode 100644 index 0000000000..2ea1b2ec56 --- /dev/null +++ b/src/System.CommandLine/Invocation/AsynchronousCliAction.cs @@ -0,0 +1,19 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Threading; +using System.Threading.Tasks; + +namespace System.CommandLine.Invocation; + +/// Defines an asynchronous behavior associated with a command line symbol. +public abstract class AsynchronousCliAction : CliAction +{ + /// + /// Performs an action when the associated symbol is invoked on the command line. + /// + /// Provides the parse results. + /// The token to monitor for cancellation requests. + /// A value that can be used as the exit code for the process. + public abstract Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default); +} \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/CliAction.cs b/src/System.CommandLine/Invocation/CliAction.cs index 1bad7b1165..671af3b041 100644 --- a/src/System.CommandLine/Invocation/CliAction.cs +++ b/src/System.CommandLine/Invocation/CliAction.cs @@ -1,67 +1,19 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System.Threading; -using System.Threading.Tasks; - namespace System.CommandLine.Invocation; -// FIX: separate files - +/// +/// Defines a behavior associated with a command line symbol. +/// public abstract class CliAction { private protected CliAction() { } - // FIX: (CliAction) change this to accommodate the fact that the core action isn't "exclusive" either, and some actions might eventually be created that run after it. - public bool Terminating { get; protected set; } = true; -} - -/// -/// Defines the behavior of a symbol. -/// -public abstract class SynchronousCliAction : CliAction -{ - /// - /// Performs an action when the associated symbol is invoked on the command line. - /// - /// Provides the parse results. - /// A value that can be used as the exit code for the process. - public abstract int Invoke(ParseResult parseResult); -} - -public abstract class AsynchronousCliAction : CliAction -{ /// - /// Performs an action when the associated symbol is invoked on the command line. + /// Indicates that the action terminates a command line invocation, and later actions are skipped. /// - /// Provides the parse results. - /// The token to monitor for cancellation requests. - /// A value that can be used as the exit code for the process. - public abstract Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default); -} - -internal class AnonymousSynchronousCliAction : SynchronousCliAction -{ - private readonly Func _syncAction; - - internal AnonymousSynchronousCliAction(Func action) - => _syncAction = action; - - /// - public override int Invoke(ParseResult parseResult) => - _syncAction(parseResult); -} - -internal class AnonymousAsynchronousCliAction : AsynchronousCliAction -{ - private readonly Func> _asyncAction; - - internal AnonymousAsynchronousCliAction(Func> action) - => _asyncAction = action; - - /// - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken) => - _asyncAction(parseResult, cancellationToken); + public bool Terminating { get; protected set; } = true; } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/ParseErrorAction.cs b/src/System.CommandLine/Invocation/ParseErrorAction.cs index b2e6d80ee4..4881eed525 100644 --- a/src/System.CommandLine/Invocation/ParseErrorAction.cs +++ b/src/System.CommandLine/Invocation/ParseErrorAction.cs @@ -7,10 +7,21 @@ namespace System.CommandLine.Invocation; +/// +/// Provides command line output with error details in the case of a parsing error. +/// public sealed class ParseErrorAction : SynchronousCliAction { + /// + /// Indicates whether to show help along with error details when an error is found during parsing. + /// + /// When set to , indicates that help will be shown along with parse error details. When set to false, help will not be shown. public bool ShowHelp { get; set; } = true; + /// + /// Indicates whether to show typo suggestions along with error details when an error is found during parsing. + /// + /// When set to , indicates that suggestions will be shown along with parse error details. When set to false, suggestions will not be shown. public bool ShowTypoCorrections { get; set; } = true; /// diff --git a/src/System.CommandLine/Invocation/SynchronousCliAction.cs b/src/System.CommandLine/Invocation/SynchronousCliAction.cs new file mode 100644 index 0000000000..8b815da5ed --- /dev/null +++ b/src/System.CommandLine/Invocation/SynchronousCliAction.cs @@ -0,0 +1,17 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace System.CommandLine.Invocation; + +/// +/// Defines a synchronous behavior associated with a command line symbol. +/// +public abstract class SynchronousCliAction : CliAction +{ + /// + /// Performs an action when the associated symbol is invoked on the command line. + /// + /// Provides the parse results. + /// A value that can be used as the exit code for the process. + public abstract int Invoke(ParseResult parseResult); +} \ No newline at end of file diff --git a/src/System.CommandLine/VersionOption.cs b/src/System.CommandLine/VersionOption.cs index 85ece7faae..3186262469 100644 --- a/src/System.CommandLine/VersionOption.cs +++ b/src/System.CommandLine/VersionOption.cs @@ -7,6 +7,9 @@ namespace System.CommandLine { + /// + /// A standard option that indicates that version information should be displayed for the app. + /// public sealed class VersionOption : CliOption { private CliAction? _action; From f06f115b3eb533831e26c246fafd615746bd8d63 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 6 Jun 2023 10:02:08 -0700 Subject: [PATCH 12/19] rename DiagramAction to ParseDiagramAction --- src/System.CommandLine/ParseDiagramDirective.cs | 2 +- src/System.CommandLine/ParseResult.cs | 2 +- src/System.CommandLine/Parsing/ParseDiagramAction.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/System.CommandLine/ParseDiagramDirective.cs b/src/System.CommandLine/ParseDiagramDirective.cs index d3f5d61ead..a9139a2593 100644 --- a/src/System.CommandLine/ParseDiagramDirective.cs +++ b/src/System.CommandLine/ParseDiagramDirective.cs @@ -21,7 +21,7 @@ public DiagramDirective() : base("diagram") /// public override CliAction? Action { - get => _action ??= new DiagramAction(ParseErrorReturnValue); + get => _action ??= new ParseDiagramAction(ParseErrorReturnValue); set => _action = value ?? throw new ArgumentNullException(nameof(value)); } diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 062a44d5b4..add745032d 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -197,7 +197,7 @@ static T Convert(ArgumentConversionResult validatedResult) } /// - public override string ToString() => DiagramAction.Diagram(this).ToString(); + public override string ToString() => ParseDiagramAction.Diagram(this).ToString(); /// /// Gets the result, if any, for the specified argument. diff --git a/src/System.CommandLine/Parsing/ParseDiagramAction.cs b/src/System.CommandLine/Parsing/ParseDiagramAction.cs index 0a8f86bfc0..9420a79b37 100644 --- a/src/System.CommandLine/Parsing/ParseDiagramAction.cs +++ b/src/System.CommandLine/Parsing/ParseDiagramAction.cs @@ -13,11 +13,11 @@ namespace System.CommandLine.Parsing /// Implements the [diagram] directive action, which when specified on the command line /// will short circuit normal command handling and display a diagram explaining the parse result for the command line input. /// - internal sealed class DiagramAction : SynchronousCliAction + internal sealed class ParseDiagramAction : SynchronousCliAction { private readonly int _parseErrorReturnValue; - internal DiagramAction(int parseErrorReturnValue) => _parseErrorReturnValue = parseErrorReturnValue; + internal ParseDiagramAction(int parseErrorReturnValue) => _parseErrorReturnValue = parseErrorReturnValue; public override int Invoke(ParseResult parseResult) { From 69f420570107f9dd7be242dbeeec76d8d6489394 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 6 Jun 2023 10:10:35 -0700 Subject: [PATCH 13/19] split CompletionAction into a separate file --- .../CompletionTests.cs | 1 - .../Completions/CompletionAction.cs | 40 +++++++++++ .../Completions/SuggestDirective.cs | 67 +++++-------------- .../Parsing/ParseDiagramAction.cs | 3 +- 4 files changed, 59 insertions(+), 52 deletions(-) create mode 100644 src/System.CommandLine/Completions/CompletionAction.cs diff --git a/src/System.CommandLine.Tests/CompletionTests.cs b/src/System.CommandLine.Tests/CompletionTests.cs index c1704e4735..a69df4fc81 100644 --- a/src/System.CommandLine.Tests/CompletionTests.cs +++ b/src/System.CommandLine.Tests/CompletionTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.CommandLine.Completions; -using System.CommandLine.Parsing; using System.CommandLine.Tests.Utility; using System.IO; using System.Linq; diff --git a/src/System.CommandLine/Completions/CompletionAction.cs b/src/System.CommandLine/Completions/CompletionAction.cs new file mode 100644 index 0000000000..b4c0592730 --- /dev/null +++ b/src/System.CommandLine/Completions/CompletionAction.cs @@ -0,0 +1,40 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; +using System.Linq; + +namespace System.CommandLine.Completions; + +/// +/// Implements the action for the [suggest] directive, which when specified on the command line will short circuit normal command handling and display a diagram explaining the parse result for the command line input. +/// +internal sealed class CompletionAction : SynchronousCliAction +{ + private readonly SuggestDirective _directive; + + internal CompletionAction(SuggestDirective suggestDirective) => _directive = suggestDirective; + + /// + public override int Invoke(ParseResult parseResult) + { + string? parsedValues = parseResult.GetResult(_directive)!.Values.SingleOrDefault(); + string? rawInput = parseResult.CommandLineText; + + int position = !string.IsNullOrEmpty(parsedValues) ? int.Parse(parsedValues) : rawInput?.Length ?? 0; + + var commandLineToComplete = parseResult.Tokens.LastOrDefault(t => t.Type != CliTokenType.Directive)?.Value ?? ""; + + var completionParseResult = parseResult.RootCommandResult.Command.Parse(commandLineToComplete, parseResult.Configuration); + + var completions = completionParseResult.GetCompletions(position); + + parseResult.Configuration.Output.WriteLine( + string.Join( + Environment.NewLine, + completions)); + + return 0; + } +} \ No newline at end of file diff --git a/src/System.CommandLine/Completions/SuggestDirective.cs b/src/System.CommandLine/Completions/SuggestDirective.cs index 86584baec7..16812b0794 100644 --- a/src/System.CommandLine/Completions/SuggestDirective.cs +++ b/src/System.CommandLine/Completions/SuggestDirective.cs @@ -1,56 +1,25 @@ using System.CommandLine.Invocation; -using System.CommandLine.Parsing; -using System.Linq; -namespace System.CommandLine.Completions -{ - /// - /// Enables the use of the [suggest] directive which when specified in command line input short circuits normal command handling and writes a newline-delimited list of suggestions suitable for use by most shells to provide command line completions. - /// - /// The dotnet-suggest tool requires the suggest directive to be enabled for an application to provide completions. - public sealed class SuggestDirective : CliDirective - { - private CliAction? _action; - - /// - public SuggestDirective() : base("suggest") - { - } - - /// - public override CliAction? Action - { - get => _action ??= new SuggestDirectiveAction(this); - set => _action = value ?? throw new ArgumentNullException(nameof(value)); - } - - private sealed class SuggestDirectiveAction : SynchronousCliAction - { - // FIX: (SuggestDirectiveAction) rename (CompletionAction?), make public - private readonly SuggestDirective _directive; - - internal SuggestDirectiveAction(SuggestDirective suggestDirective) => _directive = suggestDirective; - - public override int Invoke(ParseResult parseResult) - { - string? parsedValues = parseResult.GetResult(_directive)!.Values.SingleOrDefault(); - string? rawInput = parseResult.CommandLineText; - - int position = !string.IsNullOrEmpty(parsedValues) ? int.Parse(parsedValues) : rawInput?.Length ?? 0; +namespace System.CommandLine.Completions; - var commandLineToComplete = parseResult.Tokens.LastOrDefault(t => t.Type != CliTokenType.Directive)?.Value ?? ""; - - var completionParseResult = parseResult.RootCommandResult.Command.Parse(commandLineToComplete, parseResult.Configuration); - - var completions = completionParseResult.GetCompletions(position); +/// +/// Enables the use of the [suggest] directive which when specified in command line input short circuits normal command handling and writes a newline-delimited list of suggestions suitable for use by most shells to provide command line completions. +/// +/// The dotnet-suggest tool requires the suggest directive to be enabled for an application to provide completions. +public sealed class SuggestDirective : CliDirective +{ + private CliAction? _action; - parseResult.Configuration.Output.WriteLine( - string.Join( - Environment.NewLine, - completions)); + /// + public SuggestDirective() : base("suggest") + { + } - return 0; - } - } + /// + public override CliAction? Action + { + get => _action ??= new CompletionAction(this); + set => _action = value ?? throw new ArgumentNullException(nameof(value)); } + } \ No newline at end of file diff --git a/src/System.CommandLine/Parsing/ParseDiagramAction.cs b/src/System.CommandLine/Parsing/ParseDiagramAction.cs index 9420a79b37..474fcfd3af 100644 --- a/src/System.CommandLine/Parsing/ParseDiagramAction.cs +++ b/src/System.CommandLine/Parsing/ParseDiagramAction.cs @@ -10,8 +10,7 @@ namespace System.CommandLine.Parsing { /// - /// Implements the [diagram] directive action, which when specified on the command line - /// will short circuit normal command handling and display a diagram explaining the parse result for the command line input. + /// Implements the [diagram] directive action, which when specified on the command line will short circuit normal command handling and display a diagram explaining the parse result for the command line input. /// internal sealed class ParseDiagramAction : SynchronousCliAction { From 7e0b55ad55e00fbe176bcf1ed45d77edb578231a Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 6 Jun 2023 10:10:41 -0700 Subject: [PATCH 14/19] update baseline --- ...sts.System_CommandLine_api_is_not_changed.approved.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 5be1c526bd..3cfe6b523a 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -45,7 +45,9 @@ System.CommandLine public System.Collections.Generic.IList Subcommands { get; } public System.Boolean TreatUnmatchedTokensAsErrors { get; set; } public System.Collections.Generic.List> Validators { get; } - public System.Void Add(CliSymbol symbol) + public System.Void Add(CliArgument argument) + public System.Void Add(CliOption option) + public System.Void Add(CliCommand command) public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) public System.Collections.Generic.IEnumerator GetEnumerator() public ParseResult Parse(System.Collections.Generic.IReadOnlyList args, CliConfiguration configuration = null) @@ -222,8 +224,8 @@ System.CommandLine.Invocation public abstract class AsynchronousCliAction : CliAction public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) public abstract class CliAction - public System.Boolean Exclusive { get; } - protected System.Void set_Exclusive(System.Boolean value) + public System.Boolean Terminating { get; } + protected System.Void set_Terminating(System.Boolean value) public class ParseErrorAction : SynchronousCliAction .ctor() public System.Boolean ShowHelp { get; set; } From c65020e16e2afb34429dddf12dc0dd93bf784762 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 6 Jun 2023 10:29:58 -0700 Subject: [PATCH 15/19] exclude benchmarks project --- System.CommandLine.sln | 15 --------------- .../CommandLine/Perf_Parser_TypoCorrection.cs | 1 - 2 files changed, 16 deletions(-) diff --git a/System.CommandLine.sln b/System.CommandLine.sln index 84234d3f2a..d7159951ca 100644 --- a/System.CommandLine.sln +++ b/System.CommandLine.sln @@ -50,8 +50,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Renderin EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Rendering.Tests", "src\System.CommandLine.Rendering.Tests\System.CommandLine.Rendering.Tests.csproj", "{9E574595-A9CD-441A-9328-1D4DD5B531E8}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Benchmarks", "src\System.CommandLine.Benchmarks\System.CommandLine.Benchmarks.csproj", "{C39B0705-993E-43DB-B66A-A37A587F0BF7}" -EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Hosting", "src\System.CommandLine.Hosting\System.CommandLine.Hosting.csproj", "{644C4B4A-4A32-4307-9F71-C3BF901FFB66}" EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.CommandLine.Hosting.Tests", "src\System.CommandLine.Hosting.Tests\System.CommandLine.Hosting.Tests.csproj", "{39483140-BC26-4CAD-BBAE-3DC76C2F16CF}" @@ -196,18 +194,6 @@ Global {9E574595-A9CD-441A-9328-1D4DD5B531E8}.Release|x64.Build.0 = Release|Any CPU {9E574595-A9CD-441A-9328-1D4DD5B531E8}.Release|x86.ActiveCfg = Release|Any CPU {9E574595-A9CD-441A-9328-1D4DD5B531E8}.Release|x86.Build.0 = Release|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Debug|Any CPU.Build.0 = Debug|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Debug|x64.ActiveCfg = Debug|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Debug|x64.Build.0 = Debug|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Debug|x86.ActiveCfg = Debug|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Debug|x86.Build.0 = Debug|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Release|Any CPU.ActiveCfg = Release|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Release|Any CPU.Build.0 = Release|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Release|x64.ActiveCfg = Release|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Release|x64.Build.0 = Release|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Release|x86.ActiveCfg = Release|Any CPU - {C39B0705-993E-43DB-B66A-A37A587F0BF7}.Release|x86.Build.0 = Release|Any CPU {644C4B4A-4A32-4307-9F71-C3BF901FFB66}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {644C4B4A-4A32-4307-9F71-C3BF901FFB66}.Debug|Any CPU.Build.0 = Debug|Any CPU {644C4B4A-4A32-4307-9F71-C3BF901FFB66}.Debug|x64.ActiveCfg = Debug|Any CPU @@ -307,7 +293,6 @@ Global {8D9A8DCB-DC74-4B3A-B1C6-046C9C4F458E} = {6749FB3E-39DE-4321-A39E-525278E9408D} {27E3BFFC-4412-4E4C-A656-B9D35B8A0F3E} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {9E574595-A9CD-441A-9328-1D4DD5B531E8} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} - {C39B0705-993E-43DB-B66A-A37A587F0BF7} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {644C4B4A-4A32-4307-9F71-C3BF901FFB66} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {39483140-BC26-4CAD-BBAE-3DC76C2F16CF} = {E5B1EC71-0FC4-4FAA-9C65-32D5016FBC45} {0BF6958D-9EE3-4623-B3D6-4DA77EAC1906} = {6749FB3E-39DE-4321-A39E-525278E9408D} diff --git a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_TypoCorrection.cs b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_TypoCorrection.cs index 10a584604b..90a7350f25 100644 --- a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_TypoCorrection.cs +++ b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_TypoCorrection.cs @@ -23,7 +23,6 @@ public Perf_Parser_TypoCorrection() _configuration = new CliConfiguration(new CliRootCommand { option }) { - EnableTypoCorrections = true, Output = System.IO.TextWriter.Null }; } From 008524c9f464739e2c267b5092cd5d52ea216c56 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Wed, 7 Jun 2023 13:56:45 -0700 Subject: [PATCH 16/19] Address PR comments --- .../DirectiveTests.cs | 4 +- .../HelpOptionTests.cs | 3 +- .../Invocation/InvocationTests.cs | 39 +++++++++++++++---- .../Invocation/TypoCorrectionTests.cs | 8 +++- .../ParseErrorReportingTests.cs | 12 +----- .../TestCliActions.cs | 4 +- src/System.CommandLine/CliConfiguration.cs | 5 --- .../AnonymousAsynchronousCliAction.cs | 2 +- .../AnonymousSynchronousCliAction.cs | 2 +- .../Invocation/CliAction.cs | 2 +- .../Invocation/InvocationPipeline.cs | 16 ++++---- src/System.CommandLine/ParseResult.cs | 14 +++---- .../Parsing/ParseOperation.cs | 3 +- 13 files changed, 64 insertions(+), 50 deletions(-) diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index ce62c50b22..215fd6181c 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -72,14 +72,14 @@ public async Task Multiple_instances_of_the_same_directive_can_be_invoked(bool i var testDirective = new TestDirective("test") { Action = invokeAsync - ? new AsynchronousNonterminatingTestAction(incrementCallCount, terminating: false) + ? new AsynchronousTestAction(incrementCallCount, terminating: false) : new SynchronousTestAction(incrementCallCount, terminating: false) }; var config = new CliConfiguration(new CliRootCommand { Action = invokeAsync - ? new AsynchronousNonterminatingTestAction(verifyActionWasCalled, terminating: false) + ? new AsynchronousTestAction(verifyActionWasCalled, terminating: false) : new SynchronousTestAction(verifyActionWasCalled, terminating: false) }) { diff --git a/src/System.CommandLine.Tests/HelpOptionTests.cs b/src/System.CommandLine.Tests/HelpOptionTests.cs index 304e3bc2c0..ccdda34c1f 100644 --- a/src/System.CommandLine.Tests/HelpOptionTests.cs +++ b/src/System.CommandLine.Tests/HelpOptionTests.cs @@ -179,12 +179,11 @@ public async Task Help_option_with_custom_aliases_default_aliases_replaced(strin CliConfiguration config = new(command) { Output = new StringWriter(), - EnableParseErrorReporting = false }; await config.InvokeAsync(helpAlias); - config.Output.ToString().Should().Be(""); + config.Output.ToString().Should().NotContain(helpAlias); } } } \ No newline at end of file diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index 0fdeb85b4e..b9d5fec1e6 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -5,7 +5,6 @@ using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.IO; -using System.Runtime.InteropServices.ComTypes; using System.Threading; using System.Threading.Tasks; using FluentAssertions; @@ -195,10 +194,10 @@ public async Task Anonymous_RootCommand_int_returning_Action_can_set_custom_resu } [Fact] - public void Option_action_takes_precedence_over_command_action() + public void Terminating_option_action_short_circuits_command_action() { - bool wasCalled = false; - SynchronousTestAction optionAction = new(_ => wasCalled = true); + bool optionActionWasCalled = false; + SynchronousTestAction optionAction = new(_ => optionActionWasCalled = true, terminating: true); bool commandActionWasCalled = false; CliOption option = new("--test") @@ -214,17 +213,41 @@ public void Option_action_takes_precedence_over_command_action() commandActionWasCalled = true; }); - ParseResult parseResult = command.Parse("cmd --test true"); + ParseResult parseResult = command.Parse("cmd --test"); parseResult.Action.Should().NotBeNull(); - wasCalled.Should().BeFalse(); + optionActionWasCalled.Should().BeFalse(); commandActionWasCalled.Should().BeFalse(); parseResult.Invoke().Should().Be(0); - wasCalled.Should().BeTrue(); + optionActionWasCalled.Should().BeTrue(); commandActionWasCalled.Should().BeFalse(); } + [Fact] + public void Nonterminating_option_action_does_not_short_circuit_command_action() + { + bool optionActionWasCalled = false; + SynchronousTestAction optionAction = new(_ => optionActionWasCalled = true, terminating: false); + bool commandActionWasCalled = false; + + CliOption option = new("--test") + { + Action = optionAction + }; + CliCommand command = new CliCommand("cmd") + { + option + }; + command.SetAction(_ => { commandActionWasCalled = true; }); + + ParseResult parseResult = command.Parse("cmd --test"); + + parseResult.Invoke().Should().Be(0); + optionActionWasCalled.Should().BeTrue(); + commandActionWasCalled.Should().BeTrue(); + } + [Fact] public void When_multiple_options_with_actions_are_present_then_only_the_last_one_is_invoked() { @@ -286,7 +309,7 @@ public void Directive_action_takes_precedence_over_option_action() [Theory] [InlineData(true)] [InlineData(false)] - public async Task Nonexclusive_actions_handle_exceptions_and_return_an_error_return_code(bool invokeAsync) + public async Task Nontermninating_actions_handle_exceptions_and_return_an_error_return_code(bool invokeAsync) { var nonexclusiveAction = new NonexclusiveTestAction { diff --git a/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs b/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs index 6aef6c2ec3..1ff2552479 100644 --- a/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs +++ b/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs @@ -1,5 +1,7 @@ +using System.CommandLine.Help; using System.CommandLine.Invocation; using System.IO; +using System.Linq; using System.Threading.Tasks; using FluentAssertions; using Xunit; @@ -151,14 +153,18 @@ public async Task Arguments_are_not_suggested() argument, command }; + CliConfiguration configuration = new(rootCommand) { - EnableParseErrorReporting = false, Output = new StringWriter() }; var result = rootCommand.Parse("een", configuration); + var parseErrorAction = (ParseErrorAction)result.Action; + parseErrorAction.ShowHelp = false; + parseErrorAction.ShowTypoCorrections = true; + await result.InvokeAsync(); configuration.Output.ToString().Should().NotContain("the-argument"); diff --git a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs index b9c626b30c..cc91854881 100644 --- a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs +++ b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs @@ -1,9 +1,6 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - using System.CommandLine.Help; using System.CommandLine.Invocation; using System.IO; @@ -23,16 +20,11 @@ public void Parse_error_reporting_reports_error_when_help_is_used_and_required_s new HelpOption() }; - CliConfiguration config = new (root) - { - EnableParseErrorReporting = true - }; - - var parseResult = root.Parse("", config); + var parseResult = root.Parse(""); parseResult.Errors.Should().NotBeEmpty(); - var result = config.Invoke(""); + var result = parseResult.Invoke(); result.Should().Be(1); } diff --git a/src/System.CommandLine.Tests/TestCliActions.cs b/src/System.CommandLine.Tests/TestCliActions.cs index b07818d0b8..d02f46625e 100644 --- a/src/System.CommandLine.Tests/TestCliActions.cs +++ b/src/System.CommandLine.Tests/TestCliActions.cs @@ -24,11 +24,11 @@ public override int Invoke(ParseResult parseResult) } } -public class AsynchronousNonterminatingTestAction : AsynchronousCliAction +public class AsynchronousTestAction : AsynchronousCliAction { private readonly Action _invoke; - public AsynchronousNonterminatingTestAction(Action invoke, bool terminating = true) + public AsynchronousTestAction(Action invoke, bool terminating = true) { _invoke = invoke; Terminating = terminating; diff --git a/src/System.CommandLine/CliConfiguration.cs b/src/System.CommandLine/CliConfiguration.cs index 04c7d96c7f..bebc4e60b4 100644 --- a/src/System.CommandLine/CliConfiguration.cs +++ b/src/System.CommandLine/CliConfiguration.cs @@ -65,11 +65,6 @@ public CliConfiguration(CliCommand rootCommand) /// public bool EnableDefaultExceptionHandler { get; set; } = true; - /// - /// Configures the command line to write error information to standard error when there are errors parsing command line input. Enabled by default. - /// - public bool EnableParseErrorReporting { get; set; } = true; - /// /// Enables signaling and handling of process termination (Ctrl+C, SIGINT, SIGTERM) via a /// that can be passed to a during invocation. diff --git a/src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs b/src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs index 329a6e2312..5bc4bd490f 100644 --- a/src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs +++ b/src/System.CommandLine/Invocation/AnonymousAsynchronousCliAction.cs @@ -6,7 +6,7 @@ namespace System.CommandLine.Invocation; -internal class AnonymousAsynchronousCliAction : AsynchronousCliAction +internal sealed class AnonymousAsynchronousCliAction : AsynchronousCliAction { private readonly Func> _asyncAction; diff --git a/src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs b/src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs index f2e2893558..683f2d5010 100644 --- a/src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs +++ b/src/System.CommandLine/Invocation/AnonymousSynchronousCliAction.cs @@ -3,7 +3,7 @@ namespace System.CommandLine.Invocation; -internal class AnonymousSynchronousCliAction : SynchronousCliAction +internal sealed class AnonymousSynchronousCliAction : SynchronousCliAction { private readonly Func _syncAction; diff --git a/src/System.CommandLine/Invocation/CliAction.cs b/src/System.CommandLine/Invocation/CliAction.cs index 671af3b041..e004c62226 100644 --- a/src/System.CommandLine/Invocation/CliAction.cs +++ b/src/System.CommandLine/Invocation/CliAction.cs @@ -15,5 +15,5 @@ private protected CliAction() /// /// Indicates that the action terminates a command line invocation, and later actions are skipped. /// - public bool Terminating { get; protected set; } = true; + public bool Terminating { get; protected init; } = true; } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index e88608bc32..232638e3d8 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -20,11 +20,11 @@ internal static async Task InvokeAsync(ParseResult parseResult, Cancellatio try { - if (parseResult.NonexclusiveActions is not null) + if (parseResult.PreActions is not null) { - for (int i = 0; i < parseResult.NonexclusiveActions.Count; i++) + for (int i = 0; i < parseResult.PreActions.Count; i++) { - var action = parseResult.NonexclusiveActions[i]; + var action = parseResult.PreActions[i]; switch (action) { @@ -87,12 +87,12 @@ internal static int Invoke(ParseResult parseResult) case SynchronousCliAction syncAction: try { - if (parseResult.NonexclusiveActions is not null) + if (parseResult.PreActions is not null) { #if DEBUG - for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) + for (var i = 0; i < parseResult.PreActions.Count; i++) { - var action = parseResult.NonexclusiveActions[i]; + var action = parseResult.PreActions[i]; if (action is not SynchronousCliAction) { @@ -103,9 +103,9 @@ internal static int Invoke(ParseResult parseResult) } #endif - for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) + for (var i = 0; i < parseResult.PreActions.Count; i++) { - if (parseResult.NonexclusiveActions[i] is SynchronousCliAction syncPreaction) + if (parseResult.PreActions[i] is SynchronousCliAction syncPreaction) { syncPreaction.Invoke(parseResult); } diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index add745032d..fc3af8a7f9 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -21,7 +21,7 @@ public sealed class ParseResult private readonly IReadOnlyList _unmatchedTokens; private CompletionContext? _completionContext; private readonly CliAction? _action; - private readonly List? _nonexclusiveActions; + private readonly List? _preActions; private Dictionary? _namedResults; internal ParseResult( @@ -33,13 +33,13 @@ internal ParseResult( List? errors, string? commandLineText = null, CliAction? action = null, - List? nonexclusiveActions = null) + List? preActions = null) { Configuration = configuration; _rootCommandResult = rootCommandResult; CommandResult = commandResult; _action = action; - _nonexclusiveActions = nonexclusiveActions; + _preActions = preActions; // skip the root command when populating Tokens property if (tokens.Count > 1) @@ -305,11 +305,11 @@ public int Invoke() { useAsync = true; } - else if (NonexclusiveActions is not null) + else if (PreActions is not null) { - for (var i = 0; i < NonexclusiveActions.Count; i++) + for (var i = 0; i < PreActions.Count; i++) { - var action = NonexclusiveActions[i]; + var action = PreActions[i]; if (action is AsynchronousCliAction) { useAsync = true; @@ -334,7 +334,7 @@ public int Invoke() /// public CliAction? Action => _action ?? CommandResult.Command.Action; - internal IReadOnlyList? NonexclusiveActions => _nonexclusiveActions; + internal IReadOnlyList? PreActions => _preActions; private SymbolResult SymbolToComplete(int? position = null) { diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 4778b6de3e..0a7c950ca2 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -65,8 +65,7 @@ internal ParseResult Parse() if (_primaryAction is null) { - if (_configuration.EnableParseErrorReporting && - _symbolResultTree.ErrorCount > 0) + if (_symbolResultTree.ErrorCount > 0) { _primaryAction = new ParseErrorAction(); } From 51af30f49fb7d7a7f8003f0128119cd0ec2f782e Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Wed, 7 Jun 2023 23:06:40 -0700 Subject: [PATCH 17/19] support non-terminating option actions --- .../Invocation/InvocationTests.cs | 57 +++---------------- .../Invocation/InvocationPipeline.cs | 4 +- .../Parsing/ParseOperation.cs | 36 ++++++++---- 3 files changed, 34 insertions(+), 63 deletions(-) diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index b9d5fec1e6..f0c37460af 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -134,10 +134,8 @@ public void RootCommand_Invoke_returns_1_when_handler_throws() [Fact] public void Custom_RootCommand_Action_can_set_custom_result_code_via_Invoke() { - var rootCommand = new CliRootCommand - { - Action = new SynchronousCustomReturnCodeAction() - }; + var rootCommand = new CliRootCommand(); + rootCommand.SetAction(_ => 123); rootCommand.Parse("").Invoke().Should().Be(123); } @@ -145,10 +143,8 @@ public void Custom_RootCommand_Action_can_set_custom_result_code_via_Invoke() [Fact] public async Task Custom_RootCommand_Action_can_set_custom_result_code_via_InvokeAsync() { - var rootCommand = new CliRootCommand - { - Action = new AsynchronousCustomReturnCodeAction() - }; + var rootCommand = new CliRootCommand(); + rootCommand.SetAction((_, _) => Task.FromResult(456)); (await rootCommand.Parse("").InvokeAsync()).Should().Be(456); } @@ -239,7 +235,7 @@ public void Nonterminating_option_action_does_not_short_circuit_command_action() { option }; - command.SetAction(_ => { commandActionWasCalled = true; }); + command.SetAction(_ => commandActionWasCalled = true); ParseResult parseResult = command.Parse("cmd --test"); @@ -309,12 +305,9 @@ public void Directive_action_takes_precedence_over_option_action() [Theory] [InlineData(true)] [InlineData(false)] - public async Task Nontermninating_actions_handle_exceptions_and_return_an_error_return_code(bool invokeAsync) + public async Task Nontermninating_option_actions_handle_exceptions_and_return_an_error_return_code(bool invokeAsync) { - var nonexclusiveAction = new NonexclusiveTestAction - { - ThrowOnInvoke = true - }; + var nonexclusiveAction = new SynchronousTestAction(_ => throw new Exception("oops!"), terminating: false); var command = new CliRootCommand { @@ -323,6 +316,7 @@ public async Task Nontermninating_actions_handle_exceptions_and_return_an_error_ Action = nonexclusiveAction } }; + command.SetAction(_ => 0); int returnCode; @@ -354,40 +348,5 @@ public async Task Command_InvokeAsync_with_cancelation_token_invokes_command_han cts.Cancel(); await command.Parse("test").InvokeAsync(cancellationToken: cts.Token); } - - private class SynchronousCustomReturnCodeAction : SynchronousCliAction - { - public override int Invoke(ParseResult context) - => 123; - } - - private class AsynchronousCustomReturnCodeAction : AsynchronousCliAction - { - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) - => Task.FromResult(456); - } - - private class NonexclusiveTestAction : SynchronousCliAction - { - public NonexclusiveTestAction() - { - Terminating = false; - } - - public bool ThrowOnInvoke { get; set; } - - public bool HasBeenInvoked { get; private set; } - - public override int Invoke(ParseResult parseResult) - { - HasBeenInvoked = true; - if (ThrowOnInvoke) - { - throw new Exception("oops!"); - } - - return 0; - } - } } } diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 232638e3d8..6727a927b8 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -105,9 +105,9 @@ internal static int Invoke(ParseResult parseResult) for (var i = 0; i < parseResult.PreActions.Count; i++) { - if (parseResult.PreActions[i] is SynchronousCliAction syncPreaction) + if (parseResult.PreActions[i] is SynchronousCliAction syncPreAction) { - syncPreaction.Invoke(parseResult); + syncPreAction.Invoke(parseResult); } } } diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 0a7c950ca2..88cc656791 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -20,7 +20,7 @@ internal sealed class ParseOperation private bool _isHelpRequested; private bool _isTerminatingDirectiveSpecified; private CliAction? _primaryAction; - private List? _nonexclusiveActions; + private List? _preActions; public ParseOperation( List tokens, @@ -80,7 +80,7 @@ internal ParseResult Parse() _symbolResultTree.Errors, _rawInput, _primaryAction, - _nonexclusiveActions); + _preActions); } private void ParseSubcommand() @@ -186,17 +186,24 @@ private void ParseOption() if (!_symbolResultTree.TryGetValue(option, out SymbolResult? symbolResult)) { - // directives have a precedence over --help and --version - if (!_isTerminatingDirectiveSpecified) + if (option.Action is not null) { - if (option.Action is not null) + // directives have a precedence over --help and --version + if (!_isTerminatingDirectiveSpecified) { if (option is HelpOption) { _isHelpRequested = true; } - _primaryAction = option.Action; + if (option.Action.Terminating) + { + _primaryAction = option.Action; + } + else + { + AddPreAction(option.Action); + } } } @@ -334,17 +341,22 @@ void ParseDirective() } else { - if (_nonexclusiveActions is null) - { - _nonexclusiveActions = new(); - } - - _nonexclusiveActions.Add(directive.Action); + AddPreAction(directive.Action); } } } } + private void AddPreAction(CliAction action) + { + if (_preActions is null) + { + _preActions = new(); + } + + _preActions.Add(action); + } + private void AddCurrentTokenToUnmatched() { if (CurrentToken.Type == CliTokenType.DoubleDash) From 8fd95c4570e42636f3853858ad744659820cba6b Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Thu, 8 Jun 2023 09:00:23 -0700 Subject: [PATCH 18/19] reuse IsBoolean() method --- src/System.CommandLine/CliArgument.cs | 2 +- src/System.CommandLine/Parsing/ParseOperation.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/CliArgument.cs b/src/System.CommandLine/CliArgument.cs index 868dcb1cff..aa453bfd72 100644 --- a/src/System.CommandLine/CliArgument.cs +++ b/src/System.CommandLine/CliArgument.cs @@ -61,7 +61,7 @@ public List>> CompletionSour if (_completionSources is null) { Type? valueType = ValueType; - if (valueType == typeof(bool) || valueType == typeof(bool?)) + if (IsBoolean()) { _completionSources = new () { diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 88cc656791..778b813528 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -248,8 +248,7 @@ private void ParseOptionArguments(OptionResult optionResult) break; } } - else if ((argument.ValueType == typeof(bool) || argument.ValueType == typeof(bool?)) && - !bool.TryParse(CurrentToken.Value, out _)) + else if (argument.IsBoolean() && !bool.TryParse(CurrentToken.Value, out _)) { // Don't greedily consume the following token for bool. The presence of the option token (i.e. a flag) is sufficient. break; From 801bf541f480c1614f9c81c27ae3c3fe825f26d9 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Thu, 8 Jun 2023 10:03:48 -0700 Subject: [PATCH 19/19] update API baseline --- ...rovalTests.System_CommandLine_api_is_not_changed.approved.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index 3cfe6b523a..c90623e373 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -60,7 +60,6 @@ System.CommandLine .ctor(CliCommand rootCommand) public System.Collections.Generic.List Directives { get; } public System.Boolean EnableDefaultExceptionHandler { get; set; } - public System.Boolean EnableParseErrorReporting { get; set; } public System.Boolean EnablePosixBundling { get; set; } public System.IO.TextWriter Error { get; set; } public System.IO.TextWriter Output { get; set; }