From cddf056f4d88e23ba7c2aa2263b9a2b1608ff651 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 3 Mar 2023 15:44:11 +0100 Subject: [PATCH 1/6] Extend test to expect what commits we want to be listed when there's a conflict --- pkg/integration/tests/interactive_rebase/shared.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/integration/tests/interactive_rebase/shared.go b/pkg/integration/tests/interactive_rebase/shared.go index db45f42e3..2d3746af4 100644 --- a/pkg/integration/tests/interactive_rebase/shared.go +++ b/pkg/integration/tests/interactive_rebase/shared.go @@ -7,6 +7,13 @@ import ( func handleConflictsFromSwap(t *TestDriver) { t.Common().AcknowledgeConflicts() + t.Views().Commits(). + Lines( + Contains("pick").Contains("commit two"), + // Would be nice to see "three" here, it's the one that conflicted + Contains("<-- YOU ARE HERE --- commit one"), + ) + t.Views().Files(). IsFocused(). Lines( From 3d76c734aab1c92510b491fae25f94c1a8f5c0d5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 8 Mar 2023 17:50:51 +0100 Subject: [PATCH 2/6] Add test for amending a commit, causing a conflict --- .../amend_commit_with_conflict.go | 74 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 75 insertions(+) create mode 100644 pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go diff --git a/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go b/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go new file mode 100644 index 000000000..0483487f3 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go @@ -0,0 +1,74 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var AmendCommitWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Amends a staged file to a commit, causing a conflict there.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file", "1\n").Commit("one") + shell.UpdateFileAndAdd("file", "1\n2\n").Commit("two") + shell.UpdateFileAndAdd("file", "1\n2\n3\n").Commit("three") + shell.UpdateFileAndAdd("file", "1\n2\n4\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("three"), + Contains("two"), + Contains("one"), + ). + NavigateToLine(Contains("two")). + Press(keys.Commits.AmendToCommit). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Amend commit")). + Content(Contains("Are you sure you want to amend this commit with your staged files?")). + Confirm() + t.Common().AcknowledgeConflicts() + }). + Lines( + Contains("pick").Contains("three"), + // Would be nice to see "fixup! two" here, because that's what git is trying to apply right now + Contains("<-- YOU ARE HERE --- two"), + Contains("one"), + ) + + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU file"), + ). + PressEnter() + + t.Views().MergeConflicts(). + IsFocused(). + TopLines( + Contains("1"), + Contains("2"), + Contains("<<<<<<< HEAD"), + Contains("======="), + Contains("4"), + Contains(">>>>>>>"), + ). + SelectNextItem(). + PressPrimaryAction() // pick "4" + + t.Common().ContinueOnConflictsResolved() + + t.Common().AcknowledgeConflicts() + + t.Views().Commits(). + Lines( + // Would be nice to see "three" here, because that's what git is trying to apply right now + Contains("<-- YOU ARE HERE --- two"), + Contains("one"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 5e45d7e55..2c047b38f 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -95,6 +95,7 @@ var tests = []*components.IntegrationTest{ filter_by_path.SelectFile, filter_by_path.TypeFile, interactive_rebase.AdvancedInteractiveRebase, + interactive_rebase.AmendCommitWithConflict, interactive_rebase.AmendFirstCommit, interactive_rebase.AmendFixupCommit, interactive_rebase.AmendHeadCommitDuringRebase, From ba160cb5db424330e59df41303215fe1a64e5b7c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 8 Mar 2023 21:24:48 +0100 Subject: [PATCH 3/6] Add test for a pick that fails and gets rescheduled This test is interesting because it already behaves as desired: since git has rescheduled the "pick" command, we do _not_ want to show a "conflict" entry in this case, as we would see the same commit twice then. --- .../interactive_rebase/pick_rescheduled.go | 47 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 48 insertions(+) create mode 100644 pkg/integration/tests/interactive_rebase/pick_rescheduled.go diff --git a/pkg/integration/tests/interactive_rebase/pick_rescheduled.go b/pkg/integration/tests/interactive_rebase/pick_rescheduled.go new file mode 100644 index 000000000..bd0f385f9 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/pick_rescheduled.go @@ -0,0 +1,47 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var PickRescheduled = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Makes a pick during a rebase fail because it would overwrite an untracked file", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "1\n").Commit("one") + shell.UpdateFileAndAdd("file2", "2\n").Commit("two") + shell.UpdateFileAndAdd("file3", "3\n").Commit("three") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("three").IsSelected(), + Contains("two"), + Contains("one"), + ). + NavigateToLine(Contains("one")). + Press(keys.Universal.Edit). + Lines( + Contains("pick").Contains("three"), + Contains("pick").Contains("two"), + Contains("<-- YOU ARE HERE --- one").IsSelected(), + ). + Tap(func() { + t.Shell().CreateFile("file3", "other content\n") + t.Common().ContinueRebase() + t.ExpectPopup().Alert().Title(Equals("Error")). + Content(Contains("The following untracked working tree files would be overwritten by merge"). + Contains("Please move or remove them before you merge.")). + Confirm() + }). + Lines( + Contains("pick").Contains("three"), + Contains("<-- YOU ARE HERE --- two"), + Contains("one"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 2c047b38f..c25df5c5d 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -109,6 +109,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.FixupSecondCommit, interactive_rebase.Move, interactive_rebase.MoveInRebase, + interactive_rebase.PickRescheduled, interactive_rebase.Rebase, interactive_rebase.RewordFirstCommit, interactive_rebase.RewordLastCommit, From d66ca7751c22a888497d53830a4d94db02506def Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 12 Mar 2023 13:00:46 +0100 Subject: [PATCH 4/6] Add test for rewording a commit and failing with an error The point of this test is to verify that the <--- YOU ARE HERE --- display is correct when the last command in a rebase was "reword". --- .../reword_commit_with_editor_and_fail.go | 45 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 46 insertions(+) create mode 100644 pkg/integration/tests/interactive_rebase/reword_commit_with_editor_and_fail.go diff --git a/pkg/integration/tests/interactive_rebase/reword_commit_with_editor_and_fail.go b/pkg/integration/tests/interactive_rebase/reword_commit_with_editor_and_fail.go new file mode 100644 index 000000000..99bac839d --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/reword_commit_with_editor_and_fail.go @@ -0,0 +1,45 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RewordCommitWithEditorAndFail = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Rewords a commit with editor, and fails because an empty commit message is given", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + }, + SetupRepo: func(shell *Shell) { + shell. + CreateNCommits(3). + SetConfig("core.editor", "sh -c 'echo .git/COMMIT_EDITMSG'") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit 03").IsSelected(), + Contains("commit 02"), + Contains("commit 01"), + ). + NavigateToLine(Contains("commit 02")). + Press(keys.Commits.RenameCommitWithEditor). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Reword in editor")). + Content(Contains("Are you sure you want to reword this commit in your editor?")). + Confirm() + }). + Lines( + Contains("commit 03"), + Contains("<-- YOU ARE HERE --- commit 02").IsSelected(), + Contains("commit 01"), + ) + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("exit status 1")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index c25df5c5d..aa303a44c 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -111,6 +111,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.MoveInRebase, interactive_rebase.PickRescheduled, interactive_rebase.Rebase, + interactive_rebase.RewordCommitWithEditorAndFail, interactive_rebase.RewordFirstCommit, interactive_rebase.RewordLastCommit, interactive_rebase.RewordYouAreHereCommit, From 3928d0ebda6952975059405544183bb1e63c0e1f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 26 Feb 2023 11:51:02 +0100 Subject: [PATCH 5/6] Insert fake todo entry for a conflicting commit that is being applied When stopping in a rebase because of a conflict, it is nice to see the commit that git is trying to apply. Create a fake todo entry labelled "conflict" for this, and show the "<-- YOU ARE HERE ---" string for that one (in red) instead of for the real current head. --- pkg/commands/git_commands/commit_loader.go | 98 ++++++++++ .../git_commands/commit_loader_test.go | 177 ++++++++++++++++++ pkg/commands/models/commit.go | 2 + .../controllers/local_commits_controller.go | 6 +- pkg/gui/presentation/commits.go | 14 +- .../tests/branch/rebase_and_drop.go | 6 +- .../amend_commit_with_conflict.go | 8 +- .../edit_the_confl_commit.go | 47 +++++ .../tests/interactive_rebase/shared.go | 4 +- .../sync/pull_rebase_interactive_conflict.go | 3 +- .../pull_rebase_interactive_conflict_drop.go | 6 +- pkg/integration/tests/test_list.go | 1 + 12 files changed, 357 insertions(+), 15 deletions(-) create mode 100644 pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 777f13d5c..e5b9ab0f4 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -310,6 +310,17 @@ func (self *CommitLoader) getInteractiveRebasingCommits() ([]*models.Commit, err return nil, nil } + // See if the current commit couldn't be applied because it conflicted; if + // so, add a fake entry for it + if conflictedCommitSha := self.getConflictedCommit(todos); conflictedCommitSha != "" { + commits = append(commits, &models.Commit{ + Sha: conflictedCommitSha, + Name: "", + Status: models.StatusRebasing, + Action: models.ActionConflict, + }) + } + for _, t := range todos { if t.Command == todo.UpdateRef { t.Msg = strings.TrimPrefix(t.Ref, "refs/heads/") @@ -328,6 +339,93 @@ func (self *CommitLoader) getInteractiveRebasingCommits() ([]*models.Commit, err return commits, nil } +func (self *CommitLoader) getConflictedCommit(todos []todo.Todo) string { + bytesContent, err := self.readFile(filepath.Join(self.dotGitDir, "rebase-merge/done")) + if err != nil { + self.Log.Error(fmt.Sprintf("error occurred reading rebase-merge/done: %s", err.Error())) + return "" + } + + doneTodos, err := todo.Parse(bytes.NewBuffer(bytesContent)) + if err != nil { + self.Log.Error(fmt.Sprintf("error occurred while parsing rebase-merge/done file: %s", err.Error())) + return "" + } + + amendFileExists := false + if _, err := os.Stat(filepath.Join(self.dotGitDir, "rebase-merge/amend")); err == nil { + amendFileExists = true + } + + return self.getConflictedCommitImpl(todos, doneTodos, amendFileExists) +} + +func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos []todo.Todo, amendFileExists bool) string { + // Should never be possible, but just to be safe: + if len(doneTodos) == 0 { + self.Log.Error("no done entries in rebase-merge/done file") + return "" + } + lastTodo := doneTodos[len(doneTodos)-1] + if lastTodo.Command == todo.Break || lastTodo.Command == todo.Exec || lastTodo.Command == todo.Reword { + return "" + } + + // In certain cases, git reschedules commands that failed. One example is if + // a patch would overwrite an untracked file (another one is an "exec" that + // failed, but we don't care about that here because we dealt with exec + // already above). To detect this, compare the last command of the "done" + // file against the first command of "git-rebase-todo"; if they are the + // same, the command was rescheduled. + if len(doneTodos) > 0 && len(todos) > 0 && doneTodos[len(doneTodos)-1] == todos[0] { + // Command was rescheduled, no need to display it + return "" + } + + // Older versions of git have a bug whereby, if a command is rescheduled, + // the last successful command is appended to the "done" file again. To + // detect this, we need to compare the second-to-last done entry against the + // first todo entry, and also compare the last done entry against the + // last-but-two done entry; this latter check is needed for the following + // case: + // pick A + // exec make test + // pick B + // exec make test + // If pick B fails with conflicts, then the "done" file contains + // pick A + // exec make test + // pick B + // and git-rebase-todo contains + // exec make test + // Without the last condition we would erroneously treat this as the exec + // command being rescheduled, so we wouldn't display our fake entry for + // "pick B". + if len(doneTodos) >= 3 && len(todos) > 0 && doneTodos[len(doneTodos)-2] == todos[0] && + doneTodos[len(doneTodos)-1] == doneTodos[len(doneTodos)-3] { + // Command was rescheduled, no need to display it + return "" + } + + if lastTodo.Command == todo.Edit { + if amendFileExists { + // Special case for "edit": if the "amend" file exists, the "edit" + // command was successful, otherwise it wasn't + return "" + } + } + + // I don't think this is ever possible, but again, just to be safe: + if lastTodo.Commit == "" { + self.Log.Error("last command in rebase-merge/done file doesn't have a commit") + return "" + } + + // Any other todo that has a commit associated with it must have failed with + // a conflict, otherwise we wouldn't have stopped the rebase: + return lastTodo.Commit +} + // assuming the file starts like this: // From e93d4193e6dd45ca9cf3a5a273d7ba6cd8b8fb20 Mon Sep 17 00:00:00 2001 // From: Lazygit Tester diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index a1a2c7e41..5c0102964 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" @@ -319,3 +320,179 @@ func TestGetCommits(t *testing.T) { }) } } + +func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { + scenarios := []struct { + testName string + todos []todo.Todo + doneTodos []todo.Todo + amendFileExists bool + expectedSha string + }{ + { + testName: "no done todos", + todos: []todo.Todo{}, + doneTodos: []todo.Todo{}, + amendFileExists: false, + expectedSha: "", + }, + { + testName: "common case (conflict)", + todos: []todo.Todo{}, + doneTodos: []todo.Todo{ + { + Command: todo.Pick, + Commit: "deadbeef", + }, + { + Command: todo.Pick, + Commit: "fa1afe1", + }, + }, + amendFileExists: false, + expectedSha: "fa1afe1", + }, + { + testName: "last command was 'break'", + todos: []todo.Todo{}, + doneTodos: []todo.Todo{ + {Command: todo.Break}, + }, + amendFileExists: false, + expectedSha: "", + }, + { + testName: "last command was 'exec'", + todos: []todo.Todo{}, + doneTodos: []todo.Todo{ + { + Command: todo.Exec, + ExecCommand: "make test", + }, + }, + amendFileExists: false, + expectedSha: "", + }, + { + testName: "last command was 'reword'", + todos: []todo.Todo{}, + doneTodos: []todo.Todo{ + {Command: todo.Reword}, + }, + amendFileExists: false, + expectedSha: "", + }, + { + testName: "'pick' was rescheduled", + todos: []todo.Todo{ + { + Command: todo.Pick, + Commit: "fa1afe1", + }, + }, + doneTodos: []todo.Todo{ + { + Command: todo.Pick, + Commit: "fa1afe1", + }, + }, + amendFileExists: false, + expectedSha: "", + }, + { + testName: "'pick' was rescheduled, buggy git version", + todos: []todo.Todo{ + { + Command: todo.Pick, + Commit: "fa1afe1", + }, + }, + doneTodos: []todo.Todo{ + { + Command: todo.Pick, + Commit: "deadbeaf", + }, + { + Command: todo.Pick, + Commit: "fa1afe1", + }, + { + Command: todo.Pick, + Commit: "deadbeaf", + }, + }, + amendFileExists: false, + expectedSha: "", + }, + { + testName: "conflicting 'pick' after 'exec'", + todos: []todo.Todo{ + { + Command: todo.Exec, + ExecCommand: "make test", + }, + }, + doneTodos: []todo.Todo{ + { + Command: todo.Pick, + Commit: "deadbeaf", + }, + { + Command: todo.Exec, + ExecCommand: "make test", + }, + { + Command: todo.Pick, + Commit: "fa1afe1", + }, + }, + amendFileExists: false, + expectedSha: "fa1afe1", + }, + { + testName: "'edit' with amend file", + todos: []todo.Todo{}, + doneTodos: []todo.Todo{ + { + Command: todo.Edit, + Commit: "fa1afe1", + }, + }, + amendFileExists: true, + expectedSha: "", + }, + { + testName: "'edit' without amend file", + todos: []todo.Todo{}, + doneTodos: []todo.Todo{ + { + Command: todo.Edit, + Commit: "fa1afe1", + }, + }, + amendFileExists: false, + expectedSha: "fa1afe1", + }, + } + for _, scenario := range scenarios { + t.Run(scenario.testName, func(t *testing.T) { + common := utils.NewDummyCommon() + + builder := &CommitLoader{ + Common: common, + cmd: oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)), + getRebaseMode: func() (enums.RebaseMode, error) { return enums.REBASE_MODE_INTERACTIVE, nil }, + dotGitDir: ".git", + readFile: func(filename string) ([]byte, error) { + return []byte(""), nil + }, + walkFiles: func(root string, fn filepath.WalkFunc) error { + return nil + }, + } + + sha := builder.getConflictedCommitImpl(scenario.todos, scenario.doneTodos, scenario.amendFileExists) + assert.Equal(t, scenario.expectedSha, sha) + }) + } +} diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index e0ff875a0..73ed523d2 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -26,6 +26,8 @@ const ( // Conveniently for us, the todo package starts the enum at 1, and given // that it doesn't have a "none" value, we're setting ours to 0 ActionNone todo.TodoCommand = 0 + // "Comment" is the last one of the todo package's enum entries + ActionConflict = todo.Comment + 1 ) // Commit : A git commit diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 46c3ad6b5..0ba80c768 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -385,6 +385,10 @@ func (self *LocalCommitsController) interactiveRebase(action todo.TodoCommand) e // commit meaning you are trying to edit the todo file rather than actually // begin a rebase. It then updates the todo file with that action func (self *LocalCommitsController) handleMidRebaseCommand(action todo.TodoCommand, commit *models.Commit) (bool, error) { + if commit.Action == models.ActionConflict { + return true, self.c.ErrorMsg(self.c.Tr.ChangingThisActionIsNotAllowed) + } + if !commit.IsTODO() { if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE { // If we are in a rebase, the only action that is allowed for @@ -434,7 +438,7 @@ func (self *LocalCommitsController) moveDown(commit *models.Commit) error { } if commit.IsTODO() { - if !commits[index+1].IsTODO() { + if !commits[index+1].IsTODO() || commits[index+1].Action == models.ActionConflict { return nil } diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index a2bba30ed..4da03f02f 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -17,6 +17,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/theme" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/kyokomi/emoji/v2" + "github.com/samber/lo" "github.com/sasha-s/go-deadlock" ) @@ -103,7 +104,11 @@ func GetCommitListDisplayStrings( for i, commit := range filteredCommits { unfilteredIdx := i + startIdx bisectStatus = getBisectStatus(unfilteredIdx, commit.Sha, bisectInfo, bisectBounds) - isYouAreHereCommit := showYouAreHereLabel && unfilteredIdx == rebaseOffset + isYouAreHereCommit := false + if showYouAreHereLabel && (commit.Action == models.ActionConflict || unfilteredIdx == rebaseOffset) { + isYouAreHereCommit = true + showYouAreHereLabel = false + } lines = append(lines, displayCommit( common, commit, @@ -272,7 +277,7 @@ func displayCommit( actionString := "" if commit.Action != models.ActionNone { - todoString := commit.Action.String() + todoString := lo.Ternary(commit.Action == models.ActionConflict, "conflict", commit.Action.String()) actionString = actionColorMap(commit.Action).Sprint(todoString) + " " } @@ -295,7 +300,8 @@ func displayCommit( } if isYouAreHereCommit { - youAreHere := style.FgYellow.Sprintf("<-- %s ---", common.Tr.YouAreHere) + color := lo.Ternary(commit.Action == models.ActionConflict, style.FgRed, style.FgYellow) + youAreHere := color.Sprintf("<-- %s ---", common.Tr.YouAreHere) name = fmt.Sprintf("%s %s", youAreHere, name) } @@ -391,6 +397,8 @@ func actionColorMap(action todo.TodoCommand) style.TextStyle { return style.FgGreen case todo.Fixup: return style.FgMagenta + case models.ActionConflict: + return style.FgRed default: return style.FgYellow } diff --git a/pkg/integration/tests/branch/rebase_and_drop.go b/pkg/integration/tests/branch/rebase_and_drop.go index 9c95e40e1..4c6712f23 100644 --- a/pkg/integration/tests/branch/rebase_and_drop.go +++ b/pkg/integration/tests/branch/rebase_and_drop.go @@ -53,7 +53,8 @@ var RebaseAndDrop = NewIntegrationTest(NewIntegrationTestArgs{ TopLines( MatchesRegexp(`pick.*to keep`).IsSelected(), MatchesRegexp(`pick.*to remove`), - MatchesRegexp("YOU ARE HERE.*second-change-branch unrelated change"), + MatchesRegexp(`conflict.*YOU ARE HERE.*first change`), + MatchesRegexp("second-change-branch unrelated change"), MatchesRegexp("second change"), MatchesRegexp("original"), ). @@ -62,7 +63,8 @@ var RebaseAndDrop = NewIntegrationTest(NewIntegrationTestArgs{ TopLines( MatchesRegexp(`pick.*to keep`), MatchesRegexp(`drop.*to remove`).IsSelected(), - MatchesRegexp("YOU ARE HERE.*second-change-branch unrelated change"), + MatchesRegexp(`conflict.*YOU ARE HERE.*first change`), + MatchesRegexp("second-change-branch unrelated change"), MatchesRegexp("second change"), MatchesRegexp("original"), ) 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 0483487f3..2226b5196 100644 --- a/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go +++ b/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go @@ -35,8 +35,8 @@ var AmendCommitWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ }). Lines( Contains("pick").Contains("three"), - // Would be nice to see "fixup! two" here, because that's what git is trying to apply right now - Contains("<-- YOU ARE HERE --- two"), + Contains("conflict").Contains("<-- YOU ARE HERE --- fixup! two"), + Contains("two"), Contains("one"), ) @@ -66,8 +66,8 @@ var AmendCommitWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Lines( - // Would be nice to see "three" here, because that's what git is trying to apply right now - Contains("<-- YOU ARE HERE --- two"), + Contains("<-- YOU ARE HERE --- three"), + 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 new file mode 100644 index 000000000..96ec81c74 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go @@ -0,0 +1,47 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var EditTheConflCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Swap two commits, causing a conflict; then try to interact with the 'confl' commit, which results in an error.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("myfile", "one") + shell.Commit("commit one") + shell.UpdateFileAndAdd("myfile", "two") + shell.Commit("commit two") + shell.UpdateFileAndAdd("myfile", "three") + shell.Commit("commit three") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit three").IsSelected(), + Contains("commit two"), + Contains("commit one"), + ). + Press(keys.Commits.MoveDownCommit). + Tap(func() { + t.Common().AcknowledgeConflicts() + }). + Focus(). + Lines( + Contains("pick").Contains("commit two"), + Contains("conflict").Contains("<-- YOU ARE HERE --- commit three"), + Contains("commit one"), + ). + NavigateToLine(Contains("<-- YOU ARE HERE --- commit three")). + Press(keys.Commits.RenameCommit) + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("Changing this kind of rebase todo entry is not allowed")). + Confirm() + }, +}) diff --git a/pkg/integration/tests/interactive_rebase/shared.go b/pkg/integration/tests/interactive_rebase/shared.go index 2d3746af4..2e42c7f76 100644 --- a/pkg/integration/tests/interactive_rebase/shared.go +++ b/pkg/integration/tests/interactive_rebase/shared.go @@ -10,8 +10,8 @@ func handleConflictsFromSwap(t *TestDriver) { t.Views().Commits(). Lines( Contains("pick").Contains("commit two"), - // Would be nice to see "three" here, it's the one that conflicted - Contains("<-- YOU ARE HERE --- commit one"), + Contains("conflict").Contains("<-- YOU ARE HERE --- commit three"), + Contains("commit one"), ) t.Views().Files(). diff --git a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go index 896c5cc45..9e66f3bcd 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go @@ -47,7 +47,8 @@ var PullRebaseInteractiveConflict = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Lines( Contains("pick").Contains("five"), - Contains("YOU ARE HERE").Contains("three"), + Contains("conflict").Contains("YOU ARE HERE").Contains("four"), + Contains("three"), Contains("two"), Contains("one"), ) 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 1950571b3..9ca3a0ebe 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go @@ -48,14 +48,16 @@ var PullRebaseInteractiveConflictDrop = NewIntegrationTest(NewIntegrationTestArg Focus(). Lines( Contains("pick").Contains("five").IsSelected(), - Contains("YOU ARE HERE").Contains("three"), + Contains("conflict").Contains("YOU ARE HERE").Contains("four"), + Contains("three"), Contains("two"), Contains("one"), ). Press(keys.Universal.Remove). Lines( Contains("drop").Contains("five").IsSelected(), - Contains("YOU ARE HERE").Contains("three"), + Contains("conflict").Contains("YOU ARE HERE").Contains("four"), + Contains("three"), Contains("two"), Contains("one"), ) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index aa303a44c..02d4f8432 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -105,6 +105,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.DropTodoCommitWithUpdateRefShowBranchHeads, interactive_rebase.EditFirstCommit, interactive_rebase.EditNonTodoCommitDuringRebase, + interactive_rebase.EditTheConflCommit, interactive_rebase.FixupFirstCommit, interactive_rebase.FixupSecondCommit, interactive_rebase.Move, From 1998d0724f18283f410ca57bebe67fefa4aaebfa Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Mar 2023 18:56:07 +0100 Subject: [PATCH 6/6] Add a test for stopping at an "edit" command that conflicts This test is almost identical to swap_in_rebase_with_conflict.go, except that it sets the commit that will conflict to "edit". This test is interesting because there's special code needed to determine whether an "edit" command conflicted or not, i.e. whether to show the "confl" entry. In this case we do. We have lots of other tests already that have "edit" commands that don't conflict, so that's covered already. --- .../swap_in_rebase_with_conflict_and_edit.go | 52 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 53 insertions(+) create mode 100644 pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict_and_edit.go 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 new file mode 100644 index 000000000..494c79552 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/swap_in_rebase_with_conflict_and_edit.go @@ -0,0 +1,52 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SwapInRebaseWithConflictAndEdit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Via an edit-triggered rebase, swap two commits, causing a conflict, then edit the commit that will conflict.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("myfile", "one") + shell.Commit("commit one") + shell.UpdateFileAndAdd("myfile", "two") + shell.Commit("commit two") + shell.UpdateFileAndAdd("myfile", "three") + shell.Commit("commit three") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit three").IsSelected(), + Contains("commit two"), + Contains("commit one"), + ). + NavigateToLine(Contains("commit one")). + Press(keys.Universal.Edit). + Lines( + Contains("commit three"), + Contains("commit two"), + Contains("<-- YOU ARE HERE --- commit one").IsSelected(), + ). + NavigateToLine(Contains("commit two")). + Press(keys.Commits.MoveUpCommit). + Lines( + Contains("commit two").IsSelected(), + Contains("commit three"), + Contains("<-- YOU ARE HERE --- commit one"), + ). + NavigateToLine(Contains("commit three")). + Press(keys.Universal.Edit). + SelectPreviousItem(). + Tap(func() { + t.Common().ContinueRebase() + }) + + handleConflictsFromSwap(t) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 02d4f8432..c1549df83 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -121,6 +121,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.SquashDownSecondCommit, interactive_rebase.SquashFixupsAboveFirstCommit, interactive_rebase.SwapInRebaseWithConflict, + interactive_rebase.SwapInRebaseWithConflictAndEdit, interactive_rebase.SwapWithConflict, misc.ConfirmOnQuit, misc.InitialOpen,