From 203c023f67b7dd6e249a06bdc238694fd2a7ce39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Thu, 13 Jun 2024 11:43:06 +0200 Subject: [PATCH 01/11] Apply help to all symbols in the CLI --- .../HelpSubsystem.cs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/System.CommandLine.Subsystems/HelpSubsystem.cs b/src/System.CommandLine.Subsystems/HelpSubsystem.cs index b4a922420..1f438e5a8 100644 --- a/src/System.CommandLine.Subsystems/HelpSubsystem.cs +++ b/src/System.CommandLine.Subsystems/HelpSubsystem.cs @@ -24,8 +24,23 @@ public class HelpSubsystem(IAnnotationProvider? annotationProvider = null) Arity = ArgumentArity.Zero }; - protected internal override void Initialize(InitializationContext context) - => context.Configuration.RootCommand.Add(HelpOption); + protected internal override void Initialize(InitializationContext context) + { + AddOptionRecursively(context.Configuration.RootCommand, HelpOption); + + // I imagine this method should belong to CliCommand + // or some extensions method, but I didn't want to change + // too much in this PR without consulting you first + static void AddOptionRecursively(CliCommand command, CliOption option) + { + command.Add(option); + + foreach (var subcommand in command.Subcommands) + { + AddOptionRecursively(subcommand, option); + } + } + } protected internal override bool GetIsActivated(ParseResult? parseResult) => parseResult is not null && parseResult.GetValue(HelpOption); From fb6952aadbf0c02f10509b0fb8e53fa769402e50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Thu, 13 Jun 2024 11:43:23 +0200 Subject: [PATCH 02/11] Add HelpSubsystem tests --- .../AlternateSubsystems.cs | 9 ++ .../HelpSubsystemTests.cs | 89 +++++++++++++++++++ ...System.CommandLine.Subsystems.Tests.csproj | 1 + .../TestData.cs | 19 ++++ 4 files changed, 118 insertions(+) create mode 100644 src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs diff --git a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs b/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs index abab37351..1869f763c 100644 --- a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs +++ b/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs @@ -17,6 +17,15 @@ protected override void Execute(PipelineResult pipelineResult) } } + internal class AlternateHelp : HelpSubsystem + { + protected override void Execute(PipelineResult pipelineResult) + { + pipelineResult.ConsoleHack.WriteLine($"***Help me!***"); + pipelineResult.SetSuccess(); + } + } + internal class VersionThatUsesHelpData : VersionSubsystem { // for testing, this class accepts a symbol and accesses its description diff --git a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs new file mode 100644 index 000000000..77c16c06f --- /dev/null +++ b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs @@ -0,0 +1,89 @@ +// 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 FluentAssertions; +using Xunit; +using System.CommandLine.Parsing; + +namespace System.CommandLine.Subsystems.Tests +{ + public class HelpSubsystemTests + { + [Fact] + public void When_help_subsystem_is_used_the_help_option_is_added_to_each_command_in_the_tree() + { + var rootCommand = new CliRootCommand + { + new CliOption("-x"), // add option that is expected for the test data used here + new CliCommand("a") + }; + var configuration = new CliConfiguration(rootCommand); + + var pipeline = Pipeline.CreateEmpty(); + pipeline.Help = new HelpSubsystem(); + + // Parse is used because directly calling Initialize would be unusual + var result = pipeline.Parse(configuration, ""); + + rootCommand.Options.Should().NotBeNull(); + rootCommand.Options + .Count(x => x.Name == "--help") + .Should() + .Be(1); + var subcommand = rootCommand.Subcommands.First(); + subcommand.Options.Should().NotBeNull(); + subcommand.Options + .Count(x => x.Name == "--help") + .Should() + .Be(1); + } + + [Theory] + [ClassData(typeof(TestData.Help))] + public void Help_is_activated_only_when_requested(string input, bool result) + { + CliRootCommand rootCommand = [new CliOption("-x")]; // add random option as empty CLIs are rare + var configuration = new CliConfiguration(rootCommand); + var helpSubsystem = new HelpSubsystem(); + var args = CliParser.SplitCommandLine(input).ToList().AsReadOnly(); + + Subsystem.Initialize(helpSubsystem, configuration, args); + + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var isActive = Subsystem.GetIsActivated(helpSubsystem, parseResult); + + isActive.Should().Be(result); + } + + [Fact] + public void Outputs_help_message() + { + var consoleHack = new ConsoleHack().RedirectToBuffer(true); + var helpSubsystem = new HelpSubsystem(); + Subsystem.Execute(helpSubsystem, new PipelineResult(null, "", null, consoleHack)); + consoleHack.GetBuffer().Trim().Should().Be("Help me!"); + } + + [Fact] + public void Custom_help_subsystem_can_be_used() + { + var consoleHack = new ConsoleHack().RedirectToBuffer(true); + var pipeline = Pipeline.CreateEmpty(); + pipeline.Help = new AlternateSubsystems.AlternateHelp(); + + pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); + consoleHack.GetBuffer().Trim().Should().Be($"***Help me!***"); + } + + [Fact] + public void Custom_help_subsystem_can_replace_standard() + { + var consoleHack = new ConsoleHack().RedirectToBuffer(true); + var pipeline = Pipeline.CreateEmpty(); + pipeline.Help = new AlternateSubsystems.AlternateHelp(); + + pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); + consoleHack.GetBuffer().Trim().Should().Be($"***Help me!***"); + } + } +} \ No newline at end of file diff --git a/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj b/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj index 33a7efd6e..c125df573 100644 --- a/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj +++ b/src/System.CommandLine.Subsystems.Tests/System.CommandLine.Subsystems.Tests.csproj @@ -32,6 +32,7 @@ --> + diff --git a/src/System.CommandLine.Subsystems.Tests/TestData.cs b/src/System.CommandLine.Subsystems.Tests/TestData.cs index 817d362d2..fddcc81cf 100644 --- a/src/System.CommandLine.Subsystems.Tests/TestData.cs +++ b/src/System.CommandLine.Subsystems.Tests/TestData.cs @@ -93,4 +93,23 @@ internal class Value : IEnumerable IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } + + internal class Help : IEnumerable + { + // This data only works if the CLI has a --version with a -v alias and also has a -x option + private readonly List _data = + [ + ["--help", true], + ["-h", true], + ["-hx", true], + ["-xh", true], + ["-x", false], + [null, false], + ["", false], + ]; + + public IEnumerator GetEnumerator() => _data.GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } } From 240817350b55a9734c0e3d46d1465777c03c8c5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Thu, 13 Jun 2024 12:05:33 +0200 Subject: [PATCH 03/11] Removed code comment --- src/System.CommandLine.Subsystems/HelpSubsystem.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/System.CommandLine.Subsystems/HelpSubsystem.cs b/src/System.CommandLine.Subsystems/HelpSubsystem.cs index 1f438e5a8..d935bdcf2 100644 --- a/src/System.CommandLine.Subsystems/HelpSubsystem.cs +++ b/src/System.CommandLine.Subsystems/HelpSubsystem.cs @@ -28,9 +28,6 @@ protected internal override void Initialize(InitializationContext context) { AddOptionRecursively(context.Configuration.RootCommand, HelpOption); - // I imagine this method should belong to CliCommand - // or some extensions method, but I didn't want to change - // too much in this PR without consulting you first static void AddOptionRecursively(CliCommand command, CliOption option) { command.Add(option); From 63ae37a657c839fd8086444e83d16c0a2403e3ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:09:32 +0200 Subject: [PATCH 04/11] Removed unnecessary not null check Value declared as non-null Co-authored-by: Kevin B --- src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs index 77c16c06f..550baf510 100644 --- a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs @@ -25,7 +25,6 @@ public void When_help_subsystem_is_used_the_help_option_is_added_to_each_command // Parse is used because directly calling Initialize would be unusual var result = pipeline.Parse(configuration, ""); - rootCommand.Options.Should().NotBeNull(); rootCommand.Options .Count(x => x.Name == "--help") .Should() From 577c60d64b7c9854fb54383a8b9e0289ea9e2854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:10:12 +0200 Subject: [PATCH 05/11] Removed unnecessary string interpolation on constant string value Co-authored-by: Kevin B --- src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs index 550baf510..e93908358 100644 --- a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs @@ -71,7 +71,7 @@ public void Custom_help_subsystem_can_be_used() pipeline.Help = new AlternateSubsystems.AlternateHelp(); pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); - consoleHack.GetBuffer().Trim().Should().Be($"***Help me!***"); + consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); } [Fact] From f4a627a9a818d55be39b024bbe0863422a51a14a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:10:40 +0200 Subject: [PATCH 06/11] Removed unnecessary not null check Value declared as non-null Co-authored-by: Kevin B --- src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs index e93908358..0a12b0b14 100644 --- a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs @@ -30,7 +30,6 @@ public void When_help_subsystem_is_used_the_help_option_is_added_to_each_command .Should() .Be(1); var subcommand = rootCommand.Subcommands.First(); - subcommand.Options.Should().NotBeNull(); subcommand.Options .Count(x => x.Name == "--help") .Should() From 141a74abbcf046374b1bf98599e559002ea99be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:11:00 +0200 Subject: [PATCH 07/11] Removed unnecessary string interpolation on constant string value Co-authored-by: Kevin B --- src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs index 0a12b0b14..b0d9a50e2 100644 --- a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs @@ -81,7 +81,7 @@ public void Custom_help_subsystem_can_replace_standard() pipeline.Help = new AlternateSubsystems.AlternateHelp(); pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); - consoleHack.GetBuffer().Trim().Should().Be($"***Help me!***"); + consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); } } } \ No newline at end of file From 93fb212d100f07dafa5c9b9bf654d55349a1c67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:11:24 +0200 Subject: [PATCH 08/11] Removed unnecessary string interpolation on constant string value Co-authored-by: Kevin B --- src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs b/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs index 1869f763c..9a95aecb2 100644 --- a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs +++ b/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs @@ -21,7 +21,7 @@ internal class AlternateHelp : HelpSubsystem { protected override void Execute(PipelineResult pipelineResult) { - pipelineResult.ConsoleHack.WriteLine($"***Help me!***"); + pipelineResult.ConsoleHack.WriteLine("***Help me!***"); pipelineResult.SetSuccess(); } } From 910aa7e12da21c742d7225e6b5a9fcc7371e64b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:14:27 +0200 Subject: [PATCH 09/11] Changed namespace declaration to file scoped --- .../HelpSubsystemTests.cs | 127 +++++++++--------- 1 file changed, 63 insertions(+), 64 deletions(-) diff --git a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs index b0d9a50e2..0c5a65f6d 100644 --- a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs @@ -5,83 +5,82 @@ using Xunit; using System.CommandLine.Parsing; -namespace System.CommandLine.Subsystems.Tests +namespace System.CommandLine.Subsystems.Tests; + +public class HelpSubsystemTests { - public class HelpSubsystemTests + [Fact] + public void When_help_subsystem_is_used_the_help_option_is_added_to_each_command_in_the_tree() { - [Fact] - public void When_help_subsystem_is_used_the_help_option_is_added_to_each_command_in_the_tree() - { - var rootCommand = new CliRootCommand - { - new CliOption("-x"), // add option that is expected for the test data used here - new CliCommand("a") - }; - var configuration = new CliConfiguration(rootCommand); + var rootCommand = new CliRootCommand + { + new CliOption("-x"), // add option that is expected for the test data used here + new CliCommand("a") + }; + var configuration = new CliConfiguration(rootCommand); - var pipeline = Pipeline.CreateEmpty(); - pipeline.Help = new HelpSubsystem(); + var pipeline = Pipeline.CreateEmpty(); + pipeline.Help = new HelpSubsystem(); - // Parse is used because directly calling Initialize would be unusual - var result = pipeline.Parse(configuration, ""); + // Parse is used because directly calling Initialize would be unusual + var result = pipeline.Parse(configuration, ""); - rootCommand.Options - .Count(x => x.Name == "--help") - .Should() - .Be(1); - var subcommand = rootCommand.Subcommands.First(); - subcommand.Options - .Count(x => x.Name == "--help") - .Should() - .Be(1); - } + rootCommand.Options + .Count(x => x.Name == "--help") + .Should() + .Be(1); + var subcommand = rootCommand.Subcommands.First(); + subcommand.Options + .Count(x => x.Name == "--help") + .Should() + .Be(1); + } - [Theory] - [ClassData(typeof(TestData.Help))] - public void Help_is_activated_only_when_requested(string input, bool result) - { - CliRootCommand rootCommand = [new CliOption("-x")]; // add random option as empty CLIs are rare - var configuration = new CliConfiguration(rootCommand); - var helpSubsystem = new HelpSubsystem(); - var args = CliParser.SplitCommandLine(input).ToList().AsReadOnly(); + [Theory] + [ClassData(typeof(TestData.Help))] + public void Help_is_activated_only_when_requested(string input, bool result) + { + CliRootCommand rootCommand = [new CliOption("-x")]; // add random option as empty CLIs are rare + var configuration = new CliConfiguration(rootCommand); + var helpSubsystem = new HelpSubsystem(); + var args = CliParser.SplitCommandLine(input).ToList().AsReadOnly(); - Subsystem.Initialize(helpSubsystem, configuration, args); + Subsystem.Initialize(helpSubsystem, configuration, args); - var parseResult = CliParser.Parse(rootCommand, input, configuration); - var isActive = Subsystem.GetIsActivated(helpSubsystem, parseResult); + var parseResult = CliParser.Parse(rootCommand, input, configuration); + var isActive = Subsystem.GetIsActivated(helpSubsystem, parseResult); - isActive.Should().Be(result); - } + isActive.Should().Be(result); + } - [Fact] - public void Outputs_help_message() - { - var consoleHack = new ConsoleHack().RedirectToBuffer(true); - var helpSubsystem = new HelpSubsystem(); - Subsystem.Execute(helpSubsystem, new PipelineResult(null, "", null, consoleHack)); - consoleHack.GetBuffer().Trim().Should().Be("Help me!"); - } + [Fact] + public void Outputs_help_message() + { + var consoleHack = new ConsoleHack().RedirectToBuffer(true); + var helpSubsystem = new HelpSubsystem(); + Subsystem.Execute(helpSubsystem, new PipelineResult(null, "", null, consoleHack)); + consoleHack.GetBuffer().Trim().Should().Be("Help me!"); + } - [Fact] - public void Custom_help_subsystem_can_be_used() - { - var consoleHack = new ConsoleHack().RedirectToBuffer(true); - var pipeline = Pipeline.CreateEmpty(); - pipeline.Help = new AlternateSubsystems.AlternateHelp(); + [Fact] + public void Custom_help_subsystem_can_be_used() + { + var consoleHack = new ConsoleHack().RedirectToBuffer(true); + var pipeline = Pipeline.CreateEmpty(); + pipeline.Help = new AlternateSubsystems.AlternateHelp(); - pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); - consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); - } + pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); + consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); + } - [Fact] - public void Custom_help_subsystem_can_replace_standard() - { - var consoleHack = new ConsoleHack().RedirectToBuffer(true); - var pipeline = Pipeline.CreateEmpty(); - pipeline.Help = new AlternateSubsystems.AlternateHelp(); + [Fact] + public void Custom_help_subsystem_can_replace_standard() + { + var consoleHack = new ConsoleHack().RedirectToBuffer(true); + var pipeline = Pipeline.CreateEmpty(); + pipeline.Help = new AlternateSubsystems.AlternateHelp(); - pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); - consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); - } + pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); + consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); } } \ No newline at end of file From e028d1afd0aac330fa84b9a0ef6bd31da47f1df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:15:05 +0200 Subject: [PATCH 10/11] Consolided Help subsystem tests with same body --- .../HelpSubsystemTests.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs index 0c5a65f6d..c7d189066 100644 --- a/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/HelpSubsystemTests.cs @@ -72,15 +72,4 @@ public void Custom_help_subsystem_can_be_used() pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); } - - [Fact] - public void Custom_help_subsystem_can_replace_standard() - { - var consoleHack = new ConsoleHack().RedirectToBuffer(true); - var pipeline = Pipeline.CreateEmpty(); - pipeline.Help = new AlternateSubsystems.AlternateHelp(); - - pipeline.Execute(new CliConfiguration(new CliRootCommand()), "-h", consoleHack); - consoleHack.GetBuffer().Trim().Should().Be("***Help me!***"); - } } \ No newline at end of file From 9a48fef3e284376f533c451cfeb6c57b1e696450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ha=C5=9Bnik?= Date: Tue, 18 Jun 2024 19:15:26 +0200 Subject: [PATCH 11/11] Removed mistakenly copied comment --- src/System.CommandLine.Subsystems.Tests/TestData.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.CommandLine.Subsystems.Tests/TestData.cs b/src/System.CommandLine.Subsystems.Tests/TestData.cs index fddcc81cf..9093065b1 100644 --- a/src/System.CommandLine.Subsystems.Tests/TestData.cs +++ b/src/System.CommandLine.Subsystems.Tests/TestData.cs @@ -96,7 +96,6 @@ internal class Value : IEnumerable internal class Help : IEnumerable { - // This data only works if the CLI has a --version with a -v alias and also has a -x option private readonly List _data = [ ["--help", true],