Skip to content

Commit 572e3f1

Browse files
committed
opts: ListOpts: implement cobra.SliceValue to fix shell completion
Cobra's shell completion has specific rules to decide whether a flag can be accepted multiple times. If a flag does not meet that rule, it only completes the flag name once; some of those rules depend on the "type" of the option to end with "Array" or "Slice", which most of our options don't. Starting with Cobra 1.9, it also checks whether an option implements the [cobra.SliceValue] interface (see [spf13/cobra 2210]). This patch implements the [cobra.SliceValue] interface on ListOpts, so that these options can be completed multiple times. In a follow-up, we can update our code to replace our uses of `GetAll()`, which is identical with the `GetSlice()` method, and potentially deprecate the old method. Before this patch, ListOpts would only be completed once when completing flag names. For example, the following would show the `--label` flag the first time, but omit it if a `--label` flag was already set; docker run--l<TAB> --label (Set meta data on a container) --link-local-ip (Container IPv4/IPv6 link-local addresses) --label-file (Read in a line delimited file of labels) --log-driver (Logging driver for the container) --link (Add link to another container) --log-opt (Log driver options) docker run --label hello --l<TAB> --label-file (Read in a line delimited file of labels) --link-local-ip (Container IPv4/IPv6 link-local addresses) --log-opt (Log driver options) --link (Add link to another container) --log-driver (Logging driver for the container) With this patch, the completion script correctly identifies the `--label` flag to be accepted multiple times, and also completes it when already set; docker run --label hello --l<TAB> --label (Set meta data on a container) --link-local-ip (Container IPv4/IPv6 link-local addresses) --label-file (Read in a line delimited file of labels) --log-driver (Logging driver for the container) --link (Add link to another container) --log-opt (Log driver options) [cobra.SliceValue]: https://pkg.go.dev/github.com/spf13/cobra@v1.9.1#SliceValue [spf13/cobra 2210]: spf13/cobra#2210 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent 61c6818 commit 572e3f1

2 files changed

Lines changed: 16 additions & 4 deletions

File tree

opts/opts.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,16 @@ func (opts *ListOpts) GetAll() []string {
8484
return *opts.values
8585
}
8686

87+
// GetSlice returns the values of slice.
88+
//
89+
// It implements [cobra.SliceValue] to allow shell completion to be provided
90+
// multiple times.
91+
//
92+
// [cobra.SliceValue]: https://pkg.go.dev/github.com/spf13/cobra@v1.9.1#SliceValue
93+
func (opts *ListOpts) GetSlice() []string {
94+
return *opts.values
95+
}
96+
8797
// GetAllOrEmpty returns the values of the slice
8898
// or an empty slice when there are no values.
8999
func (opts *ListOpts) GetAllOrEmpty() []string {

opts/opts_test.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func TestMapOpts(t *testing.T) {
112112
}
113113
}
114114

115+
//nolint:gocyclo // ignore "cyclomatic complexity 17 is too high"
115116
func TestListOptsWithoutValidator(t *testing.T) {
116117
o := NewListOpts(nil)
117118
err := o.Set("foo")
@@ -145,12 +146,13 @@ func TestListOptsWithoutValidator(t *testing.T) {
145146
if o.String() != "[bar bar]" {
146147
t.Errorf("%s != [bar bar]", o.String())
147148
}
148-
listOpts := o.GetAll()
149-
if len(listOpts) != 2 || listOpts[0] != "bar" || listOpts[1] != "bar" {
149+
if listOpts := o.GetAll(); len(listOpts) != 2 || listOpts[0] != "bar" || listOpts[1] != "bar" {
150150
t.Errorf("Expected [[bar bar]], got [%v]", listOpts)
151151
}
152-
mapListOpts := o.GetMap()
153-
if len(mapListOpts) != 1 {
152+
if listOpts := o.GetSlice(); len(listOpts) != 2 || listOpts[0] != "bar" || listOpts[1] != "bar" {
153+
t.Errorf("Expected [[bar bar]], got [%v]", listOpts)
154+
}
155+
if mapListOpts := o.GetMap(); len(mapListOpts) != 1 {
154156
t.Errorf("Expected [map[bar:{}]], got [%v]", mapListOpts)
155157
}
156158
}

0 commit comments

Comments
 (0)