-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Hide help flag from help output #645
Conversation
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Codecov Report
@@ Coverage Diff @@
## master #645 +/- ##
==========================================
- Coverage 49.4% 49.39% -0.01%
==========================================
Files 208 208
Lines 17194 17195 +1
==========================================
Hits 8494 8494
- Misses 8267 8268 +1
Partials 433 433 |
@@ -26,6 +26,7 @@ func SetupRootCommand(rootCmd *cobra.Command) { | |||
|
|||
rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage") | |||
rootCmd.PersistentFlags().MarkShorthandDeprecated("help", "please use --help") | |||
rootCmd.PersistentFlags().Lookup("help").Hidden = true |
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.
hm, didn't the .golden
file need to be updated? https://github.com/docker/cli/pull/642/files#r147009761
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.
I expected it would, but it doesn't because of how the flags are applied.
This help flag is applied as a persistent flag to the root command, so when commands are added to the root, they inherit this version of the flag, and don't add the default.
In the test we are testing directly against the swarm update
command, so the root command is not there to provide this flag. Cobra adds its own default help flag that is not hidden, so the test output still contains the --help
flag.
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 🐮
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, thanks for explaining!
Follow up to #642