diff --git a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs b/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs index d4db61f88..abab37351 100644 --- a/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs +++ b/src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs @@ -10,11 +10,10 @@ internal class AlternateSubsystems { internal class AlternateVersion : VersionSubsystem { - protected override CliExit Execute(PipelineResult pipelineResult) + protected override void Execute(PipelineResult pipelineResult) { pipelineResult.ConsoleHack.WriteLine($"***{CliExecutable.ExecutableVersion}***"); - pipelineResult.AlreadyHandled = true; - return CliExit.SuccessfullyHandled(pipelineResult.ParseResult); + pipelineResult.SetSuccess(); } } @@ -29,12 +28,12 @@ public VersionThatUsesHelpData(CliSymbol symbol) private CliSymbol Symbol { get; } - protected override CliExit Execute(PipelineResult pipelineResult) + protected override void Execute(PipelineResult pipelineResult) { TryGetAnnotation(Symbol, HelpAnnotations.Description, out string? description); pipelineResult.ConsoleHack.WriteLine(description); pipelineResult.AlreadyHandled = true; - return CliExit.SuccessfullyHandled(pipelineResult.ParseResult); + pipelineResult.SetSuccess(); } } @@ -44,23 +43,23 @@ internal class VersionWithInitializeAndTeardown : VersionSubsystem internal bool ExecutionWasRun; internal bool TeardownWasRun; - protected override CliConfiguration Initialize(InitializationContext context) + protected override void Initialize(InitializationContext context) { + base.Initialize(context); // marker hack needed because ConsoleHack not available in initialization InitializationWasRun = true; - return base.Initialize(context); } - protected override CliExit Execute(PipelineResult pipelineResult) + protected override void Execute(PipelineResult pipelineResult) { ExecutionWasRun = true; - return base.Execute(pipelineResult); + base.Execute(pipelineResult); } - protected override CliExit TearDown(CliExit cliExit) + protected override void TearDown(PipelineResult pipelineResult) { TeardownWasRun = true; - return base.TearDown(cliExit); + base.TearDown(pipelineResult); } } diff --git a/src/System.CommandLine.Subsystems.Tests/PipelineTests.cs b/src/System.CommandLine.Subsystems.Tests/PipelineTests.cs index 27308a20e..62f1ac834 100644 --- a/src/System.CommandLine.Subsystems.Tests/PipelineTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/PipelineTests.cs @@ -43,7 +43,7 @@ public void Subsystem_runs_in_pipeline_only_when_requested(string input, bool sh var exit = pipeline.Execute(GetNewTestConfiguration(), input, console); exit.ExitCode.Should().Be(0); - exit.Handled.Should().Be(shouldRun); + exit.AlreadyHandled.Should().Be(shouldRun); if (shouldRun) { console.GetBuffer().Trim().Should().Be(TestData.AssemblyVersionString); @@ -61,7 +61,7 @@ public void Subsystem_runs_with_explicit_parse_only_when_requested(string input, var exit = pipeline.Execute(result, input, console); exit.ExitCode.Should().Be(0); - exit.Handled.Should().Be(shouldRun); + exit.AlreadyHandled.Should().Be(shouldRun); if (shouldRun) { console.GetBuffer().Trim().Should().Be(TestData.AssemblyVersionString); @@ -79,7 +79,7 @@ public void Subsystem_runs_initialize_and_teardown_when_requested(string input, var exit = pipeline.Execute(GetNewTestConfiguration(), input, console); exit.ExitCode.Should().Be(0); - exit.Handled.Should().Be(shouldRun); + exit.AlreadyHandled.Should().Be(shouldRun); versionSubsystem.InitializationWasRun.Should().BeTrue(); versionSubsystem.ExecutionWasRun.Should().Be(shouldRun); versionSubsystem.TeardownWasRun.Should().BeTrue(); @@ -109,7 +109,7 @@ public void Subsystem_works_without_pipeline(string input, bool shouldRun) var exit = Subsystem.Execute(versionSubsystem, parseResult, input, console); exit.Should().NotBeNull(); exit.ExitCode.Should().Be(0); - exit.Handled.Should().BeTrue(); + exit.AlreadyHandled.Should().BeTrue(); console.GetBuffer().Trim().Should().Be(TestData.AssemblyVersionString); } } @@ -132,7 +132,7 @@ public void Subsystem_works_without_pipeline_style2(string input, bool shouldRun var exit = Subsystem.ExecuteIfNeeded(versionSubsystem, parseResult, input, console); exit.ExitCode.Should().Be(0); - exit.Handled.Should().Be(shouldRun); + exit.AlreadyHandled.Should().Be(shouldRun); console.GetBuffer().Trim().Should().Be(expectedVersion); } diff --git a/src/System.CommandLine.Subsystems.Tests/VersionFunctionalTests.cs b/src/System.CommandLine.Subsystems.Tests/VersionFunctionalTests.cs index dfb5681a5..8804cca62 100644 --- a/src/System.CommandLine.Subsystems.Tests/VersionFunctionalTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/VersionFunctionalTests.cs @@ -26,7 +26,7 @@ public void When_the_version_option_is_specified_then_the_version_is_written_to_ var exit = pipeline.Execute(configuration, "-v", consoleHack); exit.ExitCode.Should().Be(0); - exit.Handled.Should().BeTrue(); + exit.AlreadyHandled.Should().BeTrue(); consoleHack.GetBuffer().Should().Be($"{version}{newLine}"); } diff --git a/src/System.CommandLine.Subsystems/CliExit.cs b/src/System.CommandLine.Subsystems/CliExit.cs deleted file mode 100644 index be1190675..000000000 --- a/src/System.CommandLine.Subsystems/CliExit.cs +++ /dev/null @@ -1,35 +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.CommandLine.Subsystems; - -namespace System.CommandLine; - - // TODO: Consider what info is needed after invocation. If it's the whole pipeline context, consider collapsing this with that class. -public class CliExit -{ - internal CliExit(PipelineResult pipelineResult) - : this(pipelineResult.ParseResult, pipelineResult.AlreadyHandled, pipelineResult.ExitCode) - { } - - private CliExit(ParseResult? parseResult, bool handled, int exitCode) - { - ExitCode = exitCode; - Handled = handled; - ParseResult = parseResult; - } - public ParseResult? ParseResult { get; set; } - - public int ExitCode { get; } - - public static implicit operator int(CliExit cliExit) => cliExit.ExitCode; - - public static implicit operator bool(CliExit cliExit) => !cliExit.Handled; - - - public bool Handled { get; } - - public static CliExit NotRun(ParseResult? parseResult) => new(parseResult, false, 0); - - public static CliExit SuccessfullyHandled(ParseResult? parseResult) => new(parseResult, true, 0); -} diff --git a/src/System.CommandLine.Subsystems/CompletionSubsystem.cs b/src/System.CommandLine.Subsystems/CompletionSubsystem.cs index 3ec5b166d..434d4e193 100644 --- a/src/System.CommandLine.Subsystems/CompletionSubsystem.cs +++ b/src/System.CommandLine.Subsystems/CompletionSubsystem.cs @@ -18,9 +18,9 @@ protected internal override bool GetIsActivated(ParseResult? parseResult) ? false : false; - protected internal override CliExit Execute(PipelineResult pipelineResult) + protected internal override void Execute(PipelineResult pipelineResult) { pipelineResult.ConsoleHack.WriteLine("Not yet implemented"); - return CliExit.SuccessfullyHandled(pipelineResult.ParseResult); + pipelineResult.SetSuccess(); } } diff --git a/src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs b/src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs index 3c0bdd1d4..c31ea6e2a 100644 --- a/src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs +++ b/src/System.CommandLine.Subsystems/Directives/DiagramSubsystem.cs @@ -13,14 +13,14 @@ public class DiagramSubsystem( IAnnotationProvider? annotationProvider = null) //protected internal override bool GetIsActivated(ParseResult? parseResult) // => parseResult is not null && option is not null && parseResult.GetValue(option); - protected internal override CliExit Execute(PipelineResult pipelineResult) + protected internal override void Execute(PipelineResult pipelineResult) { // Gather locations //var locations = pipelineResult.ParseResult.LocationMap // .Concat(Map(pipelineResult.ParseResult.Configuration.PreProcessedLocations)); pipelineResult.ConsoleHack.WriteLine("Output diagram"); - return CliExit.SuccessfullyHandled(pipelineResult.ParseResult); + pipelineResult.SetSuccess(); } diff --git a/src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs b/src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs index 6b4b7d967..efb51bcee 100644 --- a/src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs +++ b/src/System.CommandLine.Subsystems/Directives/DirectiveSubsystem.cs @@ -19,7 +19,7 @@ public DirectiveSubsystem(string name, SubsystemKind kind, IAnnotationProvider? Id = id ?? name; } - protected internal override CliConfiguration Initialize(InitializationContext context) + protected internal override void Initialize(InitializationContext context) { for (int i = 0; i < context.Args.Count; i++) { @@ -50,8 +50,6 @@ protected internal override CliConfiguration Initialize(InitializationContext co break; } } - - return context.Configuration; } protected internal override bool GetIsActivated(ParseResult? parseResult) diff --git a/src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs b/src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs index ed43c8d62..66a1fd6b7 100644 --- a/src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs +++ b/src/System.CommandLine.Subsystems/Directives/ResponseSubsystem.cs @@ -9,11 +9,8 @@ namespace System.CommandLine.Directives; public class ResponseSubsystem() : CliSubsystem("Response", SubsystemKind.Response, null) { - protected internal override CliConfiguration Initialize(InitializationContext context) - { - context.Configuration.ResponseFileTokenReplacer = Replacer; - return context.Configuration; - } + protected internal override void Initialize(InitializationContext context) + => context.Configuration.ResponseFileTokenReplacer = Replacer; public static (List? tokens, List? errors) Replacer(string responseSourceName) { diff --git a/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs b/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs index 0948ac017..9effb620f 100644 --- a/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs +++ b/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs @@ -23,14 +23,14 @@ protected internal override bool GetIsActivated(ParseResult? parseResult) => parseResult is not null && parseResult.Errors.Any(); // TODO: properly test execute directly when parse result is usable in tests - protected internal override CliExit Execute(PipelineResult pipelineResult) + protected internal override void Execute(PipelineResult pipelineResult) { var _ = pipelineResult.ParseResult ?? throw new ArgumentException("The parse result has not been set", nameof(pipelineResult)); Report(pipelineResult.ConsoleHack, pipelineResult.ParseResult.Errors); - return CliExit.SuccessfullyHandled(pipelineResult.ParseResult); + pipelineResult.SetSuccess(); } public void Report(ConsoleHack consoleHack, IReadOnlyList errors) diff --git a/src/System.CommandLine.Subsystems/HelpSubsystem.cs b/src/System.CommandLine.Subsystems/HelpSubsystem.cs index 3be155a18..b4a922420 100644 --- a/src/System.CommandLine.Subsystems/HelpSubsystem.cs +++ b/src/System.CommandLine.Subsystems/HelpSubsystem.cs @@ -24,21 +24,17 @@ public class HelpSubsystem(IAnnotationProvider? annotationProvider = null) Arity = ArgumentArity.Zero }; - protected internal override CliConfiguration Initialize(InitializationContext context) - { - context.Configuration.RootCommand.Add(HelpOption); - - return context.Configuration; - } + protected internal override void Initialize(InitializationContext context) + => context.Configuration.RootCommand.Add(HelpOption); protected internal override bool GetIsActivated(ParseResult? parseResult) => parseResult is not null && parseResult.GetValue(HelpOption); - protected internal override CliExit Execute(PipelineResult pipelineResult) + protected internal override void Execute(PipelineResult pipelineResult) { // TODO: Match testable output pattern pipelineResult.ConsoleHack.WriteLine("Help me!"); - return CliExit.SuccessfullyHandled(pipelineResult.ParseResult); + pipelineResult.SetSuccess(); } public bool TryGetDescription (CliSymbol symbol, out string? description) diff --git a/src/System.CommandLine.Subsystems/Pipeline.cs b/src/System.CommandLine.Subsystems/Pipeline.cs index 18298cea7..4bceb1db1 100644 --- a/src/System.CommandLine.Subsystems/Pipeline.cs +++ b/src/System.CommandLine.Subsystems/Pipeline.cs @@ -51,20 +51,21 @@ public ParseResult Parse(CliConfiguration configuration, IReadOnlyList a return parseResult; } - public CliExit Execute(CliConfiguration configuration, string rawInput, ConsoleHack? consoleHack = null) + public PipelineResult Execute(CliConfiguration configuration, string rawInput, ConsoleHack? consoleHack = null) => Execute(configuration, CliParser.SplitCommandLine(rawInput).ToArray(), rawInput, consoleHack); - public CliExit Execute(CliConfiguration configuration, string[] args, string rawInput, ConsoleHack? consoleHack = null) + public PipelineResult Execute(CliConfiguration configuration, string[] args, string rawInput, ConsoleHack? consoleHack = null) { - var cliExit = Execute(Parse(configuration, args), rawInput, consoleHack); - return TearDownSubsystems(cliExit); + var pipelineResult = Execute(Parse(configuration, args), rawInput, consoleHack); + TearDownSubsystems(pipelineResult); + return pipelineResult; } - public CliExit Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null) + public PipelineResult Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null) { var pipelineResult = new PipelineResult(parseResult, rawInput, this, consoleHack ?? new ConsoleHack()); ExecuteSubsystems(pipelineResult); - return new CliExit(pipelineResult); + return pipelineResult; } // TODO: Consider whether this should be public. It would simplify testing, but would it do anything else @@ -95,18 +96,17 @@ protected virtual void InitializeSubsystems(InitializationContext context) /// Perform any cleanup operations /// /// The context of the current execution - protected virtual CliExit TearDownSubsystems(CliExit cliExit) + protected virtual void TearDownSubsystems(PipelineResult pipelineResult) { - // TODO: Work on this design as the last cliExit wins and they may not all be well behaved + // TODO: Work on this design as the last pipelineResult wins and they may not all be well behaved var subsystems = Subsystems.Reverse(); foreach (var subsystem in subsystems) { if (subsystem is not null) { - cliExit = subsystem.TearDown(cliExit); + subsystem.TearDown(pipelineResult); } } - return cliExit; } protected virtual void ExecuteSubsystems(PipelineResult pipelineResult) @@ -117,7 +117,7 @@ protected virtual void ExecuteSubsystems(PipelineResult pipelineResult) { if (subsystem is not null) { - pipelineResult = subsystem.ExecuteIfNeeded(pipelineResult); + subsystem.ExecuteIfNeeded(pipelineResult); } } } diff --git a/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs b/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs index fdc22d5aa..157a45f44 100644 --- a/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs +++ b/src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs @@ -66,9 +66,9 @@ protected internal bool TryGetAnnotation(CliSymbol symbol, AnnotationId< /// Executes the behavior of the subsystem. For example, help would write information to the console. /// /// The context contains data like the ParseResult, and allows setting of values like whether execution was handled and the CLI should terminate - /// A CliExit object with information such as whether the CLI should terminate - protected internal virtual CliExit Execute(PipelineResult pipelineResult) - => CliExit.NotRun(pipelineResult.ParseResult); + /// A PipelineResult object with information such as whether the CLI should terminate + protected internal virtual void Execute(PipelineResult pipelineResult) + => pipelineResult.NotRun(pipelineResult.ParseResult); internal PipelineResult ExecuteIfNeeded(PipelineResult pipelineResult) => ExecuteIfNeeded(pipelineResult.ParseResult, pipelineResult); @@ -107,11 +107,11 @@ internal PipelineResult ExecuteIfNeeded(ParseResult? parseResult, PipelineResult /// True if parsing should continue // there might be a better design that supports a message // TODO: Because of this and similar usage, consider combining CLI declaration and config. ArgParse calls this the parser, which I like // TODO: Why does Intitialize return a configuration? - protected internal virtual CliConfiguration Initialize(InitializationContext context) - => context.Configuration; + protected internal virtual void Initialize(InitializationContext context) + {} // TODO: Determine if this is needed. - protected internal virtual CliExit TearDown(CliExit cliExit) - => cliExit; + protected internal virtual void TearDown(PipelineResult pipelineResult) + {} } diff --git a/src/System.CommandLine.Subsystems/Subsystems/PipelineResult.cs b/src/System.CommandLine.Subsystems/Subsystems/PipelineResult.cs index 00c02bc9a..2c9e56d73 100644 --- a/src/System.CommandLine.Subsystems/Subsystems/PipelineResult.cs +++ b/src/System.CommandLine.Subsystems/Subsystems/PipelineResult.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. + + namespace System.CommandLine.Subsystems; public class PipelineResult(ParseResult? parseResult, string rawInput, Pipeline? pipeline, ConsoleHack? consoleHack = null) @@ -13,4 +15,14 @@ public class PipelineResult(ParseResult? parseResult, string rawInput, Pipeline? public bool AlreadyHandled { get; set; } public int ExitCode { get; set; } + public void NotRun(ParseResult? parseResult) + { + // no op because defaults are false and 0 + } + + public void SetSuccess() + { + AlreadyHandled = true; + ExitCode = 0; + } } diff --git a/src/System.CommandLine.Subsystems/Subsystems/Subsystem.cs b/src/System.CommandLine.Subsystems/Subsystems/Subsystem.cs index 440c313ef..e55d28a7d 100644 --- a/src/System.CommandLine.Subsystems/Subsystems/Subsystem.cs +++ b/src/System.CommandLine.Subsystems/Subsystems/Subsystem.cs @@ -8,22 +8,29 @@ public class Subsystem public static void Initialize(CliSubsystem subsystem, CliConfiguration configuration, IReadOnlyList args) => subsystem.Initialize(new InitializationContext(configuration, args)); - public static CliExit Execute(CliSubsystem subsystem, PipelineResult pipelineResult) + public static void Execute(CliSubsystem subsystem, PipelineResult pipelineResult) => subsystem.Execute(pipelineResult); public static bool GetIsActivated(CliSubsystem subsystem, ParseResult parseResult) => subsystem.GetIsActivated(parseResult); - public static CliExit ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null) - => new(subsystem.ExecuteIfNeeded(new PipelineResult(parseResult, rawInput, null, consoleHack))); - - public static CliExit Execute(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null) - => subsystem.Execute(new PipelineResult(parseResult, rawInput, null, consoleHack)); + public static PipelineResult ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null) + => subsystem.ExecuteIfNeeded(new PipelineResult(parseResult, rawInput, null, consoleHack)); + public static PipelineResult Execute(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null) + { + var pipelineResult = new PipelineResult(parseResult, rawInput,null, consoleHack); + subsystem.Execute(pipelineResult); + return pipelineResult; + } internal static PipelineResult ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack, PipelineResult? pipelineResult = null) - => subsystem.ExecuteIfNeeded(pipelineResult ?? new PipelineResult(parseResult, rawInput, null, consoleHack)); + { + pipelineResult ??= new PipelineResult(parseResult, rawInput, null, consoleHack); + subsystem.ExecuteIfNeeded(pipelineResult ); + return pipelineResult; + } - internal static PipelineResult ExecuteIfNeeded(CliSubsystem subsystem, PipelineResult pipelineResult) + internal static void ExecuteIfNeeded(CliSubsystem subsystem, PipelineResult pipelineResult) => subsystem.ExecuteIfNeeded(pipelineResult); } diff --git a/src/System.CommandLine.Subsystems/ValueSubsystem.cs b/src/System.CommandLine.Subsystems/ValueSubsystem.cs index 6152f019a..2257743cc 100644 --- a/src/System.CommandLine.Subsystems/ValueSubsystem.cs +++ b/src/System.CommandLine.Subsystems/ValueSubsystem.cs @@ -29,10 +29,10 @@ protected internal override bool GetIsActivated(ParseResult? parseResult) /// /// Note to inheritors: Call base for all ValueSubsystem methods that you override to ensure correct behavior /// - protected internal override CliExit Execute(PipelineResult pipelineResult) + protected internal override void Execute(PipelineResult pipelineResult) { parseResult ??= pipelineResult.ParseResult; - return base.Execute(pipelineResult); + base.Execute(pipelineResult); } private void SetValue(CliSymbol symbol, object? value) diff --git a/src/System.CommandLine.Subsystems/VersionSubsystem.cs b/src/System.CommandLine.Subsystems/VersionSubsystem.cs index c538a896c..8cb07d116 100644 --- a/src/System.CommandLine.Subsystems/VersionSubsystem.cs +++ b/src/System.CommandLine.Subsystems/VersionSubsystem.cs @@ -34,30 +34,27 @@ public string? SpecificVersion ?.GetCustomAttribute() ?.InformationalVersion; - protected internal override CliConfiguration Initialize(InitializationContext context) + protected internal override void Initialize(InitializationContext context) { var option = new CliOption("--version", ["-v"]) { Arity = ArgumentArity.Zero }; context.Configuration.RootCommand.Add(option); - - return context.Configuration; } // TODO: Stash option rather than using string protected internal override bool GetIsActivated(ParseResult? parseResult) => parseResult is not null && parseResult.GetValue("--version"); - protected internal override CliExit Execute(PipelineResult pipelineResult) + protected internal override void Execute(PipelineResult pipelineResult) { var subsystemVersion = SpecificVersion; var version = subsystemVersion is null ? CliExecutable.ExecutableVersion : subsystemVersion; pipelineResult.ConsoleHack.WriteLine(version); - pipelineResult.AlreadyHandled = true; - return CliExit.SuccessfullyHandled(pipelineResult.ParseResult); + pipelineResult.SetSuccess(); } }