Powderhouse removed CliExit and fluent style for pipeline execution

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);
             }
         }
 

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);
         }
 

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}");
         }
 

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();
     }
 }

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();
     }
 
 

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)

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<string>? tokens, List<string>? errors) Replacer(string responseSourceName)
     {

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<ParseError> errors)

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)

src/System.CommandLine.Subsystems/Pipeline.cs

@@ -51,20 +51,21 @@ public ParseResult Parse(CliConfiguration configuration, IReadOnlyList<string> 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
     /// </summary>
     /// <param name="pipelineResult">The context of the current execution</param>
-    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);
             }
         }
     }

src/System.CommandLine.Subsystems/Subsystems/CliSubsystem.cs

@@ -66,9 +66,9 @@ protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId<
     /// Executes the behavior of the subsystem. For example, help would write information to the console.
     /// </summary>
     /// <param name="pipelineResult">The context contains data like the ParseResult, and allows setting of values like whether execution was handled and the CLI should terminate </param>
-    /// <returns>A CliExit object with information such as whether the CLI should terminate</returns>
-    protected internal virtual CliExit Execute(PipelineResult pipelineResult)
-        => CliExit.NotRun(pipelineResult.ParseResult);
+    /// <returns>A PipelineResult object with information such as whether the CLI should terminate</returns>
+    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
     /// <returns>True if parsing should continue</returns> // 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)
+    {}
 
 }

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;
+    }
 }