Allow rejection of wrong number of arguments

[rrthomas]

At present, it's really hard to tell whether your command was passed the number of arguments it asked for. I gave up and patched commander, and still had to check in two places (one for too many, one for zero; too few and greater than zero are checked automatically by the argument parser).

A solution would be to insist that the arguments have to match the syntax (i.e. optional arguments may be omitted, required arguments must be present, additional arguments are not allowed), and if zero arguments is not a valid number, treat that case as if "--help" were given (but preferably exit with error).

I have a patch for this which I can make into a pull request.

[thethomaseffect]

When you say arguments, do you mean options or do you mean the number of arguments passed to the command/option?

[rrthomas]

I mean the number of arguments passed to the command.

[thethomaseffect]

Could you provide an example?

[rrthomas]

Suppose I declare:

program.arguments("<host> [cmd]")

then commander should give an error if I supply fewer than one or more than two arguments.

When a command can take an arbitrary number of arguments, that can be indicated by ..., and then any number of arguments greater than the minimum is allowed, e.g.

program.arguments("<style> <file>...")

The patch I have does all of this (and I've been using it now for some weeks!).

[thethomaseffect]

Okay I get you now, thanks for the example! @SomeKittens @zhiyelee could you guys provide some input on this?

I'll be honest, I never even considered getting arguments like that. Using options means the order is not significant, so your examples would change to:

$ ./program --cmd 'example' host.domain.com
$ ./program host.domain.com
$ ./program --style '/path/to/style.css' --file '/path/to/file1.html,/path/to/file2.html'

Obviously the biggest problem with the above is that you'd have to check if the options were provided but we could solve this with a .required() function for options that adds a check to make sure it was provided.

I think doing it this way would make the way to use your app a lot more obvious to users from just looking at the way it was called. I understand this isn't what your patch does but I'm hesitant to change the parser too much from it's current version without a really good reason.

[rrthomas]

What I have done is simply brought to top-level commands the ability to specify argument syntax that already exists for sub-commands, and made it a bit more rigorous.

My second example specifies two mandatory arguments, whereas you've replaced them with options, which are by their nature optional. I think the idea of "mandatory options" is likely to confuse.

You're right that this is a change to the parser; however, it can be made backwards-compatible by defaulting the maximum number of allowed arguments to Infinity, so that if the arguments method is not called, things will work as before.

[SomeKittens]

This should be done in your verification functions.

[rrthomas]

@SomeKittens As I said in my initial report, finding out whether you got a suitable number of arguments is currently quite hard: even after patching commander, I still had to make checks in two separate places in my code.

Further, what this PR does is simply extend to top-level commands the ability to specify argument syntax that already exists for sub-commands. Once you have that, commander might as well check the number of arguments: still having to do it in your code breaks DRY. Commander has the information to hand, why not relieve the user of this bit of administrivia?

[thethomaseffect]

Yeah I've changed my mind, but I propose a simpler solution: Check if number of provided arguments matches total number of required arguments and check if number of provided arguments exceeds total expected arguments in cases where variadic arguments aren't used. If either of these fails print usage.

The only issue with this is I'm working towards making usage an optional plugin enabled by default, so it might be best to sit on this until that's done so there isn't even more code to change later. (If this seems strange, a lot of people just like using commander for parsing args and don't want extra behavior that prints to console like --help, or want to change the way they behave)

If you want to split up a pull request and change the top-level command argument syntax now though that would be great, and we can add automatic usage printing at a later time :)

[rrthomas]

The top-level command argument syntax change is PR#258. Sorry that I only just got around to reading your last comment, I'd in fact already made the PR you asked for, AFAICT!

[hb20007]

This would be a useful feature for the CLI I'm working on. Currently, I'm using a variadic argument to capture all arguments as an array and then checking if the array length is what I expect.

[shadowspawn]

I think the PR referred to here for argument syntax has been merged, too few arguments causes an error, but no default error for too many arguments.

This issue has not had much activity in four years. It isn't likely to get acted on due to this report.

Feel free to open a new issue if it comes up again, with new information and renewed interest.

Thank you for your contributions.