Feature discussion: Inherit options in subcommands

[arve0]

Hi! Thanks for commander 🙌
Is commander considered feature complete? Any chance for a inherit options feature? Or always populating options in subcommands when same is also defined in parent commands?

I find it hard to create a hierarchy of commands that share common options. I also find the "options are global behavior" unintuitive. Especially when you are unaware of any parent command using the same option.

I've read issues like #1750, #1739, #1616 and #1229.

Take this example, where print and print pdf would have very different action-implementations:

#!/usr/bin/env node
const { Command } = require('commander');

class MyRootCommand extends Command {
  createCommand(name) {
    const cmd = new Command(name);
    cmd.option('-v, --verbose', 'use verbose logging');
    return cmd;
  }
}

const program = new MyRootCommand();

const print = program.command('print')
  .option('--a4', 'Use A4 sized paper')
  .action((options) => {
    console.log('print options: %O', options);
  });

print.command('pdf')
  .option('--a4', 'Use A4 sized paper')
  .action((options) => {
    console.log('print pdf options: %O', options);
  });

program.parse();

The help suggests that --a4 is available to both "print" and "print pdf":

> ./common-options.js print --help
Usage: common-options print [options] [command]

Options:
  -v, --verbose  use verbose logging
  --a4           Use A4 sized paper
  -h, --help     display help for command

Commands:
  pdf [options]

> ./common-options.js print pdf --help
Usage: common-options print pdf [options]

Options:
  --a4        Use A4 sized paper
  -h, --help  display help for command

Though, it is not:

> ./common-options.js print --a4
print options: { a4: true }

> ./common-options.js print pdf --a4
print pdf options: {}

Solvable through optsWithGlobals or splitting into print-printer and print-pdf commands. Maybe also with splitting print pdf to separate executable?

Anyhow, would be nice if options was set in subcommands when same options are defined longer up in the hierarchy.

[shadowspawn]

Thanks for the detailed description.

The feature which is a good match for most of your description is .enablePositionalOptions(). This turns off the "global" behaviour where the option can appear before or after a subcommand.

I also find the "options are global behavior" unintuitive. Especially when you are unaware of any parent command using the same option.

With positional options, options after the subcommand belong to the subcommand.

Take this example, where print and print pdf would have very different action-implementations

With positional options, the print --a4 sets the print option and print pdf --a4 sets the pdf option.

[shadowspawn]

Is commander considered feature complete? Any chance for a inherit options feature? Or always populating options in subcommands when same is also defined in parent commands?

Have a look at #1551 for some previous coverage of enhancements to global options. I have had a go at the two most popular: one turned into an example file instead, and .optsWithGlobals() came out of the other.

"always populating" is similar to #1551 (comment), and was the least popular (somewhat to my surprise at the time).

[shadowspawn]

(.enablePositionalOptions() came out of #1229 which you mentioned, so you may have already looked at it.)

[arve0]

Thanks for your swift reply! 🙂

The feature which is a good match for most of your description is .enablePositionalOptions(). This turns off the "global" behaviour where the option can appear before or after a subcommand.

I missed that, thanks for the pointer.

With positional options, the print --a4 sets the print option and print pdf --a4 sets the pdf option.

Out of curiosity, that is useful when running both actions?

"always populating" is similar to #1551 (comment), and was the least popular (somewhat to my surprise at the time).

I'm surprised too.

Do you want to keep it the way it is? What about throwing when defining the same option twice?

[shadowspawn]

Out of curiosity, that is useful when running both actions?

Yes, the command-specific option is passed into the action handler (if that is what you are asking).

Do you want to keep it the way it is?

I don't think "inherit" is a common or clean enough use case to try and add explicit support. But willing to discuss it in more detail if you wish.

What about throwing when defining the same option twice?

A major consideration for throwing is it would completely break any existing programs that happen to have overlapping options across levels, and are currently working fine apart from the overlapping option in the subcommand. Commander has a big installed base, so adding throws with helpful guidance to authors of new code has to be balanced against breaking existing code. It could be considered in a major release.

Can the guidance in a throw cover enough of the likely cases, not just that there is a problem? The two scenarios that spring to mind are:

• program with a subcommands and a default behaviour with own options, where a solution is move default behaviour into a subcommand and use isDefault
• overlapping flags at different levels, where a solution for some cases is .enablePositionalOptions())

