Skip to content

cli/container: use github.com/moby/sys/capability for completions - #5480

Merged
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:moby_sys_capability
Oct 1, 2024
Merged

cli/container: use github.com/moby/sys/capability for completions#5480
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:moby_sys_capability

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

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).

- What I did

- How I did it

- How to verify it

- Description for the changelog

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

@thaJeztah thaJeztah added this to the 28.0.0 milestone Sep 26, 2024
@thaJeztah thaJeztah self-assigned this Sep 26, 2024
@codecov-commenter

codecov-commenter commented Sep 26, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.07%. Comparing base (f4fab2c) to head (462e082).
Report is 70 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5480      +/-   ##
==========================================
+ Coverage   60.05%   60.07%   +0.02%     
==========================================
  Files         345      345              
  Lines       23440    23447       +7     
==========================================
+ Hits        14076    14085       +9     
+ Misses       8390     8388       -2     
  Partials      974      974              
Comment thread cli/command/container/completion.go
@thaJeztah
thaJeztah force-pushed the moby_sys_capability branch from fb9858c to 7f0b119 Compare October 1, 2024 12:00
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>
@thaJeztah
thaJeztah force-pushed the moby_sys_capability branch from 7f0b119 to 462e082 Compare October 1, 2024 12:01
@thaJeztah

Copy link
Copy Markdown
Member Author

@laurazard updated; I added a minimal unit-test. Perhaps wouldn't hurt to add tests for some of the other ones as well (but didn't want to stuff those in this PR).

For the "case-insensitive" auto-complete, I also want to do some experimenting in follow-ups. Related to that, I opened a PR to normalize and de-duplicate values in the client before we send the request;

@laurazard

Copy link
Copy Markdown
Member

Thanks! LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment