Fix option parsing with sub-commands.

[sennav]

Hi, my subcommand options were being wrongly parsed by the main command, so I made this PR to just pass them through in case of using a subcommand. Please let me know if I should add or modify anything.

[abetomo]

Thank you!
Since the test has failed, could you confirm it?

[sennav]

Hey, I tried to fix the tests but it seems that there's something with subcommands that I don't understand. Commander expects the subcommands to be the first argument of the argument list, in the tests is common to call the subcommand in any other position.

The README state that:

When .command() is invoked with a description argument, no .action(callback) should be called to handle sub-commands, otherwise there will be an error. This tells commander that you're going to use separate executables for sub-commands, much like git(1) and other popular tools.

But the tests calls subcommands with description argument and with action. So, as I said before, there's something I don't understand about the requirements. I assumed that when using subcommands, all options should be parsed by the subcommand, which is a separate script using commander. Reading the tests I guess that some of them should be parsed by the main command and others by the subcommand which is quite confusing, for me at least.

The problem I was trying to fix was: suppose I define a command foo with subcommand bar <filename>, and defined a boolean option for bar, say -f which is a boolean option with no args. If I issued a command like foo bar -f filename.txt commander will parse filename.txt as a argument of -f and the barscript will never receive it. This is solved by my PR, but I have no clue on how to fix your tests.

I don't know how to proceed on this, and I'm not sure if you want this to be merged, so I'm waiting feedback.

[abetomo]

@sennav Thank you for your reply.

There are two kinds of subcommands.

1. To execute an executable command as a subcommand
2. When processing with callback

I think this case is the case of 1

diff --git a/index.js b/index.js
index 5b6a7d5..633b6f2 100644
--- a/index.js
+++ b/index.js
@@ -675,7 +675,7 @@ Command.prototype.optionFor = function(arg) {
 
 Command.prototype.isSubcommand = function(name) {
   for (var i = 0; i < this.commands.length; ++i) {
-    if (name === this.commands[i]._name) {
+    if (this.commands[i]._execs[name] === true) {
       return true;
     }
   }

What about with a fix like that?

[sennav]

@abetomo thank you for clarifying this, I added your suggestion, please let me know if there's anything else to do.

[abetomo]

@sennav It was good if it was helpful.

[shadowspawn]

I think we need some better details about expected pattern for git-style commands! I need to do some experimentation to better understand before reviewing this PR.

Note also, #734 covers some similar ground and includes mentions of aliases which I am not sure this PR covers.

A specific example of a command which this PR fixes would be useful.

[sennav]

Hi I made this PR a while ago, I tried to reproduce the problem I was having with a sample code and I couldn't so I guess it was fixed somehow. I'll close it, thanks.

[shadowspawn]

Thanks for update @sennav