Skip to content

Commit

Permalink
WIP: make printing output easier (approach 1)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
thaJeztah committed Aug 23, 2023
1 parent 17df150 commit 3f7e0c0
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
18 changes: 11 additions & 7 deletions cli/command/context/list.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package context

import (
"fmt"
"io"
"os"
"sort"

Expand Down Expand Up @@ -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,
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions cli/command/context/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/docker/cli/cli/command"
"github.com/spf13/cobra"
"gotest.tools/v3/assert"
"gotest.tools/v3/golden"
)
Expand All @@ -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")
}

Expand All @@ -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")
}

Expand Down
1 change: 0 additions & 1 deletion cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 3f7e0c0

Please sign in to comment.