Refactor interface for ApplyPatch

Author: jesseduffield

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, workingTreeCommands.ApplyPatch,
+patchBuilder := patch.NewPatchBuilder(cmn.Log,
 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/patch.go

@@ -2,13 +2,16 @@ package git_commands
 
 import (
 "fmt"
+"path/filepath"
+"time"
 
 "github.com/fsmiamoto/git-todo-parser/todo"
 "github.com/go-errors/errors"
 "github.com/jesseduffield/lazygit/pkg/app/daemon"
 "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,14 +42,61 @@ 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 {
 return err
 }
 
 // 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,15 +3,12 @@ package git_commands
 import (
 "fmt"
 "os"
-"path/filepath"
 "strings"
-"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 {
@@ -268,33 +265,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