Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

images: print hint when invoking "docker images" with ambiguous argument #4849

Merged
merged 3 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 57 additions & 13 deletions cli/command/image/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package image

import (
"context"
"fmt"
"io"

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
Expand All @@ -21,10 +23,11 @@ type imagesOptions struct {
showDigests bool
format string
filter opts.FilterOpt
calledAs string
}

// 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{
Expand All @@ -35,7 +38,11 @@ func NewImagesCommand(dockerCli command.Cli) *cobra.Command {
if len(args) > 0 {
options.matchName = args[0]
}
return runImages(cmd.Context(), dockerCli, options)
// 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{
"category-top": "7",
Expand All @@ -55,45 +62,82 @@ 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)
}

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
}

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

imageCtx := formatter.ImageContext{
Context: formatter.Context{
Output: dockerCli.Out(),
Output: dockerCLI.Out(),
Format: formatter.NewImageFormat(format, options.quiet, options.showDigests),
Trunc: !options.noTrunc,
},
Digest: options.showDigests,
}
return formatter.ImageWrite(imageCtx, images)
if err := formatter.ImageWrite(imageCtx, images); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why print the empty image list before checking if its ambiguous?
I guess it can make it a little more obvious what could be happening.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I continued treating as a regular search that didn't return a result. eg, a search for an image named build could still be legit, so keeping the behavior as usual, just the extra message on stderr

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 <object> <verb>"
// 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)
}
}
14 changes: 14 additions & 0 deletions cli/command/image/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
2 changes: 2 additions & 0 deletions cli/command/image/testdata/list-command-ambiguous.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

No images found matching "ls": did you mean "docker image ls"?
Loading