Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default supported verbs/options should not return non-zero exit code #841

Open
I3urny opened this issue Aug 12, 2022 · 2 comments
Open

Default supported verbs/options should not return non-zero exit code #841

I3urny opened this issue Aug 12, 2022 · 2 comments

Comments

@I3urny
Copy link

I3urny commented Aug 12, 2022

This library should not return a non-zero exit code for the default supported verbs and options:

  • help
  • version
  • --help
  • --version
using System;
using CommandLine;

public class Program
{
    [Verb("add", HelpText = "Add file contents to the index.")]
    class AddOptions
    {
        [Option(Default = false, HelpText = "Prints all messages to standard output.")]
        public bool Verbose { get; set; }
    }

    [Verb("commit", HelpText = "Record changes to the repository.")]
    class CommitOptions
    {
    }

    static int Main(string[] args)
    {
        var exitCode = Parser.Default.ParseArguments<AddOptions, CommitOptions>(args)
            .MapResult(
                (AddOptions opts) => 0,
                (CommitOptions opts) => 0,
                errs => 1
            );

        Console.WriteLine($"exitCode = {exitCode}");
        return exitCode;
    }
}

Executing this code results in the following outputs:

> ConsoleApp1.exe version
ConsoleApp1 1.0.0.0
exitCode = 1

> ConsoleApp1.exe help
ConsoleApp1 1.0.0.0
Copyright c  2022

  add        Add file contents to the index.

  commit     Record changes to the repository.

  help       Display more information on a specific command.

  version    Display version information.

exitCode = 1

> ConsoleApp1.exe --version
ConsoleApp1 1.0.0.0
exitCode = 1

> ConsoleApp1.exe --help
ConsoleApp1 1.0.0.0
Copyright c  2022

  add        Add file contents to the index.

  commit     Record changes to the repository.

  help       Display more information on a specific command.

  version    Display version information.

exitCode = 1

> ConsoleApp1.exe help add
ConsoleApp1 1.0.0.0
Copyright c  2022

  --verbose    (Default: false) Prints all messages to standard output.

  --help       Display this help screen.

  --version    Display version information.

exitCode = 1

> ConsoleApp1.exe add --version
ConsoleApp1 1.0.0.0
exitCode = 1

> ConsoleApp1.exe add --help
ConsoleApp1 1.0.0.0
Copyright c  2022

  --verbose    (Default: false) Prints all messages to standard output.

  --help       Display this help screen.

  --version    Display version information.

exitCode = 1

As far as I can tell, those errors can be handled by the consumer as in your code example or by doing it like suggested in #660.

However, since those verbs/options are supported they should not result in an error and therefore also should not require to be handled by the consumer.

@I3urny
Copy link
Author

I3urny commented Sep 4, 2022

Right, so I took a closer look at the code:

The biggest problem is that .MapResult does not necessarily return an integer but rather a generic type meaning that the library cannot always set an appropriate default value to return for help/version. Returning the default value of a type via the default literal would suffice in my case but that does not necessarily apply to other programs that for example use a custom integer, string or boolean.

Rather, the consumer would have to define a return value for help/version and at that point there is not much of a difference between setting a value for those verbs/options or handling the error in a custom function.

// current way
static int Main(string[] args)
{
    return Parser.Default.ParseArguments<AddOptions, CommitOptions>(args)
        .MapResult(
            (AddOptions opts) => 0,     // value for parsed AddOptions
            (CommitOptions opts) => 0,  // value for parsed CommitOptions
            HandleParseError            // function for notParsed/help/version
        );
}

static int HandleParseError(IEnumerable<Error> errs)
{
    var exitCode = 1;

    if (errs.IsHelp() || errs.IsVersion())
        exitCode = 0;

    return exitCode;
}
// alternative way, if .MapResult returned the default value for the given type
static int Main(string[] args)
{
    return Parser.Default.ParseArguments<AddOptions, CommitOptions>(args)
        .MapResult(
            (AddOptions opts) => 0,     // value for parsed AddOptions
            (CommitOptions opts) => 0,  // value for parsed CommitOptions
            errs => 1                   // value for notParsed
        );
}

// alternative way, if the consumer had to define a default value
static int Main(string[] args)
{
    return Parser.Default.ParseArguments<AddOptions, CommitOptions>(args)
        .MapResult(
            0,                          // value for help/version
            (AddOptions opts) => 0,     // value for parsed AddOptions
            (CommitOptions opts) => 0,  // value for parsed CommitOptions
            errs => 1                   // value for notParsed
        );
}

Unless the maintainers of this project want the .MapResult method to return the default value of the specified type, we will have to handle the "errors" caused by help/version ourselves.

@marcopelegrini
Copy link

This behaviour is very weird, no stablished command line I know would return != for version/help options

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants