Skip to content

Commit 9fbba57

Browse files
marckhouzammarckhouzam
committed
Address comments
Signed-off-by: Marc Khouzam <[email protected]>
1 parent 8e51700 commit 9fbba57

File tree

2 files changed

+104
-99
lines changed

2 files changed

+104
-99
lines changed

‎command.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -469,10 +469,8 @@ func (c *Command) HelpFunc() func(*Command, []string) {
469469
}
470470
}
471471

472-
// Help puts out the help for the command.
472+
// Help invokes the HelpFunc without arguments.
473473
// Kept for backwards compatibility.
474-
// No longer used because it does not allow
475-
// to pass arguments to the help command.
476474
// Can be a simple way to trigger the help
477475
// if arguments are not needed.
478476
func (c *Command) Help() error {
@@ -1124,11 +1122,14 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
11241122
if errors.Is(err, flag.ErrHelp) {
11251123
// The call to execute() above has parsed the flags.
11261124
// We therefore only pass the remaining arguments to the help function.
1127-
argWoFlags := cmd.Flags().Args()
1125+
remainingArgs := cmd.Flags().Args()
11281126
if cmd.DisableFlagParsing {
1129-
argWoFlags = flags
1127+
// For commands that have DisableFlagParsing == true, the flag parsing
1128+
// was not done and cmd.Flags().Args() is not filled. We therefore
1129+
// use the full set of arguments, which include flags.
1130+
remainingArgs = flags
11301131
}
1131-
cmd.HelpFunc()(cmd, argWoFlags)
1132+
cmd.HelpFunc()(cmd, remainingArgs)
11321133
return cmd, nil
11331134
}
11341135

‎command_test.go

Lines changed: 97 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
//nolint:goconst
1615
package cobra
1716

1817
import (
@@ -1080,189 +1079,194 @@ func TestHelpExecutedOnNonRunnableChild(t *testing.T) {
10801079
checkStringContains(t, output, childCmd.Long)
10811080
}
10821081

1083-
func TestHelpOverrideOnRoot(t *testing.T) {
1084-
var helpCalled bool
1085-
rootCmd := &Command{Use: "root"}
1086-
rootCmd.SetHelpFunc(func(c *Command, args []string) {
1087-
helpCalled = true
1088-
if c.Name() != "root" {
1089-
t.Errorf(`Expected command name: "root", got %q`, c.Name())
1082+
type overridingHelp struct {
1083+
expectedCmdName string
1084+
expectedArgs []string
1085+
1086+
helpCalled bool
1087+
err error
1088+
}
1089+
1090+
func (o *overridingHelp) helpFunc() func(c *Command, args []string) {
1091+
return func(c *Command, args []string) {
1092+
o.helpCalled = true
1093+
if c.Name() != o.expectedCmdName {
1094+
o.err = fmt.Errorf("Expected command name: %q, got %q", o.expectedCmdName, c.Name())
1095+
return
1096+
}
1097+
1098+
if len(args) != len(o.expectedArgs) {
1099+
o.err = fmt.Errorf("Expected args %v, got %v", o.expectedArgs, args)
1100+
return
10901101
}
1091-
if len(args) != 2 || args[0] != "arg1" || args[1] != "arg2" {
1092-
t.Errorf("Expected args [args1 arg2], got %v", args)
1102+
1103+
for i, arg := range o.expectedArgs {
1104+
if args[i] != arg {
1105+
o.err = fmt.Errorf("Expected args %v, got %v", o.expectedArgs, args)
1106+
return
1107+
}
10931108
}
1094-
})
1109+
}
1110+
}
1111+
1112+
func (o *overridingHelp) checkError() error {
1113+
if o.err != nil {
1114+
return o.err
1115+
}
1116+
if !o.helpCalled {
1117+
return fmt.Errorf("Overridden help function not called")
1118+
}
1119+
return nil
1120+
}
1121+
1122+
func TestHelpOverrideOnRoot(t *testing.T) {
1123+
rootCmd := &Command{Use: "root"}
1124+
1125+
override := overridingHelp{
1126+
expectedCmdName: rootCmd.Name(),
1127+
expectedArgs: []string{"arg1", "arg2"},
1128+
}
1129+
rootCmd.SetHelpFunc(override.helpFunc())
10951130

10961131
_, err := executeCommand(rootCmd, "arg1", "arg2", "--help")
10971132
if err != nil {
1098-
t.Errorf("Unexpected error: %v", err)
1133+
t.Errorf("Unexpected error executing command: %v", err)
10991134
}
11001135

1101-
if !helpCalled {
1102-
t.Error("Overridden help function not called")
1136+
if err = override.checkError(); err != nil {
1137+
t.Errorf("Unexpected error from help function: %v", err)
11031138
}
11041139
}
11051140

11061141
func TestHelpOverrideOnChild(t *testing.T) {
1107-
var helpCalled bool
11081142
rootCmd := &Command{Use: "root"}
11091143
subCmd := &Command{Use: "child"}
11101144
rootCmd.AddCommand(subCmd)
11111145

1112-
subCmd.SetHelpFunc(func(c *Command, args []string) {
1113-
helpCalled = true
1114-
if c.Name() != "child" {
1115-
t.Errorf(`Expected command name: "child", got %q`, c.Name())
1116-
}
1117-
if len(args) != 2 || args[0] != "arg1" || args[1] != "arg2" {
1118-
t.Errorf("Expected args [args1 arg2], got %v", args)
1119-
}
1120-
})
1146+
override := overridingHelp{
1147+
expectedCmdName: subCmd.Name(),
1148+
expectedArgs: []string{"arg1", "arg2"},
1149+
}
1150+
subCmd.SetHelpFunc(override.helpFunc())
11211151

11221152
_, err := executeCommand(rootCmd, "child", "arg1", "arg2", "--help")
11231153
if err != nil {
1124-
t.Errorf("Unexpected error: %v", err)
1154+
t.Errorf("Unexpected error executing command: %v", err)
11251155
}
11261156

1127-
if !helpCalled {
1128-
t.Error("Overridden help function not called")
1157+
if err = override.checkError(); err != nil {
1158+
t.Errorf("Unexpected error from help function: %v", err)
11291159
}
11301160
}
11311161

11321162
func TestHelpOverrideOnRootWithChild(t *testing.T) {
1133-
var helpCalled bool
11341163
rootCmd := &Command{Use: "root"}
11351164
subCmd := &Command{Use: "child"}
11361165
rootCmd.AddCommand(subCmd)
11371166

1138-
rootCmd.SetHelpFunc(func(c *Command, args []string) {
1139-
helpCalled = true
1140-
if c.Name() != "child" {
1141-
t.Errorf(`Expected command name: "child", got %q`, c.Name())
1142-
}
1143-
if len(args) != 2 || args[0] != "arg1" || args[1] != "arg2" {
1144-
t.Errorf("Expected args [args1 arg2], got %v", args)
1145-
}
1146-
})
1167+
override := overridingHelp{
1168+
expectedCmdName: subCmd.Name(),
1169+
expectedArgs: []string{"arg1", "arg2"},
1170+
}
1171+
rootCmd.SetHelpFunc(override.helpFunc())
11471172

11481173
_, err := executeCommand(rootCmd, "child", "arg1", "arg2", "--help")
11491174
if err != nil {
1150-
t.Errorf("Unexpected error: %v", err)
1175+
t.Errorf("Unexpected error executing command: %v", err)
11511176
}
11521177

1153-
if !helpCalled {
1154-
t.Error("Overridden help function not called")
1178+
if err = override.checkError(); err != nil {
1179+
t.Errorf("Unexpected error from help function: %v", err)
11551180
}
11561181
}
11571182

11581183
func TestHelpOverrideOnRootWithChildAndFlags(t *testing.T) {
1159-
var helpCalled bool
11601184
rootCmd := &Command{Use: "root"}
11611185
subCmd := &Command{Use: "child"}
11621186
rootCmd.AddCommand(subCmd)
11631187

11641188
var myFlag bool
11651189
subCmd.Flags().BoolVar(&myFlag, "myflag", false, "")
11661190

1167-
rootCmd.SetHelpFunc(func(c *Command, args []string) {
1168-
helpCalled = true
1169-
if c.Name() != "child" {
1170-
t.Errorf(`Expected command name: "child", got %q`, c.Name())
1171-
}
1172-
if len(args) != 2 || args[0] != "arg1" || args[1] != "arg2" {
1173-
t.Errorf("Expected args [args1 arg2], got %v", args)
1174-
}
1175-
})
1191+
override := overridingHelp{
1192+
expectedCmdName: subCmd.Name(),
1193+
expectedArgs: []string{"arg1", "arg2"},
1194+
}
1195+
rootCmd.SetHelpFunc(override.helpFunc())
11761196

11771197
_, err := executeCommand(rootCmd, "child", "arg1", "--myflag", "arg2", "--help")
11781198
if err != nil {
1179-
t.Errorf("Unexpected error: %v", err)
1199+
t.Errorf("Unexpected error executing command: %v", err)
11801200
}
11811201

1182-
if !helpCalled {
1183-
t.Error("Overridden help function not called")
1202+
if err = override.checkError(); err != nil {
1203+
t.Errorf("Unexpected error from help function: %v", err)
11841204
}
11851205
}
11861206

11871207
func TestHelpOverrideOnRootWithChildAndFlagsButParsingDisabled(t *testing.T) {
1188-
var helpCalled bool
11891208
rootCmd := &Command{Use: "root"}
11901209
subCmd := &Command{Use: "child", DisableFlagParsing: true}
11911210
rootCmd.AddCommand(subCmd)
11921211

11931212
var myFlag bool
11941213
subCmd.Flags().BoolVar(&myFlag, "myflag", false, "")
11951214

1196-
rootCmd.SetHelpFunc(func(c *Command, args []string) {
1197-
helpCalled = true
1198-
if c.Name() != "child" {
1199-
t.Errorf(`Expected command name: "child", got %q`, c.Name())
1200-
}
1201-
if len(args) != 4 ||
1202-
args[0] != "arg1" || args[1] != "--myflag" || args[2] != "arg2" || args[3] != "--help" {
1203-
t.Errorf("Expected args [args1 --myflag arg2 --help], got %v", args)
1204-
}
1205-
})
1215+
override := overridingHelp{
1216+
expectedCmdName: subCmd.Name(),
1217+
expectedArgs: []string{"arg1", "--myflag", "arg2", "--help"},
1218+
}
1219+
rootCmd.SetHelpFunc(override.helpFunc())
12061220

12071221
_, err := executeCommand(rootCmd, "child", "arg1", "--myflag", "arg2", "--help")
12081222
if err != nil {
1209-
t.Errorf("Unexpected error: %v", err)
1223+
t.Errorf("Unexpected error executing command: %v", err)
12101224
}
12111225

1212-
if !helpCalled {
1213-
t.Error("Overridden help function not called")
1226+
if err = override.checkError(); err != nil {
1227+
t.Errorf("Unexpected error from help function: %v", err)
12141228
}
12151229
}
12161230

12171231
func TestHelpCommandOverrideOnChild(t *testing.T) {
1218-
var helpCalled bool
12191232
rootCmd := &Command{Use: "root"}
12201233
subCmd := &Command{Use: "child"}
12211234
rootCmd.AddCommand(subCmd)
12221235

1223-
subCmd.SetHelpFunc(func(c *Command, args []string) {
1224-
helpCalled = true
1225-
if c.Name() != "child" {
1226-
t.Errorf(`Expected command name: "child", got %q`, c.Name())
1227-
}
1228-
if len(args) != 2 || args[0] != "arg1" || args[1] != "arg2" {
1229-
t.Errorf("Expected args [args1 arg2], got %v", args)
1230-
}
1231-
})
1236+
override := overridingHelp{
1237+
expectedCmdName: subCmd.Name(),
1238+
expectedArgs: []string{"arg1", "arg2"},
1239+
}
1240+
subCmd.SetHelpFunc(override.helpFunc())
12321241

12331242
_, err := executeCommand(rootCmd, "help", "child", "arg1", "arg2")
12341243
if err != nil {
1235-
t.Errorf("Unexpected error: %v", err)
1244+
t.Errorf("Unexpected error executing command: %v", err)
12361245
}
12371246

1238-
if !helpCalled {
1239-
t.Error("Overridden help function not called")
1247+
if err = override.checkError(); err != nil {
1248+
t.Errorf("Unexpected error from help function: %v", err)
12401249
}
12411250
}
12421251

12431252
func TestHelpCommandOverrideOnRootWithChild(t *testing.T) {
1244-
var helpCalled bool
12451253
rootCmd := &Command{Use: "root"}
12461254
subCmd := &Command{Use: "child"}
12471255
rootCmd.AddCommand(subCmd)
12481256

1249-
rootCmd.SetHelpFunc(func(c *Command, args []string) {
1250-
helpCalled = true
1251-
if c.Name() != "child" {
1252-
t.Errorf(`Expected command name: "child", got %q`, c.Name())
1253-
}
1254-
if len(args) != 2 || args[0] != "arg1" || args[1] != "arg2" {
1255-
t.Errorf("Expected args [args1 arg2], got %v", args)
1256-
}
1257-
})
1257+
override := overridingHelp{
1258+
expectedCmdName: subCmd.Name(),
1259+
expectedArgs: []string{"arg1", "arg2"},
1260+
}
1261+
rootCmd.SetHelpFunc(override.helpFunc())
12581262

12591263
_, err := executeCommand(rootCmd, "help", "child", "arg1", "arg2")
12601264
if err != nil {
1261-
t.Errorf("Unexpected error: %v", err)
1265+
t.Errorf("Unexpected error executing command: %v", err)
12621266
}
12631267

1264-
if !helpCalled {
1265-
t.Error("Overridden help function not called")
1268+
if err = override.checkError(); err != nil {
1269+
t.Errorf("Unexpected error from help function: %v", err)
12661270
}
12671271
}
12681272

0 commit comments

Comments
 (0)