diff --git a/command.go b/command.go index 01740d5e13..4cd907a558 100644 --- a/command.go +++ b/command.go @@ -165,6 +165,8 @@ type Command struct { versionFlag Flag // whether this is a completion command isCompletionCommand bool + // whether this is the built-in help command + builtInHelp bool } func (cmd *Command) Command(name string) *Command { @@ -419,7 +421,7 @@ 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 { + if cmd.builtInHelp || cmd.isCompletionCommand { return nil } for pCmd := cmd; pCmd != nil; pCmd = pCmd.parent { diff --git a/command_run.go b/command_run.go index ec583fea2b..5549e25270 100644 --- a/command_run.go +++ b/command_run.go @@ -194,7 +194,7 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context } } else { tracef("running ShowCommandHelp with %[1]q", cmd.Name) - if err := ShowCommandHelp(ctx, cmd, cmd.Name); err != nil { + if err := ShowCommandHelp(ctx, cmd.parent, cmd.Name); err != nil { tracef("SILENTLY IGNORING ERROR running ShowCommandHelp with %[1]q %[2]v", cmd.Name, err) } } @@ -248,7 +248,14 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context if cmd.OnUsageError != nil { err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil) } else { - _ = ShowSubcommandHelp(cmd) + fmt.Fprintf(cmd.Root().ErrWriter, "Incorrect Usage: %s\n\n", err.Error()) + if cmd.parent == nil { + _ = ShowRootCommandHelp(cmd) + } else { + if err := ShowCommandHelp(ctx, cmd.parent, cmd.Name); err != nil { + _ = ShowSubcommandHelp(cmd) + } + } } return ctx, err } @@ -328,7 +335,14 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context if cmd.OnUsageError != nil { err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil) } else { - _ = ShowSubcommandHelp(cmd) + fmt.Fprintf(cmd.Root().ErrWriter, "Incorrect Usage: %s\n\n", err.Error()) + if cmd.parent == nil { + _ = ShowRootCommandHelp(cmd) + } else { + if err := ShowCommandHelp(ctx, cmd.parent, cmd.Name); err != nil { + _ = ShowSubcommandHelp(cmd) + } + } } return ctx, err } diff --git a/command_test.go b/command_test.go index d12aab0095..2a71504535 100644 --- a/command_test.go +++ b/command_test.go @@ -1966,6 +1966,183 @@ func TestRequiredFlagCommandRunBehavior(t *testing.T) { } } +func TestCommand_IncorrectUsageOnRequiredFlagsViaRun(t *testing.T) { + t.Run("root command missing required flag", func(t *testing.T) { + r := require.New(t) + var buf bytes.Buffer + var errBuf bytes.Buffer + + cmd := &Command{ + Writer: &buf, + ErrWriter: &errBuf, + Flags: []Flag{ + &StringFlag{Name: "requiredFlag", Required: true}, + }, + } + + _ = cmd.Run(buildTestContext(t), []string{"command"}) + r.Contains(errBuf.String(), "Incorrect Usage") + r.Contains(buf.String(), "NAME:") + r.Contains(buf.String(), "command") + }) + + t.Run("subcommand missing required flag", func(t *testing.T) { + r := require.New(t) + var buf bytes.Buffer + var errBuf bytes.Buffer + + cmd := &Command{ + Writer: &buf, + ErrWriter: &errBuf, + Commands: []*Command{ + { + Name: "sub", + Flags: []Flag{ + &StringFlag{Name: "requiredFlag", Required: true}, + }, + Action: func(ctx context.Context, cmd *Command) error { + return nil + }, + }, + }, + } + + _ = cmd.Run(buildTestContext(t), []string{"command", "sub"}) + r.Contains(errBuf.String(), "Incorrect Usage") + r.Contains(buf.String(), "NAME:") + r.Contains(buf.String(), "sub") + }) +} + +func TestCommand_IncorrectUsageOnMutuallyExclusiveFlagsViaRun(t *testing.T) { + t.Run("root command with mutex violation", func(t *testing.T) { + r := require.New(t) + var buf bytes.Buffer + var errBuf bytes.Buffer + + cmd := &Command{ + Writer: &buf, + ErrWriter: &errBuf, + MutuallyExclusiveFlags: []MutuallyExclusiveFlags{ + { + Flags: [][]Flag{ + {&StringFlag{Name: "foo1"}}, + {&StringFlag{Name: "foo2"}}, + }, + }, + }, + } + + _ = cmd.Run(buildTestContext(t), []string{"command", "--foo1", "v1", "--foo2", "v2"}) + r.Contains(errBuf.String(), "Incorrect Usage") + r.Contains(buf.String(), "NAME:") + r.Contains(buf.String(), "command") + }) + + t.Run("subcommand with mutex violation", 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: "sub", + MutuallyExclusiveFlags: []MutuallyExclusiveFlags{ + { + Flags: [][]Flag{ + {&StringFlag{Name: "foo1"}}, + {&StringFlag{Name: "foo2"}}, + }, + }, + }, + }, + }, + } + + _ = cmd.Run(buildTestContext(t), []string{"command", "sub", "--foo1", "v1", "--foo2", "v2"}) + r.Contains(errBuf.String(), "Incorrect Usage") + r.Contains(buf.String(), "NAME:") + r.Contains(buf.String(), "sub") + }) +} + +func TestCommand_IncorrectUsageOnSubcommandWithFailingShowCommandHelp(t *testing.T) { + t.Run("required flags fallback to ShowSubcommandHelp", func(t *testing.T) { + oldShowCommandHelp := ShowCommandHelp + defer func() { ShowCommandHelp = oldShowCommandHelp }() + ShowCommandHelp = func(_ context.Context, _ *Command, _ string) error { + return errors.New("forced error") + } + + r := require.New(t) + var buf bytes.Buffer + var errBuf bytes.Buffer + + cmd := &Command{ + Writer: &buf, + ErrWriter: &errBuf, + Commands: []*Command{ + { + Name: "sub", + Flags: []Flag{ + &StringFlag{Name: "requiredFlag", Required: true}, + }, + Action: func(ctx context.Context, cmd *Command) error { + return nil + }, + }, + }, + } + + _ = cmd.Run(buildTestContext(t), []string{"command", "sub"}) + r.Contains(errBuf.String(), "Incorrect Usage") + r.Contains(buf.String(), "sub") + }) + + t.Run("mutex flags fallback to ShowSubcommandHelp", func(t *testing.T) { + oldShowCommandHelp := ShowCommandHelp + defer func() { ShowCommandHelp = oldShowCommandHelp }() + ShowCommandHelp = func(_ context.Context, _ *Command, _ string) error { + return errors.New("forced error") + } + + 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: "sub", + MutuallyExclusiveFlags: []MutuallyExclusiveFlags{ + { + Flags: [][]Flag{ + {&StringFlag{Name: "foo1"}}, + {&StringFlag{Name: "foo2"}}, + }, + }, + }, + }, + }, + } + + _ = cmd.Run(buildTestContext(t), []string{"command", "sub", "--foo1", "v1", "--foo2", "v2"}) + r.Contains(errBuf.String(), "Incorrect Usage") + r.Contains(buf.String(), "sub") + }) +} + func TestCommandHelpPrinter(t *testing.T) { oldPrinter := HelpPrinter defer func() { diff --git a/help.go b/help.go index 76c6ac4f30..7b7b4097d0 100644 --- a/help.go +++ b/help.go @@ -65,11 +65,12 @@ var ArgsUsageCommandHelp = "[command]" func buildHelpCommand(withAction bool) *Command { cmd := &Command{ - Name: helpName, - Aliases: []string{helpAlias}, - Usage: UsageCommandHelp, - ArgsUsage: ArgsUsageCommandHelp, - HideHelp: true, + Name: helpName, + Aliases: []string{helpAlias}, + Usage: UsageCommandHelp, + ArgsUsage: ArgsUsageCommandHelp, + HideHelp: true, + builtInHelp: true, } if withAction { @@ -100,7 +101,7 @@ func helpCommandAction(ctx context.Context, cmd *Command) error { // to // $ app foo // which will then be handled as case 3 - if cmd.parent != nil && (cmd.HasName(helpName) || cmd.HasName(helpAlias)) { + if cmd.parent != nil && cmd.builtInHelp { tracef("setting cmd to cmd.parent") cmd = cmd.parent }