fix: parse flags for help subcommand (urfave#2271)

PR urfave#2245 introduced a regression by short-circuiting pre-parse for any
command named "help", which silently dropped flags and arguments on
user-supplied help commands. Keep the pre-parse path intact and instead
skip the required-flag enforcement for the help and completion commands,
which was the actual goal of urfave#2245.

Fixes urfave#2271

Author: barry3406

command.go

@@ -408,6 +408,12 @@ func (cmd *Command) checkRequiredFlag(f Flag) (bool, string) {
 }
 
 func (cmd *Command) checkAllRequiredFlags() requiredFlagsErr {
+	// The help and completion commands are allowed to run without
+	// enforcement of required flags, since they do not invoke user
+	// actions that depend on those flag values.
+	if cmd.Name == helpName || cmd.isCompletionCommand {
+		return nil
+	}
 	for pCmd := cmd; pCmd != nil; pCmd = pCmd.parent {
 		if err := pCmd.checkRequiredFlags(); err != nil {
 			return err

command_run.go

@@ -141,8 +141,8 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context
 	var rargs Args = &stringSliceArgs{v: osArgs}
 	var args Args = &stringSliceArgs{rargs.Tail()}
 
-	if cmd.isCompletionCommand || cmd.Name == helpName {
-		tracef("special command detected, skipping pre-parse (cmd=%[1]q)", cmd.Name)
+	if cmd.isCompletionCommand {
+		tracef("completion command detected, skipping pre-parse (cmd=%[1]q)", cmd.Name)
 		cmd.parsedArgs = args
 		return ctx, cmd.Action(ctx, cmd)
 	}

help_test.go

@@ -164,6 +164,35 @@ GLOBAL OPTIONS:
 		"expected output to include usage text")
 }
 
+// Test_HelpCommand_ParsesFlags is a regression test for #2271: flags
+// declared on a user-supplied command named "help" must still be parsed.
+// Previously in v3.6.2 flag pre-parsing was skipped for any command named
+// "help", causing such flags to be silently dropped.
+func Test_HelpCommand_ParsesFlags(t *testing.T) {
+	var parsed string
+
+	cmd := &Command{
+		Name: "app",
+		Commands: []*Command{
+			{
+				Name: "help",
+				Flags: []Flag{
+					&StringFlag{Name: "topic", Aliases: []string{"t"}},
+				},
+				Action: func(_ context.Context, c *Command) error {
+					parsed = c.String("topic")
+					return nil
+				},
+			},
+		},
+	}
+
+	err := cmd.Run(buildTestContext(t), []string{"app", "help", "--topic", "foo"})
+	require.NoError(t, err)
+	assert.Equal(t, "foo", parsed,
+		"expected --topic flag on help subcommand to be parsed")
+}
+
 func Test_Help_Custom_Flags(t *testing.T) {
 	oldFlag := HelpFlag
 	defer func() {