Skip to content

Commit f6d49b5

Browse files
committed
Use first class task objects instead of global counter
The global counter approach is easy to understand but it's brittle and depends on implicit behaviour that is not very discoverable. With a global counter, if any goroutine accidentally decrements the counter twice, we'll think lazygit is idle when it's actually busy. Likewise if a goroutine accidentally increments the counter twice we'll think lazygit is busy when it's actually idle. With the new approach we have a map of tasks where each task can either be busy or not. We create a new task and add it to the map when we spawn a worker goroutine (among other things) and we remove it once the task is done. The task can also be paused and continued for situations where we switch back and forth between running a program and asking for user input. In order for this to work with `git push` (and other commands that require credentials) we need to obtain the task from gocui when we create the worker goroutine, and then pass it along to the commands package to pause/continue the task as required. This is MUCH more discoverable than the old approach which just decremented and incremented the global counter from within the commands package, but it's at the cost of expanding some function signatures (arguably a good thing). Likewise, whenever you want to call WithWaitingStatus or WithLoaderPanel the callback will now have access to the task for pausing/ continuing. We only need to actually make use of this functionality in a couple of places so it's a high price to pay, but I don't know if I want to introduce a WithWaitingStatusTask and WithLoaderPanelTask function (open to suggestions).
1 parent ef7c5e3 commit f6d49b5

39 files changed

+302
-219
lines changed

pkg/commands/git_commands/remote.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package git_commands
22

33
import (
44
"fmt"
5+
6+
"github.com/jesseduffield/gocui"
57
)
68

