[Discussion] Designing a simpler command setup API

[jonsequitur]

Over the course of beta 2 and beta 4, a couple of conventions were removed because they'd led to too much confusion. We still haven't found the right balance of simplicity and composability, though. The conversation has come up in a number of different issues, so let's use this one to focus the discussion and refine the design. I'll post a few proposals below.

Recap

• In beta 2, we removed CommandHandler.Create from System.CommandLine (and moved it to System.CommandLine.NamingConventionBinder) because its use of naming convention wasn't obvious, and naming mismatches between option or argument names and their associated handler parameters became the most common source of problems people had while trying to use the API. (It was also a reflection-heavy API and needed to be removed in order to support trimming and NativeAOT compilation.) We introduced the Command.SetHandler extension methods as a replacement.

• By beta 4, a problem with a different convention had become apparent in SetHandler, which was that its params IValueDescriptor[] parameter, designed to handle binding parameters from either the ParseResult or ServiceProvider, was confusing people as well. SetHandler could be called without passing any options or arguments to bind to, leading it to try to resolve everything from the service provider, which is the less common case. The fix in beta 4 was to change the params parameter to a IValueDescriptor<T> for each generic parameter. This made code a little less verbose because it plays more nicely with C# type inference. But removing the service provider fallback led to more ceremony for accessing injected objects.

Current SetHandler problems

1. Verbosity

The most common criticism now of SetHandler is that it requires too much ceremony to use. The problem is illustrated by this code snippet, which needs to reference option three times:

var option = new Option<int>("-i"); // 1

var command = new RootCommand
{
    option // 2 
};

command.SetHandler(i => { }, option); // 3

An API that composes these together is needed, but we hope to avoid too much coupling between the handling and parsing APIs. The parser API defines the grammar with a good deal of flexibility, while the handler API defines how to bind symbols regardless of their position in the parsed input. A convenience method that sets up both at once will likely prevent people from creating specific parser configurations, but as these won't be the majority case, a convenience API is warranted.

2. Service provider access is unclear

In the beta2 version of SetHandler, parameters of commonly-used types such as ParseResult, CancellationToken, and IConsole would be implicitly bound from the service provider. The beta 4 version requires that the IValueDescriptor<T> parameter count match the generic type parameter count (e.g. public static void SetHandler<T1, T2>(this Command command, Action<T1, T2> handle, IValueDescriptor<T1> symbol1, IValueDescriptor<T2> symbol2). This makes it unclear what to pass to get instances of these other types. This can be done by implementing a custom BinderBase<T> but this also requires too much ceremony.

3. Directly returning exit codes isn't supported (#1570)

A detailed explanation is in the comment here: #1570 (comment)

[jonsequitur]

Proposal 1

Here's a straw man for an API, thrown out for the purpose of discussion. I encourage people to make other proposals, whether alternatives or alterations.

var root = new RootCommand()
    .SetUp(
        new Option<int>("-x"), // Adds the option to the command
        new Option<int>("-y"), 
        new Argument<string>(), 
        (x, y, message) => { 
            // Do something awesome! 
        });

If you have more options than parameters in your handler, or need to bind objects from the service provider, here are some extra facets, Option.Combine and Bind.FromServiceProvider:

var root = new RootCommand()
    .SetUp( 
        Option.Combine( 
            new Option<int>("-x"),
            new Option<int>("-y"),
            (x, y) => new Point(x, y)), // Adds both options to the command
                                        // No need to implement a custom BinderBase<T>
        new Argument<string>(),         // Arguments and options can be  added directly
        Bind.FromServiceProvider<IConsole>(), // No need to implement a custom BinderBase<T>
        (point, message, console) => // Type inference!
        {
            // Do something awesome!
        });

I'm interested in what people think are the pros, cons, and questions regarding this API. I'll start.

🙂 You don't need to reference a given Option<T> or Argument<T> instance more than once in the code (addressing problem 1 above).

🙂 Use of the service provider (Bind.FromServiceProvider<T>) is explicit (partially addressing problem 2 above).

🙂 You don't need to implement your own BinderBase<T>.

🙂 SetUp returns the Command instance, so the API could support subcommands.

☹️ ....but subcommands have no role in binding, so the API might be a little more cluttered if they're included.

☹️ The name SetUp doesn't explain what it does. (A clearer name might be AddOptionsAndArgumentsAndDefineHandlerThatCanAlsoAccessObjectsFromTheServiceProvider, but that's a little long.)

