diff --git a/go.mod b/go.mod index ae9990519bb..30ae0e97806 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/spf13/afero v1.9.5 github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad - github.com/stefanhaller/git-todo-parser v0.0.7-0.20240406123903-fd957137b6e2 + github.com/stefanhaller/git-todo-parser v0.0.7-0.20250429125209-dcf39e4641f5 github.com/stretchr/testify v1.10.0 github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 diff --git a/go.sum b/go.sum index 98389ca8175..3c1aaab393d 100644 --- a/go.sum +++ b/go.sum @@ -286,8 +286,8 @@ github.com/spf13/afero v1.9.5 h1:stMpOSZFs//0Lv29HduCmli3GUfpFoF3Y1Q/aXj/wVM= github.com/spf13/afero v1.9.5/go.mod h1:UBogFpq8E9Hx+xc5CNTTEpTnuHVmXDwZcZcE1eb/UhQ= github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad h1:fiWzISvDn0Csy5H0iwgAuJGQTUpVfEMJJd4nRFXogbc= github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad/go.mod h1:qLr4V1qq6nMqFKkMo8ZTx3f+BZEkzsRUY10Xsm2mwU0= -github.com/stefanhaller/git-todo-parser v0.0.7-0.20240406123903-fd957137b6e2 h1:RTNWemd/9z9A5L/AggEP3OdhRz5dXETB/wdAlYF0SuM= -github.com/stefanhaller/git-todo-parser v0.0.7-0.20240406123903-fd957137b6e2/go.mod h1:HFt9hGqMzgQ+gVxMKcvTvGaFz4Y0yYycqqAp2V3wcJY= +github.com/stefanhaller/git-todo-parser v0.0.7-0.20250429125209-dcf39e4641f5 h1:ZCI0NPs0xXd00Ej9lX+wwbHjQDkstJa3kUbX7WCOF8I= +github.com/stefanhaller/git-todo-parser v0.0.7-0.20250429125209-dcf39e4641f5/go.mod h1:HFt9hGqMzgQ+gVxMKcvTvGaFz4Y0yYycqqAp2V3wcJY= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v0.0.0-20161117074351-18a02ba4a312/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= diff --git a/pkg/app/daemon/rebase.go b/pkg/app/daemon/rebase.go index 8cc16d3b1af..30740d09cb9 100644 --- a/pkg/app/daemon/rebase.go +++ b/pkg/app/daemon/rebase.go @@ -22,7 +22,7 @@ func (self *TodoLine) ToString() string { if self.Action == "break" { return self.Action + "\n" } else { - return self.Action + " " + self.Commit.Hash + " " + self.Commit.Name + "\n" + return self.Action + " " + self.Commit.Hash() + " " + self.Commit.Name + "\n" } } diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 96b3d1b1331..6b1dfacdf3b 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -66,6 +66,7 @@ type GetCommitsOptions struct { // If non-empty, show divergence from this ref (left-right log) RefToShowDivergenceFrom string MainBranches *MainBranches + HashPool *utils.StringPool } // GetCommits obtains the commits of the current branch @@ -74,7 +75,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, if opts.IncludeRebaseCommits && opts.FilterPath == "" { var err error - commits, err = self.MergeRebasingCommits(commits) + commits, err = self.MergeRebasingCommits(opts.HashPool, commits) if err != nil { return nil, err } @@ -89,7 +90,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, defer wg.Done() logErr = self.getLogCmd(opts).RunAndProcessLines(func(line string) (bool, error) { - commit := self.extractCommitFromLine(line, opts.RefToShowDivergenceFrom != "") + commit := self.extractCommitFromLine(opts.HashPool, line, opts.RefToShowDivergenceFrom != "") commits = append(commits, commit) return false, nil }) @@ -121,7 +122,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, } for _, commit := range commits { - if commit.Hash == firstPushedCommit { + if commit.Hash() == firstPushedCommit { passedFirstPushedCommit = true } if !commit.IsTODO() { @@ -159,7 +160,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, return commits, nil } -func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*models.Commit, error) { +func (self *CommitLoader) MergeRebasingCommits(hashPool *utils.StringPool, commits []*models.Commit) ([]*models.Commit, error) { // chances are we have as many commits as last time so we'll set the capacity to be the old length result := make([]*models.Commit, 0, len(commits)) for i, commit := range commits { @@ -172,7 +173,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod workingTreeState := self.getWorkingTreeState() addConflictedRebasingCommit := true if workingTreeState.CherryPicking || workingTreeState.Reverting { - sequencerCommits, err := self.getHydratedSequencerCommits(workingTreeState) + sequencerCommits, err := self.getHydratedSequencerCommits(hashPool, workingTreeState) if err != nil { return nil, err } @@ -181,7 +182,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod } if workingTreeState.Rebasing { - rebasingCommits, err := self.getHydratedRebasingCommits(addConflictedRebasingCommit) + rebasingCommits, err := self.getHydratedRebasingCommits(hashPool, addConflictedRebasingCommit) if err != nil { return nil, err } @@ -196,7 +197,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod // then puts them into a commit object // example input: // 8ad01fe32fcc20f07bc6693f87aa4977c327f1e1|10 hours ago|Jesse Duffield| (HEAD -> master, tag: v0.15.2)|refresh commits when adding a tag -func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool) *models.Commit { +func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line string, showDivergence bool) *models.Commit { split := strings.SplitN(line, "\x00", 8) hash := split[0] @@ -234,7 +235,7 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool parents = strings.Split(parentHashes, " ") } - return &models.Commit{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: hash, Name: message, Tags: tags, @@ -244,16 +245,16 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool AuthorEmail: authorEmail, Parents: parents, Divergence: divergence, - } + }) } -func (self *CommitLoader) getHydratedRebasingCommits(addConflictingCommit bool) ([]*models.Commit, error) { +func (self *CommitLoader) getHydratedRebasingCommits(hashPool *utils.StringPool, addConflictingCommit bool) ([]*models.Commit, error) { todoFileHasShortHashes := self.version.IsOlderThan(2, 25, 2) - return self.getHydratedTodoCommits(self.getRebasingCommits(addConflictingCommit), todoFileHasShortHashes) + return self.getHydratedTodoCommits(hashPool, self.getRebasingCommits(hashPool, addConflictingCommit), todoFileHasShortHashes) } -func (self *CommitLoader) getHydratedSequencerCommits(workingTreeState models.WorkingTreeState) ([]*models.Commit, error) { - commits := self.getSequencerCommits() +func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool, workingTreeState models.WorkingTreeState) ([]*models.Commit, error) { + commits := self.getSequencerCommits(hashPool) if len(commits) > 0 { // If we have any commits in .git/sequencer/todo, then the last one of // those is the conflicting one. @@ -262,22 +263,22 @@ func (self *CommitLoader) getHydratedSequencerCommits(workingTreeState models.Wo // For single-commit cherry-picks and reverts, git apparently doesn't // use the sequencer; in that case, CHERRY_PICK_HEAD or REVERT_HEAD is // our conflicting commit, so synthesize it here. - conflicedCommit := self.getConflictedSequencerCommit(workingTreeState) + conflicedCommit := self.getConflictedSequencerCommit(hashPool, workingTreeState) if conflicedCommit != nil { commits = append(commits, conflicedCommit) } } - return self.getHydratedTodoCommits(commits, true) + return self.getHydratedTodoCommits(hashPool, commits, true) } -func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) { +func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) { if len(todoCommits) == 0 { return nil, nil } commitHashes := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) { - return commit.Hash, commit.Hash != "" + return commit.Hash(), commit.Hash() != "" }) // note that we're not filtering these as we do non-rebasing commits just because @@ -292,8 +293,8 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t fullCommits := map[string]*models.Commit{} err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { - commit := self.extractCommitFromLine(line, false) - fullCommits[commit.Hash] = commit + commit := self.extractCommitFromLine(hashPool, line, false) + fullCommits[commit.Hash()] = commit return false, nil }) if err != nil { @@ -315,9 +316,9 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t hydratedCommits := make([]*models.Commit, 0, len(todoCommits)) for _, rebasingCommit := range todoCommits { - if rebasingCommit.Hash == "" { + if rebasingCommit.Hash() == "" { hydratedCommits = append(hydratedCommits, rebasingCommit) - } else if commit := findFullCommit(rebasingCommit.Hash); commit != nil { + } else if commit := findFullCommit(rebasingCommit.Hash()); commit != nil { commit.Action = rebasingCommit.Action commit.Status = rebasingCommit.Status hydratedCommits = append(hydratedCommits, commit) @@ -331,7 +332,7 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t // git-rebase-todo example: // pick ac446ae94ee560bdb8d1d057278657b251aaef17 ac446ae // pick afb893148791a2fbd8091aeb81deba4930c73031 afb8931 -func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*models.Commit { +func (self *CommitLoader) getRebasingCommits(hashPool *utils.StringPool, addConflictingCommit bool) []*models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading git-rebase-todo: %s", err.Error())) @@ -350,7 +351,7 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model // See if the current commit couldn't be applied because it conflicted; if // so, add a fake entry for it if addConflictingCommit { - if conflictedCommit := self.getConflictedCommit(todos); conflictedCommit != nil { + if conflictedCommit := self.getConflictedCommit(hashPool, todos); conflictedCommit != nil { commits = append(commits, conflictedCommit) } } @@ -364,18 +365,18 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model // Command does not have a commit associated, skip continue } - commits = utils.Prepend(commits, &models.Commit{ + commits = utils.Prepend(commits, models.NewCommit(hashPool, models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusRebasing, Action: t.Command, - }) + })) } return commits } -func (self *CommitLoader) getConflictedCommit(todos []todo.Todo) *models.Commit { +func (self *CommitLoader) getConflictedCommit(hashPool *utils.StringPool, todos []todo.Todo) *models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/done")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading rebase-merge/done: %s", err.Error())) @@ -391,10 +392,10 @@ func (self *CommitLoader) getConflictedCommit(todos []todo.Todo) *models.Commit amendFileExists, _ := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/amend")) messageFileExists, _ := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/message")) - return self.getConflictedCommitImpl(todos, doneTodos, amendFileExists, messageFileExists) + return self.getConflictedCommitImpl(hashPool, todos, doneTodos, amendFileExists, messageFileExists) } -func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos []todo.Todo, amendFileExists bool, messageFileExists bool) *models.Commit { +func (self *CommitLoader) getConflictedCommitImpl(hashPool *utils.StringPool, todos []todo.Todo, doneTodos []todo.Todo, amendFileExists bool, messageFileExists bool) *models.Commit { // Should never be possible, but just to be safe: if len(doneTodos) == 0 { self.Log.Error("no done entries in rebase-merge/done file") @@ -465,14 +466,14 @@ func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos [ // Any other todo that has a commit associated with it must have failed with // a conflict, otherwise we wouldn't have stopped the rebase: - return &models.Commit{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: lastTodo.Commit, Action: lastTodo.Command, Status: models.StatusConflicted, - } + }) } -func (self *CommitLoader) getSequencerCommits() []*models.Commit { +func (self *CommitLoader) getSequencerCommits(hashPool *utils.StringPool) []*models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer/todo")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading sequencer/todo: %s", err.Error())) @@ -493,18 +494,18 @@ func (self *CommitLoader) getSequencerCommits() []*models.Commit { // Command does not have a commit associated, skip continue } - commits = utils.Prepend(commits, &models.Commit{ + commits = utils.Prepend(commits, models.NewCommit(hashPool, models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusCherryPickingOrReverting, Action: t.Command, - }) + })) } return commits } -func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.WorkingTreeState) *models.Commit { +func (self *CommitLoader) getConflictedSequencerCommit(hashPool *utils.StringPool, workingTreeState models.WorkingTreeState) *models.Commit { var shaFile string var action todo.TodoCommand if workingTreeState.CherryPicking { @@ -526,11 +527,11 @@ func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.W if len(lines) == 0 { return nil } - return &models.Commit{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: lines[0], Status: models.StatusConflicted, Action: action, - } + }) } func setCommitMergedStatuses(ancestor string, commits []*models.Commit) { @@ -541,7 +542,7 @@ func setCommitMergedStatuses(ancestor string, commits []*models.Commit) { passedAncestor := false for i, commit := range commits { // some commits aren't really commits and don't have hashes, such as the update-ref todo - if commit.Hash != "" && strings.HasPrefix(ancestor, commit.Hash) { + if commit.Hash() != "" && strings.HasPrefix(ancestor, commit.Hash()) { passedAncestor = true } if commit.Status != models.StatusPushed && commit.Status != models.StatusUnpushed { @@ -609,4 +610,4 @@ func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) oscommands.ICmdObj { 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 e3e8c346bec..69d0de0173a 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -10,6 +10,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/stefanhaller/git-todo-parser/todo" "github.com/stretchr/testify/assert" ) @@ -27,13 +28,13 @@ var singleCommitOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18d func TestGetCommits(t *testing.T) { type scenario struct { - testName string - runner *oscommands.FakeCmdObjRunner - expectedCommits []*models.Commit - expectedError error - logOrder string - opts GetCommitsOptions - mainBranches []string + testName string + runner *oscommands.FakeCmdObjRunner + expectedCommitOpts []models.NewCommitOpts + expectedError error + logOrder string + opts GetCommitsOptions + mainBranches []string } scenarios := []scenario{ @@ -43,10 +44,10 @@ 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), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, { testName: "should use proper upstream name for branch", @@ -54,10 +55,10 @@ 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), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, { testName: "should return commits if they are present", @@ -68,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 @@ -79,7 +80,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing where our branch diverged from the master branch so that we can mark that commit and parent commits as 'merged' ExpectGitArgs([]string{"merge-base", "HEAD", "refs/remotes/origin/master", "refs/remotes/origin/main"}, "26c07b1ab33860a1a7591a0638f9925ccf497ffa", nil), - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ { Hash: "0eea75e8c631fba6b58135697835d58ba4c18dbc", Name: "better typing for rebase mode", @@ -204,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")). @@ -213,7 +214,7 @@ func TestGetCommits(t *testing.T) { ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/main"}, "", errors.New("error")). ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/heads/main"}, "", errors.New("error")), - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ { Hash: "0eea75e8c631fba6b58135697835d58ba4c18dbc", Name: "better typing for rebase mode", @@ -240,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")). @@ -251,7 +252,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing where our branch diverged from the master branch so that we can mark that commit and parent commits as 'merged' ExpectGitArgs([]string{"merge-base", "HEAD", "refs/remotes/origin/master", "refs/remotes/origin/develop", "refs/remotes/origin/1.0-hotfixes"}, "26c07b1ab33860a1a7591a0638f9925ccf497ffa", nil), - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ { Hash: "0eea75e8c631fba6b58135697835d58ba4c18dbc", Name: "better typing for rebase mode", @@ -275,10 +276,10 @@ 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), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, { testName: "should set filter path", @@ -286,10 +287,10 @@ 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", "--no-show-signature", "--", "src"}, "", nil), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, } @@ -313,12 +314,17 @@ func TestGetCommits(t *testing.T) { }, } + hashPool := &utils.StringPool{} + common.UserConfig().Git.MainBranches = scenario.mainBranches opts := scenario.opts opts.MainBranches = NewMainBranches(common, cmd) + opts.HashPool = hashPool commits, err := builder.GetCommits(opts) - assert.Equal(t, scenario.expectedCommits, commits) + expectedCommits := lo.Map(scenario.expectedCommitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + assert.Equal(t, expectedCommits, commits) assert.Equal(t, scenario.expectedError, err) scenario.runner.CheckForMissingCalls() @@ -327,6 +333,8 @@ func TestGetCommits(t *testing.T) { } func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { + hashPool := &utils.StringPool{} + scenarios := []struct { testName string todos []todo.Todo @@ -356,11 +364,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: &models.Commit{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, - }, + }), }, { testName: "last command was 'break'", @@ -457,11 +465,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: &models.Commit{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, - }, + }), }, { testName: "'edit' with amend file", @@ -486,11 +494,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, amendFileExists: false, messageFileExists: true, - expectedResult: &models.Commit{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Edit, Status: models.StatusConflicted, - }, + }), }, { testName: "'edit' without amend and without message file", @@ -523,7 +531,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, } - hash := builder.getConflictedCommitImpl(scenario.todos, scenario.doneTodos, scenario.amendFileExists, scenario.messageFileExists) + hash := builder.getConflictedCommitImpl(hashPool, scenario.todos, scenario.doneTodos, scenario.amendFileExists, scenario.messageFileExists) assert.Equal(t, scenario.expectedResult, hash) }) } @@ -531,22 +539,22 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { type scenario struct { - testName string - commits []*models.Commit - ancestor string - expectedCommits []*models.Commit + testName string + commitOpts []models.NewCommitOpts + ancestor string + expectedCommitOpts []models.NewCommitOpts } scenarios := []scenario{ { testName: "basic", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, {Hash: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusPushed}, {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, }, ancestor: "67890", - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, {Hash: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusMerged}, {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusMerged}, @@ -554,13 +562,13 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { }, { testName: "with update-ref", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, {Hash: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone}, {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, }, ancestor: "deadbeef", - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, {Hash: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone}, {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, @@ -570,9 +578,14 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { for _, scenario := range scenarios { t.Run(scenario.testName, func(t *testing.T) { - expectedCommits := scenario.commits - setCommitMergedStatuses(scenario.ancestor, expectedCommits) - assert.Equal(t, scenario.expectedCommits, expectedCommits) + hashPool := &utils.StringPool{} + + commits := lo.Map(scenario.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + setCommitMergedStatuses(scenario.ancestor, commits) + expectedCommits := lo.Map(scenario.expectedCommitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + assert.Equal(t, expectedCommits, commits) }) } } diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index 77563ab88bb..49633cf3b03 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -156,13 +156,13 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s baseIndex := sourceCommitIdx + 1 changes := []daemon.ChangeTodoAction{ - {Hash: commits[sourceCommitIdx].Hash, NewAction: todo.Edit}, - {Hash: commits[destinationCommitIdx].Hash, NewAction: todo.Edit}, + {Hash: commits[sourceCommitIdx].Hash(), NewAction: todo.Edit}, + {Hash: commits[destinationCommitIdx].Hash(), NewAction: todo.Edit}, } self.os.LogCommand(logTodoChanges(changes), false) err := self.rebase.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ - baseHashOrRoot: commits[baseIndex].Hash, + baseHashOrRoot: commits[baseIndex].Hash(), overrideEditor: true, instruction: daemon.NewChangeTodoActionsInstruction(changes), }).Run() @@ -218,7 +218,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitIdx int, stash bool) error { if stash { - if err := self.stash.Push(self.Tr.StashPrefix + commits[commitIdx].Hash); err != nil { + if err := self.stash.Push(self.Tr.StashPrefix + commits[commitIdx].Hash()); err != nil { return err } } @@ -323,7 +323,7 @@ func (self *PatchCommands) diffHeadAgainstCommit(commit *models.Commit) (string, cmdArgs := NewGitCmd("diff"). Config("diff.noprefix=false"). Arg("--no-ext-diff"). - Arg("HEAD.." + commit.Hash). + Arg("HEAD.." + commit.Hash()). ToArgv() return self.cmd.New(cmdArgs).RunWithOutput() diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 188fb72cd56..94891fa1042 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -55,7 +55,7 @@ func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, su func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int) (oscommands.ICmdObj, error) { changes := []daemon.ChangeTodoAction{{ - Hash: commits[index].Hash, + Hash: commits[index].Hash(), NewAction: todo.Reword, }} self.os.LogCommand(logTodoChanges(changes), false) @@ -80,7 +80,7 @@ func (self *RebaseCommands) SetCommitAuthor(commits []*models.Commit, start, end func (self *RebaseCommands) AddCommitCoAuthor(commits []*models.Commit, start, end int, value string) error { return self.GenericAmend(commits, start, end, func(commit *models.Commit) error { - return self.commit.AddCoAuthor(commit.Hash, value) + return self.commit.AddCoAuthor(commit.Hash(), value) }) } @@ -113,7 +113,7 @@ func (self *RebaseCommands) MoveCommitsDown(commits []*models.Commit, startIdx i baseHashOrRoot := getBaseHashOrRoot(commits, endIdx+2) hashes := lo.Map(commits[startIdx:endIdx+1], func(commit *models.Commit, _ int) string { - return commit.Hash + return commit.Hash() }) return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ @@ -127,7 +127,7 @@ func (self *RebaseCommands) MoveCommitsUp(commits []*models.Commit, startIdx int baseHashOrRoot := getBaseHashOrRoot(commits, endIdx+1) hashes := lo.Map(commits[startIdx:endIdx+1], func(commit *models.Commit, _ int) string { - return commit.Hash + return commit.Hash() }) return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ @@ -147,7 +147,7 @@ func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, startIdx changes := lo.FilterMap(commits[startIdx:endIdx+1], func(commit *models.Commit, _ int) (daemon.ChangeTodoAction, bool) { return daemon.ChangeTodoAction{ - Hash: commit.Hash, + Hash: commit.Hash(), NewAction: action, }, !commit.IsMerge() }) @@ -293,7 +293,7 @@ func (self *RebaseCommands) getHashOfLastCommitMade() (string, error) { func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) error { commit := commits[commitIndex] - if err := self.commit.CreateFixupCommit(commit.Hash); err != nil { + if err := self.commit.CreateFixupCommit(commit.Hash()); err != nil { return err } @@ -305,7 +305,7 @@ func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) e return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseHashOrRoot: getBaseHashOrRoot(commits, commitIndex+1), overrideEditor: true, - instruction: daemon.NewMoveFixupCommitDownInstruction(commit.Hash, fixupHash, true), + instruction: daemon.NewMoveFixupCommitDownInstruction(commit.Hash(), fixupHash, true), }).Run() } @@ -318,7 +318,7 @@ func (self *RebaseCommands) MoveFixupCommitDown(commits []*models.Commit, target return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseHashOrRoot: getBaseHashOrRoot(commits, targetCommitIndex+1), overrideEditor: true, - instruction: daemon.NewMoveFixupCommitDownInstruction(commits[targetCommitIndex].Hash, fixupHash, false), + instruction: daemon.NewMoveFixupCommitDownInstruction(commits[targetCommitIndex].Hash(), fixupHash, false), }).Run() } @@ -326,7 +326,7 @@ func todoFromCommit(commit *models.Commit) utils.Todo { if commit.Action == todo.UpdateRef { return utils.Todo{Ref: commit.Name} } else { - return utils.Todo{Hash: commit.Hash} + return utils.Todo{Hash: commit.Hash()} } } @@ -334,7 +334,7 @@ func todoFromCommit(commit *models.Commit) utils.Todo { func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo.TodoCommand) error { commitsWithAction := lo.Map(commits, func(commit *models.Commit, _ int) utils.TodoChange { return utils.TodoChange{ - Hash: commit.Hash, + Hash: commit.Hash(), NewAction: action, } }) @@ -383,7 +383,7 @@ func (self *RebaseCommands) MoveTodosUp(commits []*models.Commit) error { // SquashAllAboveFixupCommits squashes all fixup! commits above the given one func (self *RebaseCommands) SquashAllAboveFixupCommits(commit *models.Commit) error { - hashOrRoot := commit.Hash + "^" + hashOrRoot := commit.Hash() + "^" if commit.IsFirstCommit() { hashOrRoot = "--root" } @@ -420,7 +420,7 @@ func (self *RebaseCommands) BeginInteractiveRebaseForCommitRange( changes := make([]daemon.ChangeTodoAction, 0, end-start) for commitIndex := end; commitIndex >= start; commitIndex-- { changes = append(changes, daemon.ChangeTodoAction{ - Hash: commits[commitIndex].Hash, + Hash: commits[commitIndex].Hash(), NewAction: todo.Edit, }) } @@ -538,7 +538,7 @@ func (self *RebaseCommands) CherryPickCommits(commits []*models.Commit) error { Arg("--allow-empty"). ArgIf(self.version.IsAtLeast(2, 45, 0), "--empty=keep", "--keep-redundant-commits"). ArgIf(hasMergeCommit, "-m1"). - Arg(lo.Reverse(lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash }))...). + Arg(lo.Reverse(lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash() }))...). ToArgv() return self.cmd.New(cmdArgs).Run() @@ -547,7 +547,7 @@ func (self *RebaseCommands) CherryPickCommits(commits []*models.Commit) error { func (self *RebaseCommands) DropMergeCommit(commits []*models.Commit, commitIndex int) error { return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseHashOrRoot: getBaseHashOrRoot(commits, commitIndex+1), - instruction: daemon.NewDropMergeCommitInstruction(commits[commitIndex].Hash), + instruction: daemon.NewDropMergeCommitInstruction(commits[commitIndex].Hash()), }).Run() } @@ -559,7 +559,7 @@ func getBaseHashOrRoot(commits []*models.Commit, index int) string { // be starting a rebase from 300 commits ago (which is the original commit limit // at time of writing) if index < len(commits) { - return commits[index].Hash + return commits[index].Hash() } else { return "--root" } diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index 47d7e970307..feb52397308 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -10,6 +10,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/git_config" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" "github.com/stretchr/testify/assert" ) @@ -97,7 +98,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { type scenario struct { testName string gitConfigMockResponses map[string]string - commits []*models.Commit + commitOpts []models.NewCommitOpts commitIndex int fileName []string runner *oscommands.FakeCmdObjRunner @@ -108,7 +109,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { { testName: "returns error when index outside of range of commits", gitConfigMockResponses: nil, - commits: []*models.Commit{}, + commitOpts: []models.NewCommitOpts{}, commitIndex: 0, fileName: []string{"test999.txt"}, runner: oscommands.NewFakeRunner(t), @@ -119,7 +120,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { { testName: "returns error when using gpg", gitConfigMockResponses: map[string]string{"commit.gpgSign": "true"}, - commits: []*models.Commit{{Name: "commit", Hash: "123456"}}, + commitOpts: []models.NewCommitOpts{{Name: "commit", Hash: "123456"}}, commitIndex: 0, fileName: []string{"test999.txt"}, runner: oscommands.NewFakeRunner(t), @@ -130,7 +131,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { { testName: "checks out file if it already existed", gitConfigMockResponses: nil, - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit", Hash: "123456"}, {Name: "commit2", Hash: "abcdef"}, }, @@ -158,7 +159,11 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses), }) - s.test(instance.DiscardOldFileChanges(s.commits, s.commitIndex, s.fileName)) + hashPool := &utils.StringPool{} + commits := lo.Map(s.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + + s.test(instance.DiscardOldFileChanges(commits, s.commitIndex, s.fileName)) s.runner.CheckForMissingCalls() }) } diff --git a/pkg/commands/git_commands/reflog_commit_loader.go b/pkg/commands/git_commands/reflog_commit_loader.go index 721bb99e71f..1e981b1e126 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -7,6 +7,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" + "github.com/jesseduffield/lazygit/pkg/utils" ) type ReflogCommitLoader struct { @@ -23,14 +24,14 @@ 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(lastReflogCommit *models.Commit, filterPath string, filterAuthor string) ([]*models.Commit, bool, error) { +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). ToArgv() @@ -39,7 +40,7 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit onlyObtainedNewReflogCommits := false err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { - commit, ok := self.parseLine(line) + commit, ok := self.parseLine(hashPool, line) if !ok { return false, nil } @@ -65,10 +66,10 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit } func (self *ReflogCommitLoader) sameReflogCommit(a *models.Commit, b *models.Commit) bool { - return a.Hash == b.Hash && a.UnixTimestamp == b.UnixTimestamp && a.Name == b.Name + return a.Hash() == b.Hash() && a.UnixTimestamp == b.UnixTimestamp && a.Name == b.Name } -func (self *ReflogCommitLoader) parseLine(line string) (*models.Commit, bool) { +func (self *ReflogCommitLoader) parseLine(hashPool *utils.StringPool, line string) (*models.Commit, bool) { fields := strings.SplitN(line, "\x00", 4) if len(fields) <= 3 { return nil, false @@ -82,11 +83,11 @@ func (self *ReflogCommitLoader) parseLine(line string) (*models.Commit, bool) { parents = strings.Split(parentHashes, " ") } - return &models.Commit{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: fields[0], Name: fields[2], UnixTimestamp: int64(unixTimestamp), Status: models.StatusReflog, Parents: parents, - }, true + }), true } diff --git a/pkg/commands/git_commands/reflog_commit_loader_test.go b/pkg/commands/git_commands/reflog_commit_loader_test.go index c9eca3e158e..3a90688a2b5 100644 --- a/pkg/commands/git_commands/reflog_commit_loader_test.go +++ b/pkg/commands/git_commands/reflog_commit_loader_test.go @@ -8,6 +8,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/sanity-io/litter" "github.com/stretchr/testify/assert" ) @@ -26,29 +27,31 @@ func TestGetReflogCommits(t *testing.T) { lastReflogCommit *models.Commit filterPath string filterAuthor string - expectedCommits []*models.Commit + expectedCommitOpts []models.NewCommitOpts expectedOnlyObtainedNew bool expectedError error } + hashPool := &utils.StringPool{} + scenarios := []scenario{ { 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", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, "", nil), lastReflogCommit: nil, - expectedCommits: []*models.Commit{}, + expectedCommitOpts: []models.NewCommitOpts{}, expectedOnlyObtainedNew: false, expectedError: nil, }, { 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", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), lastReflogCommit: nil, - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ { Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", @@ -91,16 +94,16 @@ 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", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), - lastReflogCommit: &models.Commit{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, - }, - expectedCommits: []*models.Commit{ + }), + expectedCommitOpts: []models.NewCommitOpts{ { Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", @@ -115,17 +118,17 @@ 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", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P", "--follow", "--", "path"}, reflogOutput, nil), - lastReflogCommit: &models.Commit{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, - }, + }), filterPath: "path", - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ { Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", @@ -140,17 +143,17 @@ 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", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P", "--author=John Doe "}, reflogOutput, nil), - lastReflogCommit: &models.Commit{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, - }, + }), filterAuthor: "John Doe ", - expectedCommits: []*models.Commit{ + expectedCommitOpts: []models.NewCommitOpts{ { Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", @@ -165,11 +168,11 @@ 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", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, "", errors.New("haha")), lastReflogCommit: nil, filterPath: "", - expectedCommits: nil, + expectedCommitOpts: nil, expectedOnlyObtainedNew: false, expectedError: errors.New("haha"), }, @@ -182,11 +185,16 @@ func TestGetReflogCommits(t *testing.T) { cmd: oscommands.NewDummyCmdObjBuilder(scenario.runner), } - commits, onlyObtainednew, err := builder.GetReflogCommits(scenario.lastReflogCommit, scenario.filterPath, scenario.filterAuthor) + commits, onlyObtainednew, err := builder.GetReflogCommits(hashPool, scenario.lastReflogCommit, scenario.filterPath, scenario.filterAuthor) assert.Equal(t, scenario.expectedOnlyObtainedNew, onlyObtainednew) assert.Equal(t, scenario.expectedError, err) t.Logf("actual commits: \n%s", litter.Sdump(commits)) - assert.Equal(t, scenario.expectedCommits, commits) + var expectedCommits []*models.Commit + if scenario.expectedCommitOpts != nil { + expectedCommits = lo.Map(scenario.expectedCommitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + } + assert.Equal(t, expectedCommits, commits) scenario.runner.CheckForMissingCalls() }) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index 085404d8f75..eca66b3ac2c 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -4,13 +4,14 @@ import ( "fmt" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/stefanhaller/git-todo-parser/todo" ) // Special commit hash for empty tree object const EmptyTreeCommitHash = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" -type CommitStatus int +type CommitStatus uint8 const ( StatusNone CommitStatus = iota @@ -29,7 +30,7 @@ const ( ActionNone todo.TodoCommand = 0 ) -type Divergence int +type Divergence uint8 // For a divergence log (left/right comparison of two refs) this is set to // either DivergenceLeft or DivergenceRight for each commit; for normal @@ -42,35 +43,74 @@ const ( // Commit : A git commit type Commit struct { - Hash string + hash *string Name string - Status CommitStatus - Action todo.TodoCommand Tags []string ExtraInfo string // something like 'HEAD -> master, tag: v0.15.2' AuthorName string // something like 'Jesse Duffield' AuthorEmail string // something like 'jessedduffield@gmail.com' UnixTimestamp int64 - Divergence Divergence // set to DivergenceNone unless we are showing the divergence view // Hashes of parent commits (will be multiple if it's a merge commit) - Parents []string + parents []*string + + Status CommitStatus + Action todo.TodoCommand + Divergence Divergence // set to DivergenceNone unless we are showing the divergence view +} + +type NewCommitOpts struct { + Hash string + Name string + Status CommitStatus + Action todo.TodoCommand + Tags []string + ExtraInfo string + AuthorName string + AuthorEmail string + UnixTimestamp int64 + Divergence Divergence + Parents []string +} + +func NewCommit(hashPool *utils.StringPool, opts NewCommitOpts) *Commit { + return &Commit{ + hash: hashPool.Add(opts.Hash), + Name: opts.Name, + Status: opts.Status, + Action: opts.Action, + Tags: opts.Tags, + ExtraInfo: opts.ExtraInfo, + AuthorName: opts.AuthorName, + AuthorEmail: opts.AuthorEmail, + UnixTimestamp: opts.UnixTimestamp, + Divergence: opts.Divergence, + parents: lo.Map(opts.Parents, func(s string, _ int) *string { return hashPool.Add(s) }), + } +} + +func (c *Commit) Hash() string { + return *c.hash +} + +func (c *Commit) HashPtr() *string { + return c.hash } func (c *Commit) ShortHash() string { - return utils.ShortHash(c.Hash) + return utils.ShortHash(c.Hash()) } func (c *Commit) FullRefName() string { - return c.Hash + return c.Hash() } func (c *Commit) RefName() string { - return c.Hash + return c.Hash() } func (c *Commit) ShortRefName() string { - return c.Hash[:7] + return c.Hash()[:7] } func (c *Commit) ParentRefName() string { @@ -80,8 +120,16 @@ func (c *Commit) ParentRefName() string { return c.RefName() + "^" } +func (c *Commit) Parents() []string { + return lo.Map(c.parents, func(s *string, _ int) string { return *s }) +} + +func (c *Commit) ParentPtrs() []*string { + return c.parents +} + func (c *Commit) IsFirstCommit() bool { - return len(c.Parents) == 0 + return len(c.parents) == 0 } func (c *Commit) ID() string { @@ -89,11 +137,11 @@ func (c *Commit) ID() string { } func (c *Commit) Description() string { - return fmt.Sprintf("%s %s", c.Hash[:7], c.Name) + return fmt.Sprintf("%s %s", c.Hash()[:7], c.Name) } func (c *Commit) IsMerge() bool { - return len(c.Parents) > 1 + return len(c.parents) > 1 } // returns true if this commit is not actually in the git log but instead diff --git a/pkg/config/app_config_test.go b/pkg/config/app_config_test.go index 9f9abc38a94..c9199fdc7e5 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -691,7 +691,7 @@ keybinding: `) func BenchmarkMigrationOnLargeConfiguration(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = computeMigratedConfig("path doesn't matter", largeConfiguration) } } diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 2414501845c..6f92b34fae0 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -32,12 +32,12 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext { ) getDisplayStrings := func(startIdx int, endIdx int) [][]string { - selectedCommitHash := "" + var selectedCommitHashPtr *string if c.Context().Current().GetKey() == LOCAL_COMMITS_CONTEXT_KEY { selectedCommit := viewModel.GetSelected() if selectedCommit != nil { - selectedCommitHash = selectedCommit.Hash + selectedCommitHashPtr = selectedCommit.HashPtr() } } @@ -57,7 +57,7 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext { c.UserConfig().Gui.ShortTimeFormat, time.Now(), c.UserConfig().Git.ParseEmoji, - selectedCommitHash, + selectedCommitHashPtr, startIdx, endIdx, shouldShowGraph(c), @@ -192,7 +192,7 @@ func (self *LocalCommitsContext) GetSelectedCommitHash() string { if commit == nil { return "" } - return commit.Hash + return commit.Hash() } func (self *LocalCommitsContext) SelectCommitByHash(hash string) bool { @@ -200,7 +200,7 @@ func (self *LocalCommitsContext) SelectCommitByHash(hash string) bool { return false } - if _, idx, found := lo.FindIndexOf(self.GetItems(), func(c *models.Commit) bool { return c.Hash == hash }); found { + if _, idx, found := lo.FindIndexOf(self.GetItems(), func(c *models.Commit) bool { return c.Hash() == hash }); found { self.SetSelection(idx) return true } @@ -219,7 +219,7 @@ func (self *LocalCommitsContext) RefForAdjustingLineNumberInDiff() string { if commits == nil { return "" } - return commits[0].Hash + return commits[0].Hash() } func (self *LocalCommitsContext) ModelSearchResults(searchStr string, caseSensitive bool) []gocui.SearchPosition { @@ -284,7 +284,7 @@ func searchModelCommits(caseSensitive bool, commits []*models.Commit, columnPosi // that we render. So we just set the XStart and XEnd values to the // start and end of the commit hash column, which is the second one. result := gocui.SearchPosition{XStart: columnPositions[1], XEnd: columnPositions[2] - 1, Y: idx} - return result, strings.Contains(normalize(commit.Hash), searchStr) || + return result, strings.Contains(normalize(commit.Hash()), searchStr) || strings.Contains(normalize(commit.Name), searchStr) || strings.Contains(normalize(commit.ExtraInfo), searchStr) // allow searching for tags }) diff --git a/pkg/gui/context/sub_commits_context.go b/pkg/gui/context/sub_commits_context.go index 1eed352eb87..650a577c187 100644 --- a/pkg/gui/context/sub_commits_context.go +++ b/pkg/gui/context/sub_commits_context.go @@ -46,11 +46,11 @@ func NewSubCommitsContext( return [][]string{} } - selectedCommitHash := "" + var selectedCommitHashPtr *string if c.Context().Current().GetKey() == SUB_COMMITS_CONTEXT_KEY { selectedCommit := viewModel.GetSelected() if selectedCommit != nil { - selectedCommitHash = selectedCommit.Hash + selectedCommitHashPtr = selectedCommit.HashPtr() } } branches := []*models.Branch{} @@ -72,7 +72,7 @@ func NewSubCommitsContext( c.UserConfig().Gui.ShortTimeFormat, time.Now(), c.UserConfig().Git.ParseEmoji, - selectedCommitHash, + selectedCommitHashPtr, startIdx, endIdx, shouldShowGraph(c), @@ -221,7 +221,7 @@ func (self *SubCommitsContext) RefForAdjustingLineNumberInDiff() string { if commits == nil { return "" } - return commits[0].Hash + return commits[0].Hash() } func (self *SubCommitsContext) ModelSearchResults(searchStr string, caseSensitive bool) []gocui.SearchPosition { diff --git a/pkg/gui/controllers/basic_commits_controller.go b/pkg/gui/controllers/basic_commits_controller.go index 89f0905e50f..97a1ffeacb7 100644 --- a/pkg/gui/controllers/basic_commits_controller.go +++ b/pkg/gui/controllers/basic_commits_controller.go @@ -153,7 +153,7 @@ func (self *BasicCommitsController) getCommitMessageBody(hash string) string { } func (self *BasicCommitsController) copyCommitAttribute(commit *models.Commit) error { - commitMessageBody := self.getCommitMessageBody(commit.Hash) + commitMessageBody := self.getCommitMessageBody(commit.Hash()) var commitMessageBodyDisabled *types.DisabledReason if commitMessageBody == "" { commitMessageBodyDisabled = &types.DisabledReason{ @@ -235,16 +235,16 @@ func (self *BasicCommitsController) copyCommitAttribute(commit *models.Commit) e func (self *BasicCommitsController) copyCommitHashToClipboard(commit *models.Commit) error { self.c.LogAction(self.c.Tr.Actions.CopyCommitHashToClipboard) - if err := self.c.OS().CopyToClipboard(commit.Hash); err != nil { + if err := self.c.OS().CopyToClipboard(commit.Hash()); err != nil { return err } - self.c.Toast(fmt.Sprintf("'%s' %s", commit.Hash, self.c.Tr.CopiedToClipboard)) + self.c.Toast(fmt.Sprintf("'%s' %s", commit.Hash(), self.c.Tr.CopiedToClipboard)) return nil } func (self *BasicCommitsController) copyCommitURLToClipboard(commit *models.Commit) error { - url, err := self.c.Helpers().Host.GetCommitURL(commit.Hash) + url, err := self.c.Helpers().Host.GetCommitURL(commit.Hash()) if err != nil { return err } @@ -259,7 +259,7 @@ func (self *BasicCommitsController) copyCommitURLToClipboard(commit *models.Comm } func (self *BasicCommitsController) copyCommitDiffToClipboard(commit *models.Commit) error { - diff, err := self.c.Git().Commit.GetCommitDiff(commit.Hash) + diff, err := self.c.Git().Commit.GetCommitDiff(commit.Hash()) if err != nil { return err } @@ -274,7 +274,7 @@ func (self *BasicCommitsController) copyCommitDiffToClipboard(commit *models.Com } func (self *BasicCommitsController) copyAuthorToClipboard(commit *models.Commit) error { - author, err := self.c.Git().Commit.GetCommitAuthor(commit.Hash) + author, err := self.c.Git().Commit.GetCommitAuthor(commit.Hash()) if err != nil { return err } @@ -291,7 +291,7 @@ func (self *BasicCommitsController) copyAuthorToClipboard(commit *models.Commit) } func (self *BasicCommitsController) copyCommitMessageToClipboard(commit *models.Commit) error { - message, err := self.c.Git().Commit.GetCommitMessage(commit.Hash) + message, err := self.c.Git().Commit.GetCommitMessage(commit.Hash()) if err != nil { return err } @@ -316,7 +316,7 @@ func (self *BasicCommitsController) copyCommitMessageBodyToClipboard(commitMessa } func (self *BasicCommitsController) copyCommitSubjectToClipboard(commit *models.Commit) error { - message, err := self.c.Git().Commit.GetCommitSubject(commit.Hash) + message, err := self.c.Git().Commit.GetCommitSubject(commit.Hash()) if err != nil { return err } @@ -343,7 +343,7 @@ func (self *BasicCommitsController) copyCommitTagsToClipboard(commit *models.Com } func (self *BasicCommitsController) openInBrowser(commit *models.Commit) error { - url, err := self.c.Helpers().Host.GetCommitURL(commit.Hash) + url, err := self.c.Helpers().Host.GetCommitURL(commit.Hash()) if err != nil { return err } @@ -361,7 +361,7 @@ func (self *BasicCommitsController) newBranch(commit *models.Commit) error { } func (self *BasicCommitsController) createResetMenu(commit *models.Commit) error { - return self.c.Helpers().Refs.CreateGitResetMenu(commit.Hash) + return self.c.Helpers().Refs.CreateGitResetMenu(commit.Hash()) } func (self *BasicCommitsController) checkout(commit *models.Commit) error { @@ -374,7 +374,7 @@ func (self *BasicCommitsController) copyRange(*models.Commit) error { func (self *BasicCommitsController) canCopyCommits(selectedCommits []*models.Commit, startIdx int, endIdx int) *types.DisabledReason { for _, commit := range selectedCommits { - if commit.Hash == "" { + if commit.Hash() == "" { return &types.DisabledReason{Text: self.c.Tr.CannotCherryPickNonCommit, ShowErrorInPanel: true} } } diff --git a/pkg/gui/controllers/bisect_controller.go b/pkg/gui/controllers/bisect_controller.go index 892493d7b7e..0cd8d1d0b79 100644 --- a/pkg/gui/controllers/bisect_controller.go +++ b/pkg/gui/controllers/bisect_controller.go @@ -69,7 +69,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c // Originally we were allowing the user to, from the bisect menu, select whether // they were talking about the selected commit or the current bisect commit, // and that was a bit confusing (and required extra keypresses). - selectCurrentAfter := info.GetCurrentHash() == "" || info.GetCurrentHash() == commit.Hash + selectCurrentAfter := info.GetCurrentHash() == "" || info.GetCurrentHash() == commit.Hash() // we need to wait to reselect if our bisect commits aren't ancestors of our 'start' // ref, because we'll be reloading our commits in that case. waitToReselect := selectCurrentAfter && !self.c.Git().Bisect.ReachableFromStart(info) @@ -79,7 +79,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c // use the selected commit in that case. bisecting := info.GetCurrentHash() != "" - hashToMark := lo.Ternary(bisecting, info.GetCurrentHash(), commit.Hash) + hashToMark := lo.Ternary(bisecting, info.GetCurrentHash(), commit.Hash()) shortHashToMark := utils.ShortHash(hashToMark) // For marking a commit as bad, when we're not already bisecting, we require @@ -131,12 +131,12 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c Key: 's', }, } - if info.GetCurrentHash() != "" && info.GetCurrentHash() != commit.Hash { + if info.GetCurrentHash() != "" && info.GetCurrentHash() != commit.Hash() { menuItems = append(menuItems, lo.ToPtr(types.MenuItem{ Label: fmt.Sprintf(self.c.Tr.Bisect.SkipSelected, commit.ShortHash()), OnPress: func() error { self.c.LogAction(self.c.Tr.Actions.BisectSkip) - if err := self.c.Git().Bisect.Skip(commit.Hash); err != nil { + if err := self.c.Git().Bisect.Skip(commit.Hash()); err != nil { return err } @@ -172,7 +172,7 @@ func (self *BisectController) openStartBisectMenu(info *git_commands.BisectInfo, return err } - if err := self.c.Git().Bisect.Mark(commit.Hash, info.NewTerm()); err != nil { + if err := self.c.Git().Bisect.Mark(commit.Hash(), info.NewTerm()); err != nil { return err } @@ -189,7 +189,7 @@ func (self *BisectController) openStartBisectMenu(info *git_commands.BisectInfo, return err } - if err := self.c.Git().Bisect.Mark(commit.Hash, info.OldTerm()); err != nil { + if err := self.c.Git().Bisect.Mark(commit.Hash(), info.OldTerm()); err != nil { return err } @@ -292,7 +292,7 @@ func (self *BisectController) selectCurrentBisectCommit() { if info.GetCurrentHash() != "" { // find index of commit with that hash, move cursor to that. for i, commit := range self.c.Model().Commits { - if commit.Hash == info.GetCurrentHash() { + if commit.Hash() == info.GetCurrentHash() { self.context().SetSelection(i) self.context().HandleFocus(types.OnFocusOpts{}) break diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index f231152db79..ae21f467cdb 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -67,7 +67,7 @@ func (self *CustomPatchOptionsMenuAction) Call() error { if self.c.Context().Current().GetKey() == self.c.Contexts().LocalCommits.GetKey() { selectedCommit := self.c.Contexts().LocalCommits.GetSelected() - if selectedCommit != nil && self.c.Git().Patch.PatchBuilder.To != selectedCommit.Hash { + if selectedCommit != nil && self.c.Git().Patch.PatchBuilder.To != selectedCommit.Hash() { var disabledReason *types.DisabledReason if self.c.Contexts().LocalCommits.AreMultipleItemsSelected() { @@ -80,7 +80,7 @@ func (self *CustomPatchOptionsMenuAction) Call() error { append( []*types.MenuItem{ { - Label: fmt.Sprintf(self.c.Tr.MovePatchToSelectedCommit, selectedCommit.Hash), + Label: fmt.Sprintf(self.c.Tr.MovePatchToSelectedCommit, selectedCommit.Hash()), Tooltip: self.c.Tr.MovePatchToSelectedCommitTooltip, OnPress: self.handleMovePatchToSelectedCommit, Key: 'm', @@ -106,7 +106,7 @@ func (self *CustomPatchOptionsMenuAction) Call() error { func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() int { for index, commit := range self.c.Model().Commits { - if commit.Hash == self.c.Git().Patch.PatchBuilder.To { + if commit.Hash() == self.c.Git().Patch.PatchBuilder.To { return index } } diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index 6b13b0fda74..5b96dc84c07 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -44,7 +44,7 @@ func (self *CherryPickHelper) CopyRange(commitsList []*models.Commit, context ty commitSet := self.getData().SelectedHashSet() allCommitsCopied := lo.EveryBy(commitsList[startIdx:endIdx+1], func(commit *models.Commit) bool { - return commitSet.Includes(commit.Hash) + return commitSet.Includes(commit.Hash()) }) // if all selected commits are already copied, we'll uncopy them diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index 7035849d4ff..26a2cf64c76 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -66,7 +66,7 @@ func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff(commit *models.Comm return task } - cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash, self.c.Modes().Filtering.GetPath()) + cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.c.Modes().Filtering.GetPath()) return types.NewRunPtyTask(cmdObj.GetCmd()) } diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index 2c320ef53ba..c5cdbcf7eb6 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -342,6 +342,6 @@ func (self *FixupHelper) blameAddedLines(commits []*models.Commit, addedLineHunk func (self *FixupHelper) findCommit(commits []*models.Commit, hash string) (*models.Commit, int, bool) { return lo.FindIndexOf(commits, func(commit *models.Commit) bool { - return commit.Hash == hash + return commit.Hash() == hash }) } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 93ad110f304..64bed07135d 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -332,6 +332,7 @@ func (self *RefreshHelper) refreshCommitsWithLimit() error { RefForPushedStatus: checkedOutBranchName, All: self.c.Contexts().LocalCommits.GetShowWholeGitGraph(), MainBranches: self.c.Model().MainBranches, + HashPool: self.c.Model().HashPool, }, ) if err != nil { @@ -360,6 +361,7 @@ func (self *RefreshHelper) refreshSubCommitsWithLimit() error { RefToShowDivergenceFrom: self.c.Contexts().SubCommits.GetRefToShowDivergenceFrom(), RefForPushedStatus: self.c.Contexts().SubCommits.GetRef().FullRefName(), MainBranches: self.c.Model().MainBranches, + HashPool: self.c.Model().HashPool, }, ) if err != nil { @@ -406,7 +408,7 @@ func (self *RefreshHelper) refreshRebaseCommits() error { self.c.Mutexes().LocalCommitsMutex.Lock() defer self.c.Mutexes().LocalCommitsMutex.Unlock() - updatedCommits, err := self.c.Git().Loaders.CommitLoader.MergeRebasingCommits(self.c.Model().Commits) + updatedCommits, err := self.c.Git().Loaders.CommitLoader.MergeRebasingCommits(self.c.Model().HashPool, self.c.Model().Commits) if err != nil { return err } @@ -455,7 +457,7 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele // 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(nil, "", "") + reflogCommits, _, err = self.c.Git().Loaders.ReflogCommitLoader.GetReflogCommits(self.c.Model().HashPool, nil, "", "") if err != nil { self.c.Log.Error(err) } @@ -624,7 +626,7 @@ func (self *RefreshHelper) refreshReflogCommits() error { refresh := func(stateCommits *[]*models.Commit, filterPath string, filterAuthor string) error { commits, onlyObtainedNewReflogCommits, err := self.c.Git().Loaders.ReflogCommitLoader. - GetReflogCommits(lastReflogCommit, filterPath, filterAuthor) + GetReflogCommits(self.c.Model().HashPool, lastReflogCommit, filterPath, filterAuthor) if err != nil { return err } diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index 9cc10983be3..808b7866f77 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -268,10 +268,10 @@ func (self *RefsHelper) CreateGitResetMenu(ref string) error { func (self *RefsHelper) CreateCheckoutMenu(commit *models.Commit) error { branches := lo.Filter(self.c.Model().Branches, func(branch *models.Branch, _ int) bool { - return commit.Hash == branch.CommitHash && branch.Name != self.c.Model().CheckedOutBranch + return commit.Hash() == branch.CommitHash && branch.Name != self.c.Model().CheckedOutBranch }) - hash := commit.Hash + hash := commit.Hash() menuItems := []*types.MenuItem{ { diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index 3e8f88b7d55..cffd8e8ed5c 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/pkg/gui/controllers/helpers/sub_commits_helper.go @@ -45,6 +45,7 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error { RefForPushedStatus: opts.Ref.FullRefName(), RefToShowDivergenceFrom: opts.RefToShowDivergenceFrom, MainBranches: self.c.Model().MainBranches, + HashPool: self.c.Model().HashPool, }, ) if err != nil { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 16cdab3ecc8..ad844f90e4d 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -359,7 +359,7 @@ func (self *LocalCommitsController) fixup(selectedCommits []*models.Commit, star } func (self *LocalCommitsController) reword(commit *models.Commit) error { - commitMessage, err := self.c.Git().Commit.GetCommitMessage(commit.Hash) + commitMessage, err := self.c.Git().Commit.GetCommitMessage(commit.Hash()) if err != nil { return err } @@ -556,7 +556,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.EditCommit) selectionRangeAndMode := self.getSelectionRangeAndMode() - err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash) + err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash()) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( err, types.RefreshOptions{Mode: types.BLOCK_UI, Then: func() error { @@ -564,7 +564,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( for _, c := range commitsToEdit[:len(commitsToEdit)-1] { // Merge commits can't be set to "edit", so just skip them if !c.IsMerge() { - todos = append(todos, &models.Commit{Hash: c.Hash, Action: todo.Pick}) + todos = append(todos, models.NewCommit(self.c.Model().HashPool, models.NewCommitOpts{Hash: c.Hash(), Action: todo.Pick})) } } if len(todos) > 0 { @@ -589,8 +589,8 @@ type SelectionRangeAndMode struct { func (self *LocalCommitsController) getSelectionRangeAndMode() SelectionRangeAndMode { selectedIdx, rangeStartIdx, rangeSelectMode := self.context().GetSelectionRangeAndMode() commits := self.c.Model().Commits - selectedHash := commits[selectedIdx].Hash - rangeStartHash := commits[rangeStartIdx].Hash + selectedHash := commits[selectedIdx].Hash() + rangeStartHash := commits[rangeStartIdx].Hash() return SelectionRangeAndMode{selectedHash, rangeStartHash, rangeSelectMode} } @@ -599,10 +599,10 @@ func (self *LocalCommitsController) restoreSelectionRangeAndMode(selectionRangeA // new lines can be added for update-ref commands in the TODO file, due to // stacked branches. So the selected commits may be in different positions in the list. _, newSelectedIdx, ok1 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool { - return c.Hash == selectionRangeAndMode.selectedHash + return c.Hash() == selectionRangeAndMode.selectedHash }) _, newRangeStartIdx, ok2 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool { - return c.Hash == selectionRangeAndMode.rangeStartHash + return c.Hash() == selectionRangeAndMode.rangeStartHash }) if ok1 && ok2 { self.context().SetSelectionRangeAndMode(newSelectedIdx, newRangeStartIdx, selectionRangeAndMode.mode) @@ -868,7 +868,7 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end } else { promptText = self.c.Tr.ConfirmRevertCommitRange } - hashes := lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash }) + hashes := lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash() }) isMerge := lo.SomeBy(commits, func(c *models.Commit) bool { return c.IsMerge() }) self.c.Confirm(types.ConfirmOpts{ @@ -911,7 +911,7 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err return self.c.Helpers().WorkingTree.WithEnsureCommittableFiles(func() error { self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit) return self.c.WithWaitingStatusSync(self.c.Tr.CreatingFixupCommitStatus, func() error { - if err := self.c.Git().Commit.CreateFixupCommit(commit.Hash); err != nil { + if err := self.c.Git().Commit.CreateFixupCommit(commit.Hash()); err != nil { return err } @@ -978,7 +978,7 @@ func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCo headOfOwnerBranchIdx := -1 for i := self.context().GetSelectedLineIdx(); i > 0; i-- { if lo.SomeBy(self.c.Model().Branches, func(b *models.Branch) bool { - return b.CommitHash == self.c.Model().Commits[i].Hash + return b.CommitHash == self.c.Model().Commits[i].Hash() }) { headOfOwnerBranchIdx = i break @@ -993,7 +993,7 @@ func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCo } func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, includeFileChanges bool) error { - commitMessage, err := self.c.Git().Commit.GetCommitMessage(commit.Hash) + commitMessage, err := self.c.Git().Commit.GetCommitMessage(commit.Hash()) if err != nil { return err } @@ -1139,7 +1139,7 @@ func isFixupCommit(subject string) (string, bool) { } func (self *LocalCommitsController) createTag(commit *models.Commit) error { - return self.c.Helpers().Tags.OpenCreateTagPrompt(commit.Hash, func() {}) + return self.c.Helpers().Tags.OpenCreateTagPrompt(commit.Hash(), func() {}) } func (self *LocalCommitsController) openSearch() error { @@ -1304,11 +1304,11 @@ func (self *LocalCommitsController) canPaste() *types.DisabledReason { } func (self *LocalCommitsController) markAsBaseCommit(commit *models.Commit) error { - if commit.Hash == self.c.Modes().MarkedBaseCommit.GetHash() { + if commit.Hash() == self.c.Modes().MarkedBaseCommit.GetHash() { // Reset when invoking it again on the marked commit self.c.Modes().MarkedBaseCommit.SetHash("") } else { - self.c.Modes().MarkedBaseCommit.SetHash(commit.Hash) + self.c.Modes().MarkedBaseCommit.SetHash(commit.Hash()) } self.c.PostRefreshUpdate(self.c.Contexts().LocalCommits) return nil diff --git a/pkg/gui/controllers/reflog_commits_controller.go b/pkg/gui/controllers/reflog_commits_controller.go index 4596ad080eb..ea35eca442d 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.Modes().Filtering.GetPath()) task = types.NewRunPtyTask(cmdObj.GetCmd()) } diff --git a/pkg/gui/controllers/undo_controller.go b/pkg/gui/controllers/undo_controller.go index 9922f73d6b1..74adf7067b9 100644 --- a/pkg/gui/controllers/undo_controller.go +++ b/pkg/gui/controllers/undo_controller.go @@ -207,7 +207,7 @@ func (self *UndoController) parseReflogForActions(onUserAction func(counter int, prevCommitHash := "" if len(reflogCommits)-1 >= reflogCommitIdx+1 { - prevCommitHash = reflogCommits[reflogCommitIdx+1].Hash + prevCommitHash = reflogCommits[reflogCommitIdx+1].Hash() } if rebaseFinishCommitHash == "" { @@ -216,11 +216,11 @@ func (self *UndoController) parseReflogForActions(onUserAction func(counter int, } else if ok, _ := utils.FindStringSubmatch(reflogCommit.Name, `^\[lazygit redo\]`); ok { counter-- } else if ok, _ := utils.FindStringSubmatch(reflogCommit.Name, `^rebase (-i )?\(abort\)|^rebase (-i )?\(finish\)`); ok { - rebaseFinishCommitHash = reflogCommit.Hash + rebaseFinishCommitHash = reflogCommit.Hash() } else if ok, match := utils.FindStringSubmatch(reflogCommit.Name, `^checkout: moving from ([\S]+) to ([\S]+)`); ok { action = &reflogAction{kind: CHECKOUT, from: match[1], to: match[2]} } else if ok, _ := utils.FindStringSubmatch(reflogCommit.Name, `^commit|^reset: moving to|^pull`); ok { - action = &reflogAction{kind: COMMIT, from: prevCommitHash, to: reflogCommit.Hash} + action = &reflogAction{kind: COMMIT, from: prevCommitHash, to: reflogCommit.Hash()} } else if ok, _ := utils.FindStringSubmatch(reflogCommit.Name, `^rebase (-i )?\(start\)`); ok { // if we're here then we must be currently inside an interactive rebase action = &reflogAction{kind: CURRENT_REBASE, from: prevCommitHash} diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 90a2a64d48f..54c14c0c54f 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -558,6 +558,7 @@ func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context { FilesTrie: patricia.NewTrie(), Authors: map[string]*models.Author{}, MainBranches: git_commands.NewMainBranches(gui.c.Common, gui.os.Cmd), + HashPool: &utils.StringPool{}, }, Modes: &types.Modes{ Filtering: filtering.New(startArgs.FilterPath, ""), diff --git a/pkg/gui/modes/cherrypicking/cherry_picking.go b/pkg/gui/modes/cherrypicking/cherry_picking.go index 82dae33230b..2ad3e61aaf8 100644 --- a/pkg/gui/modes/cherrypicking/cherry_picking.go +++ b/pkg/gui/modes/cherrypicking/cherry_picking.go @@ -39,27 +39,27 @@ func (self *CherryPicking) SelectedHashSet() *set.Set[string] { } hashes := lo.Map(self.CherryPickedCommits, func(commit *models.Commit, _ int) string { - return commit.Hash + return commit.Hash() }) return set.NewFromSlice(hashes) } func (self *CherryPicking) Add(selectedCommit *models.Commit, commitsList []*models.Commit) { commitSet := self.SelectedHashSet() - commitSet.Add(selectedCommit.Hash) + commitSet.Add(selectedCommit.Hash()) self.update(commitSet, commitsList) } func (self *CherryPicking) Remove(selectedCommit *models.Commit, commitsList []*models.Commit) { commitSet := self.SelectedHashSet() - commitSet.Remove(selectedCommit.Hash) + commitSet.Remove(selectedCommit.Hash()) self.update(commitSet, commitsList) } func (self *CherryPicking) update(selectedHashSet *set.Set[string], commitsList []*models.Commit) { self.CherryPickedCommits = lo.Filter(commitsList, func(commit *models.Commit, _ int) bool { - return selectedHashSet.Includes(commit.Hash) + return selectedHashSet.Includes(commit.Hash()) }) } diff --git a/pkg/gui/presentation/authors/authors.go b/pkg/gui/presentation/authors/authors.go index 28c375f7522..5b939d76dec 100644 --- a/pkg/gui/presentation/authors/authors.go +++ b/pkg/gui/presentation/authors/authors.go @@ -21,7 +21,7 @@ type authorNameCacheKey struct { var ( authorInitialCache = make(map[string]string) authorNameCache = make(map[authorNameCacheKey]string) - authorStyleCache = make(map[string]style.TextStyle) + authorStyleCache = make(map[string]*style.TextStyle) ) const authorNameWildcard = "*" @@ -73,7 +73,7 @@ func AuthorWithLength(authorName string, length int) string { return LongAuthor(authorName, length) } -func AuthorStyle(authorName string) style.TextStyle { +func AuthorStyle(authorName string) *style.TextStyle { if value, ok := authorStyleCache[authorName]; ok { return value } @@ -85,9 +85,9 @@ func AuthorStyle(authorName string) style.TextStyle { value := trueColorStyle(authorName) - authorStyleCache[authorName] = value + authorStyleCache[authorName] = &value - return value + return &value } func trueColorStyle(str string) style.TextStyle { diff --git a/pkg/gui/presentation/branches.go b/pkg/gui/presentation/branches.go index dfb363bc700..1e4e9dc3511 100644 --- a/pkg/gui/presentation/branches.go +++ b/pkg/gui/presentation/branches.go @@ -20,7 +20,7 @@ import ( ) type colorMatcher struct { - patterns map[string]style.TextStyle + patterns map[string]*style.TextStyle isRegex bool // NOTE: this value is needed only until the deprecated branchColors config is removed and only regex color patterns are used } @@ -142,14 +142,14 @@ func (m *colorMatcher) match(name string) (*style.TextStyle, bool) { if m.isRegex { for pattern, style := range m.patterns { if matched, _ := regexp.MatchString(pattern, name); matched { - return &style, true + return style, true } } } else { // old behavior using the deprecated branchColors behavior matching on branch type branchType := strings.Split(name, "/")[0] if value, ok := m.patterns[branchType]; ok { - return &value, true + return value, true } } diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 116ad5c5018..63c2b3c9b54 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -28,7 +28,7 @@ type pipeSetCacheKey struct { } var ( - pipeSetCache = make(map[pipeSetCacheKey][][]*graph.Pipe) + pipeSetCache = make(map[pipeSetCacheKey][][]graph.Pipe) mutex deadlock.Mutex ) @@ -51,7 +51,7 @@ func GetCommitListDisplayStrings( shortTimeFormat string, now time.Time, parseEmoji bool, - selectedCommitHash string, + selectedCommitHashPtr *string, startIdx int, endIdx int, showGraph bool, @@ -102,7 +102,7 @@ func GetCommitListDisplayStrings( graphLines := graph.RenderAux( graphPipeSets, graphCommits, - selectedCommitHash, + selectedCommitHashPtr, ) allGraphLines = append(allGraphLines, graphLines...) } @@ -119,7 +119,7 @@ func GetCommitListDisplayStrings( graphLines := graph.RenderAux( graphPipeSets, graphCommits, - selectedCommitHash, + selectedCommitHashPtr, ) allGraphLines = append(allGraphLines, graphLines...) } @@ -140,7 +140,7 @@ func GetCommitListDisplayStrings( graphLines := graph.RenderAux( graphPipeSets, graphCommits, - selectedCommitHash, + selectedCommitHashPtr, ) getGraphLine = func(idx int) string { if idx >= graphOffset { @@ -175,7 +175,7 @@ func GetCommitListDisplayStrings( !lo.Contains(common.UserConfig().Git.MainBranches, b.Name) && // Don't show a marker for the head commit unless the // rebase.updateRefs config is on - (hasRebaseUpdateRefsConfig || b.CommitHash != commits[0].Hash) + (hasRebaseUpdateRefsConfig || b.CommitHash != commits[0].Hash()) })) lines := make([][]string, 0, len(filteredCommits)) @@ -183,8 +183,8 @@ func GetCommitListDisplayStrings( willBeRebased := markedBaseCommit == "" for i, commit := range filteredCommits { unfilteredIdx := i + startIdx - bisectStatus = getBisectStatus(unfilteredIdx, commit.Hash, bisectInfo, bisectBounds) - isMarkedBaseCommit := commit.Hash != "" && commit.Hash == markedBaseCommit + bisectStatus = getBisectStatus(unfilteredIdx, commit.Hash(), bisectInfo, bisectBounds) + isMarkedBaseCommit := commit.Hash() != "" && commit.Hash() == markedBaseCommit if isMarkedBaseCommit { willBeRebased = true } @@ -218,11 +218,11 @@ func getbisectBounds(commits []*models.Commit, bisectInfo *git_commands.BisectIn bisectBounds := &bisectBounds{} for i, commit := range commits { - if commit.Hash == bisectInfo.GetNewHash() { + if commit.Hash() == bisectInfo.GetNewHash() { bisectBounds.newIndex = i } - status, ok := bisectInfo.Status(commit.Hash) + status, ok := bisectInfo.Status(commit.Hash()) if ok && status == git_commands.BisectStatusOld { bisectBounds.oldIndex = i return bisectBounds @@ -245,11 +245,11 @@ func indexOfFirstNonTODOCommit(commits []*models.Commit) int { return 0 } -func loadPipesets(commits []*models.Commit) [][]*graph.Pipe { +func loadPipesets(commits []*models.Commit) [][]graph.Pipe { // given that our cache key is a commit hash and a commit count, it's very important that we don't actually try to render pipes // when dealing with things like filtered commits. cacheKey := pipeSetCacheKey{ - commitHash: commits[0].Hash, + commitHash: commits[0].Hash(), commitCount: len(commits), divergence: commits[0].Divergence, } @@ -258,7 +258,7 @@ func loadPipesets(commits []*models.Commit) [][]*graph.Pipe { if !ok { // pipe sets are unique to a commit head. and a commit count. Sometimes we haven't loaded everything for that. // so let's just cache it based on that. - getStyle := func(commit *models.Commit) style.TextStyle { + getStyle := func(commit *models.Commit) *style.TextStyle { return authors.AuthorStyle(commit.AuthorName) } pipeSets = graph.GetPipeSets(commits, getStyle) @@ -363,10 +363,10 @@ func displayCommit( hashString := "" hashColor := getHashColor(commit, diffName, cherryPickedCommitHashSet, bisectStatus, bisectInfo) hashLength := common.UserConfig().Gui.CommitHashLength - if hashLength >= len(commit.Hash) { - hashString = hashColor.Sprint(commit.Hash) + if hashLength >= len(commit.Hash()) { + hashString = hashColor.Sprint(commit.Hash()) } else if hashLength > 0 { - hashString = hashColor.Sprint(commit.Hash[:hashLength]) + hashString = hashColor.Sprint(commit.Hash()[:hashLength]) } else if !icons.IsIconEnabled() { // hashLength <= 0 hashString = hashColor.Sprint("*") } @@ -400,7 +400,7 @@ func displayCommit( tagString = theme.DiffTerminalColor.SetBold().Sprint(strings.Join(commit.Tags, " ")) + " " } - if branchHeadsToVisualize.Includes(commit.Hash) && + if branchHeadsToVisualize.Includes(commit.Hash()) && // Don't show branch head on commits that are already merged to a main branch commit.Status != models.StatusMerged && // Don't show branch head on a "pick" todo if the rebase.updateRefs config is on @@ -482,7 +482,7 @@ func getHashColor( return getBisectStatusColor(bisectStatus) } - diffed := commit.Hash != "" && commit.Hash == diffName + diffed := commit.Hash() != "" && commit.Hash() == diffName hashColor := theme.DefaultTextColor switch commit.Status { case models.StatusUnpushed: @@ -500,7 +500,7 @@ func getHashColor( if diffed { hashColor = theme.DiffTerminalColor - } else if cherryPickedCommitHashSet.Includes(commit.Hash) { + } else if cherryPickedCommitHashSet.Includes(commit.Hash()) { hashColor = theme.CherryPickedCommitTextStyle } else if commit.Divergence == models.DivergenceRight && commit.Status != models.StatusMerged { hashColor = style.FgBlue diff --git a/pkg/gui/presentation/commits_test.go b/pkg/gui/presentation/commits_test.go index 08bd3a19c06..595c3d75071 100644 --- a/pkg/gui/presentation/commits_test.go +++ b/pkg/gui/presentation/commits_test.go @@ -11,6 +11,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/stefanhaller/git-todo-parser/todo" "github.com/stretchr/testify/assert" "github.com/xo/terminfo" @@ -23,7 +24,7 @@ func formatExpected(expected string) string { func TestGetCommitListDisplayStrings(t *testing.T) { scenarios := []struct { testName string - commits []*models.Commit + commitOpts []models.NewCommitOpts branches []*models.Branch currentBranchName string hasUpdateRefConfig bool @@ -35,7 +36,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { shortTimeFormat string now time.Time parseEmoji bool - selectedCommitHash string + selectedCommitHashPtr *string startIdx int endIdx int showGraph bool @@ -45,7 +46,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }{ { testName: "no commits", - commits: []*models.Commit{}, + commitOpts: []models.NewCommitOpts{}, startIdx: 0, endIdx: 1, showGraph: false, @@ -56,7 +57,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "some commits", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1"}, {Name: "commit2", Hash: "hash2"}, }, @@ -73,7 +74,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "commit with tags", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Tags: []string{"tag1", "tag2"}}, {Name: "commit2", Hash: "hash2"}, }, @@ -90,7 +91,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "show local branch head, except the current branch, main branches, or merged branches", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1"}, {Name: "commit2", Hash: "hash2"}, {Name: "commit3", Hash: "hash3"}, @@ -119,7 +120,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "show local branch head for head commit if updateRefs is on", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1"}, {Name: "commit2", Hash: "hash2"}, }, @@ -142,7 +143,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "don't show local branch head for head commit if updateRefs is off", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1"}, {Name: "commit2", Hash: "hash2"}, }, @@ -165,7 +166,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "show local branch head and tag if both exist", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1"}, {Name: "commit2", Hash: "hash2", Tags: []string{"some-tag"}}, {Name: "commit3", Hash: "hash3"}, @@ -187,7 +188,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "showing graph", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Parents: []string{"hash2", "hash3"}}, {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}}, {Name: "commit3", Hash: "hash3", Parents: []string{"hash4"}}, @@ -210,7 +211,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "showing graph, including rebase commits", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Parents: []string{"hash2", "hash3"}, Action: todo.Pick}, {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}, Action: todo.Pick}, {Name: "commit3", Hash: "hash3", Parents: []string{"hash4"}}, @@ -233,7 +234,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "showing graph, including rebase commits, with offset", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Parents: []string{"hash2", "hash3"}, Action: todo.Pick}, {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}, Action: todo.Pick}, {Name: "commit3", Hash: "hash3", Parents: []string{"hash4"}}, @@ -255,7 +256,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "startIdx is past TODO commits", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Parents: []string{"hash2", "hash3"}, Action: todo.Pick}, {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}, Action: todo.Pick}, {Name: "commit3", Hash: "hash3", Parents: []string{"hash4"}}, @@ -275,7 +276,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "only showing TODO commits", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Parents: []string{"hash2", "hash3"}, Action: todo.Pick}, {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}, Action: todo.Pick}, {Name: "commit3", Hash: "hash3", Parents: []string{"hash4"}}, @@ -295,7 +296,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "no TODO commits, towards bottom", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Parents: []string{"hash2", "hash3"}}, {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}}, {Name: "commit3", Hash: "hash3", Parents: []string{"hash4"}}, @@ -314,7 +315,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "only TODO commits except last", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", Parents: []string{"hash2", "hash3"}, Action: todo.Pick}, {Name: "commit2", Hash: "hash2", Parents: []string{"hash3"}, Action: todo.Pick}, {Name: "commit3", Hash: "hash3", Parents: []string{"hash4"}, Action: todo.Pick}, @@ -334,7 +335,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "graph in divergence view - all commits visible", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1r", Parents: []string{"hash2r"}, Divergence: models.DivergenceRight}, {Name: "commit2", Hash: "hash2r", Parents: []string{"hash3r", "hash5r"}, Divergence: models.DivergenceRight}, {Name: "commit3", Hash: "hash3r", Parents: []string{"hash4r"}, Divergence: models.DivergenceRight}, @@ -363,7 +364,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "graph in divergence view - not all remote commits visible", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1r", Parents: []string{"hash2r"}, Divergence: models.DivergenceRight}, {Name: "commit2", Hash: "hash2r", Parents: []string{"hash3r", "hash5r"}, Divergence: models.DivergenceRight}, {Name: "commit3", Hash: "hash3r", Parents: []string{"hash4r"}, Divergence: models.DivergenceRight}, @@ -390,7 +391,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "graph in divergence view - not all local commits", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1r", Parents: []string{"hash2r"}, Divergence: models.DivergenceRight}, {Name: "commit2", Hash: "hash2r", Parents: []string{"hash3r", "hash5r"}, Divergence: models.DivergenceRight}, {Name: "commit3", Hash: "hash3r", Parents: []string{"hash4r"}, Divergence: models.DivergenceRight}, @@ -416,7 +417,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "graph in divergence view - no remote commits visible", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1r", Parents: []string{"hash2r"}, Divergence: models.DivergenceRight}, {Name: "commit2", Hash: "hash2r", Parents: []string{"hash3r", "hash5r"}, Divergence: models.DivergenceRight}, {Name: "commit3", Hash: "hash3r", Parents: []string{"hash4r"}, Divergence: models.DivergenceRight}, @@ -441,7 +442,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "graph in divergence view - no local commits visible", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1r", Parents: []string{"hash2r"}, Divergence: models.DivergenceRight}, {Name: "commit2", Hash: "hash2r", Parents: []string{"hash3r", "hash5r"}, Divergence: models.DivergenceRight}, {Name: "commit3", Hash: "hash3r", Parents: []string{"hash4r"}, Divergence: models.DivergenceRight}, @@ -464,7 +465,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "graph in divergence view - no remote commits present", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1l", Parents: []string{"hash2l"}, Divergence: models.DivergenceLeft}, {Name: "commit2", Hash: "hash2l", Parents: []string{"hash3l", "hash4l"}, Divergence: models.DivergenceLeft}, {Name: "commit3", Hash: "hash3l", Parents: []string{"hash4l"}, Divergence: models.DivergenceLeft}, @@ -487,7 +488,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "graph in divergence view - no local commits present", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1r", Parents: []string{"hash2r"}, Divergence: models.DivergenceRight}, {Name: "commit2", Hash: "hash2r", Parents: []string{"hash3r", "hash5r"}, Divergence: models.DivergenceRight}, {Name: "commit3", Hash: "hash3r", Parents: []string{"hash4r"}, Divergence: models.DivergenceRight}, @@ -506,7 +507,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { }, { testName: "custom time format", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Name: "commit1", Hash: "hash1", UnixTimestamp: 1577844184, AuthorName: "Jesse Duffield"}, {Name: "commit2", Hash: "hash2", UnixTimestamp: 1576844184, AuthorName: "Jesse Duffield"}, }, @@ -543,9 +544,14 @@ func TestGetCommitListDisplayStrings(t *testing.T) { for _, s := range scenarios { if !focusing || s.focus { t.Run(s.testName, func(t *testing.T) { + hashPool := &utils.StringPool{} + + commits := lo.Map(s.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + result := GetCommitListDisplayStrings( common, - s.commits, + commits, s.branches, s.currentBranchName, s.hasUpdateRefConfig, @@ -557,7 +563,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { s.shortTimeFormat, s.now, s.parseEmoji, - s.selectedCommitHash, + s.selectedCommitHashPtr, s.startIdx, s.endIdx, s.showGraph, diff --git a/pkg/gui/presentation/files_test.go b/pkg/gui/presentation/files_test.go index ac97aa2bc8e..9a16eac0855 100644 --- a/pkg/gui/presentation/files_test.go +++ b/pkg/gui/presentation/files_test.go @@ -149,8 +149,10 @@ func TestRenderCommitFileTree(t *testing.T) { for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { + hashPool := &utils.StringPool{} + viewModel := filetree.NewCommitFileTreeViewModel(func() []*models.CommitFile { return s.files }, utils.NewDummyLog(), true) - viewModel.SetRef(&models.Commit{}) + viewModel.SetRef(models.NewCommit(hashPool, models.NewCommitOpts{Hash: "1234"})) viewModel.SetTree() for _, path := range s.collapsedPaths { viewModel.ToggleCollapsed(path) diff --git a/pkg/gui/presentation/graph/cell.go b/pkg/gui/presentation/graph/cell.go index cc2ad53c36c..a890613f0e2 100644 --- a/pkg/gui/presentation/graph/cell.go +++ b/pkg/gui/presentation/graph/cell.go @@ -25,7 +25,7 @@ type Cell struct { up, down, left, right bool cellType cellType rightStyle *style.TextStyle - style style.TextStyle + style *style.TextStyle } func (cell *Cell) render(writer io.StringWriter) { @@ -44,7 +44,7 @@ func (cell *Cell) render(writer io.StringWriter) { var rightStyle *style.TextStyle if cell.rightStyle == nil { - rightStyle = &cell.style + rightStyle = cell.style } else { rightStyle = cell.rightStyle } @@ -59,7 +59,7 @@ func (cell *Cell) render(writer io.StringWriter) { styledSecondChar = cachedSprint(*rightStyle, second) } - _, _ = writer.WriteString(cachedSprint(cell.style, adjustedFirst)) + _, _ = writer.WriteString(cachedSprint(*cell.style, adjustedFirst)) _, _ = writer.WriteString(styledSecondChar) } @@ -104,19 +104,19 @@ func (cell *Cell) reset() { cell.right = false } -func (cell *Cell) setUp(style style.TextStyle) *Cell { +func (cell *Cell) setUp(style *style.TextStyle) *Cell { cell.up = true cell.style = style return cell } -func (cell *Cell) setDown(style style.TextStyle) *Cell { +func (cell *Cell) setDown(style *style.TextStyle) *Cell { cell.down = true cell.style = style return cell } -func (cell *Cell) setLeft(style style.TextStyle) *Cell { +func (cell *Cell) setLeft(style *style.TextStyle) *Cell { cell.left = true if !cell.up && !cell.down { // vertical trumps left @@ -126,15 +126,15 @@ func (cell *Cell) setLeft(style style.TextStyle) *Cell { } //nolint:unparam -func (cell *Cell) setRight(style style.TextStyle, override bool) *Cell { +func (cell *Cell) setRight(style *style.TextStyle, override bool) *Cell { cell.right = true if cell.rightStyle == nil || override { - cell.rightStyle = &style + cell.rightStyle = style } return cell } -func (cell *Cell) setStyle(style style.TextStyle) *Cell { +func (cell *Cell) setStyle(style *style.TextStyle) *Cell { cell.style = style return cell } diff --git a/pkg/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index 441c913010a..4e204c03ff6 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -23,58 +23,53 @@ const ( ) type Pipe struct { - fromPos int - toPos int - fromHash string - toHash string + fromHash *string + toHash *string + style *style.TextStyle + fromPos int16 + toPos int16 kind PipeKind - style style.TextStyle } -var highlightStyle = style.FgLightWhite.SetBold() - -func ContainsCommitHash(pipes []*Pipe, hash string) bool { - for _, pipe := range pipes { - if equalHashes(pipe.fromHash, hash) { - return true - } - } - return false -} +var ( + highlightStyle = style.FgLightWhite.SetBold() + EmptyTreeCommitHash = models.EmptyTreeCommitHash + StartCommitHash = "START" +) -func (self Pipe) left() int { +func (self Pipe) left() int16 { return min(self.fromPos, self.toPos) } -func (self Pipe) right() int { +func (self Pipe) right() int16 { return max(self.fromPos, self.toPos) } -func RenderCommitGraph(commits []*models.Commit, selectedCommitHash string, getStyle func(c *models.Commit) style.TextStyle) []string { +func RenderCommitGraph(commits []*models.Commit, selectedCommitHashPtr *string, getStyle func(c *models.Commit) *style.TextStyle) []string { pipeSets := GetPipeSets(commits, getStyle) if len(pipeSets) == 0 { return nil } - lines := RenderAux(pipeSets, commits, selectedCommitHash) + lines := RenderAux(pipeSets, commits, selectedCommitHashPtr) return lines } -func GetPipeSets(commits []*models.Commit, getStyle func(c *models.Commit) style.TextStyle) [][]*Pipe { +func GetPipeSets(commits []*models.Commit, getStyle func(c *models.Commit) *style.TextStyle) [][]Pipe { if len(commits) == 0 { return nil } - pipes := []*Pipe{{fromPos: 0, toPos: 0, fromHash: "START", toHash: commits[0].Hash, kind: STARTS, style: style.FgDefault}} + pipes := []Pipe{{fromPos: 0, toPos: 0, fromHash: &StartCommitHash, toHash: commits[0].HashPtr(), kind: STARTS, style: &style.FgDefault}} - return lo.Map(commits, func(commit *models.Commit, _ int) []*Pipe { + return lo.Map(commits, func(commit *models.Commit, _ int) []Pipe { pipes = getNextPipes(pipes, commit, getStyle) return pipes }) } -func RenderAux(pipeSets [][]*Pipe, commits []*models.Commit, selectedCommitHash string) []string { +func RenderAux(pipeSets [][]Pipe, commits []*models.Commit, selectedCommitHashPtr *string) []string { maxProcs := runtime.GOMAXPROCS(0) // splitting up the rendering of the graph into multiple goroutines allows us to render the graph in parallel @@ -98,7 +93,7 @@ func RenderAux(pipeSets [][]*Pipe, commits []*models.Commit, selectedCommitHash if k > 0 { prevCommit = commits[k-1] } - line := renderPipeSet(pipeSet, selectedCommitHash, prevCommit) + line := renderPipeSet(pipeSet, selectedCommitHashPtr, prevCommit) innerLines = append(innerLines, line) } chunks[i] = innerLines @@ -111,8 +106,8 @@ func RenderAux(pipeSets [][]*Pipe, commits []*models.Commit, selectedCommitHash return lo.Flatten(chunks) } -func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *models.Commit) style.TextStyle) []*Pipe { - maxPos := 0 +func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *models.Commit) *style.TextStyle) []Pipe { + maxPos := int16(0) for _, pipe := range prevPipes { if pipe.toPos > maxPos { maxPos = pipe.toPos @@ -121,16 +116,16 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod // a pipe that terminated in the previous line has no bearing on the current line // so we'll filter those out - currentPipes := lo.Filter(prevPipes, func(pipe *Pipe, _ int) bool { + currentPipes := lo.Filter(prevPipes, func(pipe Pipe, _ int) bool { return pipe.kind != TERMINATES }) - newPipes := make([]*Pipe, 0, len(currentPipes)+len(commit.Parents)) + newPipes := make([]Pipe, 0, len(currentPipes)+len(commit.ParentPtrs())) // start by assuming that we've got a brand new commit not related to any preceding commit. // (this only happens when we're doing `git log --all`). These will be tacked onto the far end. pos := maxPos + 1 for _, pipe := range currentPipes { - if equalHashes(pipe.toHash, commit.Hash) { + if equalHashes(pipe.toHash, commit.HashPtr()) { // turns out this commit does have a descendant so we'll place it right under the first instance pos = pipe.toPos break @@ -138,74 +133,72 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } // a taken spot is one where a current pipe is ending on + // Note: this set and similar ones below use int instead of int16 because + // that's much more efficient. We cast the int16 values we store in these + // sets to int on every access. takenSpots := set.New[int]() // a traversed spot is one where a current pipe is starting on, ending on, or passing through traversedSpots := set.New[int]() - if len(commit.Parents) > 0 { // merge commit - newPipes = append(newPipes, &Pipe{ - fromPos: pos, - toPos: pos, - fromHash: commit.Hash, - toHash: commit.Parents[0], - kind: STARTS, - style: getStyle(commit), - }) - } else if len(commit.Parents) == 0 { // root commit - newPipes = append(newPipes, &Pipe{ - fromPos: pos, - toPos: pos, - fromHash: commit.Hash, - toHash: models.EmptyTreeCommitHash, - kind: STARTS, - style: getStyle(commit), - }) + var toHash *string + if commit.IsFirstCommit() { + toHash = &EmptyTreeCommitHash + } else { + toHash = commit.ParentPtrs()[0] } + newPipes = append(newPipes, Pipe{ + fromPos: pos, + toPos: pos, + fromHash: commit.HashPtr(), + toHash: toHash, + kind: STARTS, + style: getStyle(commit), + }) traversedSpotsForContinuingPipes := set.New[int]() for _, pipe := range currentPipes { - if !equalHashes(pipe.toHash, commit.Hash) { - traversedSpotsForContinuingPipes.Add(pipe.toPos) + if !equalHashes(pipe.toHash, commit.HashPtr()) { + traversedSpotsForContinuingPipes.Add(int(pipe.toPos)) } } - getNextAvailablePosForContinuingPipe := func() int { - i := 0 + getNextAvailablePosForContinuingPipe := func() int16 { + i := int16(0) for { - if !traversedSpots.Includes(i) { + if !traversedSpots.Includes(int(i)) { return i } i++ } } - getNextAvailablePosForNewPipe := func() int { - i := 0 + getNextAvailablePosForNewPipe := func() int16 { + i := int16(0) for { // a newly created pipe is not allowed to end on a spot that's already taken, // nor on a spot that's been traversed by a continuing pipe. - if !takenSpots.Includes(i) && !traversedSpotsForContinuingPipes.Includes(i) { + if !takenSpots.Includes(int(i)) && !traversedSpotsForContinuingPipes.Includes(int(i)) { return i } i++ } } - traverse := func(from, to int) { + traverse := func(from, to int16) { left, right := from, to if left > right { left, right = right, left } for i := left; i <= right; i++ { - traversedSpots.Add(i) + traversedSpots.Add(int(i)) } - takenSpots.Add(to) + takenSpots.Add(int(to)) } for _, pipe := range currentPipes { - if equalHashes(pipe.toHash, commit.Hash) { + if equalHashes(pipe.toHash, commit.HashPtr()) { // terminating here - newPipes = append(newPipes, &Pipe{ + newPipes = append(newPipes, Pipe{ fromPos: pipe.toPos, toPos: pos, fromHash: pipe.fromHash, @@ -217,7 +210,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } else if pipe.toPos < pos { // continuing here availablePos := getNextAvailablePosForContinuingPipe() - newPipes = append(newPipes, &Pipe{ + newPipes = append(newPipes, Pipe{ fromPos: pipe.toPos, toPos: availablePos, fromHash: pipe.fromHash, @@ -230,34 +223,34 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } if commit.IsMerge() { - for _, parent := range commit.Parents[1:] { + for _, parent := range commit.ParentPtrs()[1:] { availablePos := getNextAvailablePosForNewPipe() // need to act as if continuing pipes are going to continue on the same line. - newPipes = append(newPipes, &Pipe{ + newPipes = append(newPipes, Pipe{ fromPos: pos, toPos: availablePos, - fromHash: commit.Hash, + fromHash: commit.HashPtr(), toHash: parent, kind: STARTS, style: getStyle(commit), }) - takenSpots.Add(availablePos) + takenSpots.Add(int(availablePos)) } } for _, pipe := range currentPipes { - if !equalHashes(pipe.toHash, commit.Hash) && pipe.toPos > pos { + if !equalHashes(pipe.toHash, commit.HashPtr()) && pipe.toPos > pos { // continuing on, potentially moving left to fill in a blank spot last := pipe.toPos for i := pipe.toPos; i > pos; i-- { - if takenSpots.Includes(i) || traversedSpots.Includes(i) { + if takenSpots.Includes(int(i)) || traversedSpots.Includes(int(i)) { break } else { last = i } } - newPipes = append(newPipes, &Pipe{ + newPipes = append(newPipes, Pipe{ fromPos: pipe.toPos, toPos: last, fromHash: pipe.fromHash, @@ -270,7 +263,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } // not efficient but doing it for now: sorting my pipes by toPos, then by kind - slices.SortFunc(newPipes, func(a, b *Pipe) int { + slices.SortFunc(newPipes, func(a, b Pipe) int { if a.toPos == b.toPos { return cmp.Compare(a.kind, b.kind) } @@ -281,12 +274,12 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } func renderPipeSet( - pipes []*Pipe, - selectedCommitHash string, + pipes []Pipe, + selectedCommitHashPtr *string, prevCommit *models.Commit, ) string { - maxPos := 0 - commitPos := 0 + maxPos := int16(0) + commitPos := int16(0) startCount := 0 for _, pipe := range pipes { if pipe.kind == STARTS { @@ -302,11 +295,11 @@ func renderPipeSet( } isMerge := startCount > 1 - cells := lo.Map(lo.Range(maxPos+1), func(i int, _ int) *Cell { - return &Cell{cellType: CONNECTION, style: style.FgDefault} + cells := lo.Map(lo.Range(int(maxPos)+1), func(i int, _ int) *Cell { + return &Cell{cellType: CONNECTION, style: &style.FgDefault} }) - renderPipe := func(pipe *Pipe, style style.TextStyle, overrideRightStyle bool) { + renderPipe := func(pipe *Pipe, style *style.TextStyle, overrideRightStyle bool) { left := pipe.left() right := pipe.right() @@ -329,10 +322,10 @@ func renderPipeSet( // we don't want to highlight two commits if they're contiguous. We only want // to highlight multiple things if there's an actual visible pipe involved. highlight := true - if prevCommit != nil && equalHashes(prevCommit.Hash, selectedCommitHash) { + if prevCommit != nil && equalHashes(prevCommit.HashPtr(), selectedCommitHashPtr) { highlight = false for _, pipe := range pipes { - if equalHashes(pipe.fromHash, selectedCommitHash) && (pipe.kind != TERMINATES || pipe.fromPos != pipe.toPos) { + if equalHashes(pipe.fromHash, selectedCommitHashPtr) && (pipe.kind != TERMINATES || pipe.fromPos != pipe.toPos) { highlight = true } } @@ -340,19 +333,19 @@ func renderPipeSet( // so we have our commit pos again, now it's time to build the cells. // we'll handle the one that's sourced from our selected commit last so that it can override the other cells. - selectedPipes, nonSelectedPipes := utils.Partition(pipes, func(pipe *Pipe) bool { - return highlight && equalHashes(pipe.fromHash, selectedCommitHash) + selectedPipes, nonSelectedPipes := utils.Partition(pipes, func(pipe Pipe) bool { + return highlight && equalHashes(pipe.fromHash, selectedCommitHashPtr) }) for _, pipe := range nonSelectedPipes { if pipe.kind == STARTS { - renderPipe(pipe, pipe.style, true) + renderPipe(&pipe, pipe.style, true) } } for _, pipe := range nonSelectedPipes { if pipe.kind != STARTS && !(pipe.kind == TERMINATES && pipe.fromPos == commitPos && pipe.toPos == commitPos) { - renderPipe(pipe, pipe.style, false) + renderPipe(&pipe, pipe.style, false) } } @@ -362,9 +355,9 @@ func renderPipeSet( } } for _, pipe := range selectedPipes { - renderPipe(pipe, highlightStyle, true) + renderPipe(&pipe, &highlightStyle, true) if pipe.toPos == commitPos { - cells[pipe.toPos].setStyle(highlightStyle) + cells[pipe.toPos].setStyle(&highlightStyle) } } @@ -384,13 +377,12 @@ func renderPipeSet( return writer.String() } -func equalHashes(a, b string) bool { - // if our selectedCommitHash is an empty string we treat that as meaning there is no selected commit hash - if a == "" || b == "" { +func equalHashes(a, b *string) bool { + // if our selectedCommitHashPtr is nil, there is no selected commit + if a == nil || b == nil { return false } - length := min(len(a), len(b)) - // parent hashes are only stored up to 20 characters for some reason so we'll truncate to that for comparison - return a[:length] == b[:length] + // We know that all hashes are stored in the pool, so we can compare their addresses + return a == b } diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 39f4400da20..13709267e20 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -11,6 +11,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gui/presentation/authors" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/xo/terminfo" ) @@ -18,12 +19,12 @@ import ( func TestRenderCommitGraph(t *testing.T) { tests := []struct { name string - commits []*models.Commit + commitOpts []models.NewCommitOpts expectedOutput string }{ { name: "with some merges", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"3"}}, {Hash: "3", Parents: []string{"4"}}, @@ -57,7 +58,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "with a path that has room to move to the left", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"3", "4"}}, {Hash: "4", Parents: []string{"3", "5"}}, @@ -75,7 +76,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "with a new commit", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"3", "4"}}, {Hash: "4", Parents: []string{"3", "5"}}, @@ -95,7 +96,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "with a path that has room to move to the left and continues", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"3", "4"}}, {Hash: "3", Parents: []string{"5", "4"}}, @@ -113,7 +114,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "with a path that has room to move to the left and continues", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"3", "4"}}, {Hash: "3", Parents: []string{"5", "4"}}, @@ -133,7 +134,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "with a path that has room to move to the left and continues", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2", "3"}}, {Hash: "3", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"4", "5"}}, @@ -149,7 +150,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "new merge path fills gap before continuing path on right", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2", "3", "4", "5"}}, {Hash: "4", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"A"}}, @@ -165,7 +166,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "with a path that has room to move to the left and continues", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"3", "4"}}, {Hash: "3", Parents: []string{"5", "4"}}, @@ -187,7 +188,7 @@ func TestRenderCommitGraph(t *testing.T) { }, { name: "with a path that has room to move to the left and continues", - commits: []*models.Commit{ + commitOpts: []models.NewCommitOpts{ {Hash: "1", Parents: []string{"2"}}, {Hash: "2", Parents: []string{"3", "4"}}, {Hash: "3", Parents: []string{"5", "4"}}, @@ -218,8 +219,12 @@ func TestRenderCommitGraph(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - getStyle := func(c *models.Commit) style.TextStyle { return style.FgDefault } - lines := RenderCommitGraph(test.commits, "blah", getStyle) + hashPool := &utils.StringPool{} + + getStyle := func(c *models.Commit) *style.TextStyle { return &style.FgDefault } + commits := lo.Map(test.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) + lines := RenderCommitGraph(commits, hashPool.Add("blah"), getStyle) trimmedExpectedOutput := "" for _, line := range strings.Split(strings.TrimPrefix(test.expectedOutput, "\n"), "\n") { @@ -230,7 +235,7 @@ func TestRenderCommitGraph(t *testing.T) { output := "" for i, line := range lines { - description := test.commits[i].Hash + description := test.commitOpts[i].Hash output += strings.TrimSpace(description+" "+utils.Decolorise(line)) + "\n" } t.Log("\nactual: \n" + output) @@ -251,9 +256,12 @@ func TestRenderPipeSet(t *testing.T) { magenta := style.FgMagenta nothing := style.Nothing + hashPool := &utils.StringPool{} + pool := func(s string) *string { return hashPool.Add(s) } + tests := []struct { name string - pipes []*Pipe + pipes []Pipe commit *models.Commit prevCommit *models.Commit expectedStr string @@ -261,33 +269,33 @@ func TestRenderPipeSet(t *testing.T) { }{ { name: "single cell", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: TERMINATES, style: cyan}, - {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: STARTS, style: green}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: &cyan}, + {fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: STARTS, style: &green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "◯", expectedStyles: []style.TextStyle{green}, }, { name: "single cell, selected", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "selected", kind: TERMINATES, style: cyan}, - {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "c", kind: STARTS, style: green}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("selected"), kind: TERMINATES, style: &cyan}, + {fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("c"), kind: STARTS, style: &green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "◯", expectedStyles: []style.TextStyle{highlightStyle}, }, { name: "terminating hook and starting hook, selected", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "selected", kind: TERMINATES, style: cyan}, - {fromPos: 1, toPos: 0, fromHash: "c", toHash: "selected", kind: TERMINATES, style: yellow}, - {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "d", kind: STARTS, style: green}, - {fromPos: 0, toPos: 1, fromHash: "selected", toHash: "e", kind: STARTS, style: green}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("selected"), kind: TERMINATES, style: &cyan}, + {fromPos: 1, toPos: 0, fromHash: pool("c"), toHash: pool("selected"), kind: TERMINATES, style: &yellow}, + {fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("d"), kind: STARTS, style: &green}, + {fromPos: 0, toPos: 1, fromHash: pool("selected"), toHash: pool("e"), kind: STARTS, style: &green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "⏣─╮", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -295,13 +303,13 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "terminating hook and starting hook, prioritise the terminating one", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: TERMINATES, style: red}, - {fromPos: 1, toPos: 0, fromHash: "c", toHash: "b", kind: TERMINATES, style: magenta}, - {fromPos: 0, toPos: 0, fromHash: "b", toHash: "d", kind: STARTS, style: green}, - {fromPos: 0, toPos: 1, fromHash: "b", toHash: "e", kind: STARTS, style: green}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: &red}, + {fromPos: 1, toPos: 0, fromHash: pool("c"), toHash: pool("b"), kind: TERMINATES, style: &magenta}, + {fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("d"), kind: STARTS, style: &green}, + {fromPos: 0, toPos: 1, fromHash: pool("b"), toHash: pool("e"), kind: STARTS, style: &green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "⏣─│", expectedStyles: []style.TextStyle{ green, green, magenta, @@ -309,14 +317,14 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting and terminating pipe sharing some space", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a1", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, - {fromPos: 1, toPos: 1, fromHash: "b1", toHash: "b2", kind: CONTINUES, style: magenta}, - {fromPos: 3, toPos: 0, fromHash: "e1", toHash: "a2", kind: TERMINATES, style: green}, - {fromPos: 0, toPos: 2, fromHash: "a2", toHash: "c3", kind: STARTS, style: yellow}, - }, - prevCommit: &models.Commit{Hash: "a1"}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: &yellow}, + {fromPos: 1, toPos: 1, fromHash: pool("b1"), toHash: pool("b2"), kind: CONTINUES, style: &magenta}, + {fromPos: 3, toPos: 0, fromHash: pool("e1"), toHash: pool("a2"), kind: TERMINATES, style: &green}, + {fromPos: 0, toPos: 2, fromHash: pool("a2"), toHash: pool("c3"), kind: STARTS, style: &yellow}, + }, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─│─┬─╯", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, yellow, green, green, @@ -324,14 +332,14 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting and terminating pipe sharing some space, with selection", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a1", toHash: "selected", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a3", kind: STARTS, style: yellow}, - {fromPos: 1, toPos: 1, fromHash: "b1", toHash: "b2", kind: CONTINUES, style: magenta}, - {fromPos: 3, toPos: 0, fromHash: "e1", toHash: "selected", kind: TERMINATES, style: green}, - {fromPos: 0, toPos: 2, fromHash: "selected", toHash: "c3", kind: STARTS, style: yellow}, - }, - prevCommit: &models.Commit{Hash: "a1"}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("selected"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a3"), kind: STARTS, style: &yellow}, + {fromPos: 1, toPos: 1, fromHash: pool("b1"), toHash: pool("b2"), kind: CONTINUES, style: &magenta}, + {fromPos: 3, toPos: 0, fromHash: pool("e1"), toHash: pool("selected"), kind: TERMINATES, style: &green}, + {fromPos: 0, toPos: 2, fromHash: pool("selected"), toHash: pool("c3"), kind: STARTS, style: &yellow}, + }, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣───╮ ╯", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, highlightStyle, highlightStyle, nothing, green, @@ -339,13 +347,13 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "many terminating pipes", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a1", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, - {fromPos: 1, toPos: 0, fromHash: "b1", toHash: "a2", kind: TERMINATES, style: magenta}, - {fromPos: 2, toPos: 0, fromHash: "c1", toHash: "a2", kind: TERMINATES, style: green}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: &yellow}, + {fromPos: 1, toPos: 0, fromHash: pool("b1"), toHash: pool("a2"), kind: TERMINATES, style: &magenta}, + {fromPos: 2, toPos: 0, fromHash: pool("c1"), toHash: pool("a2"), kind: TERMINATES, style: &green}, }, - prevCommit: &models.Commit{Hash: "a1"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "◯─┴─╯", expectedStyles: []style.TextStyle{ yellow, magenta, magenta, green, green, @@ -353,14 +361,14 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting pipe passing through", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a1", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, - {fromPos: 0, toPos: 3, fromHash: "a2", toHash: "d3", kind: STARTS, style: yellow}, - {fromPos: 1, toPos: 1, fromHash: "b1", toHash: "b3", kind: CONTINUES, style: magenta}, - {fromPos: 2, toPos: 2, fromHash: "c1", toHash: "c3", kind: CONTINUES, style: green}, - }, - prevCommit: &models.Commit{Hash: "a1"}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: &yellow}, + {fromPos: 0, toPos: 3, fromHash: pool("a2"), toHash: pool("d3"), kind: STARTS, style: &yellow}, + {fromPos: 1, toPos: 1, fromHash: pool("b1"), toHash: pool("b3"), kind: CONTINUES, style: &magenta}, + {fromPos: 2, toPos: 2, fromHash: pool("c1"), toHash: pool("c3"), kind: CONTINUES, style: &green}, + }, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─│─│─╮", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, green, yellow, yellow, @@ -368,14 +376,14 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting and terminating path crossing continuing path", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a1", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, - {fromPos: 0, toPos: 1, fromHash: "a2", toHash: "b3", kind: STARTS, style: yellow}, - {fromPos: 1, toPos: 1, fromHash: "b1", toHash: "a2", kind: CONTINUES, style: green}, - {fromPos: 2, toPos: 0, fromHash: "c1", toHash: "a2", kind: TERMINATES, style: magenta}, - }, - prevCommit: &models.Commit{Hash: "a1"}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: &yellow}, + {fromPos: 0, toPos: 1, fromHash: pool("a2"), toHash: pool("b3"), kind: STARTS, style: &yellow}, + {fromPos: 1, toPos: 1, fromHash: pool("b1"), toHash: pool("a2"), kind: CONTINUES, style: &green}, + {fromPos: 2, toPos: 0, fromHash: pool("c1"), toHash: pool("a2"), kind: TERMINATES, style: &magenta}, + }, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─│─╯", expectedStyles: []style.TextStyle{ yellow, yellow, green, magenta, magenta, @@ -383,14 +391,14 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "another clash of starting and terminating paths", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a1", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, - {fromPos: 0, toPos: 1, fromHash: "a2", toHash: "b3", kind: STARTS, style: yellow}, - {fromPos: 2, toPos: 2, fromHash: "c1", toHash: "c3", kind: CONTINUES, style: green}, - {fromPos: 3, toPos: 0, fromHash: "d1", toHash: "a2", kind: TERMINATES, style: magenta}, - }, - prevCommit: &models.Commit{Hash: "a1"}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: &yellow}, + {fromPos: 0, toPos: 1, fromHash: pool("a2"), toHash: pool("b3"), kind: STARTS, style: &yellow}, + {fromPos: 2, toPos: 2, fromHash: pool("c1"), toHash: pool("c3"), kind: CONTINUES, style: &green}, + {fromPos: 3, toPos: 0, fromHash: pool("d1"), toHash: pool("a2"), kind: TERMINATES, style: &magenta}, + }, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "⏣─┬─│─╯", expectedStyles: []style.TextStyle{ yellow, yellow, yellow, magenta, green, magenta, magenta, @@ -398,11 +406,11 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "commit whose previous commit is selected", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: &yellow}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "◯", expectedStyles: []style.TextStyle{ yellow, @@ -410,11 +418,11 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "commit whose previous commit is selected and is a merge commit", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 1, toPos: 1, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 1, toPos: 1, fromHash: pool("selected"), toHash: pool("b3"), kind: CONTINUES, style: &red}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "◯ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, highlightStyle, @@ -422,12 +430,12 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "commit whose previous commit is selected and is a merge commit, with continuing pipe inbetween", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 1, toPos: 1, fromHash: "z1", toHash: "z3", kind: CONTINUES, style: green}, - {fromPos: 2, toPos: 2, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("selected"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 1, toPos: 1, fromHash: pool("z1"), toHash: pool("z3"), kind: CONTINUES, style: &green}, + {fromPos: 2, toPos: 2, fromHash: pool("selected"), toHash: pool("b3"), kind: CONTINUES, style: &red}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "◯ │ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, green, nothing, highlightStyle, @@ -435,13 +443,13 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "when previous commit is selected, not a merge commit, and spawns a continuing pipe", - pipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a1", toHash: "a2", kind: TERMINATES, style: red}, - {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: green}, - {fromPos: 0, toPos: 1, fromHash: "a2", toHash: "b3", kind: STARTS, style: green}, - {fromPos: 1, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: yellow}, + pipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a1"), toHash: pool("a2"), kind: TERMINATES, style: &red}, + {fromPos: 0, toPos: 0, fromHash: pool("a2"), toHash: pool("a3"), kind: STARTS, style: &green}, + {fromPos: 0, toPos: 1, fromHash: pool("a2"), toHash: pool("b3"), kind: STARTS, style: &green}, + {fromPos: 1, toPos: 0, fromHash: pool("selected"), toHash: pool("a2"), kind: TERMINATES, style: &yellow}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "⏣─╯", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -454,7 +462,7 @@ func TestRenderPipeSet(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - actualStr := renderPipeSet(test.pipes, "selected", test.prevCommit) + actualStr := renderPipeSet(test.pipes, pool("selected"), test.prevCommit) t.Log("actual cells:") t.Log(actualStr) expectedStr := "" @@ -474,50 +482,53 @@ func TestRenderPipeSet(t *testing.T) { } func TestGetNextPipes(t *testing.T) { + hashPool := &utils.StringPool{} + pool := func(s string) *string { return hashPool.Add(s) } + tests := []struct { - prevPipes []*Pipe + prevPipes []Pipe commit *models.Commit - expected []*Pipe + expected []Pipe }{ { - prevPipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: STARTS, style: style.FgDefault}, + prevPipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: STARTS, style: &style.FgDefault}, }, - commit: &models.Commit{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "b", Parents: []string{"c"}, - }, - expected: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: TERMINATES, style: style.FgDefault}, - {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: STARTS, style: style.FgDefault}, + }), + expected: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: &style.FgDefault}, + {fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: STARTS, style: &style.FgDefault}, }, }, { - prevPipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: TERMINATES, style: style.FgDefault}, - {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: STARTS, style: style.FgDefault}, - {fromPos: 0, toPos: 1, fromHash: "b", toHash: "d", kind: STARTS, style: style.FgDefault}, + prevPipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: TERMINATES, style: &style.FgDefault}, + {fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: STARTS, style: &style.FgDefault}, + {fromPos: 0, toPos: 1, fromHash: pool("b"), toHash: pool("d"), kind: STARTS, style: &style.FgDefault}, }, - commit: &models.Commit{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "d", Parents: []string{"e"}, - }, - expected: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: CONTINUES, style: style.FgDefault}, - {fromPos: 1, toPos: 1, fromHash: "b", toHash: "d", kind: TERMINATES, style: style.FgDefault}, - {fromPos: 1, toPos: 1, fromHash: "d", toHash: "e", kind: STARTS, style: style.FgDefault}, + }), + expected: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("b"), toHash: pool("c"), kind: CONTINUES, style: &style.FgDefault}, + {fromPos: 1, toPos: 1, fromHash: pool("b"), toHash: pool("d"), kind: TERMINATES, style: &style.FgDefault}, + {fromPos: 1, toPos: 1, fromHash: pool("d"), toHash: pool("e"), kind: STARTS, style: &style.FgDefault}, }, }, { - prevPipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "root", kind: TERMINATES, style: style.FgDefault}, + prevPipes: []Pipe{ + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("root"), kind: TERMINATES, style: &style.FgDefault}, }, - commit: &models.Commit{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "root", Parents: []string{}, - }, - expected: []*Pipe{ - {fromPos: 1, toPos: 1, fromHash: "root", toHash: models.EmptyTreeCommitHash, kind: STARTS, style: style.FgDefault}, + }), + expected: []Pipe{ + {fromPos: 1, toPos: 1, fromHash: pool("root"), toHash: pool(models.EmptyTreeCommitHash), kind: STARTS, style: &style.FgDefault}, }, }, } @@ -526,11 +537,11 @@ func TestGetNextPipes(t *testing.T) { defer color.ForceSetColorLevel(oldColorLevel) for _, test := range tests { - getStyle := func(c *models.Commit) style.TextStyle { return style.FgDefault } + getStyle := func(c *models.Commit) *style.TextStyle { return &style.FgDefault } pipes := getNextPipes(test.prevPipes, test.commit, getStyle) // rendering cells so that it's easier to see what went wrong - actualStr := renderPipeSet(pipes, "selected", nil) - expectedStr := renderPipeSet(test.expected, "selected", nil) + actualStr := renderPipeSet(pipes, pool("selected"), nil) + expectedStr := renderPipeSet(test.expected, pool("selected"), nil) t.Log("expected cells:") t.Log(expectedStr) t.Log("actual cells:") @@ -543,19 +554,21 @@ func BenchmarkRenderCommitGraph(b *testing.B) { oldColorLevel := color.ForceSetColorLevel(terminfo.ColorLevelMillions) defer color.ForceSetColorLevel(oldColorLevel) - commits := generateCommits(50) - getStyle := func(commit *models.Commit) style.TextStyle { + hashPool := &utils.StringPool{} + + commits := generateCommits(hashPool, 50) + getStyle := func(commit *models.Commit) *style.TextStyle { return authors.AuthorStyle(commit.AuthorName) } b.ResetTimer() - for i := 0; i < b.N; i++ { - RenderCommitGraph(commits, "selected", getStyle) + for b.Loop() { + RenderCommitGraph(commits, hashPool.Add("selected"), getStyle) } } -func generateCommits(count int) []*models.Commit { +func generateCommits(hashPool *utils.StringPool, count int) []*models.Commit { rnd := rand.New(rand.NewSource(1234)) - pool := []*models.Commit{{Hash: "a", AuthorName: "A"}} + pool := []*models.Commit{models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a", AuthorName: "A"})} commits := make([]*models.Commit, 0, count) authorPool := []string{"A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"} for len(commits) < count { @@ -565,22 +578,28 @@ func generateCommits(count int) []*models.Commit { // I need to pick a random number of parents to add parentCount := rnd.Intn(2) + 1 + parentHashes := currentCommit.Parents() for j := 0; j < parentCount; j++ { reuseParent := rnd.Intn(6) != 1 && j <= len(pool)-1 && j != 0 var newParent *models.Commit if reuseParent { newParent = pool[j] } else { - newParent = &models.Commit{ - Hash: fmt.Sprintf("%s%d", currentCommit.Hash, j), + newParent = models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: fmt.Sprintf("%s%d", currentCommit.Hash(), j), AuthorName: authorPool[rnd.Intn(len(authorPool))], - } + }) pool = append(pool, newParent) } - currentCommit.Parents = append(currentCommit.Parents, newParent.Hash) + parentHashes = append(parentHashes, newParent.Hash()) } - commits = append(commits, currentCommit) + changedCommit := models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: currentCommit.Hash(), + AuthorName: currentCommit.AuthorName, + Parents: parentHashes, + }) + commits = append(commits, changedCommit) } return commits diff --git a/pkg/gui/presentation/icons/git_icons.go b/pkg/gui/presentation/icons/git_icons.go index 55ada2e512d..ed91121d09e 100644 --- a/pkg/gui/presentation/icons/git_icons.go +++ b/pkg/gui/presentation/icons/git_icons.go @@ -62,7 +62,7 @@ func IconForTag(tag *models.Tag) string { } func IconForCommit(commit *models.Commit) string { - if len(commit.Parents) > 1 { + if commit.IsMerge() { return MERGE_COMMIT_ICON } return COMMIT_ICON diff --git a/pkg/gui/presentation/reflog_commits.go b/pkg/gui/presentation/reflog_commits.go index b97064d5111..b84a10e594c 100644 --- a/pkg/gui/presentation/reflog_commits.go +++ b/pkg/gui/presentation/reflog_commits.go @@ -21,8 +21,8 @@ func GetReflogCommitListDisplayStrings(commits []*models.Commit, fullDescription } return lo.Map(commits, func(commit *models.Commit, _ int) []string { - diffed := commit.Hash == diffName - cherryPicked := cherryPickedCommitHashSet.Includes(commit.Hash) + diffed := commit.Hash() == diffName + cherryPicked := cherryPickedCommitHashSet.Includes(commit.Hash()) return displayFunc(commit, reflogCommitDisplayAttributes{ cherryPicked: cherryPicked, diff --git a/pkg/gui/services/custom_commands/session_state_loader.go b/pkg/gui/services/custom_commands/session_state_loader.go index 933a99711a6..87465ec1a14 100644 --- a/pkg/gui/services/custom_commands/session_state_loader.go +++ b/pkg/gui/services/custom_commands/session_state_loader.go @@ -26,8 +26,8 @@ func commitShimFromModelCommit(commit *models.Commit) *Commit { } return &Commit{ - Hash: commit.Hash, - Sha: commit.Hash, + Hash: commit.Hash(), + Sha: commit.Hash(), Name: commit.Name, Status: commit.Status, Action: commit.Action, @@ -37,7 +37,7 @@ func commitShimFromModelCommit(commit *models.Commit) *Commit { AuthorEmail: commit.AuthorEmail, UnixTimestamp: commit.UnixTimestamp, Divergence: commit.Divergence, - Parents: commit.Parents, + Parents: commit.Parents(), } } @@ -173,8 +173,8 @@ func makeCommitRange(commits []*models.Commit, _ int, _ int) *CommitRange { } return &CommitRange{ - From: commits[len(commits)-1].Hash, - To: commits[0].Hash, + From: commits[len(commits)-1].Hash(), + To: commits[0].Hash(), } } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 863d7c2004e..061fee12522 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -305,6 +305,8 @@ type Model struct { FilesTrie *patricia.Trie Authors map[string]*models.Author + + HashPool *utils.StringPool } // if you add a new mutex here be sure to instantiate it. We're using pointers to diff --git a/pkg/utils/color.go b/pkg/utils/color.go index 05e0aa9bc03..17e40033731 100644 --- a/pkg/utils/color.go +++ b/pkg/utils/color.go @@ -57,11 +57,12 @@ func IsValidHexValue(v string) bool { return true } -func SetCustomColors(customColors map[string]string) map[string]style.TextStyle { - return lo.MapValues(customColors, func(c string, key string) style.TextStyle { +func SetCustomColors(customColors map[string]string) map[string]*style.TextStyle { + return lo.MapValues(customColors, func(c string, key string) *style.TextStyle { if s, ok := style.ColorMap[c]; ok { - return s.Foreground + return &s.Foreground } - return style.New().SetFg(style.NewRGBColor(color.HEX(c, false))) + value := style.New().SetFg(style.NewRGBColor(color.HEX(c, false))) + return &value }) } diff --git a/pkg/utils/formatting_test.go b/pkg/utils/formatting_test.go index ac2adee5f6c..37e6702a5f9 100644 --- a/pkg/utils/formatting_test.go +++ b/pkg/utils/formatting_test.go @@ -253,25 +253,25 @@ func TestRenderDisplayStrings(t *testing.T) { } func BenchmarkStringWidthAsciiOriginal(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { runewidth.StringWidth("some ASCII string") } } func BenchmarkStringWidthAsciiOptimized(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { StringWidth("some ASCII string") } } func BenchmarkStringWidthNonAsciiOriginal(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { runewidth.StringWidth("some non-ASCII string 🍉") } } func BenchmarkStringWidthNonAsciiOptimized(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { StringWidth("some non-ASCII string 🍉") } } diff --git a/pkg/utils/string_pool.go b/pkg/utils/string_pool.go new file mode 100644 index 00000000000..98932f4d567 --- /dev/null +++ b/pkg/utils/string_pool.go @@ -0,0 +1,14 @@ +package utils + +import "sync" + +// A simple string pool implementation that can help reduce memory usage for +// cases where the same string is used multiple times. +type StringPool struct { + sync.Map +} + +func (self *StringPool) Add(s string) *string { + poolEntry, _ := self.LoadOrStore(s, &s) + return poolEntry.(*string) +} diff --git a/vendor/github.com/stefanhaller/git-todo-parser/todo/parse.go b/vendor/github.com/stefanhaller/git-todo-parser/todo/parse.go index 12bec439b4f..bf40f531a08 100644 --- a/vendor/github.com/stefanhaller/git-todo-parser/todo/parse.go +++ b/vendor/github.com/stefanhaller/git-todo-parser/todo/parse.go @@ -150,9 +150,6 @@ func parseLine(line string, commentChar byte) (Todo, error) { } func isCommand(i TodoCommand, s string) bool { - if i < 0 || i > Comment { - return false - } return len(s) > 0 && (todoCommandInfo[i].cmd == s || todoCommandInfo[i].nickname == s) } diff --git a/vendor/github.com/stefanhaller/git-todo-parser/todo/todo.go b/vendor/github.com/stefanhaller/git-todo-parser/todo/todo.go index 3b2c4529004..1ed22382add 100644 --- a/vendor/github.com/stefanhaller/git-todo-parser/todo/todo.go +++ b/vendor/github.com/stefanhaller/git-todo-parser/todo/todo.go @@ -1,6 +1,6 @@ package todo -type TodoCommand int +type TodoCommand uint8 const ( Pick TodoCommand = iota + 1 diff --git a/vendor/modules.txt b/vendor/modules.txt index e2ec62f924f..9859a3944de 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -323,7 +323,7 @@ github.com/spf13/afero/mem # github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad ## explicit github.com/spkg/bom -# github.com/stefanhaller/git-todo-parser v0.0.7-0.20240406123903-fd957137b6e2 +# github.com/stefanhaller/git-todo-parser v0.0.7-0.20250429125209-dcf39e4641f5 ## explicit; go 1.13 github.com/stefanhaller/git-todo-parser/todo # github.com/stretchr/testify v1.10.0