Incorrectly guessed parameters to boolean options should be arguments to a command.

[karthikv]

To illustrate the issue, consider the simple command below that prints the string it is given. Call this file cmd.js:

#!/usr/bin/env node
var program = require('commander');

program.command('print <str>')
  .option('-e, --example', 'Example boolean option')
  .action(function(str, env) {
    console.log(str);
  });

program.parse(process.argv);

When run without any options, it works fine. The problem arises when the -e or --example boolean option is provided before the input <str>, like so:

$ ./cmd.js print hello
# => hello

$ ./cmd.js print -e hello
# => error: missing required argument `str'

Even though -e is a boolean option, "hello" is guessed to be its argument in the lib/commander.js on line 504:

// looks like an option
if (arg.length > 1 && '-' == arg[0]) {
  unknownOptions.push(arg);

  // If the next argument looks like it might be
  // an argument for this option, we pass it on.
  // If it isn't, then it'll simply be ignored
  if (argv[i+1] && '-' != argv[i+1][0]) {
    unknownOptions.push(argv[++i]);
  }
  continue;
}

The "hello" argument is then ignored, which makes the command fail.

I have fixed this issue in the latest commit below by reinserting incorrectly guessed option parameters back into the arguments array for a command. Let me know if you have any questions about the code and if there any changes I need to make.

I also noticed that the date test in test.prompt.js was failing. It seems to be that this line:

actual.should.be.an.instanceof(Date);

was acting up even though actual was indeed an instance of Date. In fact, I changed it to:

(new Date()).should.be.an.instanceof(Date);

and it still failed. As a result, I modified the test to use should.equal(true) instead, like so:

(actual instanceof Date).should.equal(true);

and it passed.

Update: I've omitted the test fix above to make this pull request focus on just one issue. Let me know if you'd like another pull request for the tests; it's just a minor bug anyway.

I've been using commander.js for one of my personal projects and have been pleased with its robustness. I'd love to fix these issues and give back. Don't hesitate if you have any questions or need me to modify something.

[tj]

hmm yeah sorry for the delay, I'll take a closer look at this when I get back on monday

[karthikv]

I'm uncertain if you already saw the thread for issue #56, which tries to solve this same problem, but the solution that was integrated there doesn't work in all cases. See my comment for more information.

On another note, I limited this pull request to just solving the parameter issue and got rid of the test fix. The latter was just a minor bug anyway; let me know if you'd like another pull request for that.

[noazark]

I'd love to see this fixed, is there still interest in this PR?

[zhiyelee]

I will look into this later.

[SomeKittens]

@zhiyelee Any updates? We can close this if not.

[whyleee]

👍 this issue breaks my tool which acts as a tiny git replacement. For ex. this command (called by bower) is not working: git clone https://github.com/whyleee/global-tunnel.git -b v1.1.0 --progress D:\test --depth 1, when progress is configured as a boolean option. I made a fix for that long time ago (whyleee@2180137) and it looks like the issue is still not fixed.

[shadowspawn]

This Pull Request has not had any activity in over six months. 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.