dotnet / command-line-api

Command line parsing, invocation, and rendering of terminal output.
https://github.com/dotnet/command-line-api/wiki
MIT License
3.34k stars 375 forks source link

Powderhouse Pipeline API #2409

Open KathleenDollard opened 2 months ago

KathleenDollard commented 2 months ago

Scenarios:

Changes to consider that do not need explanation

Basic API shape

Accessing subsystems directly is an advanced scenario so is via static methods on the Subsystem class.

The method parameters are discussed in the next sections.

// Only used by subsystem authors
public abstract class CliSubsystem
{
    protected CliSubsystem(string name, SubsystemKind subsystemKind, IAnnotationProvider? annotationProvider)

    public string Name { get; }
    public SubsystemKind SubsystemKind { get; }

    protected internal bool TryGetAnnotation<TValue>(CliSymbol symbol, AnnotationId<TValue> id, [NotNullWhen(true)] out TValue? value)
    protected internal void SetAnnotation<TValue>(CliSymbol symbol, AnnotationId<TValue> id, TValue value)

    protected internal virtual bool RunsEvenIfAlreadyHandled { get; protected set; }

    protected internal virtual CliConfiguration Initialize(InitializationContext context)
    protected internal virtual bool GetIsActivated(PipelineContext pipelineContext) => false;
    protected internal virtual CliExit Execute(ExecutionContext pipelineContext) => CliExit.NotRun(pipelineContext.ParseResult);
    internal PipelineContext ExecuteIfNeeded(PipelineContext pipelineContext)

    protected internal virtual CliExit TearDown(CliExit cliExit) 
    internal PipelineContext ExecuteIfNeeded(ParseResult? parseResult, PipelineContext pipelineContext)
}

public class Subsystem
{
    public static void Initialize(CliSubsystem subsystem, CliConfiguration configuration, IReadOnlyList<string> args)
    public static bool GetIsActivated(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
    public static CliExit Execute(CliSubsystem subsystem, PipelineContext pipelineContext)
    public static CliExit Execute(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
    public static CliExit ExecuteIfNeeded(CliSubsystem subsystem, ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null)
    public static PipelineContext ExecuteIfNeeded(CliSubsystem subsystem, PipelineContext pipelineContext)
}

public class Pipeline
{
    public static Pipeline Create(HelpSubsystem? help = null,
                                  VersionSubsystem? version = null,
                                  CompletionSubsystem? completion = null,
                                  DiagramSubsystem? diagram = null,
                                  ErrorReportingSubsystem? errorReporting = null,
                                  ValueSubsystem? value = null)
        => new()
        {
            Help = help ?? new HelpSubsystem(),
            Version = version ?? new VersionSubsystem(),
            Completion = completion ?? new CompletionSubsystem(),
            Diagram = diagram ?? new DiagramSubsystem(),
            ErrorReporting = errorReporting ?? new ErrorReportingSubsystem(),
            Value = value ?? new ValueSubsystem()
        };

    public static Pipeline CreateEmpty() 
        => new();

    private Pipeline() { }

    public HelpSubsystem? Help { get; set; }
    public VersionSubsystem? Version { get; set; }
    public CompletionSubsystem? Completion { get; set; }
    public DiagramSubsystem? Diagram { get; set; }
    public ErrorReportingSubsystem? ErrorReporting { get; set; }
    public ValueSubsystem? Value { get; set; }

    public ParseResult Parse(CliConfiguration configuration, string rawInput)
    public ParseResult Parse(CliConfiguration configuration, IReadOnlyList<string> args) // Initializes subsystems. Is that good?

    public CliExit Execute(CliConfiguration configuration, string rawInput, ConsoleHack? consoleHack = null)
    public CliExit Execute(CliConfiguration configuration, string[] args, string rawInput, ConsoleHack? consoleHack = null)
    public CliExit Execute(ParseResult parseResult, string rawInput, ConsoleHack? consoleHack = null

    protected virtual void InitializeSubsystems(InitializationContext context)
    protected virtual CliExit TearDownSubsystems(CliExit cliExit)
    protected virtual void ExecuteSubsystems(PipelineContext pipelineContext)
    protected static void ExecuteIfNeeded(CliSubsystem? subsystem, PipelineContext pipelineContex

    // These were created to allow control over ordering, but now seem a questionable idea.
    protected virtual void InitializeHelp(InitializationContext context)
    protected virtual void InitializeVersion(InitializationContext context)
    protected virtual void InitializeCompletion(InitializationContext context)
    protected virtual void InitializeDiagram(InitializationContext context)
    protected virtual void InitializeErrorReporting(InitializationContext context)
    protected virtual CliExit TearDownHelp(CliExit cliExit)
    protected virtual CliExit? TearDownVersion(CliExit cliExit)
    protected virtual CliExit TearDownCompletion(CliExit cliExit)
    protected virtual CliExit TearDownDiagram(CliExit cliExit)
    protected virtual CliExit TearDownErrorReporting(CliExit cliExit)
    protected virtual void ExecuteHelp(PipelineContext context)
    protected virtual void ExecuteVersion(PipelineContext context)
    protected virtual void ExecuteCompletion(PipelineContext context)
    protected virtual void ExecuteDiagram(PipelineContext context)
    protected virtual void ExecuteErrorReporting(PipelineContext context)
}

Note the public Subsystem methods will create a temporary pipeline and context if needed. This avoids extra overloads on the subsystems.

Design 1

This design favors limiting passed data to what is available (not providing a null ParseResult to Initialize) and comfortably usable (not providing an exit code to CheckIsActive).

public class InitializationContext(CliConfiguration configuration, IReadOnlyList<string> args)
{
    public CliConfiguration Configuration { get; }
    public IReadOnlyList<string> Args { get; } 
}

public class PipelineContext(ParseResult? parseResult, string rawInput, Pipeline? pipeline, ConsoleHack? consoleHack = null)
{
    public ParseResult? ParseResult { get; } 
    public string RawInput { get; } 
    public Pipeline Pipeline { get; } 
    public ConsoleHack ConsoleHack { get; } 
}

public class ExecutionContext(ParseResult? parseResult, string rawInput, Pipeline? pipeline, ConsoleHack? consoleHack = null)
{
    public ParseResult? ParseResult { get; } 
    public string RawInput { get; } 
    public Pipeline Pipeline { get; } 
    public ConsoleHack ConsoleHack { get; } 

    public bool AlreadyHandled { get; set; }
    public int ExitCode { get; set; }
}

Design 2

Use the pipeline as the source of truth and have the Pipeline be a required parameter to CliSubsystem constructor.

This would add the following to Pipeline shown above (and soon to be renamed Cli).

    public CliConfiguration Configuration? { get; }
    public IReadOnlyList<string>? Args { get; } 
    public ParseResult? ParseResult { get; } 
    public string? RawInput { get; } 
    public ConsoleHack ConsoleHack { get; } 

    public bool AlreadyHandled { get; set; }
    public int ExitCode { get; set; }

This would expect, perhaps require, that everything in the template used the same ConsoleHack (with a great new name of course).

Configuration, Ars, ParseResult and RawInput would be null or throw if called prior to parsing.

This would tie the current state of the CLI to a particular execution.

Design 3

This design retains the Pipeline as shown in Basic API Shape and creates a new class called Cli. In this mindset, the Pipeline is what is done to the Cli and the Cli is a combination of the Cli shape and the pipeline context information.

I have a hard time getting into this mindset.

Current plan

I am most comfortable in the mindset that there is a CLI configuration/definition and a pipeline it can run against, and the run is ephemeral. Thus I think the first design is the right one. With tweaks as we like.