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

Inconsistent behavior when defining sub-command #256

Closed
rrthomas opened this issue Sep 8, 2014 · 6 comments
Closed

Inconsistent behavior when defining sub-command #256

rrthomas opened this issue Sep 8, 2014 · 6 comments

Comments

@rrthomas
Copy link
Contributor

rrthomas commented Sep 8, 2014

The documented API for defining a sub-command is:

  program
    .command('setup')
    .description('run remote setup commands')
    .action(function(){
      console.log('setup');
    });

This does not automatically call addImplicitHelpCommand. The undocumented form

 program.command('setup', 'run remote setup commands')

does. It would seem reasonable to move the code that sets the executables flag, which in turn causes addImplicitHelpCommand to be called, from the parse method into the description method. Would a patch to do this be acceptable?

@abetomo
Copy link
Collaborator

abetomo commented Mar 16, 2018

Closing due to possible answer suggested and no response registered. Feel free to comment and we can re-open in case this issue persisted.

@abetomo abetomo closed this as completed Mar 16, 2018
@rrthomas
Copy link
Contributor Author

I don't understand why you closed this: my "possible answer" involved patching commander.js, so it's not a workaround.

@abetomo
Copy link
Collaborator

abetomo commented Mar 16, 2018

I'm sorry! I was mistaken.

@abetomo abetomo reopened this Mar 16, 2018
@abetomo
Copy link
Collaborator

abetomo commented Mar 16, 2018

I am looking forward to PR.

rrthomas added a commit to rrthomas/commander.js that referenced this issue Mar 16, 2018
Move the setting of executables to true from the command method’s handling
of its undocumented desc parameter to the description method.
@rrthomas
Copy link
Contributor Author

(Note: my original comment had one incorrect detail: the setting of executables is in the command method, not the parse method.)

rrthomas added a commit to rrthomas/commander.js that referenced this issue Mar 16, 2018
Move the setting of executables to true from the command method’s handling
of its undocumented desc parameter to the description method.
@rrthomas
Copy link
Contributor Author

After finding a test failure and studying the code some more, I see that actually the problem is with my reading of the current documentation. Closing.

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