Skip to content

[27.x backport] assorted improvements for shell completion#5540

Merged
vvoland merged 15 commits intodocker:27.xfrom
thaJeztah:27.x_backport_completion_improvements
Oct 18, 2024
Merged

[27.x backport] assorted improvements for shell completion#5540
vvoland merged 15 commits intodocker:27.xfrom
thaJeztah:27.x_backport_completion_improvements

Conversation

@thaJeztah
Copy link
Member

backports:

- Description for the changelog

- Improve shell-completion of containers for `docker rm` [docker/cli#5527](https://github.com/docker/cli/pull/5527)
- Add shell-completion for `--platform` flags [docker/cli#5516](https://github.com/docker/cli/pull/5516)
- 

- A picture of a cute animal (not mandatory but encouraged)

We used a hard-coded list of capabilities that we copied from containerd,
but the new "capability" package allows use to have a maintained list
of capabilities.

There's likely still some improvements to be made;

First of all, the capability package could provide a function to get the list
of strings.

On the completion-side, we need to consider what format is most convenient;
currently we use the canonical name (uppercase and "CAP_" prefix), however,
tab-completion is case-sensitive by default, so requires the user to type
uppercase letters to filter the list of options.

Bash completion provides a `completion-ignore-case on` option to make completion
case-insensitive (https://askubuntu.com/a/87066), but it looks to be a global
option; the current cobra.CompletionOptions also don't provide this as an option
to be used in the generated completion-script.

Fish completion has `smartcase` (by default?) which matches any case if
all of the input is lowercase.

Zsh does not have a dedicated option, but allows setting matching-rules
(see https://superuser.com/a/1092328).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 462e082)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit d49e72c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add a utility for completing platform strings.

Platforms offers completion for platform-strings. It provides a non-exhaustive
list of platforms to be used for completion. Platform-strings are based on
[runtime.GOOS] and [runtime.GOARCH], but with (optional) variants added. A
list of recognised os/arch combinations from the Go runtime can be obtained
through "go tool dist list".

Some noteworthy exclusions from this list:

  - arm64 images ("windows/arm64", "windows/arm64/v8") do not yet exist for windows.
  - we don't (yet) include `os-variant` for completion (as can be used for Windows images)
  - we don't (yet) include platforms for which we don't build binaries, such as
    BSD platforms (freebsd, netbsd, openbsd), android, macOS (darwin).
  - we currently exclude architectures that may have unofficial builds,
    but don't have wide adoption (and no support), such as loong64, mipsXXX,
    ppc64 (non-le) to prevent confusion.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit ce1aebc)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With this patch, completion is provided for `--platform` flags:

    docker run --platform<TAB>
    linux           linux/amd64     linux/arm/v5    linux/arm/v7    linux/arm64/v8  linux/riscv64   wasip1          windows
    linux/386       linux/arm       linux/arm/v6    linux/arm64     linux/ppc64le   linux/s390x     wasip1/wasm     windows/amd64

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8c7f713)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With this patch, completion is provided for `--platform` flags:

    docker pull --platform<TAB>
    linux           linux/amd64     linux/arm/v5    linux/arm/v7    linux/arm64/v8  linux/riscv64   wasip1          windows
    linux/386       linux/arm       linux/arm/v6    linux/arm64     linux/ppc64le   linux/s390x     wasip1/wasm     windows/amd64

Note that `docker buildx build` (with BuildKit) does not yet provide completion;
it's provided through buildx, and uses a different format (accepting multiple
comma-separated platforms). Interestingly, tab-completion for `docker build`
currently uses completion for non-buildkit, and has some other issues that may
have to be looked into.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit d42cf96)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b8cddc6)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8f2e566)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a5ca5b3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5171319)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit be197da)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Had to apply a small patch in the last commit due to the master branch using a type that was moved;

44.74 # github.com/docker/cli/cli/command/container
44.74 cli/command/container/rm.go:41:84: undefined: container.Summary
diff --git a/cli/command/container/rm.go b/cli/command/container/rm.go
index 12beef64f..b234b6009 100644
--- a/cli/command/container/rm.go
+++ b/cli/command/container/rm.go
@@ -8,6 +8,7 @@ import (
        "github.com/docker/cli/cli"
        "github.com/docker/cli/cli/command"
        "github.com/docker/cli/cli/command/completion"
+       "github.com/docker/docker/api/types"
        "github.com/docker/docker/api/types/container"
        "github.com/docker/docker/errdefs"
        "github.com/pkg/errors"
@@ -38,7 +39,7 @@ func NewRmCommand(dockerCli command.Cli) *cobra.Command {
                Annotations: map[string]string{
                        "aliases": "docker container rm, docker container remove, docker rm",
                },
-               ValidArgsFunction: completion.ContainerNames(dockerCli, true, func(ctr container.Summary) bool {
+               ValidArgsFunction: completion.ContainerNames(dockerCli, true, func(ctr types.Container) bool {
                        return opts.force || ctr.State == "exited" || ctr.State == "created"
                }),
        }
@thaJeztah thaJeztah self-assigned this Oct 17, 2024
@thaJeztah
Copy link
Member Author

LOL "surprise" there were more places to do the same 😂

65.03 cli/command/completion/functions_test.go:43:21: undefined: container.Summary
65.03 cli/command/completion/functions_test.go:71:37: undefined: container.Summary
65.03 cli/command/completion/functions_test.go:72:32: undefined: container.Summary
65.03 cli/command/completion/functions_test.go:84:28: undefined: container.Summary
65.03 cli/command/completion/functions_test.go:97:28: undefined: container.Summary
65.03 cli/command/completion/functions_test.go:109:28: undefined: container.Summary
65.03 cli/command/completion/functions_test.go:118:30: undefined: container.Summary
65.03 cli/command/completion/functions_test.go:119:30: undefined: container.Summary
@codecov-commenter
Copy link

codecov-commenter commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 58.65%. Comparing base (2668b11) to head (b018e55).
Report is 17 commits behind head on 27.x.

Additional details and impacted files
@@            Coverage Diff             @@
##             27.x    #5540      +/-   ##
==========================================
- Coverage   59.77%   58.65%   -1.13%     
==========================================
  Files         345      345              
  Lines       23443    29042    +5599     
==========================================
+ Hits        14014    17035    +3021     
- Misses       8454    11036    +2582     
+ Partials      975      971       -4     
thaJeztah and others added 5 commits October 17, 2024 23:42
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit f3b4094)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit ab418a3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 302d73f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit e1c472a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Harald Albers <github@albersweb.de>
(cherry picked from commit 147630a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the 27.x_backport_completion_improvements branch from de6ca8d to b018e55 Compare October 17, 2024 21:43
@vvoland vvoland merged commit 4241e00 into docker:27.x Oct 18, 2024
@thaJeztah thaJeztah deleted the 27.x_backport_completion_improvements branch October 18, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment