Reorg/rename in pipeline

Made Initialize/Teardown subystems protected from public

Author: KathleenDollard

src/System.CommandLine.Subsystems/Pipeline.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// 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.Parsing;
@@ -13,6 +13,32 @@ public class Pipeline
     public ErrorReportingSubsystem? ErrorReporting { get; set; }
     public CompletionSubsystem? Completion { get; set; }
 
+    public ParseResult Parse(CliConfiguration configuration, string rawInput)
+        => Parse(configuration, CliParser.SplitCommandLine(rawInput).ToArray());
+
+    public ParseResult Parse(CliConfiguration configuration, string[] args)
+    {
+        InitializeSubsystems(configuration);
+        var parseResult = CliParser.Parse(configuration.RootCommand, args, configuration);
+        return parseResult;
+    }
+
+    public CliExit 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)
+    {
+        var cliExit = Execute(Parse(configuration, args), rawInput, consoleHack);
+        return TearDownSubsystems(cliExit);
+    }
+
+    public CliExit Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
+    {
+        var pipelineContext = new PipelineContext(parseResult, rawInput, this, consoleHack ?? new ConsoleHack());
+        ExecuteSubsystems(pipelineContext);
+        return new CliExit(pipelineContext);
+    }
+
     protected virtual void InitializeHelp(CliConfiguration configuration)
         => Help?.Initialize(configuration);
 
@@ -22,20 +48,28 @@ protected virtual void InitializeVersion(CliConfiguration configuration)
     protected virtual void InitializeErrorReporting(CliConfiguration configuration)
         => ErrorReporting?.Initialize(configuration);
 
-    protected virtual void InitializeCompletions(CliConfiguration configuration)
+    protected virtual void InitializeCompletion(CliConfiguration configuration)
         => Completion?.Initialize(configuration);
 
-    protected virtual void TearDownHelp(PipelineContext context)
-        => Help?.TearDown(context);
+    protected virtual CliExit TearDownHelp(CliExit cliExit)
+        => Help is null
+                ? cliExit
+                : Help.TearDown(cliExit);
 
-    protected virtual void TearDownVersion(PipelineContext context)
-        => Version?.TearDown(context);
+    protected virtual CliExit? TearDownVersion(CliExit cliExit)
+        => Version is null
+                ? cliExit
+                : Version.TearDown(cliExit);
 
-    protected virtual void TearDownErrorReporting(PipelineContext context)
-        => ErrorReporting?.TearDown(context);
+    protected virtual CliExit TearDownErrorReporting(CliExit cliExit)
+        => ErrorReporting is null
+                ? cliExit
+                : ErrorReporting.TearDown(cliExit);
 
-    protected virtual void TearDownCompletions(PipelineContext context)
-        => Completion?.TearDown(context);
+    protected virtual CliExit TearDownCompletions(CliExit cliExit)
+        => Completion is null
+                ? cliExit
+                : Completion.TearDown(cliExit);
 
     protected virtual void ExecuteHelp(PipelineContext context)
         => ExecuteIfNeeded(Help, context);
@@ -49,58 +83,57 @@ protected virtual void ExecuteErrorReporting(PipelineContext context)
     protected virtual void ExecuteCompletions(PipelineContext context)
         => ExecuteIfNeeded(Completion, context);
 
-    protected static void ExecuteIfNeeded(CliSubsystem? subsystem, PipelineContext pipelineContext)
-    {
-        if (subsystem is not null && (!pipelineContext.AlreadyHandled || subsystem.RunsEvenIfAlreadyHandled))
-        {
-            subsystem.ExecuteIfNeeded(pipelineContext);
-        }
-    }
-
-    public virtual void InitializeExtensions(CliConfiguration configuration)
+    // TODO: Consider whether this should be public. It would simplify testing, but would it do anything else
+    // TODO: Confirm that it is OK for ConsoleHack to be unavailable in Initialize
+    /// <summary>
+    /// Perform any setup for the subsystem. This may include adding to the CLI definition,
+    /// such as adding a help option. It is important that work only needed when the subsystem
+    /// 
+    /// </summary>
+    /// <param name="configuration"></param>
+    /// <remarks>
+    /// Note to inheritors: The ordering of initializing should normally be in the reverse order than tear down 
+    /// </remarks>
+    protected virtual void InitializeSubsystems(CliConfiguration configuration)
     {
         InitializeHelp(configuration);
         InitializeVersion(configuration);
         InitializeErrorReporting(configuration);
-        InitializeCompletions(configuration);
+        InitializeCompletion(configuration);
     }
 
-    public virtual void TearDownExtensions(PipelineContext pipelineContext)
+    // TODO: Consider whether this should be public
+    // TODO: Would Dispose be a better alternative? This may be non-dispose like things, such as removing options?
+    /// <summary>
+    /// Perform any cleanup operations
+    /// </summary>
+    /// <param name="pipelineContext">The context of the current execution</param>
+    /// <remarks>
+    /// Note to inheritors: The ordering of tear down should normally be in the reverse order than initializing
+    /// </remarks>
+    protected virtual CliExit TearDownSubsystems(CliExit cliExit)
     {
-        TearDownHelp(pipelineContext);
-        TearDownVersion(pipelineContext);
-        TearDownErrorReporting(pipelineContext);
-        TearDownCompletions(pipelineContext);
+        TearDownCompletions(cliExit);
+        TearDownErrorReporting(cliExit);
+        TearDownVersion(cliExit);
+        TearDownHelp(cliExit);
+        return cliExit;
     }
 
-    protected virtual void ExecuteRequestedExtensions(PipelineContext pipelineContext)
+    protected virtual void ExecuteSubsystems(PipelineContext pipelineContext)
     {
         ExecuteHelp(pipelineContext);
         ExecuteVersion(pipelineContext);
         ExecuteErrorReporting(pipelineContext);
         ExecuteCompletions(pipelineContext);
     }
 
-    public ParseResult Parse(CliConfiguration configuration, string rawInput)
-        => Parse(configuration, CliParser.SplitCommandLine(rawInput).ToArray());
-
-    public ParseResult Parse(CliConfiguration configuration, string[] args)
+    protected static void ExecuteIfNeeded(CliSubsystem? subsystem, PipelineContext pipelineContext)
     {
-        InitializeExtensions(configuration);
-        var parseResult = CliParser.Parse(configuration.RootCommand, args, configuration);
-        return parseResult;
+        if (subsystem is not null && (!pipelineContext.AlreadyHandled || subsystem.RunsEvenIfAlreadyHandled))
+        {
+            subsystem.ExecuteIfNeeded(pipelineContext);
+        }
     }
 
-    public CliExit 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)
-        => Execute(Parse(configuration, args), rawInput, consoleHack);
-
-    public CliExit Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
-    {
-        var pipelineContext = new PipelineContext(parseResult, rawInput, this, consoleHack ?? new ConsoleHack());
-        ExecuteRequestedExtensions(pipelineContext);
-        return new CliExit(pipelineContext);
-    }
 }

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

@@ -118,6 +118,7 @@ internal PipelineContext ExecuteIfNeeded(ParseResult? parseResult, PipelineConte
     protected internal virtual CliConfiguration Initialize(CliConfiguration configuration) => configuration;
 
     // TODO: Determine if this is needed.
-    protected internal virtual PipelineContext TearDown(PipelineContext pipelineContext) => pipelineContext;
+    protected internal virtual CliExit TearDown(CliExit cliExit) 
+        => cliExit;
 
 }