From 00d043d743fa1180bcb8763798058a111f3218f1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 1 Jul 2025 11:23:16 +0200 Subject: [PATCH 1/5] Cleanup: implement AllFilesInPatch using lo.Keys Curiously, the function was never called so far, but we're going to use it later in this branch. --- pkg/commands/patch/patch_builder.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/commands/patch/patch_builder.go b/pkg/commands/patch/patch_builder.go index 3fc8ebcf7..1edce0741 100644 --- a/pkg/commands/patch/patch_builder.go +++ b/pkg/commands/patch/patch_builder.go @@ -286,11 +286,5 @@ func (p *PatchBuilder) NewPatchRequired(from string, to string, reverse bool) bo } func (p *PatchBuilder) AllFilesInPatch() []string { - files := make([]string, 0, len(p.fileInfoMap)) - - for filename := range p.fileInfoMap { - files = append(files, filename) - } - - return files + return lo.Keys(p.fileInfoMap) } From 823aa640b9d66964f28a7852bdd743bb4c8f8763 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 1 Jul 2025 12:57:16 +0200 Subject: [PATCH 2/5] Add test for moving patch to index when there's a modified file This is functionality that works already, we only add the test for more complete test coverage. However, there's a detail problem, and the test demonstrates this: we keep the stash even if there was no conflict. We'll fix this next. --- .../move_to_index_with_modified_file.go | 64 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 65 insertions(+) create mode 100644 pkg/integration/tests/patch_building/move_to_index_with_modified_file.go diff --git a/pkg/integration/tests/patch_building/move_to_index_with_modified_file.go b/pkg/integration/tests/patch_building/move_to_index_with_modified_file.go new file mode 100644 index 000000000..b9af98d15 --- /dev/null +++ b/pkg/integration/tests/patch_building/move_to_index_with_modified_file.go @@ -0,0 +1,64 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveToIndexWithModifiedFile = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move a patch from a commit to the index, with a modified file in the working tree that conflicts with the patch", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "1\n2\n3\n4\n") + shell.Commit("first commit") + shell.UpdateFileAndAdd("file1", "11\n2\n3\n4\n") + shell.Commit("second commit") + shell.UpdateFile("file1", "111\n2\n3\n4\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("second commit").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("M file1"), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().Secondary().Content(Contains("-1\n+11")) + + t.Common().SelectPatchOption(Contains("Move patch out into index")) + + t.ExpectPopup().Confirmation().Title(Equals("Must stash")). + Content(Contains("Pulling a patch out into the index requires stashing and unstashing your changes.")). + Confirm() + + t.Views().Files(). + Focus(). + Lines( + Equals("MM file1"), + ) + + t.Views().Main(). + Content(Contains("-11\n+111\n")) + t.Views().Secondary(). + Content(Contains("-1\n+11\n")) + + /* EXPECTED: + t.Views().Stash().IsEmpty() + ACTUAL: */ + t.Views().Stash().Lines( + Contains("Auto-stashing changes"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index e1dd36dbf..7d7bcf901 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -310,6 +310,7 @@ var tests = []*components.IntegrationTest{ patch_building.MoveToIndexPartOfAdjacentAddedLines, patch_building.MoveToIndexPartial, patch_building.MoveToIndexWithConflict, + patch_building.MoveToIndexWithModifiedFile, patch_building.MoveToIndexWorksEvenIfNoprefixIsSet, patch_building.MoveToLaterCommit, patch_building.MoveToLaterCommitPartialHunk, From 32bd9e6029e1677cc2b7dd55f92d4743a26f63c7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 1 Jul 2025 14:25:50 +0200 Subject: [PATCH 3/5] Drop stash when successfully unstashing files after moving patch to index --- pkg/commands/git_commands/patch.go | 2 +- .../tests/patch_building/move_to_index_with_modified_file.go | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index c43dc0561..495d50ad6 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -259,7 +259,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId } if stash { - if err := self.stash.Apply(0); err != nil { + if err := self.stash.Pop(0); err != nil { return err } } diff --git a/pkg/integration/tests/patch_building/move_to_index_with_modified_file.go b/pkg/integration/tests/patch_building/move_to_index_with_modified_file.go index b9af98d15..93aba6d41 100644 --- a/pkg/integration/tests/patch_building/move_to_index_with_modified_file.go +++ b/pkg/integration/tests/patch_building/move_to_index_with_modified_file.go @@ -54,11 +54,6 @@ var MoveToIndexWithModifiedFile = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Secondary(). Content(Contains("-1\n+11\n")) - /* EXPECTED: t.Views().Stash().IsEmpty() - ACTUAL: */ - t.Views().Stash().Lines( - Contains("Auto-stashing changes"), - ) }, }) From c440a208a67e20d0d5a52a58b7880ce7c59ea1ef Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 1 Jul 2025 14:52:54 +0200 Subject: [PATCH 4/5] Add tests for applying a patch when there's a modified file The tests show that this currently fails with the confusing error message "does not match index", regardless of whether the patch conflicts with the modifications or not. We'll improve this in the next commit. I don't bother adding tests for reverting a patch, as the code is basically the same as for apply. --- .../apply_with_modified_file_conflict.go | 60 +++++++++++++++++++ .../apply_with_modified_file_no_conflict.go | 60 +++++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 3 files changed, 122 insertions(+) create mode 100644 pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go create mode 100644 pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go diff --git a/pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go b/pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go new file mode 100644 index 000000000..42d4802b7 --- /dev/null +++ b/pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go @@ -0,0 +1,60 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ApplyWithModifiedFileConflict = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Apply a custom patch, with a modified file in the working tree that conflicts with the patch", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.NewBranch("branch-a") + shell.CreateFileAndAdd("file1", "1\n2\n3\n") + shell.Commit("first commit") + + shell.NewBranch("branch-b") + shell.UpdateFileAndAdd("file1", "11\n2\n3\n") + shell.Commit("update") + + shell.Checkout("branch-a") + shell.UpdateFile("file1", "111\n2\n3\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("branch-a").IsSelected(), + Contains("branch-b"), + ). + Press(keys.Universal.NextItem). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + Lines( + Contains("update").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("M file1").IsSelected(), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().Secondary().Content(Contains("-1\n+11\n")) + + t.Common().SelectPatchOption(MatchesRegexp(`Apply patch$`)) + + t.ExpectPopup().Alert().Title(Equals("Error")). + Content(Equals("error: file1: does not match index")). + Confirm() + }, +}) diff --git a/pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go b/pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go new file mode 100644 index 000000000..20d54e03e --- /dev/null +++ b/pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go @@ -0,0 +1,60 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ApplyWithModifiedFileNoConflict = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Apply a custom patch, with a modified file in the working tree that does not conflict with the patch", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.NewBranch("branch-a") + shell.CreateFileAndAdd("file1", "1\n2\n3\n") + shell.Commit("first commit") + + shell.NewBranch("branch-b") + shell.UpdateFileAndAdd("file1", "1\n2\n3\n4\n") + shell.Commit("update") + + shell.Checkout("branch-a") + shell.UpdateFile("file1", "11\n2\n3\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("branch-a").IsSelected(), + Contains("branch-b"), + ). + Press(keys.Universal.NextItem). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + Lines( + Contains("update").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("M file1").IsSelected(), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().Secondary().Content(Contains("3\n+4")) + + t.Common().SelectPatchOption(MatchesRegexp(`Apply patch$`)) + + t.ExpectPopup().Alert().Title(Equals("Error")). + Content(Equals("error: file1: does not match index")). + Confirm() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 7d7bcf901..fa5aff628 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -300,6 +300,8 @@ var tests = []*components.IntegrationTest{ patch_building.Apply, patch_building.ApplyInReverse, patch_building.ApplyInReverseWithConflict, + patch_building.ApplyWithModifiedFileConflict, + patch_building.ApplyWithModifiedFileNoConflict, patch_building.EditLineInPatchBuildingPanel, patch_building.MoveRangeToIndex, patch_building.MoveToEarlierCommit, From 3df894ec92ce21be666fd0c44a6d36d959b8c8c9 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 30 Jun 2025 17:59:36 +0200 Subject: [PATCH 5/5] Stage affected unstaged files when applying or reverting a patch Unlike moving a patch to the index, applying or reverting a patch didn't auto-stash, which means that applying a patch when there's a modified (but unstaged) file in the working tree would error out with the message "error: file1: does not match index", regardless of whether those modifications conflict with the patch or not. To fix this, we *could* add auto-stashing like we do for the "move patch to index" command. However, in this case we rather simply stage the affected files (after asking for confirmation). This has a few advantages: - it only changes the staging state of those files that are contained in the patch (whereas auto-stashing always changes all files to unstaged) - it doesn't unnecessarily show a confirmation if none of the modified files are affected by the patch - if the patch conflicts with the modified files, the conflicts were "backwards" ("ours" was the patch, "theirs" the modified file); it is more logical if "ours" is the current state of the file, and "theirs" is the patch. It's a little unfortunate that the behavior isn't exactly the same as for "move patch to index", but for that one we do need the auto-stash because of the rebase that runs behind the scenes. --- .../custom_patch_options_menu_action.go | 59 ++++++++++++++++--- pkg/i18n/english.go | 4 ++ .../apply_with_modified_file_conflict.go | 27 ++++++++- .../apply_with_modified_file_no_conflict.go | 13 +++- 4 files changed, 91 insertions(+), 12 deletions(-) diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index 2053666a7..ce6c2493f 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -4,10 +4,13 @@ import ( "errors" "fmt" + "github.com/jesseduffield/generics/set" "github.com/jesseduffield/gocui" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) type CustomPatchOptionsMenuAction struct { @@ -267,16 +270,42 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() e func (self *CustomPatchOptionsMenuAction) handleApplyPatch(reverse bool) error { self.returnFocusFromPatchExplorerIfNecessary() - action := self.c.Tr.Actions.ApplyPatch - if reverse { - action = "Apply patch in reverse" + affectedUnstagedFiles := self.getAffectedUnstagedFiles() + + apply := func() error { + action := self.c.Tr.Actions.ApplyPatch + if reverse { + action = "Apply patch in reverse" + } + self.c.LogAction(action) + + if len(affectedUnstagedFiles) > 0 { + if err := self.c.Git().WorkingTree.StageFiles(affectedUnstagedFiles, nil); err != nil { + return err + } + } + + if err := self.c.Git().Patch.ApplyCustomPatch(reverse, true); err != nil { + return err + } + + self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + return nil } - self.c.LogAction(action) - if err := self.c.Git().Patch.ApplyCustomPatch(reverse, true); err != nil { - return err + + if len(affectedUnstagedFiles) > 0 { + self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.MustStageFilesAffectedByPatchTitle, + Prompt: self.c.Tr.MustStageFilesAffectedByPatchWarning, + HandleConfirm: func() error { + return apply() + }, + }) + + return nil } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) - return nil + + return apply() } func (self *CustomPatchOptionsMenuAction) copyPatchToClipboard() error { @@ -291,3 +320,17 @@ func (self *CustomPatchOptionsMenuAction) copyPatchToClipboard() error { return nil } + +// Returns a list of files that have unstaged changes and are contained in the patch. +func (self *CustomPatchOptionsMenuAction) getAffectedUnstagedFiles() []string { + unstagedFiles := set.NewFromSlice(lo.FilterMap(self.c.Model().Files, func(f *models.File, _ int) (string, bool) { + if f.GetHasUnstagedChanges() { + return f.GetPath(), true + } + return "", false + })) + + return lo.Filter(self.c.Git().Patch.PatchBuilder.AllFilesInPatch(), func(patchFile string, _ int) bool { + return unstagedFiles.Includes(patchFile) + }) +} diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 36a119c36..ca7377a8e 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -820,6 +820,8 @@ type TranslationSet struct { MovePatchToSelectedCommit string MovePatchToSelectedCommitTooltip string CopyPatchToClipboard string + MustStageFilesAffectedByPatchTitle string + MustStageFilesAffectedByPatchWarning string NoMatchesFor string MatchesFor string SearchKeybindings string @@ -1909,6 +1911,8 @@ func EnglishTranslationSet() *TranslationSet { MovePatchToSelectedCommit: "Move patch to selected commit (%s)", MovePatchToSelectedCommitTooltip: "Move the patch out of its original commit and into the selected commit. This is achieved by starting an interactive rebase at the original commit, applying the patch in reverse, then continuing the rebase up to the selected commit, before applying the patch forward and amending the selected commit. The rebase is then continued to completion. If commits between the source and destination commit depend on the patch, you may need to resolve conflicts.", CopyPatchToClipboard: "Copy patch to clipboard", + MustStageFilesAffectedByPatchTitle: "Must stage files", + MustStageFilesAffectedByPatchWarning: "Applying a patch to the index requires staging the unstaged files that are affected by the patch. Note that you might get conflicts when applying the patch. Continue?", NoMatchesFor: "No matches for '%s' %s", ExitSearchMode: "%s: Exit search mode", ExitTextFilterMode: "%s: Exit filter mode", diff --git a/pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go b/pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go index 42d4802b7..5b529ad70 100644 --- a/pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go +++ b/pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go @@ -53,8 +53,31 @@ var ApplyWithModifiedFileConflict = NewIntegrationTest(NewIntegrationTestArgs{ t.Common().SelectPatchOption(MatchesRegexp(`Apply patch$`)) - t.ExpectPopup().Alert().Title(Equals("Error")). - Content(Equals("error: file1: does not match index")). + t.ExpectPopup().Confirmation().Title(Equals("Must stage files")). + Content(Contains("Applying a patch to the index requires staging the unstaged files that are affected by the patch.")). Confirm() + + t.ExpectPopup().Alert().Title(Equals("Error")). + Content(Contains("Applied patch to 'file1' with conflicts.")). + Confirm() + + t.Views().Files(). + Focus(). + Lines( + Equals("UU file1").IsSelected(), + ). + PressEnter() + + t.Views().MergeConflicts(). + IsFocused(). + Lines( + Equals("<<<<<<< ours"), + Equals("111"), + Equals("======="), + Equals("11"), + Equals(">>>>>>> theirs"), + Equals("2"), + Equals("3"), + ) }, }) diff --git a/pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go b/pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go index 20d54e03e..66d32a654 100644 --- a/pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go +++ b/pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go @@ -53,8 +53,17 @@ var ApplyWithModifiedFileNoConflict = NewIntegrationTest(NewIntegrationTestArgs{ t.Common().SelectPatchOption(MatchesRegexp(`Apply patch$`)) - t.ExpectPopup().Alert().Title(Equals("Error")). - Content(Equals("error: file1: does not match index")). + t.ExpectPopup().Confirmation().Title(Equals("Must stage files")). + Content(Contains("Applying a patch to the index requires staging the unstaged files that are affected by the patch.")). Confirm() + + t.Views().Files(). + Focus(). + Lines( + Equals("M file1").IsSelected(), + ) + + t.Views().Main(). + Content(Contains("-1\n+11\n 2\n 3\n+4")) }, })