Skip to content

Conversation

@thaJeztah
Copy link
Member

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)

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

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 <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant