opts: ListOpts: implement cobra.SliceValue to fix shell completion #6030
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6030 +/- ##
=======================================
Coverage 58.89% 58.90%
=======================================
Files 358 358
Lines 29962 29964 +2
=======================================
+ Hits 17647 17649 +2
Misses 11334 11334
Partials 981 981 🚀 New features to boost your workflow:
|
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>
edcfe8c to
572e3f1
Compare
This was referenced Apr 23, 2025
robmry
approved these changes
Apr 23, 2025
| @@ -145,12 +146,13 @@ func TestListOptsWithoutValidator(t *testing.T) { | |||
| if o.String() != "[bar bar]" { | |||
| t.Errorf("%s != [bar bar]", o.String()) | |||
| } | |||
| listOpts := o.GetAll() | |||
| if len(listOpts) != 2 || listOpts[0] != "bar" || listOpts[1] != "bar" { | |||
| if listOpts := o.GetAll(); len(listOpts) != 2 || listOpts[0] != "bar" || listOpts[1] != "bar" { | |||
There was a problem hiding this comment.
A DeepEqual might make this easier to read, and avoid the nolint? The is is already imported anyway. I've not tried it, but something like ...
assert.Check(t, is.DeepEqual(o.GetAll(), []string{"bar", "bar"}))
There was a problem hiding this comment.
Ah, yes, I want to have a look at this file, as there's more to fix in that area, so for now I chose to "mostly" keep it as-is.
Will likely do some follow-ups to modernise tests in this package a bit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 theGetSlice()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
--labelflag the first time, but omit it if a--labelflag was already set;With this patch, the completion script correctly identifies the
--labelflag to be accepted multiple times, and also completes it when already set;- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)