images: print hint when invoking "docker images" with ambiguous argument #4849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #4849 +/- ##
==========================================
+ Coverage 59.58% 61.28% +1.69%
==========================================
Files 288 287 -1
Lines 24821 20038 -4783
==========================================
- Hits 14790 12280 -2510
+ Misses 9145 6866 -2279
- Partials 886 892 +6 |
cpuguy83
approved these changes
Feb 4, 2024
| 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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
@laurazard @jalonsogo ptal 🤗 |
vvoland
approved these changes
Feb 5, 2024
|
Thanks all! Let me bring this in; we can tweak the hint if we want to in follow ups. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
docker volumesalias #4601docker volumesdoes not exist #4600cli/command/images: runImages: inline intermediate var
cli/command/images: runImages: use proper camel-case for dockerCLI
images: print hint when invoking "docker images" with ambiguous argument
The
docker imagestop-level subcommand predates thedocker <object> <verb>convention (e.g.
docker image ls), but accepts a positional argument tosearch/filter images by name (globbing). It's common for users to accidentally
mistake these commands, and to use (e.g.)
docker images ls, expectingto 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:
With this patch applied:
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)