diff --git a/command_setup.go b/command_setup.go index 8cc73a02fc..646e270ce7 100644 --- a/command_setup.go +++ b/command_setup.go @@ -99,6 +99,10 @@ func (cmd *Command) setupDefaults(osArgs []string) { var localVersionFlag Flag if globalVersionFlag, ok := VersionFlag.(*BoolFlag); ok { flag := *globalVersionFlag + // Drop any alias a user flag already claims (e.g. -v + // for --verbose) so the user flag wins but --version + // still works. See #2229. + flag.Aliases = dropClashingAliases(flag.Aliases, cmd.allFlags(), flag.Name) localVersionFlag = &flag } else { localVersionFlag = VersionFlag @@ -255,3 +259,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 6b8d93390a..7b50e57fe3 100644 --- a/command_test.go +++ b/command_test.go @@ -3019,6 +3019,54 @@ func TestCustomFlagsUsed(t *testing.T) { assert.NoError(t, err, "Run returned unexpected error") } +// Regression for #2229. When a user flag claims the -v alias, only that +// alias is yielded: --version must still print the version. +func TestVersionFlagWorksWhenAliasYieldedToUserFlag(t *testing.T) { + buf := new(bytes.Buffer) + called := false + + cmd := &Command{ + Name: "boom", + Version: "0.1.0", + Writer: buf, + Flags: []Flag{ + &BoolFlag{Name: "verbose", Aliases: []string{"v"}}, + }, + Action: func(_ context.Context, _ *Command) error { + called = true + return nil + }, + } + + err := cmd.Run(buildTestContext(t), []string{"boom", "--version"}) + + assert.NoError(t, err) + assert.False(t, called, "version should short-circuit the action") + assert.Contains(t, buf.String(), "0.1.0") +} + +func TestDropClashingAliases(t *testing.T) { + verbose := &BoolFlag{Name: "verbose", Aliases: []string{"v"}} + + for _, tc := range []struct { + name string + aliases []string + flags []Flag + selfName string + want []string + }{ + {name: "no aliases", aliases: nil, flags: []Flag{verbose}, selfName: "version", want: nil}, + {name: "no flags", aliases: []string{"v"}, flags: nil, selfName: "version", want: []string{"v"}}, + {name: "drops clashing alias", aliases: []string{"v"}, flags: []Flag{verbose}, selfName: "version", want: []string{}}, + {name: "keeps free alias", aliases: []string{"V"}, flags: []Flag{verbose}, selfName: "version", want: []string{"V"}}, + {name: "keeps selfName even if claimed", aliases: []string{"verbose"}, flags: []Flag{verbose}, selfName: "verbose", want: []string{"verbose"}}, + } { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, dropClashingAliases(tc.aliases, tc.flags, tc.selfName)) + }) + } +} + func TestCustomHelpVersionFlags(t *testing.T) { cmd := &Command{ Writer: io.Discard,