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

Feature discussion: Inherit options in subcommands #1753

Closed
arve0 opened this issue Jun 14, 2022 · 14 comments
Closed

Feature discussion: Inherit options in subcommands #1753

arve0 opened this issue Jun 14, 2022 · 14 comments

Comments

@arve0
Copy link

arve0 commented Jun 14, 2022

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
Copy link
Collaborator

shadowspawn commented Jun 14, 2022

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@arve0
Copy link
Author

arve0 commented Jun 16, 2022

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
Copy link
Collaborator

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
Copy link
Author

arve0 commented Jun 17, 2022

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
Copy link
Author

arve0 commented Jun 17, 2022

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
Copy link
Author

arve0 commented Jun 17, 2022

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
Copy link
Collaborator

shadowspawn commented Jun 17, 2022

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link

jcalfee commented Sep 15, 2022

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
Copy link
Collaborator

@jcalfee
Copy link

jcalfee commented May 22, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants