various improvements to shell completions

thaJeztah commented 5 days ago

[x] follow-up to / stacked on https://github.com/docker/cli/pull/5232

cli/command/completion: add FileNames utility

This is just a convenience function to allow defining completion to use the default (complete with filenames and directories).

cli/command/completion: add EnvVarNames utility

EnvVarNames offers completion for environment-variable names. This completion can be used for "--env" and "--build-arg" flags, which allow obtaining the value of the given environment-variable if present in the local environment, so we only should complete the names of the environment variables, and not their value. This also prevents the completion script from printing values of environment variables containing sensitive values.

For example;

export MY_VAR=hello
docker run --rm --env MY_VAR alpine printenv MY_VAR
hello

Before this patch:

docker run --env GO
GO111MODULE=auto        GOLANG_VERSION=1.21.12  GOPATH=/go              GOTOOLCHAIN=local

With this patch:

docker run --env GO<tab>
GO111MODULE     GOLANG_VERSION  GOPATH          GOTOOLCHAIN

cli/command/completion: add FromList utility

It's an alias for cobra.FixedCompletions but takes a variadic list of strings, so that it's not needed to construct an array for this.

cli/command/container: provide flag-completion for "docker create"

"docker run" and "docker create" are mostly identical, so we can copy the same completion functions,

We could possibly create a utility for this (similar to addFlags() which configures both commands with the flags they share). I considered combining his with addFlags(), but that utility is also used in various tests, in which we don't need this feature, so keeping that for a future exercise.

cmd/docker: fix completion for --context

registerCompletionFuncForGlobalFlags was called from newDockerCommand, at which time no context-store is initialized yet, so it would return a nil value, probably resulting in store.Names to panic, but these errors are not shown when running the completion. As a result, the flag completion would fall back to completing from filenames.

This patch changes the function to dynamically get the context-store; this fixes the problem mentioned above, because at the time the completion function is invoked, the CLI is fully initialized, and does have a context-store available.

A (non-exported) interface is defined to allow the function to accept alternative implementations (not requiring a full command.DockerCLI).

Before this patch:

docker context create one
docker context create two

docker --context <TAB>
.DS_Store                   .idea/                      Makefile
.dockerignore               .mailmap                    build/
...

With this patch:

docker context create one
docker context create two

docker --context <TAB>
default  one      two

cli/context/store: Names(): fix panic when called with nil-interface

Before this, it would panic when a nil-interface was passed.

cli/command/container: add completion for --cap-add, --cap-drop

With this patch:

docker run --cap-add <TAB>
ALL                     CAP_KILL                CAP_SETUID
CAP_AUDIT_CONTROL       CAP_LEASE               CAP_SYSLOG
CAP_AUDIT_READ          CAP_LINUX_IMMUTABLE     CAP_SYS_ADMIN
CAP_AUDIT_WRITE         CAP_MAC_ADMIN           CAP_SYS_BOOT
CAP_BLOCK_SUSPEND       CAP_MAC_OVERRIDE        CAP_SYS_CHROOT
CAP_BPF                 CAP_MKNOD               CAP_SYS_MODULE
CAP_CHECKPOINT_RESTORE  CAP_NET_ADMIN           CAP_SYS_NICE
CAP_CHOWN               CAP_NET_BIND_SERVICE    CAP_SYS_PACCT
CAP_DAC_OVERRIDE        CAP_NET_BROADCAST       CAP_SYS_PTRACE
CAP_DAC_READ_SEARCH     CAP_NET_RAW             CAP_SYS_RAWIO
CAP_FOWNER              CAP_PERFMON             CAP_SYS_RESOURCE
CAP_FSETID              CAP_SETFCAP             CAP_SYS_TIME
CAP_IPC_LOCK            CAP_SETGID              CAP_SYS_TTY_CONFIG
CAP_IPC_OWNER           CAP_SETPCAP             CAP_WAKE_ALARM

cli/command/container: add completion for --restart

With this patch:

docker run --restart <TAB>
always  no  on-failure  unless-stopped

cli/command/container: add completion for --volumes-from

With this patch:

docker run --volumes-from amazing_nobel
amazing_cannon     boring_wozniak         determined_banzai
elegant_solomon    reverent_booth         amazing_nobel

cli/command/container: add completion for --stop-signal

With this patch:

docker run --stop-signal <TAB>
ABRT  IOT      RTMAX-4   RTMIN     RTMIN+11  TSTP
ALRM  KILL     RTMAX-5   RTMIN+1   RTMIN+12  TTIN
BUS   PIPE     RTMAX-6   RTMIN+2   RTMIN+13  TTOU
CHLD  POLL     RTMAX-7   RTMIN+3   RTMIN+14  URG
CLD   PROF     RTMAX-8   RTMIN+4   RTMIN+15  USR1
CONT  PWR      RTMAX-9   RTMIN+5   SEGV      USR2
FPE   QUIT     RTMAX-10  RTMIN+6   STKFLT    VTALRM
HUP   RTMAX    RTMAX-11  RTMIN+7   STOP      WINCH
ILL   RTMAX-1  RTMAX-12  RTMIN+8   SYS       XCPU
INT   RTMAX-2  RTMAX-13  RTMIN+9   TERM      XFSZ
IO    RTMAX-3  RTMAX-14  RTMIN+10  TRAP

- Description for the changelog

add and improve shell completions for various flags

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

codecov-commenter commented 5 days ago

Codecov Report

Attention: Patch coverage is 58.13953% with 18 lines in your changes missing coverage. Please review.

Project coverage is 61.52%. Comparing base (cfbf88f) to head (0e18564).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #5238 +/- ## ========================================== + Coverage 61.49% 61.52% +0.03% ========================================== Files 298 299 +1 Lines 20811 20818 +7 ========================================== + Hits 12797 12809 +12 Misses 7102 7102 + Partials 912 907 -5 ```

thaJeztah commented 5 days ago

Ah! Forgot to remove something I previously used. Also looks like we're running some deprecated linters


#16 0.209 level=warning msg="[lintersdb] The linter named \"megacheck\" is deprecated. It has been split into: gosimple, staticcheck, unused."
#16 0.209 level=warning msg="[lintersdb] The name \"vet\" is deprecated. The linter has been renamed to: govet."
#16 65.71 cli/command/completion/functions.go:123:2: ineffectual assignment to toComplete (ineffassign)
#16 65.71   toComplete = strings.ToLower(toComplete)
#16 65.71   ^

thaJeztah commented 9 hours ago

Thanks for reviewing! I'll have a look at fixing the typo when I'm near my computer.

Also; wrt the "disable descriptions" https://github.com/docker/cli/pull/5238#discussion_r1666894131 it looks like buildx DOES enable them for its completion, so maybe we should enable it in our code as well; there may be some commands that could use a slight touch-up for those as some have descriptions that are a bit confusing, so still best to do in a follow up

docker / cli