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)
Ack. This is what I need to combine
Options andInvocationContextwith the strongly-typedAction. I’ll wait for beta5.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:
So users can write
return ExitCode.Success;overreturn 0;.Commands are expected to handle failure situations and turn them into exit codes. I’m surprised
SetHandlerAPI 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
SetHandlermethods can have anenumconstraint:We can include a default
enum:Example with a custom
enum: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:
(These parameters can include types like
CancellationTokenorInvocationContextusing a custom binder, which we’ll simplify in beta 5 by introducing something likeBind.FromServiceProvider<T>().)Those that provide an
InvocationContext, like this: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
SetHandleroverloads, accepting these delegate types:ActionAction<InvocationContext>Action<T1>throughAction<T1, T2, T3, T4, T5, T6, T7, T8>Func<Task>Func<InvocationContext, Task>Func<T1, Task>throughAction<T1, T2, T3, T4, T5, T6, T7, T8, Task>If you return
Task<int>from any of theFunc*overloads, it will be used to set the exit code:The problem here is that you get a compiler error if you try to use an
asyncdelegate:Supporting directly returning an exit code would double the number of
SetHandleroverloads, adding:Func<int>Func<InvocationContext, int>Func<T1, int>throughAction<T1, T2, T3, T4, T5, T6, T7, T8, int>Func<Task<int>>Func<InvocationContext, Task<int>>Func<T1, Task<int>>throughAction<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
Taskto 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.