Refactor interface for ApplyPatch

pkg/commands/git.go

@@ -117,8 +117,7 @@ func NewGitCommandAux(
 	workingTreeCommands := git_commands.NewWorkingTreeCommands(gitCommon, submoduleCommands, fileLoader)
 	rebaseCommands := git_commands.NewRebaseCommands(gitCommon, commitCommands, workingTreeCommands)
 	stashCommands := git_commands.NewStashCommands(gitCommon, fileLoader, workingTreeCommands)
-	// TODO: have patch builder take workingTreeCommands in its entirety
+	patchBuilder := patch.NewPatchBuilder(cmn.Log,
-	patchBuilder := patch.NewPatchBuilder(cmn.Log, workingTreeCommands.ApplyPatch,
 		func(from string, to string, reverse bool, filename string, plain bool) (string, error) {
 			// TODO: make patch builder take Gui.IgnoreWhitespaceInDiffView into
 			// account. For now we just pass false.

pkg/commands/git_commands/deps_test.go

@@ -7,6 +7,7 @@ import (
 	gogit "github.com/jesseduffield/go-git/v5"
 	"github.com/jesseduffield/lazygit/pkg/commands/git_config"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
+	"github.com/jesseduffield/lazygit/pkg/commands/patch"
 	"github.com/jesseduffield/lazygit/pkg/common"
 	"github.com/jesseduffield/lazygit/pkg/config"
 	"github.com/jesseduffield/lazygit/pkg/utils"
@@ -117,6 +118,26 @@ func buildWorkingTreeCommands(deps commonDeps) *WorkingTreeCommands {
 	return NewWorkingTreeCommands(gitCommon, submoduleCommands, fileLoader)
 }
 
+func buildPatchCommands(deps commonDeps) *PatchCommands {
+	gitCommon := buildGitCommon(deps)
+	rebaseCommands := buildRebaseCommands(deps)
+	commitCommands := buildCommitCommands(deps)
+	statusCommands := buildStatusCommands(deps)
+	stashCommands := buildStashCommands(deps)
+	loadFileFn := func(from string, to string, reverse bool, filename string, plain bool) (string, error) {
+		return "", nil
+	}
+	patchBuilder := patch.NewPatchBuilder(gitCommon.Log, loadFileFn)
+
+	return NewPatchCommands(gitCommon, rebaseCommands, commitCommands, statusCommands, stashCommands, patchBuilder)
+}
+
+func buildStatusCommands(deps commonDeps) *StatusCommands {
+	gitCommon := buildGitCommon(deps)
+
+	return NewStatusCommands(gitCommon)
+}
+
 func buildStashCommands(deps commonDeps) *StashCommands {
 	gitCommon := buildGitCommon(deps)
 	fileLoader := buildFileLoader(gitCommon)

pkg/commands/git_commands/patch.go

@@ -2,6 +2,8 @@ package git_commands
 
 import (
 	"fmt"
+	"path/filepath"
+	"time"
 
 	"github.com/fsmiamoto/git-todo-parser/todo"
 	"github.com/go-errors/errors"
@@ -9,6 +11,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/patch"
 	"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
+	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
 type PatchCommands struct {
@@ -39,6 +42,53 @@ func NewPatchCommands(
 	}
 }
 
+type ApplyPatchOpts struct {
+	ThreeWay bool
+	Cached   bool
+	Index    bool
+	Reverse  bool
+}
+
+func (self *PatchCommands) ApplyCustomPatch(reverse bool) error {
+	patch := self.PatchBuilder.PatchToApply(reverse)
+
+	return self.ApplyPatch(patch, ApplyPatchOpts{
+		Index:    true,
+		ThreeWay: true,
+		Reverse:  reverse,
+	})
+}
+
+func (self *PatchCommands) ApplyPatch(patch string, opts ApplyPatchOpts) error {
+	filepath, err := self.SaveTemporaryPatch(patch)
+	if err != nil {
+		return err
+	}
+
+	return self.applyPatchFile(filepath, opts)
+}
+
+func (self *PatchCommands) applyPatchFile(filepath string, opts ApplyPatchOpts) error {
+	cmdStr := NewGitCmd("apply").
+		ArgIf(opts.ThreeWay, "--3way").
+		ArgIf(opts.Cached, "--cached").
+		ArgIf(opts.Index, "--index").
+		ArgIf(opts.Reverse, "--reverse").
+		Arg(self.cmd.Quote(filepath)).
+		ToString()
+
+	return self.cmd.New(cmdStr).Run()
+}
+
+func (self *PatchCommands) SaveTemporaryPatch(patch string) (string, error) {
+	filepath := filepath.Join(self.os.GetTempDir(), utils.GetCurrentRepoName(), time.Now().Format("Jan _2 15.04.05.000000000")+".patch")
+	self.Log.Infof("saving temporary patch to %s", filepath)
+	if err := self.os.CreateFileWithContent(filepath, patch); err != nil {
+		return "", err
+	}
+	return filepath, nil
+}
+
 // DeletePatchesFromCommit applies a patch in reverse for a commit
 func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, commitIndex int) error {
 	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIndex); err != nil {
@@ -46,7 +96,7 @@ func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, com
 	}
 
 	// apply each patch in reverse
-	if err := self.PatchBuilder.ApplyPatches(true); err != nil {
+	if err := self.ApplyCustomPatch(true); err != nil {
 		_ = self.rebase.AbortRebase()
 		return err
 	}
@@ -72,7 +122,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
 		}
 
 		// apply each patch forward
-		if err := self.PatchBuilder.ApplyPatches(false); err != nil {
+		if err := self.ApplyCustomPatch(false); err != nil {
 			// Don't abort the rebase here; this might cause conflicts, so give
 			// the user a chance to resolve them
 			return err
@@ -121,7 +171,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
 	}
 
 	// apply each patch in reverse
-	if err := self.PatchBuilder.ApplyPatches(true); err != nil {
+	if err := self.ApplyCustomPatch(true); err != nil {
 		_ = self.rebase.AbortRebase()
 		return err
 	}
@@ -144,7 +194,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
 	self.rebase.onSuccessfulContinue = func() error {
 		// now we should be up to the destination, so let's apply forward these patches to that.
 		// ideally we would ensure we're on the right commit but I'm not sure if that check is necessary
-		if err := self.rebase.workingTree.ApplyPatch(patch, "index", "3way"); err != nil {
+		if err := self.ApplyPatch(patch, ApplyPatchOpts{Index: true, ThreeWay: true}); err != nil {
 			// Don't abort the rebase here; this might cause conflicts, so give
 			// the user a chance to resolve them
 			return err
@@ -177,7 +227,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
 		return err
 	}
 
-	if err := self.PatchBuilder.ApplyPatches(true); err != nil {
+	if err := self.ApplyCustomPatch(true); err != nil {
 		if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING {
 			_ = self.rebase.AbortRebase()
 		}
@@ -201,7 +251,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
 
 	self.rebase.onSuccessfulContinue = func() error {
 		// add patches to index
-		if err := self.rebase.workingTree.ApplyPatch(patch, "index", "3way"); err != nil {
+		if err := self.ApplyPatch(patch, ApplyPatchOpts{Index: true, ThreeWay: true}); err != nil {
 			if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING {
 				_ = self.rebase.AbortRebase()
 			}
@@ -226,7 +276,7 @@ func (self *PatchCommands) PullPatchIntoNewCommit(commits []*models.Commit, comm
 		return err
 	}
 
-	if err := self.PatchBuilder.ApplyPatches(true); err != nil {
+	if err := self.ApplyCustomPatch(true); err != nil {
 		_ = self.rebase.AbortRebase()
 		return err
 	}
@@ -242,7 +292,7 @@ func (self *PatchCommands) PullPatchIntoNewCommit(commits []*models.Commit, comm
 		return err
 	}
 
-	if err := self.rebase.workingTree.ApplyPatch(patch, "index", "3way"); err != nil {
+	if err := self.ApplyPatch(patch, ApplyPatchOpts{Index: true, ThreeWay: true}); err != nil {
 		_ = self.rebase.AbortRebase()
 		return err
 	}

pkg/commands/git_commands/patch_test.go

@@ -0,0 +1,66 @@
+package git_commands
+
+import (
+	"fmt"
+	"os"
+	"regexp"
+	"testing"
+
+	"github.com/go-errors/errors"
+	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestWorkingTreeApplyPatch(t *testing.T) {
+	type scenario struct {
+		testName string
+		runner   *oscommands.FakeCmdObjRunner
+		test     func(error)
+	}
+
+	expectFn := func(regexStr string, errToReturn error) func(cmdObj oscommands.ICmdObj) (string, error) {
+		return func(cmdObj oscommands.ICmdObj) (string, error) {
+			re := regexp.MustCompile(regexStr)
+			cmdStr := cmdObj.ToString()
+			matches := re.FindStringSubmatch(cmdStr)
+			assert.Equal(t, 2, len(matches), fmt.Sprintf("unexpected command: %s", cmdStr))
+
+			filename := matches[1]
+
+			content, err := os.ReadFile(filename)
+			assert.NoError(t, err)
+
+			assert.Equal(t, "test", string(content))
+
+			return "", errToReturn
+		}
+	}
+
+	scenarios := []scenario{
+		{
+			testName: "valid case",
+			runner: oscommands.NewFakeRunner(t).
+				ExpectFunc(expectFn(`git apply --cached "(.*)"`, nil)),
+			test: func(err error) {
+				assert.NoError(t, err)
+			},
+		},
+		{
+			testName: "command returns error",
+			runner: oscommands.NewFakeRunner(t).
+				ExpectFunc(expectFn(`git apply --cached "(.*)"`, errors.New("error"))),
+			test: func(err error) {
+				assert.Error(t, err)
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		s := s
+		t.Run(s.testName, func(t *testing.T) {
+			instance := buildPatchCommands(commonDeps{runner: s.runner})
+			s.test(instance.ApplyPatch("test", ApplyPatchOpts{Cached: true}))
+			s.runner.CheckForMissingCalls()
+		})
+	}
+}

pkg/commands/git_commands/working_tree.go

@@ -3,14 +3,11 @@ package git_commands
 import (
 	"fmt"
 	"os"
-	"path/filepath"
-	"time"
 
 	"github.com/go-errors/errors"
 	"github.com/jesseduffield/generics/slices"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
-	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
 type WorkingTreeCommands struct {
@@ -273,33 +270,6 @@ func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain
 	return self.cmd.New(cmdStr).DontLog()
 }
 
-func (self *WorkingTreeCommands) ApplyPatch(patch string, flags ...string) error {
-	filepath, err := self.SaveTemporaryPatch(patch)
-	if err != nil {
-		return err
-	}
-
-	return self.ApplyPatchFile(filepath, flags...)
-}
-
-func (self *WorkingTreeCommands) ApplyPatchFile(filepath string, flags ...string) error {
-	flagStr := ""
-	for _, flag := range flags {
-		flagStr += " --" + flag
-	}
-
-	return self.cmd.New(fmt.Sprintf("git apply%s %s", flagStr, self.cmd.Quote(filepath))).Run()
-}
-
-func (self *WorkingTreeCommands) SaveTemporaryPatch(patch string) (string, error) {
-	filepath := filepath.Join(self.os.GetTempDir(), utils.GetCurrentRepoName(), time.Now().Format("Jan _2 15.04.05.000000000")+".patch")
-	self.Log.Infof("saving temporary patch to %s", filepath)
-	if err := self.os.CreateFileWithContent(filepath, patch); err != nil {
-		return "", err
-	}
-	return filepath, nil
-}
-
 // ShowFileDiff get the diff of specified from and to. Typically this will be used for a single commit so it'll be 123abc^..123abc
 // but when we're in diff mode it could be any 'from' to any 'to'. The reverse flag is also here thanks to diff mode.
 func (self *WorkingTreeCommands) ShowFileDiff(from string, to string, reverse bool, fileName string, plain bool,

pkg/commands/git_commands/working_tree_test.go

@@ -2,8 +2,6 @@ package git_commands
 
 import (
 	"fmt"
-	"os"
-	"regexp"
 	"testing"
 
 	"github.com/go-errors/errors"
@@ -430,60 +428,6 @@ func TestWorkingTreeCheckoutFile(t *testing.T) {
 	}
 }
 
-func TestWorkingTreeApplyPatch(t *testing.T) {
-	type scenario struct {
-		testName string
-		runner   *oscommands.FakeCmdObjRunner
-		test     func(error)
-	}
-
-	expectFn := func(regexStr string, errToReturn error) func(cmdObj oscommands.ICmdObj) (string, error) {
-		return func(cmdObj oscommands.ICmdObj) (string, error) {
-			re := regexp.MustCompile(regexStr)
-			cmdStr := cmdObj.ToString()
-			matches := re.FindStringSubmatch(cmdStr)
-			assert.Equal(t, 2, len(matches), fmt.Sprintf("unexpected command: %s", cmdStr))
-
-			filename := matches[1]
-
-			content, err := os.ReadFile(filename)
-			assert.NoError(t, err)
-
-			assert.Equal(t, "test", string(content))
-
-			return "", errToReturn
-		}
-	}
-
-	scenarios := []scenario{
-		{
-			testName: "valid case",
-			runner: oscommands.NewFakeRunner(t).
-				ExpectFunc(expectFn(`git apply --cached "(.*)"`, nil)),
-			test: func(err error) {
-				assert.NoError(t, err)
-			},
-		},
-		{
-			testName: "command returns error",
-			runner: oscommands.NewFakeRunner(t).
-				ExpectFunc(expectFn(`git apply --cached "(.*)"`, errors.New("error"))),
-			test: func(err error) {
-				assert.Error(t, err)
-			},
-		},
-	}
-
-	for _, s := range scenarios {
-		s := s
-		t.Run(s.testName, func(t *testing.T) {
-			instance := buildWorkingTreeCommands(commonDeps{runner: s.runner})
-			s.test(instance.ApplyPatch("test", "cached"))
-			s.runner.CheckForMissingCalls()
-		})
-	}
-}
-
 func TestWorkingTreeDiscardUnstagedFileChanges(t *testing.T) {
 	type scenario struct {
 		testName string

pkg/commands/patch/patch_builder.go

@@ -29,7 +29,6 @@ type fileInfo struct {
 }
 
 type (
-	applyPatchFunc   func(patch string, flags ...string) error
 	loadFileDiffFunc func(from string, to string, reverse bool, filename string, plain bool) (string, error)
 )
 
@@ -47,17 +46,14 @@ type PatchBuilder struct {
 	// fileInfoMap starts empty but you add files to it as you go along
 	fileInfoMap map[string]*fileInfo
 	Log         *logrus.Entry
-	applyPatch  applyPatchFunc
 
 	// loadFileDiff loads the diff of a file, for a given to (typically a commit SHA)
 	loadFileDiff loadFileDiffFunc
 }
 
-// NewPatchBuilder returns a new PatchBuilder
+func NewPatchBuilder(log *logrus.Entry, loadFileDiff loadFileDiffFunc) *PatchBuilder {
-func NewPatchBuilder(log *logrus.Entry, applyPatch applyPatchFunc, loadFileDiff loadFileDiffFunc) *PatchBuilder {
 	return &PatchBuilder{
 		Log:          log,
-		applyPatch:   applyPatch,
 		loadFileDiff: loadFileDiff,
 	}
 }
@@ -70,6 +66,20 @@ func (p *PatchBuilder) Start(from, to string, reverse bool, canRebase bool) {
 	p.fileInfoMap = map[string]*fileInfo{}
 }
 
+func (p *PatchBuilder) PatchToApply(reverse bool) string {
+	patch := ""
+
+	for filename, info := range p.fileInfoMap {
+		if info.mode == UNSELECTED {
+			continue
+		}
+
+		patch += p.RenderPatchForFile(filename, true, reverse)
+	}
+
+	return patch
+}
+
 func (p *PatchBuilder) addFileWhole(info *fileInfo) {
 	info.mode = WHOLE
 	lineCount := len(strings.Split(info.diff, "\n"))
@@ -234,25 +244,6 @@ func (p *PatchBuilder) GetFileIncLineIndices(filename string) ([]int, error) {
 	return info.includedLineIndices, nil
 }
 
-func (p *PatchBuilder) ApplyPatches(reverse bool) error {
-	patch := ""
-
-	applyFlags := []string{"index", "3way"}
-	if reverse {
-		applyFlags = append(applyFlags, "reverse")
-	}
-
-	for filename, info := range p.fileInfoMap {
-		if info.mode == UNSELECTED {
-			continue
-		}
-
-		patch += p.RenderPatchForFile(filename, true, reverse)
-	}
-
-	return p.applyPatch(patch, applyFlags...)
-}
-
 // clears the patch
 func (p *PatchBuilder) Reset() {
 	p.To = ""

pkg/gui/controllers/custom_patch_options_menu_action.go

@@ -199,7 +199,7 @@ func (self *CustomPatchOptionsMenuAction) handleApplyPatch(reverse bool) error {
 		action = "Apply patch in reverse"
 	}
 	self.c.LogAction(action)
-	if err := self.c.Git().Patch.PatchBuilder.ApplyPatches(reverse); err != nil {
+	if err := self.c.Git().Patch.ApplyCustomPatch(reverse); err != nil {
 		return self.c.Error(err)
 	}
 	return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})

pkg/gui/controllers/staging_controller.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	"github.com/jesseduffield/gocui"
+	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/patch"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
 )
@@ -213,15 +214,14 @@ func (self *StagingController) applySelection(reverse bool) error {
 
 	// apply the patch then refresh this panel
 	// create a new temp file with the patch, then call git apply with that patch
-	applyFlags := []string{}
-	if reverse {
-		applyFlags = append(applyFlags, "reverse")
-	}
-	if !reverse || self.staged {
-		applyFlags = append(applyFlags, "cached")
-	}
 	self.c.LogAction(self.c.Tr.Actions.ApplyPatch)
-	err := self.c.Git().WorkingTree.ApplyPatch(patchToApply, applyFlags...)
+	err := self.c.Git().Patch.ApplyPatch(
+		patchToApply,
+		git_commands.ApplyPatchOpts{
+			Reverse: reverse,
+			Cached:  !reverse || self.staged,
+		},
+	)
 	if err != nil {
 		return self.c.Error(err)
 	}
@@ -262,7 +262,7 @@ func (self *StagingController) editHunk() error {
 		}).
 		FormatPlain()
 
-	patchFilepath, err := self.c.Git().WorkingTree.SaveTemporaryPatch(patchText)
+	patchFilepath, err := self.c.Git().Patch.SaveTemporaryPatch(patchText)
 	if err != nil {
 		return err
 	}
@@ -289,11 +289,13 @@ func (self *StagingController) editHunk() error {
 		}).
 		FormatPlain()
 
-	applyFlags := []string{"cached"}
+	if err := self.c.Git().Patch.ApplyPatch(
-	if self.staged {
+		newPatchText,
-		applyFlags = append(applyFlags, "reverse")
+		git_commands.ApplyPatchOpts{
-	}
+			Reverse: self.staged,
-	if err := self.c.Git().WorkingTree.ApplyPatch(newPatchText, applyFlags...); err != nil {
+			Cached:  true,
+		},
+	); err != nil {
 		return self.c.Error(err)
 	}
 

pkg/gui/presentation/files_test.go

@@ -136,7 +136,6 @@ M file1
 			}
 			patchBuilder := patch.NewPatchBuilder(
 				utils.NewDummyLog(),
-				func(patch string, flags ...string) error { return nil },
 				func(from string, to string, reverse bool, filename string, plain bool) (string, error) {
 					return "", nil
 				},