Skip to content

Commit adff99b

Browse files
fix: keep -h reserved for help flag
The previous change avoided a panic by falling back to a long-only help flag when shorthand "h" was already in use. Reviewer feedback noted this would be a behavior change: -h is intentionally reserved for help, and the panic acts as an early signal of a developer conflict. Restore the panic on shorthand conflicts, but make it explicit and clearer by detecting an existing -h flag and panicking with a targeted message. Update the test to assert the reserved-shorthand behavior. Refs: spf13#2367 Context: reviewer feedback on PR
1 parent bb3fbc8 commit adff99b

2 files changed

Lines changed: 6 additions & 11 deletions

File tree

command.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,11 +1226,10 @@ func (c *Command) InitDefaultHelpFlag() {
12261226
} else {
12271227
usage += name
12281228
}
1229-
if c.Flags().ShorthandLookup("h") == nil {
1230-
c.Flags().BoolP(helpFlagName, "h", false, usage)
1231-
} else {
1232-
c.Flags().Bool(helpFlagName, false, usage)
1229+
if f := c.Flags().ShorthandLookup("h"); f != nil {
1230+
panic(fmt.Sprintf("shorthand '-h' is reserved for the help flag, but is already used by flag '%s'", f.Name))
12331231
}
1232+
c.Flags().BoolP(helpFlagName, "h", false, usage)
12341233
_ = c.Flags().SetAnnotation(helpFlagName, FlagSetByCobraAnnotation, []string{"true"})
12351234
}
12361235
}

command_test.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -939,23 +939,19 @@ func TestInitHelpFlagMergesFlags(t *testing.T) {
939939
}
940940

941941
// related to https://github.com/spf13/cobra/issues/2359
942-
func TestInitDefaultHelpFlagDoesNotPanicWhenHShorthandAlreadyUsed(t *testing.T) {
942+
func TestInitDefaultHelpFlagPanicsWhenHShorthandAlreadyUsed(t *testing.T) {
943943
cmd := &Command{Use: "root"}
944944

945945
cmd.PersistentFlags().BoolP("ayuda", "h", false, "help")
946946

947947
func() {
948948
defer func() {
949-
if r := recover(); r != nil {
950-
t.Fatalf("expected InitDefaultHelpFlag not to panic, got: %v", r)
949+
if r := recover(); r == nil {
950+
t.Fatalf("expected InitDefaultHelpFlag to panic")
951951
}
952952
}()
953953
cmd.InitDefaultHelpFlag()
954954
}()
955-
956-
if cmd.Flags().Lookup("help") == nil {
957-
t.Fatalf("expected default help flag to be defined")
958-
}
959955
}
960956

961957
func TestHelpCommandExecuted(t *testing.T) {

0 commit comments

Comments
 (0)