diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index 11b75c8f0..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 @@ -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() diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 7cdac4c33..e8480ec1b 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/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` 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 new file mode 100644 index 000000000..f1a14698e --- /dev/null +++ b/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 +} diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index 112cf6b77..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,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() }) } diff --git a/pkg/commands/git_commands/reflog_commit_loader.go b/pkg/commands/git_commands/reflog_commit_loader.go index 1e981b1e1..053fc4598 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -25,24 +25,22 @@ func NewReflogCommitLoader(common *common.Common, cmd oscommands.ICmdObjBuilder) // GetReflogCommits only returns the new reflog commits since the given lastReflogCommit // if none is passed (i.e. it's value is nil) then we get all the reflog commits func (self *ReflogCommitLoader) GetReflogCommits(hashPool *utils.StringPool, lastReflogCommit *models.Commit, filterPath string, filterAuthor string) ([]*models.Commit, bool, error) { - commits := make([]*models.Commit, 0) - cmdArgs := NewGitCmd("log"). Config("log.showSignature=false"). Arg("-g"). - Arg("--abbrev=40"). - 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 - err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { + + commits, err := loadCommits(cmdObj, filterPath, func(line string) (*models.Commit, bool) { commit, ok := self.parseLine(hashPool, line) if !ok { - return false, nil + return nil, false } // note that the unix timestamp here is the timestamp of the COMMIT, not the reflog entry itself, @@ -52,11 +50,10 @@ func (self *ReflogCommitLoader) GetReflogCommits(hashPool *utils.StringPool, las if lastReflogCommit != nil && self.sameReflogCommit(commit, lastReflogCommit) { onlyObtainedNewReflogCommits = true // after this point we already have these reflogs loaded so we'll simply return the new ones - return true, nil + return nil, true } - commits = append(commits, commit) - return false, nil + return commit, false }) if err != nil { return nil, false, err diff --git a/pkg/commands/git_commands/reflog_commit_loader_test.go b/pkg/commands/git_commands/reflog_commit_loader_test.go index 01e479e5c..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", "--abbrev=40", "--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", "--abbrev=40", "--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", "--abbrev=40", "--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", "--abbrev=40", "--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", "--abbrev=40", "--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", "--abbrev=40", "--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.go b/pkg/gui/controllers.go index dec0ab23a..ba116b9ed 100644 --- a/pkg/gui/controllers.go +++ b/pkg/gui/controllers.go @@ -90,12 +90,6 @@ func (gui *Gui) resetHelpersAndControllers() { modeHelper, ) - setSubCommits := func(commits []*models.Commit) { - gui.Mutexes.SubCommitsMutex.Lock() - defer gui.Mutexes.SubCommitsMutex.Unlock() - - gui.State.Model.SubCommits = commits - } gui.helpers = &helpers.Helpers{ Refs: refsHelper, Host: helpers.NewHostHelper(helperCommon), @@ -135,7 +129,7 @@ func (gui *Gui) resetHelpersAndControllers() { ), Search: searchHelper, Worktree: worktreeHelper, - SubCommits: helpers.NewSubCommitsHelper(helperCommon, refreshHelper, setSubCommits), + SubCommits: helpers.NewSubCommitsHelper(helperCommon, refreshHelper), } gui.CustomCommandsClient = custom_commands.NewClient( diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index 9a08f15ec..1fd73081a 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -57,18 +57,41 @@ func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff(commit *models.Comm from, to := refRange.From, refRange.To args := []string{from.ParentRefName(), to.RefName(), "--stat", "-p"} args = append(args, "--") - if path := self.c.Modes().Filtering.GetPath(); path != "" { - args = append(args, path) + if filterPath := self.c.Modes().Filtering.GetPath(); filterPath != "" { + // If both refs are commits, filter by the union of their paths. This is useful for + // example when diffing a range of commits in filter-by-path mode across a rename. + fromCommit, ok1 := from.(*models.Commit) + toCommit, ok2 := to.(*models.Commit) + if ok1 && ok2 { + paths := append(self.FilterPathsForCommit(fromCommit), self.FilterPathsForCommit(toCommit)...) + args = append(args, lo.Uniq(paths)...) + } else { + // If either ref is not a commit (which is possible in sticky diff mode, when + // diffing against a branch or tag), we just filter by the filter path; that's the + // best we can do in this case. + args = append(args, filterPath) + } } cmdObj := self.c.Git().Diff.DiffCmdObj(args) prefix := style.FgYellow.Sprintf("%s %s-%s\n\n", self.c.Tr.ShowingDiffForRange, from.ShortRefName(), to.ShortRefName()) 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/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index dc396be3f..24958c65d 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -449,21 +449,8 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele self.c.Mutexes().RefreshingBranchesMutex.Lock() defer self.c.Mutexes().RefreshingBranchesMutex.Unlock() - reflogCommits := self.c.Model().FilteredReflogCommits - if self.c.Modes().Filtering.Active() && self.c.UserConfig().Git.LocalBranchSortOrder == "recency" { - // in filter mode we filter our reflog commits to just those containing the path - // however we need all the reflog entries to populate the recencies of our branches - // which allows us to order them correctly. So if we're filtering we'll just - // manually load all the reflog commits here - var err error - reflogCommits, _, err = self.c.Git().Loaders.ReflogCommitLoader.GetReflogCommits(self.c.Model().HashPool, nil, "", "") - if err != nil { - self.c.Log.Error(err) - } - } - branches, err := self.c.Git().Loaders.BranchLoader.Load( - reflogCommits, + self.c.Model().ReflogCommits, self.c.Model().MainBranches, self.c.Model().Branches, loadBehindCounts, @@ -620,12 +607,13 @@ func (self *RefreshHelper) refreshReflogCommits() error { // pulling state into its own variable in case it gets swapped out for another state // and we get an out of bounds exception model := self.c.Model() - var lastReflogCommit *models.Commit - if len(model.ReflogCommits) > 0 { - lastReflogCommit = model.ReflogCommits[0] - } refresh := func(stateCommits *[]*models.Commit, filterPath string, filterAuthor string) error { + var lastReflogCommit *models.Commit + if filterPath == "" && filterAuthor == "" && len(*stateCommits) > 0 { + lastReflogCommit = (*stateCommits)[0] + } + commits, onlyObtainedNewReflogCommits, err := self.c.Git().Loaders.ReflogCommitLoader. GetReflogCommits(self.c.Model().HashPool, lastReflogCommit, filterPath, filterAuthor) if err != nil { diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index cffd8e8ed..e61c4fee6 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/pkg/gui/controllers/helpers/sub_commits_helper.go @@ -11,18 +11,15 @@ type SubCommitsHelper struct { c *HelperCommon refreshHelper *RefreshHelper - setSubCommits func([]*models.Commit) } func NewSubCommitsHelper( c *HelperCommon, refreshHelper *RefreshHelper, - setSubCommits func([]*models.Commit), ) *SubCommitsHelper { return &SubCommitsHelper{ c: c, refreshHelper: refreshHelper, - setSubCommits: setSubCommits, } } @@ -73,3 +70,10 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error { self.c.Context().Push(self.c.Contexts().SubCommits, types.OnFocusOpts{}) return nil } + +func (self *SubCommitsHelper) setSubCommits(commits []*models.Commit) { + self.c.Mutexes().SubCommitsMutex.Lock() + defer self.c.Mutexes().SubCommitsMutex.Unlock() + + self.c.Model().SubCommits = commits +} 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/gui/controllers/undo_controller.go b/pkg/gui/controllers/undo_controller.go index ecff5bea8..cd3b04e4f 100644 --- a/pkg/gui/controllers/undo_controller.go +++ b/pkg/gui/controllers/undo_controller.go @@ -199,7 +199,7 @@ func (self *UndoController) reflogRedo() error { // Though we might support this later, hence the use of the CURRENT_REBASE action kind. func (self *UndoController) parseReflogForActions(onUserAction func(counter int, action reflogAction) (bool, error)) error { counter := 0 - reflogCommits := self.c.Model().FilteredReflogCommits + reflogCommits := self.c.Model().ReflogCommits rebaseFinishCommitHash := "" var action *reflogAction for reflogCommitIdx, reflogCommit := range reflogCommits { diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index bf7267a8c..7d8528ee8 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -289,10 +289,11 @@ type Model struct { Worktrees []*models.Worktree // FilteredReflogCommits are the ones that appear in the reflog panel. - // when in filtering mode we only include the ones that match the given path + // When in filtering mode we only include the ones that match the given path FilteredReflogCommits []*models.Commit - // ReflogCommits are the ones used by the branches panel to obtain recency values - // if we're not in filtering mode, CommitFiles and FilteredReflogCommits will be + // ReflogCommits are the ones used by the branches panel to obtain recency values, + // and for the undo functionality. + // If we're not in filtering mode, CommitFiles and FilteredReflogCommits will be // one and the same ReflogCommits []*models.Commit diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index 6b711e5a8..4c833f2ab 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -235,6 +235,10 @@ func (self *Shell) DeleteFileAndAdd(fileName string) *Shell { GitAdd(fileName) } +func (self *Shell) RenameFileInGit(oldName string, newName string) *Shell { + return self.RunCommand([]string{"git", "mv", oldName, newName}) +} + // creates commits 01, 02, 03, ..., n with a new file in each // The reason for padding with zeroes is so that it's easier to do string // matches on the commit messages when there are many of them 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 new file mode 100644 index 000000000..720a681e1 --- /dev/null +++ b/pkg/integration/tests/filter_by_path/show_diffs_for_renamed_file.go @@ -0,0 +1,135 @@ +package filter_by_path + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ShowDiffsForRenamedFile = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Filter commits by file path for a file that was renamed, and verify that it shows the diffs correctly", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("oldFile", "a\nb\nc\n") + shell.Commit("add old file") + shell.UpdateFileAndAdd("oldFile", "x\nb\nc\n") + shell.Commit("update old file") + shell.CreateFileAndAdd("unrelatedFile", "content of unrelated file\n") + shell.Commit("add unrelated file") + shell.RenameFileInGit("oldFile", "newFile") + shell.Commit("rename file") + shell.UpdateFileAndAdd("newFile", "y\nb\nc\n") + shell.UpdateFileAndAdd("unrelatedFile", "updated content of unrelated file\n") + shell.Commit("update both files") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("update both files").IsSelected(), + Contains("rename file"), + Contains("add unrelated file"), + Contains("update old file"), + Contains("add old file"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("▼ /").IsSelected(), + Equals(" M newFile"), + Equals(" M unrelatedFile"), + ). + SelectNextItem(). + Press(keys.Universal.FilteringMenu) + + t.ExpectPopup().Menu().Title(Equals("Filtering")). + Select(Contains("Filter by 'newFile'")).Confirm() + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("update both files").IsSelected(), + Contains("rename file"), + Contains("update old file"), + Contains("add old file"), + ) + + t.Views().Main().ContainsLines( + Equals(" update both files"), + Equals("---"), + Equals(" newFile | 2 +-"), + Equals(" 1 file changed, 1 insertion(+), 1 deletion(-)"), + Equals(""), + Equals("diff --git a/newFile b/newFile"), + Contains("index"), + Equals("--- a/newFile"), + Equals("+++ b/newFile"), + Equals("@@ -1,3 +1,3 @@"), + Equals("-x"), + Equals("+y"), + Equals(" b"), + Equals(" c"), + ) + + t.Views().Commits().SelectNextItem() + + t.Views().Main().ContainsLines( + Equals(" rename file"), + Equals("---"), + Equals(" oldFile => newFile | 0"), + Equals(" 1 file changed, 0 insertions(+), 0 deletions(-)"), + Equals(""), + Equals("diff --git a/oldFile b/newFile"), + Equals("similarity index 100%"), + Equals("rename from oldFile"), + Equals("rename to newFile"), + ) + + t.Views().Commits().SelectNextItem() + + t.Views().Main().ContainsLines( + Equals(" update old file"), + Equals("---"), + Equals(" oldFile | 2 +-"), + Equals(" 1 file changed, 1 insertion(+), 1 deletion(-)"), + Equals(""), + Equals("diff --git a/oldFile b/oldFile"), + Contains("index"), + Equals("--- a/oldFile"), + Equals("+++ b/oldFile"), + Equals("@@ -1,3 +1,3 @@"), + Equals("-a"), + Equals("+x"), + Equals(" b"), + Equals(" c"), + ) + + t.Views().Commits(). + Press(keys.Universal.RangeSelectUp). + Press(keys.Universal.RangeSelectUp) + + t.Views().Main().ContainsLines( + Contains("Showing diff for range"), + Equals(""), + Equals(" oldFile => newFile | 2 +-"), + Equals(" 1 file changed, 1 insertion(+), 1 deletion(-)"), + Equals(""), + Equals("diff --git a/oldFile b/newFile"), + Equals("similarity index 66%"), + Equals("rename from oldFile"), + Equals("rename to newFile"), + Contains("index"), + Equals("--- a/oldFile"), + Equals("+++ b/newFile"), + Equals("@@ -1,3 +1,3 @@"), + Equals("-a"), + Equals("+y"), + Equals(" b"), + Equals(" c"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 4a8754284..1c5f5d75f 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -235,6 +235,7 @@ var tests = []*components.IntegrationTest{ filter_by_path.CliArg, filter_by_path.KeepSameCommitSelectedOnExit, filter_by_path.SelectFile, + filter_by_path.ShowDiffsForRenamedFile, filter_by_path.TypeFile, interactive_rebase.AdvancedInteractiveRebase, interactive_rebase.AmendCommitWithConflict,