Merge pull request #785 from jonsequitur/make-UseHelp-idempotent

allow UseHelp to be called more than once on the same command

Author: jonsequitur

src/System.CommandLine.Tests/UseHelpTests.cs

@@ -34,7 +34,7 @@ public async Task UseHelp_writes_help_for_the_specified_command()
 
             _console.Out.ToString().Should().Contain($"{RootCommand.ExecutableName} command subcommand");
         }
-
+         
         [Fact]
         public async Task UseHelp_interrupts_execution_of_the_specified_command()
         {
@@ -73,20 +73,6 @@ public async Task UseHelp_accepts_default_values(string value)
             _console.Out.ToString().Should().StartWith("Usage:");
         }
 
-        [Fact]
-        public async Task UseHelp_accepts_custom_aliases()
-        {
-            var parser =
-                new CommandLineBuilder()
-                    .AddCommand(new Command("command"))
-                    .UseHelp(new Option(new[] { "~cthulhu" }))
-                    .Build();
-
-            await parser.InvokeAsync("command ~cthulhu", _console);
-
-            _console.Out.ToString().Should().StartWith("Usage:");
-        }
-
         [Fact]
         public async Task UseHelp_does_not_display_when_option_defined_with_same_alias()
         {
@@ -138,5 +124,60 @@ public void There_are_no_parse_errors_when_help_is_invoked_on_subcommand()
                   .Should()
                   .BeEmpty();
         }
+
+        [Theory]
+        [InlineData("-h")]
+        [InlineData("inner -h")]
+        public void UseHelp_can_be_called_more_than_once_on_the_same_CommandLineBuilder(string commandline)
+        {
+            var root = new RootCommand
+            {
+                new Command("inner")
+            };
+
+            var parser = new CommandLineBuilder(root)
+                         .UseHelp()
+                         .UseHelp()
+                         .Build();
+
+            var console1 = new TestConsole();
+
+            parser.Invoke(commandline, console1);
+
+            console1.Out.ToString().Should().Contain("Usage:");
+            console1.Error.ToString().Should().BeEmpty();
+        }
+
+        [Theory]
+        [InlineData("-h")]
+        [InlineData("inner -h")]
+        public void UseHelp_can_be_called_more_than_once_on_the_same_command_with_different_CommandLineBuilders(string commandline)
+        {
+            var root = new RootCommand
+            {
+                new Command("inner")
+            };
+
+            var parser1 = new CommandLineBuilder(root)
+                          .UseHelp()
+                          .Build();
+
+            var console1 = new TestConsole();
+
+            parser1.Invoke(commandline, console1);
+
+            console1.Out.ToString().Should().Contain("Usage:");
+            console1.Error.ToString().Should().BeEmpty();
+
+            var parser2 = new CommandLineBuilder(root)
+                          .UseHelp()
+                          .Build();
+            var console2 = new TestConsole();
+
+            parser2.Invoke(commandline, console2);
+
+            console2.Out.ToString().Should().Contain("Usage:");
+            console2.Error.ToString().Should().BeEmpty();
+        }
     }
 }

src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs

@@ -268,38 +268,69 @@ void Default(Exception exception, InvocationContext context)
 
         public static CommandLineBuilder UseHelp(this CommandLineBuilder builder)
         {
-            var helpOption = new Option(new []
-            {
-                "-h",
-                "/h",
-                "--help",
-                "-?",
-                "/?"
-            }, "Show help and usage information");
+            var helpOption = new HelpOption();
 
             return builder.UseHelp(helpOption);
         }
 
-        public static CommandLineBuilder UseHelp(
+        internal static CommandLineBuilder UseHelp(
             this CommandLineBuilder builder,
             Option helpOption)
         {
-            builder.HelpOption = helpOption ?? throw new ArgumentNullException(nameof(helpOption));
-            builder.HelpOption.Argument.Arity = ArgumentArity.Zero;
-            
-            builder.Command.AddGlobalOption(builder.HelpOption);
-
-            builder.AddMiddleware(async (context, next) =>
+            if (builder.HelpOption == null)
             {
-                if (!ShowHelp(context, builder.HelpOption))
+                builder.HelpOption = helpOption; 
+                builder.Command.TryAddGlobalOption(helpOption);
+
+                builder.AddMiddleware(async (context, next) =>
                 {
-                    await next(context);
-                }
-            }, MiddlewareOrderInternal.HelpOption);
+                    if (!ShowHelp(context, builder.HelpOption))
+                    {
+                        await next(context);
+                    }
+                }, MiddlewareOrderInternal.HelpOption);
+
+                return builder;
+            }
 
             return builder;
         }
 
+        private class HelpOption : Option
+        {
+            public HelpOption() : base(new[]
+            {
+                "-h",
+                "/h",
+                "--help",
+                "-?",
+                "/?"
+            }, "Show help and usage information")
+            {
+            }
+
+            public override Argument Argument
+            {
+                get => Argument.None;
+                set => throw new NotSupportedException();
+            }
+
+            protected bool Equals(HelpOption other)
+            {
+                return other != null;
+            }
+
+            public override bool Equals(object obj)
+            {
+                return obj is HelpOption;
+            }
+
+            public override int GetHashCode()
+            {
+                return typeof(HelpOption).GetHashCode();
+            }
+        }
+
         public static TBuilder UseHelpBuilder<TBuilder>(this TBuilder builder, Func<BindingContext, IHelpBuilder> getHelpBuilder)
             where TBuilder : CommandLineBuilder
         {
@@ -381,7 +412,7 @@ public static CommandLineBuilder UseSuggestDirective(
                 {
                     int position;
 
-                    if (values.FirstOrDefault() is string positionString)
+                    if (values.FirstOrDefault() is { } positionString)
                     {
                         position = int.Parse(positionString);
                     }

src/System.CommandLine/Command.cs

@@ -36,6 +36,20 @@ public void AddGlobalOption(Option option)
             Children.AddWithoutAliasCollisionCheck(option);
         }
 
+        public bool TryAddGlobalOption(Option option)
+        {
+            if (!_globalOptions.IsAnyAliasInUse(option, out _))
+            {
+                _globalOptions.Add(option);
+                Children.AddWithoutAliasCollisionCheck(option);
+                return true;
+            }
+            else
+            {
+                return false;
+            }
+        }
+
         public void Add(Symbol symbol) => AddSymbol(symbol);
 
         public void Add(Argument argument) => AddArgument(argument);