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 1 commit
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
Prev Previous commit
images: print hint when invoking "docker images" with ambiguous argument
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.

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 <github@gone.nl>
  • Loading branch information
thaJeztah committed Feb 3, 2024
commit 809eb8cdeea7e053e360561e6cef33526f580655
48 changes: 47 additions & 1 deletion 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,6 +23,7 @@ type imagesOptions struct {
showDigests bool
format string
filter opts.FilterOpt
calledAs string
}

// NewImagesCommand creates a new `docker images` command
Expand All @@ -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{
Expand Down Expand Up @@ -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 {
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