From 1328bb3381e99ab4402cd78dfe71d878510f907a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 3 Feb 2024 17:54:23 +0100 Subject: [PATCH 1/3] cli/command/images: runImages: inline intermediate var Signed-off-by: Sebastiaan van Stijn --- cli/command/image/list.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cli/command/image/list.go b/cli/command/image/list.go index 887ca9c9a4..b0df7aa2a1 100644 --- a/cli/command/image/list.go +++ b/cli/command/image/list.go @@ -68,12 +68,10 @@ func runImages(ctx context.Context, dockerCli command.Cli, options imagesOptions filters.Add("reference", options.matchName) } - listOptions := image.ListOptions{ + images, err := dockerCli.Client().ImageList(ctx, image.ListOptions{ All: options.all, Filters: filters, - } - - images, err := dockerCli.Client().ImageList(ctx, listOptions) + }) if err != nil { return err } From b158181a1d500633a0d8ecdf0061e2ed699bf6ed Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 3 Feb 2024 17:56:06 +0100 Subject: [PATCH 2/3] cli/command/images: runImages: use proper camel-case for dockerCLI Signed-off-by: Sebastiaan van Stijn --- cli/command/image/list.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cli/command/image/list.go b/cli/command/image/list.go index b0df7aa2a1..a52db38261 100644 --- a/cli/command/image/list.go +++ b/cli/command/image/list.go @@ -24,7 +24,7 @@ type imagesOptions struct { } // NewImagesCommand creates a new `docker images` command -func NewImagesCommand(dockerCli command.Cli) *cobra.Command { +func NewImagesCommand(dockerCLI command.Cli) *cobra.Command { options := imagesOptions{filter: opts.NewFilterOpt()} cmd := &cobra.Command{ @@ -35,7 +35,7 @@ func NewImagesCommand(dockerCli command.Cli) *cobra.Command { if len(args) > 0 { options.matchName = args[0] } - return runImages(cmd.Context(), dockerCli, options) + return runImages(cmd.Context(), dockerCLI, options) }, Annotations: map[string]string{ "category-top": "7", @@ -55,20 +55,20 @@ func NewImagesCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func newListCommand(dockerCli command.Cli) *cobra.Command { - cmd := *NewImagesCommand(dockerCli) +func newListCommand(dockerCLI command.Cli) *cobra.Command { + cmd := *NewImagesCommand(dockerCLI) cmd.Aliases = []string{"list"} cmd.Use = "ls [OPTIONS] [REPOSITORY[:TAG]]" return &cmd } -func runImages(ctx context.Context, dockerCli command.Cli, options imagesOptions) error { +func runImages(ctx context.Context, dockerCLI command.Cli, options imagesOptions) error { filters := options.filter.Value() if options.matchName != "" { filters.Add("reference", options.matchName) } - images, err := dockerCli.Client().ImageList(ctx, image.ListOptions{ + images, err := dockerCLI.Client().ImageList(ctx, image.ListOptions{ All: options.all, Filters: filters, }) @@ -78,8 +78,8 @@ func runImages(ctx context.Context, dockerCli command.Cli, options imagesOptions format := options.format if len(format) == 0 { - if len(dockerCli.ConfigFile().ImagesFormat) > 0 && !options.quiet { - format = dockerCli.ConfigFile().ImagesFormat + if len(dockerCLI.ConfigFile().ImagesFormat) > 0 && !options.quiet { + format = dockerCLI.ConfigFile().ImagesFormat } else { format = formatter.TableFormatKey } @@ -87,7 +87,7 @@ func runImages(ctx context.Context, dockerCli command.Cli, options imagesOptions imageCtx := formatter.ImageContext{ Context: formatter.Context{ - Output: dockerCli.Out(), + Output: dockerCLI.Out(), Format: formatter.NewImageFormat(format, options.quiet, options.showDigests), Trunc: !options.noTrunc, }, From 809eb8cdeea7e053e360561e6cef33526f580655 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 3 Feb 2024 17:57:48 +0100 Subject: [PATCH 3/3] images: print hint when invoking "docker images" with ambiguous argument The `docker images` top-level subcommand predates the `docker ` convention (e.g. `docker image ls`), but accepts a positional argument to search/filter images by name (globbing). It's common for users to accidentally mistake these commands, and to use (e.g.) `docker images ls`, expecting to see all images, but ending up with an empty list because no image named "ls" was found. Disallowing these search-terms would be a breaking change, but we can print and informational message to help the users correct their mistake. Before this patch: docker images ls REPOSITORY TAG IMAGE ID CREATED SIZE With this patch applied: docker images ls REPOSITORY TAG IMAGE ID CREATED SIZE No images found matching "ls": did you mean "docker image ls"? Signed-off-by: Sebastiaan van Stijn --- cli/command/image/list.go | 48 ++++++++++++++++++- cli/command/image/list_test.go | 14 ++++++ .../testdata/list-command-ambiguous.golden | 2 + 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 cli/command/image/testdata/list-command-ambiguous.golden diff --git a/cli/command/image/list.go b/cli/command/image/list.go index a52db38261..a691efed45 100644 --- a/cli/command/image/list.go +++ b/cli/command/image/list.go @@ -2,6 +2,8 @@ package image import ( "context" + "fmt" + "io" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -21,6 +23,7 @@ type imagesOptions struct { showDigests bool format string filter opts.FilterOpt + calledAs string } // NewImagesCommand creates a new `docker images` command @@ -35,6 +38,10 @@ func NewImagesCommand(dockerCLI command.Cli) *cobra.Command { if len(args) > 0 { options.matchName = args[0] } + // Pass through how the command was invoked. We use this to print + // warnings when an ambiguous argument was passed when using the + // legacy (top-level) "docker images" subcommand. + options.calledAs = cmd.CalledAs() return runImages(cmd.Context(), dockerCLI, options) }, Annotations: map[string]string{ @@ -93,5 +100,44 @@ func runImages(ctx context.Context, dockerCLI command.Cli, options imagesOptions }, Digest: options.showDigests, } - return formatter.ImageWrite(imageCtx, images) + if err := formatter.ImageWrite(imageCtx, images); err != nil { + return err + } + if options.matchName != "" && len(images) == 0 && options.calledAs == "images" { + printAmbiguousHint(dockerCLI.Err(), options.matchName) + } + return nil +} + +// printAmbiguousHint prints an informational warning if the provided filter +// argument is ambiguous. +// +// The "docker images" top-level subcommand predates the "docker " +// convention (e.g. "docker image ls"), but accepts a positional argument to +// search/filter images by name (globbing). It's common for users to accidentally +// mistake these commands, and to use (e.g.) "docker images ls", expecting +// to see all images, but ending up with an empty list because no image named +// "ls" was found. +// +// Disallowing these search-terms would be a breaking change, but we can print +// and informational message to help the users correct their mistake. +func printAmbiguousHint(stdErr io.Writer, matchName string) { + switch matchName { + // List of subcommands for "docker image" and their aliases (see "docker image --help"): + case "build", + "history", + "import", + "inspect", + "list", + "load", + "ls", + "prune", + "pull", + "push", + "rm", + "save", + "tag": + + _, _ = fmt.Fprintf(stdErr, "\nNo images found matching %q: did you mean \"docker image %[1]s\"?\n", matchName) + } } diff --git a/cli/command/image/list_test.go b/cli/command/image/list_test.go index d8b583d8d4..2b5259b1f5 100644 --- a/cli/command/image/list_test.go +++ b/cli/command/image/list_test.go @@ -95,3 +95,17 @@ func TestNewListCommandAlias(t *testing.T) { assert.Check(t, cmd.HasAlias("list")) assert.Check(t, !cmd.HasAlias("other")) } + +func TestNewListCommandAmbiguous(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + cmd := NewImagesCommand(cli) + cmd.SetOut(io.Discard) + + // Set the Use field to mimic that the command was called as "docker images", + // not "docker image ls". + cmd.Use = "images" + cmd.SetArgs([]string{"ls"}) + err := cmd.Execute() + assert.NilError(t, err) + golden.Assert(t, cli.ErrBuffer().String(), "list-command-ambiguous.golden") +} diff --git a/cli/command/image/testdata/list-command-ambiguous.golden b/cli/command/image/testdata/list-command-ambiguous.golden new file mode 100644 index 0000000000..2d8ffa9efa --- /dev/null +++ b/cli/command/image/testdata/list-command-ambiguous.golden @@ -0,0 +1,2 @@ + +No images found matching "ls": did you mean "docker image ls"?