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] Completion for events --filter #5614

Merged

Conversation

thaJeztah
Copy link
Member


Adds completion for docker events --filter.

There are different types of filters, requiring different completion logic.
Some filters have static values, others require API lookups.
In some cases, trailing spaces of intermediate completions have to be suppressed.

The completion logic is complex because cobra does not offer support for compound flag values.
The handler for the --filter flag serves as the central entrypoint to all completions.

How to verify:
in a dev container, run make completion, then trigger completions, e.g.

$ docker events --filter <TAB><TAB>
container=  event=      label=      node=       type=       
daemon=     image=      network=    scope=      volume= 
$ docker events --filter event=c<TAB><TAB>
checkpoint  commit      connect     copy        create
$ docker events --filter image=<TAB><TAB>
docker-cli-dev:latest  docker-cli-e2e:latest  registry:2

The completions should correspond to those of the legacy bash completion.

@codecov-commenter
Copy link

codecov-commenter commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 70.37037% with 32 lines in your changes missing coverage. Please review.

Project coverage is 58.41%. Comparing base (9128f7b) to head (8caf347).
Report is 17 commits behind head on 27.x.

Additional details and impacted files
@@            Coverage Diff             @@
##             27.x    #5614      +/-   ##
==========================================
+ Coverage   58.38%   58.41%   +0.02%     
==========================================
  Files         336      337       +1     
  Lines       28781    28899     +118     
==========================================
+ Hits        16805    16881      +76     
- Misses      11009    11050      +41     
- Partials      967      968       +1     

@thaJeztah
Copy link
Member Author

OH! this needs some minor patching for this branch;

63.76 cli/command/system/cmd.go:1: : # github.com/docker/cli/cli/command/system [github.com/docker/cli/cli/command/system.test]
63.76 cli/command/system/client_test.go:22:79: undefined: container.Summary
63.76 cli/command/system/client_test.go:38:103: undefined: container.Summary
63.76 cli/command/system/client_test.go:42:21: undefined: container.Summary
63.76 cli/command/system/completion_test.go:30:86: undefined: container.Summary
63.76 cli/command/system/completion_test.go:31:25: undefined: container.Summary
63.76 cli/command/system/completion_test.go:42:86: undefined: container.Summary (typecheck)
63.76 package system

@laurazard
Copy link
Member

It's a small nit, but I wonder if we should squash your changes adjusting the types into the original commit that introduced them – otherwise if we have to revert them or do anything we need to remember to move them both together because one won't build without the other.

WDYT @thaJeztah?

@thaJeztah
Copy link
Member Author

Ah, yes, I'm good with squashing; with these I'm always leaning two ways; "is it clearer to have a separate commit showing this is what we modified" or (also for bisecting) "this is the backport (but it's modified)".

Let me squash the commits, as I think that's what we usually did.

Signed-off-by: Harald Albers <[email protected]>
(cherry picked from commit d4f4cf1)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Some small adjustments for this branch due to some times being renamed
in master;

    63.76 cli/command/system/cmd.go:1: : # github.com/docker/cli/cli/command/system [github.com/docker/cli/cli/command/system.test]
    63.76 cli/command/system/client_test.go:22:79: undefined: container.Summary
    63.76 cli/command/system/client_test.go:38:103: undefined: container.Summary
    63.76 cli/command/system/client_test.go:42:21: undefined: container.Summary
    63.76 cli/command/system/completion_test.go:30:86: undefined: container.Summary
    63.76 cli/command/system/completion_test.go:31:25: undefined: container.Summary
    63.76 cli/command/system/completion_test.go:42:86: undefined: container.Summary (typecheck)

Signed-off-by: Harald Albers <[email protected]>
(cherry picked from commit e1c5180)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the 27.x_backport_completion-events--filter branch from 4430362 to 8caf347 Compare November 14, 2024 13:55
@thaJeztah
Copy link
Member Author

Squashed! ❤️

@laurazard
Copy link
Member

Looks great, thanks!

@laurazard laurazard merged commit 929e861 into docker:27.x Nov 14, 2024
87 checks passed
@thaJeztah thaJeztah deleted the 27.x_backport_completion-events--filter branch November 14, 2024 15:47
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