dotnet / command-line-api

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

Command handler breaking changes not updated in doc #1693

Open AArnott opened 2 years ago

AArnott commented 2 years ago

The https://github.com/dotnet/command-line-api/blob/main/docs/How-To.md#pass-parameters-to-a-method doc prescribes this:

static async Task Main(string[] args)
{
    var rootCommand = new RootCommand();

    rootCommand.Add(new Option<int>("--an-int"));
    rootCommand.Add(new Option<string>("--a-string"));

    rootCommand.SetHandler((int i, string s) => DoSomething(i, s));

    await rootCommand.InvokeAsync(args);
}

public static void DoSomething(int anInt, string aString)
{
    /* do something */
}

But this fails at runtime with:

Unhandled exception: System.ArgumentException: Service not found for type System.Int32.

With quite a bit of trial and error, I learned that I can get it to work with the latest version by storing each option in a local so I can also pass it to the SetHandler method:

using System.CommandLine;

var intOption = new Option<int>("--an-int");
var stringOption = new Option<string>("--a-string");
var rootCommand = new RootCommand
{
    intOption,
    stringOption,
};
rootCommand.SetHandler((int i, string s) => DoSomething(i, s), intOption, stringOption);

await rootCommand.InvokeAsync(args);

static void DoSomething(int anInt, string aString)
{
    /* do something */
}

But this feels overly verbose. Why must I tell the same command about each option twice for it to work? Why can't the handler default to the options and ordering as provided to the command itself?

Anyway, the docs and the latest release on nuget.org are out of sync. Please update one of them.

AArnott commented 2 years ago

Just noticed that #1537 mentions the breaking change and the required syntax as I show above. So it sounds like the doc I linked to is outdated.

baronfel commented 2 years ago

This is all on docs.microsoft.com now, and we've got some cleanup to do in the repo: https://docs.microsoft.com/en-us/dotnet/standard/commandline/define-commands

adschm commented 2 years ago

Quite unfortunate that this moved away from the neat, tiny and simple setup it used beforehand.

Am I really required to track myriads of local variables for options now, for no functional benefit at all?

adschm commented 2 years ago

Oh, even better: The new syntax only supports 8 options, and I have 9. So, looks like the new syntax has broken this package for my use-case entirely, and I will have to find something better.

KalleOlaviNiemitalo commented 2 years ago

@adschm if you use a SetHandler method that gives the InvocationContext to your callback, then the callback can read any number of option values from there.

adschm commented 2 years ago

@KalleOlaviNiemitalo

@adschm if you use a SetHandler method that gives the InvocationContext to your callback, then the callback can read any number of option values from there.

Thanks, but this will still force me to have the options as variables? So instead of just 9 arguments before, I will now still need 9 local variables for the options plus 9 local variables for the values inside the Action? Just to have something working which worked without all of that before?

Edit: I'm aware that this is at least partially off-topic here, so feel free to hide me if you prefer.

jonsequitur commented 2 years ago

We're aware of these issues and we're working on improvements and taking suggestions. There's a longer discussion at #1776.