Show diff for renamed file when filtering by path

pkg/commands/git_commands/commit.go

@@ -253,7 +253,7 @@ func (self *CommitCommands) AmendHeadCmdObj() *oscommands.CmdObj {
 	return self.cmd.New(cmdArgs)
 }
 
-func (self *CommitCommands) ShowCmdObj(hash string, filterPath string) *oscommands.CmdObj {
+func (self *CommitCommands) ShowCmdObj(hash string, filterPaths []string) *oscommands.CmdObj {
 	contextSize := self.UserConfig().Git.DiffContextSize
 
 	extDiffCmd := self.UserConfig().Git.Paging.ExternalDiffCommand
@@ -270,7 +270,8 @@ func (self *CommitCommands) ShowCmdObj(hash string, filterPath string) *oscomman
 		Arg(hash).
 		ArgIf(self.UserConfig().Git.IgnoreWhitespaceInDiffView, "--ignore-all-space").
 		Arg(fmt.Sprintf("--find-renames=%d%%", self.UserConfig().Git.RenameSimilarityThreshold)).
-		ArgIf(filterPath != "", "--", filterPath).
+		Arg("--").
+		Arg(filterPaths...).
 		Dir(self.repoPaths.worktreePath).
 		ToArgv()
 

pkg/commands/git_commands/commit_loader.go

@@ -89,11 +89,13 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit,
 	go utils.Safe(func() {
 		defer wg.Done()
 
-		logErr = self.getLogCmd(opts).RunAndProcessLines(func(line string) (bool, error) {
-			commit := self.extractCommitFromLine(opts.HashPool, line, opts.RefToShowDivergenceFrom != "")
-			commits = append(commits, commit)
-			return false, nil
+		var realCommits []*models.Commit
+		realCommits, logErr = loadCommits(self.getLogCmd(opts), opts.FilterPath, func(line string) (*models.Commit, bool) {
+			return self.extractCommitFromLine(opts.HashPool, line, opts.RefToShowDivergenceFrom != ""), false
 		})
+		if logErr == nil {
+			commits = append(commits, realCommits...)
+		}
 	})
 
 	var ancestor string
@@ -292,7 +294,10 @@ func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, tod
 
 	fullCommits := map[string]*models.Commit{}
 	err := cmdObj.RunAndProcessLines(func(line string) (bool, error) {
-		commit := self.extractCommitFromLine(hashPool, line, false)
+		if line == "" || line[0] != '+' {
+			return false, nil
+		}
+		commit := self.extractCommitFromLine(hashPool, line[1:], false)
 		fullCommits[commit.Hash()] = commit
 		return false, nil
 	})
@@ -599,7 +604,7 @@ func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) *oscommands.CmdObj {
 		Arg("--abbrev=40").
 		ArgIf(opts.FilterAuthor != "", "--author="+opts.FilterAuthor).
 		ArgIf(opts.Limit, "-300").
-		ArgIf(opts.FilterPath != "", "--follow").
+		ArgIf(opts.FilterPath != "", "--follow", "--name-status").
 		Arg("--no-show-signature").
 		ArgIf(opts.RefToShowDivergenceFrom != "", "--left-right").
 		Arg("--").
@@ -609,4 +614,4 @@ func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) *oscommands.CmdObj {
 	return self.cmd.New(cmdArgs).DontLog()
 }
 
-const prettyFormat = `--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s`
+const prettyFormat = `--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s`

pkg/commands/git_commands/commit_loader_test.go

@@ -15,16 +15,16 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var commitsOutput = strings.ReplaceAll(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode
-b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|[email protected]|origin/better-tests|e94e8fc5b6fab4cb755f|>|fix logging
-e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|[email protected]|tag: 123, tag: 456|d8084cd558925eb7c9c3|>|refactor
-d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|[email protected]||65f910ebd85283b5cce9|>|WIP
-65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|[email protected]||26c07b1ab33860a1a759|>|WIP
-26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|[email protected]||3d4470a6c072208722e5|>|WIP
-3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|[email protected]||053a66a7be3da43aacdc|>|WIP
-053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|[email protected]||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00")
+var commitsOutput = strings.ReplaceAll(`+0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode
++b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|[email protected]|origin/better-tests|e94e8fc5b6fab4cb755f|>|fix logging
++e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|[email protected]|tag: 123, tag: 456|d8084cd558925eb7c9c3|>|refactor
++d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|[email protected]||65f910ebd85283b5cce9|>|WIP
++65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|[email protected]||26c07b1ab33860a1a759|>|WIP
++26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|[email protected]||3d4470a6c072208722e5|>|WIP
++3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|[email protected]||053a66a7be3da43aacdc|>|WIP
++053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|[email protected]||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00")
 
-var singleCommitOutput = strings.ReplaceAll(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode`, "|", "\x00")
+var singleCommitOutput = strings.ReplaceAll(`+0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode`, "|", "\x00")
 
 func TestGetCommits(t *testing.T) {
 	type scenario struct {
@@ -44,7 +44,7 @@ func TestGetCommits(t *testing.T) {
 			opts:     GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
-				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
+				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
 
 			expectedCommitOpts: []models.NewCommitOpts{},
 			expectedError:      nil,
@@ -55,7 +55,7 @@ func TestGetCommits(t *testing.T) {
 			opts:     GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
-				ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
+				ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
 
 			expectedCommitOpts: []models.NewCommitOpts{},
 			expectedError:      nil,
@@ -69,7 +69,7 @@ func TestGetCommits(t *testing.T) {
 				// here it's seeing which commits are yet to be pushed
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
 				// here it's actually getting all the commits in a formatted form, one per line
-				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil).
+				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil).
 				// here it's testing which of the configured main branches have an upstream
 				ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil).       // this one does
 				ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")).                   // this one doesn't, so it checks origin instead
@@ -205,7 +205,7 @@ func TestGetCommits(t *testing.T) {
 				// here it's seeing which commits are yet to be pushed
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
 				// here it's actually getting all the commits in a formatted form, one per line
-				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
+				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
 				// here it's testing which of the configured main branches exist; neither does
 				ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "", errors.New("error")).
 				ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/master"}, "", errors.New("error")).
@@ -241,7 +241,7 @@ func TestGetCommits(t *testing.T) {
 				// here it's seeing which commits are yet to be pushed
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
 				// here it's actually getting all the commits in a formatted form, one per line
-				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
+				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
 				// here it's testing which of the configured main branches exist
 				ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil).
 				ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")).
@@ -276,7 +276,7 @@ func TestGetCommits(t *testing.T) {
 			opts:     GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
-				ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
+				ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
 
 			expectedCommitOpts: []models.NewCommitOpts{},
 			expectedError:      nil,
@@ -287,7 +287,7 @@ func TestGetCommits(t *testing.T) {
 			opts:     GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
-				ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil),
+				ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--follow", "--name-status", "--no-show-signature", "--", "src"}, "", nil),
 
 			expectedCommitOpts: []models.NewCommitOpts{},
 			expectedError:      nil,

pkg/commands/git_commands/commit_loading_shared.go

@@ -0,0 +1,74 @@
+package git_commands
+
+import (
+	"strings"
+
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
+	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
+	"github.com/samber/lo"
+)
+
+func loadCommits(
+	cmd *oscommands.CmdObj,
+	filterPath string,
+	parseLogLine func(string) (*models.Commit, bool),
+) ([]*models.Commit, error) {
+	commits := []*models.Commit{}
+
+	var commit *models.Commit
+	var filterPaths []string
+	// A string pool that stores interned strings to reduce memory usage
+	pool := make(map[string]string)
+
+	finishLastCommit := func() {
+		if commit != nil {
+			// Only set the filter paths if we have one that is not contained in the original
+			// filter path. When filtering on a directory, all file paths will start with that
+			// directory, so we needn't bother storing the individual paths. Likewise, if we
+			// filter on a file and the file path hasn't changed, we needn't store it either.
+			// Only if a file has been moved or renamed do we need to store the paths, but then
+			// we need them all so that we can properly render a diff for the rename.
+			if lo.SomeBy(filterPaths, func(path string) bool {
+				return !strings.HasPrefix(path, filterPath)
+			}) {
+				commit.FilterPaths = lo.Map(filterPaths, func(path string, _ int) string {
+					if v, ok := pool[path]; ok {
+						return v
+					}
+					pool[path] = path
+					return path
+				})
+			}
+			commits = append(commits, commit)
+			commit = nil
+			filterPaths = nil
+		}
+	}
+	err := cmd.RunAndProcessLines(func(line string) (bool, error) {
+		if line == "" {
+			return false, nil
+		}
+
+		if line[0] == '+' {
+			finishLastCommit()
+			var stop bool
+			commit, stop = parseLogLine(line[1:])
+			if stop {
+				commit = nil
+				return true, nil
+			}
+		} else if commit != nil && filterPath != "" {
+			// We are filtering by path, and this line is the output of the --name-status flag
+			fields := strings.Split(line, "\t")
+			// We don't bother looking at the first field (it will be 'A', 'M', 'R072' or a bunch of others).
+			// All we care about is the path(s), and there will be one for 'M' and 'A', and two for 'R' or 'C',
+			// in which case we want them both so that we can show the diff between the two.
+			if len(fields) > 1 {
+				filterPaths = append(filterPaths, fields[1:]...)
+			}
+		}
+		return false, nil
+	})
+	finishLastCommit()
+	return commits, err
+}

pkg/commands/git_commands/commit_test.go

@@ -251,7 +251,7 @@ func TestCommitCreateAmendCommit(t *testing.T) {
 func TestCommitShowCmdObj(t *testing.T) {
 	type scenario struct {
 		testName            string
-		filterPath          string
+		filterPaths         []string
 		contextSize         uint64
 		similarityThreshold int
 		ignoreWhitespace    bool
@@ -262,16 +262,16 @@ func TestCommitShowCmdObj(t *testing.T) {
 	scenarios := []scenario{
 		{
 			testName:            "Default case without filter path",
-			filterPath:          "",
+			filterPaths:         []string{},
 			contextSize:         3,
 			similarityThreshold: 50,
 			ignoreWhitespace:    false,
 			extDiffCmd:          "",
-			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--find-renames=50%"},
+			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--find-renames=50%", "--"},
 		},
 		{
 			testName:            "Default case with filter path",
-			filterPath:          "file.txt",
+			filterPaths:         []string{"file.txt"},
 			contextSize:         3,
 			similarityThreshold: 50,
 			ignoreWhitespace:    false,
@@ -280,39 +280,39 @@ func TestCommitShowCmdObj(t *testing.T) {
 		},
 		{
 			testName:            "Show diff with custom context size",
-			filterPath:          "",
+			filterPaths:         []string{},
 			contextSize:         77,
 			similarityThreshold: 50,
 			ignoreWhitespace:    false,
 			extDiffCmd:          "",
-			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890", "--find-renames=50%"},
+			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890", "--find-renames=50%", "--"},
 		},
 		{
 			testName:            "Show diff with custom similarity threshold",
-			filterPath:          "",
+			filterPaths:         []string{},
 			contextSize:         3,
 			similarityThreshold: 33,
 			ignoreWhitespace:    false,
 			extDiffCmd:          "",
-			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--find-renames=33%"},
+			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--find-renames=33%", "--"},
 		},
 		{
 			testName:            "Show diff, ignoring whitespace",
-			filterPath:          "",
+			filterPaths:         []string{},
 			contextSize:         77,
 			similarityThreshold: 50,
 			ignoreWhitespace:    true,
 			extDiffCmd:          "",
-			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890", "--ignore-all-space", "--find-renames=50%"},
+			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890", "--ignore-all-space", "--find-renames=50%", "--"},
 		},
 		{
 			testName:            "Show diff with external diff command",
-			filterPath:          "",
+			filterPaths:         []string{},
 			contextSize:         3,
 			similarityThreshold: 50,
 			ignoreWhitespace:    false,
 			extDiffCmd:          "difft --color=always",
-			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.external=difft --color=always", "-c", "diff.noprefix=false", "show", "--ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--find-renames=50%"},
+			expected:            []string{"-C", "/path/to/worktree", "-c", "diff.external=difft --color=always", "-c", "diff.noprefix=false", "show", "--ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--find-renames=50%", "--"},
 		},
 	}
 
@@ -330,7 +330,7 @@ func TestCommitShowCmdObj(t *testing.T) {
 			}
 			instance := buildCommitCommands(commonDeps{userConfig: userConfig, appState: &config.AppState{}, runner: runner, repoPaths: &repoPaths})
 
-			assert.NoError(t, instance.ShowCmdObj("1234567890", s.filterPath).Run())
+			assert.NoError(t, instance.ShowCmdObj("1234567890", s.filterPaths).Run())
 			runner.CheckForMissingCalls()
 		})
 	}