Added feedback support for direct streaming

When the Text format is selected the output is sent straigh to the
output stream, otherwise it is buffered and returned as a
`feedback.Result` to be used at the end of the job.

cli/feedback/feedback.go

@@ -18,6 +18,7 @@
 package feedback
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -68,11 +69,32 @@ func ParseOutputFormat(in string) (OutputFormat, bool) {
 }
 
 var (
-	stdOut io.Writer    = os.Stdout
-	stdErr io.Writer    = os.Stderr
-	format OutputFormat = Text
+	stdOut         io.Writer
+	stdErr         io.Writer
+	feedbackOut    io.Writer
+	feedbackErr    io.Writer
+	bufferOut      *bytes.Buffer
+	bufferErr      *bytes.Buffer
+	format         OutputFormat
+	formatSelected bool
 )
 
+func init() {
+	reset()
+}
+
+// reset resets the feedback package to its initial state, useful for unit testing
+func reset() {
+	stdOut = os.Stdout
+	stdErr = os.Stderr
+	feedbackOut = os.Stdout
+	feedbackErr = os.Stderr
+	bufferOut = &bytes.Buffer{}
+	bufferErr = &bytes.Buffer{}
+	format = Text
+	formatSelected = false
+}
+
 // Result is anything more complex than a sentence that needs to be printed
 // for the user.
 type Result interface {
@@ -84,53 +106,50 @@ var tr = i18n.Tr
 
 // SetOut can be used to change the out writer at runtime
 func SetOut(out io.Writer) {
+	if formatSelected {
+		panic("output format already selected")
+	}
 	stdOut = out
 }
 
 // SetErr can be used to change the err writer at runtime
 func SetErr(err io.Writer) {
+	if formatSelected {
+		panic("output format already selected")
+	}
 	stdErr = err
 }
 
 // SetFormat can be used to change the output format at runtime
 func SetFormat(f OutputFormat) {
+	if formatSelected {
+		panic("output format already selected")
+	}
 	format = f
+	formatSelected = true
+
+	if format == Text {
+		feedbackOut = stdOut
+		feedbackErr = stdErr
+	} else {
+		feedbackOut = bufferOut
+		feedbackErr = bufferErr
+	}
 }
 
 // GetFormat returns the output format currently set
 func GetFormat() OutputFormat {
 	return format
 }
 
-// OutputWriter returns the underlying io.Writer to be used when the Print*
-// api is not enough
-func OutputWriter() io.Writer {
-	return stdOut
-}
-
-// ErrorWriter is the same as OutputWriter but exposes the underlying error
-// writer.
-func ErrorWriter() io.Writer {
-	return stdErr
-}
-
 // Printf behaves like fmt.Printf but writes on the out writer and adds a newline.
 func Printf(format string, v ...interface{}) {
 	Print(fmt.Sprintf(format, v...))
 }
 
 // Print behaves like fmt.Print but writes on the out writer and adds a newline.
-func Print(v interface{}) {
-	switch format {
-	case JSON, MinifiedJSON:
-		printJSON(v)
-	case YAML:
-		printYAML(v)
-	case Text:
-		fmt.Fprintln(stdOut, v)
-	default:
-		panic("unknown output format")
-	}
+func Print(v string) {
+	fmt.Fprintln(feedbackOut, v)
 }
 
 // Errorf behaves like fmt.Printf but writes on the error writer and adds a
@@ -156,46 +175,39 @@ func Error(v ...interface{}) {
 	logrus.Error(fmt.Sprint(v...))
 }
 
-// printJSON is a convenient wrapper to provide feedback by printing the
-// desired output in a pretty JSON format. It adds a newline to the output.
-func printJSON(v interface{}) {
-	var d []byte
-	var err error
-	if format == JSON {
-		d, err = json.MarshalIndent(v, "", "  ")
-	} else if format == MinifiedJSON {
-		d, err = json.Marshal(v)
-	}
-	if err != nil {
-		Errorf(tr("Error during JSON encoding of the output: %v"), err)
-	} else {
-		fmt.Fprintf(stdOut, "%v\n", string(d))
-	}
-}
-
-// printYAML is a convenient wrapper to provide feedback by printing the
-// desired output in YAML format. It adds a newline to the output.
-func printYAML(v interface{}) {
-	d, err := yaml.Marshal(v)
-	if err != nil {
-		Errorf(tr("Error during YAML encoding of the output: %v"), err)
-		return
-	}
-	fmt.Fprintf(stdOut, "%v\n", string(d))
-}
-
 // PrintResult is a convenient wrapper to provide feedback for complex data,
 // where the contents can't be just serialized to JSON but requires more
 // structure.
 func PrintResult(res Result) {
+	var data string
 	switch format {
-	case JSON, MinifiedJSON:
-		printJSON(res.Data())
+	case JSON:
+		d, err := json.MarshalIndent(res.Data(), "", "  ")
+		if err != nil {
+			Errorf("Error during JSON encoding of the output: %v", err)
+			return
+		}
+		data = string(d)
+	case MinifiedJSON:
+		d, err := json.Marshal(res.Data())
+		if err != nil {
+			Errorf("Error during JSON encoding of the output: %v", err)
+			return
+		}
+		data = string(d)
 	case YAML:
-		printYAML(res.Data())
+		d, err := yaml.Marshal(res.Data())
+		if err != nil {
+			Errorf("Error during YAML encoding of the output: %v", err)
+			return
+		}
+		data = string(d)
 	case Text:
-		Print(res.String())
+		data = res.String()
 	default:
 		panic("unknown output format")
 	}
+	if data != "" {
+		fmt.Fprintln(stdOut, data)
+	}
 }

cli/feedback/feedback_test.go

@@ -0,0 +1,123 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2020 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+package feedback
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestOutputSelection(t *testing.T) {
+	reset()
+
+	myErr := new(bytes.Buffer)
+	myOut := new(bytes.Buffer)
+	SetOut(myOut)
+	SetErr(myErr)
+	SetFormat(Text)
+
+	// Could not change output stream after format has been set
+	require.Panics(t, func() { SetOut(nil) })
+	require.Panics(t, func() { SetErr(nil) })
+
+	// Coule not change output format twice
+	require.Panics(t, func() { SetFormat(JSON) })
+
+	Print("Hello")
+	require.Equal(t, myOut.String(), "Hello\n")
+}
+
+func TestJSONOutputStream(t *testing.T) {
+	reset()
+
+	require.Panics(t, func() { OutputStreams() })
+
+	SetFormat(JSON)
+	stdout, stderr, res := OutputStreams()
+	fmt.Fprint(stdout, "Hello")
+	fmt.Fprint(stderr, "Hello ERR")
+
+	d, err := json.Marshal(res())
+	require.NoError(t, err)
+	require.JSONEq(t, `{"stdout":"Hello","stderr":"Hello ERR"}`, string(d))
+
+	stdout.Write([]byte{0xc2, 'A'}) // Invaid UTF-8
+
+	d, err = json.Marshal(res())
+	require.NoError(t, err)
+	require.JSONEq(t, string(d), `{"stdout":"Hello\ufffdA","stderr":"Hello ERR"}`)
+}
+
+func TestJsonOutputOnCustomStreams(t *testing.T) {
+	reset()
+
+	myErr := new(bytes.Buffer)
+	myOut := new(bytes.Buffer)
+	SetOut(myOut)
+	SetErr(myErr)
+	SetFormat(JSON)
+
+	// Could not change output stream after format has been set
+	require.Panics(t, func() { SetOut(nil) })
+	require.Panics(t, func() { SetErr(nil) })
+	// Could not change output format twice
+	require.Panics(t, func() { SetFormat(JSON) })
+
+	Print("Hello") // Output interactive data
+
+	require.Equal(t, "", myOut.String())
+	require.Equal(t, "", myErr.String())
+	require.Equal(t, "Hello\n", bufferOut.String())
+
+	PrintResult(&testResult{Success: true})
+
+	require.JSONEq(t, myOut.String(), `{ "success": true }`)
+	require.Equal(t, myErr.String(), "")
+	myOut.Reset()
+
+	_, _, res := OutputStreams()
+	PrintResult(&testResult{Success: false, Output: res()})
+
+	require.JSONEq(t, `
+{
+  "success": false,
+  "output": {
+    "stdout": "Hello\n",
+    "stderr": ""
+  }
+}`, myOut.String())
+	require.Equal(t, myErr.String(), "")
+}
+
+type testResult struct {
+	Success bool                 `json:"success"`
+	Output  *OutputStreamsResult `json:"output,omitempty"`
+}
+
+func (r *testResult) Data() interface{} {
+	return r
+}
+
+func (r *testResult) String() string {
+	if r.Success {
+		return "Success"
+	}
+	return "Failure"
+}

cli/feedback/stdio.go

@@ -0,0 +1,68 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2020 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+package feedback
+
+import (
+	"io"
+)
+
+// OutputStreams returns the underlying io.Writer to directly stream to
+// stdout and stderr.
+// If the selected output format is not Text, the returned writers will
+// accumulate the output until command execution is completed.
+// This function returns also a callback that must be called when the
+// command execution is completed, it will return a *OutputStreamsResult
+// object that can be used as a Result or to retrieve the output to embed
+// it in another object.
+func OutputStreams() (io.Writer, io.Writer, func() *OutputStreamsResult) {
+	if !formatSelected {
+		panic("output format not yet selected")
+	}
+	if format == Text {
+		return stdOut, stdErr, getOutputStreamResult
+	}
+	return bufferOut, bufferErr, getOutputStreamResult
+}
+
+func getOutputStreamResult() *OutputStreamsResult {
+	return &OutputStreamsResult{
+		Stdout: bufferOut.String(),
+		Stderr: bufferErr.String(),
+	}
+}
+
+// OutputStreamsResult contains the accumulated stdout and stderr output
+// when the selected output format is not Text.
+type OutputStreamsResult struct {
+	Stdout string `json:"stdout"`
+	Stderr string `json:"stderr"`
+}
+
+// Data returns the result object itself, it is used to implement the Result interface.
+func (r *OutputStreamsResult) Data() interface{} {
+	// In case of non-Text output format, the output is accumulared so retrun the buffer as a Result object
+	return r
+}
+
+func (r *OutputStreamsResult) String() string {
+	// In case of Text output format, the output is streamed to stdout and stderr directly, no need to print anything
+	return ""
+}
+
+// Empty returns true if both Stdout and Stderr are empty.
+func (r *OutputStreamsResult) Empty() bool {
+	return r.Stdout == "" && r.Stderr == ""
+}