Merge pull request #2360 from dearchap/issue_2126

fix: let --help flag take precedence over parse errors

Author: dearchap

command_run.go

@@ -175,6 +175,14 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context
 		deferErr = err
 
 		cmd.isInError = true
+		if cmd.checkHelp() {
+			if cmd.parent == nil {
+				_ = ShowRootCommandHelp(cmd)
+			} else {
+				_ = ShowSubcommandHelp(cmd)
+			}
+			return ctx, nil
+		}
 		if cmd.OnUsageError != nil {
 			err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil)
 			err = cmd.handleExitCoder(ctx, err)
@@ -193,10 +201,8 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context
 					tracef("SILENTLY IGNORING ERROR running ShowRootCommandHelp %[1]v (cmd=%[2]q)", err, cmd.Name)
 				}
 			} else {
-				tracef("running ShowCommandHelp with %[1]q", cmd.Name)
-				if err := ShowCommandHelp(ctx, cmd.parent, cmd.Name); err != nil {
-					tracef("SILENTLY IGNORING ERROR running ShowCommandHelp with %[1]q %[2]v", cmd.Name, err)
-				}
+				tracef("running ShowSubcommandHelp for %[1]q", cmd.Name)
+				_ = ShowSubcommandHelp(cmd)
 			}
 		}
 

command_test.go

@@ -2363,6 +2363,104 @@ func TestCommand_OrderOfOperations(t *testing.T) {
 	})
 }
 
+func TestCommand_HelpFlagTakesPrecedenceOverParseErrors(t *testing.T) {
+	t.Run("root command with --help and bad flag", func(t *testing.T) {
+		r := require.New(t)
+		var buf bytes.Buffer
+		var errBuf bytes.Buffer
+
+		cmd := &Command{
+			Writer:    &buf,
+			ErrWriter: &errBuf,
+			Action: func(ctx context.Context, cmd *Command) error {
+				return nil
+			},
+		}
+
+		err := cmd.Run(buildTestContext(t), []string{"command", "--help", "--undefined"})
+		r.NoError(err)
+		r.Contains(buf.String(), "NAME:")
+		r.Contains(buf.String(), "command")
+		r.NotContains(errBuf.String(), "Incorrect Usage")
+	})
+
+	t.Run("root command with -h and bad flag", func(t *testing.T) {
+		r := require.New(t)
+		var buf bytes.Buffer
+		var errBuf bytes.Buffer
+
+		cmd := &Command{
+			Writer:    &buf,
+			ErrWriter: &errBuf,
+			Action: func(ctx context.Context, cmd *Command) error {
+				return nil
+			},
+		}
+
+		err := cmd.Run(buildTestContext(t), []string{"command", "-h", "--undefined"})
+		r.NoError(err)
+		r.Contains(buf.String(), "NAME:")
+		r.Contains(buf.String(), "command")
+		r.NotContains(errBuf.String(), "Incorrect Usage")
+	})
+
+	t.Run("subcommand with --help and bad flag", func(t *testing.T) {
+		r := require.New(t)
+		var buf bytes.Buffer
+		var errBuf bytes.Buffer
+
+		cmd := &Command{
+			Writer:    &buf,
+			ErrWriter: &errBuf,
+			Action: func(ctx context.Context, cmd *Command) error {
+				return nil
+			},
+			Commands: []*Command{
+				{
+					Name: "foo",
+					Action: func(ctx context.Context, cmd *Command) error {
+						return nil
+					},
+				},
+			},
+		}
+
+		err := cmd.Run(buildTestContext(t), []string{"command", "foo", "--help", "--undefined"})
+		r.NoError(err)
+		r.Contains(buf.String(), "NAME:")
+		r.Contains(buf.String(), "foo")
+		r.NotContains(errBuf.String(), "Incorrect Usage")
+	})
+
+	t.Run("subcommand with bad flag shows Incorrect Usage and help", func(t *testing.T) {
+		r := require.New(t)
+		var buf bytes.Buffer
+		var errBuf bytes.Buffer
+
+		cmd := &Command{
+			Writer:    &buf,
+			ErrWriter: &errBuf,
+			Action: func(ctx context.Context, cmd *Command) error {
+				return nil
+			},
+			Commands: []*Command{
+				{
+					Name: "foo",
+					Action: func(ctx context.Context, cmd *Command) error {
+						return nil
+					},
+				},
+			},
+		}
+
+		err := cmd.Run(buildTestContext(t), []string{"command", "foo", "--undefined"})
+		r.Error(err)
+		r.Contains(buf.String(), "NAME:")
+		r.Contains(buf.String(), "foo")
+		r.Contains(errBuf.String(), "Incorrect Usage")
+	})
+}
+
 func TestFlagActionOrder(t *testing.T) {
 	tests := []struct {
 		Name string

help.go

@@ -86,14 +86,22 @@ func helpCommandAction(ctx context.Context, cmd *Command) error {
 
 	tracef("doing help for cmd %[1]q with args %[2]q", cmd, args)
 
-	// This action can be triggered by a "default" action of a command
-	// or via cmd.Run when cmd == helpCmd. So we have following possibilities
+	// helpCommandAction is triggered in several ways:
 	//
-	// 1 $ app
-	// 2 $ app help
-	// 3 $ app foo
-	// 4 $ app help foo
-	// 5 $ app foo help
+	//   * the command has no user-defined Action (default action fallback)
+	//   * the --help / -h flag was parsed (via cmd.checkHelp())
+	//   * the "help" subcommand (or "h" alias) was dispatched
+	//
+	// Possible invocations:
+	//
+	//   $ app                  # default action; show root help
+	//   $ app --help / -h      # flag; show root help (ignores subsequent args)
+	//   $ app help / h         # subcommand; show root help
+	//   $ app help / h foo     # subcommand; show help for subcommand "foo"
+	//   $ app --help / -h foo  # flag; show help for subcommand "foo"
+	//   $ app foo --help / -h  # flag on subcommand; show help for "foo"
+	//   $ app foo help / h     # subcommand on subcommand; show help for "foo"
+	//   $ app foo (no action)  # default action on subcommand; show help for "foo"
 
 	// Case 4. when executing a help command set the context to parent
 	// to allow resolution of subsequent args. This will transform