dotnet / command-line-api

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

Duplicate "--version" keys #1691

Open mmckechney opened 2 years ago

mmckechney commented 2 years ago

After updating to the latest release (System.CommandLine v2.0.0-beta3.22114.1), I am now getting the following error at runtime:

Unhandled exception. System.ArgumentException: An item with the same key has already been added. Key: --version
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.CommandLine.Parsing.StringExtensions.ValidTokens(Command command)
   at System.CommandLine.Parsing.StringExtensions.Tokenize(IReadOnlyList`1 args, CommandLineConfiguration configuration)
   at System.CommandLine.Parsing.Parser.Parse(IReadOnlyList`1 arguments, String rawInput)
   at System.CommandLine.CommandExtensions.GetDefaultInvocationPipeline(Command command, String[] args)
   at System.CommandLine.CommandExtensions.InvokeAsync(Command command, String[] args, IConsole console)

No other changes were made to the app that was previously working. I do use System.CommandLine.NamingConventionBinder to be compatible with the older CommandHandler.Create model, could this be the problem?

jonsequitur commented 2 years ago

Can you provide a code sample to reproduce this?

mmckechney commented 2 years ago

Full sample code can be found here: https://gist.github.com/mmckechney/7bd92a4417458fb8b919b44bac5c5b89

In creating this sample I discovered the root of the problem: The use of the CommandLineBuilder to add additional help context breaks when you include UseDefaults() with the latest Beta3 release.


//This throws exception with: "One or more errors occurred. (An item with the same key has already been added. Key: --version)'"
  var parser = new CommandLineBuilder(rootCommand)
                .UseDefaults()
                .UseHelp(ctx =>
                {
                    ctx.HelpBuilder.CustomizeLayout(_ => System.CommandLine.Help.HelpBuilder.Default
                                             .GetLayout()
                                             .Prepend(
                                                 _ => _.Output.WriteLine("**Adding extra help here**")
                                             ));
                }).Build();

However: I also noticed that if you remove the UseDefaults(), the program runs, but Prepend help text does not get displayed. Is there a new method to prepend to your help message or is this a bug?


//This runs, but the help output is not modified as expected
  var parser = new CommandLineBuilder(rootCommand)
                .UseHelp(ctx =>
                {
                    ctx.HelpBuilder.CustomizeLayout(_ => System.CommandLine.Help.HelpBuilder.Default
                                             .Prepend(
                                                 _ => _.Output.WriteLine("**Adding extra help here**")
                                             ));
                }).Build();
zachgharst commented 2 years ago

Is there an update on this other than the milestone tag? Do we have any way to work around this? I'm experiencing the same problem. I just want to add a logo and copyright before description.

jonsequitur commented 2 years ago

I'm not seeing this issue when calling Invoke on the Parser returned from CommandLineBuilder.Build.

I do see it when I call Invoke directly on the RootCommand.

That should allow people to work around this while we sort out the issue.

perlun commented 1 year ago

For reference, this was also spotted by another user some months later: https://github.com/dotnet/command-line-api/issues/1791#issuecomment-1173015923

perlun commented 1 year ago

I am running into this in my application, but perhaps for slightly different reasons than the rest of you. I have a custom --version option defined like this (heavily simplified example):

var versionOption = new Option<VoidObject>(new[] { "--version", "-v" }, "Show version information");

var rootCommand = new RootCommand { ... };
rootCommand.AddOption(versionOption);

return new CommandLineBuilder(rootCommand)
    .UseDefaults() // <-- this triggers the problem
    .Build()
    .Invoke(args, console);

Changing from .UseDefaults() to .UseHelp() works in my case (but some of my application's options still causes other exceptions, likely because of some other incompatibility since the upgrade).

KalleOlaviNiemitalo commented 1 year ago

There is no UseHelp method any more. The CliRootCommand constructor now adds VersionOption.

https://github.com/dotnet/command-line-api/blob/299ea1ce63170991db8551eee2f9828c0a1b4bde/src/System.CommandLine/CliRootCommand.cs#L26-L30

If you don't want the predefined behavior of VersionOption, you can do one of

But perhaps there is still something to improve in the error handling if duplicate --version options are added.

Regenhardt commented 6 months ago

Still same problem with version 2.0.0-beta4.22272.1. It seems the internal VersionOption is added when the InvokeAsync is called, not on construction, so I see no way of removing it to add my own or setting my own handler to that one before invoking.

This is my RootCommand:

public class TodoCommand : RootCommand
{
    private static readonly Option<bool> Version = new Option<bool>(["-v", "--version"], "Prints out the todo CLI version.");
    public TodoCommand(IServiceProvider serviceProvider)
    {
        // Add static parameters
        Description = "A CLI to manage Microsoft to do items.";

        // Add options
        Add(Version);

        // Add handlers
        this.SetHandler(TodoCommandHandler.Create(), Version);

        // Add subcommands
        Add(new AddCommand(serviceProvider));
        Add(new ListCommand(serviceProvider));
        Add(new CompleteCommand(serviceProvider));
        Add(new RemoveCommand(serviceProvider));
    }
}

And this is how I call it:

var config = new ConfigurationBuilder()
    .AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
    .Build();

var todoCliConfig = new TodoCliConfiguration();
config.Bind("TodoCliConfiguration", todoCliConfig);

var services = new ServiceCollection()
    .AddSingleton(typeof(TodoCliConfiguration), todoCliConfig)
    .AddTransient<ITodoItemRepository>(factory => new TodoItemRepository(TodoCliAuthenticationProviderFactory.GetAuthenticationProvider(factory)));

var serviceProvider = services.BuildServiceProvider();

var todoCommand = new TodoCommand(serviceProvider);
return await todoCommand
    .InvokeAsync(args); // Exception here
Regenhardt commented 6 months ago

Also using Command instead of RootCommand throws the same, I guess the command you use as your root will automatically be treated as the root command.