☹️ Bind isn't discoverable. Is there a better place to put this, perhaps on an existing type such as Handler?

☹️ Combine will require a large number of overloads for different generic type parameter arities, plus probably one that accepts an InvocationContext.

🤔 Would Option.Combine imply other combine method for Argument<T>, for example? (We might want to avoid this because arguments are order-dependent and an API like this might obscure that.)

🤔 How would global options work? Proposal: Make IsGlobal public. (#1736)

🤔 If I add an option to a command using Command.Add and then call this API, what should happen? Proposal: Only add the option or argument if it's not already present on the command, allowing use of the existing parser configuration API alongside this.

[bgever]

☹️ ....but subcommands have no role in binding, so the API might be a little more cluttered if they're included.

I've been preferring an approach where my commands are subclasses of Command to allow for better code isolation, especially to aid with unit tests. The sub commands get added in the constructor.

Therefore a purely chained setup with commands would clutter things only more.

[KathleenDollard]

@bgever How do you setup your command handler. Do you have an example you could share. Its a very different way than we have been thinking about the base layer, although it comes up often in considering wrapping tools.

[bgever]

@KathleenDollard I'm first setting things up in Program.cs:

using System.CommandLine;
using System.CommandLine.Builder;
using System.CommandLine.Parsing;
using Microsoft.Extensions.DependencyInjection;
using MyApp.Cli.Commands;
using MyApp.Cli.Models;
using MyApp.Cli.Services;

// See https://aka.ms/new-console-template for more information.

var rootCommand = new RootCommand();
rootCommand.Description = "MyApp CLI";

rootCommand.AddCommand(new StatusCommand());
rootCommand.AddCommand(new UserCommand());
rootCommand.AddCommand(new PushCommand());
rootCommand.AddCommand(new PullCommand());
rootCommand.AddCommand(new InitCommand());

var commandLineBuilder = new CommandLineBuilder(rootCommand);
commandLineBuilder.AddMiddleware(async (context, next) =>
{
    AppConfig appConfig = new()
    {
        Auth0Domain = "***.us.auth0.com",
        Auth0ClientId = "***",
        ApiBaseUri = new Uri("https://***/api/")
    };

    if (context.ParseResult.Directives.Contains("dev"))
    {
        context.Console.WriteLine("[DEVELOPER MODE]");
        appConfig.Auth0Domain = "***.us.auth0.com";
        appConfig.Auth0ClientId = "***";
        appConfig.ApiBaseUri = new Uri("https://localhost:5001/api/");
    }
    context.BindingContext.AddService(services => appConfig);
    context.BindingContext.AddService(services => new UserService());
    context.BindingContext.AddService(services => new Auth0Client(services.GetRequiredService<AppConfig>()));
    context.BindingContext.AddService(services => new TokenService(
        services.GetRequiredService<UserService>(), services.GetRequiredService<Auth0Client>()));
    context.BindingContext.AddService(services => new ProjectService());
    context.BindingContext.AddService(services => new FileGlobService(Directory.GetCurrentDirectory()));
    context.BindingContext.AddService(services =>
    {
        HttpClient httpClient = new() { BaseAddress = services.GetRequiredService<AppConfig>().ApiBaseUri };
        httpClient.DefaultRequestHeaders.UserAgent.ParseAdd("myapp-cli");
        return new ApiClient(httpClient, services.GetRequiredService<TokenService>());
    });

    await next(context);
});

return await commandLineBuilder
    .UseDefaults()
    .Build()
    .InvokeAsync(args);

And then I have individual command files, like the ones listed in Program.cs like the UserCommand below, which also has sub commands.

Because the handler is exposed as a method on the class, it's easy to unit test this individual command by simply instantiating the class and calling the handler with mocks.

As you can see, I've already adopted the Bind helper from @jonsequitur's example code.

using System.CommandLine;
using System.CommandLine.Invocation;
using System.CommandLine.IO;
using MyApp.Cli.Commands.User;
using MyApp.Cli.Primitives;
using MyApp.Cli.Services;

namespace MyApp.Cli.Commands;

public class UserCommand : Command
{
    public UserCommand() : base("user")
    {
        Description = "Display user authentication status";

        AddCommand(new LoginCommand());
        AddCommand(new LogoutCommand());

        var refreshOption = new Option<bool>("--refresh", "Force access token refresh even if hasn't expired yet");
        Add(refreshOption);

        this.SetHandler(
            UserHandler,
            refreshOption,
            Bind.FromServiceProvider<TokenService>(),
            Bind.FromServiceProvider<Auth0Client>(),
            Bind.FromServiceProvider<InvocationContext>());
    }

    public async Task UserHandler(
        bool refresh, TokenService tokenService, Auth0Client auth0Client, InvocationContext context)
    {
        var console = context.Console;
        var cancellationToken = context.GetCancellationToken();
        string? accessToken = await tokenService.GetAccessToken(cancellationToken, refresh);
        if (accessToken == null)
        {
            console.Error.WriteLine("User not logged in");
            context.ExitCode = 10;
            return;
        }

        var userInfo = await auth0Client.GetUserInfo(accessToken, cancellationToken);
        console.WriteLine($"Logged in as {userInfo.FullName} ({userInfo.Email})");
    }
}

[KathleenDollard]

cool. That is one of the reasons Command is not sealed.

[KathleenDollard]

Comments on the non-happy questions. My opinions here are heavily based on our conversations today

☹️ ....but subcommands have no role in binding, so the API might be a little more cluttered if they're included.

Do not include subcommands. This takes us into the same messy space that we abandoned when we abandoned the fluent approach.

☹️ The name SetUp doesn't explain what it does. (A clearer name might be AddOptionsAndArgumentsAndDefineHandlerThatCanAlsoAccessObjectsFromTheServiceProvider, but that's a little long.)

