Merge pull request #5234 from thaJeztah/nicer_missing_commands

cli: improve output and consistency for unknown (sub)commands
This commit is contained in:
Sebastiaan van Stijn 2024-07-17 01:22:03 +02:00 committed by GitHub
commit ce4469a696
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
41 changed files with 91 additions and 84 deletions

View File

@ -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

View File

@ -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"},

View File

@ -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"},

View File

@ -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"},

View File

@ -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)},

View File

@ -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"},

View File

@ -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",

View File

@ -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",

View File

@ -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 {

View File

@ -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",

View File

@ -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",

View File

@ -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",

View File

@ -23,7 +23,7 @@ func TestNewPullCommandErrors(t *testing.T) {
}{
{
name: "wrong-args",
expectedError: "requires exactly 1 argument.",
expectedError: "requires 1 argument",
args: []string{},
},
{

View File

@ -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",

View File

@ -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",

View File

@ -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",

View File

@ -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)

View File

@ -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"},

View File

@ -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"},

View File

@ -19,7 +19,7 @@ func TestNetworkConnectErrors(t *testing.T) {
expectedError string
}{
{
expectedError: "requires exactly 2 arguments",
expectedError: "requires 2 arguments",
},
{
args: []string{"toto", "titi"},

View File

@ -21,7 +21,7 @@ func TestNetworkCreateErrors(t *testing.T) {
expectedError string
}{
{
expectedError: "exactly 1 argument",
expectedError: "1 argument",
},
{
args: []string{"toto"},

View File

@ -17,7 +17,7 @@ func TestNetworkDisconnectErrors(t *testing.T) {
expectedError string
}{
{
expectedError: "requires exactly 2 arguments",
expectedError: "requires 2 arguments",
},
{
args: []string{"toto", "titi"},

View File

@ -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"},

View File

@ -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"},

View File

@ -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"},

View File

@ -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"},

View File

@ -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",

View File

@ -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"},

View File

@ -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",

View File

@ -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",

View File

@ -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",
},
}

View File

@ -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: "",
},
{

View File

@ -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",

View File

@ -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",

View File

@ -1,8 +1,6 @@
package cli
import (
"strings"
"github.com/pkg/errors"
"github.com/spf13/cobra"
)
@ -14,15 +12,20 @@ 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(
"%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,
)
}
@ -33,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,
)
}
}
@ -51,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,
)
}
}
@ -69,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,
)
}
}
@ -88,17 +88,21 @@ 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,
)
}
}
// 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 {

View File

@ -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 {

View File

@ -82,7 +82,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)

View File

@ -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: unknown command: docker invalid"))
}
func TestVersion(t *testing.T) {

View File

@ -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",

View File

@ -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

View File

@ -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