Refactor subsystem initialization

The configuration and args are now wrapped in an InitializationContext

Author: KathleenDollard

src/System.CommandLine.Subsystems.Tests/AlternateSubsystems.cs

@@ -45,11 +45,11 @@ internal class VersionWithInitializeAndTeardown : VersionSubsystem
             internal bool ExecutionWasRun;
             internal bool TeardownWasRun;
 
-            protected override CliConfiguration Initialize(CliConfiguration configuration)
+            protected override CliConfiguration Initialize(InitializationContext context)
             {
                 // marker hack needed because ConsoleHack not available in initialization
                 InitializationWasRun = true;
-                return base.Initialize(configuration);
+                return base.Initialize(context);
             }
 
             protected override CliExit Execute(PipelineContext pipelineContext)

src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs

@@ -9,7 +9,7 @@ namespace System.CommandLine;
 public class ErrorReportingSubsystem : CliSubsystem
 {
     public ErrorReportingSubsystem(IAnnotationProvider? annotationProvider = null)
-        : base(ErrorReportingAnnotations.Prefix, annotationProvider, SubsystemKind.ErrorReporting)
+        : base(ErrorReportingAnnotations.Prefix, SubsystemKind.ErrorReporting, annotationProvider)
     { }
 
     // TODO: Stash option rather than using string

src/System.CommandLine.Subsystems/HelpSubsystem.cs

@@ -16,7 +16,7 @@ namespace System.CommandLine;
 //          .With(help.Description, "Greet the user");
 //
 public class HelpSubsystem(IAnnotationProvider? annotationProvider = null) 
-    : CliSubsystem(HelpAnnotations.Prefix, annotationProvider: annotationProvider, SubsystemKind.Help)
+    : CliSubsystem(HelpAnnotations.Prefix, SubsystemKind.Help, annotationProvider)
 {
     public void SetDescription(CliSymbol symbol, string description) 
         => SetAnnotation(symbol, HelpAnnotations.Description, description);
@@ -29,15 +29,16 @@ public string GetDescription(CliSymbol symbol)
     public AnnotationAccessor<string> Description 
         => new(this, HelpAnnotations.Description);
 
-    protected internal override CliConfiguration Initialize(CliConfiguration configuration)
+    protected internal override CliConfiguration Initialize(InitializationContext context)
     {
         var option = new CliOption<bool>("--help", ["-h"])
         {
+            // TODO: Why don't we accept bool like any other bool option?
             Arity = ArgumentArity.Zero
         };
-        configuration.RootCommand.Add(option);
+        context.Configuration.RootCommand.Add(option);
 
-        return configuration;
+        return context.Configuration;
     }
 
     protected internal override bool GetIsActivated(ParseResult? parseResult)

src/System.CommandLine.Subsystems/Pipeline.cs

@@ -16,9 +16,9 @@ public class Pipeline
     public ParseResult Parse(CliConfiguration configuration, string rawInput)
         => Parse(configuration, CliParser.SplitCommandLine(rawInput).ToArray());
 
-    public ParseResult Parse(CliConfiguration configuration, string[] args)
+    public ParseResult Parse(CliConfiguration configuration, IReadOnlyList<string> args)
     {
-        InitializeSubsystems(configuration);
+        InitializeSubsystems(new InitializationContext(configuration, args));
         var parseResult = CliParser.Parse(configuration.RootCommand, args, configuration);
         return parseResult;
     }
@@ -39,17 +39,17 @@ public CliExit Execute(ParseResult parseResult, string rawInput, ConsoleHack? co
         return new CliExit(pipelineContext);
     }
 
-    protected virtual void InitializeHelp(CliConfiguration configuration)
-        => Help?.Initialize(configuration);
+    protected virtual void InitializeHelp(InitializationContext context)
+        => Help?.Initialize(context);
 
-    protected virtual void InitializeVersion(CliConfiguration configuration)
-        => Version?.Initialize(configuration);
+    protected virtual void InitializeVersion(InitializationContext context)
+        => Version?.Initialize(context);
 
-    protected virtual void InitializeErrorReporting(CliConfiguration configuration)
-        => ErrorReporting?.Initialize(configuration);
+    protected virtual void InitializeCompletion(InitializationContext context)
+        => Completion?.Initialize(context);
 
-    protected virtual void InitializeCompletion(CliConfiguration configuration)
-        => Completion?.Initialize(configuration);
+    protected virtual void InitializeErrorReporting(InitializationContext context)
+        => ErrorReporting?.Initialize(context);
 
     protected virtual CliExit TearDownHelp(CliExit cliExit)
         => Help is null
@@ -61,28 +61,28 @@ protected virtual CliExit TearDownHelp(CliExit cliExit)
                 ? cliExit
                 : Version.TearDown(cliExit);
 
+    protected virtual CliExit TearDownCompletion(CliExit cliExit)
+        => Completion is null
+                ? cliExit
+                : Completion.TearDown(cliExit);
+
     protected virtual CliExit TearDownErrorReporting(CliExit cliExit)
         => ErrorReporting is null
                 ? cliExit
                 : ErrorReporting.TearDown(cliExit);
 
-    protected virtual CliExit TearDownCompletions(CliExit cliExit)
-        => Completion is null
-                ? cliExit
-                : Completion.TearDown(cliExit);
-
     protected virtual void ExecuteHelp(PipelineContext context)
         => ExecuteIfNeeded(Help, context);
 
     protected virtual void ExecuteVersion(PipelineContext context)
         => ExecuteIfNeeded(Version, context);
 
+    protected virtual void ExecuteCompletion(PipelineContext context)
+        => ExecuteIfNeeded(Completion, context);
+
     protected virtual void ExecuteErrorReporting(PipelineContext context)
         => ExecuteIfNeeded(ErrorReporting, context);
 
-    protected virtual void ExecuteCompletions(PipelineContext context)
-        => ExecuteIfNeeded(Completion, context);
-
     // 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>
@@ -94,12 +94,12 @@ protected virtual void ExecuteCompletions(PipelineContext context)
     /// <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)
+    protected virtual void InitializeSubsystems(InitializationContext context)
     {
-        InitializeHelp(configuration);
-        InitializeVersion(configuration);
-        InitializeErrorReporting(configuration);
-        InitializeCompletion(configuration);
+        InitializeHelp(context);
+        InitializeVersion(context);
+        InitializeCompletion(context);
+        InitializeErrorReporting(context);
     }
 
     // TODO: Consider whether this should be public

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

@@ -1,7 +1,6 @@
 // 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.Annotations;
 using System.Diagnostics.CodeAnalysis;
 
 namespace System.CommandLine.Subsystems;
@@ -12,7 +11,7 @@ namespace System.CommandLine.Subsystems;
 /// <param name="annotationProvider"></param>
 public abstract class CliSubsystem
 {
-    protected CliSubsystem(string name, IAnnotationProvider? annotationProvider, SubsystemKind subsystemKind)
+    protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProvider? annotationProvider)
     {
         Name = name;
         _annotationProvider = annotationProvider;
@@ -115,7 +114,8 @@ internal PipelineContext ExecuteIfNeeded(ParseResult? parseResult, PipelineConte
     /// <param name="configuration">The CLI configuration, which contains the RootCommand for customization</param>
     /// <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
-    protected internal virtual CliConfiguration Initialize(CliConfiguration configuration) => configuration;
+    protected internal virtual CliConfiguration Initialize(InitializationContext context)
+        => context.Configuration;
 
     // TODO: Determine if this is needed.
     protected internal virtual CliExit TearDown(CliExit cliExit) 

src/System.CommandLine.Subsystems/Subsystems/InitializationContext.cs

@@ -0,0 +1,10 @@
+// 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 InitializationContext(CliConfiguration configuration, IReadOnlyList<string> args)
+{
+    public CliConfiguration Configuration { get; } = configuration;
+    public IReadOnlyList<string> Args { get; } = args;
+}

src/System.CommandLine.Subsystems/Subsystems/Subsystem.cs

@@ -5,24 +5,24 @@ namespace System.CommandLine.Subsystems;
 
 public class Subsystem
 {
-    public static void Initialize(CliSubsystem subsystem, CliConfiguration configuration)
-        => subsystem.Initialize(configuration);
+    public static void Initialize(CliSubsystem subsystem, CliConfiguration configuration, IReadOnlyList<string> args)
+        => subsystem.Initialize(new InitializationContext(configuration, args));
 
     public static CliExit Execute(CliSubsystem subsystem, PipelineContext pipelineContext)
         => subsystem.Execute(pipelineContext);
 
     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 PipelineContext(parseResult,  rawInput, null,consoleHack)));
+    public static CliExit ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
+        => new(subsystem.ExecuteIfNeeded(new PipelineContext(parseResult, rawInput, null, consoleHack)));
 
     public static CliExit Execute(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
         => subsystem.Execute(new PipelineContext(parseResult, rawInput, null, consoleHack));
 
 
-    internal static PipelineContext ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack, PipelineContext? pipelineContext = null) 
-        => subsystem.ExecuteIfNeeded(pipelineContext ?? new PipelineContext(parseResult, rawInput, null,consoleHack));
+    internal static PipelineContext ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack, PipelineContext? pipelineContext = null)
+        => subsystem.ExecuteIfNeeded(pipelineContext ?? new PipelineContext(parseResult, rawInput, null, consoleHack));
 
     internal static PipelineContext ExecuteIfNeeded(CliSubsystem subsystem, PipelineContext pipelineContext)
         => subsystem.ExecuteIfNeeded(pipelineContext);

src/System.CommandLine.Subsystems/VersionSubsystem.cs

@@ -12,7 +12,7 @@ public class VersionSubsystem : CliSubsystem
     private string? specificVersion = null;
 
     public VersionSubsystem(IAnnotationProvider? annotationProvider = null)
-        : base(VersionAnnotations.Prefix, annotationProvider, SubsystemKind.Version)
+        : base(VersionAnnotations.Prefix, SubsystemKind.Version, annotationProvider)
     {
     }
 
@@ -34,16 +34,15 @@ public string? SpecificVersion
             ?.GetCustomAttribute<AssemblyInformationalVersionAttribute>()
             ?.InformationalVersion;
 
-
-    protected internal override CliConfiguration Initialize(CliConfiguration configuration)
+    protected internal override CliConfiguration Initialize(InitializationContext context)
     {
         var option = new CliOption<bool>("--version", ["-v"])
         {
             Arity = ArgumentArity.Zero
         };
-        configuration.RootCommand.Add(option);
+        context.Configuration.RootCommand.Add(option);
 
-        return configuration;
+        return context.Configuration;
     }
 
     // TODO: Stash option rather than using string