dotnet / command-line-api

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

Command handlers are always executed #2013

Open emceelovin opened 1 year ago

emceelovin commented 1 year ago

If I run this basic test program(almost verbatim with the MSDN docs), the main argument handler is executed even when no command line arguments are specified at all

using System.CommandLine;
using Stacks.Parsing;

Option<string> mainArgument = new(
    aliases: new string[] { "--main", "-m" },
    description: "path to stacks main sdl file");

RootCommand stacksCommands = new()
{
    mainArgument
};

stacksCommands.SetHandler(MainArgumentHandler, mainArgument);

await stacksCommands.InvokeAsync(args);

Console.ReadKey();

void MainArgumentHandler(string path)
{
    new Parser(path).TryParse(out var appModel);
}

Maybe I'm missing something really silly from the docs? I would assume no commands would be executed with cli arguments missing. If I run the program with a malformed argument, it fails during parsing, and the glossary/index of commands pops up. I would assume this would be the case when there are no command line arguments are specified as well.

This is what I would think would popup when args(string[] args in a top-level statement based project) is empty: image

Also, what is the reason handlers cannot be bound to Options themselves? That code is kind of smelly to me. Even if there is - obvious - internal binding of handlers to options in a command, they're just Action/Action<T>'s. RootCommand could just grab that from the Option<T> from it. Then it could just be a constructor argument or propety on an Option itself. Much more clean IMO. The code above now becomes:

using System.CommandLine;
using Stacks.Parsing;

await new RootCommand()
{
    new Option<string>(
        aliases: new string[] { "--main", "-m" },
        description: "path to stacks main sdl file",
        handler: MainArgumentHandler)
}
.InvokeAsync(args);

Console.ReadKey();

void MainArgumentHandler(string path)
{
    new Parser(path).TryParse(out var appModel);
}
KalleOlaviNiemitalo commented 1 year ago

Option<string> mainArgument is optional unless you set Option.IsRequired.

KalleOlaviNiemitalo commented 1 year ago
stacksCommands.SetHandler(MainArgumentHandler, mainArgument);

This doesn't set a handler for mainArgument. This sets MainArgumentHandler as the handler for RootCommand stacksCommands, and arranges to pass the parsed value of Option<string> mainArgument to the string path parameter of void MainArgumentHandler(string path). If the user runs the command with valid syntax, then MainArgumentHandler is called. If the user does not specify the --main or -m option, then string path will be the default value of the option, which I suppose is null.

KalleOlaviNiemitalo commented 1 year ago

Instead of Option<string>, you could also use some Option<AppModel> type and the Option<T>(String[], ParseArgument<T>, Boolean, String)-system-boolean-system-string)) constructor, then implement a method for the ParseArgument<AppModel> delegate. That way, you'd associate the AppModel parsing with the option itself, rather than with the handler of the command.

emceelovin commented 1 year ago
stacksCommands.SetHandler(MainArgumentHandler, mainArgument);

This doesn't set a handler for mainArgument. This sets MainArgumentHandler as the handler for RootCommand stacksCommands, and arranges to pass the parsed value of Option<string> mainArgument to the string path parameter of void MainArgumentHandler(string path). If the user runs the command with valid syntax, then MainArgumentHandler is called. If the user does not specify the --main or -m option, then string path will be the default value of the option, which I suppose is null.

Thank you. Appreciate that very much. This fixed both of my issues. I must've missed the required/not required part of the docs. It's not called now, and it displays the appropriate error page. So basically, with this package, the handler for RootCommand is basically mapped to the data type bound to each Option?

E.g. prog.exe -o1 <int arg> -o2 <string arg> -o3 <bool arg> -> void RootCommandHandler(int arg1, string arg2, bool arg3) Where, as you mentioned, will have values(default is IsRequired == false), or whats specified? If this is the case, are there any plans for a source generator based approach to generate a stubbed Main that maps directly to command line arguments? It would be really convenient - and should've existed since day one - if csharp supported a typed Main. I've dealt with making generators a lot. It shouldn't be hard to come up with something. Maybe something like this:

class CreateUserArgs
{
    public string? FirstName { get; set; }

    public string? LastName { get; set; }
}

class Privileges
{
}

class CommandsAttribute
    : Attribute
{
    public CommandsAttribute(params Type[] types)
    {
    }
}

static class ConsoleApp
{
    [Commands(typeof(CreateUserArgs), typeof(Privileges))]
    static void Main(string[] args)
    {
        // generated by generator
        MyMain(parsedArguments, ...);
    }

    static void MyMain(CreateUserArgs user, Privileges?/* nullable indicates IsRequired = false*/ privileges)
    {
    }
}

It would be pretty easy to listen with the syntax visitor for the ConsoleApp TypeDeclarationSyntax, grab the Main MethodDeclarationSyntax from the Member's property, and then use whatever parser you guys have that converts the string[] into types based on the ParameteListSyntax.

Option<string> mainArgument is optional unless you set Option.IsRequired.

Can you elaborate on this? Also, how come there isn't an IsRequired parameter on Option<T>? Then it could just be specified in the constructor.

KalleOlaviNiemitalo commented 1 year ago

AFAIK a source generator is being worked on. See https://github.com/dotnet/csharplang/discussions/701#discussioncomment-4509437

emceelovin commented 1 year ago

That's great to see other people would like to see the same thing