Skip to content

Commit

Permalink
cli: improve output and consistency for unknown (sub)commands
Browse files Browse the repository at this point in the history
Before this patch, output for invalid top-level and sub-commands differed.
For top-level commands, the CLI would print an error-message and a suggestion
to use `--help`. For missing *subcommands*, we would hit a different code-path,
and different output, which includes full "usage" / "help" output.

While it is a common convention to show usage output, and may have been
a nice gesture when docker was still young and only had a few commands
and options ("you did something wrong; here's an overview of what you
can use"), that's no longer the case, and many commands have a _very_
long output.

The result of this is that the error message, which is the relevant
information in this case - "You mis-typed something" - is lost in the
output, and hard to find (sometimes even requiring scrolling back).

The output is also confusing, because it _looks_ like something ran
successfully (most of the output is not about the error!).

Even further; the suggested resolution (try `--help` to see the correct
options) is rather redundant, because running teh command with `--help`
produces _exactly_ the same output as was just showh, baring the error
message. As a fun fact, due to the usage output being printed, the
output even contains not one, but _two_ "call to actions";

- `See 'docker volume --help'.` (under the erro message)
- `Run 'docker volume COMMAND --help' for more information on a command.`
  (under the usage output)

In short; the output is too verbose, confusing, and doesn't provide
a good UX. Let's reduce the output produced so that the focus is on the
important information.

This patch:

- Changes the usage to the short-usage.
- Changes the error-message to mention the _full_ command instead of only
  the command after `docker` (so `docker no-such-command` instead of
  `no-such-command`).
- Prefixes the error message with the binary / root-command name
  (usually `docker:`); this is something we can still decide on, but
  it's a pattern we already use in some places. The motivation for this
  is that `docker` commands can often produce output that's a combination
  of output from the CLI itself, output from the daemon, and even output
  from the container. The `docker:` prefix helps to distinguish where
  the message originated from (the `docker` CLI in this case).
- Adds an empty line between the error-message and the "call to action"
  (`Run 'docker volume --help'...` in the example below). This helps
  separating the error message ("unkown flag") from the call-to-action.

Before this patch:

Unknown top-level command:

    docker nosuchcommand foo
    docker: 'nosuchcommand' is not a docker command.
    See 'docker --help'

Unknown sub-command:

    docker volume nosuchcommand foo

    Usage:  docker volume COMMAND

    Manage volumes

    Commands:
      create      Create a volume
      inspect     Display detailed information on one or more volumes
      ls          List volumes
      prune       Remove unused local volumes
      rm          Remove one or more volumes
      update      Update a volume (cluster volumes only)

    Run 'docker volume COMMAND --help' for more information on a command.

After this patch:

Unknown top-level command:

    docker nosuchcommand foo
    docker: unknown command: docker nosuchcommand

    Run 'docker --help' for more information

Unknown sub-command:

    docker volume nosuchcommand foo
    docker: unknown command: 'docker volume nosuchcommand'

    Usage:  docker volume COMMAND

    Run 'docker volume --help' for more information

Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
thaJeztah committed Jul 5, 2024
1 parent 5aae44b commit a6e96c7
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 10 deletions.
2 changes: 1 addition & 1 deletion cli-plugins/manager/cobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) (err e
cmd.HelpFunc()(rootCmd, args)
return nil
}
return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'", cmd.Name())
return fmt.Errorf("docker: unknown command: docker %s\n\nRun 'docker --help' for more information", cmd.Name())
},
ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
// Delegate completion to plugin
Expand Down
15 changes: 12 additions & 3 deletions cli/required.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package cli

import (
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
)
Expand All @@ -14,7 +12,13 @@ func NoArgs(cmd *cobra.Command, args []string) error {
}

if cmd.HasSubCommands() {
return errors.Errorf("\n" + strings.TrimRight(cmd.UsageString(), "\n"))
return errors.Errorf(
"%[1]s: unknown command: %[2]s %[3]s\n\nUsage: %[4]s\n\nRun '%[2]s --help' for more information",
binName(cmd),
cmd.CommandPath(),
args[0],
cmd.UseLine(),
)
}

return errors.Errorf(
Expand Down Expand Up @@ -99,6 +103,11 @@ func ExactArgs(number int) cobra.PositionalArgs {
}
}

// binName returns the name of the binary / root command (usually 'docker').
func binName(cmd *cobra.Command) string {
return cmd.Root().Name()
}

//nolint:unparam
func pluralize(word string, number int) string {
if number == 1 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand {
if len(args) == 0 {
return command.ShowHelp(dockerCli.Err())(cmd, args)
}
return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'", args[0])
return fmt.Errorf("docker: unknown command: docker %s\n\nRun 'docker --help' for more information", args[0])
},
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
return isSupported(cmd, dockerCli)
Expand Down
2 changes: 1 addition & 1 deletion cmd/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestExitStatusForInvalidSubcommandWithHelpFlag(t *testing.T) {

func TestExitStatusForInvalidSubcommand(t *testing.T) {
err := runCliCommand(t, nil, nil, "invalid")
assert.Check(t, is.ErrorContains(err, "docker: 'invalid' is not a docker command."))
assert.Check(t, is.ErrorContains(err, "docker: 'docker invalid' is not a docker command."))
}

func TestVersion(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions e2e/cli-plugins/testdata/docker-badmeta-err.golden
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
docker: 'badmeta' is not a docker command.
See 'docker --help'
docker: unknown command: docker badmeta

Run 'docker --help' for more information
5 changes: 3 additions & 2 deletions e2e/cli-plugins/testdata/docker-nonexistent-err.golden
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
docker: 'nonexistent' is not a docker command.
See 'docker --help'
docker: unknown command: docker nonexistent

Run 'docker --help' for more information

0 comments on commit a6e96c7

Please sign in to comment.