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().