From 40d34bca1bffe2f5e84b18d7fd94d5b3c02275a6 Mon Sep 17 00:00:00 2001
From: Alessio Treglia <alessio@tendermint.com>
Date: Thu, 1 Oct 2020 17:28:00 +0200
Subject: [PATCH] Fix stderr printing functions (#894)

* Fix stderr printing functions

Follow-up of #822

* Errors go to stderr as per POSIX

* use PrintErrf() instead of extra call to Sprintf()

* Error messages should always be printed to os.Stderr.

* add test case for Print* redirection

Thanks: @bukowa for the patch.
---
 README.md       |  2 +-
 command.go      | 14 +++++++-------
 command_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/README.md b/README.md
index dd937e5..b8cda7a 100644
--- a/README.md
+++ b/README.md
@@ -177,7 +177,7 @@ var rootCmd = &cobra.Command{
 
 func Execute() {
   if err := rootCmd.Execute(); err != nil {
-    fmt.Println(err)
+    fmt.Fprintln(os.Stderr, err)
     os.Exit(1)
   }
 }
diff --git a/command.go b/command.go
index 27d39d5..77b399e 100644
--- a/command.go
+++ b/command.go
@@ -367,7 +367,7 @@ func (c *Command) UsageFunc() (f func(*Command) error) {
 		c.mergePersistentFlags()
 		err := tmpl(c.OutOrStderr(), c.UsageTemplate(), c)
 		if err != nil {
-			c.Println(err)
+			c.PrintErrln(err)
 		}
 		return err
 	}
@@ -395,7 +395,7 @@ func (c *Command) HelpFunc() func(*Command, []string) {
 		// See https://github.com/spf13/cobra/issues/1002
 		err := tmpl(c.OutOrStdout(), c.HelpTemplate(), c)
 		if err != nil {
-			c.Println(err)
+			c.PrintErrln(err)
 		}
 	}
 }
@@ -938,8 +938,8 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 			c = cmd
 		}
 		if !c.SilenceErrors {
-			c.Println("Error:", err.Error())
-			c.Printf("Run '%v --help' for usage.\n", c.CommandPath())
+			c.PrintErrln("Error:", err.Error())
+			c.PrintErrf("Run '%v --help' for usage.\n", c.CommandPath())
 		}
 		return c, err
 	}
@@ -967,7 +967,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 		// If root command has SilentErrors flagged,
 		// all subcommands should respect it
 		if !cmd.SilenceErrors && !c.SilenceErrors {
-			c.Println("Error:", err.Error())
+			c.PrintErrln("Error:", err.Error())
 		}
 
 		// If root command has SilentUsage flagged,
@@ -1209,12 +1209,12 @@ func (c *Command) PrintErr(i ...interface{}) {
 
 // PrintErrln is a convenience method to Println to the defined Err output, fallback to Stderr if not set.
 func (c *Command) PrintErrln(i ...interface{}) {
-	c.Print(fmt.Sprintln(i...))
+	c.PrintErr(fmt.Sprintln(i...))
 }
 
 // PrintErrf is a convenience method to Printf to the defined Err output, fallback to Stderr if not set.
 func (c *Command) PrintErrf(format string, i ...interface{}) {
-	c.Print(fmt.Sprintf(format, i...))
+	c.PrintErr(fmt.Sprintf(format, i...))
 }
 
 // CommandPath returns the full path to this command.
diff --git a/command_test.go b/command_test.go
index 16cc41b..3a47a81 100644
--- a/command_test.go
+++ b/command_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"reflect"
 	"strings"
@@ -1636,6 +1637,47 @@ func TestUsageStringRedirected(t *testing.T) {
 	}
 }
 
+func TestCommandPrintRedirection(t *testing.T) {
+	errBuff, outBuff := bytes.NewBuffer(nil), bytes.NewBuffer(nil)
+	root := &Command{
+		Run: func(cmd *Command, args []string) {
+
+			cmd.PrintErr("PrintErr")
+			cmd.PrintErrln("PrintErr", "line")
+			cmd.PrintErrf("PrintEr%s", "r")
+
+			cmd.Print("Print")
+			cmd.Println("Print", "line")
+			cmd.Printf("Prin%s", "t")
+		},
+	}
+
+	root.SetErr(errBuff)
+	root.SetOut(outBuff)
+
+	if err := root.Execute(); err != nil {
+		t.Error(err)
+	}
+
+	gotErrBytes, err := ioutil.ReadAll(errBuff)
+	if err != nil {
+		t.Error(err)
+	}
+
+	gotOutBytes, err := ioutil.ReadAll(outBuff)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if wantErr := []byte("PrintErrPrintErr line\nPrintErr"); !bytes.Equal(gotErrBytes, wantErr) {
+		t.Errorf("got: '%s' want: '%s'", gotErrBytes, wantErr)
+	}
+
+	if wantOut := []byte("PrintPrint line\nPrint"); !bytes.Equal(gotOutBytes, wantOut) {
+		t.Errorf("got: '%s' want: '%s'", gotOutBytes, wantOut)
+	}
+}
+
 func TestFlagErrorFunc(t *testing.T) {
 	c := &Command{Use: "c", Run: emptyRun}