Extra arguments : document and add configuration to fail if they exist

[Lectem]

By default it seems that any extra argument will be available in an array after the command object.

The issue comes from the fact that it is not a documented behavior, and if the callback does not have this additional array parameter, it will silently be executed.

While this can be useful if you want to parse those extra arguments yourself, I think the default should be to error out when some arguments are not parsed (or at least, give it as an config option).

Right now, one must do the following:

.command('somecommand <filePath>')
.action((filePath, cmdObj, additionalArgs) => {
        if (additionalArgs && additionalArgs.length > 0)
        {
            cmdObj.help();
        }
        // ...
});

It would be nice if something like this could have the same effect:

.command('somecommand <filePath>')
.noExtraArgs(true)
.action((filePath) => {
        // ...
});

[shadowspawn]

Related issue, which prompted some improvements: #749

I want the error detection myself in one of my own programs.

The arguments are (now) available through cmdObj.args. Are the additional arguments passed to the action handler worthwhile as a documented pattern, or a bit clumsy and not needed as there is an alternative? I wasn't entirely happy with the pattern at the time I added the extra parameters and left them undocumented to see whether they would prove useful, but there is an argument that everything should be passed to the action handler.

[Lectem]

Are the additional arguments passed to the action handler worthwhile as a documented pattern, or a bit clumsy and not needed as there is an alternative?

I do not consider cmdObj.args as a direct alternative since it contains all arguments, not only the extra arguments.
I'm not really in favor of one method or another as I don't have the need for those extra arguments except to validate there aren't any (the noExtraArgs config).
I think it kind of makes sense to have an additional parameter to the callback, but I would probably return an empty array instead of undefined when there are no extra elements. And/or a method cmdObj.extraArgs could wrap cmdObj.args.slice() I suppose?

[shadowspawn]

Related: #259 #1000

[lydell]

I created a helper function for it:

function handleTooManyArgs(action) {
  return (...args) => {
    if (args.length < 2) {
      action(...args);
    } else {
      // The arguments to Commander actions are:
      // expectedCliArg1, expectedCliArg2, expectedCliArgN, Cmd, restCliArgs
      const rest = args[args.length - 1];
      if (rest.length > 0) {
        const expected = args.length - 2;
        const s = expected === 1 ? '' : 's';
        console.error(
          `Expected ${expected} argument${s}, but got ${
            expected + rest.length
          }.`
        );
        process.exit(1);
      } else {
        action(...args);
      }
    }
  };
}

  program
    .command('install <package>')
    .action(
      handleTooManyArgs((packageName, cmd) => {
        // This is the regular .action callback
      })
    );

[shadowspawn]

I'm giving this a try in #1399, still a work in progress.

[shadowspawn]

Second try. Opt-in error for now. #1407

Get the error check by calling .allowExcessArguments(false).

[shadowspawn]

I have switched to an error by default for extra command-arguments. Can opt-out by calling .allowExcessArguments().

This is available in v7.0.0-2 prerelease.

Gentle readers, speak up soon if you think error-by-default is a bad idea! 📣

[shadowspawn]

I am finding this is breaking quite a lot of my unit tests (due to lazily not declaring arguments) and not picking up actual problems. I think for Commander v7 it would be better to make this an opt-in behaviour that is inherited to subcommands (so can easily turn it on for whole program). Command 7 has breaking changes with options storage and action handler parameters, so plenty else for users to deal with!

I'll think a bit more and do a PR for consideration.

[shadowspawn]

Included in Commander 7.0.0.

[lydell]

Thanks! commander 7 is great, well done! 🎉