diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 59a704b27..7e304ea44 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -8,6 +8,7 @@ import ( "regexp" "strconv" "strings" + "sync" "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/generics/slices" @@ -15,6 +16,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/common" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -84,31 +86,60 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, commits = append(commits, rebasingCommits...) } + wg := sync.WaitGroup{} + + wg.Add(2) + + var logErr error + go utils.Safe(func() { + defer wg.Done() + + logErr = self.getLogCmd(opts).RunAndProcessLines(func(line string) (bool, error) { + commit := self.extractCommitFromLine(line) + commits = append(commits, commit) + return false, nil + }) + }) + + var ancestor string + go utils.Safe(func() { + defer wg.Done() + + ancestor = self.getMergeBase(opts.RefName) + }) + passedFirstPushedCommit := false + // I can get this before firstPushedCommit, err := self.getFirstPushedCommit(opts.RefName) if err != nil { // must have no upstream branch so we'll consider everything as pushed passedFirstPushedCommit = true } - err = self.getLogCmd(opts).RunAndProcessLines(func(line string) (bool, error) { - commit := self.extractCommitFromLine(line) + wg.Wait() + + if logErr != nil { + return nil, logErr + } + + for _, commit := range commits { if commit.Sha == firstPushedCommit { passedFirstPushedCommit = true } - commit.Status = map[bool]models.CommitStatus{true: models.StatusUnpushed, false: models.StatusPushed}[!passedFirstPushedCommit] - commits = append(commits, commit) - return false, nil - }) - if err != nil { - return nil, err + if passedFirstPushedCommit { + commit.Status = models.StatusPushed + } else { + commit.Status = models.StatusUnpushed + } } if len(commits) == 0 { return commits, nil } - commits = self.setCommitMergedStatuses(opts.RefName, commits) + if ancestor != "" { + commits = self.setCommitMergedStatuses(ancestor, commits) + } return commits, nil } @@ -464,11 +495,7 @@ func (self *CommitLoader) commitFromPatch(content string) *models.Commit { } } -func (self *CommitLoader) setCommitMergedStatuses(refName string, commits []*models.Commit) []*models.Commit { - ancestor := self.getMergeBase(refName) - if ancestor == "" { - return commits - } +func (self *CommitLoader) setCommitMergedStatuses(ancestor string, commits []*models.Commit) []*models.Commit { passedAncestor := false for i, commit := range commits { if strings.HasPrefix(ancestor, commit.Sha) { @@ -510,13 +537,25 @@ func (self *CommitLoader) getMergeBase(refName string) string { } func (self *CommitLoader) getExistingMainBranches() []string { - return lo.FilterMap(self.UserConfig.Git.MainBranches, - func(branchName string, _ int) (string, bool) { + var existingBranches []string + var wg sync.WaitGroup + + mainBranches := self.UserConfig.Git.MainBranches + existingBranches = make([]string, len(mainBranches)) + + for i, branchName := range mainBranches { + wg.Add(1) + i := i + branchName := branchName + go utils.Safe(func() { + defer wg.Done() + // Try to determine upstream of local main branch if ref, err := self.cmd.New( NewGitCmd("rev-parse").Arg("--symbolic-full-name", branchName+"@{u}").ToArgv(), ).DontLog().RunWithOutput(); err == nil { - return strings.TrimSpace(ref), true + existingBranches[i] = strings.TrimSpace(ref) + return } // If this failed, a local branch for this main branch doesn't exist or it @@ -525,7 +564,8 @@ func (self *CommitLoader) getExistingMainBranches() []string { if err := self.cmd.New( NewGitCmd("rev-parse").Arg("--verify", "--quiet", ref).ToArgv(), ).DontLog().Run(); err == nil { - return ref, true + existingBranches[i] = ref + return } // If this failed as well, try if we have the main branch as a local @@ -535,11 +575,18 @@ func (self *CommitLoader) getExistingMainBranches() []string { if err := self.cmd.New( NewGitCmd("rev-parse").Arg("--verify", "--quiet", ref).ToArgv(), ).DontLog().Run(); err == nil { - return ref, true + existingBranches[i] = ref } - - return "", false }) + } + + wg.Wait() + + existingBranches = lo.Filter(existingBranches, func(branch string, _ int) bool { + return branch != "" + }) + + return existingBranches } func ignoringWarnings(commandOutput string) string { diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 8398613a8..bcbc33c02 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -1,12 +1,12 @@ package git_commands import ( - "errors" "path/filepath" "strings" "testing" "github.com/fsmiamoto/git-todo-parser/todo" + "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" diff --git a/pkg/commands/git_commands/deps_test.go b/pkg/commands/git_commands/deps_test.go index 5926584b3..a960098df 100644 --- a/pkg/commands/git_commands/deps_test.go +++ b/pkg/commands/git_commands/deps_test.go @@ -118,7 +118,7 @@ func buildWorkingTreeCommands(deps commonDeps) *WorkingTreeCommands { return NewWorkingTreeCommands(gitCommon, submoduleCommands, fileLoader) } -func buildPatchCommands(deps commonDeps) *PatchCommands { +func buildPatchCommands(deps commonDeps) *PatchCommands { //nolint:golint,unused gitCommon := buildGitCommon(deps) rebaseCommands := buildRebaseCommands(deps) commitCommands := buildCommitCommands(deps) @@ -132,7 +132,7 @@ func buildPatchCommands(deps commonDeps) *PatchCommands { return NewPatchCommands(gitCommon, rebaseCommands, commitCommands, statusCommands, stashCommands, patchBuilder) } -func buildStatusCommands(deps commonDeps) *StatusCommands { +func buildStatusCommands(deps commonDeps) *StatusCommands { //nolint:golint,unused gitCommon := buildGitCommon(deps) return NewStatusCommands(gitCommon) diff --git a/pkg/commands/git_commands/patch_test.go b/pkg/commands/git_commands/patch_test.go deleted file mode 100644 index 8acf471d4..000000000 --- a/pkg/commands/git_commands/patch_test.go +++ /dev/null @@ -1,68 +0,0 @@ -package git_commands - -import ( - "fmt" - "os" - "testing" - - "github.com/go-errors/errors" - "github.com/jesseduffield/lazygit/pkg/commands/oscommands" - "github.com/stretchr/testify/assert" -) - -func TestPatchApplyPatch(t *testing.T) { - type scenario struct { - testName string - opts ApplyPatchOpts - runner *oscommands.FakeCmdObjRunner - test func(error) - } - - // expectedArgs excludes the last argument which is an indeterminate filename - expectFn := func(expectedArgs []string, errToReturn error) func(cmdObj oscommands.ICmdObj) (string, error) { - return func(cmdObj oscommands.ICmdObj) (string, error) { - args := cmdObj.Args() - - assert.Equal(t, len(args), len(expectedArgs)+1, fmt.Sprintf("unexpected command: %s", cmdObj.ToString())) - - filename := args[len(args)-1] - - content, err := os.ReadFile(filename) - assert.NoError(t, err) - - assert.Equal(t, "test", string(content)) - - return "", errToReturn - } - } - - scenarios := []scenario{ - { - testName: "valid case", - opts: ApplyPatchOpts{Cached: true}, - runner: oscommands.NewFakeRunner(t). - ExpectFunc(expectFn([]string{"git", "apply", "--cached"}, nil)), - test: func(err error) { - assert.NoError(t, err) - }, - }, - { - testName: "command returns error", - opts: ApplyPatchOpts{Cached: true}, - runner: oscommands.NewFakeRunner(t). - ExpectFunc(expectFn([]string{"git", "apply", "--cached"}, errors.New("error"))), - test: func(err error) { - assert.Error(t, err) - }, - }, - } - - for _, s := range scenarios { - s := s - t.Run(s.testName, func(t *testing.T) { - instance := buildPatchCommands(commonDeps{runner: s.runner}) - s.test(instance.ApplyPatch("test", s.opts)) - s.runner.CheckForMissingCalls() - }) - } -} diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index b616ec609..d10746220 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -79,7 +79,7 @@ func TestRebaseRebaseBranch(t *testing.T) { // environment variables that suppress an interactive editor func TestRebaseSkipEditorCommand(t *testing.T) { cmdArgs := []string{"git", "blah"} - runner := oscommands.NewFakeRunner(t).ExpectFunc(func(cmdObj oscommands.ICmdObj) (string, error) { + runner := oscommands.NewFakeRunner(t).ExpectFunc("matches editor env var", func(cmdObj oscommands.ICmdObj) bool { assert.EqualValues(t, cmdArgs, cmdObj.Args()) envVars := cmdObj.GetEnvVars() for _, regexStr := range []string{ @@ -94,11 +94,11 @@ func TestRebaseSkipEditorCommand(t *testing.T) { return regexp.MustCompile(regexStr).MatchString(envVar) }) if !foundMatch { - t.Errorf("expected environment variable %s to be set", regexStr) + return false } } - return "", nil - }) + return true + }, "", nil) instance := buildRebaseCommands(commonDeps{runner: runner}) err := instance.runSkipEditorCommand(instance.cmd.New(cmdArgs)) assert.NoError(t, err) diff --git a/pkg/commands/git_commands/reflog_commit_loader.go b/pkg/commands/git_commands/reflog_commit_loader.go index 5033b03c1..1160839d6 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -35,34 +35,19 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit ToArgv() cmdObj := self.cmd.New(cmdArgs).DontLog() + onlyObtainedNewReflogCommits := false err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { - fields := strings.SplitN(line, "\x00", 4) - if len(fields) <= 3 { + commit, ok := self.parseLine(line) + if !ok { return false, nil } - unixTimestamp, _ := strconv.Atoi(fields[1]) - - parentHashes := fields[3] - parents := []string{} - if len(parentHashes) > 0 { - parents = strings.Split(parentHashes, " ") - } - - commit := &models.Commit{ - Sha: fields[0], - Name: fields[2], - UnixTimestamp: int64(unixTimestamp), - Status: models.StatusReflog, - Parents: parents, - } - // note that the unix timestamp here is the timestamp of the COMMIT, not the reflog entry itself, // so two consecutive reflog entries may have both the same SHA and therefore same timestamp. // We use the reflog message to disambiguate, and fingers crossed that we never see the same of those // twice in a row. Reason being that it would mean we'd be erroneously exiting early. - if lastReflogCommit != nil && commit.Sha == lastReflogCommit.Sha && commit.UnixTimestamp == lastReflogCommit.UnixTimestamp && commit.Name == lastReflogCommit.Name { + if lastReflogCommit != nil && self.sameReflogCommit(commit, lastReflogCommit) { onlyObtainedNewReflogCommits = true // after this point we already have these reflogs loaded so we'll simply return the new ones return true, nil @@ -77,3 +62,30 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit return commits, onlyObtainedNewReflogCommits, nil } + +func (self *ReflogCommitLoader) sameReflogCommit(a *models.Commit, b *models.Commit) bool { + return a.Sha == b.Sha && a.UnixTimestamp == b.UnixTimestamp && a.Name == b.Name +} + +func (self *ReflogCommitLoader) parseLine(line string) (*models.Commit, bool) { + fields := strings.SplitN(line, "\x00", 4) + if len(fields) <= 3 { + return nil, false + } + + unixTimestamp, _ := strconv.Atoi(fields[1]) + + parentHashes := fields[3] + parents := []string{} + if len(parentHashes) > 0 { + parents = strings.Split(parentHashes, " ") + } + + return &models.Commit{ + Sha: fields[0], + Name: fields[2], + UnixTimestamp: int64(unixTimestamp), + Status: models.StatusReflog, + Parents: parents, + }, true +} diff --git a/pkg/commands/git_commands/remote_loader.go b/pkg/commands/git_commands/remote_loader.go index 553a8f0c0..c2a8da025 100644 --- a/pkg/commands/git_commands/remote_loader.go +++ b/pkg/commands/git_commands/remote_loader.go @@ -1,15 +1,15 @@ package git_commands import ( - "fmt" - "regexp" "strings" + "sync" "github.com/jesseduffield/generics/slices" gogit "github.com/jesseduffield/go-git/v5" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" + "github.com/jesseduffield/lazygit/pkg/utils" ) type RemoteLoader struct { @@ -31,29 +31,31 @@ func NewRemoteLoader( } func (self *RemoteLoader) GetRemotes() ([]*models.Remote, error) { - cmdArgs := NewGitCmd("branch").Arg("-r").ToArgv() - remoteBranchesStr, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput() - if err != nil { - return nil, err - } + wg := sync.WaitGroup{} + wg.Add(1) + + var remoteBranchesByRemoteName map[string][]*models.RemoteBranch + var remoteBranchesErr error + go utils.Safe(func() { + defer wg.Done() + + remoteBranchesByRemoteName, remoteBranchesErr = self.getRemoteBranchesByRemoteName() + }) goGitRemotes, err := self.getGoGitRemotes() if err != nil { return nil, err } - // first step is to get our remotes from go-git + wg.Wait() + + if remoteBranchesErr != nil { + return nil, remoteBranchesErr + } + remotes := slices.Map(goGitRemotes, func(goGitRemote *gogit.Remote) *models.Remote { remoteName := goGitRemote.Config().Name - - re := regexp.MustCompile(fmt.Sprintf(`(?m)^\s*%s\/([\S]+)`, regexp.QuoteMeta(remoteName))) - matches := re.FindAllStringSubmatch(remoteBranchesStr, -1) - branches := slices.Map(matches, func(match []string) *models.RemoteBranch { - return &models.RemoteBranch{ - Name: match[1], - RemoteName: remoteName, - } - }) + branches := remoteBranchesByRemoteName[remoteName] return &models.Remote{ Name: goGitRemote.Config().Name, @@ -76,3 +78,42 @@ func (self *RemoteLoader) GetRemotes() ([]*models.Remote, error) { return remotes, nil } + +func (self *RemoteLoader) getRemoteBranchesByRemoteName() (map[string][]*models.RemoteBranch, error) { + remoteBranchesByRemoteName := make(map[string][]*models.RemoteBranch) + + cmdArgs := NewGitCmd("branch").Arg("-r").ToArgv() + err := self.cmd.New(cmdArgs).DontLog().RunAndProcessLines(func(line string) (bool, error) { + // excluding lines like 'origin/HEAD -> origin/master' (there will be a separate + // line for 'origin/master') + if strings.Contains(line, "->") { + return false, nil + } + + line = strings.TrimSpace(line) + + split := strings.SplitN(line, "/", 2) + if len(split) != 2 { + return false, nil + } + remoteName := split[0] + name := split[1] + + _, ok := remoteBranchesByRemoteName[remoteName] + if !ok { + remoteBranchesByRemoteName[remoteName] = []*models.RemoteBranch{} + } + + remoteBranchesByRemoteName[remoteName] = append(remoteBranchesByRemoteName[remoteName], + &models.RemoteBranch{ + Name: name, + RemoteName: remoteName, + }) + return false, nil + }) + if err != nil { + return nil, err + } + + return remoteBranchesByRemoteName, nil +} diff --git a/pkg/commands/oscommands/cmd_obj_runner.go b/pkg/commands/oscommands/cmd_obj_runner.go index b82e3a25c..23ec47070 100644 --- a/pkg/commands/oscommands/cmd_obj_runner.go +++ b/pkg/commands/oscommands/cmd_obj_runner.go @@ -6,6 +6,7 @@ import ( "io" "regexp" "strings" + "time" "github.com/go-errors/errors" "github.com/jesseduffield/gocui" @@ -102,10 +103,14 @@ func (self *cmdObjRunner) RunWithOutputAux(cmdObj ICmdObj) (string, error) { self.logCmdObj(cmdObj) } + t := time.Now() output, err := sanitisedCommandOutput(cmdObj.GetCmd().CombinedOutput()) if err != nil { self.log.WithField("command", cmdObj.ToString()).Error(output) } + + self.log.Infof("%s (%s)", cmdObj.ToString(), time.Since(t)) + return output, err } @@ -116,12 +121,15 @@ func (self *cmdObjRunner) RunWithOutputsAux(cmdObj ICmdObj) (string, string, err self.logCmdObj(cmdObj) } + t := time.Now() var outBuffer, errBuffer bytes.Buffer cmd := cmdObj.GetCmd() cmd.Stdout = &outBuffer cmd.Stderr = &errBuffer err := cmd.Run() + self.log.Infof("%s (%s)", cmdObj.ToString(), time.Since(t)) + stdout := outBuffer.String() stderr, err := sanitisedCommandOutput(errBuffer.Bytes(), err) if err != nil { @@ -144,6 +152,7 @@ func (self *cmdObjRunner) RunAndProcessLines(cmdObj ICmdObj, onLine func(line st if cmdObj.ShouldLog() { self.logCmdObj(cmdObj) } + t := time.Now() cmd := cmdObj.GetCmd() stdoutPipe, err := cmd.StdoutPipe() @@ -171,6 +180,8 @@ func (self *cmdObjRunner) RunAndProcessLines(cmdObj ICmdObj, onLine func(line st _ = cmd.Wait() + self.log.Infof("%s (%s)", cmdObj.ToString(), time.Since(t)) + return nil } @@ -237,9 +248,14 @@ func (self *cmdObjRunner) runAndStreamAux( } }() + t := time.Now() + onRun(handler, cmdWriter) err = cmd.Wait() + + self.log.Infof("%s (%s)", cmdObj.ToString(), time.Since(t)) + if err != nil { errStr := stderr.String() if errStr != "" { diff --git a/pkg/commands/oscommands/fake_cmd_obj_runner.go b/pkg/commands/oscommands/fake_cmd_obj_runner.go index fa34f01be..dc75228a0 100644 --- a/pkg/commands/oscommands/fake_cmd_obj_runner.go +++ b/pkg/commands/oscommands/fake_cmd_obj_runner.go @@ -6,18 +6,36 @@ import ( "regexp" "runtime" "strings" + "sync" "testing" "github.com/go-errors/errors" - "github.com/stretchr/testify/assert" + "github.com/samber/lo" + "golang.org/x/exp/slices" ) // for use in testing type FakeCmdObjRunner struct { - t *testing.T - expectedCmds []func(ICmdObj) (string, error) - expectedCmdIndex int + t *testing.T + // commands can be run in any order; mimicking the concurrent behaviour of + // production code. + expectedCmds []CmdObjMatcher + + invokedCmdIndexes []int + + mutex sync.Mutex +} + +type CmdObjMatcher struct { + description string + // returns true if the matcher matches the command object + test func(ICmdObj) bool + + // output of the command + output string + // error of the command + err error } var _ ICmdObjRunner = &FakeCmdObjRunner{} @@ -26,23 +44,40 @@ func NewFakeRunner(t *testing.T) *FakeCmdObjRunner { //nolint:thelper return &FakeCmdObjRunner{t: t} } +func (self *FakeCmdObjRunner) remainingExpectedCmds() []CmdObjMatcher { + return lo.Filter(self.expectedCmds, func(_ CmdObjMatcher, i int) bool { + return !lo.Contains(self.invokedCmdIndexes, i) + }) +} + func (self *FakeCmdObjRunner) Run(cmdObj ICmdObj) error { _, err := self.RunWithOutput(cmdObj) return err } func (self *FakeCmdObjRunner) RunWithOutput(cmdObj ICmdObj) (string, error) { - if self.expectedCmdIndex > len(self.expectedCmds)-1 { + self.mutex.Lock() + defer self.mutex.Unlock() + + if len(self.remainingExpectedCmds()) == 0 { self.t.Errorf("ran too many commands. Unexpected command: `%s`", cmdObj.ToString()) return "", errors.New("ran too many commands") } - expectedCmd := self.expectedCmds[self.expectedCmdIndex] - output, err := expectedCmd(cmdObj) + for i := range self.expectedCmds { + if lo.Contains(self.invokedCmdIndexes, i) { + continue + } + expectedCmd := self.expectedCmds[i] + matched := expectedCmd.test(cmdObj) + if matched { + self.invokedCmdIndexes = append(self.invokedCmdIndexes, i) + return expectedCmd.output, expectedCmd.err + } + } - self.expectedCmdIndex++ - - return output, err + self.t.Errorf("Unexpected command: `%s`", cmdObj.ToString()) + return "", nil } func (self *FakeCmdObjRunner) RunWithOutputs(cmdObj ICmdObj) (string, string, error) { @@ -72,63 +107,84 @@ func (self *FakeCmdObjRunner) RunAndProcessLines(cmdObj ICmdObj, onLine func(lin return nil } -func (self *FakeCmdObjRunner) ExpectFunc(fn func(cmdObj ICmdObj) (string, error)) *FakeCmdObjRunner { - self.expectedCmds = append(self.expectedCmds, fn) +func (self *FakeCmdObjRunner) ExpectFunc(description string, fn func(cmdObj ICmdObj) bool, output string, err error) *FakeCmdObjRunner { + self.mutex.Lock() + defer self.mutex.Unlock() - return self -} - -func (self *FakeCmdObjRunner) Expect(expectedCmdStr string, output string, err error) *FakeCmdObjRunner { - self.ExpectFunc(func(cmdObj ICmdObj) (string, error) { - cmdStr := cmdObj.ToString() - assert.Equal(self.t, expectedCmdStr, cmdStr, fmt.Sprintf("expected command %d to be %s, but was %s", self.expectedCmdIndex+1, expectedCmdStr, cmdStr)) - - return output, err + self.expectedCmds = append(self.expectedCmds, CmdObjMatcher{ + test: fn, + output: output, + err: err, + description: description, }) return self } func (self *FakeCmdObjRunner) ExpectArgs(expectedArgs []string, output string, err error) *FakeCmdObjRunner { - self.ExpectFunc(func(cmdObj ICmdObj) (string, error) { + description := fmt.Sprintf("matches args %s", strings.Join(expectedArgs, " ")) + self.ExpectFunc(description, func(cmdObj ICmdObj) bool { args := cmdObj.GetCmd().Args if runtime.GOOS == "windows" { // thanks to the secureexec package, the first arg is something like // '"C:\\Program Files\\Git\\mingw64\\bin\\.exe" // on windows so we'll just ensure it contains our program - assert.Contains(self.t, args[0], expectedArgs[0]) + if !strings.Contains(args[0], expectedArgs[0]) { + return false + } } else { // first arg is the program name - assert.Equal(self.t, expectedArgs[0], args[0]) + if expectedArgs[0] != args[0] { + return false + } } - assert.EqualValues(self.t, expectedArgs[1:], args[1:], fmt.Sprintf("command %d did not match expectation", self.expectedCmdIndex+1)) + if !slices.Equal(expectedArgs[1:], args[1:]) { + return false + } - return output, err - }) + return true + }, output, err) return self } func (self *FakeCmdObjRunner) ExpectGitArgs(expectedArgs []string, output string, err error) *FakeCmdObjRunner { - self.ExpectFunc(func(cmdObj ICmdObj) (string, error) { + description := fmt.Sprintf("matches git args %s", strings.Join(expectedArgs, " ")) + self.ExpectFunc(description, func(cmdObj ICmdObj) bool { // first arg is 'git' on unix and something like '"C:\\Program Files\\Git\\mingw64\\bin\\git.exe" on windows so we'll just ensure it ends in either 'git' or 'git.exe' re := regexp.MustCompile(`git(\.exe)?$`) args := cmdObj.GetCmd().Args if !re.MatchString(args[0]) { - self.t.Errorf("expected first arg to end in .git or .git.exe but was %s", args[0]) + return false } - assert.EqualValues(self.t, expectedArgs, args[1:], fmt.Sprintf("command %d did not match expectation", self.expectedCmdIndex+1)) - return output, err - }) + if !slices.Equal(expectedArgs, args[1:]) { + return false + } + + return true + }, output, err) return self } func (self *FakeCmdObjRunner) CheckForMissingCalls() { - if self.expectedCmdIndex < len(self.expectedCmds) { - self.t.Errorf("expected command %d to be called, but was not", self.expectedCmdIndex+1) + self.mutex.Lock() + defer self.mutex.Unlock() + + remaining := self.remainingExpectedCmds() + if len(remaining) > 0 { + self.t.Errorf( + "expected %d more command(s) to be run. Remaining commands:\n%s", + len(remaining), + strings.Join( + lo.Map(remaining, func(cmdObj CmdObjMatcher, _ int) string { + return cmdObj.description + }), + "\n", + ), + ) } } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 5a70414b6..c8fbe8627 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" "sync" + "time" "github.com/jesseduffield/generics/set" "github.com/jesseduffield/generics/slices" @@ -51,6 +52,11 @@ func NewRefreshHelper( } func (self *RefreshHelper) Refresh(options types.RefreshOptions) error { + t := time.Now() + defer func() { + self.c.Log.Infof(fmt.Sprintf("Refresh took %s", time.Since(t))) + }() + if options.Scope == nil { self.c.Log.Infof( "refreshing all scopes in %s mode", @@ -85,59 +91,78 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error { scopeSet = set.NewFromSlice(options.Scope) } - refresh := func(f func()) { + wg := sync.WaitGroup{} + refresh := func(name string, f func()) { if options.Mode == types.ASYNC { self.c.OnWorker(func(t gocui.Task) { f() }) } else { - f() + wg.Add(1) + go utils.Safe(func() { + t := time.Now() + defer wg.Done() + f() + self.c.Log.Infof(fmt.Sprintf("refreshed %s in %s", name, time.Since(t))) + }) } } if scopeSet.Includes(types.COMMITS) || scopeSet.Includes(types.BRANCHES) || scopeSet.Includes(types.REFLOG) || scopeSet.Includes(types.BISECT_INFO) { - refresh(self.refreshCommits) + // whenever we change commits, we should update branches because the upstream/downstream + // counts can change. Whenever we change branches we should also change commits + // e.g. in the case of switching branches. + refresh("commits and commit files", self.refreshCommitsAndCommitFiles) + refresh("reflog and branches", self.refreshReflogAndBranches) } else if scopeSet.Includes(types.REBASE_COMMITS) { // the above block handles rebase commits so we only need to call this one // if we've asked specifically for rebase commits and not those other things - refresh(func() { _ = self.refreshRebaseCommits() }) + refresh("rebase commits", func() { _ = self.refreshRebaseCommits() }) } if scopeSet.Includes(types.SUB_COMMITS) { - refresh(func() { _ = self.refreshSubCommitsWithLimit() }) + refresh("sub commits", func() { _ = self.refreshSubCommitsWithLimit() }) } // reason we're not doing this if the COMMITS type is included is that if the COMMITS type _is_ included we will refresh the commit files context anyway if scopeSet.Includes(types.COMMIT_FILES) && !scopeSet.Includes(types.COMMITS) { - refresh(func() { _ = self.refreshCommitFilesContext() }) + refresh("commit files", func() { _ = self.refreshCommitFilesContext() }) } + fileWg := sync.WaitGroup{} if scopeSet.Includes(types.FILES) || scopeSet.Includes(types.SUBMODULES) { - refresh(func() { _ = self.refreshFilesAndSubmodules() }) + fileWg.Add(1) + refresh("files", func() { + _ = self.refreshFilesAndSubmodules() + fileWg.Done() + }) } if scopeSet.Includes(types.STASH) { - refresh(func() { _ = self.refreshStashEntries() }) + refresh("stash", func() { _ = self.refreshStashEntries() }) } if scopeSet.Includes(types.TAGS) { - refresh(func() { _ = self.refreshTags() }) + refresh("tags", func() { _ = self.refreshTags() }) } if scopeSet.Includes(types.REMOTES) { - refresh(func() { _ = self.refreshRemotes() }) + refresh("remotes", func() { _ = self.refreshRemotes() }) } if scopeSet.Includes(types.STAGING) { - refresh(func() { _ = self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{}) }) + refresh("staging", func() { + fileWg.Wait() + _ = self.stagingHelper.RefreshStagingPanel(types.OnFocusOpts{}) + }) } if scopeSet.Includes(types.PATCH_BUILDING) { - refresh(func() { _ = self.patchBuildingHelper.RefreshPatchBuildingPanel(types.OnFocusOpts{}) }) + refresh("patch building", func() { _ = self.patchBuildingHelper.RefreshPatchBuildingPanel(types.OnFocusOpts{}) }) } if scopeSet.Includes(types.MERGE_CONFLICTS) || scopeSet.Includes(types.FILES) { - refresh(func() { _ = self.mergeConflictsHelper.RefreshMergeState() }) + refresh("merge conflicts", func() { _ = self.mergeConflictsHelper.RefreshMergeState() }) } self.refreshStatus() @@ -145,6 +170,8 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) error { if options.Then != nil { options.Then() } + + wg.Wait() } if options.Mode == types.BLOCK_UI { @@ -212,41 +239,29 @@ func (self *RefreshHelper) refreshReflogCommitsConsideringStartup() { } } -// whenever we change commits, we should update branches because the upstream/downstream -// counts can change. Whenever we change branches we should probably also change commits -// e.g. in the case of switching branches. -func (self *RefreshHelper) refreshCommits() { - wg := sync.WaitGroup{} - wg.Add(2) +func (self *RefreshHelper) refreshReflogAndBranches() { + self.refreshReflogCommitsConsideringStartup() - go utils.Safe(func() { - self.refreshReflogCommitsConsideringStartup() + self.refreshBranches() +} - self.refreshBranches() - wg.Done() - }) - - go utils.Safe(func() { - _ = self.refreshCommitsWithLimit() - ctx, ok := self.c.Contexts().CommitFiles.GetParentContext() - if ok && ctx.GetKey() == context.LOCAL_COMMITS_CONTEXT_KEY { - // This makes sense when we've e.g. just amended a commit, meaning we get a new commit SHA at the same position. - // However if we've just added a brand new commit, it pushes the list down by one and so we would end up - // showing the contents of a different commit than the one we initially entered. - // Ideally we would know when to refresh the commit files context and when not to, - // or perhaps we could just pop that context off the stack whenever cycling windows. - // For now the awkwardness remains. - commit := self.c.Contexts().LocalCommits.GetSelected() - if commit != nil { - self.c.Contexts().CommitFiles.SetRef(commit) - self.c.Contexts().CommitFiles.SetTitleRef(commit.RefName()) - _ = self.refreshCommitFilesContext() - } +func (self *RefreshHelper) refreshCommitsAndCommitFiles() { + _ = self.refreshCommitsWithLimit() + ctx, ok := self.c.Contexts().CommitFiles.GetParentContext() + if ok && ctx.GetKey() == context.LOCAL_COMMITS_CONTEXT_KEY { + // This makes sense when we've e.g. just amended a commit, meaning we get a new commit SHA at the same position. + // However if we've just added a brand new commit, it pushes the list down by one and so we would end up + // showing the contents of a different commit than the one we initially entered. + // Ideally we would know when to refresh the commit files context and when not to, + // or perhaps we could just pop that context off the stack whenever cycling windows. + // For now the awkwardness remains. + commit := self.c.Contexts().LocalCommits.GetSelected() + if commit != nil { + self.c.Contexts().CommitFiles.SetRef(commit) + self.c.Contexts().CommitFiles.SetTitleRef(commit.RefName()) + _ = self.refreshCommitFilesContext() } - wg.Done() - }) - - wg.Wait() + } } func (self *RefreshHelper) refreshCommitsWithLimit() error { diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 85ec21128..58c973712 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -1,6 +1,8 @@ package gui import ( + "time" + "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/tasks" @@ -129,6 +131,11 @@ func (gui *Gui) render() { // if the context's view is set to another context we do nothing. // if the context's view is the current view we trigger a focus; re-selecting the current item. func (gui *Gui) postRefreshUpdate(c types.Context) error { + t := time.Now() + defer func() { + gui.Log.Infof("postRefreshUpdate for %s took %s", c.GetKey(), time.Since(t)) + }() + if err := c.HandleRender(); err != nil { return err }