Setup is good enough. If we get a better name, that would be grand.

☹️ Bind isn't discoverable. Is there a better place to put this, perhaps on an existing type?

☹️ Combine will require a large number of overloads for different generic type parameter arities, plus probably one that accepts an InvocationContext.

I dislike the user of the random static classes. Put Combine on Option and only combine Options for now. Bind is more problematic, but lets find an existing class to add the static method to, not creating a new one.

🤔 Would Option.Combine imply other combine method for Argument, for example? (We might want to avoid this because arguments are order-dependent and an API like this might obscure that.)

Not now.

🤔 How would global options work? Proposal: Make IsGlobal public. (#1736)

Yes, public and settable, or settable via the constructor

🤔 If I add an option to a command using Command.Add and then call this API, what should happen? Proposal: Only add the option or argument if it's not already present on the command, allowing use of the existing parser configuration API alongside this.

User TryAdd... If it is there, ignore the new one. Only check the current command, not its ancestors : mostly for perf but also because people might depend on an implementation detail.

[KathleenDollard]

Consider Register instead of SetUp.

Capital U in SetUp is ick.

[KathleenDollard]

Proposal 2-K Setting the System.CommandLine handler

This proposal is heavily influenced by the 6/30 design review.

There are several approaches to setting the handler for a System.CommandLine:

• Direct setting of the Handler property.
• A helper method, possibly named SetHandler which adds the specified options, binds them to a handler, and sets the handler.
• Creating a class that implements ICommandHandler
• "Flavors" which are other styles of usage that we anticipate being built on System.CommandLine. These will be separate libraries, and we anticipate they will all directly set the Handler property.

This proposal is focuses on optimizing the first two user approaches for usability and discoverability.

Users may use these three approaches together, and they should play nicely. But, it will be an effort to switch between them, so each approach should minimize the scenarios where a user needs to switch to another scenario.

Return value

One of the considerations in the rest of this proposal is the number of overloads, which is driven by the cross product of two concerns, each of which has two options (4 resulting sets of overloads). These concerns are whether the return value is a Task to support async, and whether it returns int or is void returning. In some cases, generic arity needs to be considered, and in those cases the number of supported generic arguments is multiplied by 4. This is not only a huge number of overloads, it obscures that there are 4 basic categories.

To improve this, we can remove the void returning overloads. When this overload is used, the return value is set via the InvocationContext.ExitCode. This exit code is important for communication between pipeline steps, and should remain. After removing the void overloads, the return value of methods would be the exit code and it would also be set as the InvocationContext.ExitCode. If there are valid scenarios where explicitly setting the InvocationContext.ExitCode to the return value, we could return a nullable and only set InvocationContext.ExitCode when it is not null.

This leaves us two overloads, or two sets of overloads for sync and async. These are fundamentally different, and it is a general best practice to include Async in naming, so we should consider splitting async operations. Prior to any other change proposed here, that would mean a Handler and a HandlerAsync, and a SetHandler and a SetHandlerAsync, which is discussed further below.

Note that nothing in this proposal requires we do changes to overloads, but I wrote it up as though we also made this orthogonal decision.

Directly setting the Handler property

There was some discussion yesterday about two methods, but I think we are best served to have a clear structure (which we have now) where things are explicitly set up, and a single helper method to make it easier. The helper method is discussed in the next major section of this proposal.

The code for the first approach would be similar to:

var option1 = new Option(...);
var option2 = new Option(...);
var arg1 = new Argument(...);
var arg2 =  new Argument(...);

var command = new Command(...)
{
    option1,
    option2,
    arg1,
    arg2
};

command.Handler = context => {  
    var x = context.Get(option1);
    var y = context.Get(option2);
    var a = context.Get(arg1);
    var b = context.Get(arg2);
    // do work with the variables you gathered
}

Improving the discoverability of Handler usage

The type of the Handler is currently just Delegate, which gives no hint about usage. Since this is a property, there are no overloads available and we have to have an explicit type or just use the unspecified Delegate. I think sync/async is sufficiently important to create a sync and an async handler - Handler and HandlerAsync. The user would not have to look further at their code to determine if the command usage supported async. The property declarations would result in good discoverability for how the handler would be used:

public Func<InvocationContext, int> Handler { get...}
public Func<InvocationContext, Task<int>> HandlerAsync { get...}

The user experience would be to create a command, discovering the build supporting features like AddOption and then explore Handler and HandlerAsync. Because the only argument to the handler would be the InvocationContext the user would naturally explore that type to find the GetValue method. Since this method has overloads for Option<T> and Argument<T> the programmer could reason out that they needed to maintain access to the option and arguments via variables or fields.

Unnecessary IntelliSense entries should be avoided on this mainline scenario.

Implementation notes

The handler is used in other ways internally, particularly in SetHandler's implementation. This may be implemented as an additional internal handler, or the user's delegate may be wrapped in a method group that has a InvocationContext parameter, or some other approach.

What happens if the user sets both the Handler and the HandlerAsync properties? There may be a scenario where this is correct because one is called with Invoke and the other with InvokeAsync. Since that would apparently be an side case, we should write an analyzer, or at least include in the SCL Validate, and give a warning when this occurs.

Simplifying setting and adding options and arguments

After an option or argument is created, it will always be added to at least one command, and generally will be added immediately to exactly one command. Forcing the user to do this as separate steps is unnecessary boiler plate. This bumps into API guidelines and so we should consider a range of options, but guidelines should not force the user to write two lines of code when one is sufficient and more clear. Also, in the code above, note that it is not possible for the user to compile if they either leave out a declaration or fail to get a value they then attempt to use. There is no help in confirming that everything is added to the command. The pit of success is a single step, however we do that. A few options to provoke further thought:

var command = new Command(...);

var option1 = cmd.AddOption(new Option(...)); // This is not fluent, it returns the option, not the command
var option1 = CreateAndAdd(cmd, new Option(...));
var option1 = new Option(...).AddToCommand(cmd);

In addition to a pattern that guarantees the options and arguments are added and that the already verbose process of creating a CLI is as terse as appropriate, these all connect the option or argument mentally to its command making code easier to peruse. You can't judge this from these short examples, so here is an example from docs:

var delayArgument = cmd.AddOption(new Argument<int>
    (name: "delay",
    description: "An argument that is parsed as an int.",
    getDefaultValue: () => 42));
var delayArgument = CreateAndAdd(cmd, new Argument<int>
    (name: "delay",
    description: "An argument that is parsed as an int.",
    getDefaultValue: () => 42));
var delayArgument = new Argument<int>
    (name: "delay",
    description: "An argument that is parsed as an int.",
    getDefaultValue: () => 42).AddToCommand(cmd);

Alternatives

Do nothing

This will leave usage of Handler difficult to discover and is not recommended.

Add a SetHandler overload that just takes a Func<...>

This would allow us to retain a single Handler property, and we could offer overloads for sync and async. But unless we also make the Handler property readonly, folks will have to reason about two approaches to the same thing. If we went this route, the resulting SetHandler overloads would be something like (assuming we limit overloads, per earlier discussion):

public static  void SetHandler(this Command command, Func<InvocationContext, int> handler)...;
public static void SetHandler(this Command command, Func<InvocationContext, Task<int>> handler)...;
public static void SetHandler<T1>(this Command command, Action<T1> handle, IValueDescriptor<T1> symbol1)...;
public static void SetHandler<T1, T2>(this Command command, Action<T1, T2> handle, IValueDescriptor<T1> symbol1, IValueDescriptor<T2> symbol2)...;
...

The downside of this approach is that the mechanism for one of the approaches is made more difficult to discover because of the presence of the other approach.

A helper method for binding

SetHandler has had a lot of work based on extensive feedback as the major way we have encouraged people to work with System.CommandLine. These changes have reduced the ways you could mess up and improved performance. It is painful to see another change to how it is used :(.

This approach is to use Proposal 1 in this issue, but name it SetHandler:

var cmd = new Command(...){...}; 
cmd.SetHandler(
        new Option<int>("-x"), // Adds the option to the command
        new Option<int>("-y"), 
        new Argument<string>(), 
        (x, y, message) => { 
            // Do something awesome! 
        });

This is a predictable helper command for the underlying approach.

What if there is already an option or argument on the command?

The existing one will be used. First one wins. We could discuss that, but it seems the best way to manage backward compatibility with existing code and to be clear.

What about the order dependency of arguments?

Options do not have an order dependency, arguments do. Having multiple arguments on a command is not common, but is also not rare. The arguments will have to appear in the user's delegate in the same order they appear in the syntax of the CLI. This does not seem onerous, but is something to communicate.

It is also a reason to allow the first symbol to win. For example: It could be a subtle bug if you had two string args that you had added in the correct order with explicit AddArguments, and your delegate took them in the reverse order. We assume it is much more common to add options and arguments before the SetHandler call than after it, and thus allowing the first to win preserves the old behavior.

What about global and shared options?

Note that for performance reasons we will only look at the current command to see if the option or argument already exists, and not look up the CLI tree. For most options and arguments this is the correct behavior. There are two cases where it is not: global and shared options.

Global options are placed on a command and are then implicitly part of all of its descendants. They are often part of the pipeline and it is uncommon for them to be used in handlers, and they do not be declared on a command to be part of the syntax of that command. Examples are --help and --version.

Shared options and arguments are placed on a command and are used by a descendant. All options and arguments in System.CommandLine are available for use on descendants, shared refers to those that are explicitly added as part of the syntax on a descendant. An example is the project name property of the .NET CLI's add command: dotnet <PROJECTNAME> add package <PACKAGENAME>

In addition to these, there can also be repeated options. That is when an option or argument is declared once, but used in many places. An example would be if a verbosity option was created once to ensure consistency, but used on many commands - always syntactically appearing only on the leaf node.

Since the proposed SetHandler does not check its ancestors, any options or arguments included for binding purposes will also be added to the command. This means that by default, all options that are used on more than one command are treated as repeated options by default. When an option or argument appears on both a leaf and an ancestor node, the leaf usage blocks access to the ancestor one.

Global options are identified within System.CommandLine and we can make that explicit by adding a property on options. We can simply skip adding the option to the command if it is identified as a global option.

Shared options require a gesture to avoid adding them to the command. A few thoughts on naming this gesture:

cmd.SetHandler(
        new Option<int>("-x"), // Adds the option to the command
        new Option<int>("-y").NoAdd(), // Does not add option
        new Option<int>("-y").SkipAdding(), // Does not add option
        new Option<int>("-y").FromAncestor, // Does not add option
        new Option<int>("-y").FromOther, // Does not add option
        ...

If this gesture is present, the option or argument is available to the delegate, but is not added to the command.

[kabua]

Over the years, I have written my share of CLI applications and just started a new CLI project today. Therefore, I came across System.CommandLine and was very excited by its prospects. Consequently, I read most of the docs and implemented the "Get started tutorial." After trying to restructure the tutorial into something that looked more like production quality code, I discovered a few pain points, which led me to this discussion.

If I may be so bold as to share my view of what "a simpler command setup API" would look like to me. Based on the concept of Separation of Concern, I first dissected the anatomy of a menu system. I came up with the following four parts: definition, structure, binding, and processing.

Here's a conceptual view:

An anatomy of a menu system
    1) Definitions only:
        - Command 
        - Argument
        - Option
    
    2) Structure:
        Global Settings
            Setting option=Verbose
            Setting option=Trace
        MainMenu
            MenuItem command=File
                MenuItem command=New
            MenuItem command=Quotes
                Inherited Settings
                    Setting option=File
                MenuItem command=Read
                MenuItem command=Add
                MenuItem command=Delete
            MenuItem command=Build
                MenuItem command=Build Solution
                MenuItem command=Rebuild Solution
            MenuItem command=Debug
                MenuItem command=Output
            MenuItem command=Exit

    3) Bindings:
        Bind a command to a method

    4) Processing:
        Process the command line and execute the methods which are associated with the command(s)

