Handle pending actions properly in git commands that require credentials

I don't know if this is a hack or not: we run a git command and increment the pending action
count to 1 but at some point the command requests a username or password, so we need to prompt
the user to enter that. At that point we don't want to say that there is a pending action,
so we decrement the action count before prompting the user and then re-increment it again afterward.

Given that we panic when the counter goes below zero, it's important that it's not zero
when we run the git command (should be impossible anyway).

I toyed with a different approach using channels and a long-running goroutine that
handles all commands that request credentials but it feels over-engineered compared to this
commit's approach.

pkg/commands/oscommands/cmd_obj_runner.go

@@ -19,15 +19,6 @@ type ICmdObjRunner interface {
 	RunAndProcessLines(cmdObj ICmdObj, onLine func(line string) (bool, error)) error
 }
 
-type CredentialType int
-
-const (
-	Password CredentialType = iota
-	Username
-	Passphrase
-	PIN
-)
-
 type cmdObjRunner struct {
 	log   *logrus.Entry
 	guiIO *guiIO
@@ -182,26 +173,6 @@ func (self *cmdObjRunner) RunAndProcessLines(cmdObj ICmdObj, onLine func(line st
 	return nil
 }
 
-// Whenever we're asked for a password we just enter a newline, which will
-// eventually cause the command to fail.
-var failPromptFn = func(CredentialType) string { return "\n" }
-
-func (self *cmdObjRunner) runWithCredentialHandling(cmdObj ICmdObj) error {
-	var promptFn func(CredentialType) string
-
-	switch cmdObj.GetCredentialStrategy() {
-	case PROMPT:
-		promptFn = self.guiIO.promptForCredentialFn
-	case FAIL:
-		promptFn = failPromptFn
-	case NONE:
-		// we should never land here
-		return errors.New("runWithCredentialHandling called but cmdObj does not have a credential strategy")
-	}
-
-	return self.runAndDetectCredentialRequest(cmdObj, promptFn)
-}
-
 func (self *cmdObjRunner) logCmdObj(cmdObj ICmdObj) {
 	self.guiIO.logCommandFn(cmdObj.ToString(), true)
 }
@@ -233,25 +204,6 @@ func (self *cmdObjRunner) runAndStream(cmdObj ICmdObj) error {
 	})
 }
 
