diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 57f9f586b..6d03b7f74 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -861,9 +861,11 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, self.onUIThreadUnlessRepoChanged(env, func() { var selectionRange *localCommitSelectionRange + var newConflictedCommitIdx *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) } self.c.Model().BisectInfo = bisectInfo @@ -883,7 +885,9 @@ func (self *RefreshHelper) refreshCommitsWithLimit(captured capturedCommitState, self.c.Contexts().LocalCommits.SetSelection(headCommitIdx) } case types.KeepCommitSelectionByHash: - if selectionRange != nil { + if newConflictedCommitIdx != nil { + self.c.Contexts().LocalCommits.SetSelection(*newConflictedCommitIdx) + } else if selectionRange != nil { selectedIdx, rangeStartIdx, found := findLocalCommitSelectionRange(commits, selectionRange) if found { self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, selectionRange.mode) @@ -968,6 +972,24 @@ func hasRestorableCommitHash(commits []*models.Commit, idx int) bool { return idx >= 0 && idx < len(commits) && commits[idx].Hash() != "" } +// Returns the index of the conflicted commit in the new commits slice, if there is one and it has a +// different hash than the one before had (or there wasn't one before). Otherwise returns nil. +func findNewConflictedCommit(previousCommits []*models.Commit, commits []*models.Commit) *int { + previousConflictedCommit, _ := lo.Find(previousCommits, func(commit *models.Commit) bool { + return commit.Status == models.StatusConflicted + }) + + newConflictedCommit, idx, hasConflict := lo.FindIndexOf(commits, func(commit *models.Commit) bool { + return commit.Status == models.StatusConflicted + }) + + if hasConflict && (previousConflictedCommit == nil || previousConflictedCommit.Hash() != newConflictedCommit.Hash()) { + return &idx + } + + return nil +} + // 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/gui/controllers/helpers/refresh_helper_test.go b/pkg/gui/controllers/helpers/refresh_helper_test.go index b92a64285..5e58c56a3 100644 --- a/pkg/gui/controllers/helpers/refresh_helper_test.go +++ b/pkg/gui/controllers/helpers/refresh_helper_test.go @@ -152,6 +152,62 @@ func TestFindLocalCommitSelectionRange(t *testing.T) { } } +func TestFindNewConflictedCommit(t *testing.T) { + testCases := []struct { + name string + previousCommits []*models.Commit + commits []*models.Commit + expectedIdx *int + }{ + { + name: "finds a newly conflicted commit", + previousCommits: makeCommits("a", "b"), + commits: []*models.Commit{ + makeCommits("a")[0], + makeConflictedCommit("b"), + }, + expectedIdx: lo.ToPtr(1), + }, + { + name: "finds a different conflicted commit", + previousCommits: []*models.Commit{ + makeConflictedCommit("a"), + }, + commits: []*models.Commit{ + makeConflictedCommit("b"), + }, + expectedIdx: lo.ToPtr(0), + }, + { + name: "ignores the same conflicted commit", + previousCommits: []*models.Commit{ + makeConflictedCommit("a"), + }, + commits: []*models.Commit{ + makeConflictedCommit("a"), + }, + expectedIdx: nil, + }, + { + name: "reports not found when there is no conflict", + previousCommits: makeCommits("a"), + commits: makeCommits("a", "b"), + expectedIdx: nil, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + idx := findNewConflictedCommit(testCase.previousCommits, testCase.commits) + + assert.Equal(t, testCase.expectedIdx != nil, idx != nil) + if idx != nil { + assert.Equal(t, *testCase.expectedIdx, *idx) + } + }) + } +} + func TestGetGithubBaseRemote(t *testing.T) { cases := []struct { name string @@ -320,3 +376,7 @@ func makeTodoCommit(action todo.TodoCommand) *models.Commit { func makeTodoCommitWithHash(hash string, action todo.TodoCommand) *models.Commit { return models.NewCommit(&utils.StringPool{}, models.NewCommitOpts{Hash: hash, Action: action}) } + +func makeConflictedCommit(hash string) *models.Commit { + return models.NewCommit(&utils.StringPool{}, models.NewCommitOpts{Hash: hash, Status: models.StatusConflicted}) +} diff --git a/pkg/integration/tests/branch/rebase_and_drop.go b/pkg/integration/tests/branch/rebase_and_drop.go index 2e8ef5c39..bbb8abf00 100644 --- a/pkg/integration/tests/branch/rebase_and_drop.go +++ b/pkg/integration/tests/branch/rebase_and_drop.go @@ -54,15 +54,15 @@ var RebaseAndDrop = NewIntegrationTest(NewIntegrationTestArgs{ Focus(). TopLines( Contains("─── Pending rebase todos"), - MatchesRegexp(`pick.*to keep`).IsSelected(), + MatchesRegexp(`pick.*to keep`), MatchesRegexp(`pick.*to remove`), - MatchesRegexp(`pick.*CONFLICT.*first change`), + MatchesRegexp(`pick.*CONFLICT.*first change`).IsSelected(), Contains("─── Commits"), MatchesRegexp("second-change-branch unrelated change"), MatchesRegexp("second change"), MatchesRegexp("original"), ). - SelectNextItem(). + NavigateToLine(Contains("to remove")). Press(keys.Universal.Remove). TopLines( Contains("─── Pending rebase todos"), diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go index 7468f921c..abdfcae82 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go @@ -79,10 +79,9 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{ Focus(). TopLines( Contains("second-change-branch unrelated change"), - Contains("second change"), - Contains("first change").IsSelected(), + Contains("second change").IsSelected(), + Contains("first change"), ). - SelectPreviousItem(). Tap(func() { // because we picked 'Second change' when resolving the conflict, // we now see this commit as having replaced First Change with Second Change, diff --git a/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_amend.go b/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_amend.go index 7b8bf1ca1..8ef3368d6 100644 --- a/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_amend.go +++ b/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_amend.go @@ -31,7 +31,7 @@ var AmendWhenThereAreConflictsAndAmend = NewIntegrationTest(NewIntegrationTestAr Lines( Contains("─── Pending rebase todos"), Contains("pick").Contains("commit three"), - Contains("pick").Contains("<-- CONFLICT --- file1 changed in branch"), + Contains("pick").Contains("<-- CONFLICT --- file1 changed in branch").IsSelected(), Contains("─── Commits"), Contains("commit two"), Contains("file1 changed in master"), diff --git a/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_cancel.go b/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_cancel.go index 91eff7d59..fe7c67ddf 100644 --- a/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_cancel.go +++ b/pkg/integration/tests/commit/amend_when_there_are_conflicts_and_cancel.go @@ -35,7 +35,7 @@ var AmendWhenThereAreConflictsAndCancel = NewIntegrationTest(NewIntegrationTestA Lines( Contains("─── Pending rebase todos"), Contains("pick").Contains("commit three"), - Contains("pick").Contains("<-- CONFLICT --- file1 changed in branch"), + Contains("pick").Contains("<-- CONFLICT --- file1 changed in branch").IsSelected(), Contains("─── Commits"), Contains("commit two"), Contains("file1 changed in master"), diff --git a/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go b/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go index 6e25a8496..5ad60aad5 100644 --- a/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go +++ b/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go @@ -46,7 +46,7 @@ var RevertWithConflictMultipleCommits = NewIntegrationTest(NewIntegrationTestArg Lines( Contains("─── Pending reverts"), Contains("revert").Contains("CI unrelated change"), - Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("revert").Contains("CI <-- CONFLICT --- add first line").IsSelected(), Contains("─── Commits"), Contains("CI ○ add second line"), Contains("CI ○ add first line"), diff --git a/pkg/integration/tests/commit/revert_with_conflict_single_commit.go b/pkg/integration/tests/commit/revert_with_conflict_single_commit.go index 57679160d..374b40338 100644 --- a/pkg/integration/tests/commit/revert_with_conflict_single_commit.go +++ b/pkg/integration/tests/commit/revert_with_conflict_single_commit.go @@ -40,7 +40,7 @@ var RevertWithConflictSingleCommit = NewIntegrationTest(NewIntegrationTestArgs{ }). Lines( Contains("─── Pending reverts"), - Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("revert").Contains("CI <-- CONFLICT --- add first line").IsSelected(), Contains("─── Commits"), Contains("CI ○ add second line"), Contains("CI ○ add first line"), diff --git a/pkg/integration/tests/commit/shared.go b/pkg/integration/tests/commit/shared.go index e143eb7a0..c1a66c13e 100644 --- a/pkg/integration/tests/commit/shared.go +++ b/pkg/integration/tests/commit/shared.go @@ -45,7 +45,7 @@ func doTheRebaseForAmendTests(t *TestDriver, keys config.KeybindingConfig) { Lines( Contains("─── Pending rebase todos"), Contains("pick").Contains("commit three"), - Contains("pick").Contains("<-- CONFLICT --- file1 changed in branch"), + Contains("pick").Contains("<-- CONFLICT --- file1 changed in branch").IsSelected(), Contains("─── Commits"), Contains("commit two"), Contains("file1 changed in master"), diff --git a/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go b/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go index 497db50d7..ade48d389 100644 --- a/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go +++ b/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go @@ -36,7 +36,7 @@ var AmendCommitWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ Lines( Contains("─── Pending rebase todos"), Contains("pick").Contains("three"), - Contains("fixup").Contains("<-- CONFLICT --- fixup! two"), + Contains("fixup").Contains("<-- CONFLICT --- fixup! two").IsSelected(), Contains("─── Commits"), Contains("two"), Contains("one"), @@ -69,7 +69,7 @@ var AmendCommitWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Lines( Contains("─── Pending rebase todos"), - Contains("<-- CONFLICT --- three"), + Contains("<-- CONFLICT --- three").IsSelected(), Contains("─── Commits"), Contains("two"), Contains("one"), diff --git a/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go b/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go index 81ddf685a..b395e4747 100644 --- a/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go +++ b/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go @@ -33,7 +33,7 @@ var EditTheConflCommit = NewIntegrationTest(NewIntegrationTestArgs{ Focus(). Lines( Contains("─── Pending rebase todos"), - Contains("pick").Contains("commit two"), + Contains("pick").Contains("commit two").IsSelected(), Contains("pick").Contains("<-- CONFLICT --- commit three"), Contains("─── Commits"), Contains("commit one"), diff --git a/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go b/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go index da3285f04..630edc823 100644 --- a/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go +++ b/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go @@ -55,7 +55,7 @@ var RevertMultipleCommitsInInteractiveRebase = NewIntegrationTest(NewIntegration Contains("CI unrelated change 2"), Contains("─── Pending reverts"), Contains("revert").Contains("CI unrelated change 1"), - Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("revert").Contains("CI <-- CONFLICT --- add first line").IsSelected(), Contains("─── Commits"), Contains("CI ○ add second line"), Contains("CI ○ add first line"), diff --git a/pkg/integration/tests/interactive_rebase/revert_single_commit_in_interactive_rebase.go b/pkg/integration/tests/interactive_rebase/revert_single_commit_in_interactive_rebase.go index 722fa95f2..d4ba4312d 100644 --- a/pkg/integration/tests/interactive_rebase/revert_single_commit_in_interactive_rebase.go +++ b/pkg/integration/tests/interactive_rebase/revert_single_commit_in_interactive_rebase.go @@ -49,10 +49,10 @@ var RevertSingleCommitInInteractiveRebase = NewIntegrationTest(NewIntegrationTes Contains("CI unrelated change 2"), Contains("CI unrelated change 1"), Contains("─── Pending reverts"), - Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("revert").Contains("CI <-- CONFLICT --- add first line").IsSelected(), Contains("─── Commits"), Contains("CI ○ add second line"), - Contains("CI ○ add first line").IsSelected(), + Contains("CI ○ add first line"), Contains("CI ○ add empty file"), ). Press(keys.Commits.MoveDownCommit). diff --git a/pkg/integration/tests/interactive_rebase/shared.go b/pkg/integration/tests/interactive_rebase/shared.go index d1d80fafb..522f425c1 100644 --- a/pkg/integration/tests/interactive_rebase/shared.go +++ b/pkg/integration/tests/interactive_rebase/shared.go @@ -4,14 +4,25 @@ import ( . "github.com/jesseduffield/lazygit/pkg/integration/components" ) -func handleConflictsFromSwap(t *TestDriver, expectedCommand string) { +func handleConflictsFromSwap(t *TestDriver, expectedCommand string, selectConflict bool) { t.Common().AcknowledgeConflicts() + // If the conflict comes from directly moving a commit, we want to keep the moved commit + // selected, so selectConflict is false. In other cases (e.g. a conflict after "continue + // rebase") we want to select the conflict commit. + commitTwoMatcher := Contains("pick").Contains("commit two") + conflictMatcher := Contains(expectedCommand).Contains("<-- CONFLICT --- commit three") + if selectConflict { + conflictMatcher.IsSelected() + } else { + commitTwoMatcher.IsSelected() + } + t.Views().Commits(). Lines( Contains("─── Pending rebase todos"), - Contains("pick").Contains("commit two"), - Contains(expectedCommand).Contains("<-- CONFLICT --- commit three"), + commitTwoMatcher, + conflictMatcher, Contains("─── Commits"), Contains("commit one"), ) diff --git a/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict.go b/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict.go index 20b77c03f..693c37a9b 100644 --- a/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict.go +++ b/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict.go @@ -48,6 +48,6 @@ var SwapInRebaseWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ t.Common().ContinueRebase() }) - handleConflictsFromSwap(t, "pick") + handleConflictsFromSwap(t, "pick", true) }, }) diff --git a/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict_and_edit.go b/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict_and_edit.go index 1e9ff4934..7ee710ebe 100644 --- a/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict_and_edit.go +++ b/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict_and_edit.go @@ -51,6 +51,6 @@ var SwapInRebaseWithConflictAndEdit = NewIntegrationTest(NewIntegrationTestArgs{ t.Common().ContinueRebase() }) - handleConflictsFromSwap(t, "edit") + handleConflictsFromSwap(t, "edit", true) }, }) diff --git a/pkg/integration/tests/interactive_rebase/swap_with_conflict.go b/pkg/integration/tests/interactive_rebase/swap_with_conflict.go index 1ea71356e..5f91d9f04 100644 --- a/pkg/integration/tests/interactive_rebase/swap_with_conflict.go +++ b/pkg/integration/tests/interactive_rebase/swap_with_conflict.go @@ -28,6 +28,6 @@ var SwapWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ ). Press(keys.Commits.MoveDownCommit) - handleConflictsFromSwap(t, "pick") + handleConflictsFromSwap(t, "pick", false) }, }) diff --git a/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go b/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go index a619128fa..7f0d3584f 100644 --- a/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go +++ b/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go @@ -76,10 +76,11 @@ var MoveToEarlierCommitFromAddedFile = NewIntegrationTest(NewIntegrationTestArgs t.Views().Commits(). Focus(). Lines( - Contains("commit to move from"), - Contains("destination commit").IsSelected(), + Contains("commit to move from").IsSelected(), + Contains("destination commit"), Contains("first commit"), ). + NavigateToLine(Contains("destination commit")). PressEnter() t.Views().CommitFiles(). diff --git a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go index 050ae2e2f..2e08688df 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go @@ -50,7 +50,7 @@ var PullRebaseInteractiveConflict = NewIntegrationTest(NewIntegrationTestArgs{ Lines( Contains("─── Pending rebase todos"), Contains("pick").Contains("five"), - Contains("pick").Contains("CONFLICT").Contains("four"), + Contains("pick").Contains("CONFLICT").Contains("four").IsSelected(), Contains("─── Commits"), Contains("three"), Contains("two"), @@ -83,13 +83,12 @@ var PullRebaseInteractiveConflict = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Focus(). Lines( - Contains("five").IsSelected(), - Contains("four"), + Contains("five"), + Contains("four").IsSelected(), Contains("three"), Contains("two"), Contains("one"), - ). - SelectNextItem() + ) t.Views().Main(). Content( diff --git a/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go b/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go index 707564a8d..38b63608e 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go @@ -50,13 +50,14 @@ var PullRebaseInteractiveConflictDrop = NewIntegrationTest(NewIntegrationTestArg Focus(). Lines( Contains("─── Pending rebase todos"), - Contains("pick").Contains("five").IsSelected(), - Contains("pick").Contains("CONFLICT").Contains("four"), + Contains("pick").Contains("five"), + Contains("pick").Contains("CONFLICT").Contains("four").IsSelected(), Contains("─── Commits"), Contains("three"), Contains("two"), Contains("one"), ). + NavigateToLine(Contains("five")). Press(keys.Universal.Remove). Lines( Contains("─── Pending rebase todos"),