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

WIP: Add support for nested commands (without executables) #1129

Closed

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Dec 29, 2019

Pull Request

Problem

Commander does not support nested commands (without using git-style executables).

Issue: #1 #63 #655 #764
Related PR: #245 #1024

Solution

Rework command dispatching to allow chaining subcommands, and add routine to add a previously defined Command (particularly for adding commands defined in other modules).

This is as an experiment inspired by the recent work in #1024 and older work in #245.

@shadowspawn
Copy link
Collaborator Author

shadowspawn commented Dec 29, 2019

Adding nested commands using .command

const program = require('commander');

program
  .command('sub1')
  .option('--sub1')
  .action((cmd) => {
    console.log('sub1 called');
    console.log(`--sub1 is ${cmd.sub1}`);
  })
  .command('sub2')
  .option('--sub2')
  .action((cmd) => {
    console.log('sub2 called');
    console.log(`--sub2 is ${cmd.sub2}`);
    console.log(`--sub1 is ${cmd.parent.sub1}`);
  });

program.parse(process.argv);
$ node chained.js sub1
sub1 called
--sub1 is undefined
$ node chained.js sub1 sub2
sub2 called
--sub2 is undefined
--sub1 is undefined
$ node chained.js sub1 --sub1 sub2 --sub2
sub2 called
--sub2 is true
--sub1 is true

@shadowspawn
Copy link
Collaborator Author

shadowspawn commented Dec 29, 2019

Adding nested commands using .addCommand, to support coding pattern with externally defined commands as proposed in #1024.

const commander = require('commander');

const sub1 = new commander.Command('sub1')
  .action(() => {
    console.log('sub1 action');
  })
sub1
  .command('sub2')
  .action(() => {
    console.log('sub2 action');
  });

const program = new commander.Command();
program
  .addCommand(sub1);

program.parse(process.argv);
$ node addCommand.js sub1
sub1 action
$ node addCommand.js sub1 sub2
sub2 action

@byCedric
Copy link

Absolutely love it! ❤️ Is there any way I can help? I can be an early tester if you need, or update some documentation 😄

@shadowspawn
Copy link
Collaborator Author

Thanks @byCedric 😃

Early testing will be good. I have a couple of areas I am still working on: help message when no handler called, and possible misfires with support for root level command. I'll post comments here as make progress towards code-complete.

@shadowspawn
Copy link
Collaborator Author

I am happy this is mostly working and looking a workable approach, but not happy building on top of the historical parsing which is split across multiple routines and relies on wiring up and dispatching events. It is hard to add the final details and reason about the impact on legacy behaviours.

The known missing piece is a message for when no handler gets called when parsing stops in an intermediate command with no action handler. Reports about other current shortcomings welcome.

I have switched to working on a rewrite of the parsing, with the hopes of making wider changes and adding nested commands on top of new behaviour instead of old.

@shadowspawn shadowspawn added this to the v5.0.0 milestone Jan 5, 2020
@shadowspawn
Copy link
Collaborator Author

I am having another more ambitious go in #1149

@shadowspawn shadowspawn removed this from the v5.0.0 milestone Jan 31, 2020
@shadowspawn shadowspawn deleted the feature/nested-commands branch February 1, 2020 03:31
@shadowspawn
Copy link
Collaborator Author

shadowspawn commented Feb 1, 2020

FYI @byCedric , pre-release of 5.0.0 has been published #1163

@byCedric
Copy link

byCedric commented Feb 2, 2020

Thanks! I'll give it a spin ASAP! 🚀

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

Successfully merging this pull request may close these issues.

2 participants