From bb4d03db1fe560fa8d69ace690b3e6772b5aa342 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 11 Jun 2024 15:22:35 +0200 Subject: [PATCH] Show todos (and conflicting commit) for cherry-pick and revert --- pkg/commands/git_commands/commit_loader.go | 132 +++++++++++++++--- .../revert_with_conflict_multiple_commits.go | 87 ++++++++++++ .../revert_with_conflict_single_commit.go | 6 - ..._multiple_commits_in_interactive_rebase.go | 125 +++++++++++++++++ ...ert_single_commit_in_interactive_rebase.go | 99 +++++++++++++ pkg/integration/tests/test_list.go | 3 + 6 files changed, 426 insertions(+), 26 deletions(-) create mode 100644 pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go create mode 100644 pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go create mode 100644 pkg/integration/tests/interactive_rebase/revert_single_commit_in_interactive_rebase.go diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index bffe4a9fb..bf31bc287 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -169,19 +169,26 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod } } - if !self.getWorkingTreeState().Rebasing { - // not in rebase mode so return original commits - return result, nil + workingTreeState := self.getWorkingTreeState() + addConflictedRebasingCommit := true + if workingTreeState.CherryPicking || workingTreeState.Reverting { + sequencerCommits, err := self.getHydratedSequencerCommits(workingTreeState) + if err != nil { + return nil, err + } + result = append(sequencerCommits, result...) + addConflictedRebasingCommit = false } - rebasingCommits, err := self.getHydratedRebasingCommits() - if err != nil { - return nil, err + if workingTreeState.Rebasing { + rebasingCommits, err := self.getHydratedRebasingCommits(addConflictedRebasingCommit) + if err != nil { + return nil, err + } + if len(rebasingCommits) > 0 { + result = append(rebasingCommits, result...) + } } - if len(rebasingCommits) > 0 { - result = append(rebasingCommits, result...) - } - return result, nil } @@ -240,14 +247,36 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool } } -func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error) { - commits := self.getRebasingCommits() +func (self *CommitLoader) getHydratedRebasingCommits(addConflictingCommit bool) ([]*models.Commit, error) { + todoFileHasShortHashes := self.version.IsOlderThan(2, 25, 2) + return self.getHydratedTodoCommits(self.getRebasingCommits(addConflictingCommit), todoFileHasShortHashes) +} - if len(commits) == 0 { +func (self *CommitLoader) getHydratedSequencerCommits(workingTreeState models.WorkingTreeState) ([]*models.Commit, error) { + commits := self.getSequencerCommits() + if len(commits) > 0 { + // If we have any commits in .git/sequencer/todo, then the last one of + // those is the conflicting one. + commits[len(commits)-1].Status = models.StatusConflicted + } else { + // For single-commit cherry-picks and reverts, git apparently doesn't + // use the sequencer; in that case, CHERRY_PICK_HEAD or REVERT_HEAD is + // our conflicting commit, so synthesize it here. + conflicedCommit := self.getConflictedSequencerCommit(workingTreeState) + if conflicedCommit != nil { + commits = append(commits, conflicedCommit) + } + } + + return self.getHydratedTodoCommits(commits, true) +} + +func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) { + if len(todoCommits) == 0 { return nil, nil } - commitHashes := lo.FilterMap(commits, func(commit *models.Commit, _ int) (string, bool) { + commitHashes := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) { return commit.Hash, commit.Hash != "" }) @@ -271,7 +300,7 @@ func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error) return nil, err } - findFullCommit := lo.Ternary(self.version.IsOlderThan(2, 25, 2), + findFullCommit := lo.Ternary(todoFileHasShortHashes, func(hash string) *models.Commit { for s, c := range fullCommits { if strings.HasPrefix(s, hash) { @@ -284,8 +313,8 @@ func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error) return fullCommits[hash] }) - hydratedCommits := make([]*models.Commit, 0, len(commits)) - for _, rebasingCommit := range commits { + hydratedCommits := make([]*models.Commit, 0, len(todoCommits)) + for _, rebasingCommit := range todoCommits { if rebasingCommit.Hash == "" { hydratedCommits = append(hydratedCommits, rebasingCommit) } else if commit := findFullCommit(rebasingCommit.Hash); commit != nil { @@ -302,7 +331,7 @@ func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error) // git-rebase-todo example: // pick ac446ae94ee560bdb8d1d057278657b251aaef17 ac446ae // pick afb893148791a2fbd8091aeb81deba4930c73031 afb8931 -func (self *CommitLoader) getRebasingCommits() []*models.Commit { +func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading git-rebase-todo: %s", err.Error())) @@ -320,8 +349,10 @@ func (self *CommitLoader) getRebasingCommits() []*models.Commit { // See if the current commit couldn't be applied because it conflicted; if // so, add a fake entry for it - if conflictedCommit := self.getConflictedCommit(todos); conflictedCommit != nil { - commits = append(commits, conflictedCommit) + if addConflictingCommit { + if conflictedCommit := self.getConflictedCommit(todos); conflictedCommit != nil { + commits = append(commits, conflictedCommit) + } } for _, t := range todos { @@ -435,6 +466,67 @@ func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos [ } } +func (self *CommitLoader) getSequencerCommits() []*models.Commit { + bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer/todo")) + if err != nil { + self.Log.Error(fmt.Sprintf("error occurred reading sequencer/todo: %s", err.Error())) + // we assume an error means the file doesn't exist so we just return + return nil + } + + commits := []*models.Commit{} + + todos, err := todo.Parse(bytes.NewBuffer(bytesContent), self.config.GetCoreCommentChar()) + if err != nil { + self.Log.Error(fmt.Sprintf("error occurred while parsing sequencer/todo file: %s", err.Error())) + return nil + } + + for _, t := range todos { + if t.Commit == "" { + // Command does not have a commit associated, skip + continue + } + commits = utils.Prepend(commits, &models.Commit{ + Hash: t.Commit, + Name: t.Msg, + Status: models.StatusRebasing, + Action: t.Command, + }) + } + + return commits +} + +func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.WorkingTreeState) *models.Commit { + var shaFile string + var action todo.TodoCommand + if workingTreeState.CherryPicking { + shaFile = "CHERRY_PICK_HEAD" + action = todo.Pick + } else if workingTreeState.Reverting { + shaFile = "REVERT_HEAD" + action = todo.Revert + } else { + return nil + } + bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), shaFile)) + if err != nil { + self.Log.Error(fmt.Sprintf("error occurred reading %s: %s", shaFile, err.Error())) + // we assume an error means the file doesn't exist so we just return + return nil + } + lines := strings.Split(string(bytesContent), "\n") + if len(lines) == 0 { + return nil + } + return &models.Commit{ + Hash: lines[0], + Status: models.StatusConflicted, + Action: action, + } +} + func setCommitMergedStatuses(ancestor string, commits []*models.Commit) { if ancestor == "" { return diff --git a/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go b/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go new file mode 100644 index 000000000..b4186d55a --- /dev/null +++ b/pkg/integration/tests/commit/revert_with_conflict_multiple_commits.go @@ -0,0 +1,87 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RevertWithConflictMultipleCommits = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Reverts a range of commits, the first of which conflicts", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + // TODO: use our revert UI once we support range-select for reverts + cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ + { + Key: "X", + Context: "commits", + Command: "git -c core.editor=: revert HEAD^ HEAD^^", + }, + } + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("myfile", "") + shell.Commit("add empty file") + shell.CreateFileAndAdd("otherfile", "") + shell.Commit("unrelated change") + shell.CreateFileAndAdd("myfile", "first line\n") + shell.Commit("add first line") + shell.UpdateFileAndAdd("myfile", "first line\nsecond line\n") + shell.Commit("add second line") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("CI ◯ add second line").IsSelected(), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change"), + Contains("CI ◯ add empty file"), + ). + Press("X"). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Error")). + // The exact error message is different on different git versions, + // but they all contain the word 'conflict' somewhere. + Content(Contains("conflict")). + Confirm() + }). + Lines( + Contains("revert").Contains("CI unrelated change"), + Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change"), + Contains("CI ◯ add empty file"), + ) + + t.Views().Options().Content(Contains("View revert options: m")) + t.Views().Information().Content(Contains("Reverting (Reset)")) + + t.Views().Files().Focus(). + Lines( + Contains("UU myfile").IsSelected(), + ). + PressEnter() + + t.Views().MergeConflicts().IsFocused(). + SelectNextItem(). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Continue")). + Content(Contains("All merge conflicts resolved. Continue the revert?")). + Confirm() + + t.Views().Commits(). + Lines( + Contains(`CI ◯ Revert "unrelated change"`), + Contains(`CI ◯ Revert "add first line"`), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change"), + Contains("CI ◯ add empty file"), + ) + }, +}) 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 949bf4b34..480952464 100644 --- a/pkg/integration/tests/commit/revert_with_conflict_single_commit.go +++ b/pkg/integration/tests/commit/revert_with_conflict_single_commit.go @@ -39,16 +39,10 @@ var RevertWithConflictSingleCommit = NewIntegrationTest(NewIntegrationTestArgs{ Confirm() }). Lines( - /* EXPECTED: - Proper display of revert commits is not implemented yet; we'll do this in the next PR Contains("revert").Contains("CI <-- CONFLICT --- add first line"), Contains("CI ◯ add second line"), Contains("CI ◯ add first line"), Contains("CI ◯ add empty file"), - ACTUAL: */ - Contains("CI ◯ <-- YOU ARE HERE --- add second line"), - Contains("CI ◯ add first line"), - Contains("CI ◯ add empty file"), ) t.Views().Options().Content(Contains("View revert options: m")) 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 new file mode 100644 index 000000000..6a6dc771a --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/revert_multiple_commits_in_interactive_rebase.go @@ -0,0 +1,125 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RevertMultipleCommitsInInteractiveRebase = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Reverts a range of commits, the first of which conflicts, in the middle of an interactive rebase", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + // TODO: use our revert UI once we support range-select for reverts + cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ + { + Key: "X", + Context: "commits", + Command: "git -c core.editor=: revert HEAD^ HEAD^^", + }, + } + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("myfile", "") + shell.Commit("add empty file") + shell.CreateFileAndAdd("otherfile", "") + shell.Commit("unrelated change 1") + shell.CreateFileAndAdd("myfile", "first line\n") + shell.Commit("add first line") + shell.UpdateFileAndAdd("myfile", "first line\nsecond line\n") + shell.Commit("add second line") + shell.EmptyCommit("unrelated change 2") + shell.EmptyCommit("unrelated change 3") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("CI ◯ unrelated change 3").IsSelected(), + Contains("CI ◯ unrelated change 2"), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change 1"), + Contains("CI ◯ add empty file"), + ). + NavigateToLine(Contains("add second line")). + Press(keys.Universal.Edit). + Press("X"). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Error")). + // The exact error message is different on different git versions, + // but they all contain the word 'conflict' somewhere. + Content(Contains("conflict")). + Confirm() + }). + Lines( + Contains("CI unrelated change 3"), + Contains("CI unrelated change 2"), + Contains("revert").Contains("CI unrelated change 1"), + Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change 1"), + Contains("CI ◯ add empty file"), + ) + + t.Views().Options().Content(Contains("View revert options: m")) + t.Views().Information().Content(Contains("Reverting (Reset)")) + + t.Views().Files().Focus(). + Lines( + Contains("UU myfile").IsSelected(), + ). + PressEnter() + + t.Views().MergeConflicts().IsFocused(). + SelectNextItem(). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Continue")). + Content(Contains("All merge conflicts resolved. Continue the revert?")). + Confirm() + + t.Views().Commits(). + Lines( + /* EXPECTED: + Contains("pick").Contains("CI unrelated change 3"), + Contains("pick").Contains("CI unrelated change 2"), + Contains(`CI ◯ <-- YOU ARE HERE --- Revert "unrelated change 1"`), + Contains(`CI ◯ Revert "add first line"`), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change 1"), + Contains("CI ◯ add empty file"), + ACTUAL: */ + Contains("pick").Contains("CI unrelated change 3"), + Contains("pick").Contains("CI unrelated change 2"), + Contains("edit CI <-- CONFLICT --- add second line"), + Contains(`CI ◯ Revert "unrelated change 1"`), + Contains(`CI ◯ Revert "add first line"`), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change 1"), + Contains("CI ◯ add empty file"), + ) + + t.Views().Options().Content(Contains("View rebase options: m")) + t.Views().Information().Content(Contains("Rebasing (Reset)")) + + t.Common().ContinueRebase() + + t.Views().Commits(). + Lines( + Contains("CI ◯ unrelated change 3"), + Contains("CI ◯ unrelated change 2"), + Contains(`CI ◯ Revert "unrelated change 1"`), + Contains(`CI ◯ Revert "add first line"`), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ unrelated change 1"), + Contains("CI ◯ add empty file"), + ) + }, +}) 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 new file mode 100644 index 000000000..f6f104dc0 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/revert_single_commit_in_interactive_rebase.go @@ -0,0 +1,99 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RevertSingleCommitInInteractiveRebase = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Reverts a commit that conflicts in the middle of an interactive rebase", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("myfile", "") + shell.Commit("add empty file") + shell.CreateFileAndAdd("myfile", "first line\n") + shell.Commit("add first line") + shell.UpdateFileAndAdd("myfile", "first line\nsecond line\n") + shell.Commit("add second line") + shell.EmptyCommit("unrelated change 1") + shell.EmptyCommit("unrelated change 2") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("CI ◯ unrelated change 2").IsSelected(), + Contains("CI ◯ unrelated change 1"), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ add empty file"), + ). + NavigateToLine(Contains("add second line")). + Press(keys.Universal.Edit). + SelectNextItem(). + Press(keys.Commits.RevertCommit). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Revert commit")). + Content(MatchesRegexp(`Are you sure you want to revert \w+?`)). + Confirm() + t.ExpectPopup().Menu(). + Title(Equals("Conflicts!")). + Select(Contains("View conflicts")). + Confirm() + }). + Lines( + Contains("CI unrelated change 2"), + Contains("CI unrelated change 1"), + Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line").IsSelected(), + Contains("CI ◯ add empty file"), + ) + + t.Views().Options().Content(Contains("View revert options: m")) + t.Views().Information().Content(Contains("Reverting (Reset)")) + + t.Views().Files().IsFocused(). + Lines( + Contains("UU myfile").IsSelected(), + ). + PressEnter() + + t.Views().MergeConflicts().IsFocused(). + SelectNextItem(). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Continue")). + Content(Contains("All merge conflicts resolved. Continue the revert?")). + Confirm() + + t.Views().Commits(). + Lines( + Contains("pick").Contains("CI unrelated change 2"), + Contains("pick").Contains("CI unrelated change 1"), + Contains(`CI ◯ <-- YOU ARE HERE --- Revert "add first line"`), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ add empty file"), + ) + + t.Views().Options().Content(Contains("View rebase options: m")) + t.Views().Information().Content(Contains("Rebasing (Reset)")) + + t.Common().ContinueRebase() + + t.Views().Commits(). + Lines( + Contains("CI ◯ unrelated change 2"), + Contains("CI ◯ unrelated change 1"), + Contains(`CI ◯ Revert "add first line"`), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ add empty file"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 2c1e2d169..0d4980d88 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -128,6 +128,7 @@ var tests = []*components.IntegrationTest{ commit.ResetAuthorRange, commit.Revert, commit.RevertMerge, + commit.RevertWithConflictMultipleCommits, commit.RevertWithConflictSingleCommit, commit.Reword, commit.Search, @@ -262,6 +263,8 @@ var tests = []*components.IntegrationTest{ interactive_rebase.QuickStartKeepSelectionRange, interactive_rebase.Rebase, interactive_rebase.RebaseWithCommitThatBecomesEmpty, + interactive_rebase.RevertMultipleCommitsInInteractiveRebase, + interactive_rebase.RevertSingleCommitInInteractiveRebase, interactive_rebase.RewordCommitWithEditorAndFail, interactive_rebase.RewordFirstCommit, interactive_rebase.RewordLastCommit,