terraform icon indicating copy to clipboard operation
terraform copied to clipboard

cloudplugin: use terminal.Streams to write command output

Open sebasslash opened this issue 2 years ago • 1 comments

Use the command meta's Streams object to write output rather than pass stdout/stderr file descriptor between the command and the grpc client. This has two notable benefits:

  • terminal.Streams is the desired interface commands should use to write output to stdout/error
  • terminal.Streams readily provides terminal width information which allows the cloud command to word wrap output. We lose this information implicitly casting c.Meta.Streams.Stdout.File (and the stderr equivalent) to an io.Writer.

Fixes #

Target Release

1.6.x

sebasslash avatar Dec 07 '23 18:12 sebasslash

All right, so I smoke tested this alongside the version -json plugin PR, and it works!

Things I don't quite fully understand yet:

  • Hey, are there times when we shouldn't be word-wrapping the stdout? like with -json, for example. (Remember that newline literals are forbidden inside a json string value.)
  • What can we do to get around the "diags pretty-printed to stdout" scenario when doing json output?

nfagerlund avatar Dec 15 '23 01:12 nfagerlund

Agreed that it seems risky to assume that all output from the plugin ought to be naively wrapped by the client.

Do you think we could instead send some metadata about stdout (whether it's a terminal at all, and what its width is if so) in the initial request, and then have the plugin do the wrapping itself? (That does mean that the plugin wouldn't be able to respond properly if the terminal gets resized while it's already running, but Terraform's already not responding to terminal resizing in most cases even without the plugin, and in practice that hasn't mattered that much because Terraform output is append-only, not TUI-style.)

Generally I think we should try to be consistent in what the level of abstraction is. I could see arguments both for having the plugin just emit raw bytes to copy verbatim to stdout/stderr, or for having the plugin emit higher-level objects which the client then formats for presentation in the UI, but I would feel nervous about this mixture of the two where the server is mostly emitting raw bytes but the client will sometimes insert extra newline characters into it; that seems likely to be a gotcha for a future maintainer who isn't reading the client code carefully enough. :grimacing:

apparentlymart avatar Mar 14 '24 22:03 apparentlymart