From d798e5f09702fddb851a7afefed485929d3941f2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik <adam.sitnik@gmail.com> Date: Fri, 17 Mar 2023 16:05:40 +0100 Subject: [PATCH 1/4] expose HelpOption and HelpAction, move help configuration to the action --- ...ommandLine_api_is_not_changed.approved.txt | 15 ++- .../GeneratedCommandHandlerTests.cs | 8 +- .../Parameters/HelpBuilderParameter.cs | 27 ------ .../WellKnownTypes.cs | 6 -- .../ServiceProvider.cs | 1 - .../Help/HelpBuilderTests.Customization.cs | 54 +++++++---- .../Invocation/InvocationPipelineTests.cs | 57 +----------- src/System.CommandLine.Tests/UseHelpTests.cs | 93 ++++++++++++------- .../Builder/CommandLineBuilder.cs | 27 ------ .../Builder/CommandLineBuilderExtensions.cs | 44 ++------- .../CommandLineConfiguration.cs | 19 ---- src/System.CommandLine/Help/HelpOption.cs | 42 +-------- .../Help/HelpOptionAction.cs | 40 ++++++++ src/System.CommandLine/IO/SystemConsole.cs | 2 - .../Invocation/InvocationContext.cs | 7 -- 15 files changed, 165 insertions(+), 277 deletions(-) delete mode 100644 src/System.CommandLine.Generator/Parameters/HelpBuilderParameter.cs create mode 100644 src/System.CommandLine/Help/HelpOptionAction.cs 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 3b6cc1db73..ff56981976 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 @@ -74,8 +74,6 @@ System.CommandLine public CommandLineBuilder UseExceptionHandler(System.Func<System.Exception,System.CommandLine.Invocation.InvocationContext,System.Int32> onException = null, System.Int32 errorExitCode = 1) public CommandLineBuilder UseHelp(System.Nullable<System.Int32> maxWidth = null) public CommandLineBuilder UseHelp(System.String name, System.String[] helpAliases) - public CommandLineBuilder UseHelp(System.Action<System.CommandLine.Help.HelpContext> customize, System.Nullable<System.Int32> maxWidth = null) - public CommandLineBuilder UseHelpBuilder(System.Func<System.CommandLine.Invocation.InvocationContext,System.CommandLine.Help.HelpBuilder> getHelpBuilder) public CommandLineBuilder UseParseDirective(System.Int32 errorExitCode = 1) public CommandLineBuilder UseParseErrorReporting(System.Int32 errorExitCode = 1) public CommandLineBuilder UseSuggestDirective() @@ -85,7 +83,7 @@ System.CommandLine public CommandLineBuilder UseVersionOption(System.String name, System.String[] aliases) public class CommandLineConfiguration public static CommandLineBuilder CreateBuilder(Command rootCommand) - .ctor(Command command, System.Boolean enablePosixBundling = True, System.Boolean enableTokenReplacement = True, System.Collections.Generic.IReadOnlyList<System.CommandLine.Invocation.InvocationMiddleware> middlewarePipeline = null, System.Func<System.CommandLine.Invocation.InvocationContext,System.CommandLine.Help.HelpBuilder> helpBuilderFactory = null, System.CommandLine.Parsing.TryReplaceToken tokenReplacer = null) + .ctor(Command command, System.Boolean enablePosixBundling = True, System.Boolean enableTokenReplacement = True, System.Collections.Generic.IReadOnlyList<System.CommandLine.Invocation.InvocationMiddleware> middlewarePipeline = null, System.CommandLine.Parsing.TryReplaceToken tokenReplacer = null) public System.Collections.Generic.IReadOnlyList<Directive> Directives { get; } public System.Boolean EnablePosixBundling { get; } public System.Boolean EnableTokenReplacement { get; } @@ -201,6 +199,11 @@ System.CommandLine.Completions public TextCompletionContext AtCursorPosition(System.Int32 position) public class TokenCompletionContext : CompletionContext System.CommandLine.Help + public class HelpAction : System.CommandLine.CliAction + .ctor() + public HelpBuilder Builder { get; set; } + public System.Int32 Invoke(System.CommandLine.Invocation.InvocationContext context) + public System.Threading.Tasks.Task<System.Int32> InvokeAsync(System.CommandLine.Invocation.InvocationContext context, System.Threading.CancellationToken cancellationToken = null) public class HelpBuilder .ctor(System.Int32 maxWidth = 2147483647) public System.Int32 MaxWidth { get; } @@ -231,6 +234,11 @@ System.CommandLine.Help public HelpBuilder HelpBuilder { get; } public System.IO.TextWriter Output { get; } public System.CommandLine.ParseResult ParseResult { get; } + public class HelpOption : System.CommandLine.Option<System.Boolean>, System.CommandLine.Binding.IValueDescriptor + .ctor(System.String name, System.String[] aliases) + .ctor() + public System.Boolean Equals(System.Object obj) + public System.Int32 GetHashCode() public class TwoColumnHelpRow, System.IEquatable<TwoColumnHelpRow> .ctor(System.String firstColumnText, System.String secondColumnText) public System.String FirstColumnText { get; } @@ -242,7 +250,6 @@ System.CommandLine.Invocation public class InvocationContext .ctor(System.CommandLine.ParseResult parseResult, System.CommandLine.IConsole console = null) public System.CommandLine.IConsole Console { get; set; } - public System.CommandLine.Help.HelpBuilder HelpBuilder { get; } public System.CommandLine.ParseResult ParseResult { get; set; } public T GetValue<T>(Option<T> option) public T GetValue<T>(Argument<T> argument) diff --git a/src/System.CommandLine.Generator.Tests/GeneratedCommandHandlerTests.cs b/src/System.CommandLine.Generator.Tests/GeneratedCommandHandlerTests.cs index 91796ac8f4..721582646b 100644 --- a/src/System.CommandLine.Generator.Tests/GeneratedCommandHandlerTests.cs +++ b/src/System.CommandLine.Generator.Tests/GeneratedCommandHandlerTests.cs @@ -135,30 +135,26 @@ public async Task Can_generate_handler_with_well_know_parameters_types() InvocationContext? boundInvocationContext = null; IConsole? boundConsole = null; ParseResult? boundParseResult = null; - HelpBuilder? boundHelpBuilder = null; void Execute( InvocationContext invocationContext, IConsole console, - ParseResult parseResult, - HelpBuilder helpBuilder) + ParseResult parseResult) { boundInvocationContext = invocationContext; boundConsole = console; boundParseResult = parseResult; - boundHelpBuilder = helpBuilder; } var command = new Command("command"); - command.SetHandler<Action<InvocationContext, IConsole, ParseResult, HelpBuilder>>(Execute); + command.SetHandler<Action<InvocationContext, IConsole, ParseResult>>(Execute); await command.InvokeAsync("command", _console); boundInvocationContext.Should().NotBeNull(); boundConsole.Should().Be(_console); boundParseResult.Should().NotBeNull(); - boundHelpBuilder.Should().NotBeNull(); } [Fact] diff --git a/src/System.CommandLine.Generator/Parameters/HelpBuilderParameter.cs b/src/System.CommandLine.Generator/Parameters/HelpBuilderParameter.cs deleted file mode 100644 index e58ab4e8ab..0000000000 --- a/src/System.CommandLine.Generator/Parameters/HelpBuilderParameter.cs +++ /dev/null @@ -1,27 +0,0 @@ -using Microsoft.CodeAnalysis; - -namespace System.CommandLine.Generator.Parameters -{ - internal class HelpBuilderParameter : Parameter, IEquatable<HelpBuilderParameter> - { - public HelpBuilderParameter(ITypeSymbol helpBuilderType) - : base(helpBuilderType) - { - } - - public override string GetValueFromContext() - => "context.HelpBuilder"; - - public override int GetHashCode() - => base.GetHashCode(); - - public override bool Equals(object? obj) - => Equals(obj as HelpBuilderParameter); - - public bool Equals(HelpBuilderParameter? other) - { - if (other is null) return false; - return base.Equals(other); - } - } -} diff --git a/src/System.CommandLine.Generator/WellKnownTypes.cs b/src/System.CommandLine.Generator/WellKnownTypes.cs index 8586491283..58db8188e3 100644 --- a/src/System.CommandLine.Generator/WellKnownTypes.cs +++ b/src/System.CommandLine.Generator/WellKnownTypes.cs @@ -51,12 +51,6 @@ internal bool TryGet(ISymbol symbol, out Parameter? parameter) return true; } - if (Comparer.Equals(HelpBuilder, symbol)) - { - parameter = new HelpBuilderParameter(HelpBuilder); - return true; - } - if (symbol.MetadataName == "System.CommandLine.Binding.BindingContext" && symbol is INamedTypeSymbol bindingContext) { parameter = new BindingContextParameter(bindingContext); diff --git a/src/System.CommandLine.NamingConventionBinder/ServiceProvider.cs b/src/System.CommandLine.NamingConventionBinder/ServiceProvider.cs index b08fa94934..a0ce93c8be 100644 --- a/src/System.CommandLine.NamingConventionBinder/ServiceProvider.cs +++ b/src/System.CommandLine.NamingConventionBinder/ServiceProvider.cs @@ -18,7 +18,6 @@ internal ServiceProvider(BindingContext bindingContext) { [typeof(ParseResult)] = _ => bindingContext.ParseResult, [typeof(IConsole)] = _ => bindingContext.Console, - [typeof(HelpBuilder)] = _ => bindingContext.ParseResult.Configuration.HelpBuilderFactory(bindingContext.InvocationContext), [typeof(BindingContext)] = _ => bindingContext }; } diff --git a/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs b/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs index 078496dda4..4d98e11fb0 100644 --- a/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs +++ b/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs @@ -92,17 +92,21 @@ public void Option_can_customize_first_column_text_based_on_parse_result() ctx.Command.Equals(commandA) ? optionAFirstColumnText : optionBFirstColumnText); - var config = new CommandLineBuilder(command) - .UseDefaults() - .UseHelpBuilder(_ => helpBuilder) - .Build(); + command.Options.Add(new HelpOption() + { + Action = new HelpAction() + { + Builder = helpBuilder + } + }); var console = new TestConsole(); - config.Invoke("root a -h", console); + var config = CommandLineConfiguration.CreateBuilder(command).Build(); + command.Parse("root a -h", config).Invoke(console); console.Out.ToString().Should().Contain(optionAFirstColumnText); console = new TestConsole(); - config.Invoke("root b -h", console); + command.Parse("root b -h", config).Invoke(console); console.Out.ToString().Should().Contain(optionBFirstColumnText); } @@ -130,10 +134,15 @@ public void Option_can_customize_second_column_text_based_on_parse_result() ctx.Command.Equals(commandA) ? optionADescription : optionBDescription); + command.Options.Add(new HelpOption() + { + Action = new HelpAction() + { + Builder = helpBuilder + } + }); var config = new CommandLineBuilder(command) - .UseDefaults() - .UseHelpBuilder(_ => helpBuilder) .Build(); var console = new TestConsole(); @@ -253,14 +262,16 @@ public void Option_can_fallback_to_default_when_customizing(bool conditionA, boo firstColumnText: ctx => conditionA ? "custom 1st" : HelpBuilder.Default.GetOptionUsageLabel(option), secondColumnText: ctx => conditionB ? "custom 2nd" : option.Description ?? string.Empty); - - var config = new CommandLineBuilder(command) - .UseDefaults() - .UseHelpBuilder(_ => helpBuilder) - .Build(); + command.Options.Add(new HelpOption() + { + Action = new HelpAction() + { + Builder = helpBuilder + } + }); var console = new TestConsole(); - config.Invoke("test -h", console); + command.Parse("test -h", new CommandLineBuilder(command).Build()).Invoke(console); console.Out.ToString().Should().MatchRegex(expected); } @@ -295,13 +306,18 @@ public void Argument_can_fallback_to_default_when_customizing( defaultValue: ctx => conditionC ? "custom def" : HelpBuilder.Default.GetArgumentDefaultValue(argument)); - var config = new CommandLineBuilder(command) - .UseDefaults() - .UseHelpBuilder(_ => helpBuilder) - .Build(); + var config = new CommandLineBuilder(command).Build(); + + command.Options.Add(new HelpOption() + { + Action = new HelpAction() + { + Builder = helpBuilder + } + }); var console = new TestConsole(); - config.Invoke("test -h", console); + command.Parse("test -h", config).Invoke(console); console.Out.ToString().Should().MatchRegex(expected); } } diff --git a/src/System.CommandLine.Tests/Invocation/InvocationPipelineTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationPipelineTests.cs index c801cbc175..acbc526e6f 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationPipelineTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationPipelineTests.cs @@ -273,61 +273,12 @@ public void Synchronous_invocation_can_be_short_circuited_by_async_middleware_by } [Fact] - public async Task When_no_help_builder_is_specified_it_uses_default_implementation() + public void When_no_help_builder_is_specified_it_uses_default_implementation() { - bool handlerWasCalled = false; + HelpOption helpOption = new(); - var command = new Command("help-command"); - command.SetAction((context, cancellationToken) => - { - handlerWasCalled = true; - context.HelpBuilder.Should().NotBeNull(); - return Task.FromResult(0); - }); - - var config = new CommandLineBuilder(new RootCommand - { - command - }) - .Build(); - - await config.InvokeAsync("help-command", new TestConsole()); - - handlerWasCalled.Should().BeTrue(); - } - - [Fact] - public async Task When_help_builder_factory_is_specified_it_is_used_to_create_the_help_builder() - { - bool handlerWasCalled = false; - bool factoryWasCalled = false; - - HelpBuilder createdHelpBuilder = null; - - var command = new Command("help-command"); - command.SetAction((context, cancellationToken) => - { - handlerWasCalled = true; - context.HelpBuilder.Should().Be(createdHelpBuilder); - createdHelpBuilder.Should().NotBeNull(); - return Task.FromResult(0); - }); - - var config = new CommandLineBuilder(new RootCommand - { - command - }) - .UseHelpBuilder(context => - { - factoryWasCalled = true; - return createdHelpBuilder = new HelpBuilder(); - }) - .Build(); - - await config.InvokeAsync("help-command", new TestConsole()); - - handlerWasCalled.Should().BeTrue(); - factoryWasCalled.Should().BeTrue(); + helpOption.Action.Should().NotBeNull(); + (helpOption.Action as HelpAction).Builder.Should().NotBeNull(); } [Fact] diff --git a/src/System.CommandLine.Tests/UseHelpTests.cs b/src/System.CommandLine.Tests/UseHelpTests.cs index 3dc7268af0..ad1eceb357 100644 --- a/src/System.CommandLine.Tests/UseHelpTests.cs +++ b/src/System.CommandLine.Tests/UseHelpTests.cs @@ -270,17 +270,18 @@ public void Individual_symbols_can_be_customized() argument }; - var config = new CommandLineBuilder(rootCommand) - .UseHelp(ctx => - { - ctx.HelpBuilder.CustomizeSymbol(subcommand, secondColumnText: "The custom command description"); - ctx.HelpBuilder.CustomizeSymbol(option, secondColumnText: "The custom option description"); - ctx.HelpBuilder.CustomizeSymbol(argument, secondColumnText: "The custom argument description"); - }) - .Build(); - var console = new TestConsole(); - config.Invoke("-h", console); + + ParseResult parseResult = rootCommand.Parse("-h"); + + if (parseResult.Action is HelpAction helpAction) + { + helpAction.Builder.CustomizeSymbol(subcommand, secondColumnText: "The custom command description"); + helpAction.Builder.CustomizeSymbol(option, secondColumnText: "The custom option description"); + helpAction.Builder.CustomizeSymbol(argument, secondColumnText: "The custom argument description"); + } + + parseResult.Invoke(console); console.Out .ToString() @@ -294,11 +295,19 @@ public void Individual_symbols_can_be_customized() public void Help_sections_can_be_replaced() { var config = new CommandLineBuilder(new RootCommand()) - .UseHelp(ctx => ctx.HelpBuilder.CustomizeLayout(CustomLayout)) + .UseHelp() .Build(); var console = new TestConsole(); - config.Invoke("-h", console); + + ParseResult parseResult = config.RootCommand.Parse("-h"); + + if (parseResult.Action is HelpAction helpAction) + { + helpAction.Builder.CustomizeLayout(CustomLayout); + } + + parseResult.Invoke(console); console.Out.ToString().Should().Be($"one{NewLine}{NewLine}two{NewLine}{NewLine}three{NewLine}{NewLine}{NewLine}"); @@ -315,11 +324,18 @@ public void Help_sections_can_be_supplemented() { var command = new RootCommand("hello"); var config = new CommandLineBuilder(command) - .UseHelp(ctx => ctx.HelpBuilder.CustomizeLayout(CustomLayout)) + .UseHelp() .Build(); + ParseResult parseResult = config.RootCommand.Parse("-h"); + + if (parseResult.Action is HelpAction helpAction) + { + helpAction.Builder.CustomizeLayout(CustomLayout); + } + var console = new TestConsole(); - config.Invoke("-h", console); + parseResult.Invoke(console); var output = console.Out.ToString(); var defaultHelp = GetDefaultHelp(command); @@ -344,33 +360,37 @@ IEnumerable<Action<HelpContext>> CustomLayout(HelpContext _) [Fact] public void Layout_can_be_composed_dynamically_based_on_context() { + HelpBuilder helpBuilder = new(); var commandWithTypicalHelp = new Command("typical"); var commandWithCustomHelp = new Command("custom"); var command = new RootCommand { commandWithTypicalHelp, - commandWithCustomHelp + commandWithCustomHelp, + new HelpOption() + { + Action = new HelpAction() + { + Builder = helpBuilder + } + } }; - var config = new CommandLineBuilder(command) - .UseHelp( - ctx => - ctx.HelpBuilder - .CustomizeLayout(c => - c.Command == commandWithTypicalHelp - ? HelpBuilder.Default.GetLayout() - : new Action<HelpContext>[] - { - c => c.Output.WriteLine("Custom layout!") - } - .Concat(HelpBuilder.Default.GetLayout()))) - .Build(); + var config = new CommandLineBuilder(command).Build(); + helpBuilder.CustomizeLayout(c => + c.Command == commandWithTypicalHelp + ? HelpBuilder.Default.GetLayout() + : new Action<HelpContext>[] + { + c => c.Output.WriteLine("Custom layout!") + } + .Concat(HelpBuilder.Default.GetLayout())); var typicalOutput = new TestConsole(); - config.Invoke("typical -h", typicalOutput); + command.Parse("typical -h", config).Invoke(typicalOutput); var customOutput = new TestConsole(); - config.Invoke("custom -h", customOutput); + command.Parse("custom -h", config).Invoke(customOutput); typicalOutput.Out.ToString().Should().Be(GetDefaultHelp(commandWithTypicalHelp, false)); customOutput.Out.ToString().Should().Be($"Custom layout!{NewLine}{NewLine}{GetDefaultHelp(commandWithCustomHelp, false)}"); @@ -413,11 +433,20 @@ public void Help_default_sections_can_be_wrapped() public void Help_customized_sections_can_be_wrapped() { var config = new CommandLineBuilder(new RootCommand()) - .UseHelp(ctx => ctx.HelpBuilder.CustomizeLayout(CustomLayout), maxWidth: 10) + .UseHelp() .Build(); var console = new TestConsole(); - config.Invoke("-h", console); + + ParseResult parseResult = config.RootCommand.Parse("-h"); + + if (parseResult.Action is HelpAction helpAction) + { + helpAction.Builder = new HelpBuilder(10); + helpAction.Builder.CustomizeLayout(CustomLayout); + } + + parseResult.Invoke(console); string result = console.Out.ToString(); result.Should().Be($" 123 123{NewLine} 456 456{NewLine} 78 789{NewLine} 0{NewLine}{NewLine}{NewLine}"); diff --git a/src/System.CommandLine/Builder/CommandLineBuilder.cs b/src/System.CommandLine/Builder/CommandLineBuilder.cs index ddfcf44679..9d7c4149e2 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilder.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilder.cs @@ -35,8 +35,6 @@ public partial class CommandLineBuilder // (because each struct is of a different size) // that is why we don't use List<ValueTuple> for middleware private List<Tuple<InvocationMiddleware, int>>? _middlewareList; - private Action<HelpContext>? _customizeHelpBuilder; - private Func<InvocationContext, HelpBuilder>? _helpBuilderFactory; /// <param name="rootCommand">The root command of the application.</param> public CommandLineBuilder(Command rootCommand) @@ -49,34 +47,10 @@ public CommandLineBuilder(Command rootCommand) /// </summary> public Command Command { get; } - internal void CustomizeHelpLayout(Action<HelpContext> customize) => - _customizeHelpBuilder = customize; - - internal void UseHelpBuilderFactory(Func<InvocationContext, HelpBuilder> factory) => - _helpBuilderFactory = factory; - - private Func<InvocationContext, HelpBuilder> GetHelpBuilderFactory() - { - return CreateHelpBuilder; - - HelpBuilder CreateHelpBuilder(InvocationContext invocationContext) - { - var helpBuilder = _helpBuilderFactory is { } - ? _helpBuilderFactory(invocationContext) - : CommandLineConfiguration.DefaultHelpBuilderFactory(invocationContext, MaxHelpWidth); - - helpBuilder.OnCustomize = _customizeHelpBuilder; - - return helpBuilder; - } - } - internal HelpOption? HelpOption; internal VersionOption? VersionOption; - internal int? MaxHelpWidth; - internal TryReplaceToken? TokenReplacer; public List<Directive> Directives => _directives ??= new (); @@ -100,7 +74,6 @@ public CommandLineConfiguration Build() => middlewarePipeline: _middlewareList is null ? Array.Empty<InvocationMiddleware>() : GetMiddleware(), - helpBuilderFactory: GetHelpBuilderFactory(), tokenReplacer: TokenReplacer); private IReadOnlyList<InvocationMiddleware> GetMiddleware() diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index ea3efd76a0..6e31211733 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -141,7 +141,13 @@ int Default(Exception exception, InvocationContext context) /// <returns>The reference to this <see cref="CommandLineBuilder"/> instance.</returns> public CommandLineBuilder UseHelp(int? maxWidth = null) { - return UseHelp(new HelpOption(), maxWidth); + return UseHelp(new HelpOption() + { + Action = new HelpAction() + { + Builder = new HelpBuilder(maxWidth ?? int.MaxValue) + } + }); } /// <summary> @@ -156,51 +162,17 @@ public CommandLineBuilder UseHelp(string name, params string[] helpAliases) return UseHelp(new HelpOption(name, helpAliases)); } - /// <summary> - /// Configures the application to show help when one of the specified option aliases are used on the command line. - /// </summary> - /// <remarks>The specified aliases will override the default values.</remarks> - /// <param name="customize">A delegate that will be called to customize help if help is requested.</param> - /// <param name="maxWidth">Maximum output width for default help builder.</param> - /// <returns>The reference to this <see cref="CommandLineBuilder"/> instance.</returns> - public CommandLineBuilder UseHelp(Action<HelpContext> customize, int? maxWidth = null) - { - CustomizeHelpLayout(customize); - - if (HelpOption is null) - { - UseHelp(new HelpOption(), maxWidth); - } - - return this; - } - - internal CommandLineBuilder UseHelp(HelpOption helpOption, int? maxWidth = null) + internal CommandLineBuilder UseHelp(HelpOption helpOption) { if (HelpOption is null) { HelpOption = helpOption; OverwriteOrAdd(Command, helpOption); - - MaxHelpWidth = maxWidth; } return this; } - /// <summary> - /// Specifies an <see cref="HelpBuilder"/> to be used to format help output when help is requested. - /// </summary> - /// <param name="getHelpBuilder">A delegate that returns an instance of <see cref="HelpBuilder"/></param> - /// <returns>The reference to this <see cref="CommandLineBuilder"/> instance.</returns> - public CommandLineBuilder UseHelpBuilder( - Func<InvocationContext, HelpBuilder> getHelpBuilder) - { - UseHelpBuilderFactory(getHelpBuilder); - - return this; - } - /// <summary> /// Adds a middleware delegate to the invocation pipeline called before a command handler is invoked. /// </summary> diff --git a/src/System.CommandLine/CommandLineConfiguration.cs b/src/System.CommandLine/CommandLineConfiguration.cs index 1ee7632c14..e7fa045ec2 100644 --- a/src/System.CommandLine/CommandLineConfiguration.cs +++ b/src/System.CommandLine/CommandLineConfiguration.cs @@ -37,7 +37,6 @@ public class CommandLineConfiguration internal readonly IReadOnlyList<InvocationMiddleware> Middleware; - private Func<InvocationContext, HelpBuilder>? _helpBuilderFactory; private TryReplaceToken? _tokenReplacer; /// <summary> @@ -47,14 +46,12 @@ public class CommandLineConfiguration /// <param name="enablePosixBundling"><see langword="true"/> to enable POSIX bundling; otherwise, <see langword="false"/>.</param> /// <param name="enableTokenReplacement"><see langword="true"/> to enable token replacement; otherwise, <see langword="false"/>.</param> /// <param name="middlewarePipeline">Provide a custom middleware pipeline.</param> - /// <param name="helpBuilderFactory">Provide a custom help builder.</param> /// <param name="tokenReplacer">Replaces the specified token with any number of other tokens.</param> public CommandLineConfiguration( Command command, bool enablePosixBundling = true, bool enableTokenReplacement = true, IReadOnlyList<InvocationMiddleware>? middlewarePipeline = null, - Func<InvocationContext, HelpBuilder>? helpBuilderFactory = null, TryReplaceToken? tokenReplacer = null) : this( command, @@ -65,7 +62,6 @@ public CommandLineConfiguration( maxLevenshteinDistance: 0, processTerminationTimeout: null, middlewarePipeline: middlewarePipeline, - helpBuilderFactory: helpBuilderFactory, tokenReplacer: tokenReplacer, exceptionHandler: null) { @@ -80,7 +76,6 @@ internal CommandLineConfiguration( int maxLevenshteinDistance, TimeSpan? processTerminationTimeout, IReadOnlyList<InvocationMiddleware>? middlewarePipeline, - Func<InvocationContext, HelpBuilder>? helpBuilderFactory, TryReplaceToken? tokenReplacer, Func<Exception, InvocationContext, int>? exceptionHandler) { @@ -93,24 +88,12 @@ internal CommandLineConfiguration( ProcessTerminationTimeout = processTerminationTimeout; Middleware = middlewarePipeline ?? Array.Empty<InvocationMiddleware>(); - _helpBuilderFactory = helpBuilderFactory; _tokenReplacer = tokenReplacer; ExceptionHandler = exceptionHandler; } public static CommandLineBuilder CreateBuilder(Command rootCommand) => new CommandLineBuilder(rootCommand); - internal static HelpBuilder DefaultHelpBuilderFactory(InvocationContext context, int? requestedMaxWidth = null) - { - int maxWidth = requestedMaxWidth ?? int.MaxValue; - if (requestedMaxWidth is null && context.Console is SystemConsole systemConsole) - { - maxWidth = systemConsole.GetWindowWidth(); - } - - return new HelpBuilder(maxWidth); - } - /// <summary> /// Gets the enabled directives. /// </summary> @@ -132,8 +115,6 @@ internal static HelpBuilder DefaultHelpBuilderFactory(InvocationContext context, /// </remarks> public bool EnableTokenReplacement { get; } - internal Func<InvocationContext, HelpBuilder> HelpBuilderFactory => _helpBuilderFactory ??= context => DefaultHelpBuilderFactory(context); - internal TryReplaceToken? TokenReplacer => EnableTokenReplacement ? _tokenReplacer ??= DefaultTokenReplacer diff --git a/src/System.CommandLine/Help/HelpOption.cs b/src/System.CommandLine/Help/HelpOption.cs index bdf85feae1..1d4fccabad 100644 --- a/src/System.CommandLine/Help/HelpOption.cs +++ b/src/System.CommandLine/Help/HelpOption.cs @@ -1,24 +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.CommandLine.Invocation; -using System.CommandLine.IO; -using System.Threading; -using System.Threading.Tasks; - namespace System.CommandLine.Help { - internal class HelpOption : Option<bool> + public sealed class HelpOption : Option<bool> { - internal HelpOption(string name, string[] aliases) + public HelpOption(string name, string[] aliases) : base(name, aliases, new Argument<bool>(name) { Arity = ArgumentArity.Zero }) { AppliesToSelfAndChildren = true; Description = LocalizationResources.HelpOptionDescription(); - Action = new HelpOptionAction(); + Action = new HelpAction(); } - internal HelpOption() : this("--help", new[] { "-h", "/h", "-?", "/?" }) + public HelpOption() : this("--help", new[] { "-h", "/h", "-?", "/?" }) { } @@ -27,34 +22,5 @@ internal HelpOption() : this("--help", new[] { "-h", "/h", "-?", "/?" }) public override bool Equals(object? obj) => obj is HelpOption; public override int GetHashCode() => typeof(HelpOption).GetHashCode(); - - private sealed class HelpOptionAction : CliAction - { - private static void Display(InvocationContext context) - { - var output = context.Console.Out.CreateTextWriter(); - - var helpContext = new HelpContext(context.HelpBuilder, - context.ParseResult.CommandResult.Command, - output, - context.ParseResult); - - context.HelpBuilder.Write(helpContext); - } - - public override int Invoke(InvocationContext context) - { - Display(context); - - return 0; - } - - public override Task<int> InvokeAsync(InvocationContext context, CancellationToken cancellationToken = default) - => cancellationToken.IsCancellationRequested - ? Task.FromCanceled<int>(cancellationToken) - : Task.FromResult(Invoke(context)); - } - - } } \ No newline at end of file diff --git a/src/System.CommandLine/Help/HelpOptionAction.cs b/src/System.CommandLine/Help/HelpOptionAction.cs new file mode 100644 index 0000000000..8793513ad3 --- /dev/null +++ b/src/System.CommandLine/Help/HelpOptionAction.cs @@ -0,0 +1,40 @@ +using System.CommandLine.Invocation; +using System.CommandLine.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace System.CommandLine.Help +{ + public sealed class HelpAction : CliAction + { + private HelpBuilder? _builder; + + /// <summary> + /// Specifies an <see cref="Builder"/> to be used to format help output when help is requested. + /// </summary> + public HelpBuilder Builder + { + get => _builder ??= new HelpBuilder(Console.IsOutputRedirected ? int.MaxValue : Console.WindowWidth); + set => _builder = value ?? throw new ArgumentNullException(nameof(value)); + } + + public override int Invoke(InvocationContext context) + { + var output = context.Console.Out.CreateTextWriter(); + + var helpContext = new HelpContext(Builder, + context.ParseResult.CommandResult.Command, + output, + context.ParseResult); + + Builder.Write(helpContext); + + return 0; + } + + public override Task<int> InvokeAsync(InvocationContext context, CancellationToken cancellationToken = default) + => cancellationToken.IsCancellationRequested + ? Task.FromCanceled<int>(cancellationToken) + : Task.FromResult(Invoke(context)); + } +} diff --git a/src/System.CommandLine/IO/SystemConsole.cs b/src/System.CommandLine/IO/SystemConsole.cs index 15cb6e59c3..83f1699dd1 100644 --- a/src/System.CommandLine/IO/SystemConsole.cs +++ b/src/System.CommandLine/IO/SystemConsole.cs @@ -32,8 +32,6 @@ public SystemConsole() /// <inheritdoc /> public bool IsInputRedirected => Console.IsInputRedirected; - internal int GetWindowWidth() => IsOutputRedirected ? int.MaxValue : Console.WindowWidth; - private struct StandardErrorStreamWriter : IStandardStreamWriter { public static readonly StandardErrorStreamWriter Instance = new(); diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 861c568026..2a1a1f5112 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -1,7 +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.CommandLine.Help; using System.CommandLine.IO; namespace System.CommandLine.Invocation @@ -11,7 +10,6 @@ namespace System.CommandLine.Invocation /// </summary> public sealed class InvocationContext { - private HelpBuilder? _helpBuilder; private IConsole? _console; /// <param name="parseResult">The result of the current parse operation.</param> @@ -31,11 +29,6 @@ public IConsole Console set => _console = value; } - /// <summary> - /// Enables writing help output. - /// </summary> - public HelpBuilder HelpBuilder => _helpBuilder ??= ParseResult.Configuration.HelpBuilderFactory(this); - /// <summary> /// The parse result for the current invocation. /// </summary> From cacfbf721cf5f35206570f1041fe96a7f83c0cc0 Mon Sep 17 00:00:00 2001 From: Adam Sitnik <adam.sitnik@gmail.com> Date: Mon, 20 Mar 2023 18:02:25 +0100 Subject: [PATCH 2/4] address code review feedback: make Option.Action virtual and lazy --- ...ommandLine_api_is_not_changed.approved.txt | 4 ++++ src/System.CommandLine/Directive.cs | 2 +- .../EnvironmentVariablesDirective.cs | 12 ++++++++++- src/System.CommandLine/Help/HelpOption.cs | 10 +++++++++- src/System.CommandLine/Help/VersionOption.cs | 13 +++++++++--- src/System.CommandLine/Option.cs | 2 +- src/System.CommandLine/ParseDirective.cs | 20 +++++++++++++++++-- src/System.CommandLine/SuggestDirective.cs | 13 +++++++++++- 8 files changed, 66 insertions(+), 10 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 03240dfe54..14df623534 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 @@ -104,6 +104,7 @@ System.CommandLine public System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem> GetCompletions(System.CommandLine.Completions.CompletionContext context) public class EnvironmentVariablesDirective : Directive .ctor() + public CliAction Action { get; set; } public interface IConsole : System.CommandLine.IO.IStandardError, System.CommandLine.IO.IStandardIn, System.CommandLine.IO.IStandardOut public abstract class Option : Symbol, System.CommandLine.Binding.IValueDescriptor public CliAction Action { get; set; } @@ -131,6 +132,7 @@ System.CommandLine public static Option<T> AcceptExistingOnly<T>(this Option<T> option) public class ParseDirective : Directive .ctor(System.Int32 errorExitCode = 1) + public CliAction Action { get; set; } public class ParseResult public CliAction Action { get; } public System.CommandLine.Parsing.CommandResult CommandResult { get; } @@ -158,6 +160,7 @@ System.CommandLine .ctor(System.String description = ) public class SuggestDirective : Directive .ctor() + public CliAction Action { get; set; } public abstract class Symbol public System.String Description { get; set; } public System.Boolean IsHidden { get; set; } @@ -232,6 +235,7 @@ System.CommandLine.Help public class HelpOption : System.CommandLine.Option<System.Boolean>, System.CommandLine.Binding.IValueDescriptor .ctor(System.String name, System.String[] aliases) .ctor() + public System.CommandLine.CliAction Action { get; set; } public System.Boolean Equals(System.Object obj) public System.Int32 GetHashCode() public class TwoColumnHelpRow, System.IEquatable<TwoColumnHelpRow> diff --git a/src/System.CommandLine/Directive.cs b/src/System.CommandLine/Directive.cs index 42d90405f4..55e414febf 100644 --- a/src/System.CommandLine/Directive.cs +++ b/src/System.CommandLine/Directive.cs @@ -27,7 +27,7 @@ public Directive(string name) /// Gets or sets the <see cref="CliAction"/> for the Directive. The handler represents the action /// that will be performed when the Directive is invoked. /// </summary> - public CliAction? Action { get; set; } + public virtual CliAction? Action { get; set; } public override IEnumerable<CompletionItem> GetCompletions(CompletionContext context) => Array.Empty<CompletionItem>(); diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index fd4e59f03d..8714f90b59 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -10,8 +10,18 @@ namespace System.CommandLine /// </summary> public sealed class EnvironmentVariablesDirective : Directive { + private CliAction? _action; + public EnvironmentVariablesDirective() : base("env") - => Action = new EnvironmentVariablesDirectiveAction(this); + { + } + + /// <inheritdoc /> + public override CliAction? Action + { + get => _action ??= new EnvironmentVariablesDirectiveAction(this); + set => _action = value ?? throw new ArgumentNullException(nameof(value)); + } private sealed class EnvironmentVariablesDirectiveAction : CliAction { diff --git a/src/System.CommandLine/Help/HelpOption.cs b/src/System.CommandLine/Help/HelpOption.cs index 1d4fccabad..f9573f38f4 100644 --- a/src/System.CommandLine/Help/HelpOption.cs +++ b/src/System.CommandLine/Help/HelpOption.cs @@ -5,18 +5,26 @@ namespace System.CommandLine.Help { public sealed class HelpOption : Option<bool> { + private CliAction? _action; + public HelpOption(string name, string[] aliases) : base(name, aliases, new Argument<bool>(name) { Arity = ArgumentArity.Zero }) { AppliesToSelfAndChildren = true; Description = LocalizationResources.HelpOptionDescription(); - Action = new HelpAction(); } public HelpOption() : this("--help", new[] { "-h", "/h", "-?", "/?" }) { } + /// <inheritdoc /> + public override CliAction? Action + { + get => _action ??= new HelpAction(); + set => _action = value ?? throw new ArgumentNullException(nameof(value)); + } + internal override bool IsGreedy => false; public override bool Equals(object? obj) => obj is HelpOption; diff --git a/src/System.CommandLine/Help/VersionOption.cs b/src/System.CommandLine/Help/VersionOption.cs index 1c48691089..296500c649 100644 --- a/src/System.CommandLine/Help/VersionOption.cs +++ b/src/System.CommandLine/Help/VersionOption.cs @@ -10,13 +10,14 @@ namespace System.CommandLine.Help { - internal class VersionOption : Option<bool> + internal sealed class VersionOption : Option<bool> { + private CliAction? _action; + internal VersionOption() : base("--version", new Argument<bool>("--version") { Arity = ArgumentArity.Zero }) { Description = LocalizationResources.VersionOptionDescription(); - Action = new VersionOptionAction(); AddValidators(); } @@ -24,10 +25,16 @@ internal VersionOption(string name, string[] aliases) : base(name, aliases) { Description = LocalizationResources.VersionOptionDescription(); - Action = new VersionOptionAction(); AddValidators(); } + /// <inheritdoc /> + public override CliAction? Action + { + get => _action ??= new VersionOptionAction(); + set => _action = value ?? throw new ArgumentNullException(nameof(value)); + } + private void AddValidators() { Validators.Add(static result => diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs index 59c0f990d4..b40a9e533f 100644 --- a/src/System.CommandLine/Option.cs +++ b/src/System.CommandLine/Option.cs @@ -108,7 +108,7 @@ internal virtual bool IsGreedy /// Gets or sets the <see cref="CliAction"/> for the Option. The handler represents the action /// that will be performed when the Option is invoked. /// </summary> - public CliAction? Action { get; set; } + public virtual CliAction? Action { get; set; } string IValueDescriptor.ValueName => Name; diff --git a/src/System.CommandLine/ParseDirective.cs b/src/System.CommandLine/ParseDirective.cs index 3778e553cd..4e2126c0bd 100644 --- a/src/System.CommandLine/ParseDirective.cs +++ b/src/System.CommandLine/ParseDirective.cs @@ -3,6 +3,7 @@ using System.CommandLine.Parsing; using System.Threading; using System.Threading.Tasks; +using static System.Collections.Specialized.BitVector32; namespace System.CommandLine { @@ -11,9 +12,24 @@ namespace System.CommandLine /// </summary> public sealed class ParseDirective : Directive { + private const int DefaultErrorExitCode = 1; + private CliAction? _action; + /// <param name="errorExitCode">If the parse result contains errors, this exit code will be used when the process exits.</param> - public ParseDirective(int errorExitCode = 1) : base("parse") - => Action = new ParseDirectiveAction(errorExitCode); + public ParseDirective(int errorExitCode = DefaultErrorExitCode) : base("parse") + { + if (errorExitCode != DefaultErrorExitCode) + { + Action = new ParseDirectiveAction(errorExitCode); + } + } + + /// <inheritdoc /> + public override CliAction? Action + { + get => _action ??= new ParseDirectiveAction(DefaultErrorExitCode); + set => _action = value ?? throw new ArgumentNullException(nameof(value)); + } private sealed class ParseDirectiveAction : CliAction { diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index b8e68a07ee..5c580ff92d 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -4,6 +4,7 @@ using System.CommandLine.Parsing; using System.Threading.Tasks; using System.Threading; +using static System.Collections.Specialized.BitVector32; namespace System.CommandLine { @@ -13,8 +14,18 @@ namespace System.CommandLine /// <remarks>The <c>dotnet-suggest</c> tool requires the suggest directive to be enabled for an application to provide completions.</remarks> public sealed class SuggestDirective : Directive { + private CliAction? _action; + public SuggestDirective() : base("suggest") - => Action = new SuggestDirectiveAction(this); + { + } + + /// <inheritdoc /> + public override CliAction? Action + { + get => _action ??= new SuggestDirectiveAction(this); + set => _action = value ?? throw new ArgumentNullException(nameof(value)); + } private sealed class SuggestDirectiveAction : CliAction { From 219af6b037e79d888e847bd5d34ad2a8b61b5753 Mon Sep 17 00:00:00 2001 From: Adam Sitnik <adam.sitnik@gmail.com> Date: Tue, 21 Mar 2023 18:47:12 +0100 Subject: [PATCH 3/4] Apply suggestions from code review --- src/System.CommandLine/SuggestDirective.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine/SuggestDirective.cs b/src/System.CommandLine/SuggestDirective.cs index 5c580ff92d..9e808541a4 100644 --- a/src/System.CommandLine/SuggestDirective.cs +++ b/src/System.CommandLine/SuggestDirective.cs @@ -4,7 +4,6 @@ using System.CommandLine.Parsing; using System.Threading.Tasks; using System.Threading; -using static System.Collections.Specialized.BitVector32; namespace System.CommandLine { From bc2d75f78a7a505a1acbe7268e18c7db92c2a961 Mon Sep 17 00:00:00 2001 From: Adam Sitnik <adam.sitnik@gmail.com> Date: Tue, 21 Mar 2023 20:36:08 +0100 Subject: [PATCH 4/4] Apply suggestions from code review --- src/System.CommandLine/ParseDirective.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine/ParseDirective.cs b/src/System.CommandLine/ParseDirective.cs index 4e2126c0bd..e8c9bdaed7 100644 --- a/src/System.CommandLine/ParseDirective.cs +++ b/src/System.CommandLine/ParseDirective.cs @@ -3,7 +3,6 @@ using System.CommandLine.Parsing; using System.Threading; using System.Threading.Tasks; -using static System.Collections.Specialized.BitVector32; namespace System.CommandLine {