diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 69cddfa48..a4cef9407 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -137,6 +137,9 @@ func NewGitCommandAux( patchBuilder := patch.NewPatchBuilder(cmn.Log, func(from string, to string, reverse bool, filename string, previousPath string, plain bool) (string, error) { return workingTreeCommands.ShowFileDiff(from, to, reverse, filename, previousPath, plain) + }, + func() (string, error) { + return os.MkdirTemp("", "lazygit-custom-patch-") }) patchCommands := git_commands.NewPatchCommands(gitCommon, rebaseCommands, commitCommands, statusCommands, stashCommands, patchBuilder) bisectCommands := git_commands.NewBisectCommands(gitCommon) diff --git a/pkg/commands/git_commands/diff.go b/pkg/commands/git_commands/diff.go index a379419a8..f47097465 100644 --- a/pkg/commands/git_commands/diff.go +++ b/pkg/commands/git_commands/diff.go @@ -99,6 +99,27 @@ func (self *DiffCommands) DiffCmdObj(diffArgs []string, ignoreExternalDiff bool) ) } +// CustomPatchDiffCmdObj builds the command that renders the custom patch shown in the +// secondary pane: a `git diff --no-index` of the two file trees PatchCommands materialized +// under dir (a/ = before, b/ = after; see WriteCustomPatchDiffTrees). It uses the same pager +// wiring as DiffCmdObj so the patch renders exactly like any other diff — through a stdin +// pager, an external diff tool, or (when ignoreExternalDiff is set, the focused main view's +// raw-diff fallback) git's own colour. --no-prefix is used because the a/ and b/ tree names +// already stand in for git's conventional a//b/ path prefixes, so the diff's paths come out +// as the real repo-relative paths. +func (self *DiffCommands) CustomPatchDiffCmdObj(dir string, ignoreExternalDiff bool) *oscommands.CmdObj { + return self.cmd.New( + NewGitCmd("diff"). + AddCommonDiffArgs(self.diffRendererConfigManager, self.UserConfig(), !ignoreExternalDiff). + Arg("--no-index"). + Arg("--no-prefix"). + Arg(fmt.Sprintf("--color=%s", self.diffRendererConfigManager.GetColorArg())). + Arg("a", "b"). + Dir(dir). + ToArgv(), + ) +} + // This is a basic generic diff command that can be used for any diff operation // (e.g. copying a diff to the clipboard). It will not use a custom diff renderer, // and does not use user configs such as ignore whitespace. diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index f40f7fa6f..34d59a85f 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -2,7 +2,9 @@ package git_commands import ( "fmt" + "os" "path/filepath" + "strings" "time" "github.com/go-errors/errors" @@ -20,6 +22,11 @@ type PatchCommands struct { stash *StashCommands PatchBuilder *patch.PatchBuilder + + // lastBuiltTreeGeneration is the patch builder generation the diff trees were last + // materialized for, so EnsureCustomPatchDiffTrees rebuilds them only when the patch + // actually changed (not on every render / navigation). + lastBuiltTreeGeneration int } func NewPatchCommands( @@ -40,6 +47,91 @@ func NewPatchCommands( } } +// EnsureCustomPatchDiffTrees materializes the custom patch's diff trees if they're stale — +// i.e. the patch changed since they were last built. Called before rendering the secondary +// pane, so a mere re-render (e.g. navigating commits, which doesn't change the patch) reuses +// the existing trees, while a toggle/removal/whole-file change rebuilds them. This is what +// keeps the trees current across every path that mutates the patch. +func (self *PatchCommands) EnsureCustomPatchDiffTrees() error { + if self.PatchBuilder.Generation() == self.lastBuiltTreeGeneration { + return nil + } + if err := self.WriteCustomPatchDiffTrees(); err != nil { + return err + } + self.lastBuiltTreeGeneration = self.PatchBuilder.Generation() + return nil +} + +// WriteCustomPatchDiffTrees materializes the custom patch under the patch builder's temp +// dir as two file trees — a/ holds each patched file's "from"-side content, b/ holds that +// content with the patch applied — so the patch can be re-diffed with `git diff --no-index` +// and rendered through any pager (see DiffCommands.CustomPatchDiffCmdObj). This is what lets +// a partial, in-memory custom patch be shown the same way as any other diff; the in-memory +// aggregated patch on its own could only be fed to a stdin pager, never an external diff +// tool. The dirs are named a/b so that, with `--no-prefix`, the diff's paths come out as the +// real repo-relative paths (git's conventional a//b/ prefixes). +// +// Called whenever the patch's contents change. The temp dir's lifetime is owned by the +// patch builder (created on Start, removed on Reset), so there's nothing to clean up here +// beyond wiping the trees before rebuilding them. +func (self *PatchCommands) WriteCustomPatchDiffTrees() error { + dir := self.PatchBuilder.TempDir() + if dir == "" { + return nil + } + + aDir := filepath.Join(dir, "a") + bDir := filepath.Join(dir, "b") + for _, d := range []string{aDir, bDir} { + if err := os.RemoveAll(d); err != nil { + return err + } + if err := os.MkdirAll(d, 0o700); err != nil { + return err + } + } + + for _, filename := range self.PatchBuilder.ActiveFilenames() { + // The "from"-side content; the patch applied below turns b/ into the "after" side + // while a/ stays the "before". + content, err := self.commit.ShowFileContentCmdObj(self.PatchBuilder.From, filename).RunWithOutput() + added := err != nil // absent on the "from" side — a file the patch adds + + // a/ always holds the "before" content — empty for an added file, so the rendered + // diff still pairs it with b/ and shows the real a//b/ paths (rather than git's + // directory-comparison "added in b" form, which mangles the header). + beforeContent := content + if added { + beforeContent = "" + } + if err := self.os.CreateFileWithContent(filepath.Join(aDir, filename), beforeContent); err != nil { + return err + } + // b/ is seeded only for existing files (which the patch modifies in place); an added + // file is left absent so the patch — rendered as a /dev/null creation — creates it. + if !added { + if err := self.os.CreateFileWithContent(filepath.Join(bDir, filename), content); err != nil { + return err + } + } + } + + // Render added files as /dev/null creations (the natural form), not as diffs against an + // empty file: the latter (--- a/file) would make the atomic `git apply` expect them to + // already exist in b/, where they don't. + patchText := self.PatchBuilder.PatchToApply(false, false) + if strings.TrimSpace(patchText) == "" { + // An empty patch leaves a/ and b/ identical (or empty), so the diff is empty. + return nil + } + patchFilePath, err := self.SaveTemporaryPatch(patchText) + if err != nil { + return err + } + return self.cmd.New(NewGitCmd("apply").Arg(patchFilePath).Dir(bDir).ToArgv()).Run() +} + type ApplyPatchOpts struct { ThreeWay bool Cached bool diff --git a/pkg/commands/patch/patch_builder.go b/pkg/commands/patch/patch_builder.go index 9aaa3c268..71fb2896d 100644 --- a/pkg/commands/patch/patch_builder.go +++ b/pkg/commands/patch/patch_builder.go @@ -1,6 +1,7 @@ package patch import ( + "os" "sort" "strings" @@ -60,12 +61,26 @@ type PatchBuilder struct { // loadFileDiff loads the diff of a file, for a given to (typically a commit hash) loadFileDiff loadFileDiffFunc + + // newTempDir creates a fresh temp dir for the current patch, into which the patch is + // materialized as two file trees so it can be rendered through any pager (see + // PatchCommands.WriteCustomPatchDiffTrees). The patch builder owns its lifetime: a dir + // is created on Start and removed on Reset. Nil in tests that don't render the patch. + newTempDir func() (string, error) + tempDir string + + // generation is bumped on every change to the patch's contents, so a consumer that + // materializes the patch (the secondary pane's two file trees) can tell when it's stale + // and rebuild only then — covering every path that mutates the patch (the focused main + // view and the old explorer alike) without rebuilding on mere navigation. + generation int } -func NewPatchBuilder(log *logrus.Entry, loadFileDiff loadFileDiffFunc) *PatchBuilder { +func NewPatchBuilder(log *logrus.Entry, loadFileDiff loadFileDiffFunc, newTempDir func() (string, error)) *PatchBuilder { return &PatchBuilder{ Log: log, loadFileDiff: loadFileDiff, + newTempDir: newTempDir, } } @@ -73,6 +88,16 @@ func (p *PatchBuilder) Start(from, to string, reverse bool, canRebase bool) { p.mutex.Lock() defer p.mutex.Unlock() + p.generation++ + p.removeTempDir() + if p.newTempDir != nil { + if dir, err := p.newTempDir(); err != nil { + p.Log.Error(err) + } else { + p.tempDir = dir + } + } + p.To = to p.From = from p.reverse = reverse @@ -91,6 +116,37 @@ func (p *PatchBuilder) snapshotFileInfoMap() map[string]*fileInfo { return p.fileInfoMap } +// TempDir is the directory the current patch is materialized into for rendering, or "" if +// none was created. See PatchCommands.WriteCustomPatchDiffTrees. +func (p *PatchBuilder) TempDir() string { + return p.tempDir +} + +// Generation is bumped each time the patch's contents change; see the field comment. +func (p *PatchBuilder) Generation() int { + return p.generation +} + +func (p *PatchBuilder) removeTempDir() { + if p.tempDir != "" { + _ = os.RemoveAll(p.tempDir) + p.tempDir = "" + } +} + +// ActiveFilenames returns the files currently part of the patch (mode != UNSELECTED), in +// sorted order — the files to materialize when rendering the patch. +func (p *PatchBuilder) ActiveFilenames() []string { + filenames := make([]string, 0, len(p.fileInfoMap)) + for filename, info := range p.fileInfoMap { + if info.mode != UNSELECTED { + filenames = append(filenames, filename) + } + } + sort.Strings(filenames) + return filenames +} + func (p *PatchBuilder) PatchToApply(reverse bool, turnAddedFilesIntoDiffAgainstEmptyFile bool) string { var patch strings.Builder @@ -135,6 +191,7 @@ func (p *PatchBuilder) AddFileWhole(filename string, previousPath string) error return err } + p.generation++ p.addFileWhole(info) return nil @@ -146,6 +203,8 @@ func (p *PatchBuilder) RemoveFile(filename string, previousPath string) error { return err } + p.generation++ + p.removeFile(info) return nil @@ -182,6 +241,7 @@ func (p *PatchBuilder) AddFileLineRange(filename string, previousPath string, li if err != nil { return err } + p.generation++ info.mode = PART info.includedLineIndices = lo.Union(info.includedLineIndices, lineIndices) @@ -193,6 +253,7 @@ func (p *PatchBuilder) RemoveFileLineRange(filename string, previousPath string, if err != nil { return err } + p.generation++ info.mode = PART info.includedLineIndices, _ = lo.Difference(info.includedLineIndices, lineIndices) if len(info.includedLineIndices) == 0 { @@ -399,6 +460,9 @@ func (p *PatchBuilder) Reset() { p.mutex.Lock() defer p.mutex.Unlock() + p.generation++ + p.removeTempDir() + p.To = "" p.fileInfoMap = map[string]*fileInfo{} } diff --git a/pkg/commands/patch/patch_builder_test.go b/pkg/commands/patch/patch_builder_test.go index 46d173eb1..12950bd27 100644 --- a/pkg/commands/patch/patch_builder_test.go +++ b/pkg/commands/patch/patch_builder_test.go @@ -13,7 +13,8 @@ func newTestPatchBuilder(diff string) *PatchBuilder { pb := NewPatchBuilder(logrus.New().WithField("test", "test"), func(from, to string, reverse bool, filename string, previousPath string, plain bool) (string, error) { return diff, nil - }) + }, + nil) pb.Start("from", "to", false, true) return pb } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 1aee0b206..2d0f04d6f 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -732,16 +732,35 @@ func (self *LocalCommitsController) GetOnRenderToMain() func() { } func secondaryPatchPanelUpdateOpts(c *ControllerCommon) *types.ViewUpdateOpts { - if c.Git().Patch.PatchBuilder.Active() { - patch := c.Git().Patch.PatchBuilder.RenderAggregatedPatch(false) + patchBuilder := c.Git().Patch.PatchBuilder + if !patchBuilder.Active() { + return nil + } + // Bring the patch's diff trees up to date if the patch changed since they were last + // built (a no-op on a plain re-render, e.g. navigating commits). + if err := c.Git().Patch.EnsureCustomPatchDiffTrees(); err != nil { + c.Log.Error(err) + } + + if patchBuilder.TempDir() == "" { + // The patch wasn't materialized (its temp dir couldn't be created); fall back to + // rendering the aggregated patch directly with git's own colours. return &types.ViewUpdateOpts{ - Task: types.NewRenderStringWithoutScrollTask(patch), + Task: types.NewRenderStringWithoutScrollTask(patchBuilder.RenderAggregatedPatch(false)), Title: c.Tr.CustomPatch, } } - return nil + // Render the custom patch the same way the main view renders its diff — through the + // pager, or raw (git's own colour) under the raw-diff fallback / when no pager is + // configured — by re-diffing the two file trees the patch was materialized into. + renderRaw := c.Helpers().Staging.DiffMainViewShouldRenderRaw() + cmdObj := c.Git().Diff.CustomPatchDiffCmdObj(patchBuilder.TempDir(), renderRaw) + return &types.ViewUpdateOpts{ + Task: types.NewMainViewDiffTask(renderRaw, cmdObj.GetCmd()), + Title: c.Tr.CustomPatch, + } } func (self *LocalCommitsController) squashDown(selectedCommits []*models.Commit, startIdx int, endIdx int) error { diff --git a/pkg/gui/presentation/files_test.go b/pkg/gui/presentation/files_test.go index c7e333682..7e7b7a12d 100644 --- a/pkg/gui/presentation/files_test.go +++ b/pkg/gui/presentation/files_test.go @@ -230,6 +230,7 @@ M file1 func(from string, to string, reverse bool, filename string, previousPath string, plain bool) (string, error) { return "", nil }, + nil, ) patchBuilder.Start("from", "to", false, false) result := RenderCommitFileTree(viewModel, patchBuilder, false, &config.CustomIconsConfig{}) diff --git a/pkg/integration/tests/patch_building/specific_selection.go b/pkg/integration/tests/patch_building/specific_selection.go index 2e140e41c..7e50c5a0d 100644 --- a/pkg/integration/tests/patch_building/specific_selection.go +++ b/pkg/integration/tests/patch_building/specific_selection.go @@ -134,14 +134,12 @@ var SpecificSelection = NewIntegrationTest(NewIntegrationTestArgs{ Contains(`index`), Contains(`--- a/hunk-file`), Contains(`+++ b/hunk-file`), - Contains(`@@ -1,6 +1,6 @@`), + Contains(`@@ -1,4 +1,4 @@`), Contains(`-1a`), Contains(`+aa`), Contains(` 1b`), Contains(` 1c`), Contains(` 1d`), - Contains(` 1e`), - Contains(` 1f`), // line-file patch Contains(`diff --git a/line-file b/line-file`), Contains(`index`),