Expose Command.options property

[mshima]

Pull Request

Problem

I could not find a reason for options not to be exposes as this PR proposes.
Currently there is no way to introspect a command, options property is not exposed. That can be useful to build manuals, readme, and so one. I am targeting to build a GitHub Action definition https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions.
Options handlers, events and maybe others are parsed/settled at addOption. Once added, changing it can have unexpected results.

Solution

Add a readonly field with readonly type.

ChangeLog

Add options to Command type.

[shadowspawn]

I am not against exposing the options, but for interest this is another way of inspecting the options which will sometimes be more useful.

There is a method on the Help object:

  /** Get an array of the visible options. Includes a placeholder for the implicit help option, if there is one. */
  visibleOptions(cmd: Command): Option[];

This can be used like:

const visibleOptions = cmd.createHelp().visibleOptions(cmd));

This is used when generating the help and is public to allow other uses too.

[shadowspawn]

The readonly is a reasonable suggestion, but the options and commands properties are very similar. I would prefer to keep them symmetrical, and not make the options readonly.

[shadowspawn]

For interest, the PR that added .commands also suggested readonly! Although possibly at the time it was shallow. #1184

[mshima]

@shadowspawn thanks for the quick response.

const visibleOptions = cmd.createHelp().visibleOptions(cmd));

It's not obvious, but works for me.

How commander works is somewhat not intuitive.
This doesn't works:

const option = new Option(...);
command.addOption(option);
option.env('ENV_VAR');

So the readonly should prevent errors like:

command.options.forEach(option => (option.envVar = option.name().toUpperCase()));

Won't prevent errors like:

command.options.forEach(option => option.envVar.env(option.name().toUpperCase()));

For interest, the PR that added .commands also suggested readonly! Although possibly at the time it was shallow. #1184

Command is less restrictive that option for this kind of use cases.

[shadowspawn]

If you still want to add options, I would like it after commands (one line higher) and without the readonly.

[shadowspawn]

Options handlers, events and maybe others are parsed/settled at addOption. Once added, changing it can have unexpected results.

This is very true. A lot gets done when a command or an option is added.

[shadowspawn]

LGTM

[shadowspawn]

Adding a property or method is normally considered a non-breaking change. However, this is a property that some people are likely to have added themselves. I checked for some possible breakages and the ReadonlyArray initially proposed in this PR would break this particular client code. It seems like narrowing the type could be ok, but not this particular one.

playground

The addition of the commands property to TypeScript happened to land in a major version.

I think for safety, this should land in a major version too? (Being very paranoid!)

[shadowspawn]

Tried A & B extension and found a change in resulting type too.

playground

[shadowspawn]

I am starting to think this might not be worth adding for now:

• hasn't been asked for before
• alternative (and slightly safer) approach is use Help.visibleOptions() and can suggest that when people ask
• some chance people will get themselves into trouble, like pointed out in original post:

Options handlers, events and maybe others are parsed/settled at addOption. Once added, changing it can have unexpected results.

Do you have a preference @abetomo ?

[shadowspawn]

Closing as not a clear enough win for now.

Thanks for the suggestion @mshima