From a6e96c758e3d3b41a2a81146342344eb82d1d4ec Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 5 Jul 2024 00:57:07 +0200 Subject: [PATCH 1/2] cli: improve output and consistency for unknown (sub)commands 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 --- cli-plugins/manager/cobra.go | 2 +- cli/required.go | 15 ++++++++++++--- cmd/docker/docker.go | 2 +- cmd/docker/docker_test.go | 2 +- .../testdata/docker-badmeta-err.golden | 5 +++-- .../testdata/docker-nonexistent-err.golden | 5 +++-- 6 files changed, 21 insertions(+), 10 deletions(-) diff --git a/cli-plugins/manager/cobra.go b/cli-plugins/manager/cobra.go index feff8a8fd6..9d09018001 100644 --- a/cli-plugins/manager/cobra.go +++ b/cli-plugins/manager/cobra.go @@ -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 diff --git a/cli/required.go b/cli/required.go index 454e247613..ba4e6e8efa 100644 --- a/cli/required.go +++ b/cli/required.go @@ -1,8 +1,6 @@ package cli import ( - "strings" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -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( @@ -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 { diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 7825c7d354..ca732180f1 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -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) diff --git a/cmd/docker/docker_test.go b/cmd/docker/docker_test.go index 8fc7ade3a4..84ea272a20 100644 --- a/cmd/docker/docker_test.go +++ b/cmd/docker/docker_test.go @@ -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) { diff --git a/e2e/cli-plugins/testdata/docker-badmeta-err.golden b/e2e/cli-plugins/testdata/docker-badmeta-err.golden index df2344638a..9342bdaf55 100644 --- a/e2e/cli-plugins/testdata/docker-badmeta-err.golden +++ b/e2e/cli-plugins/testdata/docker-badmeta-err.golden @@ -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 diff --git a/e2e/cli-plugins/testdata/docker-nonexistent-err.golden b/e2e/cli-plugins/testdata/docker-nonexistent-err.golden index f2265f6b9e..12c26f5f00 100644 --- a/e2e/cli-plugins/testdata/docker-nonexistent-err.golden +++ b/e2e/cli-plugins/testdata/docker-nonexistent-err.golden @@ -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 From c60b360c33320aba9a7e6bad86d0226bf69156c0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 5 Jul 2024 02:49:31 +0200 Subject: [PATCH 2/2] cli: improve argument validation output Improve the output for these validation errors: - Removes the short command description from the output. This information does not provide much useful help, and distracts from the error message. - Reduces punctuation, and - Prefixes the error message with the binary / root-command name (usually `docker:`) to be consistent with other similar errors. - 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 and "usage" from the call-to-action. Before this patch: $ docker volume ls one two three "docker volume ls" accepts no arguments. See 'docker volume ls --help'. Usage: docker volume ls [OPTIONS] List volumes $ docker volume create one two three "docker volume create" requires at most 1 argument. See 'docker volume create --help'. Usage: docker volume create [OPTIONS] [VOLUME] Create a volume With this patch: $ docker volume ls one two three docker: 'docker volume ls' accepts no arguments Usage: docker volume ls [OPTIONS] Run 'docker volume ls --help' for more information $ docker voludocker volume create one two three docker: 'docker volume create' requires at most 1 argument Usage: docker volume create [OPTIONS] [VOLUME] SRun 'docker volume create --help' for more information Signed-off-by: Sebastiaan van Stijn --- cli/command/checkpoint/create_test.go | 4 ++-- cli/command/checkpoint/list_test.go | 4 ++-- cli/command/checkpoint/remove_test.go | 4 ++-- cli/command/config/create_test.go | 4 ++-- cli/command/config/remove_test.go | 2 +- cli/command/image/history_test.go | 2 +- cli/command/image/import_test.go | 2 +- cli/command/image/inspect_test.go | 2 +- cli/command/image/list_test.go | 2 +- cli/command/image/load_test.go | 2 +- cli/command/image/prune_test.go | 2 +- cli/command/image/pull_test.go | 2 +- cli/command/image/push_test.go | 2 +- cli/command/image/remove_test.go | 2 +- cli/command/image/save_test.go | 2 +- cli/command/image/tag_test.go | 2 +- cli/command/manifest/annotate_test.go | 2 +- cli/command/manifest/push_test.go | 2 +- cli/command/network/connect_test.go | 2 +- cli/command/network/create_test.go | 2 +- cli/command/network/disconnect_test.go | 2 +- cli/command/node/update_test.go | 4 ++-- cli/command/plugin/disable_test.go | 4 ++-- cli/command/plugin/enable_test.go | 4 ++-- cli/command/secret/remove_test.go | 2 +- cli/command/service/rollback_test.go | 4 ++-- cli/command/stack/ps_test.go | 4 ++-- cli/command/swarm/join_test.go | 4 ++-- cli/command/swarm/join_token_test.go | 4 ++-- cli/command/trust/key_generate_test.go | 4 ++-- cli/command/trust/key_load_test.go | 5 +++-- cli/command/trust/revoke_test.go | 4 ++-- cli/command/trust/sign_test.go | 4 ++-- cli/required.go | 25 ++++++++++--------------- cli/required_test.go | 18 +++++++++--------- cmd/docker/docker_test.go | 2 +- e2e/cli-plugins/flags_test.go | 4 ++-- 37 files changed, 71 insertions(+), 75 deletions(-) diff --git a/cli/command/checkpoint/create_test.go b/cli/command/checkpoint/create_test.go index 3ad7922df6..b3b7372080 100644 --- a/cli/command/checkpoint/create_test.go +++ b/cli/command/checkpoint/create_test.go @@ -20,11 +20,11 @@ func TestCheckpointCreateErrors(t *testing.T) { }{ { args: []string{"too-few-arguments"}, - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"too", "many", "arguments"}, - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"foo", "bar"}, diff --git a/cli/command/checkpoint/list_test.go b/cli/command/checkpoint/list_test.go index 7ac4a768de..437cc1b359 100644 --- a/cli/command/checkpoint/list_test.go +++ b/cli/command/checkpoint/list_test.go @@ -20,11 +20,11 @@ func TestCheckpointListErrors(t *testing.T) { }{ { args: []string{}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"too", "many", "arguments"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"foo"}, diff --git a/cli/command/checkpoint/remove_test.go b/cli/command/checkpoint/remove_test.go index 01bda3c04d..ae8402edb2 100644 --- a/cli/command/checkpoint/remove_test.go +++ b/cli/command/checkpoint/remove_test.go @@ -19,11 +19,11 @@ func TestCheckpointRemoveErrors(t *testing.T) { }{ { args: []string{"too-few-arguments"}, - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"too", "many", "arguments"}, - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"foo", "bar"}, diff --git a/cli/command/config/create_test.go b/cli/command/config/create_test.go index db9f8eac82..848f9c6e07 100644 --- a/cli/command/config/create_test.go +++ b/cli/command/config/create_test.go @@ -28,11 +28,11 @@ func TestConfigCreateErrors(t *testing.T) { }{ { args: []string{"too_few"}, - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"too", "many", "arguments"}, - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"name", filepath.Join("testdata", configDataFile)}, diff --git a/cli/command/config/remove_test.go b/cli/command/config/remove_test.go index 1466391ee5..d1c5a71f9f 100644 --- a/cli/command/config/remove_test.go +++ b/cli/command/config/remove_test.go @@ -19,7 +19,7 @@ func TestConfigRemoveErrors(t *testing.T) { }{ { args: []string{}, - expectedError: "requires at least 1 argument.", + expectedError: "requires at least 1 argument", }, { args: []string{"foo"}, diff --git a/cli/command/image/history_test.go b/cli/command/image/history_test.go index 86eeaba3ba..072bd035a6 100644 --- a/cli/command/image/history_test.go +++ b/cli/command/image/history_test.go @@ -23,7 +23,7 @@ func TestNewHistoryCommandErrors(t *testing.T) { { name: "wrong-args", args: []string{}, - expectedError: "requires exactly 1 argument.", + expectedError: "requires 1 argument", }, { name: "client-error", diff --git a/cli/command/image/import_test.go b/cli/command/image/import_test.go index 59a6ced04d..196171ed31 100644 --- a/cli/command/image/import_test.go +++ b/cli/command/image/import_test.go @@ -22,7 +22,7 @@ func TestNewImportCommandErrors(t *testing.T) { { name: "wrong-args", args: []string{}, - expectedError: "requires at least 1 argument.", + expectedError: "requires at least 1 argument", }, { name: "import-failed", diff --git a/cli/command/image/inspect_test.go b/cli/command/image/inspect_test.go index 61ea36012e..9e2163db57 100644 --- a/cli/command/image/inspect_test.go +++ b/cli/command/image/inspect_test.go @@ -21,7 +21,7 @@ func TestNewInspectCommandErrors(t *testing.T) { { name: "wrong-args", args: []string{}, - expectedError: "requires at least 1 argument.", + expectedError: "requires at least 1 argument", }, } for _, tc := range testCases { diff --git a/cli/command/image/list_test.go b/cli/command/image/list_test.go index 4917c53939..8ccec6ab56 100644 --- a/cli/command/image/list_test.go +++ b/cli/command/image/list_test.go @@ -24,7 +24,7 @@ func TestNewImagesCommandErrors(t *testing.T) { { name: "wrong-args", args: []string{"arg1", "arg2"}, - expectedError: "requires at most 1 argument.", + expectedError: "requires at most 1 argument", }, { name: "failed-list", diff --git a/cli/command/image/load_test.go b/cli/command/image/load_test.go index 9c36ce3235..5a8a499bda 100644 --- a/cli/command/image/load_test.go +++ b/cli/command/image/load_test.go @@ -24,7 +24,7 @@ func TestNewLoadCommandErrors(t *testing.T) { { name: "wrong-args", args: []string{"arg"}, - expectedError: "accepts no arguments.", + expectedError: "accepts no arguments", }, { name: "input-to-terminal", diff --git a/cli/command/image/prune_test.go b/cli/command/image/prune_test.go index 1d6b2de983..710c1cf748 100644 --- a/cli/command/image/prune_test.go +++ b/cli/command/image/prune_test.go @@ -27,7 +27,7 @@ func TestNewPruneCommandErrors(t *testing.T) { { name: "wrong-args", args: []string{"something"}, - expectedError: "accepts no arguments.", + expectedError: "accepts no arguments", }, { name: "prune-error", diff --git a/cli/command/image/pull_test.go b/cli/command/image/pull_test.go index 43bc772ea7..a28aaf6166 100644 --- a/cli/command/image/pull_test.go +++ b/cli/command/image/pull_test.go @@ -23,7 +23,7 @@ func TestNewPullCommandErrors(t *testing.T) { }{ { name: "wrong-args", - expectedError: "requires exactly 1 argument.", + expectedError: "requires 1 argument", args: []string{}, }, { diff --git a/cli/command/image/push_test.go b/cli/command/image/push_test.go index a659c6c510..3f9d2fe59f 100644 --- a/cli/command/image/push_test.go +++ b/cli/command/image/push_test.go @@ -21,7 +21,7 @@ func TestNewPushCommandErrors(t *testing.T) { { name: "wrong-args", args: []string{}, - expectedError: "requires exactly 1 argument.", + expectedError: "requires 1 argument", }, { name: "invalid-name", diff --git a/cli/command/image/remove_test.go b/cli/command/image/remove_test.go index d4340b3819..0340a65b94 100644 --- a/cli/command/image/remove_test.go +++ b/cli/command/image/remove_test.go @@ -39,7 +39,7 @@ func TestNewRemoveCommandErrors(t *testing.T) { }{ { name: "wrong args", - expectedError: "requires at least 1 argument.", + expectedError: "requires at least 1 argument", }, { name: "ImageRemove fail with force option", diff --git a/cli/command/image/save_test.go b/cli/command/image/save_test.go index c6cff4cc3c..dc64ebae93 100644 --- a/cli/command/image/save_test.go +++ b/cli/command/image/save_test.go @@ -23,7 +23,7 @@ func TestNewSaveCommandErrors(t *testing.T) { { name: "wrong args", args: []string{}, - expectedError: "requires at least 1 argument.", + expectedError: "requires at least 1 argument", }, { name: "output to terminal", diff --git a/cli/command/image/tag_test.go b/cli/command/image/tag_test.go index 77bf81c2db..65ceb60323 100644 --- a/cli/command/image/tag_test.go +++ b/cli/command/image/tag_test.go @@ -15,7 +15,7 @@ func TestCliNewTagCommandErrors(t *testing.T) { {"image1"}, {"image1", "image2", "image3"}, } - expectedError := "\"tag\" requires exactly 2 arguments." + expectedError := "'tag' requires 2 arguments" for _, args := range testCases { cmd := NewTagCommand(test.NewFakeCli(&fakeClient{})) cmd.SetArgs(args) diff --git a/cli/command/manifest/annotate_test.go b/cli/command/manifest/annotate_test.go index 089b29c716..f2c924e45f 100644 --- a/cli/command/manifest/annotate_test.go +++ b/cli/command/manifest/annotate_test.go @@ -18,7 +18,7 @@ func TestManifestAnnotateError(t *testing.T) { }{ { args: []string{"too-few-arguments"}, - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"th!si'sa/fa!ke/li$t/name", "example.com/alpine:3.0"}, diff --git a/cli/command/manifest/push_test.go b/cli/command/manifest/push_test.go index 2a11fceda2..a10d5b24db 100644 --- a/cli/command/manifest/push_test.go +++ b/cli/command/manifest/push_test.go @@ -31,7 +31,7 @@ func TestManifestPushErrors(t *testing.T) { }{ { args: []string{"one-arg", "extra-arg"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"th!si'sa/fa!ke/li$t/-name"}, diff --git a/cli/command/network/connect_test.go b/cli/command/network/connect_test.go index cfdff28806..c44fe3fc5f 100644 --- a/cli/command/network/connect_test.go +++ b/cli/command/network/connect_test.go @@ -19,7 +19,7 @@ func TestNetworkConnectErrors(t *testing.T) { expectedError string }{ { - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"toto", "titi"}, diff --git a/cli/command/network/create_test.go b/cli/command/network/create_test.go index 8eab2ff0bd..348835f02a 100644 --- a/cli/command/network/create_test.go +++ b/cli/command/network/create_test.go @@ -21,7 +21,7 @@ func TestNetworkCreateErrors(t *testing.T) { expectedError string }{ { - expectedError: "exactly 1 argument", + expectedError: "1 argument", }, { args: []string{"toto"}, diff --git a/cli/command/network/disconnect_test.go b/cli/command/network/disconnect_test.go index bcbb491d09..bb82452a6c 100644 --- a/cli/command/network/disconnect_test.go +++ b/cli/command/network/disconnect_test.go @@ -17,7 +17,7 @@ func TestNetworkDisconnectErrors(t *testing.T) { expectedError string }{ { - expectedError: "requires exactly 2 arguments", + expectedError: "requires 2 arguments", }, { args: []string{"toto", "titi"}, diff --git a/cli/command/node/update_test.go b/cli/command/node/update_test.go index 09881d7ec2..52e771a9ae 100644 --- a/cli/command/node/update_test.go +++ b/cli/command/node/update_test.go @@ -20,11 +20,11 @@ func TestNodeUpdateErrors(t *testing.T) { expectedError string }{ { - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"node1", "node2"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"nodeID"}, diff --git a/cli/command/plugin/disable_test.go b/cli/command/plugin/disable_test.go index 40bfab72ea..3920efa101 100644 --- a/cli/command/plugin/disable_test.go +++ b/cli/command/plugin/disable_test.go @@ -19,11 +19,11 @@ func TestPluginDisableErrors(t *testing.T) { }{ { args: []string{}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"too", "many", "arguments"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"plugin-foo"}, diff --git a/cli/command/plugin/enable_test.go b/cli/command/plugin/enable_test.go index 20fa25f740..bb0861e6ce 100644 --- a/cli/command/plugin/enable_test.go +++ b/cli/command/plugin/enable_test.go @@ -20,11 +20,11 @@ func TestPluginEnableErrors(t *testing.T) { }{ { args: []string{}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"too-many", "arguments"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"plugin-foo"}, diff --git a/cli/command/secret/remove_test.go b/cli/command/secret/remove_test.go index 6b0dfcc2cd..b5f4889a8c 100644 --- a/cli/command/secret/remove_test.go +++ b/cli/command/secret/remove_test.go @@ -20,7 +20,7 @@ func TestSecretRemoveErrors(t *testing.T) { }{ { args: []string{}, - expectedError: "requires at least 1 argument.", + expectedError: "requires at least 1 argument", }, { args: []string{"foo"}, diff --git a/cli/command/service/rollback_test.go b/cli/command/service/rollback_test.go index 742d9f99c4..28e7f27c68 100644 --- a/cli/command/service/rollback_test.go +++ b/cli/command/service/rollback_test.go @@ -65,12 +65,12 @@ func TestRollbackWithErrors(t *testing.T) { }{ { name: "not-enough-args", - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "too-many-args", args: []string{"service-id-1", "service-id-2"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "service-does-not-exists", diff --git a/cli/command/stack/ps_test.go b/cli/command/stack/ps_test.go index e14002e210..009d51ed83 100644 --- a/cli/command/stack/ps_test.go +++ b/cli/command/stack/ps_test.go @@ -24,11 +24,11 @@ func TestStackPsErrors(t *testing.T) { }{ { args: []string{}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"foo", "bar"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { args: []string{"foo"}, diff --git a/cli/command/swarm/join_test.go b/cli/command/swarm/join_test.go index 9a60e9e25e..6d60d693d2 100644 --- a/cli/command/swarm/join_test.go +++ b/cli/command/swarm/join_test.go @@ -23,12 +23,12 @@ func TestSwarmJoinErrors(t *testing.T) { }{ { name: "not-enough-args", - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "too-many-args", args: []string{"remote1", "remote2"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "join-failed", diff --git a/cli/command/swarm/join_token_test.go b/cli/command/swarm/join_token_test.go index 21b89e6652..c97cc0012d 100644 --- a/cli/command/swarm/join_token_test.go +++ b/cli/command/swarm/join_token_test.go @@ -27,12 +27,12 @@ func TestSwarmJoinTokenErrors(t *testing.T) { }{ { name: "not-enough-args", - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "too-many-args", args: []string{"worker", "manager"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "invalid-args", diff --git a/cli/command/trust/key_generate_test.go b/cli/command/trust/key_generate_test.go index 9781a53384..74fc3fed68 100644 --- a/cli/command/trust/key_generate_test.go +++ b/cli/command/trust/key_generate_test.go @@ -26,12 +26,12 @@ func TestTrustKeyGenerateErrors(t *testing.T) { }{ { name: "not-enough-args", - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "too-many-args", args: []string{"key-1", "key-2"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, } diff --git a/cli/command/trust/key_load_test.go b/cli/command/trust/key_load_test.go index d32883fc62..4d9e737e08 100644 --- a/cli/command/trust/key_load_test.go +++ b/cli/command/trust/key_load_test.go @@ -34,13 +34,14 @@ func TestTrustKeyLoadErrors(t *testing.T) { }{ { name: "not-enough-args", - expectedError: "exactly 1 argument", + expectedError: "1 argument", + args: []string{}, expectedOutput: "", }, { name: "too-many-args", args: []string{"iamnotakey", "alsonotakey"}, - expectedError: "exactly 1 argument", + expectedError: "1 argument", expectedOutput: "", }, { diff --git a/cli/command/trust/revoke_test.go b/cli/command/trust/revoke_test.go index 7521a7bcf2..6e2ebd66bf 100644 --- a/cli/command/trust/revoke_test.go +++ b/cli/command/trust/revoke_test.go @@ -24,12 +24,12 @@ func TestTrustRevokeCommandErrors(t *testing.T) { }{ { name: "not-enough-args", - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "too-many-args", args: []string{"remote1", "remote2"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "sha-reference", diff --git a/cli/command/trust/sign_test.go b/cli/command/trust/sign_test.go index 8249ad893f..7e8d0a08e7 100644 --- a/cli/command/trust/sign_test.go +++ b/cli/command/trust/sign_test.go @@ -32,12 +32,12 @@ func TestTrustSignCommandErrors(t *testing.T) { }{ { name: "not-enough-args", - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "too-many-args", args: []string{"image", "tag"}, - expectedError: "requires exactly 1 argument", + expectedError: "requires 1 argument", }, { name: "sha-reference", diff --git a/cli/required.go b/cli/required.go index ba4e6e8efa..8d01887d0f 100644 --- a/cli/required.go +++ b/cli/required.go @@ -22,11 +22,10 @@ func NoArgs(cmd *cobra.Command, args []string) error { } return errors.Errorf( - "%q accepts no arguments.\nSee '%s --help'.\n\nUsage: %s\n\n%s", - cmd.CommandPath(), + "%[1]s: '%[2]s' accepts no arguments\n\nUsage: %[3]s\n\nRun '%[2]s --help' for more information", + binName(cmd), cmd.CommandPath(), cmd.UseLine(), - cmd.Short, ) } @@ -37,13 +36,12 @@ func RequiresMinArgs(min int) cobra.PositionalArgs { return nil } return errors.Errorf( - "%q requires at least %d %s.\nSee '%s --help'.\n\nUsage: %s\n\n%s", + "%[1]s: '%[2]s' requires at least %[3]d %[4]s\n\nUsage: %[5]s\n\nSee '%[2]s --help' for more information", + binName(cmd), cmd.CommandPath(), min, pluralize("argument", min), - cmd.CommandPath(), cmd.UseLine(), - cmd.Short, ) } } @@ -55,13 +53,12 @@ func RequiresMaxArgs(max int) cobra.PositionalArgs { return nil } return errors.Errorf( - "%q requires at most %d %s.\nSee '%s --help'.\n\nUsage: %s\n\n%s", + "%[1]s: '%[2]s' requires at most %[3]d %[4]s\n\nUsage: %[5]s\n\nSRun '%[2]s --help' for more information", + binName(cmd), cmd.CommandPath(), max, pluralize("argument", max), - cmd.CommandPath(), cmd.UseLine(), - cmd.Short, ) } } @@ -73,14 +70,13 @@ func RequiresRangeArgs(min int, max int) cobra.PositionalArgs { return nil } return errors.Errorf( - "%q requires at least %d and at most %d %s.\nSee '%s --help'.\n\nUsage: %s\n\n%s", + "%[1]s: '%[2]s' requires at least %[3]d and at most %[4]d %[5]s\n\nUsage: %[6]s\n\nRun '%[2]s --help' for more information", + binName(cmd), cmd.CommandPath(), min, max, pluralize("argument", max), - cmd.CommandPath(), cmd.UseLine(), - cmd.Short, ) } } @@ -92,13 +88,12 @@ func ExactArgs(number int) cobra.PositionalArgs { return nil } return errors.Errorf( - "%q requires exactly %d %s.\nSee '%s --help'.\n\nUsage: %s\n\n%s", + "%[1]s: '%[2]s' requires %[3]d %[4]s\n\nUsage: %[5]s\n\nRun '%[2]s --help' for more information", + binName(cmd), cmd.CommandPath(), number, pluralize("argument", number), - cmd.CommandPath(), cmd.UseLine(), - cmd.Short, ) } } diff --git a/cli/required_test.go b/cli/required_test.go index 9c5fc0b0c8..795fb4589a 100644 --- a/cli/required_test.go +++ b/cli/required_test.go @@ -18,7 +18,7 @@ func TestRequiresNoArgs(t *testing.T) { { args: []string{"foo"}, validateFunc: NoArgs, - expectedError: "accepts no arguments.", + expectedError: "accepts no arguments", }, } for _, tc := range testCases { @@ -36,12 +36,12 @@ func TestRequiresMinArgs(t *testing.T) { }, { validateFunc: RequiresMinArgs(1), - expectedError: "at least 1 argument.", + expectedError: "at least 1 argument", }, { args: []string{"foo"}, validateFunc: RequiresMinArgs(2), - expectedError: "at least 2 arguments.", + expectedError: "at least 2 arguments", }, } for _, tc := range testCases { @@ -60,12 +60,12 @@ func TestRequiresMaxArgs(t *testing.T) { { args: []string{"foo", "bar"}, validateFunc: RequiresMaxArgs(1), - expectedError: "at most 1 argument.", + expectedError: "at most 1 argument", }, { args: []string{"foo", "bar", "baz"}, validateFunc: RequiresMaxArgs(2), - expectedError: "at most 2 arguments.", + expectedError: "at most 2 arguments", }, } for _, tc := range testCases { @@ -88,12 +88,12 @@ func TestRequiresRangeArgs(t *testing.T) { { args: []string{"foo", "bar"}, validateFunc: RequiresRangeArgs(0, 1), - expectedError: "at most 1 argument.", + expectedError: "at most 1 argument", }, { args: []string{"foo", "bar", "baz"}, validateFunc: RequiresRangeArgs(0, 2), - expectedError: "at most 2 arguments.", + expectedError: "at most 2 arguments", }, { validateFunc: RequiresRangeArgs(1, 2), @@ -115,11 +115,11 @@ func TestExactArgs(t *testing.T) { }, { validateFunc: ExactArgs(1), - expectedError: "exactly 1 argument.", + expectedError: "1 argument", }, { validateFunc: ExactArgs(2), - expectedError: "exactly 2 arguments.", + expectedError: "2 arguments", }, } for _, tc := range testCases { diff --git a/cmd/docker/docker_test.go b/cmd/docker/docker_test.go index 84ea272a20..1998b77c99 100644 --- a/cmd/docker/docker_test.go +++ b/cmd/docker/docker_test.go @@ -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: 'docker invalid' is not a docker command.")) + assert.Check(t, is.ErrorContains(err, "docker: unknown command: docker invalid")) } func TestVersion(t *testing.T) { diff --git a/e2e/cli-plugins/flags_test.go b/e2e/cli-plugins/flags_test.go index 836af32e2a..c4e5fbcbc4 100644 --- a/e2e/cli-plugins/flags_test.go +++ b/e2e/cli-plugins/flags_test.go @@ -180,14 +180,14 @@ func TestCliPluginsVersion(t *testing.T) { args: []string{"version", "foo"}, expCode: 1, expOut: icmd.None, - expErr: `"docker version" accepts no arguments.`, + expErr: `docker: 'docker version' accepts no arguments`, }, { name: "global-with-plugin-arg", args: []string{"version", "helloworld"}, expCode: 1, expOut: icmd.None, - expErr: `"docker version" accepts no arguments.`, + expErr: `docker: 'docker version' accepts no arguments`, }, { name: "global-version-flag-with-unknown-arg",