diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 8b79bd8cd..381dd641e 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -174,7 +174,7 @@ func (self *CommitLoader) MergeRebasingCommits(hashPool *utils.StringPool, commi } if workingTreeState.Rebasing { - rebasingCommits, err := self.getHydratedRebasingCommits(hashPool, addConflictedRebasingCommit) + rebasingCommits, err := self.getHydratedRebasingCommits(hashPool, commits, addConflictedRebasingCommit) if err != nil { return nil, err } @@ -251,8 +251,8 @@ func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line }) } -func (self *CommitLoader) getHydratedRebasingCommits(hashPool *utils.StringPool, addConflictingCommit bool) ([]*models.Commit, error) { - return self.getHydratedTodoCommits(hashPool, self.getRebasingCommits(hashPool, addConflictingCommit), false) +func (self *CommitLoader) getHydratedRebasingCommits(hashPool *utils.StringPool, existingCommits []*models.Commit, addConflictingCommit bool) ([]*models.Commit, error) { + return self.getHydratedTodoCommits(hashPool, self.getRebasingCommits(hashPool, addConflictingCommit), existingCommits, false) } func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool, workingTreeState models.WorkingTreeState) ([]*models.Commit, error) { @@ -271,39 +271,56 @@ func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool } } - return self.getHydratedTodoCommits(hashPool, commits, true) + return self.getHydratedTodoCommits(hashPool, commits, nil, true) } -func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) { +func (self *CommitLoader) getHydratedTodoCommits( + hashPool *utils.StringPool, + todoCommits []*models.Commit, + existingCommits []*models.Commit, + todoFileHasShortHashes bool, +) ([]*models.Commit, error) { if len(todoCommits) == 0 { return nil, nil } - commitHashes := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) { - return commit.Hash(), commit.Hash() != "" - }) - - // note that we're not filtering these as we do non-rebasing commits just because - // I suspect that will cause some damage - cmdObj := self.cmd.New( - NewGitCmd("show"). - Config("log.showSignature=false"). - Arg("--no-patch", "--oneline", "--abbrev=20", prettyFormat). - Arg(commitHashes...). - ToArgv(), - ).DontLog() - + // A refresh of only the rebasing todos should reuse the already loaded todos to avoid + // unnecessary git show calls. fullCommits := map[string]*models.Commit{} - err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { - if line == "" || line[0] != '+' { - return false, nil + for _, commit := range existingCommits { + if commit.IsTODO() && commit.Hash() != "" { + // Make a copy of the commit; that's necessary to avoid mutating the original commit + // when we later reuse it in the loop at the end of this function. + fullCommits[commit.Hash()] = lo.ToPtr(*commit) } - commit := self.extractCommitFromLine(hashPool, line[1:], false) - fullCommits[commit.Hash()] = commit - return false, nil + } + + commitHashesToFetch := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) { + return commit.Hash(), commit.Hash() != "" && fullCommits[commit.Hash()] == nil }) - if err != nil { - return nil, err + + if len(commitHashesToFetch) > 0 { + // note that we're not filtering these as we do non-rebasing commits just because + // I suspect that will cause some damage + cmdObj := self.cmd.New( + NewGitCmd("show"). + Config("log.showSignature=false"). + Arg("--no-patch", "--oneline", "--abbrev=20", prettyFormat). + Arg(commitHashesToFetch...). + ToArgv(), + ).DontLog() + + err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { + if line == "" || line[0] != '+' { + return false, nil + } + commit := self.extractCommitFromLine(hashPool, line[1:], false) + fullCommits[commit.Hash()] = commit + return false, nil + }) + if err != nil { + return nil, err + } } findFullCommit := lo.Ternary(todoFileHasShortHashes, diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 7f9873b0b..d26119720 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -538,6 +538,110 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { } } +func TestCommitLoaderGetHydratedTodoCommitsReusesExistingCommit(t *testing.T) { + hashPool := &utils.StringPool{} + runner := oscommands.NewFakeRunner(t) + loader := &CommitLoader{ + cmd: oscommands.NewDummyCmdObjBuilder(runner), + } + existingCommit := models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: "0123456789012345678901234567890123456789", + Name: "hydrated subject", + AuthorName: "Jane Doe", + AuthorEmail: "jane@example.com", + UnixTimestamp: 1234, + Parents: []string{"1123456789012345678901234567890123456789"}, + Status: models.StatusRebasing, + Action: todo.Pick, + }) + refreshedTodo := models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: existingCommit.Hash(), + Name: "subject from the todo file", + Status: models.StatusConflicted, + Action: todo.Fixup, + ActionFlag: "-C", + }) + + commits, err := loader.getHydratedTodoCommits( + hashPool, + []*models.Commit{refreshedTodo}, + []*models.Commit{existingCommit}, + false, + ) + + assert.NoError(t, err) + assert.Equal(t, []*models.Commit{ + models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: existingCommit.Hash(), + Name: "hydrated subject", + AuthorName: "Jane Doe", + AuthorEmail: "jane@example.com", + UnixTimestamp: 1234, + Parents: []string{"1123456789012345678901234567890123456789"}, + Status: models.StatusConflicted, + Action: todo.Fixup, + ActionFlag: "-C", + }), + }, commits) + assert.Equal(t, todo.Pick, existingCommit.Action) + assert.Equal(t, models.StatusRebasing, existingCommit.Status) + runner.CheckForMissingCalls() +} + +func TestCommitLoaderGetHydratedTodoCommitsLoadsMissingCommit(t *testing.T) { + hashPool := &utils.StringPool{} + existingHash := "0123456789012345678901234567890123456789" + missingHash := "2123456789012345678901234567890123456789" + missingCommitOutput := strings.ReplaceAll( + `+2123456789012345678901234567890123456789|1235|John Doe|john@example.com||>|tag: new|new subject`, + "|", + "\x00", + ) + runner := oscommands.NewFakeRunner(t).ExpectGitArgs( + []string{ + "-c", "log.showSignature=false", "show", "--no-patch", "--oneline", "--abbrev=20", + prettyFormat, missingHash, + }, + missingCommitOutput, + nil, + ) + loader := &CommitLoader{ + cmd: oscommands.NewDummyCmdObjBuilder(runner), + } + existingCommit := models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: existingHash, + Name: "existing subject", + Status: models.StatusRebasing, + Action: todo.Pick, + }) + refreshedTodos := []*models.Commit{ + models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: existingHash, + Status: models.StatusRebasing, + Action: todo.Pick, + }), + models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: missingHash, + Status: models.StatusRebasing, + Action: todo.Edit, + }), + } + + commits, err := loader.getHydratedTodoCommits( + hashPool, + refreshedTodos, + []*models.Commit{existingCommit}, + false, + ) + + assert.NoError(t, err) + assert.Len(t, commits, 2) + assert.Equal(t, "existing subject", commits[0].Name) + assert.Equal(t, "new subject", commits[1].Name) + assert.Equal(t, todo.Edit, commits[1].Action) + runner.CheckForMissingCalls() +} + func TestCommitLoader_setCommitStatuses(t *testing.T) { type scenario struct { testName string diff --git a/pkg/gui/context/simple_context.go b/pkg/gui/context/simple_context.go index 83d201f3a..626d5dfcf 100644 --- a/pkg/gui/context/simple_context.go +++ b/pkg/gui/context/simple_context.go @@ -41,7 +41,7 @@ func (self *SimpleContext) HandleFocus(opts types.OnFocusOpts) { fn(opts) } - if self.onRenderToMainFn != nil { + if self.onRenderToMainFn != nil && !opts.SkipMainViewUpdate { self.onRenderToMainFn() } } diff --git a/pkg/gui/context/worktrees_context.go b/pkg/gui/context/worktrees_context.go index 3e45f2d45..690fa6d4b 100644 --- a/pkg/gui/context/worktrees_context.go +++ b/pkg/gui/context/worktrees_context.go @@ -16,8 +16,8 @@ var _ types.IListContext = (*WorktreesContext)(nil) func NewWorktreesContext(c *ContextCommon) *WorktreesContext { viewModel := NewFilteredListViewModel( func() []*models.Worktree { return c.Model().Worktrees }, - func(Worktree *models.Worktree) []string { - return []string{Worktree.Name} + func(worktree *models.Worktree) []string { + return []string{worktree.Name, worktree.Branch} }, ) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 941bd3b9c..730ed9a24 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -119,6 +119,9 @@ type refreshEnv struct { // reload state (see RefreshOptions.DontBlockRepoSwitch). keepScrollPosition bool + // Whether refreshing a side context should leave the main view unchanged. + skipMainViewUpdate bool + // the repo generation captured when the refresh started generation int @@ -231,6 +234,7 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr background: options.Background || options.DontBlockRepoSwitch, backgroundRoutine: options.Background, keepScrollPosition: options.Background || options.DontBlockRepoSwitch, + skipMainViewUpdate: options.SkipMainViewUpdate, } if !self.captureOnUIThread(calledFromWorker, env.background, func() { env.generation = self.c.State().GetRepoGeneration() @@ -1670,11 +1674,10 @@ func (self *RefreshHelper) refreshView(context types.Context, env refreshEnv) { // the filtered list model is up to date for rendering. self.searchHelper.ReApplyFilter(context) - if env.keepScrollPosition { - self.c.PostRefreshUpdateKeepingScrollPosition(context) - } else { - self.c.PostRefreshUpdate(context) - } + self.c.PostRefreshUpdateWithOptions(context, types.OnFocusOpts{ + KeepScrollPosition: env.keepScrollPosition, + SkipMainViewUpdate: env.skipMainViewUpdate, + }) self.c.AfterLayout(func() error { // Re-applying the search must be done after re-rendering the view though, @@ -1853,7 +1856,8 @@ func (self *RefreshHelper) setGithubPullRequests(baseInfo *githubRemoteInfo, bra // This lands whenever the network call happens to return, and only // changes how the branches are rendered, not which one is selected, so // it has no business moving the viewport. - self.c.PostRefreshUpdateKeepingScrollPosition(self.c.Contexts().Branches) + self.c.PostRefreshUpdateWithOptions(self.c.Contexts().Branches, + types.OnFocusOpts{KeepScrollPosition: true}) }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 9407e3dee..884cbc941 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -191,7 +191,8 @@ func (self *LocalCommitsController) handleCommitDrag(opts gocui.ViewMouseBinding self.commitDrag.hasMoved = true if self.updateCommitDragInsertion(opts.Y) { - self.c.PostRefreshUpdateKeepingScrollPosition(self.context()) + self.c.PostRefreshUpdateWithOptions(self.context(), + types.OnFocusOpts{KeepScrollPosition: true}) } originY := self.context().GetView().OriginY() self.dragAutoscroller.Update(opts.Y - originY) @@ -344,7 +345,9 @@ func (self *LocalCommitsController) startMovingCommitsIndicator(insertionIndex i func (self *LocalCommitsController) stopMovingCommitsIndicator() { self.stopMovingCommitsIndicatorTicker() self.context().ClearDropInsertionIndex() - self.c.PostRefreshUpdate(self.context()) + self.c.PostRefreshUpdateWithOptions( + self.context(), types.OnFocusOpts{SkipMainViewUpdate: true}, + ) } func (self *LocalCommitsController) stopMovingCommitsIndicatorTicker() { @@ -1170,16 +1173,21 @@ func (self *LocalCommitsController) move( if err := self.c.Git().Rebase.MoveTodos(selectedCommits, offset); err != nil { return err } - self.context().MoveSelection(offset) - self.context().HandleFocus(types.OnFocusOpts{}) // Block input until the refresh has landed: a quick second press must // read the moved todo from the refreshed model, not grab whatever the // advanced selection index points at in the stale one. self.c.RefreshBlockingInput(types.RefreshOptions{ - Scope: []types.RefreshableView{types.REBASE_COMMITS}, - CommitSelection: types.KeepCommitSelectionIndex, - Then: onComplete, + Scope: []types.RefreshableView{types.REBASE_COMMITS}, + SkipMainViewUpdate: true, + Then: func() error { + self.context().MoveSelection(offset) + self.context().FocusLine(true) + if onComplete != nil { + return onComplete() + } + return nil + }, }) return nil } diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index d92284ea5..692df5142 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -39,11 +39,11 @@ func (self *guiCommon) RefreshFromWorker(opts types.RefreshOptions) { } func (self *guiCommon) PostRefreshUpdate(context types.Context) { - self.gui.postRefreshUpdate(context, false) + self.gui.postRefreshUpdate(context, types.OnFocusOpts{}) } -func (self *guiCommon) PostRefreshUpdateKeepingScrollPosition(context types.Context) { - self.gui.postRefreshUpdate(context, true) +func (self *guiCommon) PostRefreshUpdateWithOptions(context types.Context, opts types.OnFocusOpts) { + self.gui.postRefreshUpdate(context, opts) } func (self *guiCommon) RunSubprocessAndRefresh(cmdObj *oscommands.CmdObj) error { diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index ff73b91f6..92cc141cf 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -51,10 +51,9 @@ type IGuiCommon interface { // case would be overkill, although refresh will internally call 'PostRefreshUpdate'. // It re-focuses the context's selection, which scrolls it into view. PostRefreshUpdate(Context) - // Like PostRefreshUpdate, but leaves the view scrolled where it is. For - // refreshes that no user action is behind: those must not move the viewport - // away from wherever the user last put it. - PostRefreshUpdateKeepingScrollPosition(Context) + // Like PostRefreshUpdate, with control over scrolling and whether to update + // the main view. + PostRefreshUpdateWithOptions(Context, OnFocusOpts) // renders string to a view without resetting its origin SetViewContent(view *gocui.View, content string) diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 35662d86c..128c4f08f 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -234,6 +234,10 @@ type OnFocusOpts struct { // the view's scroll position alone instead; only for callers that maintain // it themselves, e.g. by keeping the selection at the edge of the viewport. KeepScrollPosition bool + + // Set this when the focused item hasn't changed and the main view's current + // content is still valid. + SkipMainViewUpdate bool } type OnFocusLostOpts struct { diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index c733e589e..d40a1bec5 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -72,6 +72,10 @@ type RefreshOptions struct { // letting each scope update the UI as soon as it's done. BatchUIUpdates bool + // Set this when the refresh doesn't invalidate the main view's current + // content, so refreshing the side context needn't render it again. + SkipMainViewUpdate bool + // Controls which local branch is selected after the refresh. Defaults to // KeepBranchSelectionByName. BranchSelection BranchSelectionBehavior diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index e9ad48aab..4e14e6c48 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -132,7 +132,7 @@ func (gui *Gui) renderContentOnly() { // postRefreshUpdate is to be called on a context after the state that it depends on has been refreshed // 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, keepScrollPosition bool) { +func (gui *Gui) postRefreshUpdate(c types.Context, opts types.OnFocusOpts) { t := time.Now() defer func() { gui.Log.Infof("postRefreshUpdate for %s took %s", c.GetKey(), time.Since(t)) @@ -141,14 +141,17 @@ func (gui *Gui) postRefreshUpdate(c types.Context, keepScrollPosition bool) { c.HandleRender() if gui.currentViewName() == c.GetViewName() { - c.HandleFocus(types.OnFocusOpts{KeepScrollPosition: keepScrollPosition}) + c.HandleFocus(opts) } else { // The FocusLine call is included in the HandleFocus method which we // call for focused views above; but we need to call it here for // non-focused views to ensure that an inactive selection is painted // correctly, and that integration tests see the up to date selection // state. - c.FocusLine(!keepScrollPosition) + c.FocusLine(!opts.KeepScrollPosition) + if opts.SkipMainViewUpdate { + return + } currentCtx := gui.State.ContextMgr.Current() if currentCtx.GetKey() == context.NORMAL_MAIN_CONTEXT_KEY || currentCtx.GetKey() == context.NORMAL_SECONDARY_CONTEXT_KEY { diff --git a/pkg/integration/tests/filter_and_search/filter_worktrees.go b/pkg/integration/tests/filter_and_search/filter_worktrees.go new file mode 100644 index 000000000..77dbcc744 --- /dev/null +++ b/pkg/integration/tests/filter_and_search/filter_worktrees.go @@ -0,0 +1,35 @@ +package filter_and_search + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FilterWorktrees = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Filtering worktrees by branch name", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("initial commit") + shell.NewBranch("branch-aaa") + shell.NewBranch("branch-xxx") + shell.Checkout("master") + shell.AddWorktreeCheckout("branch-aaa", "../worktree-xxx") + shell.AddWorktreeCheckout("branch-xxx", "../worktree-1") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Worktrees(). + Focus(). + Lines( + Contains("(main worktree)").IsSelected(), + Contains("worktree-1 branch-xxx"), + Contains("worktree-xxx branch-aaa"), + ). + FilterOrSearch("xxx"). + Lines( + Contains("worktree-1 branch-xxx"), + Contains("worktree-xxx branch-aaa"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 0124de268..bf4cfd5df 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -270,6 +270,7 @@ var tests = []*components.IntegrationTest{ filter_and_search.FilterRemotes, filter_and_search.FilterSearchHistory, filter_and_search.FilterUpdatesWhenModelChanges, + filter_and_search.FilterWorktrees, filter_and_search.NestedFilter, filter_and_search.NestedFilterTransient, filter_and_search.NewSearch,