command-line-api: Add SetHandler overrides for command returning an exit code

The original CommandHandler.Create API supports handler functions that return int so that the command can return an exit code other than 0 if there’s an error. It looks like the new SetHandler API requires that the handler function return either void or Task. Can we add overrides for int and Task<int> before GA? Thanks!

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 4
  • Comments: 23 (15 by maintainers)

Most upvoted comments

(These parameters can include types like CancellationToken or InvocationContext using a custom binder, which we’ll simplify in beta 5 by introducing something like Bind.FromServiceProvider<T>().)

Ack. This is what I need to combine Options and InvocationContext with the strongly-typed Action. I’ll wait for beta5.

I suppose another option that wouldn’t result in an overload explosion is to require returning an exit code on all of the SetHandler APIs. It would be a fairly disruptive change and it’s not clear to me which is the more common pattern.

I think that encourages the use of exit codes and printing user-friendly error messages over throwing Exception, which will be good for the end-user.

Maybe also consider adding something like:

public static class ExitCode
{
    public const int Success = 0;
    public const int Failure = 1;
}

So users can write return ExitCode.Success; over return 0;.

Commands are expected to handle failure situations and turn them into exit codes. I’m surprised SetHandler API doesn’t foresee this.

The current API encourages throwing exceptions over returning exit codes. That gives a bad UX for the end-user.

Maybe we should use enums to represent command results?

The SetHandler methods can have an enum constraint:

    void SetHandler<T>(Func<Task<T>> handle) where T : System.Enum;
    void SetHandler<T>(Func<T> handle) where T : System.Enum;
    ...

We can include a default enum:

enum CommandResult
{
    Success = 0,
    Failure = 1
}

Example with a custom enum:

enum MyCommandStatus
{
     Success = 0,
     Failure1 = 100,
     Failure2 = 200
}
command.SetHandler(() => MyCommandStatus.Failure1);

In beta 4, in order to keep the combinatorial overload explosion to a minimum, we only provide two flavors of SetHandler:

  • Those that bind parameters for you, like this:

    var option = new Option<int>("-i");
    // ...
    command.SetHandler(i => { }, option);
    

    (These parameters can include types like CancellationToken or InvocationContext using a custom binder, which we’ll simplify in beta 5 by introducing something like Bind.FromServiceProvider<T>().)

  • Those that provide an InvocationContext, like this:

    var option = new Option<int>("-i");
    // ...
    command.SetHandler(context => 
    { 
        var i = context.ParseResult.GetValueForOption(option);
    });
    

The current API encourages throwing exceptions over returning exit codes.

Exceptions are caught and translated into non-zero exit codes by default, or you can assign the exit code to InvocationContext.ExitCode.

Ok, so where does that leave us? Right now there are 20 SetHandler overloads, accepting these delegate types:

  • Action
  • Action<InvocationContext>
  • Action<T1> through Action<T1, T2, T3, T4, T5, T6, T7, T8>
  • Func<Task>
  • Func<InvocationContext, Task>
  • Func<T1, Task> through Action<T1, T2, T3, T4, T5, T6, T7, T8, Task>

If you return Task<int> from any of the Func* overloads, it will be used to set the exit code:

command.SetHandler(i =>
{
    return Task.FromResult(123);
}, option);

The problem here is that you get a compiler error if you try to use an async delegate:

// THIS DOES NOT COMPILE IN BETA 4
command.SetHandler(async i =>
{
    return 123;
}, option);

Supporting directly returning an exit code would double the number of SetHandler overloads, adding:

  • Func<int>
  • Func<InvocationContext, int>
  • Func<T1, int> through Action<T1, T2, T3, T4, T5, T6, T7, T8, int>
  • Func<Task<int>>
  • Func<InvocationContext, Task<int>>
  • Func<T1, Task<int>> through Action<T1, T2, T3, T4, T5, T6, T7, T8, Task<int>>

This would be a usability improvement in some ways and not in others. IntelliSense is already quite crowded for this method.

Adding support for directly returning exit codes would nearly double this. We’re looking for a more elegant solution and would love suggestions. This is related to #1776 so I’ll add these details there.

It’s hard to know what’s the right number of overloads. I think there are too many and it makes intellisense a bit confusing. (The source generator approach we’ve been experimenting with is basically just generating the exact signature you need.)

Doing away with return types (other than simple Task to support async) can help keep this under control and leaves the door open to have other return types from handlers, e.g. data objects that a rendering layer can then present in the console.