And here is a coding example

*** 1) Definitions ***
    -- Arguments --
    public static readonly Argument<string> QuoteArgument = new Argument<string>(name: "quote", description: "Text of quote.");
    public static readonly Argument<string> BylineArgument = new Argument<string>(name: "byline", description: "Byline of quote.");

    -- Options --
    public static readonly Option<bool> TraceOption = new Option<bool>(new[] { "--trace", "-t" }, "Enable tracing");
    public static readonly Option<bool> VerboseOption = new Option<bool>(new[] { "--verbose", "-v" }, "Enable verbose output");

    public static readonly Option<int> DelayOption = new Option<int>("--delay", description: "Delay between lines, specified as milliseconds per character in a line.", getDefaultValue: () => 42);
    public static readonly Option<ConsoleColor> ForegroundColorOption = new Option<ConsoleColor>("--fgcolor", description: "Foreground color of text displayed on the console.", getDefaultValue: () => ConsoleColor.White);
    public static readonly Option<FileInfo> FileOption = new Option<FileInfo>("--file", description: "An option whose argument is parsed as a FileInfo", isDefault: true, 
        parseArgument: result =>
        {
            if (result.Tokens.Count == 0)
            {
                return new FileInfo("sampleQuotes.txt");
            }

            var filePath = result.Tokens.Single().Value;
            if (!File.Exists(filePath))
            {
                result.ErrorMessage = "File does not exist";
                return null;
            }

            return new FileInfo(filePath);
        });

    public static readonly Option<bool> LightModeOption = new Option<bool>("--light-mode", "Background color of text displayed on the console: default is black, light mode is white.");
    public static readonly Option<string[]> SearchTermsOption = new Option<string[]>("--search-terms", "Strings to search for when deleting entries.")
        {
            IsRequired = true,
            AllowMultipleArgumentsPerToken = true
        };

    -- Commands --
    public static readonly Command QuotesCommand = new Command("quotes", "Work with a file that contains quotes.");
    public static readonly Command ReadCommand = new Command("read", "Read and display the file.")
        // It is possible that not all ReadCommand commands need all of these options. Therefore, it is better to define them in the structure section.
        //{
        //    DelayOption,
        //    ForegroundColorOption,
        //    LightModeOption
        //};

    public static readonly Command AddCommand = new Command("add", "Add an entry to the file.").AddAlias("insert");
    public static readonly Command DeleteCommand = new Command("delete", "Delete lines from the file.");

