-
-
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
[PR with tiny fixes] Fix help command for subcommands with an executable handler and only a short help flag. Do not use undefined long help option flag in legacy code. Change initial variable values in test for better error messages. #1930
Conversation
(cherry picked from commit 87db4ba)
version()
parameter type and change initial variable values in test for better error messagesversion()
parameter type. Do not use undefined long help option flag in legacy code. Change initial variable values in test for better error messages.
version()
parameter type. Do not use undefined long help option flag in legacy code. Change initial variable values in test for better error messages. version()
parameter type. Do not use undefined long help option flag in legacy code. Change initial variable values in test for better error messages.
The Note for myselfOne. Change. Per. PR. Always. (no matter how tiny it is) |
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.
Couple of good fixes for the reasonably recent possibility the help option might only have short flag.
Changes noted in separate comments.
This reverts commit e8bea4a.
version()
parameter type. Do not use undefined long help option flag in legacy code. Change initial variable values in test for better error messages. 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.
New changes have gone from fixing a bug with undefined (easy to approve) to refactoring code (opinionated and time consuming to review).
I want the simple fix please.
Sorry about the force pushes, there is just already enough clutter in other PRs due to this one, and I don't want to introduce even more. |
Thank you for the patience, I know this PR is a mess. |
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.
+1
Tiny fixes that don't really belong anywhere else.
ChangeLog
Changed
do not callpreSubcommand
hooks when displaying help for a subcommand with an executable handler(technically breaking, but hey, are we really going to assume someone relied on this?)deviating help flags specified for a subcommand with an executable handler via.helpOption()
are now respected by the parent's help command (passed to the subcommand instead of the parent's help flag)Fixed
.version()
optional parameter type