From 0f7f1a56df4b0785d2c202182e0d97443ecf51a0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 30 May 2025 10:08:13 +0200 Subject: [PATCH] Fix showing diffs for renamed file when filtering by path When filtering for a file path we use the --follow option for "git log", so it will follow renames of the file, which is great. However, if you then selected one of the commits before a rename, you didn't see a diff, because we would pass the original filter path to the "git show" call. To fix this, call git log with the --name-status option when filtering by path, so that each commit reports which file paths are touched in this commit; remember these in the commit object, so that we can pass them to the "git show" call later. Be careful not to store too many such paths unnecessarily. When filtering by folder rather than file, all these paths will necessarily be inside the original filter path, so detect this and don't store them in this case. There is some unfortunate code duplication between loading commits and loading reflog commits, which I am too lazy to clean up right now. --- pkg/commands/git_commands/commit.go | 4 +- pkg/commands/git_commands/commit_loader.go | 11 ++-- .../git_commands/commit_loader_test.go | 32 +++++----- .../git_commands/commit_loading_shared.go | 61 +++++++++++++++++-- pkg/commands/git_commands/commit_test.go | 16 ++--- .../git_commands/reflog_commit_loader.go | 6 +- .../git_commands/reflog_commit_loader_test.go | 22 +++---- pkg/commands/models/commit.go | 4 ++ pkg/gui/controllers/helpers/diff_helper.go | 13 +++- .../controllers/reflog_commits_controller.go | 2 +- .../show_diffs_for_renamed_file.go | 17 ------ 11 files changed, 120 insertions(+), 68 deletions(-) diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index 2927a3038..0abb98168 100644 --- a/pkg/commands/git_commands/commit.go +++ b/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 @@ -271,7 +271,7 @@ func (self *CommitCommands) ShowCmdObj(hash string, filterPath string) *oscomman ArgIf(self.UserConfig().Git.IgnoreWhitespaceInDiffView, "--ignore-all-space"). Arg(fmt.Sprintf("--find-renames=%d%%", self.UserConfig().Git.RenameSimilarityThreshold)). Arg("--"). - ArgIf(filterPath != "", filterPath). + Arg(filterPaths...). Dir(self.repoPaths.worktreePath). ToArgv() diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 9fff74cdc..e8480ec1b 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -90,7 +90,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, defer wg.Done() var realCommits []*models.Commit - realCommits, logErr = loadCommits(self.getLogCmd(opts), func(line string) (*models.Commit, bool) { + 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 { @@ -294,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 }) @@ -601,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("--"). @@ -611,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` diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 6f20e7cc6..ba0cd9d84 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/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|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode -b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|jessedduffield@gmail.com|origin/better-tests|e94e8fc5b6fab4cb755f|>|fix logging -e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|jessedduffield@gmail.com|tag: 123, tag: 456|d8084cd558925eb7c9c3|>|refactor -d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|jessedduffield@gmail.com||65f910ebd85283b5cce9|>|WIP -65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|jessedduffield@gmail.com||26c07b1ab33860a1a759|>|WIP -26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|jessedduffield@gmail.com||3d4470a6c072208722e5|>|WIP -3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|jessedduffield@gmail.com||053a66a7be3da43aacdc|>|WIP -053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|jessedduffield@gmail.com||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00") +var commitsOutput = strings.ReplaceAll(`+0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode ++b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|jessedduffield@gmail.com|origin/better-tests|e94e8fc5b6fab4cb755f|>|fix logging ++e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|jessedduffield@gmail.com|tag: 123, tag: 456|d8084cd558925eb7c9c3|>|refactor ++d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|jessedduffield@gmail.com||65f910ebd85283b5cce9|>|WIP ++65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|jessedduffield@gmail.com||26c07b1ab33860a1a759|>|WIP ++26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|jessedduffield@gmail.com||3d4470a6c072208722e5|>|WIP ++3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|jessedduffield@gmail.com||053a66a7be3da43aacdc|>|WIP ++053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|jessedduffield@gmail.com||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00") -var singleCommitOutput = strings.ReplaceAll(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode`, "|", "\x00") +var singleCommitOutput = strings.ReplaceAll(`+0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|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, diff --git a/pkg/commands/git_commands/commit_loading_shared.go b/pkg/commands/git_commands/commit_loading_shared.go index 9bf774186..f1a14698e 100644 --- a/pkg/commands/git_commands/commit_loading_shared.go +++ b/pkg/commands/git_commands/commit_loading_shared.go @@ -1,23 +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{} - err := cmd.RunAndProcessLines(func(line string) (bool, error) { - commit, stop := parseLogLine(line) - if stop { - return true, nil + 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:]...) + } } - commits = append(commits, commit) return false, nil }) + finishLastCommit() return commits, err } diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index 6abe12128..84d3126b5 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/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,7 +262,7 @@ func TestCommitShowCmdObj(t *testing.T) { scenarios := []scenario{ { testName: "Default case without filter path", - filterPath: "", + filterPaths: []string{}, contextSize: 3, similarityThreshold: 50, ignoreWhitespace: false, @@ -271,7 +271,7 @@ func TestCommitShowCmdObj(t *testing.T) { }, { testName: "Default case with filter path", - filterPath: "file.txt", + filterPaths: []string{"file.txt"}, contextSize: 3, similarityThreshold: 50, ignoreWhitespace: false, @@ -280,7 +280,7 @@ func TestCommitShowCmdObj(t *testing.T) { }, { testName: "Show diff with custom context size", - filterPath: "", + filterPaths: []string{}, contextSize: 77, similarityThreshold: 50, ignoreWhitespace: false, @@ -289,7 +289,7 @@ func TestCommitShowCmdObj(t *testing.T) { }, { testName: "Show diff with custom similarity threshold", - filterPath: "", + filterPaths: []string{}, contextSize: 3, similarityThreshold: 33, ignoreWhitespace: false, @@ -298,7 +298,7 @@ func TestCommitShowCmdObj(t *testing.T) { }, { testName: "Show diff, ignoring whitespace", - filterPath: "", + filterPaths: []string{}, contextSize: 77, similarityThreshold: 50, ignoreWhitespace: true, @@ -307,7 +307,7 @@ func TestCommitShowCmdObj(t *testing.T) { }, { testName: "Show diff with external diff command", - filterPath: "", + filterPaths: []string{}, contextSize: 3, similarityThreshold: 50, ignoreWhitespace: false, @@ -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() }) } diff --git a/pkg/commands/git_commands/reflog_commit_loader.go b/pkg/commands/git_commands/reflog_commit_loader.go index da0d1c3e0..053fc4598 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -28,16 +28,16 @@ func (self *ReflogCommitLoader) GetReflogCommits(hashPool *utils.StringPool, las cmdArgs := NewGitCmd("log"). Config("log.showSignature=false"). Arg("-g"). - Arg("--format=%H%x00%ct%x00%gs%x00%P"). + Arg("--format=+%H%x00%ct%x00%gs%x00%P"). ArgIf(filterAuthor != "", "--author="+filterAuthor). - ArgIf(filterPath != "", "--follow", "--", filterPath). + ArgIf(filterPath != "", "--follow", "--name-status", "--", filterPath). ToArgv() cmdObj := self.cmd.New(cmdArgs).DontLog() onlyObtainedNewReflogCommits := false - commits, err := loadCommits(cmdObj, func(line string) (*models.Commit, bool) { + commits, err := loadCommits(cmdObj, filterPath, func(line string) (*models.Commit, bool) { commit, ok := self.parseLine(hashPool, line) if !ok { return nil, false diff --git a/pkg/commands/git_commands/reflog_commit_loader_test.go b/pkg/commands/git_commands/reflog_commit_loader_test.go index 468f6ca06..839533f3f 100644 --- a/pkg/commands/git_commands/reflog_commit_loader_test.go +++ b/pkg/commands/git_commands/reflog_commit_loader_test.go @@ -14,11 +14,11 @@ import ( "github.com/stretchr/testify/assert" ) -var reflogOutput = strings.ReplaceAll(`c3c4b66b64c97ffeecde|1643150483|checkout: moving from A to B|51baa8c1 -c3c4b66b64c97ffeecde|1643150483|checkout: moving from B to A|51baa8c1 -c3c4b66b64c97ffeecde|1643150483|checkout: moving from A to B|51baa8c1 -c3c4b66b64c97ffeecde|1643150483|checkout: moving from master to A|51baa8c1 -f4ddf2f0d4be4ccc7efa|1643149435|checkout: moving from A to master|51baa8c1 +var reflogOutput = strings.ReplaceAll(`+c3c4b66b64c97ffeecde|1643150483|checkout: moving from A to B|51baa8c1 ++c3c4b66b64c97ffeecde|1643150483|checkout: moving from B to A|51baa8c1 ++c3c4b66b64c97ffeecde|1643150483|checkout: moving from A to B|51baa8c1 ++c3c4b66b64c97ffeecde|1643150483|checkout: moving from master to A|51baa8c1 ++f4ddf2f0d4be4ccc7efa|1643149435|checkout: moving from A to master|51baa8c1 `, "|", "\x00") func TestGetReflogCommits(t *testing.T) { @@ -39,7 +39,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "no reflog entries", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=%H%x00%ct%x00%gs%x00%P"}, "", nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=+%H%x00%ct%x00%gs%x00%P"}, "", nil), lastReflogCommit: nil, expectedCommitOpts: []models.NewCommitOpts{}, @@ -49,7 +49,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "some reflog entries", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=%H%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=+%H%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), lastReflogCommit: nil, expectedCommitOpts: []models.NewCommitOpts{ @@ -95,7 +95,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "some reflog entries where last commit is given", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=%H%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=+%H%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", @@ -119,7 +119,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "when passing filterPath", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=%H%x00%ct%x00%gs%x00%P", "--follow", "--", "path"}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=+%H%x00%ct%x00%gs%x00%P", "--follow", "--name-status", "--", "path"}, reflogOutput, nil), lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", @@ -144,7 +144,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "when passing filterAuthor", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=%H%x00%ct%x00%gs%x00%P", "--author=John Doe "}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=+%H%x00%ct%x00%gs%x00%P", "--author=John Doe "}, reflogOutput, nil), lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", @@ -169,7 +169,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "when command returns error", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=%H%x00%ct%x00%gs%x00%P"}, "", errors.New("haha")), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--format=+%H%x00%ct%x00%gs%x00%P"}, "", errors.New("haha")), lastReflogCommit: nil, filterPath: "", diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index eca66b3ac..8dfec4d4b 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -54,6 +54,10 @@ type Commit struct { // Hashes of parent commits (will be multiple if it's a merge commit) parents []*string + // When filtering by path, this contains the paths that were changed in this + // commit; nil when not filtering by path. + FilterPaths []string + Status CommitStatus Action todo.TodoCommand Divergence Divergence // set to DivergenceNone unless we are showing the divergence view diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index 9a08f15ec..0ef23de36 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -65,10 +65,21 @@ func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff(commit *models.Comm return types.NewRunPtyTaskWithPrefix(cmdObj.GetCmd(), prefix) } - cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.c.Modes().Filtering.GetPath()) + cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.FilterPathsForCommit(commit)) return types.NewRunPtyTask(cmdObj.GetCmd()) } +func (self *DiffHelper) FilterPathsForCommit(commit *models.Commit) []string { + filterPath := self.c.Modes().Filtering.GetPath() + if filterPath != "" { + if len(commit.FilterPaths) > 0 { + return commit.FilterPaths + } + return []string{filterPath} + } + return nil +} + func (self *DiffHelper) ExitDiffMode() error { self.c.Modes().Diffing = diffing.New() self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) diff --git a/pkg/gui/controllers/reflog_commits_controller.go b/pkg/gui/controllers/reflog_commits_controller.go index ea35eca44..2d0751a0b 100644 --- a/pkg/gui/controllers/reflog_commits_controller.go +++ b/pkg/gui/controllers/reflog_commits_controller.go @@ -45,7 +45,7 @@ func (self *ReflogCommitsController) GetOnRenderToMain() func() { if commit == nil { task = types.NewRenderStringTask("No reflog history") } else { - cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.c.Modes().Filtering.GetPath()) + cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.c.Helpers().Diff.FilterPathsForCommit(commit)) task = types.NewRunPtyTask(cmdObj.GetCmd()) } diff --git a/pkg/integration/tests/filter_by_path/show_diffs_for_renamed_file.go b/pkg/integration/tests/filter_by_path/show_diffs_for_renamed_file.go index 3d91e391c..b0d574056 100644 --- a/pkg/integration/tests/filter_by_path/show_diffs_for_renamed_file.go +++ b/pkg/integration/tests/filter_by_path/show_diffs_for_renamed_file.go @@ -80,7 +80,6 @@ var ShowDiffsForRenamedFile = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Main().ContainsLines( Equals(" rename file"), Equals("---"), - /* EXPECTED: Equals(" oldFile => newFile | 0"), Equals(" 1 file changed, 0 insertions(+), 0 deletions(-)"), Equals(""), @@ -88,24 +87,10 @@ var ShowDiffsForRenamedFile = NewIntegrationTest(NewIntegrationTestArgs{ Equals("similarity index 100%"), Equals("rename from oldFile"), Equals("rename to newFile"), - ACTUAL: */ - Equals(" newFile | 3 +++"), - Equals(" 1 file changed, 3 insertions(+)"), - Equals(""), - Equals("diff --git a/newFile b/newFile"), - Equals("new file mode 100644"), - Contains("index"), - Equals("--- /dev/null"), - Equals("+++ b/newFile"), - Equals("@@ -0,0 +1,3 @@"), - Equals("+x"), - Equals("+b"), - Equals("+c"), ) t.Views().Commits().SelectNextItem() - /* EXPECTED: t.Views().Main().ContainsLines( Equals(" update old file"), Equals("---"), @@ -122,7 +107,5 @@ var ShowDiffsForRenamedFile = NewIntegrationTest(NewIntegrationTestArgs{ Equals(" b"), Equals(" c"), ) - ACTUAL: */ - t.Views().Main().IsEmpty() }, })