Skip to content

Commit

Permalink
cli: FlagErrorFunc: don't print long usage output for invalid flags
Browse files Browse the repository at this point in the history
When trying to use an invalid flag, the CLI currently prints the a short
error message, instructions to use the `--help` flag to learn about the
correct usage, followed by the command's usage output.

While this is a common convention, and may have been a nice gesture when
docker was still young and only had a few commands and options ("you did
something wrong, but 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.
- 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"
  (`See 'docker volume --help'.` in the example below). This helps
  separating the error message ("unkown flag") from the call-to-action.

Before this patch:

    docker volume --no-such-flag
    unknown flag: --no-such-flag
    See 'docker volume --help'.

    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.

With this patch:

    docker volume --no-such-flag
    docker: unknown flag: --no-such-flag

    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 cad08ff commit d5aa99a
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 6 deletions.
6 changes: 1 addition & 5 deletions cli/cobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,8 @@ func FlagErrorFunc(cmd *cobra.Command, err error) error {
return nil
}

usage := ""
if cmd.HasSubCommands() {
usage = "\n\n" + cmd.UsageString()
}
return StatusError{
Status: fmt.Sprintf("%s\nSee '%s --help'.%s", err, cmd.CommandPath(), usage),
Status: fmt.Sprintf("%[1]s: %[2]s\n\nUsage: %[3]s\n\nRun '%[4]s --help' for more information", cmd.Root().Name(), err, cmd.UseLine(), cmd.CommandPath()),
StatusCode: 125,
}
}
Expand Down
2 changes: 1 addition & 1 deletion e2e/cli-plugins/help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,6 @@ func TestGlobalHelp(t *testing.T) {
})
assert.Assert(t, is.Equal(res2.Stdout(), ""))
assert.Assert(t, is.Contains(res2.Stderr(), "unknown flag: --badopt"))
assert.Assert(t, is.Contains(res2.Stderr(), "See 'docker --help"))
assert.Assert(t, is.Contains(res2.Stderr(), "Run 'docker --help' for more information"))
})
}

0 comments on commit d5aa99a

Please sign in to comment.