From 14eb4c29caaf57b4370cfc861210a827c47c8af0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 29 Apr 2025 14:55:10 +0200 Subject: [PATCH 01/17] Bump git-todo-parser --- go.mod | 2 +- go.sum | 4 ++-- vendor/github.com/stefanhaller/git-todo-parser/todo/parse.go | 3 --- vendor/github.com/stefanhaller/git-todo-parser/todo/todo.go | 2 +- vendor/modules.txt | 2 +- 5 files changed, 5 insertions(+), 8 deletions(-) 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/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 From 4cfa6e0c98a18e0c9fb1bbb410c1ed10ed0c823d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 18 Apr 2025 21:30:24 +0200 Subject: [PATCH 02/17] Modernize benchmarks See https://go.dev/blog/testing-b-loop --- pkg/config/app_config_test.go | 2 +- pkg/gui/presentation/graph/graph_test.go | 2 +- pkg/utils/formatting_test.go | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) 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/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 39f4400da20..5cb4875f5e0 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -548,7 +548,7 @@ func BenchmarkRenderCommitGraph(b *testing.B) { return authors.AuthorStyle(commit.AuthorName) } b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { RenderCommitGraph(commits, "selected", getStyle) } } 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 🍉") } } From 9d202cf9eaf334633021f9c4a4f9be04b8436c9d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 15 Apr 2025 10:33:14 +0200 Subject: [PATCH 03/17] Remove unused function --- pkg/gui/presentation/graph/graph.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkg/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index 441c913010a..ae4401a133e 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -33,15 +33,6 @@ type Pipe struct { 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 -} - func (self Pipe) left() int { return min(self.fromPos, self.toPos) } From 5b109b2dd6de00663b6082c9aeabf3c62df4d08c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 24 Apr 2025 08:28:57 +0200 Subject: [PATCH 04/17] Fix confusing variable name These are not the expected commits. --- pkg/commands/git_commands/commit_loader_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index e3e8c346bec..512e34bbc4d 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -570,9 +570,9 @@ 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) + commits := scenario.commits + setCommitMergedStatuses(scenario.ancestor, commits) + assert.Equal(t, scenario.expectedCommits, commits) }) } } From cb0c8f39bf4e9973fd96d2e76162bae92f847224 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 26 Apr 2025 20:51:53 +0200 Subject: [PATCH 05/17] Simplify code and fix comment The "// merge commit" comment was plain wrong, this is any commit that has a parent, merge or not. The "else if" condition was unnecessary, a plain "else" would have been enough. But the code in the two blocks was almost identical, so extract the one thing that was different and unify it. And while we're at it, use IsFirstCommit() instead of counting parents. --- pkg/gui/presentation/graph/graph.go | 31 ++++++++++++----------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/pkg/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index ae4401a133e..31bbe529ecf 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -133,25 +133,20 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod // 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 = models.EmptyTreeCommitHash + } else { + toHash = commit.Parents[0] } + newPipes = append(newPipes, &Pipe{ + fromPos: pos, + toPos: pos, + fromHash: commit.Hash, + toHash: toHash, + kind: STARTS, + style: getStyle(commit), + }) traversedSpotsForContinuingPipes := set.New[int]() for _, pipe := range currentPipes { From 5844ec6eb2a579da71d92e7c117a90b8e2f33c55 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 26 Apr 2025 16:00:59 +0200 Subject: [PATCH 06/17] Cleanup: use IsMerge instead of counting Parents --- pkg/gui/presentation/icons/git_icons.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 97aa7a04e66c593ef7c4fbc3d8871a135e99d06a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 26 Apr 2025 16:02:49 +0200 Subject: [PATCH 07/17] Rewrite generateCommits to avoid write access to commit.Parents We want to unexport Parents in a later commit. --- pkg/gui/presentation/graph/graph_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 5cb4875f5e0..05bf6c64887 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -565,6 +565,7 @@ 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 @@ -577,10 +578,15 @@ func generateCommits(count int) []*models.Commit { } pool = append(pool, newParent) } - currentCommit.Parents = append(currentCommit.Parents, newParent.Hash) + parentHashes = append(parentHashes, newParent.Hash) } - commits = append(commits, currentCommit) + changedCommit := &models.Commit{ + Hash: currentCommit.Hash, + AuthorName: currentCommit.AuthorName, + Parents: parentHashes, + } + commits = append(commits, changedCommit) } return commits From 1037371a449fc220ac3843271086a9ddd443b18f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 24 Apr 2025 09:13:40 +0200 Subject: [PATCH 08/17] Make Commit.Hash a getter for an unexported hash field This is in preparation for turning the hash into pointer to a string. --- pkg/app/daemon/rebase.go | 2 +- pkg/commands/git_commands/commit_loader.go | 32 +++---- .../git_commands/commit_loader_test.go | 76 +++++++++-------- pkg/commands/git_commands/patch.go | 10 +-- pkg/commands/git_commands/rebase.go | 30 +++---- pkg/commands/git_commands/rebase_test.go | 13 +-- .../git_commands/reflog_commit_loader.go | 6 +- .../git_commands/reflog_commit_loader_test.go | 34 ++++---- pkg/commands/models/commit.go | 46 ++++++++-- pkg/gui/context/local_commits_context.go | 10 +-- pkg/gui/context/sub_commits_context.go | 4 +- .../controllers/basic_commits_controller.go | 22 ++--- pkg/gui/controllers/bisect_controller.go | 14 ++-- .../custom_patch_options_menu_action.go | 6 +- .../controllers/helpers/cherry_pick_helper.go | 2 +- pkg/gui/controllers/helpers/diff_helper.go | 2 +- pkg/gui/controllers/helpers/fixup_helper.go | 2 +- pkg/gui/controllers/helpers/refs_helper.go | 4 +- .../controllers/local_commits_controller.go | 28 +++---- .../controllers/reflog_commits_controller.go | 2 +- pkg/gui/controllers/undo_controller.go | 6 +- pkg/gui/modes/cherrypicking/cherry_picking.go | 8 +- pkg/gui/presentation/commits.go | 24 +++--- pkg/gui/presentation/commits_test.go | 52 ++++++------ pkg/gui/presentation/graph/graph.go | 16 ++-- pkg/gui/presentation/graph/graph_test.go | 83 ++++++++++--------- pkg/gui/presentation/reflog_commits.go | 4 +- .../custom_commands/session_state_loader.go | 8 +- 28 files changed, 301 insertions(+), 245 deletions(-) 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..89589d78f02 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -121,7 +121,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() { @@ -234,7 +234,7 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool parents = strings.Split(parentHashes, " ") } - return &models.Commit{ + return models.NewCommit(models.NewCommitOpts{ Hash: hash, Name: message, Tags: tags, @@ -244,7 +244,7 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool AuthorEmail: authorEmail, Parents: parents, Divergence: divergence, - } + }) } func (self *CommitLoader) getHydratedRebasingCommits(addConflictingCommit bool) ([]*models.Commit, error) { @@ -277,7 +277,7 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t } 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 @@ -293,7 +293,7 @@ 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 + fullCommits[commit.Hash()] = commit return false, nil }) if err != nil { @@ -315,9 +315,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) @@ -364,12 +364,12 @@ 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(models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusRebasing, Action: t.Command, - }) + })) } return commits @@ -465,11 +465,11 @@ 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(models.NewCommitOpts{ Hash: lastTodo.Commit, Action: lastTodo.Command, Status: models.StatusConflicted, - } + }) } func (self *CommitLoader) getSequencerCommits() []*models.Commit { @@ -493,12 +493,12 @@ 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(models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusCherryPickingOrReverting, Action: t.Command, - }) + })) } return commits @@ -526,11 +526,11 @@ func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.W if len(lines) == 0 { return nil } - return &models.Commit{ + return models.NewCommit(models.NewCommitOpts{ Hash: lines[0], Status: models.StatusConflicted, Action: action, - } + }) } func setCommitMergedStatuses(ancestor string, commits []*models.Commit) { @@ -541,7 +541,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 { diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 512e34bbc4d..67fc0429c02 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{ @@ -45,8 +46,8 @@ func TestGetCommits(t *testing.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), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, { testName: "should use proper upstream name for branch", @@ -56,8 +57,8 @@ func TestGetCommits(t *testing.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), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, { testName: "should return commits if they are present", @@ -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", @@ -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", @@ -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", @@ -277,8 +278,8 @@ func TestGetCommits(t *testing.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), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, { testName: "should set filter path", @@ -288,8 +289,8 @@ func TestGetCommits(t *testing.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), - expectedCommits: []*models.Commit{}, - expectedError: nil, + expectedCommitOpts: []models.NewCommitOpts{}, + expectedError: nil, }, } @@ -318,7 +319,9 @@ func TestGetCommits(t *testing.T) { opts.MainBranches = NewMainBranches(common, cmd) 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(opts) }) + assert.Equal(t, expectedCommits, commits) assert.Equal(t, scenario.expectedError, err) scenario.runner.CheckForMissingCalls() @@ -356,11 +359,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: &models.Commit{ + expectedResult: models.NewCommit(models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, - }, + }), }, { testName: "last command was 'break'", @@ -457,11 +460,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: &models.Commit{ + expectedResult: models.NewCommit(models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, - }, + }), }, { testName: "'edit' with amend file", @@ -486,11 +489,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, amendFileExists: false, messageFileExists: true, - expectedResult: &models.Commit{ + expectedResult: models.NewCommit(models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Edit, Status: models.StatusConflicted, - }, + }), }, { testName: "'edit' without amend and without message file", @@ -531,22 +534,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 +557,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 +573,12 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { for _, scenario := range scenarios { t.Run(scenario.testName, func(t *testing.T) { - commits := scenario.commits + commits := lo.Map(scenario.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) setCommitMergedStatuses(scenario.ancestor, commits) - assert.Equal(t, scenario.expectedCommits, commits) + expectedCommits := lo.Map(scenario.expectedCommitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(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..d60295de2c6 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -97,7 +97,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 +108,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 +119,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 +130,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 +158,10 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses), }) - s.test(instance.DiscardOldFileChanges(s.commits, s.commitIndex, s.fileName)) + commits := lo.Map(s.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(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..7ddefd94e25 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -65,7 +65,7 @@ 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) { @@ -82,11 +82,11 @@ func (self *ReflogCommitLoader) parseLine(line string) (*models.Commit, bool) { parents = strings.Split(parentHashes, " ") } - return &models.Commit{ + return models.NewCommit(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..2448a1d23c0 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,7 +27,7 @@ func TestGetReflogCommits(t *testing.T) { lastReflogCommit *models.Commit filterPath string filterAuthor string - expectedCommits []*models.Commit + expectedCommitOpts []models.NewCommitOpts expectedOnlyObtainedNew bool expectedError error } @@ -38,7 +39,7 @@ func TestGetReflogCommits(t *testing.T) { 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, }, @@ -48,7 +49,7 @@ func TestGetReflogCommits(t *testing.T) { 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", @@ -93,14 +94,14 @@ func TestGetReflogCommits(t *testing.T) { runner: oscommands.NewFakeRunner(t). 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(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", @@ -117,15 +118,15 @@ func TestGetReflogCommits(t *testing.T) { 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), - lastReflogCommit: &models.Commit{ + lastReflogCommit: models.NewCommit(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", @@ -142,15 +143,15 @@ func TestGetReflogCommits(t *testing.T) { 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), - lastReflogCommit: &models.Commit{ + lastReflogCommit: models.NewCommit(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", @@ -169,7 +170,7 @@ func TestGetReflogCommits(t *testing.T) { lastReflogCommit: nil, filterPath: "", - expectedCommits: nil, + expectedCommitOpts: nil, expectedOnlyObtainedNew: false, expectedError: errors.New("haha"), }, @@ -186,7 +187,12 @@ func TestGetReflogCommits(t *testing.T) { 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(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..7078ab51e9f 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -42,7 +42,7 @@ const ( // Commit : A git commit type Commit struct { - Hash string + hash string Name string Status CommitStatus Action todo.TodoCommand @@ -57,20 +57,54 @@ type Commit struct { Parents []string } +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(opts NewCommitOpts) *Commit { + return &Commit{ + hash: 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: opts.Parents, + } +} + +func (c *Commit) Hash() 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 { @@ -89,7 +123,7 @@ 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 { diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 2414501845c..dfb270d1e0f 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -37,7 +37,7 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext { if c.Context().Current().GetKey() == LOCAL_COMMITS_CONTEXT_KEY { selectedCommit := viewModel.GetSelected() if selectedCommit != nil { - selectedCommitHash = selectedCommit.Hash + selectedCommitHash = selectedCommit.Hash() } } @@ -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..120e605b3b7 100644 --- a/pkg/gui/context/sub_commits_context.go +++ b/pkg/gui/context/sub_commits_context.go @@ -50,7 +50,7 @@ func NewSubCommitsContext( if c.Context().Current().GetKey() == SUB_COMMITS_CONTEXT_KEY { selectedCommit := viewModel.GetSelected() if selectedCommit != nil { - selectedCommitHash = selectedCommit.Hash + selectedCommitHash = selectedCommit.Hash() } } branches := []*models.Branch{} @@ -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/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/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 16cdab3ecc8..86c149740e1 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(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/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/commits.go b/pkg/gui/presentation/commits.go index 116ad5c5018..ec4a8425d25 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -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 @@ -249,7 +249,7 @@ 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, } @@ -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..13fb396e7d1 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 @@ -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,12 @@ func TestGetCommitListDisplayStrings(t *testing.T) { for _, s := range scenarios { if !focusing || s.focus { t.Run(s.testName, func(t *testing.T) { + commits := lo.Map(s.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + result := GetCommitListDisplayStrings( common, - s.commits, + commits, s.branches, s.currentBranchName, s.hasUpdateRefConfig, diff --git a/pkg/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index 31bbe529ecf..9f71bb26ebf 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -57,7 +57,7 @@ func GetPipeSets(commits []*models.Commit, getStyle func(c *models.Commit) style 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: "START", toHash: commits[0].Hash(), kind: STARTS, style: style.FgDefault}} return lo.Map(commits, func(commit *models.Commit, _ int) []*Pipe { pipes = getNextPipes(pipes, commit, getStyle) @@ -121,7 +121,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod // (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.Hash()) { // turns out this commit does have a descendant so we'll place it right under the first instance pos = pipe.toPos break @@ -142,7 +142,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod newPipes = append(newPipes, &Pipe{ fromPos: pos, toPos: pos, - fromHash: commit.Hash, + fromHash: commit.Hash(), toHash: toHash, kind: STARTS, style: getStyle(commit), @@ -150,7 +150,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod traversedSpotsForContinuingPipes := set.New[int]() for _, pipe := range currentPipes { - if !equalHashes(pipe.toHash, commit.Hash) { + if !equalHashes(pipe.toHash, commit.Hash()) { traversedSpotsForContinuingPipes.Add(pipe.toPos) } } @@ -189,7 +189,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } for _, pipe := range currentPipes { - if equalHashes(pipe.toHash, commit.Hash) { + if equalHashes(pipe.toHash, commit.Hash()) { // terminating here newPipes = append(newPipes, &Pipe{ fromPos: pipe.toPos, @@ -222,7 +222,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod newPipes = append(newPipes, &Pipe{ fromPos: pos, toPos: availablePos, - fromHash: commit.Hash, + fromHash: commit.Hash(), toHash: parent, kind: STARTS, style: getStyle(commit), @@ -233,7 +233,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } for _, pipe := range currentPipes { - if !equalHashes(pipe.toHash, commit.Hash) && pipe.toPos > pos { + if !equalHashes(pipe.toHash, commit.Hash()) && pipe.toPos > pos { // continuing on, potentially moving left to fill in a blank spot last := pipe.toPos for i := pipe.toPos; i > pos; i-- { @@ -315,7 +315,7 @@ 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.Hash(), selectedCommitHash) { highlight = false for _, pipe := range pipes { if equalHashes(pipe.fromHash, selectedCommitHash) && (pipe.kind != TERMINATES || pipe.fromPos != pipe.toPos) { diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 05bf6c64887..a4e6aabb3b2 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"}}, @@ -219,7 +220,9 @@ 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) + commits := lo.Map(test.commitOpts, + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + lines := RenderCommitGraph(commits, "blah", getStyle) trimmedExpectedOutput := "" for _, line := range strings.Split(strings.TrimPrefix(test.expectedOutput, "\n"), "\n") { @@ -230,7 +233,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) @@ -265,7 +268,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: TERMINATES, style: cyan}, {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: STARTS, style: green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", expectedStyles: []style.TextStyle{green}, }, @@ -275,7 +278,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "a", toHash: "selected", kind: TERMINATES, style: cyan}, {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "c", kind: STARTS, style: green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", expectedStyles: []style.TextStyle{highlightStyle}, }, @@ -287,7 +290,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "d", kind: STARTS, style: green}, {fromPos: 0, toPos: 1, fromHash: "selected", toHash: "e", kind: STARTS, style: green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─â•Ū", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -301,7 +304,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "b", toHash: "d", kind: STARTS, style: green}, {fromPos: 0, toPos: 1, fromHash: "b", toHash: "e", kind: STARTS, style: green}, }, - prevCommit: &models.Commit{Hash: "a"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─│", expectedStyles: []style.TextStyle{ green, green, magenta, @@ -316,7 +319,7 @@ func TestRenderPipeSet(t *testing.T) { {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"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─│─┮─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, yellow, green, green, @@ -331,7 +334,7 @@ func TestRenderPipeSet(t *testing.T) { {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"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ───â•Ū â•Ŋ", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, highlightStyle, highlightStyle, nothing, green, @@ -345,7 +348,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 0, fromHash: "b1", toHash: "a2", kind: TERMINATES, style: magenta}, {fromPos: 2, toPos: 0, fromHash: "c1", toHash: "a2", kind: TERMINATES, style: green}, }, - prevCommit: &models.Commit{Hash: "a1"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), expectedStr: "â—Ŋ─â”ī─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, magenta, magenta, green, green, @@ -360,7 +363,7 @@ func TestRenderPipeSet(t *testing.T) { {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"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─│─│─â•Ū", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, green, yellow, yellow, @@ -375,7 +378,7 @@ func TestRenderPipeSet(t *testing.T) { {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"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─│─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, yellow, green, magenta, magenta, @@ -390,7 +393,7 @@ func TestRenderPipeSet(t *testing.T) { {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"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─┮─│─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, yellow, yellow, magenta, green, magenta, magenta, @@ -402,7 +405,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ", expectedStyles: []style.TextStyle{ yellow, @@ -414,7 +417,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, {fromPos: 1, toPos: 1, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, highlightStyle, @@ -427,7 +430,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 1, fromHash: "z1", toHash: "z3", kind: CONTINUES, style: green}, {fromPos: 2, toPos: 2, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, green, nothing, highlightStyle, @@ -441,7 +444,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 1, fromHash: "a2", toHash: "b3", kind: STARTS, style: green}, {fromPos: 1, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: yellow}, }, - prevCommit: &models.Commit{Hash: "selected"}, + prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), expectedStr: "âĢ─â•Ŋ", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -483,10 +486,10 @@ func TestGetNextPipes(t *testing.T) { prevPipes: []*Pipe{ {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: STARTS, style: style.FgDefault}, }, - commit: &models.Commit{ + commit: models.NewCommit(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}, @@ -498,10 +501,10 @@ func TestGetNextPipes(t *testing.T) { {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}, }, - commit: &models.Commit{ + commit: models.NewCommit(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}, @@ -512,10 +515,10 @@ func TestGetNextPipes(t *testing.T) { prevPipes: []*Pipe{ {fromPos: 0, toPos: 0, fromHash: "a", toHash: "root", kind: TERMINATES, style: style.FgDefault}, }, - commit: &models.Commit{ + commit: models.NewCommit(models.NewCommitOpts{ Hash: "root", Parents: []string{}, - }, + }), expected: []*Pipe{ {fromPos: 1, toPos: 1, fromHash: "root", toHash: models.EmptyTreeCommitHash, kind: STARTS, style: style.FgDefault}, }, @@ -555,7 +558,7 @@ func BenchmarkRenderCommitGraph(b *testing.B) { func generateCommits(count int) []*models.Commit { rnd := rand.New(rand.NewSource(1234)) - pool := []*models.Commit{{Hash: "a", AuthorName: "A"}} + pool := []*models.Commit{models.NewCommit(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 { @@ -572,20 +575,20 @@ func generateCommits(count int) []*models.Commit { if reuseParent { newParent = pool[j] } else { - newParent = &models.Commit{ - Hash: fmt.Sprintf("%s%d", currentCommit.Hash, j), + newParent = models.NewCommit(models.NewCommitOpts{ + Hash: fmt.Sprintf("%s%d", currentCommit.Hash(), j), AuthorName: authorPool[rnd.Intn(len(authorPool))], - } + }) pool = append(pool, newParent) } - parentHashes = append(parentHashes, newParent.Hash) + parentHashes = append(parentHashes, newParent.Hash()) } - changedCommit := &models.Commit{ - Hash: currentCommit.Hash, + changedCommit := models.NewCommit(models.NewCommitOpts{ + Hash: currentCommit.Hash(), AuthorName: currentCommit.AuthorName, Parents: parentHashes, - } + }) commits = append(commits, changedCommit) } 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..b90ec185a01 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, @@ -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(), } } From e27bc15bbd7955ec87f2ca8e8d19eae8cb9120e5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Apr 2025 16:29:36 +0200 Subject: [PATCH 09/17] Store Commit.Hash by pointer (kept in a pool of hashes) This in itself is not an improvement, because hashes are unique (they are shared between real commits and rebase todos, but there are so few of those that it doesn't matter). However, it becomes an improvement once we also store parent hashes in the same pool; but the real motivation for this change is to also reuse the hash pointers in Pipe objects later in the branch. This will be a big win because in a merge-heavy git repo there are many more Pipe instances than commits. --- pkg/commands/git_commands/commit_loader.go | 53 ++++++++++--------- .../git_commands/commit_loader_test.go | 21 +++++--- pkg/commands/git_commands/rebase_test.go | 4 +- .../git_commands/reflog_commit_loader.go | 9 ++-- .../git_commands/reflog_commit_loader_test.go | 12 +++-- pkg/commands/models/commit.go | 8 +-- pkg/gui/controllers/helpers/refresh_helper.go | 8 +-- .../controllers/helpers/sub_commits_helper.go | 1 + .../controllers/local_commits_controller.go | 2 +- pkg/gui/gui.go | 1 + pkg/gui/presentation/commits_test.go | 4 +- pkg/gui/presentation/files_test.go | 4 +- pkg/gui/presentation/graph/graph_test.go | 50 +++++++++-------- pkg/gui/types/common.go | 2 + pkg/utils/string_pool.go | 14 +++++ 15 files changed, 119 insertions(+), 74 deletions(-) create mode 100644 pkg/utils/string_pool.go diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 89589d78f02..3878dd95adc 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 }) @@ -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.NewCommit(models.NewCommitOpts{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: hash, Name: message, Tags: tags, @@ -247,13 +248,13 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool }) } -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,16 +263,16 @@ 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 } @@ -292,7 +293,7 @@ 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) + commit := self.extractCommitFromLine(hashPool, line, false) fullCommits[commit.Hash()] = commit return false, nil }) @@ -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,7 +365,7 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model // Command does not have a commit associated, skip continue } - commits = utils.Prepend(commits, models.NewCommit(models.NewCommitOpts{ + commits = utils.Prepend(commits, models.NewCommit(hashPool, models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusRebasing, @@ -375,7 +376,7 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model 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.NewCommit(models.NewCommitOpts{ + 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,7 +494,7 @@ func (self *CommitLoader) getSequencerCommits() []*models.Commit { // Command does not have a commit associated, skip continue } - commits = utils.Prepend(commits, models.NewCommit(models.NewCommitOpts{ + commits = utils.Prepend(commits, models.NewCommit(hashPool, models.NewCommitOpts{ Hash: t.Commit, Name: t.Msg, Status: models.StatusCherryPickingOrReverting, @@ -504,7 +505,7 @@ func (self *CommitLoader) getSequencerCommits() []*models.Commit { 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,7 +527,7 @@ func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.W if len(lines) == 0 { return nil } - return models.NewCommit(models.NewCommitOpts{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: lines[0], Status: models.StatusConflicted, Action: action, diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 67fc0429c02..8658e25c47b 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -314,13 +314,16 @@ 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) expectedCommits := lo.Map(scenario.expectedCommitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) assert.Equal(t, expectedCommits, commits) assert.Equal(t, scenario.expectedError, err) @@ -330,6 +333,8 @@ func TestGetCommits(t *testing.T) { } func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { + hashPool := &utils.StringPool{} + scenarios := []struct { testName string todos []todo.Todo @@ -359,7 +364,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: models.NewCommit(models.NewCommitOpts{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, @@ -460,7 +465,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, }, amendFileExists: false, - expectedResult: models.NewCommit(models.NewCommitOpts{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Pick, Status: models.StatusConflicted, @@ -489,7 +494,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { }, amendFileExists: false, messageFileExists: true, - expectedResult: models.NewCommit(models.NewCommitOpts{ + expectedResult: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "fa1afe1", Action: todo.Edit, Status: models.StatusConflicted, @@ -526,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) }) } @@ -573,11 +578,13 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { for _, scenario := range scenarios { t.Run(scenario.testName, func(t *testing.T) { + hashPool := &utils.StringPool{} + commits := lo.Map(scenario.commitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + 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(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) assert.Equal(t, expectedCommits, commits) }) } diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index d60295de2c6..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" ) @@ -158,8 +159,9 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses), }) + hashPool := &utils.StringPool{} commits := lo.Map(s.commitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + 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 7ddefd94e25..10453a1e628 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,7 +24,7 @@ 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"). @@ -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 } @@ -68,7 +69,7 @@ func (self *ReflogCommitLoader) sameReflogCommit(a *models.Commit, b *models.Com 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,7 +83,7 @@ func (self *ReflogCommitLoader) parseLine(line string) (*models.Commit, bool) { parents = strings.Split(parentHashes, " ") } - return models.NewCommit(models.NewCommitOpts{ + return models.NewCommit(hashPool, models.NewCommitOpts{ Hash: fields[0], Name: fields[2], UnixTimestamp: int64(unixTimestamp), diff --git a/pkg/commands/git_commands/reflog_commit_loader_test.go b/pkg/commands/git_commands/reflog_commit_loader_test.go index 2448a1d23c0..9aa8536f54a 100644 --- a/pkg/commands/git_commands/reflog_commit_loader_test.go +++ b/pkg/commands/git_commands/reflog_commit_loader_test.go @@ -32,6 +32,8 @@ func TestGetReflogCommits(t *testing.T) { expectedError error } + hashPool := &utils.StringPool{} + scenarios := []scenario{ { testName: "no reflog entries", @@ -94,7 +96,7 @@ func TestGetReflogCommits(t *testing.T) { runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p"}, reflogOutput, nil), - lastReflogCommit: models.NewCommit(models.NewCommitOpts{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, @@ -118,7 +120,7 @@ func TestGetReflogCommits(t *testing.T) { 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), - lastReflogCommit: models.NewCommit(models.NewCommitOpts{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, @@ -143,7 +145,7 @@ func TestGetReflogCommits(t *testing.T) { 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), - lastReflogCommit: models.NewCommit(models.NewCommitOpts{ + lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", Status: models.StatusReflog, @@ -183,14 +185,14 @@ 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)) var expectedCommits []*models.Commit if scenario.expectedCommitOpts != nil { expectedCommits = lo.Map(scenario.expectedCommitOpts, - func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) } assert.Equal(t, expectedCommits, commits) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index 7078ab51e9f..bde7092dbbb 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -42,7 +42,7 @@ const ( // Commit : A git commit type Commit struct { - hash string + hash *string Name string Status CommitStatus Action todo.TodoCommand @@ -71,9 +71,9 @@ type NewCommitOpts struct { Parents []string } -func NewCommit(opts NewCommitOpts) *Commit { +func NewCommit(hashPool *utils.StringPool, opts NewCommitOpts) *Commit { return &Commit{ - hash: opts.Hash, + hash: hashPool.Add(opts.Hash), Name: opts.Name, Status: opts.Status, Action: opts.Action, @@ -88,7 +88,7 @@ func NewCommit(opts NewCommitOpts) *Commit { } func (c *Commit) Hash() string { - return c.hash + return *c.hash } func (c *Commit) ShortHash() string { 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/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 86c149740e1..ad844f90e4d 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -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.NewCommit(models.NewCommitOpts{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 { 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/presentation/commits_test.go b/pkg/gui/presentation/commits_test.go index 13fb396e7d1..5729e819ed5 100644 --- a/pkg/gui/presentation/commits_test.go +++ b/pkg/gui/presentation/commits_test.go @@ -544,8 +544,10 @@ 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(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) result := GetCommitListDisplayStrings( common, 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/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index a4e6aabb3b2..63d8cb04932 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -219,9 +219,11 @@ func TestRenderCommitGraph(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + 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(opts) }) + func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) lines := RenderCommitGraph(commits, "blah", getStyle) trimmedExpectedOutput := "" @@ -254,6 +256,8 @@ func TestRenderPipeSet(t *testing.T) { magenta := style.FgMagenta nothing := style.Nothing + hashPool := &utils.StringPool{} + tests := []struct { name string pipes []*Pipe @@ -268,7 +272,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: TERMINATES, style: cyan}, {fromPos: 0, toPos: 0, fromHash: "b", toHash: "c", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", expectedStyles: []style.TextStyle{green}, }, @@ -278,7 +282,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "a", toHash: "selected", kind: TERMINATES, style: cyan}, {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "c", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", expectedStyles: []style.TextStyle{highlightStyle}, }, @@ -290,7 +294,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "d", kind: STARTS, style: green}, {fromPos: 0, toPos: 1, fromHash: "selected", toHash: "e", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─â•Ū", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -304,7 +308,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "b", toHash: "d", kind: STARTS, style: green}, {fromPos: 0, toPos: 1, fromHash: "b", toHash: "e", kind: STARTS, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─│", expectedStyles: []style.TextStyle{ green, green, magenta, @@ -319,7 +323,7 @@ func TestRenderPipeSet(t *testing.T) { {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.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─│─┮─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, yellow, green, green, @@ -334,7 +338,7 @@ func TestRenderPipeSet(t *testing.T) { {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.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ───â•Ū â•Ŋ", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, highlightStyle, highlightStyle, nothing, green, @@ -348,7 +352,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 0, fromHash: "b1", toHash: "a2", kind: TERMINATES, style: magenta}, {fromPos: 2, toPos: 0, fromHash: "c1", toHash: "a2", kind: TERMINATES, style: green}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "â—Ŋ─â”ī─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, magenta, magenta, green, green, @@ -363,7 +367,7 @@ func TestRenderPipeSet(t *testing.T) { {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.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─│─│─â•Ū", expectedStyles: []style.TextStyle{ yellow, yellow, magenta, yellow, green, yellow, yellow, @@ -378,7 +382,7 @@ func TestRenderPipeSet(t *testing.T) { {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.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─│─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, yellow, green, magenta, magenta, @@ -393,7 +397,7 @@ func TestRenderPipeSet(t *testing.T) { {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.NewCommit(models.NewCommitOpts{Hash: "a1"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "âĢ─┮─│─â•Ŋ", expectedStyles: []style.TextStyle{ yellow, yellow, yellow, magenta, green, magenta, magenta, @@ -405,7 +409,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, {fromPos: 0, toPos: 0, fromHash: "a2", toHash: "a3", kind: STARTS, style: yellow}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ", expectedStyles: []style.TextStyle{ yellow, @@ -417,7 +421,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: red}, {fromPos: 1, toPos: 1, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, highlightStyle, @@ -430,7 +434,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 1, toPos: 1, fromHash: "z1", toHash: "z3", kind: CONTINUES, style: green}, {fromPos: 2, toPos: 2, fromHash: "selected", toHash: "b3", kind: CONTINUES, style: red}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │ │", expectedStyles: []style.TextStyle{ highlightStyle, nothing, green, nothing, highlightStyle, @@ -444,7 +448,7 @@ func TestRenderPipeSet(t *testing.T) { {fromPos: 0, toPos: 1, fromHash: "a2", toHash: "b3", kind: STARTS, style: green}, {fromPos: 1, toPos: 0, fromHash: "selected", toHash: "a2", kind: TERMINATES, style: yellow}, }, - prevCommit: models.NewCommit(models.NewCommitOpts{Hash: "selected"}), + prevCommit: models.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "âĢ─â•Ŋ", expectedStyles: []style.TextStyle{ highlightStyle, highlightStyle, highlightStyle, @@ -477,6 +481,8 @@ func TestRenderPipeSet(t *testing.T) { } func TestGetNextPipes(t *testing.T) { + hashPool := &utils.StringPool{} + tests := []struct { prevPipes []*Pipe commit *models.Commit @@ -486,7 +492,7 @@ func TestGetNextPipes(t *testing.T) { prevPipes: []*Pipe{ {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: STARTS, style: style.FgDefault}, }, - commit: models.NewCommit(models.NewCommitOpts{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "b", Parents: []string{"c"}, }), @@ -501,7 +507,7 @@ func TestGetNextPipes(t *testing.T) { {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}, }, - commit: models.NewCommit(models.NewCommitOpts{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "d", Parents: []string{"e"}, }), @@ -515,7 +521,7 @@ func TestGetNextPipes(t *testing.T) { prevPipes: []*Pipe{ {fromPos: 0, toPos: 0, fromHash: "a", toHash: "root", kind: TERMINATES, style: style.FgDefault}, }, - commit: models.NewCommit(models.NewCommitOpts{ + commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "root", Parents: []string{}, }), @@ -557,8 +563,10 @@ func BenchmarkRenderCommitGraph(b *testing.B) { } func generateCommits(count int) []*models.Commit { + hashPool := &utils.StringPool{} + rnd := rand.New(rand.NewSource(1234)) - pool := []*models.Commit{models.NewCommit(models.NewCommitOpts{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 { @@ -575,7 +583,7 @@ func generateCommits(count int) []*models.Commit { if reuseParent { newParent = pool[j] } else { - newParent = models.NewCommit(models.NewCommitOpts{ + newParent = models.NewCommit(hashPool, models.NewCommitOpts{ Hash: fmt.Sprintf("%s%d", currentCommit.Hash(), j), AuthorName: authorPool[rnd.Intn(len(authorPool))], }) @@ -584,7 +592,7 @@ func generateCommits(count int) []*models.Commit { parentHashes = append(parentHashes, newParent.Hash()) } - changedCommit := models.NewCommit(models.NewCommitOpts{ + changedCommit := models.NewCommit(hashPool, models.NewCommitOpts{ Hash: currentCommit.Hash(), AuthorName: currentCommit.AuthorName, Parents: parentHashes, 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/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) +} From 0f1f455edba96c96592203b60722498495ac5f36 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 26 Apr 2025 19:58:03 +0200 Subject: [PATCH 10/17] Make Commit.Parents a getter for an unexported parents field This is exactly the same as what we did for Hash earlier. And for the same reason: we want to turn the parents field into a slice of pointers. --- pkg/commands/models/commit.go | 12 ++++++++---- pkg/gui/presentation/graph/graph.go | 6 +++--- pkg/gui/presentation/graph/graph_test.go | 2 +- .../services/custom_commands/session_state_loader.go | 2 +- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index bde7092dbbb..1e2191e92f6 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -54,7 +54,7 @@ type Commit struct { 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 } type NewCommitOpts struct { @@ -83,7 +83,7 @@ func NewCommit(hashPool *utils.StringPool, opts NewCommitOpts) *Commit { AuthorEmail: opts.AuthorEmail, UnixTimestamp: opts.UnixTimestamp, Divergence: opts.Divergence, - Parents: opts.Parents, + parents: opts.Parents, } } @@ -114,8 +114,12 @@ func (c *Commit) ParentRefName() string { return c.RefName() + "^" } +func (c *Commit) Parents() []string { + return c.parents +} + func (c *Commit) IsFirstCommit() bool { - return len(c.Parents) == 0 + return len(c.parents) == 0 } func (c *Commit) ID() string { @@ -127,7 +131,7 @@ func (c *Commit) Description() string { } 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/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index 9f71bb26ebf..de46193ed88 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -116,7 +116,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod return pipe.kind != TERMINATES }) - newPipes := make([]*Pipe, 0, len(currentPipes)+len(commit.Parents)) + newPipes := make([]*Pipe, 0, len(currentPipes)+len(commit.Parents())) // 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 @@ -137,7 +137,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod if commit.IsFirstCommit() { toHash = models.EmptyTreeCommitHash } else { - toHash = commit.Parents[0] + toHash = commit.Parents()[0] } newPipes = append(newPipes, &Pipe{ fromPos: pos, @@ -216,7 +216,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } if commit.IsMerge() { - for _, parent := range commit.Parents[1:] { + for _, parent := range commit.Parents()[1:] { availablePos := getNextAvailablePosForNewPipe() // need to act as if continuing pipes are going to continue on the same line. newPipes = append(newPipes, &Pipe{ diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 63d8cb04932..17914d8d558 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -576,7 +576,7 @@ 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 + parentHashes := currentCommit.Parents() for j := 0; j < parentCount; j++ { reuseParent := rnd.Intn(6) != 1 && j <= len(pool)-1 && j != 0 var newParent *models.Commit diff --git a/pkg/gui/services/custom_commands/session_state_loader.go b/pkg/gui/services/custom_commands/session_state_loader.go index b90ec185a01..87465ec1a14 100644 --- a/pkg/gui/services/custom_commands/session_state_loader.go +++ b/pkg/gui/services/custom_commands/session_state_loader.go @@ -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(), } } From 722cc855089deb48a86ee0d8ef5c6980c61ad00b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Apr 2025 17:34:53 +0200 Subject: [PATCH 11/17] Store Commit.Parents in the pool too We need to pass %P instead of %p in the format string of the git log command, so that the parent hashes have the full length and can be shared with the real hashes. --- pkg/commands/git_commands/commit_loader.go | 2 +- pkg/commands/git_commands/commit_loader_test.go | 14 +++++++------- pkg/commands/git_commands/reflog_commit_loader.go | 2 +- .../git_commands/reflog_commit_loader_test.go | 12 ++++++------ pkg/commands/models/commit.go | 7 ++++--- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 3878dd95adc..6b1dfacdf3b 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -610,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 8658e25c47b..69d0de0173a 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -44,7 +44,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), + ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, expectedError: nil, @@ -55,7 +55,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), + ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, expectedError: nil, @@ -69,7 +69,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing which commits are yet to be pushed ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). // here it's actually getting all the commits in a formatted form, one per line - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil). + ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil). // here it's testing which of the configured main branches have an upstream ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil). // this one does ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")). // this one doesn't, so it checks origin instead @@ -205,7 +205,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing which commits are yet to be pushed ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). // here it's actually getting all the commits in a formatted form, one per line - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). + ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist; neither does ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "", errors.New("error")). ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/master"}, "", errors.New("error")). @@ -241,7 +241,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing which commits are yet to be pushed ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). // here it's actually getting all the commits in a formatted form, one per line - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). + ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil). ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")). @@ -276,7 +276,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), + ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, expectedError: nil, @@ -287,7 +287,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil), + ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%P%x00%m%x00%s", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, expectedError: nil, diff --git a/pkg/commands/git_commands/reflog_commit_loader.go b/pkg/commands/git_commands/reflog_commit_loader.go index 10453a1e628..1e981b1e126 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -31,7 +31,7 @@ func (self *ReflogCommitLoader) GetReflogCommits(hashPool *utils.StringPool, las 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() diff --git a/pkg/commands/git_commands/reflog_commit_loader_test.go b/pkg/commands/git_commands/reflog_commit_loader_test.go index 9aa8536f54a..3a90688a2b5 100644 --- a/pkg/commands/git_commands/reflog_commit_loader_test.go +++ b/pkg/commands/git_commands/reflog_commit_loader_test.go @@ -38,7 +38,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "no reflog entries", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p"}, "", nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, "", nil), lastReflogCommit: nil, expectedCommitOpts: []models.NewCommitOpts{}, @@ -48,7 +48,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "some reflog entries", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p"}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), lastReflogCommit: nil, expectedCommitOpts: []models.NewCommitOpts{ @@ -94,7 +94,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "some reflog entries where last commit is given", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p"}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, reflogOutput, nil), lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", @@ -118,7 +118,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "when passing filterPath", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p", "--follow", "--", "path"}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P", "--follow", "--", "path"}, reflogOutput, nil), lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", @@ -143,7 +143,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "when passing filterAuthor", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p", "--author=John Doe "}, reflogOutput, nil), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P", "--author=John Doe "}, reflogOutput, nil), lastReflogCommit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "c3c4b66b64c97ffeecde", @@ -168,7 +168,7 @@ func TestGetReflogCommits(t *testing.T) { { testName: "when command returns error", runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%p"}, "", errors.New("haha")), + ExpectGitArgs([]string{"-c", "log.showSignature=false", "log", "-g", "--abbrev=40", "--format=%h%x00%ct%x00%gs%x00%P"}, "", errors.New("haha")), lastReflogCommit: nil, filterPath: "", diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index 1e2191e92f6..638153ed806 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/stefanhaller/git-todo-parser/todo" ) @@ -54,7 +55,7 @@ type Commit struct { 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 } type NewCommitOpts struct { @@ -83,7 +84,7 @@ func NewCommit(hashPool *utils.StringPool, opts NewCommitOpts) *Commit { AuthorEmail: opts.AuthorEmail, UnixTimestamp: opts.UnixTimestamp, Divergence: opts.Divergence, - parents: opts.Parents, + parents: lo.Map(opts.Parents, func(s string, _ int) *string { return hashPool.Add(s) }), } } @@ -115,7 +116,7 @@ func (c *Commit) ParentRefName() string { } func (c *Commit) Parents() []string { - return c.parents + return lo.Map(c.parents, func(s *string, _ int) string { return *s }) } func (c *Commit) IsFirstCommit() bool { From 8d834e2eabdb7b846f451b39cceaf3383eb121b5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Apr 2025 18:26:08 +0200 Subject: [PATCH 12/17] Pack the models.Commit struct a little tighter Change the base type of some of our enums from int to uint8, and reorder fields for better packing. This reduces the size of models.Commit from 152 to 132 bytes on my machine. This doesn't improve overall memory usage significantly, but why not save a little bit of memory if it's easy. --- pkg/commands/models/commit.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index 638153ed806..37a9be28d05 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -11,7 +11,7 @@ import ( // Special commit hash for empty tree object const EmptyTreeCommitHash = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" -type CommitStatus int +type CommitStatus uint8 const ( StatusNone CommitStatus = iota @@ -30,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 @@ -45,17 +45,18 @@ const ( type Commit struct { 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 + + Status CommitStatus + Action todo.TodoCommand + Divergence Divergence // set to DivergenceNone unless we are showing the divergence view } type NewCommitOpts struct { From 13c21365c00bd19c94fe8676f31eb348afa6b945 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 15 Apr 2025 14:57:40 +0200 Subject: [PATCH 13/17] Store fromHash/toHash in Pipe struct as pointers Now that commit hashes are stored in a pool and referenced by pointer by the commits, we can use those same pointers in the pipes. --- pkg/commands/models/commit.go | 8 ++ pkg/gui/context/local_commits_context.go | 6 +- pkg/gui/context/sub_commits_context.go | 6 +- pkg/gui/presentation/commits.go | 8 +- pkg/gui/presentation/commits_test.go | 4 +- pkg/gui/presentation/graph/graph.go | 60 +++++----- pkg/gui/presentation/graph/graph_test.go | 146 ++++++++++++----------- 7 files changed, 126 insertions(+), 112 deletions(-) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index 37a9be28d05..eca66b3ac2c 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -93,6 +93,10 @@ 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()) } @@ -120,6 +124,10 @@ 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 } diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index dfb270d1e0f..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), diff --git a/pkg/gui/context/sub_commits_context.go b/pkg/gui/context/sub_commits_context.go index 120e605b3b7..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), diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index ec4a8425d25..7e6e0e838df 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -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 { diff --git a/pkg/gui/presentation/commits_test.go b/pkg/gui/presentation/commits_test.go index 5729e819ed5..595c3d75071 100644 --- a/pkg/gui/presentation/commits_test.go +++ b/pkg/gui/presentation/commits_test.go @@ -36,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 @@ -563,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/graph/graph.go b/pkg/gui/presentation/graph/graph.go index de46193ed88..80f6188db84 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -25,13 +25,17 @@ const ( type Pipe struct { fromPos int toPos int - fromHash string - toHash string + fromHash *string + toHash *string kind PipeKind style style.TextStyle } -var highlightStyle = style.FgLightWhite.SetBold() +var ( + highlightStyle = style.FgLightWhite.SetBold() + EmptyTreeCommitHash = models.EmptyTreeCommitHash + StartCommitHash = "START" +) func (self Pipe) left() int { return min(self.fromPos, self.toPos) @@ -41,13 +45,13 @@ func (self Pipe) right() int { 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 } @@ -57,7 +61,7 @@ func GetPipeSets(commits []*models.Commit, getStyle func(c *models.Commit) style 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 { pipes = getNextPipes(pipes, commit, getStyle) @@ -65,7 +69,7 @@ func GetPipeSets(commits []*models.Commit, getStyle func(c *models.Commit) style }) } -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 @@ -89,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 @@ -116,12 +120,12 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod 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 @@ -133,16 +137,16 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod // a traversed spot is one where a current pipe is starting on, ending on, or passing through traversedSpots := set.New[int]() - var toHash string + var toHash *string if commit.IsFirstCommit() { - toHash = models.EmptyTreeCommitHash + toHash = &EmptyTreeCommitHash } else { - toHash = commit.Parents()[0] + toHash = commit.ParentPtrs()[0] } newPipes = append(newPipes, &Pipe{ fromPos: pos, toPos: pos, - fromHash: commit.Hash(), + fromHash: commit.HashPtr(), toHash: toHash, kind: STARTS, style: getStyle(commit), @@ -150,7 +154,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod traversedSpotsForContinuingPipes := set.New[int]() for _, pipe := range currentPipes { - if !equalHashes(pipe.toHash, commit.Hash()) { + if !equalHashes(pipe.toHash, commit.HashPtr()) { traversedSpotsForContinuingPipes.Add(pipe.toPos) } } @@ -189,7 +193,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } for _, pipe := range currentPipes { - if equalHashes(pipe.toHash, commit.Hash()) { + if equalHashes(pipe.toHash, commit.HashPtr()) { // terminating here newPipes = append(newPipes, &Pipe{ fromPos: pipe.toPos, @@ -216,13 +220,13 @@ 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{ fromPos: pos, toPos: availablePos, - fromHash: commit.Hash(), + fromHash: commit.HashPtr(), toHash: parent, kind: STARTS, style: getStyle(commit), @@ -233,7 +237,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } 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-- { @@ -268,7 +272,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod func renderPipeSet( pipes []*Pipe, - selectedCommitHash string, + selectedCommitHashPtr *string, prevCommit *models.Commit, ) string { maxPos := 0 @@ -315,10 +319,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 } } @@ -327,7 +331,7 @@ 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) + return highlight && equalHashes(pipe.fromHash, selectedCommitHashPtr) }) for _, pipe := range nonSelectedPipes { @@ -370,13 +374,13 @@ 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)) + 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] + return (*a)[:length] == (*b)[:length] } diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 17914d8d558..25d529bbcd3 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -224,7 +224,7 @@ func TestRenderCommitGraph(t *testing.T) { 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, "blah", getStyle) + lines := RenderCommitGraph(commits, hashPool.Add("blah"), getStyle) trimmedExpectedOutput := "" for _, line := range strings.Split(strings.TrimPrefix(test.expectedOutput, "\n"), "\n") { @@ -257,6 +257,7 @@ func TestRenderPipeSet(t *testing.T) { nothing := style.Nothing hashPool := &utils.StringPool{} + pool := func(s string) *string { return hashPool.Add(s) } tests := []struct { name string @@ -269,8 +270,8 @@ 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", @@ -279,8 +280,8 @@ func TestRenderPipeSet(t *testing.T) { { 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", @@ -289,10 +290,10 @@ func TestRenderPipeSet(t *testing.T) { { 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─â•Ū", @@ -303,10 +304,10 @@ 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─│", @@ -317,11 +318,11 @@ 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}, + {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: "âĢ─│─┮─â•Ŋ", @@ -332,11 +333,11 @@ 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}, + {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: "âĢ───â•Ū â•Ŋ", @@ -347,10 +348,10 @@ 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "â—Ŋ─â”ī─â•Ŋ", @@ -361,11 +362,11 @@ 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}, + {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: "âĢ─│─│─â•Ū", @@ -376,11 +377,11 @@ 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}, + {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: "âĢ─│─â•Ŋ", @@ -391,11 +392,11 @@ 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}, + {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: "âĢ─┮─│─â•Ŋ", @@ -406,8 +407,8 @@ 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ", @@ -418,8 +419,8 @@ 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │", @@ -430,9 +431,9 @@ 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │ │", @@ -443,10 +444,10 @@ 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "âĢ─â•Ŋ", @@ -461,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 := "" @@ -482,6 +483,7 @@ 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 @@ -490,43 +492,43 @@ func TestGetNextPipes(t *testing.T) { }{ { prevPipes: []*Pipe{ - {fromPos: 0, toPos: 0, fromHash: "a", toHash: "b", kind: STARTS, style: style.FgDefault}, + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: STARTS, style: style.FgDefault}, }, 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}, + {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}, + {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.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}, + {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}, + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("root"), kind: TERMINATES, style: style.FgDefault}, }, 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}, + {fromPos: 1, toPos: 1, fromHash: pool("root"), toHash: pool(models.EmptyTreeCommitHash), kind: STARTS, style: style.FgDefault}, }, }, } @@ -538,8 +540,8 @@ func TestGetNextPipes(t *testing.T) { 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:") @@ -552,19 +554,19 @@ func BenchmarkRenderCommitGraph(b *testing.B) { oldColorLevel := color.ForceSetColorLevel(terminfo.ColorLevelMillions) defer color.ForceSetColorLevel(oldColorLevel) - commits := generateCommits(50) + hashPool := &utils.StringPool{} + + commits := generateCommits(hashPool, 50) getStyle := func(commit *models.Commit) style.TextStyle { return authors.AuthorStyle(commit.AuthorName) } b.ResetTimer() for b.Loop() { - RenderCommitGraph(commits, "selected", getStyle) + RenderCommitGraph(commits, hashPool.Add("selected"), getStyle) } } -func generateCommits(count int) []*models.Commit { - hashPool := &utils.StringPool{} - +func generateCommits(hashPool *utils.StringPool, count int) []*models.Commit { rnd := rand.New(rand.NewSource(1234)) pool := []*models.Commit{models.NewCommit(hashPool, models.NewCommitOpts{Hash: "a", AuthorName: "A"})} commits := make([]*models.Commit, 0, count) From 18e5b0a650e0b2a42e2fe671711cf55e88b05fd1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Apr 2025 18:16:59 +0200 Subject: [PATCH 14/17] Simplify equalHashes Now that all hashes that we deal with are stored in the same pool, we can simply compare their addresses. --- pkg/gui/presentation/graph/graph.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index 80f6188db84..5c5240fdd03 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -380,7 +380,6 @@ func equalHashes(a, b *string) bool { 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 } From 28aa26f30a466e5f7638407973c3998f5304f188 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 16 Apr 2025 20:22:39 +0200 Subject: [PATCH 15/17] Store Pipe objects by value in slice of Pipes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This saves some memory at the cost of a slight performance increase (I suppose reallocting the slice when adding new Pipes is slightly more expensive now). Performance of the BenchmarkRenderCommitGraph benchmark is 130Ξs before, 175Ξs after. I'm guessing this is still acceptable. --- pkg/gui/presentation/commits.go | 4 +-- pkg/gui/presentation/graph/graph.go | 36 +++++++++---------- pkg/gui/presentation/graph/graph_test.go | 46 ++++++++++++------------ 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 7e6e0e838df..0e1530415ba 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 ) @@ -245,7 +245,7 @@ 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{ diff --git a/pkg/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index 5c5240fdd03..b769e6db5f7 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -56,20 +56,20 @@ func RenderCommitGraph(commits []*models.Commit, selectedCommitHashPtr *string, 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: &StartCommitHash, toHash: commits[0].HashPtr(), 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, selectedCommitHashPtr *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 @@ -106,7 +106,7 @@ func RenderAux(pipeSets [][]*Pipe, commits []*models.Commit, selectedCommitHashP return lo.Flatten(chunks) } -func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *models.Commit) style.TextStyle) []*Pipe { +func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *models.Commit) style.TextStyle) []Pipe { maxPos := 0 for _, pipe := range prevPipes { if pipe.toPos > maxPos { @@ -116,11 +116,11 @@ 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.ParentPtrs())) + 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 @@ -143,7 +143,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } else { toHash = commit.ParentPtrs()[0] } - newPipes = append(newPipes, &Pipe{ + newPipes = append(newPipes, Pipe{ fromPos: pos, toPos: pos, fromHash: commit.HashPtr(), @@ -195,7 +195,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod for _, pipe := range currentPipes { if equalHashes(pipe.toHash, commit.HashPtr()) { // terminating here - newPipes = append(newPipes, &Pipe{ + newPipes = append(newPipes, Pipe{ fromPos: pipe.toPos, toPos: pos, fromHash: pipe.fromHash, @@ -207,7 +207,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, @@ -223,7 +223,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod 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.HashPtr(), @@ -247,7 +247,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod last = i } } - newPipes = append(newPipes, &Pipe{ + newPipes = append(newPipes, Pipe{ fromPos: pipe.toPos, toPos: last, fromHash: pipe.fromHash, @@ -260,7 +260,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) } @@ -271,7 +271,7 @@ func getNextPipes(prevPipes []*Pipe, commit *models.Commit, getStyle func(c *mod } func renderPipeSet( - pipes []*Pipe, + pipes []Pipe, selectedCommitHashPtr *string, prevCommit *models.Commit, ) string { @@ -330,19 +330,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 { + 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) } } @@ -352,7 +352,7 @@ func renderPipeSet( } } for _, pipe := range selectedPipes { - renderPipe(pipe, highlightStyle, true) + renderPipe(&pipe, highlightStyle, true) if pipe.toPos == commitPos { cells[pipe.toPos].setStyle(highlightStyle) } diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 25d529bbcd3..56ae2cb95a4 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -261,7 +261,7 @@ func TestRenderPipeSet(t *testing.T) { tests := []struct { name string - pipes []*Pipe + pipes []Pipe commit *models.Commit prevCommit *models.Commit expectedStr string @@ -269,7 +269,7 @@ func TestRenderPipeSet(t *testing.T) { }{ { name: "single cell", - pipes: []*Pipe{ + 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}, }, @@ -279,7 +279,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "single cell, selected", - pipes: []*Pipe{ + 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}, }, @@ -289,7 +289,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "terminating hook and starting hook, selected", - pipes: []*Pipe{ + 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}, @@ -303,7 +303,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "terminating hook and starting hook, prioritise the terminating one", - pipes: []*Pipe{ + 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}, @@ -317,7 +317,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting and terminating pipe sharing some space", - pipes: []*Pipe{ + 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}, @@ -332,7 +332,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting and terminating pipe sharing some space, with selection", - pipes: []*Pipe{ + 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}, @@ -347,7 +347,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "many terminating pipes", - pipes: []*Pipe{ + 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}, @@ -361,7 +361,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting pipe passing through", - pipes: []*Pipe{ + 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}, @@ -376,7 +376,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "starting and terminating path crossing continuing path", - pipes: []*Pipe{ + 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}, @@ -391,7 +391,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "another clash of starting and terminating paths", - pipes: []*Pipe{ + 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}, @@ -406,7 +406,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "commit whose previous commit is selected", - pipes: []*Pipe{ + 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}, }, @@ -418,7 +418,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "commit whose previous commit is selected and is a merge commit", - pipes: []*Pipe{ + 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}, }, @@ -430,7 +430,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "commit whose previous commit is selected and is a merge commit, with continuing pipe inbetween", - pipes: []*Pipe{ + 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}, @@ -443,7 +443,7 @@ func TestRenderPipeSet(t *testing.T) { }, { name: "when previous commit is selected, not a merge commit, and spawns a continuing pipe", - pipes: []*Pipe{ + 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}, @@ -486,25 +486,25 @@ func TestGetNextPipes(t *testing.T) { pool := func(s string) *string { return hashPool.Add(s) } tests := []struct { - prevPipes []*Pipe + prevPipes []Pipe commit *models.Commit - expected []*Pipe + expected []Pipe }{ { - prevPipes: []*Pipe{ + prevPipes: []Pipe{ {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: STARTS, style: style.FgDefault}, }, commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "b", Parents: []string{"c"}, }), - expected: []*Pipe{ + 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{ + 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}, @@ -513,21 +513,21 @@ func TestGetNextPipes(t *testing.T) { Hash: "d", Parents: []string{"e"}, }), - expected: []*Pipe{ + 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{ + prevPipes: []Pipe{ {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("root"), kind: TERMINATES, style: style.FgDefault}, }, commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "root", Parents: []string{}, }), - expected: []*Pipe{ + expected: []Pipe{ {fromPos: 1, toPos: 1, fromHash: pool("root"), toHash: pool(models.EmptyTreeCommitHash), kind: STARTS, style: style.FgDefault}, }, }, From e63abf89db685aa2526839b5bf292fc6c38569d2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 18 Apr 2025 22:35:04 +0200 Subject: [PATCH 16/17] Store TextStyle in Pipe struct as pointer The instances are held by the AuthorStyle cache. --- pkg/gui/presentation/authors/authors.go | 8 +- pkg/gui/presentation/branches.go | 6 +- pkg/gui/presentation/commits.go | 2 +- pkg/gui/presentation/graph/cell.go | 18 ++-- pkg/gui/presentation/graph/graph.go | 18 ++-- pkg/gui/presentation/graph/graph_test.go | 132 +++++++++++------------ pkg/utils/color.go | 9 +- 7 files changed, 97 insertions(+), 96 deletions(-) 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 0e1530415ba..63c2b3c9b54 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -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) 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 b769e6db5f7..fdb58777253 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -28,7 +28,7 @@ type Pipe struct { fromHash *string toHash *string kind PipeKind - style style.TextStyle + style *style.TextStyle } var ( @@ -45,7 +45,7 @@ func (self Pipe) right() int { return max(self.fromPos, self.toPos) } -func RenderCommitGraph(commits []*models.Commit, selectedCommitHashPtr *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 @@ -56,12 +56,12 @@ func RenderCommitGraph(commits []*models.Commit, selectedCommitHashPtr *string, 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: &StartCommitHash, toHash: commits[0].HashPtr(), 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 { pipes = getNextPipes(pipes, commit, getStyle) @@ -106,7 +106,7 @@ func RenderAux(pipeSets [][]Pipe, commits []*models.Commit, selectedCommitHashPt return lo.Flatten(chunks) } -func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *models.Commit) style.TextStyle) []Pipe { +func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *models.Commit) *style.TextStyle) []Pipe { maxPos := 0 for _, pipe := range prevPipes { if pipe.toPos > maxPos { @@ -293,10 +293,10 @@ func renderPipeSet( isMerge := startCount > 1 cells := lo.Map(lo.Range(maxPos+1), func(i int, _ int) *Cell { - return &Cell{cellType: CONNECTION, style: style.FgDefault} + 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() @@ -352,9 +352,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) } } diff --git a/pkg/gui/presentation/graph/graph_test.go b/pkg/gui/presentation/graph/graph_test.go index 56ae2cb95a4..13709267e20 100644 --- a/pkg/gui/presentation/graph/graph_test.go +++ b/pkg/gui/presentation/graph/graph_test.go @@ -221,7 +221,7 @@ func TestRenderCommitGraph(t *testing.T) { t.Run(test.name, func(t *testing.T) { hashPool := &utils.StringPool{} - getStyle := func(c *models.Commit) style.TextStyle { return style.FgDefault } + 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) @@ -270,8 +270,8 @@ func TestRenderPipeSet(t *testing.T) { { name: "single cell", 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", @@ -280,8 +280,8 @@ func TestRenderPipeSet(t *testing.T) { { name: "single cell, selected", 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "â—Ŋ", @@ -290,10 +290,10 @@ func TestRenderPipeSet(t *testing.T) { { name: "terminating hook and starting hook, selected", 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─â•Ū", @@ -304,10 +304,10 @@ func TestRenderPipeSet(t *testing.T) { { name: "terminating hook and starting hook, prioritise the terminating one", 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a"}), expectedStr: "âĢ─│", @@ -318,11 +318,11 @@ func TestRenderPipeSet(t *testing.T) { { name: "starting and terminating pipe sharing some space", 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}, + {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: "âĢ─│─┮─â•Ŋ", @@ -333,11 +333,11 @@ func TestRenderPipeSet(t *testing.T) { { name: "starting and terminating pipe sharing some space, with selection", 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}, + {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: "âĢ───â•Ū â•Ŋ", @@ -348,10 +348,10 @@ func TestRenderPipeSet(t *testing.T) { { name: "many terminating pipes", 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "a1"}), expectedStr: "â—Ŋ─â”ī─â•Ŋ", @@ -362,11 +362,11 @@ func TestRenderPipeSet(t *testing.T) { { name: "starting pipe passing through", 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}, + {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: "âĢ─│─│─â•Ū", @@ -377,11 +377,11 @@ func TestRenderPipeSet(t *testing.T) { { name: "starting and terminating path crossing continuing path", 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}, + {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: "âĢ─│─â•Ŋ", @@ -392,11 +392,11 @@ func TestRenderPipeSet(t *testing.T) { { name: "another clash of starting and terminating paths", 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}, + {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: "âĢ─┮─│─â•Ŋ", @@ -407,8 +407,8 @@ func TestRenderPipeSet(t *testing.T) { { name: "commit whose previous commit is selected", 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ", @@ -419,8 +419,8 @@ func TestRenderPipeSet(t *testing.T) { { name: "commit whose previous commit is selected and is a merge commit", 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │", @@ -431,9 +431,9 @@ 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: 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "â—Ŋ │ │", @@ -444,10 +444,10 @@ 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: 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{Hash: "selected"}), expectedStr: "âĢ─â•Ŋ", @@ -492,43 +492,43 @@ func TestGetNextPipes(t *testing.T) { }{ { prevPipes: []Pipe{ - {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: STARTS, style: style.FgDefault}, + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("b"), kind: STARTS, style: &style.FgDefault}, }, commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "b", Parents: []string{"c"}, }), 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}, + {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: 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}, + {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.NewCommit(hashPool, models.NewCommitOpts{ Hash: "d", Parents: []string{"e"}, }), 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}, + {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: pool("a"), toHash: pool("root"), kind: TERMINATES, style: style.FgDefault}, + {fromPos: 0, toPos: 0, fromHash: pool("a"), toHash: pool("root"), kind: TERMINATES, style: &style.FgDefault}, }, commit: models.NewCommit(hashPool, models.NewCommitOpts{ Hash: "root", Parents: []string{}, }), expected: []Pipe{ - {fromPos: 1, toPos: 1, fromHash: pool("root"), toHash: pool(models.EmptyTreeCommitHash), kind: STARTS, style: style.FgDefault}, + {fromPos: 1, toPos: 1, fromHash: pool("root"), toHash: pool(models.EmptyTreeCommitHash), kind: STARTS, style: &style.FgDefault}, }, }, } @@ -537,7 +537,7 @@ 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, pool("selected"), nil) @@ -557,7 +557,7 @@ func BenchmarkRenderCommitGraph(b *testing.B) { hashPool := &utils.StringPool{} commits := generateCommits(hashPool, 50) - getStyle := func(commit *models.Commit) style.TextStyle { + getStyle := func(commit *models.Commit) *style.TextStyle { return authors.AuthorStyle(commit.AuthorName) } b.ResetTimer() 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 }) } From 6ca627d9d7ab503b90a375f8cb0e9fb3e3196346 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 19 Apr 2025 13:27:50 +0200 Subject: [PATCH 17/17] Store fromPos/toPos as 16-bit ints, and reorder fields for better packing Hopefully, graphs will never get wider than 32768 characters. (They would get kind of hard to navigate if they did...) This reduces the size of the Pipe struct from 48 to 32 bytes, which makes a significant difference when there are many millions of instances. --- pkg/gui/presentation/graph/graph.go | 45 +++++++++++++++-------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/pkg/gui/presentation/graph/graph.go b/pkg/gui/presentation/graph/graph.go index fdb58777253..4e204c03ff6 100644 --- a/pkg/gui/presentation/graph/graph.go +++ b/pkg/gui/presentation/graph/graph.go @@ -23,12 +23,12 @@ const ( ) type Pipe struct { - fromPos int - toPos int fromHash *string toHash *string - kind PipeKind style *style.TextStyle + fromPos int16 + toPos int16 + kind PipeKind } var ( @@ -37,11 +37,11 @@ var ( 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) } @@ -107,7 +107,7 @@ func RenderAux(pipeSets [][]Pipe, commits []*models.Commit, selectedCommitHashPt } func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *models.Commit) *style.TextStyle) []Pipe { - maxPos := 0 + maxPos := int16(0) for _, pipe := range prevPipes { if pipe.toPos > maxPos { maxPos = pipe.toPos @@ -133,6 +133,9 @@ func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *mode } // 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]() @@ -155,41 +158,41 @@ func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *mode traversedSpotsForContinuingPipes := set.New[int]() for _, pipe := range currentPipes { if !equalHashes(pipe.toHash, commit.HashPtr()) { - traversedSpotsForContinuingPipes.Add(pipe.toPos) + 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 { @@ -232,7 +235,7 @@ func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *mode style: getStyle(commit), }) - takenSpots.Add(availablePos) + takenSpots.Add(int(availablePos)) } } @@ -241,7 +244,7 @@ func getNextPipes(prevPipes []Pipe, commit *models.Commit, getStyle func(c *mode // 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 @@ -275,8 +278,8 @@ func renderPipeSet( 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 { @@ -292,7 +295,7 @@ func renderPipeSet( } isMerge := startCount > 1 - cells := lo.Map(lo.Range(maxPos+1), func(i int, _ int) *Cell { + cells := lo.Map(lo.Range(int(maxPos)+1), func(i int, _ int) *Cell { return &Cell{cellType: CONNECTION, style: &style.FgDefault} })