diff --git a/cli/help.go b/cli/help.go index 0654eb7bd9..4ece61b437 100644 --- a/cli/help.go +++ b/cli/help.go @@ -4,7 +4,9 @@ import ( "bufio" _ "embed" "fmt" + "os" "regexp" + "slices" "sort" "strings" "text/tabwriter" @@ -320,6 +322,25 @@ var usageWantsArgRe = regexp.MustCompile(`<.*>`) // output for a given command. func helpFn() serpent.HandlerFunc { return func(inv *serpent.Invocation) error { + // Check for invalid subcommands before printing help. + if len(inv.Args) > 0 && !usageWantsArgRe.MatchString(inv.Command.Use) { + _, _ = fmt.Fprintf(inv.Stderr, "---\nerror: unrecognized subcommand %q\n", inv.Args[0]) + } + if len(inv.Args) > 0 { + // Return an error so that exit status is non-zero when + // a subcommand is not found. + err := xerrors.Errorf("unrecognized subcommand %q", strings.Join(inv.Args, " ")) + if slices.Contains(os.Args, "--help") { + // Subcommand error is not wrapped in RunCommandErr if command + // is invoked with --help with no HelpHandler + return &serpent.RunCommandError{ + Cmd: inv.Command, + Err: err, + } + } + return err + } + // We use stdout for help and not stderr since there's no straightforward // way to distinguish between a user error and a help request. // @@ -340,9 +361,6 @@ func helpFn() serpent.HandlerFunc { if err != nil { return err } - if len(inv.Args) > 0 && !usageWantsArgRe.MatchString(inv.Command.Use) { - _, _ = fmt.Fprintf(inv.Stderr, "---\nerror: unknown subcommand %q\n", inv.Args[0]) - } return nil } } diff --git a/cli/portforward.go b/cli/portforward.go index e4b44609d5..68a076d590 100644 --- a/cli/portforward.go +++ b/cli/portforward.go @@ -69,10 +69,6 @@ func (r *RootCmd) portForward() *serpent.Command { return xerrors.Errorf("parse port-forward specs: %w", err) } if len(specs) == 0 { - err = inv.Command.HelpHandler(inv) - if err != nil { - return xerrors.Errorf("generate help output: %w", err) - } return xerrors.New("no port-forwards requested") } diff --git a/cli/portforward_test.go b/cli/portforward_test.go index b211a840dd..9ea5335c43 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -35,15 +35,10 @@ func TestPortForward_None(t *testing.T) { inv, root := clitest.New(t, "port-forward", "blah") clitest.SetupConfig(t, member, root) - pty := ptytest.New(t).Attach(inv) - inv.Stderr = pty.Output() err := inv.Run() require.Error(t, err) require.ErrorContains(t, err, "no port-forwards") - - // Check that the help was printed. - pty.ExpectMatch("port-forward ") } func TestPortForward(t *testing.T) { diff --git a/cli/root.go b/cli/root.go index d75c319c01..27330c856f 100644 --- a/cli/root.go +++ b/cli/root.go @@ -1196,10 +1196,13 @@ func formatMultiError(from string, multi []error, opts *formatOpts) string { // formatter and add it to cliHumanFormatError function. func formatRunCommandError(err *serpent.RunCommandError, opts *formatOpts) string { var str strings.Builder - _, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("Encountered an error running %q", err.Cmd.FullName()))) + _, _ = str.WriteString(pretty.Sprint(headLineStyle(), + fmt.Sprintf( + `Encountered an error running %q, see "%s --help" for more information`, + err.Cmd.FullName(), err.Cmd.FullName()))) + _, _ = str.WriteString(pretty.Sprint(headLineStyle(), "\nerror: ")) msgString, special := cliHumanFormatError("", err.Err, opts) - _, _ = str.WriteString("\n") if special { _, _ = str.WriteString(msgString) } else { diff --git a/cli/root_test.go b/cli/root_test.go index 07439322eb..bc10cfceec 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -60,6 +60,49 @@ func TestCommandHelp(t *testing.T) { func TestRoot(t *testing.T) { t.Parallel() + t.Run("MissingRootCommand", func(t *testing.T) { + t.Parallel() + + out := new(bytes.Buffer) + + inv, _ := clitest.New(t, "idontexist") + inv.Stdout = out + + err := inv.Run() + assert.ErrorContains(t, err, + `unrecognized subcommand "idontexist"`) + require.Empty(t, out.String()) + }) + + t.Run("MissingSubcommand", func(t *testing.T) { + t.Parallel() + + out := new(bytes.Buffer) + + inv, _ := clitest.New(t, "server", "idontexist") + inv.Stdout = out + + err := inv.Run() + // subcommand error only when command has subcommands + assert.ErrorContains(t, err, + `unrecognized subcommand "idontexist"`) + require.Empty(t, out.String()) + }) + + t.Run("BadSubcommandArgs", func(t *testing.T) { + t.Parallel() + + out := new(bytes.Buffer) + + inv, _ := clitest.New(t, "list", "idontexist") + inv.Stdout = out + + err := inv.Run() + assert.ErrorContains(t, err, + `wanted no args but got 1 [idontexist]`) + require.Empty(t, out.String()) + }) + t.Run("Version", func(t *testing.T) { t.Parallel() diff --git a/go.sum b/go.sum index 75d2cbcbef..718c742125 100644 --- a/go.sum +++ b/go.sum @@ -218,8 +218,6 @@ github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc= github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= github.com/coder/retry v1.5.1/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY= -github.com/coder/serpent v0.4.0 h1:L/itwnCxfhLutxQ2mScP3tH1ro8z8+Kc/iKKyZZxEMk= -github.com/coder/serpent v0.4.0/go.mod h1:Wud83ikZF/ulbdMcEMAwqvkEIQx7+l47+ef69M/arAA= github.com/coder/serpent v0.4.1-0.20240315163851-a0148c87ea3f h1:nqJ/Mvm+nLI22n5BIYhvSmTZ6CD+MRo/aGVZwVQgr1k= github.com/coder/serpent v0.4.1-0.20240315163851-a0148c87ea3f/go.mod h1:REkJ5ZFHQUWFTPLExhXYZ1CaHFjxvGNRlLXLdsI08YA= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuOD6a/zVP3rcxezNsoDseTUw=