Collapse the paths of a moved directory into the directory itself

A commit that moves an entire package elsewhere renames hundreds of
files, and passing every one of their old paths can push the command past
the length limit the OS imposes (~32k characters on Windows). Their
common parent directory does just as well whenever everything it holds
ends up in the diff anyway.

Deciding that needs to consider every file of the diff, not only those on
display, so the paths are now derived from the model rather than from the
tree; a status filter must not make a directory look emptier than it is.

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().GetRoot().Raw(), self.context().IsFiltering())
+		paths := self.pathsForDiff(node)
 		cmdObj := self.c.Git().WorkingTree.ShowFileDiffCmdObj(from, to, reverse, paths, false)
 		task := types.NewRunPtyTask(cmdObj.GetCmd())
 
@@ -263,8 +263,7 @@ func (self *CommitFilesController) openCopyMenu() error {
 	copyFileDiffItem := &types.MenuItem{
 		Label: self.c.Tr.CopySelectedDiff,
 		OnPress: func() error {
-			paths := pathsForDiff(node.Raw(), self.context().GetRoot().Raw(), self.context().IsFiltering())
-			return self.copyDiffToClipboard(paths, self.c.Tr.FileDiffCopiedToast)
+			return self.copyDiffToClipboard(self.pathsForDiff(node), self.c.Tr.FileDiffCopiedToast)
 		},
 		DisabledReason: self.require(self.singleItemSelected())(),
 		Keys:           menuKey('s'),
@@ -617,6 +616,11 @@ func (self *CommitFilesController) GetOnClickFocusedMainView() func(mainViewName
 	}
 }
 
+func (self *CommitFilesController) pathsForDiff(node *filetree.CommitFileNode) []string {
+	return diffPathsForNode(
+		node.Raw(), self.context().GetRoot().Raw(), self.c.Model().CommitFiles, self.context().IsFiltering())
+}
+
 // NOTE: these functions are identical to those in files_controller.go (except for types) and
 // could also be cleaned up with some generics
 func normalisedSelectedCommitFileNodes(selectedNodes []*filetree.CommitFileNode) []*filetree.CommitFileNode {

pkg/gui/controllers/diff_paths.go

@@ -1,9 +1,11 @@
 package controllers
 
 import (
+	"path"
 	"strings"
 
 	"github.com/jesseduffield/lazygit/pkg/gui/filetree"
+	"github.com/samber/lo"
 )
 
 // Both models.File and models.CommitFile satisfy this. Names returns the file's
@@ -15,11 +17,11 @@ type fileWithNames[T any] interface {
 	Names() []string
 }
 
-// pathsForDiff returns the paths to limit a diff command to for showing the
-// 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 {
+// diffPathsForNode returns the paths to limit a diff command to for showing the
+// changes of the given node. files are all the files that the diff contains,
+// while root is the root of the tree the node belongs to, which holds only the
+// files matching the text filter when there is one.
+func diffPathsForNode[T any, PT fileWithNames[T]](node *filetree.Node[T], root *filetree.Node[T], files []*T, isFiltering bool) []string {
 	if file := node.GetFile(); file != nil {
 		return PT(file).Names()
 	}
@@ -30,9 +32,9 @@ func pathsForDiff[T any, PT fileWithNames[T]](node *filetree.Node[T], root *file
 		// Passing the directory would bring back the files that the filter hides,
 		// so we spell out the ones it leaves.
 		var paths []string
-		forEachFileInDir[T, PT](root, dir, func(file PT) {
-			paths = append(paths, file.Names()...)
-		})
+		for _, file := range filesInDir[T, PT](filesInTree(root), dir) {
+			paths = append(paths, PT(file).Names()...)
+		}
 		return paths
 	}
 
@@ -40,29 +42,88 @@ func pathsForDiff[T any, PT fileWithNames[T]](node *filetree.Node[T], root *file
 	// 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)
+	var outsidePaths []string
+	for _, f := range filesInDir[T, PT](files, dir) {
+		file := PT(f)
+		if p := file.GetPath(); !isInDir(p, dir) {
+			outsidePaths = append(outsidePaths, p)
 		}
-		if previousPath := file.GetPreviousPath(); previousPath != "" && !isInDir(previousPath, dir) {
-			paths = append(paths, previousPath)
+		if p := file.GetPreviousPath(); p != "" && !isInDir(p, dir) {
+			outsidePaths = append(outsidePaths, p)
 		}
-	})
-	return paths
+	}
+
+	return dropContainedPaths(append([]string{dir}, collapseToDirs[T, PT](outsidePaths, files, dir)...))
 }
 
-// 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)
+// dropContainedPaths removes the paths that another one of them contains, since
+// a pathspec that matches a directory matches everything below it anyway.
+func dropContainedPaths(paths []string) []string {
+	return lo.Filter(paths, func(p string, _ int) bool {
+		return !lo.SomeBy(paths, func(other string) bool {
+			return other != p && isInDir(p, other)
+		})
+	})
+}
+
+// collapseToDirs replaces each of the given paths with the highest directory
+// that can stand in for it, so that moving a whole directory elsewhere costs a
+// single pathspec rather than one per file. There is a limit to how long a
+// command line may get, and a commit can move a great many files at once.
+func collapseToDirs[T any, PT fileWithNames[T]](paths []string, files []*T, dir string) []string {
+	if len(paths) == 0 {
+		return nil
+	}
+
+	// A directory can stand in for the paths under it as long as everything it
+	// contains ends up in the diff anyway, which is to say as long as all of it
+	// is in the directory we are diffing too.
+	canStandIn := make(map[string]bool)
+	standsIn := func(candidate string) bool {
+		if result, ok := canStandIn[candidate]; ok {
+			return result
 		}
+
+		result := lo.EveryBy(files, func(file *T) bool {
+			return !fileIsInDir[T, PT](file, candidate) || fileIsInDir[T, PT](file, dir)
+		})
+		canStandIn[candidate] = result
+		return result
+	}
+
+	return lo.Uniq(lo.Map(paths, func(p string, _ int) string {
+		// A directory that can't stand in for the path rules out its parents
+		// too, since they contain everything it contains. We stop short of the
+		// repository root: it would leave the command with nothing to say about
+		// the directory whose diff we are showing.
+		for candidate := path.Dir(p); candidate != "." && standsIn(candidate); candidate = path.Dir(candidate) {
+			p = candidate
+		}
+		return p
+	}))
+}
+
+func filesInTree[T any](root *filetree.Node[T]) []*T {
+	files := []*T{}
+	_ = root.ForEachFile(func(file *T) error {
+		files = append(files, file)
 		return nil
 	})
+	return files
+}
+
+// filesInDir returns the files that the given directory contains, either at
+// their current or at their previous path.
+func filesInDir[T any, PT fileWithNames[T]](files []*T, dir string) []*T {
+	return lo.Filter(files, func(file *T, _ int) bool {
+		return fileIsInDir[T, PT](file, dir)
+	})
+}
+
+func fileIsInDir[T any, PT fileWithNames[T]](f *T, dir string) bool {
+	file := PT(f)
+	previousPath := file.GetPreviousPath()
+	return isInDir(file.GetPath(), dir) || (previousPath != "" && isInDir(previousPath, dir))
 }
 
 func isInDir(path string, dir string) bool {

pkg/gui/controllers/diff_paths_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestPathsForDiff(t *testing.T) {
+func TestDiffPathsForNode(t *testing.T) {
 	files := []*models.CommitFile{
 		{Path: "dir/file1", PreviousPath: "file1", ChangeStatus: "R"},
 		{Path: "dir/file2-renamed", PreviousPath: "dir/file2", ChangeStatus: "R"},
@@ -20,6 +20,7 @@ func TestPathsForDiff(t *testing.T) {
 
 	scenarios := []struct {
 		testName      string
+		files         []*models.CommitFile // defaults to the files above
 		selectedPath  string
 		isFiltering   bool
 		expectedPaths []string
@@ -51,6 +52,38 @@ func TestPathsForDiff(t *testing.T) {
 			selectedPath:  ".",
 			expectedPaths: []string{"."},
 		},
+		{
+			testName: "a whole directory moved into the selected one collapses to that directory",
+			files: []*models.CommitFile{
+				{Path: "dir/a", PreviousPath: "src/a", ChangeStatus: "R"},
+				{Path: "dir/b", PreviousPath: "src/nested/b", ChangeStatus: "R"},
+				{Path: "dir/c", PreviousPath: "src/nested/c", ChangeStatus: "R"},
+				{Path: "unrelated", ChangeStatus: "M"},
+			},
+			selectedPath:  "dir",
+			expectedPaths: []string{"dir", "src"},
+		},
+		{
+			testName: "a directory that stands in for the selected one as well",
+			files: []*models.CommitFile{
+				{Path: "a/b/c", PreviousPath: "a/c", ChangeStatus: "R"},
+				{Path: "a/b/d", ChangeStatus: "M"},
+				{Path: "unrelated", ChangeStatus: "M"},
+			},
+			selectedPath:  "a/b",
+			expectedPaths: []string{"a"},
+		},
+		{
+			testName: "a directory with changes of its own doesn't collapse",
+			files: []*models.CommitFile{
+				{Path: "dir/a", PreviousPath: "src/a", ChangeStatus: "R"},
+				{Path: "dir/b", PreviousPath: "src/nested/b", ChangeStatus: "R"},
+				{Path: "src/nested/c", ChangeStatus: "M"},
+			},
+			selectedPath: "dir",
+			// src/nested is left out of it, so that only src/a stays behind
+			expectedPaths: []string{"dir", "src/a", "src/nested/b"},
+		},
 		{
 			testName:     "directory while filtering",
 			selectedPath: "dir",
@@ -66,6 +99,7 @@ func TestPathsForDiff(t *testing.T) {
 
 	for _, s := range scenarios {
 		t.Run(s.testName, func(t *testing.T) {
+			files := lo.Ternary(s.files != nil, s.files, files)
 			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 {
@@ -73,7 +107,7 @@ func TestPathsForDiff(t *testing.T) {
 			})
 			assert.True(t, found, "no node for path %s", s.selectedPath)
 
-			assert.Equal(t, s.expectedPaths, pathsForDiff(node, root, s.isFiltering))
+			assert.Equal(t, s.expectedPaths, diffPathsForNode(node, root, files, s.isFiltering))
 		})
 	}
 }

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().GetRoot().Raw(), self.context().IsFiltering())
+	paths := self.pathsForDiff(node)
 	cmdObj := self.c.Git().WorkingTree.WorktreeFileDiffCmdObj(node, false, mainShowsStaged, paths)
 	title := self.c.Tr.UnstagedChanges
 	if mainShowsStaged {
@@ -643,6 +643,11 @@ func (self *FilesController) press(nodes []*filetree.FileNode) error {
 	return nil
 }
 
+func (self *FilesController) pathsForDiff(node *filetree.FileNode) []string {
+	return diffPathsForNode(
+		node.Raw(), self.context().GetRoot().Raw(), self.c.Model().Files, self.context().IsFiltering())
+}
+
 // unstageFilteredFiles unstages only the visible (filtered) files from the
 // given nodes, correctly partitioning by tracked/untracked.
 func (self *FilesController) unstageFilteredFiles(nodes []*filetree.FileNode) error {