[arve0]

Yes, the command-specific option is passed into the action handler (if that is what you are asking).

But that action handler never runs? Am I missing something? Take this example:

const commonOption = createOption("-d, --debug");

const program = new Command()
  .addOption(commonOption)
  .action((options) => {
    console.log("program options: %O", options)
  })
  .enablePositionalOptions();

const print = program.command('print')
  .addOption(commonOption)
  .action((options) => {
    console.log('print options: %O', options);
  });

print.command('pdf')
  .addOption(commonOption)
  .action((options) => {
    console.log('print pdf options: %O', options);
  });

program.parse();

Running yields:

❯ ./option.js -d print -d pdf -d
print pdf options: { debug: true }

Adding tests in global scope works:

if (program.opts().debug) {
  console.log("program debug set")
}

if (print.opts().debug) {
  console.log("print debug set")
}

❯ ./option.js -d print -d pdf -d
print pdf options: { debug: true }
program debug set
print debug set

That is how it's intended?

A major consideration for throwing is it would completely break any existing programs that happen to have overlapping options across levels, and are currently working fine apart from the overlapping option in the subcommand. Commander has a big installed base, so adding throws with helpful guidance to authors of new code has to be balanced against breaking existing code. It could be considered in a major release.

Fully agree!

Can the guidance in a throw cover enough of the likely cases, not just that there is a problem?

Sounds reasonable to try covering the likely cases 👍 The second one is probably common error (as multiple issues state).

program with a subcommands and a default behaviour with own options, where a solution is move default behaviour into a subcommand and use isDefault

I do not understand this sentence, but am eager to learn the use case. Can you provide an example?

[arve0]

Btw, an alternative to throwing would be not showing option in help, when it isn't considered an "regular" option to that nested subcommand.

Example, no --a4 here:

❯ ./option.js print pdf --help
Usage: option print pdf [options]

Options:
  -h, --help   display help for command

[arve0]

Can you provide an example?

Aha, this is the "make sure commands using the same options never follow a chain command -> subcommand". Like this?

const program = new Command()
  .addOption(commonOption)
  .action((options) => {
    console.log("program options: %O", options)
  });

program.command('print')
  .addOption(commonOption)
  .action((options) => {
    console.log('print options: %O', options);
  });

which should be changed into:

const program = new Command()

program.command("main", { isDefault: true })
  .addOption(commonOption)
  .action((options) => {
    console.log("program options: %O", options)
  })
  .enablePositionalOptions();

program.command('print')
  .addOption(commonOption)
  .action((options) => {
    console.log('print options: %O', options);
  });

Right?

[shadowspawn]

From #1753 (comment)

That is how it's intended?

Yes, the options for each nested command are available on the command.

A helper method for some setups is .optsWithGlobals() which merges together the options in the hierarchy.

To access the global/inherited options from within an action handler, you can access cmd.parent, but up to you to make sense of the hierarchy! For example:

print.command('pdf')
  .addOption(commonOption)
  .action((options, pdfCmdParam) => {
    console.log('print pdf options: %O', options);
    console.log('print options looking up: %O', pdfCmdParam.parent.opts());
    console.log('program options looking up: %O', pdfCmdParam.parent.parent.opts());
  });

[shadowspawn]

program with a subcommands and a default behaviour with own options, where a solution is move default behaviour into a subcommand and use isDefault

Aha, this is the "make sure commands using the same options never follow a chain command -> subcommand". Like this?
...
Right?

Yes indeed. 😄

[shadowspawn]

I think we covered some good ground with scenarios with options at multiple levels, and the existing support. I don't see any action items I want to followup for now.

Feel free to open a new issue if it comes up again, with new information and renewed interest.

Thank you for your contributions.

[jcalfee]

Maybe a bit more in the help? I can't figure out how to share options between commands and get them to show up in the help for each command. I ended up copying the options twice. Thank you!

[shadowspawn]

@jcalfee

Have a look at: https://github.com/tj/commander.js/blob/master/examples/global-options.js

[jcalfee]

Thanks, the file got refactored, and this one did the trick: https://github.com/tj/commander.js/blob/master/examples/global-options-nested.js