Repository navigation
fix: inherit HideHelpCommand in subcommands - #2395
Conversation
dearchap
left a comment
There was a problem hiding this comment.
Reviewed the change. Looks correct — the new test fails without the fix and passes with it, and test suite/vet/gofmt are clean. Two notes below (one behavior change to confirm, one minor doc touch-up).
| if !cmd.hideHelp() { | ||
| if cmd.Command(helpCommand.Name) == nil { | ||
| if !cmd.HideHelpCommand { | ||
| if !cmd.hideHelpCommand() { |
There was a problem hiding this comment.
Heads-up: this is a behavior change for nested trees, not just a bug fix. hideHelpCommand() walks the whole ancestor chain, so once any ancestor sets HideHelpCommand: true, a descendant cannot re-enable the built-in help command — even by explicitly setting HideHelpCommand: false on itself. Before this change, app config help still worked when only the root hid the command, because each command's own field value controlled the append. If that was intentional (matching HideHelp inheritance semantics), consider noting it explicitly in the release notes.
| return false | ||
| } | ||
|
|
||
| func (cmd *Command) hideHelpCommand() bool { |
There was a problem hiding this comment.
Minor: with this change HideHelpCommand now propagates to all descendants, but its doc comment in command.go still reads // Ignored if HideHelp is true. only. A short note that the value applies to this command and its subcommands would prevent confusion about why a nested command lost its help command.
The field now behaves like HideHelp: a true value applies to the whole subtree and a subcommand cannot turn it back off. Say so on the field and pin both halves of that behaviour with tests, so the shared semantics are not lost again.
|
Yes, that's intentional. HideHelp already works this way, it walks up the parent chain, so a subcommand setting HideHelp: false under a hidden root doesn't get help back either. This just makes HideHelpCommand behave the same as HideHelp. I added two tests for it so it doesn't get flipped back by accident later. Doc comment updated as you asked. I also had to update godoc-current.txt and testdata/godoc-v3.x.txt, since CI compares those against the source. For the release note, maybe something like: HideHelpCommand is now inherited by subcommands, same as HideHelp. If a command sets it, the help command is hidden for that command and everything under it. |
dearchap
left a comment
There was a problem hiding this comment.
LGTM. The fix is correct: hideHelpCommand() mirrors hideHelp() by walking the parent chain, and the parent pointers are guaranteed to be set before ensureHelp() runs for any subcommand (setupDefaults sets direct children parents before ensureHelp; setupCommandGraph uses a pre-order Walk). Verified locally: full suite + race pass, vet/gofmt clean, generated godoc matches the updated godoc-current.txt and testdata/godoc-v3.x.txt.
This PR contains the following updates: | Package | Type | Update | Change | OpenSSF | |---|---|---|---|---| | [github.com/urfave/cli/v3](https://github.com/urfave/cli) | require | minor | `v3.10.1` → `v3.14.0` | [](https://securityscorecards.dev/viewer/?uri=github.com/urfave/cli) | --- >⚠️ **Warning** > > Some dependencies could not be looked up. Check the [Dependency Dashboard](issues/23) for more information. --- ### Release Notes <details> <summary>urfave/cli (github.com/urfave/cli/v3)</summary> ### [`v3.14.0`](https://github.com/urfave/cli/releases/tag/v3.14.0) [Compare Source](urfave/cli@v3.13.0...v3.14.0) #### What's Changed - chore(deps): bump mkdocs-git-revision-date-localized-plugin from 1.5.4 to 1.6.0 in the python-packages group by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2436](urfave/cli#2436) - fix: set BoolWithInverseFlag in a short option group by [@​lazerg](https://github.com/lazerg) in [#​2440](urfave/cli#2440) - Keep arguments after a bare dash by [@​official-burak](https://github.com/official-burak) in [#​2419](urfave/cli#2419) - fix: handle a multi-byte flag at the end of a short option group by [@​SulimanAbdulrazzaq](https://github.com/SulimanAbdulrazzaq) in [#​2438](urfave/cli#2438) - fix: complete persistent flags in shell completion by [@​sb123sb123](https://github.com/sb123sb123) in [#​2437](urfave/cli#2437) - fix: restore Unwrap on the error returned by Exit by [@​fadiroot](https://github.com/fadiroot) in [#​2444](urfave/cli#2444) - fix: reject a parent's Local flag inside a short option group by [@​fadiroot](https://github.com/fadiroot) in [#​2445](urfave/cli#2445) - fix: report a flag that needs an argument but is not last in a short group by [@​yu2971512385-ui](https://github.com/yu2971512385-ui) in [#​2443](urfave/cli#2443) - Use prefixFor so flag suggestions agree with help on rune count by [@​januththedev](https://github.com/januththedev) in [#​2448](urfave/cli#2448) - fix: reject duplicate flag names by [@​deepakganesh78](https://github.com/deepakganesh78) in [#​2396](urfave/cli#2396) - fix: date a time-only timestamp today when Timezone is not UTC by [@​breken-ai](https://github.com/breken-ai) in [#​2450](urfave/cli#2450) - fix: let a subcommand's command-line value replace a persistent flag's env/file value by [@​breken-ai](https://github.com/breken-ai) in [#​2449](urfave/cli#2449) - Add urfave_cli_no_template build tag by [@​kolyshkin](https://github.com/kolyshkin) in [#​2441](urfave/cli#2441) - fix: skip empty source values for flags that cannot parse them by [@​abo3losh1](https://github.com/abo3losh1) in [#​2451](urfave/cli#2451) #### New Contributors - [@​lazerg](https://github.com/lazerg) made their first contribution in [#​2440](urfave/cli#2440) - [@​official-burak](https://github.com/official-burak) made their first contribution in [#​2419](urfave/cli#2419) - [@​SulimanAbdulrazzaq](https://github.com/SulimanAbdulrazzaq) made their first contribution in [#​2438](urfave/cli#2438) - [@​sb123sb123](https://github.com/sb123sb123) made their first contribution in [#​2437](urfave/cli#2437) - [@​fadiroot](https://github.com/fadiroot) made their first contribution in [#​2444](urfave/cli#2444) - [@​yu2971512385-ui](https://github.com/yu2971512385-ui) made their first contribution in [#​2443](urfave/cli#2443) - [@​januththedev](https://github.com/januththedev) made their first contribution in [#​2448](urfave/cli#2448) - [@​deepakganesh78](https://github.com/deepakganesh78) made their first contribution in [#​2396](urfave/cli#2396) - [@​breken-ai](https://github.com/breken-ai) made their first contribution in [#​2450](urfave/cli#2450) - [@​abo3losh1](https://github.com/abo3losh1) made their first contribution in [#​2451](urfave/cli#2451) **Full Changelog**: <urfave/cli@v3.13.0...v3.14.0> ### [`v3.13.0`](https://github.com/urfave/cli/releases/tag/v3.13.0) [Compare Source](urfave/cli@v3.12.0...v3.13.0) #### What's Changed - Trim the key of a map flag when TrimSpace is set by [@​youdie006](https://github.com/youdie006) in [#​2431](urfave/cli#2431) - Fix inverse bool flag counting with an external counter by [@​jakezwang](https://github.com/jakezwang) in [#​2432](urfave/cli#2432) - feat: add Deprecated field to flags and commands by [@​anandghegde](https://github.com/anandghegde) in [#​2429](urfave/cli#2429) - fix: parse single-rune Unicode flags by [@​znnnnnnn-wil](https://github.com/znnnnnnn-wil) in [#​2434](urfave/cli#2434) #### New Contributors - [@​jakezwang](https://github.com/jakezwang) made their first contribution in [#​2432](urfave/cli#2432) - [@​anandghegde](https://github.com/anandghegde) made their first contribution in [#​2429](urfave/cli#2429) - [@​znnnnnnn-wil](https://github.com/znnnnnnn-wil) made their first contribution in [#​2434](urfave/cli#2434) **Full Changelog**: <urfave/cli@v3.12.0...v3.13.0> ### [`v3.12.0`](https://github.com/urfave/cli/releases/tag/v3.12.0) [Compare Source](urfave/cli@v3.11.0...v3.12.0) #### What's Changed - feat: add Command.ArgValidator for tree-wide argument validation by [@​dearchap](https://github.com/dearchap) in [#​2377](urfave/cli#2377) - Single-value arguments can be marked as required by [@​idelchi](https://github.com/idelchi) in [#​2393](urfave/cli#2393) - docs: clarify that named Arguments are excluded from cmd.Args() by [@​dearchap](https://github.com/dearchap) in [#​2411](urfave/cli#2411) - fix: never run command action on shell completion past a double dash by [@​dearchap](https://github.com/dearchap) in [#​2412](urfave/cli#2412) - chore(deps): bump github.com/stretchr/testify from 1.11.1 to 1.12.0 by [@​app/dependabot](https://github.com/app/dependabot) in [#​2413](urfave/cli#2413) - Support custom FlagStringer for MutuallyExclusiveFlags by [@​SHIVANSHGARG07](https://github.com/SHIVANSHGARG07) in [#​2414](urfave/cli#2414) - Add TextFlag for encoding.TextMarshaler/TextUnmarshaler values by [@​ChrisJr404](https://github.com/ChrisJr404) in [#​2415](urfave/cli#2415) - chore(deps): bump github.com/stretchr/testify from 1.12.0 to 1.12.1 by [@​app/dependabot](https://github.com/app/dependabot) in [#​2416](urfave/cli#2416) - chore(deps): bump the python-packages group with 2 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​2417](urfave/cli#2417) - Show inherited persistent flags in child help by [@​justadityaraj](https://github.com/justadityaraj) in [#​2421](urfave/cli#2421) - fix: preserve whitespace in positional arguments by [@​lenamonj](https://github.com/lenamonj) in [#​2423](urfave/cli#2423) - fix: preserve whitespace in equals-form flag values by [@​BenYang12](https://github.com/BenYang12) in [#​2425](urfave/cli#2425) - fix: complete subcommand flags after positional arguments by [@​fzlzjerry](https://github.com/fzlzjerry) in [#​2426](urfave/cli#2426) - Fix a year-less timestamp layout losing January 1 by [@​youdie006](https://github.com/youdie006) in [#​2427](urfave/cli#2427) - docs: add go-size-analyzer, module version, and reflection by [@​imsumedhaa](https://github.com/imsumedhaa) in [#​2428](urfave/cli#2428) #### New Contributors - [@​idelchi](https://github.com/idelchi) made their first contribution in [#​2393](urfave/cli#2393) - [@​SHIVANSHGARG07](https://github.com/SHIVANSHGARG07) made their first contribution in [#​2414](urfave/cli#2414) - [@​ChrisJr404](https://github.com/ChrisJr404) made their first contribution in [#​2415](urfave/cli#2415) - [@​justadityaraj](https://github.com/justadityaraj) made their first contribution in [#​2421](urfave/cli#2421) - [@​lenamonj](https://github.com/lenamonj) made their first contribution in [#​2423](urfave/cli#2423) - [@​BenYang12](https://github.com/BenYang12) made their first contribution in [#​2425](urfave/cli#2425) - [@​youdie006](https://github.com/youdie006) made their first contribution in [#​2427](urfave/cli#2427) - [@​imsumedhaa](https://github.com/imsumedhaa) made their first contribution in [#​2428](urfave/cli#2428) **Full Changelog**: <urfave/cli@v3.11.0...v3.12.0> ### [`v3.11.0`](https://github.com/urfave/cli/releases/tag/v3.11.0) [Compare Source](urfave/cli@v3.10.1...v3.11.0) #### What's Changed - fix: correct a typo in the help message for the powershell autocompletion by [@​suzuki-shunsuke](https://github.com/suzuki-shunsuke) in [#​2383](urfave/cli#2383) - Escape backslashes in fish completion descriptions by [@​mahirhir](https://github.com/mahirhir) in [#​2378](urfave/cli#2378) - chore(deps): bump actions/setup-go from 6 to 7 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2389](urfave/cli#2389) - chore(deps): bump mkdocs-material from 9.7.6 to 9.7.7 in the python-packages group by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2391](urfave/cli#2391) - chore(deps): bump actions/setup-python from 6 to 7 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2390](urfave/cli#2390) - fix: skip all hidden flags in shell completion, not just bool flags by [@​vidigoat](https://github.com/vidigoat) in [#​2388](urfave/cli#2388) - Fix nil pointer dereference for interface-typed flag values by [@​mrueg](https://github.com/mrueg) in [#​2400](urfave/cli#2400) - Fix index out of range panic when Run is given no arguments by [@​mrueg](https://github.com/mrueg) in [#​2401](urfave/cli#2401) - fix: honor BoolWithInverseFlag DefaultText by [@​fzlzjerry](https://github.com/fzlzjerry) in [#​2403](urfave/cli#2403) - Fix Issue [#​2385](urfave/cli#2385) by [@​BrandonIrizarry](https://github.com/BrandonIrizarry) in [#​2386](urfave/cli#2386) - fix: inherit HideHelpCommand in subcommands by [@​utkarshalpha](https://github.com/utkarshalpha) in [#​2395](urfave/cli#2395) - test: cover Run-with-no-arguments paths ([#​2406](urfave/cli#2406)) by [@​dearchap](https://github.com/dearchap) in [#​2407](urfave/cli#2407) - test: add shell-completion case for hidden BoolWithInverseFlag by [@​dearchap](https://github.com/dearchap) in [#​2408](urfave/cli#2408) - test: fix ineffective mutex error-string assertion and add mirror cases by [@​dearchap](https://github.com/dearchap) in [#​2410](urfave/cli#2410) #### New Contributors - [@​mahirhir](https://github.com/mahirhir) made their first contribution in [#​2378](urfave/cli#2378) - [@​vidigoat](https://github.com/vidigoat) made their first contribution in [#​2388](urfave/cli#2388) - [@​fzlzjerry](https://github.com/fzlzjerry) made their first contribution in [#​2403](urfave/cli#2403) - [@​BrandonIrizarry](https://github.com/BrandonIrizarry) made their first contribution in [#​2386](urfave/cli#2386) - [@​utkarshalpha](https://github.com/utkarshalpha) made their first contribution in [#​2395](urfave/cli#2395) **Full Changelog**: <urfave/cli@v3.10.1...v3.11.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At 12:00 AM through 04:59 AM and 10:00 PM through 11:59 PM, Monday through Friday (`* 0-4,22-23 * * 1-5`) - Only on Sunday and Saturday (`* * * * 0,6`) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4xNTEuMiIsInVwZGF0ZWRJblZlciI6IjQ0LjE1MS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJLaW5kL0RlcGVuZGVuY2llcyJdfQ==--> Reviewed-on: https://altlinux.space/stapler/stplr/pulls/503
What type of PR is this?
What this PR does / why we need it:
HideHelpCommandinherit through the command hierarchy, matching the existing behavior ofHideHelp.helpcommand when an ancestor disabled it.--helpremains available.Which issue(s) this PR fixes:
Fixes #2129
Testing
go test ./... -count=1go run scripts/build.go generatego run scripts/build.go vetgo run scripts/build.go check-binary-sizego run scripts/build.go gfmrunRelease Notes