-// runAndDetectCredentialRequest detect a username / password / passphrase question in a command
-// promptUserForCredential is a function that gets executed when this function detect you need to fillin a password or passphrase
-// The promptUserForCredential argument will be "username", "password" or "passphrase" and expects the user's password/passphrase or username back
-func (self *cmdObjRunner) runAndDetectCredentialRequest(
-	cmdObj ICmdObj,
-	promptUserForCredential func(CredentialType) string,
-) error {
-	// setting the output to english so we can parse it for a username/password request
-	cmdObj.AddEnvVars("LANG=en_US.UTF-8", "LC_ALL=en_US.UTF-8")
-
-	return self.runAndStreamAux(cmdObj, func(handler *cmdHandler, cmdWriter io.Writer) {
-		tr := io.TeeReader(handler.stdoutPipe, cmdWriter)
-
-		go utils.Safe(func() {
-			self.processOutput(tr, handler.stdinPipe, promptUserForCredential)
-		})
-	})
-}
-
 func (self *cmdObjRunner) runAndStreamAux(
 	cmdObj ICmdObj,
 	onRun func(*cmdHandler, io.Writer),
@@ -302,7 +254,70 @@ func (self *cmdObjRunner) runAndStreamAux(
 	return nil
 }
 
-func (self *cmdObjRunner) processOutput(reader io.Reader, writer io.Writer, promptUserForCredential func(CredentialType) string) {
+type CredentialType int
+
+const (
+	Password CredentialType = iota
+	Username
+	Passphrase
+	PIN
+)
+
+// Whenever we're asked for a password we just enter a newline, which will
+// eventually cause the command to fail.
+var failPromptFn = func(CredentialType) <-chan string {
+	ch := make(chan string)
+	go func() {
+		ch <- "\n"
+	}()
+	return ch
+}
+
+func (self *cmdObjRunner) runWithCredentialHandling(cmdObj ICmdObj) error {
+	promptFn, err := self.getCredentialPromptFn(cmdObj)
+	if err != nil {
+		return err
+	}
+
+	return self.runAndDetectCredentialRequest(cmdObj, promptFn)
+}
+
+func (self *cmdObjRunner) getCredentialPromptFn(cmdObj ICmdObj) (func(CredentialType) <-chan string, error) {
+	switch cmdObj.GetCredentialStrategy() {
+	case PROMPT:
+		return self.guiIO.promptForCredentialFn, nil
+	case FAIL:
+		return failPromptFn, nil
+	default:
+		// we should never land here
+		return nil, errors.New("runWithCredentialHandling called but cmdObj does not have a credential strategy")
+	}
+}
+
+// runAndDetectCredentialRequest detect a username / password / passphrase question in a command
+// promptUserForCredential is a function that gets executed when this function detect you need to fillin a password or passphrase
+// The promptUserForCredential argument will be "username", "password" or "passphrase" and expects the user's password/passphrase or username back
+func (self *cmdObjRunner) runAndDetectCredentialRequest(
+	cmdObj ICmdObj,
+	promptUserForCredential func(CredentialType) <-chan string,
+) error {
+	// setting the output to english so we can parse it for a username/password request
+	cmdObj.AddEnvVars("LANG=en_US.UTF-8", "LC_ALL=en_US.UTF-8")
+
+	return self.runAndStreamAux(cmdObj, func(handler *cmdHandler, cmdWriter io.Writer) {
+		tr := io.TeeReader(handler.stdoutPipe, cmdWriter)
+
+		go utils.Safe(func() {
+			self.processOutput(tr, handler.stdinPipe, promptUserForCredential)
+		})
+	})
+}
+
+func (self *cmdObjRunner) processOutput(
+	reader io.Reader,
+	writer io.Writer,
+	promptUserForCredential func(CredentialType) <-chan string,
+) {
 	checkForCredentialRequest := self.getCheckForCredentialRequestFunc()
 
 	scanner := bufio.NewScanner(reader)
@@ -311,7 +326,14 @@ func (self *cmdObjRunner) processOutput(reader io.Reader, writer io.Writer, prom
 		newBytes := scanner.Bytes()
 		askFor, ok := checkForCredentialRequest(newBytes)
 		if ok {
-			toInput := promptUserForCredential(askFor)
+			responseChan := promptUserForCredential(askFor)
+			// We assume that the busy count is greater than zero here because we're
+			// in the middle of a command. We decrement it so that The user can be prompted
+			// without lazygit thinking it's still doing its own processing. This helps
+			// integration tests know how long to wait before typing in a response.
+			self.guiIO.DecrementBusyCount()
+			toInput := <-responseChan
+			self.guiIO.IncrementBusyCount()
 			// If the return data is empty we don't write anything to stdin
 			if toInput != "" {
 				_, _ = writer.Write([]byte(toInput))

pkg/commands/oscommands/cmd_obj_runner_test.go

@@ -15,6 +15,18 @@ func getRunner() *cmdObjRunner {
 	}
 }
 
+func toChanFn(f func(ct CredentialType) string) func(CredentialType) <-chan string {
+	return func(ct CredentialType) <-chan string {
+		ch := make(chan string)
+
+		go func() {
+			ch <- f(ct)
+		}()
+
+		return ch
+	}
+}
+
 func TestProcessOutput(t *testing.T) {
 	defaultPromptUserForCredential := func(ct CredentialType) string {
 		switch ct {
@@ -99,7 +111,7 @@ func TestProcessOutput(t *testing.T) {
 			reader := strings.NewReader(scenario.output)
 			writer := &strings.Builder{}
 
-			runner.processOutput(reader, writer, scenario.promptUserForCredential)
+			runner.processOutput(reader, writer, toChanFn(scenario.promptUserForCredential))
 
 			if writer.String() != scenario.expectedToWrite {
 				t.Errorf("expected to write '%s' but got '%s'", scenario.expectedToWrite, writer.String())

pkg/commands/oscommands/gui_io.go

@@ -26,15 +26,27 @@ type guiIO struct {
 	// this allows us to request info from the user like username/password, in the event
 	// that a command requests it.
 	// the 'credential' arg is something like 'username' or 'password'
-	promptForCredentialFn func(credential CredentialType) string
+	promptForCredentialFn func(credential CredentialType) <-chan string
+
+	IncrementBusyCount func()
+	DecrementBusyCount func()
 }
 
-func NewGuiIO(log *logrus.Entry, logCommandFn func(string, bool), newCmdWriterFn func() io.Writer, promptForCredentialFn func(CredentialType) string) *guiIO {
+func NewGuiIO(
+	log *logrus.Entry,
+	logCommandFn func(string, bool),
+	newCmdWriterFn func() io.Writer,
+	promptForCredentialFn func(CredentialType) <-chan string,
+	IncrementBusyCount func(),
+	DecrementBusyCount func(),
+) *guiIO {
 	return &guiIO{
 		log:                   log,
 		logCommandFn:          logCommandFn,
 		newCmdWriterFn:        newCmdWriterFn,
 		promptForCredentialFn: promptForCredentialFn,
+		IncrementBusyCount:    IncrementBusyCount,
+		DecrementBusyCount:    DecrementBusyCount,
 	}
 }
 
@@ -46,5 +58,7 @@ func NewNullGuiIO(log *logrus.Entry) *guiIO {
 		logCommandFn:          func(string, bool) {},
 		newCmdWriterFn:        func() io.Writer { return io.Discard },
 		promptForCredentialFn: failPromptFn,
+		IncrementBusyCount:    func() {},
+		DecrementBusyCount:    func() {},
 	}
 }

pkg/gui/controllers/helpers/credentials_helper.go

@@ -1,8 +1,6 @@
 package helpers
 
 import (
-	"sync"
-
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
 )
@@ -20,11 +18,11 @@ func NewCredentialsHelper(
 }
 
 // promptUserForCredential wait for a username, password or passphrase input from the credentials popup
-func (self *CredentialsHelper) PromptUserForCredential(passOrUname oscommands.CredentialType) string {
-	waitGroup := sync.WaitGroup{}
-	waitGroup.Add(1)
-
-	userInput := ""
+// We return a channel rather than returning the string directly so that the calling function knows
+// when the prompt has been created (before the user has entered anything) so that it can
+// note that we're now waiting on user input and lazygit isn't processing anything.
+func (self *CredentialsHelper) PromptUserForCredential(passOrUname oscommands.CredentialType) <-chan string {
+	ch := make(chan string)
 
 	self.c.OnUIThread(func() error {
 		title, mask := self.getTitleAndMask(passOrUname)
@@ -33,24 +31,19 @@ func (self *CredentialsHelper) PromptUserForCredential(passOrUname oscommands.Cr
 			Title: title,
 			Mask:  mask,
 			HandleConfirm: func(input string) error {
-				userInput = input
-
-				waitGroup.Done()
+				ch <- input + "\n"
 
 				return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})
 			},
 			HandleClose: func() error {
-				waitGroup.Done()
+				ch <- "\n"
 
 				return nil
 			},
 		})
 	})
 
-	// wait for username/passwords/passphrase input
-	waitGroup.Wait()
-
-	return userInput + "\n"
+	return ch
 }
 
 func (self *CredentialsHelper) getTitleAndMask(passOrUname oscommands.CredentialType) (string, bool) {

pkg/gui/gui.go

@@ -487,6 +487,8 @@ func NewGui(
 		gui.LogCommand,
 		gui.getCmdWriter,
 		credentialsHelper.PromptUserForCredential,
+		func() { gui.g.IncrementBusyCount() },
+		func() { gui.g.DecrementBusyCount() },
 	)
 
 	osCommand := oscommands.NewOSCommand(cmn, config, oscommands.GetPlatform(), guiIO)