diff --git a/command_setup.go b/command_setup.go index cac4a30314..c187f1d1cd 100644 --- a/command_setup.go +++ b/command_setup.go @@ -84,6 +84,14 @@ func (cmd *Command) setupDefaults(osArgs []string) { var localVersionFlag Flag if globalVersionFlag, ok := VersionFlag.(*BoolFlag); ok { flag := *globalVersionFlag + // A user-defined flag may already use the short + // name the default version flag wants. `-v` for + // `--verbose` is the canonical collision: the + // version flag's `v` alias used to silently + // override the user's `--verbose`. Drop any alias + // that's already taken so the user flag wins. See + // #2229. + flag.Aliases = dropClashingAliases(flag.Aliases, cmd.Flags, flag.Name) localVersionFlag = &flag } else { localVersionFlag = VersionFlag @@ -221,3 +229,31 @@ func (cmd *Command) ensureHelp() { } } } + +// dropClashingAliases removes aliases from `aliases` that are already +// claimed by a flag in `userFlags` (either as a primary name or as one +// of its own aliases). Aliases equal to `selfName` are kept so the +// flag's primary name doesn't accidentally remove itself. +func dropClashingAliases(aliases []string, userFlags []Flag, selfName string) []string { + if len(aliases) == 0 || len(userFlags) == 0 { + return aliases + } + taken := map[string]struct{}{} + for _, f := range userFlags { + for _, n := range f.Names() { + taken[n] = struct{}{} + } + } + kept := aliases[:0:0] + for _, a := range aliases { + if a == selfName { + kept = append(kept, a) + continue + } + if _, ok := taken[a]; ok { + continue + } + kept = append(kept, a) + } + return kept +} diff --git a/command_test.go b/command_test.go index 9d77c12f23..e518e5d040 100644 --- a/command_test.go +++ b/command_test.go @@ -2674,6 +2674,28 @@ func TestCustomFlagsUsed(t *testing.T) { assert.NoError(t, err, "Run returned unexpected error") } +// Regression for #2229. A user-defined --verbose/-v flag should take +// precedence over the version flag's default -v alias instead of being +// silently shadowed by it. +func TestVersionFlagYieldsAliasToUserFlag(t *testing.T) { + var seen bool + cmd := &Command{ + Name: "demo", + Version: "1.0.0", + Flags: []Flag{ + &BoolFlag{Name: "verbose", Aliases: []string{"v"}}, + }, + Writer: io.Discard, + Action: func(_ context.Context, c *Command) error { + seen = c.Bool("verbose") + return nil + }, + } + err := cmd.Run(buildTestContext(t), []string{"demo", "-v"}) + assert.NoError(t, err) + assert.True(t, seen, "expected --verbose to be set when -v is passed") +} + func TestCustomHelpVersionFlags(t *testing.T) { cmd := &Command{ Writer: io.Discard, diff --git a/help.go b/help.go index 1fba6edf0c..b7e11a5c7b 100644 --- a/help.go +++ b/help.go @@ -459,13 +459,20 @@ func DefaultPrintHelp(out io.Writer, templ string, data any) { } func checkVersion(cmd *Command) bool { - found := false - for _, name := range VersionFlag.Names() { - if cmd.Bool(name) { - found = true + // Look up the actual version flag attached to this command (which + // may have had clashing aliases stripped during setup) rather than + // going through cmd.Bool by name. cmd.Bool resolves through + // aliases, so when a user-defined flag claims one of the default + // version flag's aliases (commonly -v for --verbose), Bool would + // return the user flag's value and trigger the version path. See + // #2229. + primary := VersionFlag.Names()[0] + for _, f := range cmd.appliedFlags { + if names := f.Names(); len(names) > 0 && names[0] == primary { + return f.IsSet() } } - return found + return false } func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) {