*** 2) Structural Layout  ***
app.AddOptions(Verbose, Trace);

app.MainMenu.AddMenuItems(
    new MenuItem(FileCommand)
        .AddMenuItems(new MenuItem(NewCommand)),
    new MenuItem(QuotesCommand)
        .AddOptions(FileOption)
        .AddMenuItems(
            new MenuItem(ReadCommand, new [] { DelayOption, ForegroundColorOption, LightModeOption }),
            new MenuItem(AddCommand, new [] { QuoteArgument, BylineArgument}),
            new MenuItem(DeleteCommand, new [] { SearchTermsOption })
        ),
    new MenuItem(BuildCommand)
        .AddMenuItems(
            new MenuItem(ReadCommand, new [] { MaxFileSizeOption }),
            new MenuItem(BuildSolutionCommand),
            new MenuItem(RebuildSolutionCommand)
        ),
    new MenuItem(DebugCommand)
        .AddMenuItems(new MenuItem(OutputCommand)),
    new MenuItem(ExitCommand)
);

*** 3) Bindings ***
app.Bindings.Add(command:NewCommand, execute:(args) => 
        {
            // args.Options => [Verbose, Trace]
            // Create file
        });

app.Bindings.AddAsync(command:ReadCommand, execute:async (context, args) => 
    {
        // args.Options => [Verbose, Trace, Delay, ForegroundColor, LightMode]

        var fileService = context.Get<FileService>();   // DI

        await fileService.ReadFile(args.Options[FileOption], args.Options[DelayOption], args.Options[ForegroundColorOption], args.Options[LightModeOption], args.Options[VerboseOption]);
    });

