From bee03d3b98dba8bf8fdc64f65469aebd096b1ced Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 12 Aug 2026 10:47:47 +0200 Subject: [PATCH] Show renames when diffing a directory that a file was moved into or out of Git limits its tree diff by the pathspec before it looks for renames, so a directory only ever gets one end of a rename whose other end is outside it. Nothing is left to pair up, and the file turns into an addition or a deletion that the commit doesn't contain. Pass the other end along with the directory. This is bounded by the number of renames that cross the directory's boundary, so it costs nothing at all for the vast majority of commits. --- .../controllers/commits_files_controller.go | 4 +- pkg/gui/controllers/diff_paths.go | 56 ++++++++++--- pkg/gui/controllers/diff_paths_test.go | 79 +++++++++++++++++++ pkg/gui/controllers/files_controller.go | 2 +- .../directory_diff_with_renamed_files.go | 22 ------ .../file/directory_diff_with_renamed_files.go | 22 ------ 6 files changed, 128 insertions(+), 57 deletions(-) create mode 100644 pkg/gui/controllers/diff_paths_test.go diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index c01658b03..690114dbb 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -175,7 +175,7 @@ func (self *CommitFilesController) GetOnRenderToMain() func() { from, to := self.context().GetFromAndToForDiff() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(from) - paths := pathsForDiff(node.Raw(), self.context().IsFiltering()) + paths := pathsForDiff(node.Raw(), self.context().GetRoot().Raw(), self.context().IsFiltering()) cmdObj := self.c.Git().WorkingTree.ShowFileDiffCmdObj(from, to, reverse, paths, false) task := types.NewRunPtyTask(cmdObj.GetCmd()) @@ -263,7 +263,7 @@ func (self *CommitFilesController) openCopyMenu() error { copyFileDiffItem := &types.MenuItem{ Label: self.c.Tr.CopySelectedDiff, OnPress: func() error { - paths := pathsForDiff(node.Raw(), self.context().IsFiltering()) + paths := pathsForDiff(node.Raw(), self.context().GetRoot().Raw(), self.context().IsFiltering()) return self.copyDiffToClipboard(paths, self.c.Tr.FileDiffCopiedToast) }, DisabledReason: self.require(self.singleItemSelected())(), diff --git a/pkg/gui/controllers/diff_paths.go b/pkg/gui/controllers/diff_paths.go index 31fc81757..4cb34f3cc 100644 --- a/pkg/gui/controllers/diff_paths.go +++ b/pkg/gui/controllers/diff_paths.go @@ -1,6 +1,8 @@ package controllers import ( + "strings" + "github.com/jesseduffield/lazygit/pkg/gui/filetree" ) @@ -8,28 +10,62 @@ import ( // path, plus the path it was renamed from if it is a rename. type fileWithNames[T any] interface { *T + GetPath() string + GetPreviousPath() string Names() []string } // pathsForDiff returns the paths to limit a diff command to for showing the -// changes of the given node. For a directory this is the directory itself, -// unless a filter is active, in which case we list the files that are visible -// under it. -func pathsForDiff[T any, PT fileWithNames[T]](node *filetree.Node[T], isFiltering bool) []string { +// changes of the given node. root is the root of the tree that the node belongs +// to, and isFiltering says whether that tree is reduced to the files matching a +// text filter. +func pathsForDiff[T any, PT fileWithNames[T]](node *filetree.Node[T], root *filetree.Node[T], isFiltering bool) []string { if file := node.GetFile(); file != nil { return PT(file).Names() } + dir := node.GetPath() + if isFiltering { + // Passing the directory would bring back the files that the filter hides, + // so we spell out the ones it leaves. var paths []string - _ = node.ForEachFile(func(file *T) error { - // For a rename we need to pass both paths so that git detects it as - // a rename rather than an unrelated delete and add. - paths = append(paths, PT(file).Names()...) - return nil + forEachFileInDir[T, PT](root, dir, func(file PT) { + paths = append(paths, file.Names()...) }) return paths } - return []string{node.GetPath()} + // The directory covers everything below it, but git only pairs up the two + // ends of a rename if both are in the pathspec, and one end can well be + // outside the directory. Without that end we would get an addition or a + // deletion where the diff has a rename. + paths := []string{dir} + forEachFileInDir[T, PT](root, dir, func(file PT) { + if path := file.GetPath(); !isInDir(path, dir) { + paths = append(paths, path) + } + if previousPath := file.GetPreviousPath(); previousPath != "" && !isInDir(previousPath, dir) { + paths = append(paths, previousPath) + } + }) + return paths +} + +// forEachFileInDir calls cb for each file in the tree that the given directory +// contains, either at its current or at its previous path. +func forEachFileInDir[T any, PT fileWithNames[T]](root *filetree.Node[T], dir string, cb func(PT)) { + _ = root.ForEachFile(func(f *T) error { + file := PT(f) + previousPath := file.GetPreviousPath() + if isInDir(file.GetPath(), dir) || (previousPath != "" && isInDir(previousPath, dir)) { + cb(file) + } + return nil + }) +} + +func isInDir(path string, dir string) bool { + // "." is the root item, which contains every file + return dir == "." || strings.HasPrefix(path, dir+"/") } diff --git a/pkg/gui/controllers/diff_paths_test.go b/pkg/gui/controllers/diff_paths_test.go new file mode 100644 index 000000000..ef095a29a --- /dev/null +++ b/pkg/gui/controllers/diff_paths_test.go @@ -0,0 +1,79 @@ +package controllers + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/gui/filetree" + "github.com/samber/lo" + "github.com/stretchr/testify/assert" +) + +func TestPathsForDiff(t *testing.T) { + files := []*models.CommitFile{ + {Path: "dir/file1", PreviousPath: "file1", ChangeStatus: "R"}, + {Path: "dir/file2-renamed", PreviousPath: "dir/file2", ChangeStatus: "R"}, + {Path: "dir/sub/file3", ChangeStatus: "M"}, + {Path: "file4", PreviousPath: "dir/sub/file4", ChangeStatus: "R"}, + {Path: "file5", ChangeStatus: "M"}, + } + + scenarios := []struct { + testName string + selectedPath string + isFiltering bool + expectedPaths []string + }{ + { + testName: "file", + selectedPath: "dir/sub/file3", + expectedPaths: []string{"dir/sub/file3"}, + }, + { + testName: "renamed file", + selectedPath: "dir/file1", + expectedPaths: []string{"dir/file1", "file1"}, + }, + { + testName: "directory: pass the other end of each rename that crosses its boundary", + selectedPath: "dir", + // dir/file2-renamed was renamed within the directory, so both of its + // paths are covered by it already + expectedPaths: []string{"dir", "file1", "file4"}, + }, + { + testName: "directory without renames crossing its boundary", + selectedPath: "dir/sub", + expectedPaths: []string{"dir/sub", "file4"}, + }, + { + testName: "root", + selectedPath: ".", + expectedPaths: []string{"."}, + }, + { + testName: "directory while filtering", + selectedPath: "dir", + isFiltering: true, + expectedPaths: []string{ + "dir/file1", "file1", + "dir/file2-renamed", "dir/file2", + "dir/sub/file3", + "file4", "dir/sub/file4", + }, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + cmp := filetree.NodeSortComparator[models.CommitFile]("mixed", true) + root := filetree.BuildTreeFromCommitFiles(files, true, cmp) + node, found := lo.Find(root.Flatten(filetree.NewCollapsedPaths()), func(node *filetree.Node[models.CommitFile]) bool { + return node.GetPath() == s.selectedPath + }) + assert.True(t, found, "no node for path %s", s.selectedPath) + + assert.Equal(t, s.expectedPaths, pathsForDiff(node, root, s.isFiltering)) + }) + } +} diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index 9e7f3bf1c..0eceeef32 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -369,7 +369,7 @@ func (self *FilesController) renderWorkingTreeDiff(node *filetree.FileNode) { split := self.c.UserConfig().Gui.SplitDiff == "always" || (node.GetHasUnstagedChanges() && node.GetHasStagedChanges()) mainShowsStaged := !split && node.GetHasStagedChanges() - paths := pathsForDiff(node.Raw(), self.context().IsFiltering()) + paths := pathsForDiff(node.Raw(), self.context().GetRoot().Raw(), self.context().IsFiltering()) cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(node, false, mainShowsStaged, paths) title := self.c.Tr.UnstagedChanges if mainShowsStaged { diff --git a/pkg/integration/tests/commit/directory_diff_with_renamed_files.go b/pkg/integration/tests/commit/directory_diff_with_renamed_files.go index ef3c71e5e..f849056f7 100644 --- a/pkg/integration/tests/commit/directory_diff_with_renamed_files.go +++ b/pkg/integration/tests/commit/directory_diff_with_renamed_files.go @@ -61,7 +61,6 @@ var DirectoryDiffWithRenamedFiles = NewIntegrationTest(NewIntegrationTestArgs{ SelectedLine(Equals(" ▼ dir")) t.Views().Main(). - /* EXPECTED: ContainsLines( Equals("diff --git a/file1 b/dir/file1"), Equals("similarity index 100%"), @@ -76,27 +75,6 @@ var DirectoryDiffWithRenamedFiles = NewIntegrationTest(NewIntegrationTestArgs{ Equals("rename from dir/nested/file3"), Equals("rename to file3"), ) - ACTUAL: */ - ContainsLines( - Equals("diff --git a/dir/file1 b/dir/file1"), - Equals("new file mode 100644"), - Contains("index"), - Equals("--- /dev/null"), - Equals("+++ b/dir/file1"), - Equals("@@ -0,0 +1 @@"), - Equals("+file1 content"), - Equals("diff --git a/dir/file2 b/dir/file2-renamed"), - Equals("similarity index 100%"), - Equals("rename from dir/file2"), - Equals("rename to dir/file2-renamed"), - Equals("diff --git a/dir/nested/file3 b/dir/nested/file3"), - Equals("deleted file mode 100644"), - Contains("index"), - Equals("--- a/dir/nested/file3"), - Equals("+++ /dev/null"), - Equals("@@ -1 +0,0 @@"), - Equals("-file3 content"), - ) t.Views().CommitFiles(). SelectNextItem(). diff --git a/pkg/integration/tests/file/directory_diff_with_renamed_files.go b/pkg/integration/tests/file/directory_diff_with_renamed_files.go index 101f56950..18906bf03 100644 --- a/pkg/integration/tests/file/directory_diff_with_renamed_files.go +++ b/pkg/integration/tests/file/directory_diff_with_renamed_files.go @@ -52,7 +52,6 @@ var DirectoryDiffWithRenamedFiles = NewIntegrationTest(NewIntegrationTestArgs{ SelectedLine(Equals(" ▼ dir")) t.Views().Main(). - /* EXPECTED: ContainsLines( Equals("diff --git a/file1 b/dir/file1"), Equals("similarity index 100%"), @@ -67,27 +66,6 @@ var DirectoryDiffWithRenamedFiles = NewIntegrationTest(NewIntegrationTestArgs{ Equals("rename from dir/nested/file3"), Equals("rename to file3"), ) - ACTUAL: */ - ContainsLines( - Equals("diff --git a/dir/file1 b/dir/file1"), - Equals("new file mode 100644"), - Contains("index"), - Equals("--- /dev/null"), - Equals("+++ b/dir/file1"), - Equals("@@ -0,0 +1 @@"), - Equals("+file1 content"), - Equals("diff --git a/dir/file2 b/dir/file2-renamed"), - Equals("similarity index 100%"), - Equals("rename from dir/file2"), - Equals("rename to dir/file2-renamed"), - Equals("diff --git a/dir/nested/file3 b/dir/nested/file3"), - Equals("deleted file mode 100644"), - Contains("index"), - Equals("--- a/dir/nested/file3"), - Equals("+++ /dev/null"), - Equals("@@ -1 +0,0 @@"), - Equals("-file3 content"), - ) // The same applies when a filter reduces the directory to a single file t.Views().Files().