From 3f7e0c0252581fa3c6deb438ff21e32bb87b3910 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 30 Sep 2022 10:23:30 +0200 Subject: [PATCH] WIP: make printing output easier (approach 1) I'm looking if we can make printing output easier, and without linters warning about all the unhandled errors for `fmt.FPrintXX()`. One approach could be to use Cobra's print functions, which are convenient, but the (big!) downside is that we would then have to pass _both_ `cobra.Command` _and_ `command.Cli` everywhere. Looking at our code, it looks like the intent is to separate functionality from cobra (cobra "triggers" the command, but the command itself is agnostic to being called from cobra). The other approach is to - enhance the CLI to have utilities for printing (similar to cobra); `dockerCli.Printf(...)` - enhance the `streams` package to have utilities, but note that while both `dockerCli.Out()` and `dockerCli.Err()` are an `io.Writer`, only `dockerCli.Out()` is a `streams.Out`, so we would have to change `dockerCli.Err()` to also be a `streams.Out`. Maybe there was a reason for not doing so (`streams.Out` can detect if a terminal is attached, maybe that's not possible for `STDERR`, which (unlike `STDOUT` is not buffered? not sure if that makes a difference) Signed-off-by: Sebastiaan van Stijn --- cli/command/context/list.go | 18 +++++++++++------- cli/command/context/list_test.go | 8 ++++++-- cmd/docker/docker.go | 1 - 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/cli/command/context/list.go b/cli/command/context/list.go index 7798f131ae90..410ce315e58c 100644 --- a/cli/command/context/list.go +++ b/cli/command/context/list.go @@ -1,7 +1,7 @@ package context import ( - "fmt" + "io" "os" "sort" @@ -29,7 +29,7 @@ func newListCommand(dockerCli command.Cli) *cobra.Command { Short: "List contexts", Args: cli.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return runList(dockerCli, opts) + return runList(cmd, dockerCli, opts) }, ValidArgsFunction: completion.NoComplete, } @@ -40,7 +40,7 @@ func newListCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runList(dockerCli command.Cli, opts *listOptions) error { +func runList(cmd *cobra.Command, dockerCli command.Cli, opts *listOptions) error { if opts.format == "" { opts.format = formatter.TableFormatKey } @@ -101,19 +101,23 @@ func runList(dockerCli command.Cli, opts *listOptions) error { sort.Slice(contexts, func(i, j int) bool { return sortorder.NaturalLess(contexts[i].Name, contexts[j].Name) }) - if err := format(dockerCli, opts, contexts); err != nil { + if err := format(cmd.OutOrStdout(), opts, contexts); err != nil { return err } if os.Getenv(client.EnvOverrideHost) != "" { - _, _ = fmt.Fprintf(dockerCli.Err(), "Warning: %[1]s environment variable overrides the active context. "+ + cmd.PrintErrf("Warning: %[1]s environment variable overrides the active context. "+ "To use a context, either set the global --context flag, or unset %[1]s environment variable.\n", client.EnvOverrideHost) + + // Alternatives: + // - dockerCli.PrintErrf(.....) + // - dockerCli.Err().Printf(.....) } return nil } -func format(dockerCli command.Cli, opts *listOptions, contexts []*formatter.ClientContext) error { +func format(output io.Writer, opts *listOptions, contexts []*formatter.ClientContext) error { contextCtx := formatter.Context{ - Output: dockerCli.Out(), + Output: output, Format: formatter.NewClientContextFormat(opts.format, opts.quiet), } return formatter.ClientContextWrite(contextCtx, contexts) diff --git a/cli/command/context/list_test.go b/cli/command/context/list_test.go index 98ac2e480c55..dcc6909743dc 100644 --- a/cli/command/context/list_test.go +++ b/cli/command/context/list_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/docker/cli/cli/command" + "github.com/spf13/cobra" "gotest.tools/v3/assert" "gotest.tools/v3/golden" ) @@ -26,7 +27,10 @@ func TestList(t *testing.T) { createTestContext(t, cli, "unset") cli.SetCurrentContext("current") cli.OutBuffer().Reset() - assert.NilError(t, runList(cli, &listOptions{})) + cmd := &cobra.Command{} + cmd.SetOut(cli.Out()) + cmd.SetErr(cli.Err()) + assert.NilError(t, runList(cmd, cli, &listOptions{})) golden.Assert(t, cli.OutBuffer().String(), "list.golden") } @@ -36,7 +40,7 @@ func TestListQuiet(t *testing.T) { createTestContext(t, cli, "other") cli.SetCurrentContext("current") cli.OutBuffer().Reset() - assert.NilError(t, runList(cli, &listOptions{quiet: true})) + assert.NilError(t, runList(&cobra.Command{}, cli, &listOptions{quiet: true})) golden.Assert(t, cli.OutBuffer().String(), "quiet-list.golden") } diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 30c48b40d1a7..a50029d74af7 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -62,7 +62,6 @@ func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand { setupHelpCommand(dockerCli, cmd, helpCmd) setHelpFunc(dockerCli, cmd) - cmd.SetOut(dockerCli.Out()) commands.AddCommands(cmd, dockerCli) cli.DisableFlagsInUseLine(cmd)