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

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

Merged
merged 15 commits into from
Oct 18, 2024

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 <[email protected]>
(cherry picked from commit 462e082)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit d49e72c)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
(cherry picked from commit ce1aebc)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
(cherry picked from commit 8c7f713)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
(cherry picked from commit d42cf96)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit b8cddc6)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 8f2e566)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit a5ca5b3)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 5171319)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit be197da)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@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 <[email protected]>
(cherry picked from commit f3b4094)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit ab418a3)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 302d73f)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit e1c472a)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Harald Albers <[email protected]>
(cherry picked from commit 147630a)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@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
87 checks passed
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants