From d5aa99aa265116547960f00e66035f06932a1991 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Jul 2024 23:45:30 +0200 Subject: [PATCH] cli: FlagErrorFunc: don't print long usage output for invalid flags 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 --- cli/cobra.go | 6 +----- e2e/cli-plugins/help_test.go | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/cli/cobra.go b/cli/cobra.go index f6a69ae0701d..0f20e618943a 100644 --- a/cli/cobra.go +++ b/cli/cobra.go @@ -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, } } diff --git a/e2e/cli-plugins/help_test.go b/e2e/cli-plugins/help_test.go index 85db703d30d8..a686b9ac4546 100644 --- a/e2e/cli-plugins/help_test.go +++ b/e2e/cli-plugins/help_test.go @@ -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")) }) }