79
type RemoteCommands struct {
@@ -46,12 +48,12 @@ func (self *RemoteCommands) UpdateRemoteUrl(remoteName string, updatedUrl string
4648
return self.cmd.New(cmdArgs).Run()
4749
}
4850

49-
func (self *RemoteCommands) DeleteRemoteBranch(remoteName string, branchName string) error {
51+
func (self *RemoteCommands) DeleteRemoteBranch(task *gocui.Task, remoteName string, branchName string) error {
5052
cmdArgs := NewGitCmd("push").
5153
Arg(remoteName, "--delete", branchName).
5254
ToArgv()
5355

54-
return self.cmd.New(cmdArgs).PromptOnCredentialRequest().WithMutex(self.syncMutex).Run()
56+
return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run()
5557
}
5658

5759
// CheckRemoteBranchExists Returns remote branch

pkg/commands/git_commands/sync.go

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package git_commands
22

33
import (
44
"github.com/go-errors/errors"
5+
"github.com/jesseduffield/gocui"
56
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
67
)
78

@@ -23,7 +24,7 @@ type PushOpts struct {
2324
SetUpstream bool
2425
}
2526

26-
func (self *SyncCommands) PushCmdObj(opts PushOpts) (oscommands.ICmdObj, error) {
27+
func (self *SyncCommands) PushCmdObj(task *gocui.Task, opts PushOpts) (oscommands.ICmdObj, error) {
2728
if opts.UpstreamBranch != "" && opts.UpstreamRemote == "" {
2829
return nil, errors.New(self.Tr.MustSpecifyOriginError)
2930
}
@@ -35,41 +36,37 @@ func (self *SyncCommands) PushCmdObj(opts PushOpts) (oscommands.ICmdObj, error)
3536
ArgIf(opts.UpstreamBranch != "", opts.UpstreamBranch).
3637
ToArgv()
3738

38-
cmdObj := self.cmd.New(cmdArgs).PromptOnCredentialRequest().WithMutex(self.syncMutex)
39+
cmdObj := self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex)
3940
return cmdObj, nil
4041
}
4142

42-
func (self *SyncCommands) Push(opts PushOpts) error {
43-
cmdObj, err := self.PushCmdObj(opts)
43+
func (self *SyncCommands) Push(task *gocui.Task, opts PushOpts) error {
44+
cmdObj, err := self.PushCmdObj(task, opts)
4445
if err != nil {
4546
return err
4647
}
4748

4849
return cmdObj.Run()
4950
}
5051

51-
type FetchOptions struct {
52-
Background bool
53-
}
54-
55-
// Fetch fetch git repo
56-
func (self *SyncCommands) FetchCmdObj(opts FetchOptions) oscommands.ICmdObj {
52+
func (self *SyncCommands) Fetch(task *gocui.Task) error {
5753
cmdArgs := NewGitCmd("fetch").
5854
ArgIf(self.UserConfig.Git.FetchAll, "--all").
5955
ToArgv()
6056

6157
cmdObj := self.cmd.New(cmdArgs)
62-
if opts.Background {
63-
cmdObj.DontLog().FailOnCredentialRequest()
64-
} else {
65-
cmdObj.PromptOnCredentialRequest()
66-
}
67-
return cmdObj.WithMutex(self.syncMutex)
58+
cmdObj.PromptOnCredentialRequest(task)
59+
return cmdObj.WithMutex(self.syncMutex).Run()
6860
}
6961

70-
func (self *SyncCommands) Fetch(opts FetchOptions) error {
71-
cmdObj := self.FetchCmdObj(opts)
72-
return cmdObj.Run()
62+
func (self *SyncCommands) FetchBackground() error {
63+
cmdArgs := NewGitCmd("fetch").
64+
ArgIf(self.UserConfig.Git.FetchAll, "--all").
65+
ToArgv()
66+
67+
cmdObj := self.cmd.New(cmdArgs)
68+
cmdObj.DontLog().FailOnCredentialRequest()
69+
return cmdObj.WithMutex(self.syncMutex).Run()
7370
}
7471

7572
type PullOptions struct {
@@ -78,7 +75,7 @@ type PullOptions struct {
7875
FastForwardOnly bool
7976
}
8077

81-
func (self *SyncCommands) Pull(opts PullOptions) error {
78+
func (self *SyncCommands) Pull(task *gocui.Task, opts PullOptions) error {
8279
cmdArgs := NewGitCmd("pull").
8380
Arg("--no-edit").
8481
ArgIf(opts.FastForwardOnly, "--ff-only").
@@ -88,22 +85,22 @@ func (self *SyncCommands) Pull(opts PullOptions) error {
8885

8986
// setting GIT_SEQUENCE_EDITOR to ':' as a way of skipping it, in case the user
9087
// has 'pull.rebase = interactive' configured.
91-
return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest().WithMutex(self.syncMutex).Run()
88+
return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run()
9289
}
9390

94-
func (self *SyncCommands) FastForward(branchName string, remoteName string, remoteBranchName string) error {
91+
func (self *SyncCommands) FastForward(task *gocui.Task, branchName string, remoteName string, remoteBranchName string) error {
9592
cmdArgs := NewGitCmd("fetch").
9693
Arg(remoteName).
9794
Arg(remoteBranchName + ":" + branchName).
9895
ToArgv()
9996

100-
return self.cmd.New(cmdArgs).PromptOnCredentialRequest().WithMutex(self.syncMutex).Run()
97+
return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run()
10198
}
10299

103-
func (self *SyncCommands) FetchRemote(remoteName string) error {
100+
func (self *SyncCommands) FetchRemote(task *gocui.Task, remoteName string) error {
104101
cmdArgs := NewGitCmd("fetch").
105102
Arg(remoteName).
106103
ToArgv()
107104

108-
return self.cmd.New(cmdArgs).PromptOnCredentialRequest().WithMutex(self.syncMutex).Run()
105+
return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run()
109106
}

pkg/commands/git_commands/tag.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package git_commands
22

3+
import "github.com/jesseduffield/gocui"
4+
35
type TagCommands struct {
46
*GitCommon
57
}
@@ -34,9 +36,9 @@ func (self *TagCommands) Delete(tagName string) error {
3436
return self.cmd.New(cmdArgs).Run()
3537
}
3638

37-
func (self *TagCommands) Push(remoteName string, tagName string) error {
39+
func (self *TagCommands) Push(task *gocui.Task, remoteName string, tagName string) error {
3840
cmdArgs := NewGitCmd("push").Arg(remoteName, "tag", tagName).
3941
ToArgv()
4042

41-
return self.cmd.New(cmdArgs).PromptOnCredentialRequest().WithMutex(self.syncMutex).Run()
43+
return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).WithMutex(self.syncMutex).Run()
4244
}

pkg/commands/oscommands/cmd_obj.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"os/exec"
55
"strings"
66

7+
"github.com/jesseduffield/gocui"
78
"github.com/samber/lo"
89
"github.com/sasha-s/go-deadlock"
910
)
@@ -56,13 +57,14 @@ type ICmdObj interface {
5657
// returns true if IgnoreEmptyError() was called
5758
ShouldIgnoreEmptyError() bool
5859

59-
PromptOnCredentialRequest() ICmdObj
60+
PromptOnCredentialRequest(task *gocui.Task) ICmdObj
6061
FailOnCredentialRequest() ICmdObj
6162

6263
WithMutex(mutex *deadlock.Mutex) ICmdObj
6364
Mutex() *deadlock.Mutex
6465

6566
GetCredentialStrategy() CredentialStrategy
67+
GetTask() *gocui.Task
6668
}
6769

6870
type CmdObj struct {
@@ -85,6 +87,7 @@ type CmdObj struct {
8587

8688
// if set to true, it means we might be asked to enter a username/password by this command.
8789
credentialStrategy CredentialStrategy
90+
task *gocui.Task
8891

8992
// can be set so that we don't run certain commands simultaneously
9093
mutex *deadlock.Mutex
@@ -192,8 +195,9 @@ func (self *CmdObj) RunAndProcessLines(onLine func(line string) (bool, error)) e
192195
return self.runner.RunAndProcessLines(self, onLine)
193196
}
194197

195-
func (self *CmdObj) PromptOnCredentialRequest() ICmdObj {
198+
func (self *CmdObj) PromptOnCredentialRequest(task *gocui.Task) ICmdObj {
196199
self.credentialStrategy = PROMPT
200+
self.task = task
197201

198202
return self
199203
}
@@ -207,3 +211,7 @@ func (self *CmdObj) FailOnCredentialRequest() ICmdObj {
207211
func (self *CmdObj) GetCredentialStrategy() CredentialStrategy {
208212
return self.credentialStrategy
209213
}
214+
215+
func (self *CmdObj) GetTask() *gocui.Task {
216+
return self.task
217+
}

pkg/commands/oscommands/cmd_obj_runner.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"github.com/go-errors/errors"
11+
"github.com/jesseduffield/gocui"
1112
"github.com/jesseduffield/lazygit/pkg/utils"
1213
"github.com/sirupsen/logrus"
1314
)
@@ -308,7 +309,7 @@ func (self *cmdObjRunner) runAndDetectCredentialRequest(
308309
tr := io.TeeReader(handler.stdoutPipe, cmdWriter)
309310

310311
go utils.Safe(func() {
311-
self.processOutput(tr, handler.stdinPipe, promptUserForCredential)
312+
self.processOutput(tr, handler.stdinPipe, promptUserForCredential, cmdObj.GetTask())
312313
})
313314
})
314315
}
@@ -317,6 +318,7 @@ func (self *cmdObjRunner) processOutput(
317318
reader io.Reader,
318319
writer io.Writer,
319320
promptUserForCredential func(CredentialType) <-chan string,
321+
task *gocui.Task,
320322
) {
321323
checkForCredentialRequest := self.getCheckForCredentialRequestFunc()
322324

@@ -327,13 +329,9 @@ func (self *cmdObjRunner) processOutput(
327329
askFor, ok := checkForCredentialRequest(newBytes)
328330
if ok {
329331
responseChan := promptUserForCredential(askFor)
330-
// We assume that the busy count is greater than zero here because we're
331-
// in the middle of a command. We decrement it so that The user can be prompted
332-
// without lazygit thinking it's still doing its own processing. This helps
333-
// integration tests know how long to wait before typing in a response.
334-
self.guiIO.DecrementBusyCount()
332+
task.Pause()
335333
toInput := <-responseChan
336-
self.guiIO.IncrementBusyCount()
334+
task.Continue()
337335
// If the return data is empty we don't write anything to stdin
338336
if toInput != "" {
339337
_, _ = writer.Write([]byte(toInput))

pkg/commands/oscommands/gui_io.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,19 @@ type guiIO struct {
2727
// that a command requests it.
2828
// the 'credential' arg is something like 'username' or 'password'
2929
promptForCredentialFn func(credential CredentialType) <-chan string
30-
31-
IncrementBusyCount func()
32-
DecrementBusyCount func()
3330
}
3431

3532
func NewGuiIO(
3633
log *logrus.Entry,
3734
logCommandFn func(string, bool),
3835
newCmdWriterFn func() io.Writer,
3936
promptForCredentialFn func(CredentialType) <-chan string,
40-
IncrementBusyCount func(),
41-
DecrementBusyCount func(),
4237
) *guiIO {
4338
return &guiIO{
4439
log: log,
4540
logCommandFn: logCommandFn,
4641
newCmdWriterFn: newCmdWriterFn,
4742
promptForCredentialFn: promptForCredentialFn,
48-
IncrementBusyCount: IncrementBusyCount,
49-
DecrementBusyCount: DecrementBusyCount,
5043
}
5144
}
5245

@@ -58,7 +51,5 @@ func NewNullGuiIO(log *logrus.Entry) *guiIO {
5851
logCommandFn: func(string, bool) {},
5952
newCmdWriterFn: func() io.Writer { return io.Discard },
6053
promptForCredentialFn: failPromptFn,
61-
IncrementBusyCount: func() {},
62-
DecrementBusyCount: func() {},
6354
}
6455
}

pkg/gui/background.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"strings"
55
"time"
66

7-
"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
7+
"github.com/jesseduffield/gocui"
88
"github.com/jesseduffield/lazygit/pkg/gui/types"
99
"github.com/jesseduffield/lazygit/pkg/utils"
1010
)
@@ -86,7 +86,7 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru
8686
if self.pauseBackgroundRefreshes {
8787
continue
8888
}
89-
self.gui.c.OnWorker(func() { _ = function() })
89+
self.gui.c.OnWorker(func(*gocui.Task) { _ = function() })
9090
case <-stop:
9191
return
9292
}
@@ -95,7 +95,7 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru
9595
}
9696

9797
func (self *BackgroundRoutineMgr) backgroundFetch() (err error) {
98-
err = self.gui.git.Sync.Fetch(git_commands.FetchOptions{Background: true})
98+
err = self.gui.git.Sync.FetchBackground()
9999

100100
_ = self.gui.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS}, Mode: types.ASYNC})
101101

pkg/gui/controllers/branches_controller.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"strings"
77

8+
"github.com/jesseduffield/gocui"
89
"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
910
"github.com/jesseduffield/lazygit/pkg/commands/models"
1011
"github.com/jesseduffield/lazygit/pkg/gui/context"
@@ -363,11 +364,12 @@ func (self *BranchesController) fastForward(branch *models.Branch) error {
363364
},
364365
)
365366

366-
return self.c.WithLoaderPanel(message, func() error {
367+
return self.c.WithLoaderPanel(message, func(task *gocui.Task) error {
367368
if branch == self.c.Helpers().Refs.GetCheckedOutRef() {
368369
self.c.LogAction(action)
369370

370371
err := self.c.Git().Sync.Pull(
372+
task,
371373
git_commands.PullOptions{
372374
RemoteName: branch.UpstreamRemote,
373375
BranchName: branch.UpstreamBranch,
@@ -381,7 +383,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error {
381383
return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})
382384
} else {
383385
self.c.LogAction(action)
384-
err := self.c.Git().Sync.FastForward(branch.Name, branch.UpstreamRemote, branch.UpstreamBranch)
386+
err := self.c.Git().Sync.FastForward(task, branch.Name, branch.UpstreamRemote, branch.UpstreamBranch)
385387
if err != nil {
386388
_ = self.c.Error(err)
387389
}

pkg/gui/controllers/commits_files_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (self *CommitFilesController) discard(node *filetree.CommitFileNode) error
177177
Title: self.c.Tr.DiscardFileChangesTitle,
178178
Prompt: prompt,
179179
HandleConfirm: func() error {
180-
return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func() error {
180+
return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(*gocui.Task) error {
181181
self.c.LogAction(self.c.Tr.Actions.DiscardOldFileChange)
182182
if err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), node.GetPath()); err != nil {
183183
if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil {
@@ -205,7 +205,7 @@ func (self *CommitFilesController) edit(node *filetree.CommitFileNode) error {
205205

206206
func (self *CommitFilesController) toggleForPatch(node *filetree.CommitFileNode) error {
207207
toggle := func() error {
208-
return self.c.WithWaitingStatus(self.c.Tr.UpdatingPatch, func() error {
208+
return self.c.WithWaitingStatus(self.c.Tr.UpdatingPatch, func(*gocui.Task) error {
209209
if !self.c.Git().Patch.PatchBuilder.Active() {
210210
if err := self.startPatchBuilder(); err != nil {
211211
return err

0 commit comments

Comments
 (0)