Improve completion of containers for docker rm#5527
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5527 +/- ##
==========================================
- Coverage 60.67% 60.67% -0.01%
==========================================
Files 345 345
Lines 23488 23491 +3
==========================================
Hits 14252 14252
- Misses 8263 8266 +3
Partials 973 973 |
1c6e8d0 to
475be86
Compare
Signed-off-by: Harald Albers <github@albersweb.de>
475be86 to
147630a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
| }, | ||
| ValidArgsFunction: completion.ContainerNames(dockerCli, true), | ||
| ValidArgsFunction: completion.ContainerNames(dockerCli, true, func(ctr container.Summary) bool { | ||
| return opts.force || ctr.State == "exited" || ctr.State == "created" |
There was a problem hiding this comment.
This is totally fine for now, but I wish we had an enum (or whatever passes for one in Go) for the State.
There was a problem hiding this comment.
Ah, yes! I think I may have started working on a branch at some point, but need to dig it up. It's all a bit awkward as these parts are rather quirky, and these values are currently set in the container package, not in anything inside the api/xxx or client/xxx; https://github.com/moby/moby/blob/61030f0e87c8d893082599815897f10d440f9bda/container/state.go#L119-L158
// StateString returns a single string to describe state
func (s *State) StateString() string {
if s.Running {
if s.Paused {
return "paused"
}
if s.Restarting {
return "restarting"
}
return "running"
}
if s.RemovalInProgress {
return "removing"
}
if s.Dead {
return "dead"
}
if s.StartedAt.IsZero() {
return "created"
}
return "exited"
}
// IsValidStateString checks if the provided string is a valid container state or not.
func IsValidStateString(s string) bool {
if s != "paused" &&
s != "restarting" &&
s != "removing" &&
s != "running" &&
s != "dead" &&
s != "created" &&
s != "exited" {
return false
}
return true
}There was a problem hiding this comment.
I recall now that I also wanted to look at these completion helpers to allow passing filters so that the daemon would only return containers in specific state(s) to get the equivalent of;
docker ps --format='{{.Names}} {{.Status}}' --filter status=exited --filter status=created
container-1 created
container-2 exited
container-3 exited(and perhaps add a != option for status to allow --filter status!=running)
- What I did
The cobra completion for
docker rmcompletes all containers. This does not make much sense as not all containers are removable unless--forceis given.This PR aligns the completion to the legacy completion, where only removable containers are completed unless
--forceis given, see here.- How I did it
Added a filter to the call of
completion.ContainerNamesthat filters on container status and the value of--force.- How to verify it
- Description for the changelog
@thaJeztah PTAL