Fix applying custom patches to a dirty working tree (#4674)

- **PR Description**

Applying or reverting a custom patch when one of the files contained in
the patch had unstaged modifications in the working tree would fail with
the confusing error message "does not match index", regardless of
whether those modifications conflicted with the patch or not. You would
expect this to just work if there are no conflicts, or to get the usual
conflict markers if there are. It was possible to work around this by
manually staging those files, but few people knew about this. Fix this
by staging the files (after asking for confirmation).

Also, fix a minor problem where an auto-stash for the "move patch to
index" command was forgotten to be dropped.

Author: stefanhaller

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
 			}
 		}

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)
 }

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)
+	})
+}

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",

pkg/integration/tests/patch_building/apply_with_modified_file_conflict.go

@@ -0,0 +1,83 @@
+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().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"),
+			)
+	},
+})

pkg/integration/tests/patch_building/apply_with_modified_file_no_conflict.go

@@ -0,0 +1,69 @@
+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().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"))
+	},
+})

pkg/integration/tests/patch_building/move_to_index_with_modified_file.go

@@ -0,0 +1,59 @@
+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"))
+
+		t.Views().Stash().IsEmpty()
+	},
+})

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,
@@ -310,6 +312,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,