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
Expose Command.options property #1784
Conversation
8a7ca87
to
cab9817
Compare
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:
This is used when generating the help and is public to allow other uses too. |
typings/index.d.ts
Outdated
@@ -277,6 +277,7 @@ export class Command { | |||
processedArgs: any[]; | |||
commands: Command[]; | |||
parent: Command | null; | |||
readonly options: ReadonlyArray<Readonly<Option>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
For interest, the PR that added |
@shadowspawn thanks for the quick response.
It's not obvious, but works for me. How commander works is somewhat not intuitive.
So the readonly should prevent errors like:
Won't prevent errors like:
Command is less restrictive that option for this kind of use cases. |
typings/index.d.ts
Outdated
@@ -277,6 +277,7 @@ export class Command { | |||
processedArgs: any[]; | |||
commands: Command[]; | |||
parent: Command | null; | |||
readonly options: Option[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you still want to add options, I would like it after commands (one line higher) and without the readonly.
This is very true. A lot gets done when a command or an option is added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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 The addition of the I think for safety, this should land in a major version too? (Being very paranoid!) |
Tried |
I am starting to think this might not be worth adding for now:
Do you have a preference @abetomo ? |
Closing as not a clear enough win for now. Thanks for the suggestion @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.