diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index 39f60478a..28e6c54ad 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -211,6 +211,11 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s return err } + if err := self.pickCopyWithPatchInParent(commits[sourceCommitIdx]); err != nil { + _ = self.rebase.AbortRebase() + return err + } + // apply each patch forward if err := self.ApplyCustomPatch(false, false); err != nil { // Don't abort the rebase here; this might cause conflicts, so give @@ -414,6 +419,11 @@ func (self *PatchCommands) PullPatchIntoNewCommitBefore( return err } + if err := self.pickCopyWithPatchInParent(commits[commitIdx]); err != nil { + _ = self.rebase.AbortRebase() + return err + } + if err := self.ApplyCustomPatch(false, false); err != nil { _ = self.rebase.AbortRebase() return err @@ -427,6 +437,110 @@ func (self *PatchCommands) PullPatchIntoNewCommitBefore( return self.rebase.ContinueRebase() } +// When moving the patch to an earlier commit, or into a new commit before the +// one it was built from, we first apply it there, and the rebase then picks +// the commit that the patch was built from. Picking that commit unchanged +// would rely on git noticing that the patch is already applied, and git +// doesn't always notice this. If the patch contains only some of several +// adjacent added lines, the pick conflicts; if git can place the patch's added +// lines in more than one position, it may add them a second time. So we make +// the rebase pick a copy of the commit instead, whose parent already contains +// the patch. +func (self *PatchCommands) pickCopyWithPatchInParent(commit *models.Commit) error { + copyHash, err := self.copyCommitWithPatchInParent(commit) + if err != nil { + return err + } + + return self.rebase.ChangeTodoCommit(commit.Hash(), copyHash) +} + +// Creates a copy of the given commit, with the same tree, author, and message, +// whose parent is the commit's original parent with the custom patch applied. +// It does this without touching the working tree or the index. +func (self *PatchCommands) copyCommitWithPatchInParent(commit *models.Commit) (string, error) { + indexDir, err := os.MkdirTemp(self.os.GetTempDir(), "index") + if err != nil { + return "", err + } + defer os.RemoveAll(indexDir) + indexFileEnvVar := "GIT_INDEX_FILE=" + filepath.Join(indexDir, "index") + + cmd := self.cmd.New(NewGitCmd("read-tree").Arg(commit.Parents()[0]).ToArgv()) + err = cmd.AddEnvVars(indexFileEnvVar).DontLog().Run() + if err != nil { + return "", err + } + + patchFilePath, err := self.SaveTemporaryPatch(self.PatchBuilder.PatchToApply(false, false)) + if err != nil { + return "", err + } + cmd = self.cmd.New(NewGitCmd("apply").Arg("--cached", patchFilePath).ToArgv()) + err = cmd.AddEnvVars(indexFileEnvVar).DontLog().Run() + if err != nil { + return "", err + } + + cmd = self.cmd.New(NewGitCmd("write-tree").ToArgv()) + tree, _, err := cmd.AddEnvVars(indexFileEnvVar).DontLog().RunWithOutputs() + if err != nil { + return "", err + } + + cmd = self.cmd.New(NewGitCmd("cat-file").Arg("commit", commit.Hash()).ToArgv()) + commitObject, _, err := cmd.DontLog().RunWithOutputs() + if err != nil { + return "", err + } + + parentHash, err := self.writeCommitObject( + commitObjectWithHeader(commitObject, "tree", strings.TrimSpace(tree))) + if err != nil { + return "", err + } + + return self.writeCommitObject(commitObjectWithHeader(commitObject, "parent", parentHash)) +} + +func (self *PatchCommands) writeCommitObject(commitObject string) (string, error) { + cmd := self.cmd.New( + NewGitCmd("hash-object"). + Arg("-t", "commit", "-w", "--stdin").ToArgv()) + hash, _, err := cmd. + SetStdin(commitObject). + DontLog(). + RunWithOutputs() + return strings.TrimSpace(hash), err +} + +// Returns the given raw commit object with the value of the given header +// replaced. The commit's signature is dropped, since it no longer matches. +func commitObjectWithHeader(commitObject string, name string, value string) string { + headers, message, _ := strings.Cut(commitObject, "\n\n") + + newHeaders := []string{} + isInSignature := false + for line := range strings.SplitSeq(headers, "\n") { + // Continuation lines of a multi-line header start with a space + if isInSignature && strings.HasPrefix(line, " ") { + continue + } + + isInSignature = strings.HasPrefix(line, "gpgsig ") || strings.HasPrefix(line, "gpgsig-sha256 ") + if isInSignature { + continue + } + + if strings.HasPrefix(line, name+" ") { + line = name + " " + value + } + newHeaders = append(newHeaders, line) + } + + return strings.Join(newHeaders, "\n") + "\n\n" + message +} + // We have just applied a patch in reverse to discard it from a commit; if we // now try to apply the patch again to move it to a later commit, or to the // index, then this would conflict "with itself" in case the patch contained diff --git a/pkg/commands/git_commands/patch_test.go b/pkg/commands/git_commands/patch_test.go new file mode 100644 index 000000000..ab7518df9 --- /dev/null +++ b/pkg/commands/git_commands/patch_test.go @@ -0,0 +1,76 @@ +package git_commands + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCommitObjectWithHeader(t *testing.T) { + scenarios := []struct { + name string + commitObject string + headerName string + value string + expected string + }{ + { + name: "replace parent", + commitObject: "tree 1111\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "\nsubject\n\nbody\n", + headerName: "parent", + value: "3333", + expected: "tree 1111\nparent 3333\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "\nsubject\n\nbody\n", + }, + { + name: "replace tree, keeping other headers", + commitObject: "tree 1111\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "encoding ISO-8859-1\n" + + "\nsubject\n", + headerName: "tree", + value: "3333", + expected: "tree 3333\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "encoding ISO-8859-1\n" + + "\nsubject\n", + }, + { + name: "drop signature", + commitObject: "tree 1111\nparent 2222\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "gpgsig -----BEGIN PGP SIGNATURE-----\n \n iQEz\n -----END PGP SIGNATURE-----\n" + + "gpgsig-sha256 -----BEGIN SSH SIGNATURE-----\n U1NI\n -----END SSH SIGNATURE-----\n" + + "some-other-header value\n" + + "\nsubject\n", + headerName: "parent", + value: "3333", + expected: "tree 1111\nparent 3333\n" + + "author A U Thor 1700000000 +0100\n" + + "committer C O Mitter 1700000001 +0100\n" + + "some-other-header value\n" + + "\nsubject\n", + }, + { + name: "empty message", + commitObject: "tree 1111\nparent 2222\n\n", + headerName: "parent", + value: "3333", + expected: "tree 1111\nparent 3333\n\n", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, commitObjectWithHeader(s.commitObject, s.headerName, s.value)) + }) + } +} diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 74278b18d..c79105c1f 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -352,6 +352,17 @@ func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo ) } +// Makes the todo for the given commit in the git-rebase-todo file use a +// different commit instead +func (self *RebaseCommands) ChangeTodoCommit(hash string, newHash string) error { + return utils.ChangeTodoCommit( + filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"), + hash, + newHash, + self.config.GetCoreCommentChar(), + ) +} + func (self *RebaseCommands) DeleteUpdateRefTodos(commits []*models.Commit) error { todosToDelete := lo.Map(commits, func(commit *models.Commit, _ int) utils.Todo { return todoFromCommit(commit) diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index dc95c00df..bb7bfa7ef 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -48,6 +48,11 @@ func (self *CustomPatchOptionsMenuAction) Call() error { } if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState().None() { + var disabledReasonForMergeCommit *types.DisabledReason + if self.isPatchFromMergeCommit() { + disabledReasonForMergeCommit = &types.DisabledReason{Text: self.c.Tr.NotSupportedForPatchFromMergeCommit} + } + menuItems = append(menuItems, []*types.MenuItem{ { Label: fmt.Sprintf(self.c.Tr.RemovePatchFromOriginalCommit, utils.ShortHash(self.c.Git().Patch.PatchBuilder.To)), @@ -68,10 +73,11 @@ func (self *CustomPatchOptionsMenuAction) Call() error { Keys: menuKey('n'), }, { - Label: self.c.Tr.MovePatchIntoNewCommitBefore, - Tooltip: self.c.Tr.MovePatchIntoNewCommitBeforeTooltip, - OnPress: self.handlePullPatchIntoNewCommitBefore, - Keys: menuKey('N'), + Label: self.c.Tr.MovePatchIntoNewCommitBefore, + Tooltip: self.c.Tr.MovePatchIntoNewCommitBeforeTooltip, + OnPress: self.handlePullPatchIntoNewCommitBefore, + Keys: menuKey('N'), + DisabledReason: disabledReasonForMergeCommit, }, }...) @@ -79,7 +85,10 @@ func (self *CustomPatchOptionsMenuAction) Call() error { selectedCommit := self.c.Contexts().LocalCommits.GetSelected() if selectedCommit != nil && self.c.Git().Patch.PatchBuilder.To != selectedCommit.Hash() { - var disabledReason *types.DisabledReason + disabledReason := disabledReasonForMergeCommit + if selectedCommit.IsMerge() { + disabledReason = &types.DisabledReason{Text: self.c.Tr.CannotMovePatchToMergeCommit} + } if self.c.Contexts().LocalCommits.AreMultipleItemsSelected() { disabledReason = &types.DisabledReason{Text: self.c.Tr.RangeSelectNotSupported} } @@ -123,6 +132,11 @@ func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() int { return -1 } +func (self *CustomPatchOptionsMenuAction) isPatchFromMergeCommit() bool { + commitIndex := self.getPatchCommitIndex() + return commitIndex != -1 && self.c.Model().Commits[commitIndex].IsMerge() +} + func (self *CustomPatchOptionsMenuAction) handleDeletePatchFromCommit() error { commits := self.c.Model().Commits commitIndex := self.getPatchCommitIndex() @@ -162,39 +176,14 @@ func (self *CustomPatchOptionsMenuAction) handleMovePatchIntoWorkingTree() error } func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommit() error { - commitIndex := self.getPatchCommitIndex() - self.c.Helpers().Commits.OpenCommitMessagePanel( - &helpers.OpenCommitMessagePanelOpts{ - // Pass a commit index of one less than the moved-from commit, so that - // you can press up arrow once to recall the original commit message: - CommitIndex: commitIndex - 1, - InitialMessage: "", - SummaryTitle: self.c.Tr.CommitSummaryTitle, - DescriptionTitle: self.c.Tr.CommitDescriptionTitle, - PreserveMessage: false, - OnConfirm: func(summary string, description string) error { - commits := self.c.Model().Commits - self.c.Helpers().Commits.CloseCommitMessagePanel() - return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { - self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) - err := self.c.Git().Patch.PullPatchIntoNewCommit(commits, commitIndex, summary, description) - if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { - return err - } - self.c.OnUIThread(func() error { - self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) - return nil - }) - return nil - }) - }, - }, - ) - - return nil + return self.pullPatchIntoNewCommit(false) } func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() error { + return self.pullPatchIntoNewCommit(true) +} + +func (self *CustomPatchOptionsMenuAction) pullPatchIntoNewCommit(beforeOriginalCommit bool) error { commitIndex := self.getPatchCommitIndex() self.c.Helpers().Commits.OpenCommitMessagePanel( &helpers.OpenCommitMessagePanelOpts{ @@ -210,10 +199,21 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() e self.c.Helpers().Commits.CloseCommitMessagePanel() return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit) - err := self.c.Git().Patch.PullPatchIntoNewCommitBefore(commits, commitIndex, summary, description) - if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil { + var result error + if beforeOriginalCommit { + result = self.c.Git().Patch.PullPatchIntoNewCommitBefore(commits, commitIndex, summary, description) + } else { + result = self.c.Git().Patch.PullPatchIntoNewCommit(commits, commitIndex, summary, description) + } + if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(result); err != nil { return err } + // If the rebase stopped at conflicts, CheckMergeOrRebase has asked the + // user how to handle them; focusing the commits view would close that + // popup. + if result != nil { + return nil + } self.c.OnUIThread(func() error { self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{}) return nil diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 0c8ed7d74..05a74b0fe 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -1013,6 +1013,8 @@ type TranslationSet struct { RangeSelectUp string RangeSelectDown string RangeSelectNotSupported string + NotSupportedForPatchFromMergeCommit string + CannotMovePatchToMergeCommit string NoItemSelected string SelectedItemIsNotABranch string SelectedItemDoesNotHaveFiles string @@ -2230,6 +2232,8 @@ func EnglishTranslationSet() *TranslationSet { RangeSelectUp: "Range select up", RangeSelectDown: "Range select down", RangeSelectNotSupported: "Action does not support range selection, please select a single item", + NotSupportedForPatchFromMergeCommit: "Not supported for a patch from a merge commit", + CannotMovePatchToMergeCommit: "Cannot move a patch to a merge commit", NoItemSelected: "No item selected", SelectedItemIsNotABranch: "Selected item is not a branch", SelectedItemDoesNotHaveFiles: "Selected item does not have files to view", diff --git a/pkg/integration/tests/main_view/disable_moving_patch_from_merge_commit.go b/pkg/integration/tests/main_view/disable_moving_patch_from_merge_commit.go new file mode 100644 index 000000000..59cf687a9 --- /dev/null +++ b/pkg/integration/tests/main_view/disable_moving_patch_from_merge_commit.go @@ -0,0 +1,60 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DisableMovingPatchFromMergeCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Moving a patch from a merge commit to another commit, or into a new commit before it, is disabled", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + NewBranch("main-branch"). + CreateFileAndAdd("file1", "file1 content\n"). + Commit("first commit"). + NewBranch("feature"). + CreateFileAndAdd("file2", "file2 content\n"). + Commit("feature commit"). + Checkout("main-branch"). + CreateFileAndAdd("file3", "file3 content\n"). + Commit("main commit"). + Merge("feature") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("Merge branch 'feature' into main-branch").IsSelected(), + Contains("feature commit"), + Contains("main commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file2").IsSelected(), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().Commits(). + Focus(). + NavigateToLine(Contains("first commit")) + + t.GlobalPress(keys.Universal.CreatePatchOptionsMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Patch options")). + Select(Contains("Move patch to selected commit")). + Tooltip(Contains("Disabled: Not supported for a patch from a merge commit")). + Select(Contains("Move patch into new commit before the original commit")). + Tooltip(Contains("Disabled: Not supported for a patch from a merge commit")). + Cancel() + }, +}) diff --git a/pkg/integration/tests/main_view/disable_moving_patch_to_merge_commit.go b/pkg/integration/tests/main_view/disable_moving_patch_to_merge_commit.go new file mode 100644 index 000000000..aebeec9ac --- /dev/null +++ b/pkg/integration/tests/main_view/disable_moving_patch_to_merge_commit.go @@ -0,0 +1,71 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DisableMovingPatchToMergeCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Moving a patch to an earlier or later merge commit is disabled", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + NewBranch("main-branch"). + CreateFileAndAdd("file1", "file1 content\n"). + Commit("first commit"). + NewBranch("feature"). + CreateFileAndAdd("file2", "file2 content\n"). + Commit("feature commit"). + Checkout("main-branch"). + CreateFileAndAdd("file3", "file3 content\n"). + Commit("main commit"). + Merge("feature"). + CreateFileAndAdd("file4", "file4 content\n"). + Commit("top commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + checkMovingPatchToMergeCommitIsDisabled := func(commitToMoveFrom string, file string) { + t.Views().Commits(). + Focus(). + NavigateToLine(Contains(commitToMoveFrom)). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains(file).IsSelected(), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().Commits(). + Focus(). + NavigateToLine(Contains("Merge branch 'feature' into main-branch")) + + t.GlobalPress(keys.Universal.CreatePatchOptionsMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Patch options")). + Select(Contains("Move patch to selected commit")). + Tooltip(Contains("Disabled: Cannot move a patch to a merge commit")). + Cancel() + + t.Common().ResetCustomPatch() + } + + t.Views().Commits(). + Lines( + Contains("top commit").IsSelected(), + Contains("Merge branch 'feature' into main-branch"), + Contains("feature commit"), + Contains("main commit"), + Contains("first commit"), + ) + + checkMovingPatchToMergeCommitIsDisabled("top commit", "file4") + checkMovingPatchToMergeCommitIsDisabled("main commit", "file3") + }, +}) diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go new file mode 100644 index 000000000..3a18daff6 --- /dev/null +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_earlier_commit.go @@ -0,0 +1,69 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveAmbiguouslyPlacedAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move some added lines from a commit to an earlier commit, where a diff could also place those lines one line further down", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "first line\n}\nlast line\n") + shell.Commit("first commit") + + shell.CreateFileAndAdd("unrelated-file", "") + shell.Commit("destination commit") + + // A diff can show the added "}" and "added line" as "added line" and "}" + // added one line further down, after the existing "}" + shell.UpdateFileAndAdd("file1", "first line\nextra line\n}\nadded line\n}\nlast line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("destination commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+extra line")). + NavigateToLine(Contains("+}")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+added line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Commits().Focus().SelectNextItem() + t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ) + + t.FileSystem().FileContent( + "file1", + Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go new file mode 100644 index 000000000..275411f1d --- /dev/null +++ b/pkg/integration/tests/main_view/move_ambiguously_placed_added_lines_to_new_commit_before.go @@ -0,0 +1,68 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveAmbiguouslyPlacedAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move some added lines from a commit to a new commit before it, where a diff could also place those lines one line further down", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "first line\n}\nlast line\n") + shell.Commit("first commit") + + // A diff can show the added "}" and "added line" as "added line" and "}" + // added one line further down, after the existing "}" + shell.UpdateFileAndAdd("file1", "first line\nextra line\n}\nadded line\n}\nlast line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+extra line")). + NavigateToLine(Contains("+}")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+added line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("new commit"), + Contains("first commit"), + ) + + t.FileSystem().FileContent( + "file1", + Equals("first line\nextra line\n}\nadded line\n}\nlast line\n"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go new file mode 100644 index 000000000..ee959326c --- /dev/null +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_earlier_commit.go @@ -0,0 +1,66 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePartOfAdjacentAddedLinesToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move only one of two adjacent added lines from a commit to an earlier commit", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "") + shell.Commit("first commit") + + shell.CreateFileAndAdd("unrelated-file", "") + shell.Commit("destination commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("destination commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+1st line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Commits().Focus().SelectNextItem() + t.Common().SelectPatchOption(Contains("Move patch to selected commit")) + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from"), + Contains("destination commit").IsSelected(), + Contains("first commit"), + ). + Tap(func() { + t.Views().Main().Content(Contains("+1st line").DoesNotContain("2nd line")) + }). + SelectPreviousItem(). + Tap(func() { + t.Views().Main().Content(Contains(" 1st line\n+2nd line")) + }) + }, +}) diff --git a/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go new file mode 100644 index 000000000..5642ae6a5 --- /dev/null +++ b/pkg/integration/tests/main_view/move_part_of_adjacent_added_lines_to_new_commit_before.go @@ -0,0 +1,65 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePartOfAdjacentAddedLinesToNewCommitBefore = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move only one of two adjacent added lines from a commit to a new commit before it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines(Contains("+1st line")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("new commit"), + Contains("first commit"), + ). + Tap(func() { + t.Views().Main().Content(Contains(" 1st line\n+2nd line")) + }). + SelectNextItem(). + Tap(func() { + t.Views().Main().Content(Contains("+1st line").DoesNotContain("2nd line")) + }) + }, +}) diff --git a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go index 1cad9e646..2c583af90 100644 --- a/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go +++ b/pkg/integration/tests/main_view/move_patch_from_added_file_to_earlier_commit.go @@ -44,40 +44,13 @@ var MovePatchFromAddedFileToEarlierCommit = NewIntegrationTest(NewIntegrationTes t.Views().Commits().Focus().SelectNextItem() t.Common().SelectPatchOption(Contains("Move patch to selected commit")) - // The source commit tries to add a file that the moved line now causes to exist. - t.Common().AcknowledgeConflicts() - - t.Views().Files(). - IsFocused(). - Lines( - Contains("AA").Contains("file"), - ). - PressEnter() - - t.Views().MergeConflicts(). - IsFocused(). - TopLines( - Contains("<<<<<<< HEAD"), - Contains("2nd line"), - Contains("======="), - Contains("1st line"), - Contains("2nd line"), - Contains("3rd line"), - Contains(">>>>>>>"), - ). - SelectNextItem(). - PressPrimaryAction() - - t.Common().ContinueOnConflictsResolved("rebase") - t.Views().Commits(). - Focus(). + IsFocused(). Lines( - Contains("commit to move from").IsSelected(), - Contains("destination commit"), + Contains("commit to move from"), + Contains("destination commit").IsSelected(), Contains("first commit"), ). - NavigateToLine(Contains("destination commit")). PressEnter() t.Views().CommitFiles(). diff --git a/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go b/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go new file mode 100644 index 000000000..5d2c8fcfd --- /dev/null +++ b/pkg/integration/tests/main_view/move_patch_to_new_commit_before_with_conflict.go @@ -0,0 +1,58 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MovePatchToNewCommitBeforeWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move a patch into a new commit before the original commit, where a commit hook changes the new commit so that replaying the original commit conflicts", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "1st line\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "1st line\n2nd line\n") + shell.CreateFileAndAdd("file2", "file2 content\n") + shell.Commit("commit to move from") + + shell.CreateFile(".git/hooks/pre-commit", "#!/bin/sh\necho 'hook line' >> file1\ngit add file1\n") + shell.MakeExecutable(".git/hooks/pre-commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("▼ /").IsSelected(), + Contains("file1"), + Contains("file2"), + ). + NavigateToLine(Contains("file2")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Common().SelectPatchOption(Contains("Move patch into new commit before the original commit")) + + t.ExpectPopup().CommitMessagePanel(). + InitialText(Equals("")). + Type("new commit").Confirm() + + t.Common().AcknowledgeConflicts() + + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU").Contains("file1"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index acbe5b445..559816011 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -407,6 +407,8 @@ var tests = []*components.IntegrationTest{ main_view.CopySelectedDiffLines, main_view.CustomPatchGoesThroughTheDiffRenderer, main_view.CustomPatchIgnoresLineEndingConversion, + main_view.DisableMovingPatchFromMergeCommit, + main_view.DisableMovingPatchToMergeCommit, main_view.DiscardAllChanges, main_view.DiscardDiffLines, main_view.DiscardFromACommitOnlyWhereItCanBeRewritten, @@ -449,9 +451,13 @@ var tests = []*components.IntegrationTest{ main_view.KeepSelectedRangeWhenChangingContextSize, main_view.KeepSelectionAfterMovingPatchOut, main_view.KeepSelectionVisibleWhenDiffShrinks, + main_view.MoveAmbiguouslyPlacedAddedLinesToEarlierCommit, + main_view.MoveAmbiguouslyPlacedAddedLinesToNewCommitBefore, main_view.MoveMultiFileRangeToIndex, main_view.MoveOnWhenThePatchMarksRewrapTheDiff, + main_view.MovePartOfAdjacentAddedLinesToEarlierCommit, main_view.MovePartOfAdjacentAddedLinesToIndex, + main_view.MovePartOfAdjacentAddedLinesToNewCommitBefore, main_view.MovePartialPatchToIndex, main_view.MovePartialPatchToLaterCommit, main_view.MovePartialPatchToNewCommit, @@ -467,6 +473,7 @@ var tests = []*components.IntegrationTest{ main_view.MovePatchToLaterCommit, main_view.MovePatchToNewCommit, main_view.MovePatchToNewCommitBefore, + main_view.MovePatchToNewCommitBeforeWithConflict, main_view.MovePatchToNewCommitInStackedBranch, main_view.NavigateByHunkAndFile, main_view.NoDiffStatLinksUnderAnExternalDiff, diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index 3a3577f80..577b3da07 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -331,3 +331,30 @@ func dropMergeCommit(todos []todo.Todo, hash string) ([]todo.Todo, error) { _, idx, _ := lo.FindIndexOf(todos, isMerge) return slices.Delete(todos, idx, idx+1), nil } + +// Read a git-rebase-todo file, make the todo for the given commit use a +// different commit instead, and write it back +func ChangeTodoCommit(fileName string, hash string, newHash string, commentChar byte) error { + todos, err := ReadRebaseTodoFile(fileName, commentChar) + if err != nil { + return err + } + + newTodos, err := changeTodoCommit(todos, hash, newHash) + if err != nil { + return err + } + + return WriteRebaseTodoFile(fileName, newTodos, commentChar) +} + +func changeTodoCommit(todos []todo.Todo, hash string, newHash string) ([]todo.Todo, error) { + idx, ok := findTodo(todos, Todo{Hash: hash}) + if !ok { + // Should never happen + return nil, fmt.Errorf("Todo %s not found in git-rebase-todo", hash) + } + + todos[idx].Commit = newHash + return todos, nil +} diff --git a/pkg/utils/rebase_todo_test.go b/pkg/utils/rebase_todo_test.go index a9ac1bba5..b032e4057 100644 --- a/pkg/utils/rebase_todo_test.go +++ b/pkg/utils/rebase_todo_test.go @@ -596,6 +596,75 @@ func TestRebaseCommands_deleteTodos(t *testing.T) { } } +func TestRebaseCommands_changeTodoCommit(t *testing.T) { + scenarios := []struct { + name string + todos []todo.Todo + hash string + newHash string + expectedTodos []todo.Todo + expectedErr error + }{ + { + name: "success", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234", Msg: "first"}, + {Command: todo.Pick, Commit: "5678", Msg: "second"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "abcd", Msg: "third"}, + }, + hash: "5678", + newHash: "ef01", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234", Msg: "first"}, + {Command: todo.Pick, Commit: "ef01", Msg: "second"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "abcd", Msg: "third"}, + }, + expectedErr: nil, + }, + { + name: "abbreviated hash in todo", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + }, + hash: "56789abc", + newHash: "ef01", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "ef01"}, + }, + expectedErr: nil, + }, + { + name: "failure", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + }, + hash: "abcd", + newHash: "ef01", + expectedTodos: nil, + expectedErr: errors.New("Todo abcd not found in git-rebase-todo"), + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + actualTodos, actualErr := changeTodoCommit(scenario.todos, scenario.hash, scenario.newHash) + + if scenario.expectedErr == nil { + assert.NoError(t, actualErr) + } else { + assert.EqualError(t, actualErr, scenario.expectedErr.Error()) + } + + assert.EqualValues(t, scenario.expectedTodos, actualTodos) + }) + } +} + func Test_equalHash(t *testing.T) { scenarios := []struct { a string