Transfer settings in addCommand

[vonagam]

Pull Request

Problem

Currently there is no way to transfer private settings (related to help, where options are stored, exit callback and so on) to a command added through addCommand. (Connected to discussion in #1185.)

Solution

Transfer settings in addCommand. Can add an option to CommandOptions (inheritSettings or something) to conditionally transfer this settings with it being true by default in command() and false in direct call to addCommand.

[shadowspawn]

This is not the model I have in mind. The original motivation for .addCommand() came from discussions in #764

The intent is to allow adding subcommands that are defined and configured separately, and decouple the program and the subcommand.

Two key problems with transferring all the private settings in addCommand are:

• overwrites the custom settings which may have been made in the subcommand, the subcommand is no longer in control of its own behaviour
• a change in some program settings will cause a change in the runtime the subcommand will experience due to the settings (e.g. command or opts passed to action handler), implying code changes in program require changes in all subcommands despite them potentially being declared and implemented in separate modules

Note also: .addCommand can be used to add a command which has its own nested subcommands. It isn't just one level of settings in the general case.

(I am starting to see .addCommand() is less complete than I optimistically assumed, both in terms of implementation and documentation... My main focus at the time was nested commands. Educational!)

[vonagam]

Yes, i thought about maybe checking if a setting is present and non-default before overriding it like this:

cmd._exitCallback = cmd._exitCallback !== null ? cmd._exitCallback : this._exitCallback;

Easy to do for nullable settings, not so much for others.

About nested subcommands, you can forbid using inheritSettings on a command with subcommands already present.

But i'm not invested in this PR in any way, this is just to explore and think about alternative solutions to the original issue.

[shadowspawn]

My planned starting point is no inheritance of settings from parent command to child subcommand using .addCommand() because the child owns most of the settings, and make no assumptions about how common or independent the code is between them. If in practice people are using custom help especially and want Commander to look after making things consistent , they may ask for built-in ways to propagate or copy (a subset of) settings.

The two exceptions are default command, and hiding help.

Considering the properties, as listed in other PR:

_executableHandler // is only set in command()
_executableFile // is only set in command()

I didn't see these as being relevant to .addCommand() usage in practice, but actually people may want to leave the decision up to the child whether it is implemented using action handler or executable handler. That would require additions to Command rather than to .addCommand() to allow creating an executable command in a public way.

_defaultCommandName // is only set in command()

This one should be supported as option when added, like with .command(). It is a parent setting not a child setting.

_noHelp // is only set in command()

Who decides whether the help is displayed, the parent or the child? My first thought is perhaps allow either to hide the help! So supported as option when added, like with .command().

_helpFlags // has only setter helpOption()
_helpDescription // has only setter helpOption()
_helpShortFlag // has only setter helpOption()
_helpLongFlag // has only setter helpOption()
_helpCommandName // has only setter addHelpCommand()
_helpCommandnameAndArgs // has only setter addHelpCommand()
_helpCommandDescription // has only setter addHelpCommand()

I think these are owned by the child although in practice the author will want them consistent. If the author wants to configure then in a non-standard way they are able to call helpOption and addHelpCommand in child or in parent, and possibly pass around or apply their preferred default settings outside Commander.

_exitCallback // has only setter exitOverride()

Owned by the child, but might be useful to inherit sometimes, not least because it is useful when writing tests. However, starting out with no inheritance. Like with help, author can configure themselves.

_storeOptionsAsProperties // has only setter storeOptionsAsProperties()
_passCommandToAction // has only setter passCommandToAction()

These two are a pair and the second one changes the code in the action handler. Definitely owned by child, and author can choose to configure themselves.

[vonagam]

they may ask for built-in ways to propagate or copy (a subset of) settings.

Added inheritHelp, inheritExit options. How about this iteration?

_defaultCommandName. This one should be supported as option when added...

Agree. By the way, why can't you set or get default command name outside of adding new command though (like defaultCommand() getter/setter)?

_noHelp. perhaps allow either to hide the help!..

Agree. In what way a command will be able to set _noHelp by itself?

[vonagam]

Also can add private boolean fields _inheritedHelp and _inheritedExit to achieve this behaviour:

const nested = new Command();
const sub = new Command();
const top = new Command();

sub.addCommand(nested, { inheritHelp: true }); // nested wants to keep its exit
top.addCommand(sub, { inheritHelp: true, inheritExit: true }); // sub doesn't care

// nested command will receive help from top command, but not exit

[shadowspawn]

I am going to keep .addCommand() simple to start with, as per #1186 (comment)

Thank you for your contributions.