diff --git a/pkg/gui/controllers/bisect_controller.go b/pkg/gui/controllers/bisect_controller.go index 685f932b1..b6c82d49c 100644 --- a/pkg/gui/controllers/bisect_controller.go +++ b/pkg/gui/controllers/bisect_controller.go @@ -6,7 +6,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" - "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" @@ -59,20 +58,6 @@ func (self *BisectController) openMenu(commit *models.Commit) error { } func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, commit *models.Commit) error { - // if there is not yet a 'current' bisect commit, or if we have - // selected the current commit, we need to jump to the next 'current' commit - // after we perform a bisect action. The reason we don't unconditionally jump - // is that sometimes the user will want to go and mark a few commits as skipped - // in a row and they wouldn't want to be jumped back to the current bisect - // commit each time. - // Originally we were allowing the user to, from the bisect menu, select whether - // they were talking about the selected commit or the current bisect commit, - // and that was a bit confusing (and required extra keypresses). - selectCurrentAfter := info.GetCurrentHash() == "" || info.GetCurrentHash() == commit.Hash() - // we need to wait to reselect if our bisect commits aren't ancestors of our 'start' - // ref, because we'll be reloading our commits in that case. - waitToReselect := selectCurrentAfter && !self.c.Git().Bisect.ReachableFromStart(info) - // If we have a current hash already, then we always want to use that one. If // not, we're still picking the initial commits before we really start, so // use the selected commit in that case. @@ -98,7 +83,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c return err } - return self.afterMark(selectCurrentAfter, waitToReselect) + return self.afterMark(types.KeepCommitSelectionByHash) }, DisabledReason: singleItemIfNotBisecting, Keys: menuKey('b'), @@ -111,7 +96,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c return err } - return self.afterMark(selectCurrentAfter, waitToReselect) + return self.afterMark(types.KeepCommitSelectionByHash) }, DisabledReason: singleItemIfNotBisecting, Keys: menuKey('g'), @@ -124,7 +109,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c return err } - return self.afterMark(selectCurrentAfter, waitToReselect) + return self.afterMark(types.KeepCommitSelectionByHash) }, DisabledReason: singleItemIfNotBisecting, Keys: menuKey('s'), @@ -139,7 +124,10 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c return err } - return self.afterMark(selectCurrentAfter, waitToReselect) + // Git may pick a different commit to test now. Keep the skipped + // commit selected anyway, so that the user can go on skipping + // commits near it. + return self.afterMark(types.KeepCommitSelectionIndex) }, DisabledReason: self.require(self.singleItemSelected())(), Keys: menuKey('S'), @@ -256,15 +244,13 @@ func (self *BisectController) showBisectCompleteMessage(candidateHashes []string return nil } -func (self *BisectController) afterMark(selectCurrent bool, waitToReselect bool) error { +func (self *BisectController) afterMark(commitSelection types.CommitSelectionBehavior) error { done, candidateHashes, err := self.c.Git().Bisect.IsDone() if err != nil { return err } - if err := self.afterBisectMarkRefresh(selectCurrent, waitToReselect); err != nil { - return err - } + self.c.Refresh(types.RefreshOptions{CommitSelection: commitSelection}) if done { return self.showBisectCompleteMessage(candidateHashes) @@ -272,42 +258,3 @@ func (self *BisectController) afterMark(selectCurrent bool, waitToReselect bool) return nil } - -func (self *BisectController) afterBisectMarkRefresh(selectCurrent bool, waitToReselect bool) error { - selectFn := func() error { - if selectCurrent { - self.selectCurrentBisectCommit() - } - return nil - } - - if waitToReselect { - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{}, Then: selectFn}) - return nil - } - - if err := selectFn(); err != nil { - return err - } - - self.c.Helpers().Bisect.PostBisectCommandRefresh() - return nil -} - -func (self *BisectController) selectCurrentBisectCommit() { - info := self.c.Git().Bisect.GetInfo() - if info.GetCurrentHash() != "" { - // find index of commit with that hash, move cursor to that. - for i, commit := range self.c.Model().Commits { - if commit.Hash() == info.GetCurrentHash() { - self.context().SetSelection(i) - self.context().HandleFocus(types.OnFocusOpts{}) - break - } - } - } -} - -func (self *BisectController) context() *context.LocalCommitsContext { - return self.c.Contexts().LocalCommits -} diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index e8477c35b..4a7aa6f0c 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -870,11 +870,14 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, self.onUIThreadUnlessRepoChanged(env, func() { var selectionRange *localCommitSelectionRange - var newConflictedCommitIdx *int + var jumpToCommitIdx *int if commitSelection == types.KeepCommitSelectionByHash { selectedIdx, rangeStartIdx, rangeSelectMode := self.c.Contexts().LocalCommits.GetSelectionRangeAndMode() selectionRange = captureLocalCommitSelectionRange(self.c.Model().Commits, selectedIdx, rangeStartIdx, rangeSelectMode) - newConflictedCommitIdx = findNewConflictedCommit(self.c.Model().Commits, commits) + jumpToCommitIdx = findNewConflictedCommit(self.c.Model().Commits, commits) + if jumpToCommitIdx == nil { + jumpToCommitIdx = findNewBisectCommitToSelect(self.c.Model().BisectInfo, bisectInfo, commits) + } } self.c.Model().BisectInfo = bisectInfo @@ -894,8 +897,8 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, self.c.Contexts().LocalCommits.SetSelection(headCommitIdx) } case types.KeepCommitSelectionByHash: - if newConflictedCommitIdx != nil { - self.c.Contexts().LocalCommits.SetSelection(*newConflictedCommitIdx) + if jumpToCommitIdx != nil { + self.c.Contexts().LocalCommits.SetSelection(*jumpToCommitIdx) } else if selectionRange != nil { selectedIdx, rangeStartIdx, found := findLocalCommitSelectionRange(commits, selectionRange) if found { @@ -907,7 +910,7 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, } focusOpts := env.focusOpts() - if newConflictedCommitIdx != nil { + if jumpToCommitIdx != nil { // We selected a commit that the user needs to look at next, so // scroll it into view even if the refresh keeps the scroll position // otherwise. @@ -1007,6 +1010,35 @@ func findNewConflictedCommit(previousCommits []*models.Commit, commits []*models return nil } +// Returns the index of the commit that the user needs to look at during a +// bisect, if it is in the new commits slice and it is a different commit than +// at the previous refresh (or there wasn't one then). Otherwise returns nil. +func findNewBisectCommitToSelect(previousBisectInfo *git_commands.BisectInfo, bisectInfo *git_commands.BisectInfo, commits []*models.Commit) *int { + hash := bisectCommitToSelect(bisectInfo) + if hash == "" || hash == bisectCommitToSelect(previousBisectInfo) { + return nil + } + + _, idx, found := lo.FindIndexOf(commits, func(commit *models.Commit) bool { + return commit.Hash() == hash + }) + if found { + return &idx + } + + return nil +} + +// Returns the commit that the user needs to look at during a bisect: the one +// that git wants them to test. Returns "" if there's none. +func bisectCommitToSelect(bisectInfo *git_commands.BisectInfo) string { + if !bisectInfo.Bisecting() { + return "" + } + + return bisectInfo.GetCurrentHash() +} + // capturedSubCommitState holds the sub-commits refresh's model/context/mode // inputs, gathered on the UI thread (see captureSubCommitState) before the git // work is dispatched to a worker. diff --git a/pkg/integration/tests/bisect/from_other_branch.go b/pkg/integration/tests/bisect/from_other_branch.go index b65c88594..77d4904c9 100644 --- a/pkg/integration/tests/bisect/from_other_branch.go +++ b/pkg/integration/tests/bisect/from_other_branch.go @@ -6,7 +6,7 @@ import ( ) var FromOtherBranch = NewIntegrationTest(NewIntegrationTestArgs{ - Description: "Opening lazygit when bisect has been started from another branch. There's an issue where we don't reselect the current branch if we mark the current branch as bad so this test side-steps that problem", + Description: "Opening lazygit when bisect has been started from another branch", ExtraCmdArgs: []string{}, Skip: false, SetupRepo: func(shell *Shell) { @@ -25,11 +25,10 @@ var FromOtherBranch = NewIntegrationTest(NewIntegrationTestArgs{ Focus(). TopLines( MatchesRegexp(`<-- bad.*commit-08`), - MatchesRegexp(`<-- current.*commit-07`), + MatchesRegexp(`<-- current.*commit-07`).IsSelected(), MatchesRegexp(`\?.*commit-06`), MatchesRegexp(`<-- good.*commit-05`), ). - SelectNextItem(). Press(keys.Commits.ViewBisectOptions). Tap(func() { t.ExpectPopup().Menu().Title(Equals("Bisect")).Select(MatchesRegexp(`Mark .* as good`)).Confirm() diff --git a/pkg/integration/tests/bisect/select_current_commit.go b/pkg/integration/tests/bisect/select_current_commit.go new file mode 100644 index 000000000..4f575eae6 --- /dev/null +++ b/pkg/integration/tests/bisect/select_current_commit.go @@ -0,0 +1,35 @@ +package bisect + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SelectCurrentCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Select the current bisect commit whenever it changes, but not on every refresh", + ExtraCmdArgs: []string{}, + Skip: false, + SetupRepo: func(shell *Shell) { + shell. + CreateNCommits(10). + StartBisect("HEAD", "HEAD~9") + }, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Git.Log.ShowGraph = "never" + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + SelectedLine(Contains("CI commit-05").Contains("<-- current")). + NavigateToLine(Contains("CI commit-08")). + Tap(func() { + t.RefreshInBackground() + }). + SelectedLine(Contains("CI commit-08")). + Tap(func() { + t.Shell().RunCommand([]string{"git", "bisect", "good"}) + t.RefreshInBackground() + }). + SelectedLine(Contains("CI commit-07").Contains("<-- current")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 17f97924c..3fab3959d 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -38,6 +38,7 @@ var tests = []*components.IntegrationTest{ bisect.Basic, bisect.ChooseTerms, bisect.FromOtherBranch, + bisect.SelectCurrentCommit, bisect.Skip, branch.CheckoutAutostash, branch.CheckoutByName,