Skip to content

Commit 6bd8a4b

Browse files
thaJeztahthaJeztah
committed
cli: deprecate VisitAll, DisableFlagsInUseLine utilities
These utilities were only used internally; create a local copy where used, and deprecate the ones in cli. Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 5a99022 commit 6bd8a4b

6 files changed

Lines changed: 99 additions & 37 deletions

File tree

‎cli-plugins/plugin/plugin.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,24 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta
175175
newMetadataSubcommand(plugin, meta),
176176
)
177177

178-
cli.DisableFlagsInUseLine(cmd)
178+
visitAll(cmd,
179+
// prevent adding "[flags]" to the end of the usage line.
180+
func(c *cobra.Command) { c.DisableFlagsInUseLine = true },
181+
)
179182

180183
return cli.NewTopLevelCommand(cmd, dockerCli, opts, cmd.Flags())
181184
}
182185

186+
// visitAll traverses all commands from the root.
187+
func visitAll(root *cobra.Command, fns ...func(*cobra.Command)) {
188+
for _, cmd := range root.Commands() {
189+
visitAll(cmd, fns...)
190+
}
191+
for _, fn := range fns {
192+
fn(root)
193+
}
194+
}
195+
183196
func newMetadataSubcommand(plugin *cobra.Command, meta metadata.Metadata) *cobra.Command {
184197
if meta.ShortDescription == "" {
185198
meta.ShortDescription = plugin.Short

‎cli-plugins/plugin/plugin_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package plugin
2+
3+
import (
4+
"slices"
5+
"testing"
6+
7+
"github.com/spf13/cobra"
8+
)
9+
10+
func TestVisitAll(t *testing.T) {
11+
root := &cobra.Command{Use: "root"}
12+
sub1 := &cobra.Command{Use: "sub1"}
13+
sub1sub1 := &cobra.Command{Use: "sub1sub1"}
14+
sub1sub2 := &cobra.Command{Use: "sub1sub2"}
15+
sub2 := &cobra.Command{Use: "sub2"}
16+
17+
root.AddCommand(sub1, sub2)
18+
sub1.AddCommand(sub1sub1, sub1sub2)
19+
20+
var visited []string
21+
visitAll(root, func(ccmd *cobra.Command) {
22+
visited = append(visited, ccmd.Name())
23+
})
24+
expected := []string{"sub1sub1", "sub1sub2", "sub1", "sub2", "root"}
25+
if !slices.Equal(expected, visited) {
26+
t.Errorf("expected %#v, got %#v", expected, visited)
27+
}
28+
}

‎cli/cobra.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,19 +167,25 @@ func (tcmd *TopLevelCommand) Initialize(ops ...command.CLIOption) error {
167167
}
168168

169169
// VisitAll will traverse all commands from the root.
170-
// This is different from the VisitAll of cobra.Command where only parents
171-
// are checked.
170+
//
171+
// Deprecated: this utility was only used internally and will be removed in the next release.
172172
func VisitAll(root *cobra.Command, fn func(*cobra.Command)) {
173+
visitAll(root, fn)
174+
}
175+
176+
func visitAll(root *cobra.Command, fn func(*cobra.Command)) {
173177
for _, cmd := range root.Commands() {
174-
VisitAll(cmd, fn)
178+
visitAll(cmd, fn)
175179
}
176180
fn(root)
177181
}
178182

179183
// DisableFlagsInUseLine sets the DisableFlagsInUseLine flag on all
180184
// commands within the tree rooted at cmd.
185+
//
186+
// Deprecated: this utility was only used internally and will be removed in the next release.
181187
func DisableFlagsInUseLine(cmd *cobra.Command) {
182-
VisitAll(cmd, func(ccmd *cobra.Command) {
188+
visitAll(cmd, func(ccmd *cobra.Command) {
183189
// do not add a `[flags]` to the end of the usage line.
184190
ccmd.DisableFlagsInUseLine = true
185191
})

‎cli/cobra_test.go‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,28 +10,6 @@ import (
1010
is "gotest.tools/v3/assert/cmp"
1111
)
1212

13-
func TestVisitAll(t *testing.T) {
14-
root := &cobra.Command{Use: "root"}
15-
sub1 := &cobra.Command{Use: "sub1"}
16-
sub1sub1 := &cobra.Command{Use: "sub1sub1"}
17-
sub1sub2 := &cobra.Command{Use: "sub1sub2"}
18-
sub2 := &cobra.Command{Use: "sub2"}
19-
20-
root.AddCommand(sub1, sub2)
21-
sub1.AddCommand(sub1sub1, sub1sub2)
22-
23-
// Take the opportunity to test DisableFlagsInUseLine too
24-
DisableFlagsInUseLine(root)
25-
26-
var visited []string
27-
VisitAll(root, func(ccmd *cobra.Command) {
28-
visited = append(visited, ccmd.Name())
29-
assert.Assert(t, ccmd.DisableFlagsInUseLine, "DisableFlagsInUseLine not set on %q", ccmd.Name())
30-
})
31-
expected := []string{"sub1sub1", "sub1sub2", "sub1", "sub2", "root"}
32-
assert.DeepEqual(t, expected, visited)
33-
}
34-
3513
func TestVendorAndVersion(t *testing.T) {
3614
// Non plugin.
3715
assert.Equal(t, vendorAndVersion(&cobra.Command{Use: "test"}), "")

‎cmd/docker/docker.go‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,11 @@ func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand {
163163
cmd.SetOut(dockerCli.Out())
164164
commands.AddCommands(cmd, dockerCli)
165165

166-
cli.DisableFlagsInUseLine(cmd)
167-
setValidateArgs(dockerCli, cmd)
166+
visitAll(cmd,
167+
setValidateArgs(dockerCli),
168+
// prevent adding "[flags]" to the end of the usage line.
169+
func(c *cobra.Command) { c.DisableFlagsInUseLine = true },
170+
)
168171

169172
// flags must be the top-level command flags, not cmd.Flags()
170173
return cli.NewTopLevelCommand(cmd, dockerCli, opts, cmd.Flags())
@@ -265,14 +268,29 @@ func setHelpFunc(dockerCli command.Cli, cmd *cobra.Command) {
265268
})
266269
}
267270

268-
func setValidateArgs(dockerCli command.Cli, cmd *cobra.Command) {
269-
// The Args is handled by ValidateArgs in cobra, which does not allows a pre-hook.
270-
// As a result, here we replace the existing Args validation func to a wrapper,
271-
// where the wrapper will check to see if the feature is supported or not.
272-
// The Args validation error will only be returned if the feature is supported.
273-
cli.VisitAll(cmd, func(ccmd *cobra.Command) {
271+
// visitAll traverses all commands from the root.
272+
func visitAll(root *cobra.Command, fns ...func(*cobra.Command)) {
273+
for _, cmd := range root.Commands() {
274+
visitAll(cmd, fns...)
275+
}
276+
for _, fn := range fns {
277+
fn(root)
278+
}
279+
}
280+
281+
// The Args is handled by ValidateArgs in cobra, which does not allows a pre-hook.
282+
// As a result, here we replace the existing Args validation func to a wrapper,
283+
// where the wrapper will check to see if the feature is supported or not.
284+
// The Args validation error will only be returned if the feature is supported.
285+
func setValidateArgs(dockerCLI versionDetails) func(*cobra.Command) {
286+
return func(ccmd *cobra.Command) {
274287
// if there is no tags for a command or any of its parent,
275288
// there is no need to wrap the Args validation.
289+
//
290+
// FIXME(thaJeztah): can we memoize properties of the parent?
291+
// visitAll traverses root -> all childcommands, and hasTags
292+
// goes the reverse (cmd -> visit all parents), so we may
293+
// end traversing two directions.
276294
if !hasTags(ccmd) {
277295
return
278296
}
@@ -283,12 +301,12 @@ func setValidateArgs(dockerCli command.Cli, cmd *cobra.Command) {
283301

284302
cmdArgs := ccmd.Args
285303
ccmd.Args = func(cmd *cobra.Command, args []string) error {
286-
if err := isSupported(cmd, dockerCli); err != nil {
304+
if err := isSupported(cmd, dockerCLI); err != nil {
287305
return err
288306
}
289307
return cmdArgs(cmd, args)
290308
}
291-
})
309+
}
292310
}
293311

294312
func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command, subcommand string, envs []string) error {

‎cmd/docker/docker_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/docker/cli/cli/debug"
1515
platformsignals "github.com/docker/cli/cmd/docker/internal/signals"
1616
"github.com/sirupsen/logrus"
17+
"github.com/spf13/cobra"
1718
"gotest.tools/v3/assert"
1819
is "gotest.tools/v3/assert/cmp"
1920
)
@@ -108,3 +109,21 @@ func TestUserTerminatedError(t *testing.T) {
108109

109110
assert.Equal(t, getExitCode(context.Cause(notifyCtx)), 143)
110111
}
112+
113+
func TestVisitAll(t *testing.T) {
114+
root := &cobra.Command{Use: "root"}
115+
sub1 := &cobra.Command{Use: "sub1"}
116+
sub1sub1 := &cobra.Command{Use: "sub1sub1"}
117+
sub1sub2 := &cobra.Command{Use: "sub1sub2"}
118+
sub2 := &cobra.Command{Use: "sub2"}
119+
120+
root.AddCommand(sub1, sub2)
121+
sub1.AddCommand(sub1sub1, sub1sub2)
122+
123+
var visited []string
124+
visitAll(root, func(ccmd *cobra.Command) {
125+
visited = append(visited, ccmd.Name())
126+
})
127+
expected := []string{"sub1sub1", "sub1sub2", "sub1", "sub2", "root"}
128+
assert.DeepEqual(t, expected, visited)
129+
}

0 commit comments

Comments
 (0)