-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Have help command call help directly for subcommands, when possible #1864
Have help command call help directly for subcommands, when possible #1864
Conversation
if (!subcommandName) { | ||
this.help(); | ||
} | ||
const subCommand = this._findCommand(subcommandName); |
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 subcommandName
is undefined
, this._findCommand(subcommandName)
also returns undefined
.
How about the following code?
if (!subcommandName) {
this.help();
} else {
const subCommand = this._findCommand(subcommandName);
...
}
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 help method calls exit, so the following code will not be reached if subcommandName
is undefined
.
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.
That makes sense.
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.
👍
Pull Request
Problem
The implementation of the help command like
my-util help foo
is a bit fragile. The implementation invokes the subcommand passingthis._helpLongFlag
. This can fail if:See: #1863 (help option disabled)
Solution
Call
.help()
directly for a known (non-external) subcommand. Just use the help flags as a fallback for external subcommands.This takes a little extra code, but I have never been happy about passing through the help flags so I think it is worth it.
ChangeLog