// -- Or --

app.Bindings.AddAsync(ReadCommand, async (fileService, file, delay, fgColor, lightMode, verbose) => await fileService.ReadFile(file, delay, fgColor, lightMode, verbose),
    FileOption, DelayOption, ForegroundColorOption, LightModeOption, VerboseOption);

...

*** 4) Processing ***
return app.RunAsync(args);

Note: the various method names can be changed to whatever; it's the concept that I am trying to convey.

This modified architecture allows defining the parts (Commands, Arguments, Options) as strict definitions and independent of the structure. Thus the bindings are done outside of the command objects, which is more in line with SoC.

I strongly feel associating the command binding/handler directly with the Command is a great mistake.

The code is easy to read, understand, and change without getting bogged down with all the details from the other sections. On the other hand, if the four sections are blended, the understanding is lost, and the code feels messy and is more difficult to change.

[kabua]

This approach allows for

1. Easily discoverable. Both the System.Commandline model and from someone trying to maintain or enhance the code.
2. Easily code reviewed and verification of correctness
3. Follows the Open-Close principle (if the structure section is built line by line vs. a composite structure) but I think the composite structure is actually easier to read and modify
4. Easily testable, only the actual method needs testing. (I.e. fileService.ReadFile() or Program.ReadFile()). But it would also be nice if we could somehow unit test the actual binds, like: app.Execute(command:ReadCommand, args:fileService, file, delay, fgColor, lightMode, verbose);