-
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
Add HideHelpCommand #1083
Add HideHelpCommand #1083
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1083 +/- ##
==========================================
+ Coverage 72.91% 73.18% +0.27%
==========================================
Files 33 33
Lines 2481 2480 -1
==========================================
+ Hits 1809 1815 +6
+ Misses 565 559 -6
+ Partials 107 106 -1
Continue to review full report at Codecov.
|
2426f97
to
3e5678c
Compare
Any chance to get v2.2.0 released soon with this? |
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.
Looks good, thanks for pulling this together.
I think the only thing left is getting the coverage check passing.
4604362
to
2129edf
Compare
While `HideHelp` hides both `help` command and `--help` flag, `HideHelpCommand` only hides `help` command and leave `--help` flag as-is. The behavior of `HideHelp` is untouched in this commit. Fix urfave#523 Replace urfave#636 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2129edf
to
75e7c52
Compare
// append help to commands | ||
if len(a.Commands) > 0 { | ||
if a.Command(helpCommand.Name) == nil && !a.HideHelp { | ||
a.appendCommand(helpCommand) | ||
|
||
if HelpFlag != nil { | ||
a.appendFlag(HelpFlag) | ||
} | ||
} | ||
} |
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.
❓ (blocking question) this code is being removed because it was duplicated in Setup
above, yeah?
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.
Yes, and because of the duplication, this code path was not executed and could not be covered with tests: #1083 (comment)
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.
Nice find!
I can write up the release notes this weekend probs |
What type of PR is this?
What this PR does / why we need it:
While
HideHelp
hides bothhelp
command and--help
flag,HideHelpCommand
only hides
help
command and leave--help
flag as-is.The behavior of
HideHelp
is untouched in this commit.Which issue(s) this PR fixes:
Fixes #523
Replaces #636
Testing
go test -v -run TestHideHelpCommand
Release Notes