Suppress output from background fetch (unless there were errors) (#5044)

The output from background fetching is noisy and pollutes the command
log. Don't show it by default, unless there were errors, in which case
it is important to see e.g. which fork was deleted.

Author: stefanhaller

pkg/commands/git_commands/diff.go

@@ -55,7 +55,7 @@ func (self *DiffCommands) GetDiff(staged bool, additionalArgs ...string) (string
 			Dir(self.repoPaths.worktreePath).
 			Arg(additionalArgs...).
 			ToArgv(),
-	).RunWithOutput()
+	).DontLog().RunWithOutput()
 }
 
 type DiffToolCmdOptions struct {

pkg/commands/git_commands/sync.go

@@ -79,6 +79,7 @@ func (self *SyncCommands) FetchBackgroundCmdObj() *oscommands.CmdObj {
 
 	cmdObj := self.cmd.New(cmdArgs)
 	cmdObj.DontLog().FailOnCredentialRequest()
+	cmdObj.SuppressOutputUnlessError()
 	return cmdObj
 }
 

pkg/commands/git_commands/sync_test.go

@@ -100,7 +100,13 @@ func TestSyncPush(t *testing.T) {
 		t.Run(s.testName, func(t *testing.T) {
 			instance := buildSyncCommands(commonDeps{})
 			task := gocui.NewFakeTask()
-			s.test(instance.PushCmdObj(task, s.opts))
+			cmdObj, err := instance.PushCmdObj(task, s.opts)
+			if err == nil {
+				assert.True(t, cmdObj.ShouldLog())
+				assert.Equal(t, cmdObj.GetCredentialStrategy(), oscommands.PROMPT)
+				assert.False(t, cmdObj.ShouldSuppressOutputUnlessError())
+			}
+			s.test(cmdObj, err)
 		})
 	}
 }
@@ -119,6 +125,7 @@ func TestSyncFetch(t *testing.T) {
 			test: func(cmdObj *oscommands.CmdObj) {
 				assert.True(t, cmdObj.ShouldLog())
 				assert.Equal(t, cmdObj.GetCredentialStrategy(), oscommands.PROMPT)
+				assert.False(t, cmdObj.ShouldSuppressOutputUnlessError())
 				assert.Equal(t, cmdObj.Args(), []string{"git", "fetch", "--no-write-fetch-head"})
 			},
 		},
@@ -128,6 +135,7 @@ func TestSyncFetch(t *testing.T) {
 			test: func(cmdObj *oscommands.CmdObj) {
 				assert.True(t, cmdObj.ShouldLog())
 				assert.Equal(t, cmdObj.GetCredentialStrategy(), oscommands.PROMPT)
+				assert.False(t, cmdObj.ShouldSuppressOutputUnlessError())
 				assert.Equal(t, cmdObj.Args(), []string{"git", "fetch", "--all", "--no-write-fetch-head"})
 			},
 		},
@@ -157,6 +165,7 @@ func TestSyncFetchBackground(t *testing.T) {
 			test: func(cmdObj *oscommands.CmdObj) {
 				assert.False(t, cmdObj.ShouldLog())
 				assert.Equal(t, cmdObj.GetCredentialStrategy(), oscommands.FAIL)
+				assert.True(t, cmdObj.ShouldSuppressOutputUnlessError())
 				assert.Equal(t, cmdObj.Args(), []string{"git", "fetch", "--no-write-fetch-head"})
 			},
 		},
@@ -166,6 +175,7 @@ func TestSyncFetchBackground(t *testing.T) {
 			test: func(cmdObj *oscommands.CmdObj) {
 				assert.False(t, cmdObj.ShouldLog())
 				assert.Equal(t, cmdObj.GetCredentialStrategy(), oscommands.FAIL)
+				assert.True(t, cmdObj.ShouldSuppressOutputUnlessError())
 				assert.Equal(t, cmdObj.Args(), []string{"git", "fetch", "--all", "--no-write-fetch-head"})
 			},
 		},

pkg/commands/oscommands/cmd_obj.go

@@ -22,6 +22,9 @@ type CmdObj struct {
 	// see StreamOutput()
 	streamOutput bool
 
+	// see SuppressOutputUnlessError()
+	suppressOutputUnlessError bool
+
 	// see UsePty()
 	usePty bool
 
@@ -123,6 +126,18 @@ func (self *CmdObj) StreamOutput() *CmdObj {
 	return self
 }
 
+// when you call this, the streamed output will be suppressed unless there is an error
+func (self *CmdObj) SuppressOutputUnlessError() *CmdObj {
+	self.suppressOutputUnlessError = true
+
+	return self
+}
+
+// returns true if SuppressOutputUnlessError() was called
+func (self *CmdObj) ShouldSuppressOutputUnlessError() bool {
+	return self.suppressOutputUnlessError
+}
+
 // returns true if StreamOutput() was called
 func (self *CmdObj) ShouldStreamOutput() bool {
 	return self.streamOutput

pkg/commands/oscommands/cmd_obj_runner.go

@@ -227,7 +227,13 @@ func (self *cmdObjRunner) runAndStreamAux(
 	cmdObj *CmdObj,
 	onRun func(*cmdHandler, io.Writer),
 ) error {
-	cmdWriter := self.guiIO.newCmdWriterFn()
+	var cmdWriter io.Writer
+	var combinedOutput bytes.Buffer
+	if cmdObj.ShouldSuppressOutputUnlessError() {
+		cmdWriter = &combinedOutput
+	} else {
+		cmdWriter = self.guiIO.newCmdWriterFn()
+	}
 
 	if cmdObj.ShouldLog() {
 		self.logCmdObj(cmdObj)
@@ -267,6 +273,10 @@ func (self *cmdObjRunner) runAndStreamAux(
 	self.log.Infof("%s (%s)", cmdObj.ToString(), time.Since(t))
 
 	if err != nil {
+		if cmdObj.suppressOutputUnlessError {
+			_, _ = self.guiIO.newCmdWriterFn().Write(combinedOutput.Bytes())
+		}
+
 		errStr := stderr.String()
 		if errStr != "" {
 			return errors.New(errStr)