[breaking] Fixed a lot of --format json and, in general, how Arduino CLI output is handled (#2003)

* Added source code static-check to enforce `--format` output.

The first test run gives:

=== RUN   TestNoDirectOutputToStdOut
burnbootloader/burnbootloader.go:85:5: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
burnbootloader/burnbootloader.go:85:16: object `os.Stderr` should not be used in this package (use `feedback.*` instead)
compile/compile.go:235:84: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
compile/compile.go:235:95: object `os.Stderr` should not be used in this package (use `feedback.*` instead)
compile/compile.go:274:72: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
compile/compile.go:274:83: object `os.Stderr` should not be used in this package (use `feedback.*` instead)
compile/compile.go:296:4: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:297:4: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:298:4: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:305:3: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:306:3: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:307:3: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:308:3: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:309:3: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:311:3: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:313:4: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:319:4: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:321:5: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:325:4: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
compile/compile.go:326:4: function `fmt.Print` should not be used in this package (use `feedback.*` instead)
completion/completion.go:58:34: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
completion/completion.go:61:38: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
completion/completion.go:63:32: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
completion/completion.go:66:32: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
completion/completion.go:68:38: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
daemon/interceptors.go:29:19: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
debug/debug.go:106:82: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
monitor/term.go:38:9: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
output/rpc_progress.go:115:4: function `fmt.Print` should not be used in this package (use `feedback.*` instead)
output/rpc_progress.go:117:5: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
output/rpc_progress.go:119:5: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
upload/upload.go:163:5: object `os.Stdout` should not be used in this package (use `feedback.*` instead)
upload/upload.go:163:16: object `os.Stderr` should not be used in this package (use `feedback.*` instead)
cli.go:215:4: function `fmt.Println` should not be used in this package (use `feedback.*` instead)
--- FAIL: TestNoDirectOutputToStdOut (0.01s)
FAIL
exit status 1
FAIL	github.com/arduino/arduino-cli/cli	0.019s

* Slightly improved naming/docs of OutputFormat enumeration

Also changed a condition in a more meaningful way.

* Removed `feedback.Feedback` since only the global instance is used

We may think of bringing it back again in the future if the global
instance will be removed in a major refactoring of the CLI.

* Moved `cli/output` package into `cli/feedback`

This is the better place where it belongs, and slighlty simplifies the
golang API.

* Print progress bar and task progess only on interactive terminals

* Use feedback functions to output task progress

* User-input functions are now moved into `feedback` package

* Fix user-input function

- Final "\n" is automatically printed when entering non-passwords (no
  need to do a Println()
- The final "\n" is included in the result of ReadBytes() so it will be
  removed.

* Added cmd to test feedback functions

* Better error message

* Removed unprotected print

* Removed useless response from Upload and UploadWithProgrammer

* VersionInfo now implements feedback.Result interface

* 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.

* Replace direct use of os.Stdout/Stderr in Upload command

* Implemented feedback.Fatal and FatalError

These functions outputs the error (also in machine-encoded if user
choose to do so) and at the same time exits with os.Exit().

This API is much more readable and provides a better meaning to the CLI
commands implementation.

* Added output buffers in error messages (if used)

* Removed direct access to stdio streams in monitor command

* Removed direct access to stdio streams in debug command

* Removed direct access to stdio streams in daemon command

* Removed direct access to stdio streams in burn-bootlodaer command

* Removed direct access to stdio streams in compile command

* Removed direct access to stdio streams in completion command

* compile: print platforms stats only if present

* Removed direct access to stdio streams in --dump-profile command

* Added feedback functions to report warnings

* Moved `errorcodes` into `feedback`

* Remove direct os.Stdin access from daemon command

* Removed redundant `cli/globals` package

This will help later to make the whole `cli` package internal.

* Made `cli` package internal

* updated docs

* Removed redundant logic in getter for stdio streams

Co-authored-by: per1234 <accounts@perglass.com>

* Internationalize more strings

Co-authored-by: per1234 <accounts@perglass.com>

* Spellcheck internal/cli/feedback/stdio.go

Co-authored-by: per1234 <accounts@perglass.com>

* Spellcheck internal/cli/feedback/feedback_cmd.go

Co-authored-by: per1234 <accounts@perglass.com>

* feedback: remove stray '\r' on Windows on interactive input

* Ban use of os.Exit from cli package

* Removed unused parameter in compile.Compile

* Non-interactive stream are always buffered

* Use direct streams where appropiate

* Compile outputs profile dump as part of the result

* Report saved warnings also when erroring out

* Print compile error and suggestions as part of the result

* Add trailing newline only if compiler has produced output

* FatalResult now outputs the error on stderr

Co-authored-by: per1234 <accounts@perglass.com>

Author: cmaglie

arduino/discovery/discovery.go

@@ -23,10 +23,10 @@ import (
 	"sync"
 	"time"
 
-	"github.com/arduino/arduino-cli/cli/globals"
 	"github.com/arduino/arduino-cli/executils"
 	"github.com/arduino/arduino-cli/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	"github.com/arduino/arduino-cli/version"
 	"github.com/arduino/go-properties-orderedmap"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -301,7 +301,7 @@ func (disc *PluggableDiscovery) Run() (err error) {
 		}
 	}()
 
-	if err = disc.sendCommand("HELLO 1 \"arduino-cli " + globals.VersionInfo.VersionString + "\"\n"); err != nil {
+	if err = disc.sendCommand("HELLO 1 \"arduino-cli " + version.VersionInfo.VersionString + "\"\n"); err != nil {
 		return err
 	}
 	if msg, err := disc.waitMessage(time.Second * 10); err != nil {

arduino/monitor/monitor.go

@@ -26,9 +26,9 @@ import (
 	"strings"
 	"time"
 
-	"github.com/arduino/arduino-cli/cli/globals"
 	"github.com/arduino/arduino-cli/executils"
 	"github.com/arduino/arduino-cli/i18n"
+	"github.com/arduino/arduino-cli/version"
 	"github.com/sirupsen/logrus"
 )
 
@@ -227,7 +227,7 @@ func (mon *PluggableMonitor) Run() (err error) {
 		}
 	}()
 
-	if err = mon.sendCommand("HELLO 1 \"arduino-cli " + globals.VersionInfo.VersionString + "\"\n"); err != nil {
+	if err = mon.sendCommand("HELLO 1 \"arduino-cli " + version.VersionInfo.VersionString + "\"\n"); err != nil {
 		return err
 	}
 	if msg, err := mon.waitMessage(time.Second*10, "hello"); err != nil {

cli/feedback/exported.go

@@ -39,7 +39,7 @@ import (
 var tr = i18n.Tr
 
 // Compile FIXMEDOC
-func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream io.Writer, progressCB rpc.TaskProgressCB, debug bool) (r *rpc.CompileResponse, e error) {
+func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream io.Writer, progressCB rpc.TaskProgressCB) (r *rpc.CompileResponse, e error) {
 
 	// There is a binding between the export binaries setting and the CLI flag to explicitly set it,
 	// since we want this binding to work also for the gRPC interface we must read it here in this

cli/feedback/feedback.go

@@ -19,8 +19,8 @@ import (
 	"os"
 	"testing"
 
-	"github.com/arduino/arduino-cli/cli/instance"
 	"github.com/arduino/arduino-cli/configuration"
+	"github.com/arduino/arduino-cli/internal/cli/instance"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
 	"github.com/stretchr/testify/require"