diff options
author | Jesse Duffield <jessedduffield@gmail.com> | 2023-05-19 20:18:02 +1000 |
---|---|---|
committer | Jesse Duffield <jessedduffield@gmail.com> | 2023-05-20 20:54:39 +1000 |
commit | ee11046d354e167abd6b6b3b6f6fa7157ea67a31 (patch) | |
tree | ca3b42cbf95b58fc3dd05a2cff1434aef422eaae /pkg/commands | |
parent | 25f8b0337e1e023fd9575ecd46467810c9f49824 (diff) |
Refactor interface for ApplyPatch
Diffstat (limited to 'pkg/commands')
-rw-r--r-- | pkg/commands/git.go | 3 | ||||
-rw-r--r-- | pkg/commands/git_commands/deps_test.go | 21 | ||||
-rw-r--r-- | pkg/commands/git_commands/patch.go | 66 | ||||
-rw-r--r-- | pkg/commands/git_commands/patch_test.go | 66 | ||||
-rw-r--r-- | pkg/commands/git_commands/working_tree.go | 30 | ||||
-rw-r--r-- | pkg/commands/git_commands/working_tree_test.go | 56 | ||||
-rw-r--r-- | pkg/commands/patch/patch_builder.go | 39 |
7 files changed, 161 insertions, 120 deletions
diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 4facd4f24..95e46086f 100644 --- a/pkg/commands/git.go +++ b/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. diff --git a/pkg/commands/git_commands/deps_test.go b/pkg/commands/git_commands/deps_test.go index 6a20c72c2..5926584b3 100644 --- a/pkg/commands/git_commands/deps_test.go +++ b/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) diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index 41de5a787..2dd7c4490 100644 --- a/pkg/commands/git_commands/patch.go +++ b/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 } diff --git a/pkg/commands/git_commands/patch_test.go b/pkg/commands/git_commands/patch_test.go new file mode 100644 index 000000000..d6ff28075 --- /dev/null +++ b/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() + }) + } +} diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index edfc3de94..e7ab69d6b 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/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, diff --git a/pkg/commands/git_commands/working_tree_test.go b/pkg/commands/git_commands/working_tree_test.go index ca2881efe..6d084bfbe 100644 --- a/pkg/commands/git_commands/working_tree_test.go +++ b/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 diff --git a/pkg/commands/patch/patch_builder.go b/pkg/commands/patch/patch_builder.go index 0053ccaa5..b73891911 100644 --- a/pkg/commands/patch/patch_builder.go +++ b/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, applyPatch applyPatchFunc, loadFileDiff loadFileDiffFunc) *PatchBuilder { +func NewPatchBuilder(log *logrus.Entry, 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 = "" |