From cc0b5a2f7f26c92fe869b174b724fb66dd0931de Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 31 Jul 2025 10:42:46 +0200 Subject: [PATCH 1/4] Cleanup: remove the ignoringWarnings hack from GetMergeBase GetMergeBase is always called with a full ref, so it shouldn't need the ignoringWarnings hack (which is about ignoring warnings coming from ambiguous refs). Also, separate stdout and stderr, which would also have solved the problem. We no longer really need it now, but it's still cleaner. --- pkg/commands/git_commands/main_branches.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/git_commands/main_branches.go b/pkg/commands/git_commands/main_branches.go index 6bfc648db..109f9cfd8 100644 --- a/pkg/commands/git_commands/main_branches.go +++ b/pkg/commands/git_commands/main_branches.go @@ -69,11 +69,11 @@ func (self *MainBranches) GetMergeBase(refName string) string { // very rarely, users must quit and restart lazygit to fix it; the latter is // also not very common, but can totally happen and is not an error. - output, _ := self.cmd.New( + output, _, _ := self.cmd.New( NewGitCmd("merge-base").Arg(refName).Arg(mainBranches...). ToArgv(), - ).DontLog().RunWithOutput() - return ignoringWarnings(output) + ).DontLog().RunWithOutputs() + return strings.TrimSpace(output) } func (self *MainBranches) determineMainBranches(configuredMainBranches []string) []string { From 4b9921d0a4296f702ec968aaaafdab205197ff5d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 31 Jul 2025 11:20:05 +0200 Subject: [PATCH 2/4] Move the Ref interface from gui/types to models This is a type that can be useful for model/backend stuff, so move it there. We are going to use it in the API of the commit loader. --- pkg/{gui/types => commands/models}/ref.go | 7 +------ pkg/gui/context/branches_context.go | 2 +- pkg/gui/context/commit_files_context.go | 2 +- pkg/gui/context/local_commits_context.go | 2 +- pkg/gui/context/reflog_commits_context.go | 2 +- pkg/gui/context/remote_branches_context.go | 6 +++--- pkg/gui/context/stash_context.go | 2 +- pkg/gui/context/sub_commits_context.go | 8 ++++---- pkg/gui/context/tags_context.go | 2 +- pkg/gui/controllers/helpers/diff_helper.go | 2 +- pkg/gui/controllers/helpers/sub_commits_helper.go | 2 +- pkg/gui/controllers/switch_to_diff_files_controller.go | 3 ++- .../controllers/switch_to_sub_commits_controller.go | 7 ++++--- pkg/gui/filetree/commit_file_tree_view_model.go | 10 +++++----- pkg/gui/types/ref_range.go | 8 ++++++++ 15 files changed, 35 insertions(+), 30 deletions(-) rename pkg/{gui/types => commands/models}/ref.go (68%) create mode 100644 pkg/gui/types/ref_range.go diff --git a/pkg/gui/types/ref.go b/pkg/commands/models/ref.go similarity index 68% rename from pkg/gui/types/ref.go rename to pkg/commands/models/ref.go index 18fc7e998..6ef94ee6d 100644 --- a/pkg/gui/types/ref.go +++ b/pkg/commands/models/ref.go @@ -1,4 +1,4 @@ -package types +package models type Ref interface { FullRefName() string @@ -7,8 +7,3 @@ type Ref interface { ParentRefName() string Description() string } - -type RefRange struct { - From Ref - To Ref -} diff --git a/pkg/gui/context/branches_context.go b/pkg/gui/context/branches_context.go index 9e30e36cc..d73961275 100644 --- a/pkg/gui/context/branches_context.go +++ b/pkg/gui/context/branches_context.go @@ -59,7 +59,7 @@ func NewBranchesContext(c *ContextCommon) *BranchesContext { return self } -func (self *BranchesContext) GetSelectedRef() types.Ref { +func (self *BranchesContext) GetSelectedRef() models.Ref { branch := self.GetSelected() if branch == nil { return nil diff --git a/pkg/gui/context/commit_files_context.go b/pkg/gui/context/commit_files_context.go index d0a5b9f7e..52807faa9 100644 --- a/pkg/gui/context/commit_files_context.go +++ b/pkg/gui/context/commit_files_context.go @@ -96,7 +96,7 @@ func (self *CommitFilesContext) ModelSearchResults(searchStr string, caseSensiti return nil } -func (self *CommitFilesContext) ReInit(ref types.Ref, refRange *types.RefRange) { +func (self *CommitFilesContext) ReInit(ref models.Ref, refRange *types.RefRange) { self.SetRef(ref) self.SetRefRange(refRange) if refRange != nil { diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index c11770986..e873663c3 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -164,7 +164,7 @@ func (self *LocalCommitsContext) CanRebase() bool { return true } -func (self *LocalCommitsContext) GetSelectedRef() types.Ref { +func (self *LocalCommitsContext) GetSelectedRef() models.Ref { commit := self.GetSelected() if commit == nil { return nil diff --git a/pkg/gui/context/reflog_commits_context.go b/pkg/gui/context/reflog_commits_context.go index 518edeefa..1a882bf12 100644 --- a/pkg/gui/context/reflog_commits_context.go +++ b/pkg/gui/context/reflog_commits_context.go @@ -63,7 +63,7 @@ func (self *ReflogCommitsContext) CanRebase() bool { return false } -func (self *ReflogCommitsContext) GetSelectedRef() types.Ref { +func (self *ReflogCommitsContext) GetSelectedRef() models.Ref { commit := self.GetSelected() if commit == nil { return nil diff --git a/pkg/gui/context/remote_branches_context.go b/pkg/gui/context/remote_branches_context.go index 9e9b00eb1..65f156f25 100644 --- a/pkg/gui/context/remote_branches_context.go +++ b/pkg/gui/context/remote_branches_context.go @@ -54,7 +54,7 @@ func NewRemoteBranchesContext( } } -func (self *RemoteBranchesContext) GetSelectedRef() types.Ref { +func (self *RemoteBranchesContext) GetSelectedRef() models.Ref { remoteBranch := self.GetSelected() if remoteBranch == nil { return nil @@ -62,10 +62,10 @@ func (self *RemoteBranchesContext) GetSelectedRef() types.Ref { return remoteBranch } -func (self *RemoteBranchesContext) GetSelectedRefs() ([]types.Ref, int, int) { +func (self *RemoteBranchesContext) GetSelectedRefs() ([]models.Ref, int, int) { items, startIdx, endIdx := self.GetSelectedItems() - refs := lo.Map(items, func(item *models.RemoteBranch, _ int) types.Ref { + refs := lo.Map(items, func(item *models.RemoteBranch, _ int) models.Ref { return item }) diff --git a/pkg/gui/context/stash_context.go b/pkg/gui/context/stash_context.go index 7dc0066d9..2014de9f3 100644 --- a/pkg/gui/context/stash_context.go +++ b/pkg/gui/context/stash_context.go @@ -53,7 +53,7 @@ func (self *StashContext) CanRebase() bool { return false } -func (self *StashContext) GetSelectedRef() types.Ref { +func (self *StashContext) GetSelectedRef() models.Ref { stash := self.GetSelected() if stash == nil { return nil diff --git a/pkg/gui/context/sub_commits_context.go b/pkg/gui/context/sub_commits_context.go index e489511a0..1e084077b 100644 --- a/pkg/gui/context/sub_commits_context.go +++ b/pkg/gui/context/sub_commits_context.go @@ -141,7 +141,7 @@ func NewSubCommitsContext( type SubCommitsViewModel struct { // name of the ref that the sub-commits are shown for - ref types.Ref + ref models.Ref refToShowDivergenceFrom string *ListViewModel[*models.Commit] @@ -149,11 +149,11 @@ type SubCommitsViewModel struct { showBranchHeads bool } -func (self *SubCommitsViewModel) SetRef(ref types.Ref) { +func (self *SubCommitsViewModel) SetRef(ref models.Ref) { self.ref = ref } -func (self *SubCommitsViewModel) GetRef() types.Ref { +func (self *SubCommitsViewModel) GetRef() models.Ref { return self.ref } @@ -177,7 +177,7 @@ func (self *SubCommitsContext) CanRebase() bool { return false } -func (self *SubCommitsContext) GetSelectedRef() types.Ref { +func (self *SubCommitsContext) GetSelectedRef() models.Ref { commit := self.GetSelected() if commit == nil { return nil diff --git a/pkg/gui/context/tags_context.go b/pkg/gui/context/tags_context.go index c77a5292a..0b98b146a 100644 --- a/pkg/gui/context/tags_context.go +++ b/pkg/gui/context/tags_context.go @@ -52,7 +52,7 @@ func NewTagsContext( } } -func (self *TagsContext) GetSelectedRef() types.Ref { +func (self *TagsContext) GetSelectedRef() models.Ref { tag := self.GetSelected() if tag == nil { return nil diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index 1fd73081a..668ee916a 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -174,7 +174,7 @@ func (self *DiffHelper) IgnoringWhitespaceSubTitle() string { return "" } -func (self *DiffHelper) OpenDiffToolForRef(selectedRef types.Ref) error { +func (self *DiffHelper) OpenDiffToolForRef(selectedRef models.Ref) error { to := selectedRef.RefName() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff("") _, err := self.c.RunSubprocess(self.c.Git().Diff.OpenDiffToolCmdObj( diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index e61c4fee6..c9561a918 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/pkg/gui/controllers/helpers/sub_commits_helper.go @@ -24,7 +24,7 @@ func NewSubCommitsHelper( } type ViewSubCommitsOpts struct { - Ref types.Ref + Ref models.Ref RefToShowDivergenceFrom string TitleRef string Context types.Context diff --git a/pkg/gui/controllers/switch_to_diff_files_controller.go b/pkg/gui/controllers/switch_to_diff_files_controller.go index 387da2427..f704f8bd4 100644 --- a/pkg/gui/controllers/switch_to_diff_files_controller.go +++ b/pkg/gui/controllers/switch_to_diff_files_controller.go @@ -1,6 +1,7 @@ package controllers import ( + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -11,7 +12,7 @@ var _ types.IController = &SwitchToDiffFilesController{} type CanSwitchToDiffFiles interface { types.IListContext CanRebase() bool - GetSelectedRef() types.Ref + GetSelectedRef() models.Ref GetSelectedRefRangeForDiffFiles() *types.RefRange } diff --git a/pkg/gui/controllers/switch_to_sub_commits_controller.go b/pkg/gui/controllers/switch_to_sub_commits_controller.go index 3df7ddee8..9257e33d3 100644 --- a/pkg/gui/controllers/switch_to_sub_commits_controller.go +++ b/pkg/gui/controllers/switch_to_sub_commits_controller.go @@ -1,6 +1,7 @@ package controllers import ( + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -9,7 +10,7 @@ var _ types.IController = &SwitchToSubCommitsController{} type CanSwitchToSubCommits interface { types.IListContext - GetSelectedRef() types.Ref + GetSelectedRef() models.Ref ShowBranchHeadsInSubCommits() bool } @@ -17,7 +18,7 @@ type CanSwitchToSubCommits interface { // but an attribute on it i.e. the ref of an item. type SwitchToSubCommitsController struct { baseController - *ListControllerTrait[types.Ref] + *ListControllerTrait[models.Ref] c *ControllerCommon context CanSwitchToSubCommits } @@ -32,7 +33,7 @@ func NewSwitchToSubCommitsController( c, context, context.GetSelectedRef, - func() ([]types.Ref, int, int) { + func() ([]models.Ref, int, int) { panic("Not implemented") }, ), diff --git a/pkg/gui/filetree/commit_file_tree_view_model.go b/pkg/gui/filetree/commit_file_tree_view_model.go index c857a6a62..82c478a3e 100644 --- a/pkg/gui/filetree/commit_file_tree_view_model.go +++ b/pkg/gui/filetree/commit_file_tree_view_model.go @@ -15,8 +15,8 @@ type ICommitFileTreeViewModel interface { ICommitFileTree types.IListCursor - GetRef() types.Ref - SetRef(types.Ref) + GetRef() models.Ref + SetRef(models.Ref) GetRefRange() *types.RefRange // can be nil, in which case GetRef should be used SetRefRange(*types.RefRange) // should be set to nil when selection is not a range GetCanRebase() bool @@ -30,7 +30,7 @@ type CommitFileTreeViewModel struct { // this is e.g. the commit for which we're viewing the files, if there is no // range selection, or if the range selection can't be used for some reason - ref types.Ref + ref models.Ref // this is a commit range for which we're viewing the files. Can be nil, in // which case ref is used. @@ -55,11 +55,11 @@ func NewCommitFileTreeViewModel(getFiles func() []*models.CommitFile, common *co } } -func (self *CommitFileTreeViewModel) GetRef() types.Ref { +func (self *CommitFileTreeViewModel) GetRef() models.Ref { return self.ref } -func (self *CommitFileTreeViewModel) SetRef(ref types.Ref) { +func (self *CommitFileTreeViewModel) SetRef(ref models.Ref) { self.ref = ref } diff --git a/pkg/gui/types/ref_range.go b/pkg/gui/types/ref_range.go new file mode 100644 index 000000000..437b1aafd --- /dev/null +++ b/pkg/gui/types/ref_range.go @@ -0,0 +1,8 @@ +package types + +import "github.com/jesseduffield/lazygit/pkg/commands/models" + +type RefRange struct { + From models.Ref + To models.Ref +} From 20517330b4cdb3ccd3cf2a54dd1ff8b386afced8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 31 Jul 2025 11:54:52 +0200 Subject: [PATCH 3/4] Change GetCommitsOptions.RefForPushedStatus to a models.Ref This makes it easier to use the full ref in the git merge-base call, which avoids ambiguities when there's a tag with the same name as the current branch. This fixes a hash coloring bug in the local commits panel when there's a tag with the same name as the checked out branch; in this case all commit hashes that should be yellow were painted as red. --- pkg/commands/git_commands/commit_loader.go | 16 +++++++----- .../git_commands/commit_loader_test.go | 26 +++++++++---------- pkg/gui/controllers/helpers/refresh_helper.go | 24 ++++++++++------- .../controllers/helpers/sub_commits_helper.go | 2 +- 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index e2d546c9a..1d4314c5b 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -59,8 +59,8 @@ type GetCommitsOptions struct { FilterPath string FilterAuthor string IncludeRebaseCommits bool - RefName string // e.g. "HEAD" or "my_branch" - RefForPushedStatus string // the ref to use for determining pushed/unpushed status + RefName string // e.g. "HEAD" or "my_branch" + RefForPushedStatus models.Ref // the ref to use for determining pushed/unpushed status // determines if we show the whole git graph i.e. pass the '--all' flag All bool // If non-empty, show divergence from this ref (left-right log) @@ -112,7 +112,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, passedFirstPushedCommit := false // I can get this before firstPushedCommit, err := self.getFirstPushedCommit(opts.RefForPushedStatus) - if err != nil { + if err != nil || firstPushedCommit == "" { // must have no upstream branch so we'll consider everything as pushed passedFirstPushedCommit = true } @@ -581,11 +581,15 @@ func ignoringWarnings(commandOutput string) string { // getFirstPushedCommit returns the first commit hash which has been pushed to the ref's upstream. // all commits above this are deemed unpushed and marked as such. -func (self *CommitLoader) getFirstPushedCommit(refName string) (string, error) { +func (self *CommitLoader) getFirstPushedCommit(ref models.Ref) (string, error) { + if ref == nil { + return "", nil + } + output, err := self.cmd.New( NewGitCmd("merge-base"). - Arg(refName). - Arg(strings.TrimPrefix(refName, "refs/heads/") + "@{u}"). + Arg(ref.FullRefName()). + Arg(ref.RefName() + "@{u}"). ToArgv(), ). DontLog(). diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 087ac1c7e..cbf170e5b 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -41,9 +41,9 @@ func TestGetCommits(t *testing.T) { { testName: "should return no commits if there are none", logOrder: "topo-order", - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -52,7 +52,7 @@ func TestGetCommits(t *testing.T) { { testName: "should use proper upstream name for branch", logOrder: "topo-order", - opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false}, + opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: &models.Branch{Name: "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%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), @@ -63,11 +63,11 @@ func TestGetCommits(t *testing.T) { { testName: "should return commits if they are present", logOrder: "topo-order", - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, mainBranches: []string{"master", "main", "develop"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"merge-base", "refs/heads/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%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil). // here it's testing which of the configured main branches have an upstream @@ -199,11 +199,11 @@ func TestGetCommits(t *testing.T) { { testName: "should not call merge-base for mainBranches if none exist", logOrder: "topo-order", - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, mainBranches: []string{"master", "main"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"merge-base", "refs/heads/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%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist; neither does @@ -235,11 +235,11 @@ func TestGetCommits(t *testing.T) { { testName: "should call merge-base for all main branches that exist", logOrder: "topo-order", - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, mainBranches: []string{"master", "main", "develop", "1.0-hotfixes"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"merge-base", "refs/heads/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%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist @@ -273,9 +273,9 @@ func TestGetCommits(t *testing.T) { { testName: "should not specify order if `log.order` is `default`", logOrder: "default", - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -284,9 +284,9 @@ func TestGetCommits(t *testing.T) { { testName: "should set filter path", logOrder: "default", - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"}, + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, FilterPath: "src"}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--follow", "--name-status", "--no-show-signature", "--", "src"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 24958c65d..8ebc76d16 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -288,37 +288,37 @@ func (self *RefreshHelper) refreshCommitsAndCommitFiles() { } } -func (self *RefreshHelper) determineCheckedOutBranchName() string { +func (self *RefreshHelper) determineCheckedOutRef() models.Ref { if rebasedBranch := self.c.Git().Status.BranchBeingRebased(); rebasedBranch != "" { // During a rebase we're on a detached head, so cannot determine the // branch name in the usual way. We need to read it from the // ".git/rebase-merge/head-name" file instead. - return strings.TrimPrefix(rebasedBranch, "refs/heads/") + return &models.Branch{Name: strings.TrimPrefix(rebasedBranch, "refs/heads/")} } if bisectInfo := self.c.Git().Bisect.GetInfo(); bisectInfo.Bisecting() && bisectInfo.GetStartHash() != "" { // Likewise, when we're bisecting we're on a detached head as well. In // this case we read the branch name from the ".git/BISECT_START" file. - return bisectInfo.GetStartHash() + return &models.Branch{Name: bisectInfo.GetStartHash()} } // In all other cases, get the branch name by asking git what branch is // checked out. Note that if we're on a detached head (for reasons other // than rebasing or bisecting, i.e. it was explicitly checked out), then // this will return an empty string. - if branchName, err := self.c.Git().Branch.CurrentBranchName(); err == nil { - return branchName + if branchName, err := self.c.Git().Branch.CurrentBranchName(); err == nil && branchName != "" { + return &models.Branch{Name: branchName} } // Should never get here unless the working copy is corrupt - return "" + return nil } func (self *RefreshHelper) refreshCommitsWithLimit() error { self.c.Mutexes().LocalCommitsMutex.Lock() defer self.c.Mutexes().LocalCommitsMutex.Unlock() - checkedOutBranchName := self.determineCheckedOutBranchName() + checkedOutRef := self.determineCheckedOutRef() commits, err := self.c.Git().Loaders.CommitLoader.GetCommits( git_commands.GetCommitsOptions{ Limit: self.c.Contexts().LocalCommits.GetLimitCommits(), @@ -326,7 +326,7 @@ func (self *RefreshHelper) refreshCommitsWithLimit() error { FilterAuthor: self.c.Modes().Filtering.GetAuthor(), IncludeRebaseCommits: true, RefName: self.refForLog(), - RefForPushedStatus: checkedOutBranchName, + RefForPushedStatus: checkedOutRef, All: self.c.Contexts().LocalCommits.GetShowWholeGitGraph(), MainBranches: self.c.Model().MainBranches, HashPool: self.c.Model().HashPool, @@ -338,7 +338,11 @@ func (self *RefreshHelper) refreshCommitsWithLimit() error { self.c.Model().Commits = commits self.RefreshAuthors(commits) self.c.Model().WorkingTreeStateAtLastCommitRefresh = self.c.Git().Status.WorkingTreeState() - self.c.Model().CheckedOutBranch = checkedOutBranchName + if checkedOutRef != nil { + self.c.Model().CheckedOutBranch = checkedOutRef.RefName() + } else { + self.c.Model().CheckedOutBranch = "" + } self.refreshView(self.c.Contexts().LocalCommits) return nil @@ -360,7 +364,7 @@ func (self *RefreshHelper) refreshSubCommitsWithLimit() error { IncludeRebaseCommits: false, RefName: self.c.Contexts().SubCommits.GetRef().FullRefName(), RefToShowDivergenceFrom: self.c.Contexts().SubCommits.GetRefToShowDivergenceFrom(), - RefForPushedStatus: self.c.Contexts().SubCommits.GetRef().FullRefName(), + RefForPushedStatus: self.c.Contexts().SubCommits.GetRef(), MainBranches: self.c.Model().MainBranches, HashPool: self.c.Model().HashPool, }, diff --git a/pkg/gui/controllers/helpers/sub_commits_helper.go b/pkg/gui/controllers/helpers/sub_commits_helper.go index c9561a918..080e1b456 100644 --- a/pkg/gui/controllers/helpers/sub_commits_helper.go +++ b/pkg/gui/controllers/helpers/sub_commits_helper.go @@ -39,7 +39,7 @@ func (self *SubCommitsHelper) ViewSubCommits(opts ViewSubCommitsOpts) error { FilterAuthor: self.c.Modes().Filtering.GetAuthor(), IncludeRebaseCommits: false, RefName: opts.Ref.FullRefName(), - RefForPushedStatus: opts.Ref.FullRefName(), + RefForPushedStatus: opts.Ref, RefToShowDivergenceFrom: opts.RefToShowDivergenceFrom, MainBranches: self.c.Model().MainBranches, HashPool: self.c.Model().HashPool, From e46dc1ead637857c0b4fe60dc72a8fe24bebe092 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 30 Jul 2025 15:07:15 +0200 Subject: [PATCH 4/4] Use a better approach for determining pushed and merge statuses Previously we would call git merge-base with the upstream branch to determine where unpushed commits end and pushed commits start, and also git merge-base with the main branch(es) to see where the merged commits start. This worked ok in normal cases, but it had two problems: - when filtering by path or by author, those merge-base commits would usually not be part of the commit list, so we would miss the point where we should switch from unpushed to pushed, or from pushed to merged. The consequence was that in filtering mode, all commit hashes were always yellow. - when main was merged into a feature branch, we would color all commits from that merge on down in green, even ones that are only part of the feature branch but not main. To fix these problems, we switch our approach to one where we call git rev-list with the branch in question, with negative refspecs for the upstream branch and the main branches, respectively; this gives us the complete picture of which commits are pushed/unpushed/merged, so it also works in the cases described above. And funnily, even though intuitively it feels more expensive, it actually performs better than the merge-base calls (for normal usage scenarios at least), so the commit-loading part of refresh is faster now in general. We are talking about differences like 300ms before, 140ms after, in some unscientific measurements I took (depends a lot on repo sizes, branch length, etc.). An exception are degenerate cases like feature branches with hundreds of thousands of commits, which are slower now; but I don't think we need to worry about those too much. --- pkg/commands/git_commands/commit_loader.go | 100 +++++++----------- .../git_commands/commit_loader_test.go | 54 +++++----- 2 files changed, 66 insertions(+), 88 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 1d4314c5b..72adf1b69 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -11,6 +11,7 @@ import ( "strings" "sync" + "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" @@ -98,23 +99,25 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, } }) - var ancestor string - var remoteAncestor string + var unmergedCommitHashes *set.Set[string] + var remoteUnmergedCommitHashes *set.Set[string] + mainBranches := opts.MainBranches.Get() + go utils.Safe(func() { defer wg.Done() - ancestor = opts.MainBranches.GetMergeBase(opts.RefName) - if opts.RefToShowDivergenceFrom != "" { - remoteAncestor = opts.MainBranches.GetMergeBase(opts.RefToShowDivergenceFrom) + if len(mainBranches) > 0 { + unmergedCommitHashes = self.getReachableHashes(opts.RefName, mainBranches) + if opts.RefToShowDivergenceFrom != "" { + remoteUnmergedCommitHashes = self.getReachableHashes(opts.RefToShowDivergenceFrom, mainBranches) + } } }) - passedFirstPushedCommit := false - // I can get this before - firstPushedCommit, err := self.getFirstPushedCommit(opts.RefForPushedStatus) - if err != nil || firstPushedCommit == "" { - // must have no upstream branch so we'll consider everything as pushed - passedFirstPushedCommit = true + var unpushedCommitHashes *set.Set[string] + if opts.RefForPushedStatus != nil { + unpushedCommitHashes = self.getReachableHashes(opts.RefForPushedStatus.FullRefName(), + append([]string{opts.RefForPushedStatus.RefName() + "@{u}"}, mainBranches...)) } wg.Wait() @@ -123,19 +126,6 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, return nil, logErr } - for _, commit := range commits { - if commit.Hash() == firstPushedCommit { - passedFirstPushedCommit = true - } - if !commit.IsTODO() { - if passedFirstPushedCommit { - commit.Status = models.StatusPushed - } else { - commit.Status = models.StatusUnpushed - } - } - } - if len(commits) == 0 { return commits, nil } @@ -153,10 +143,10 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, localSectionStart = len(commits) } - setCommitMergedStatuses(remoteAncestor, commits[:localSectionStart]) - setCommitMergedStatuses(ancestor, commits[localSectionStart:]) + setCommitStatuses(unpushedCommitHashes, remoteUnmergedCommitHashes, commits[:localSectionStart]) + setCommitStatuses(unpushedCommitHashes, unmergedCommitHashes, commits[localSectionStart:]) } else { - setCommitMergedStatuses(ancestor, commits) + setCommitStatuses(unpushedCommitHashes, unmergedCommitHashes, commits) } return commits, nil @@ -549,56 +539,40 @@ func (self *CommitLoader) getConflictedSequencerCommit(hashPool *utils.StringPoo }) } -func setCommitMergedStatuses(ancestor string, commits []*models.Commit) { - if ancestor == "" { - return - } - - passedAncestor := false +func setCommitStatuses(unpushedCommitHashes *set.Set[string], unmergedCommitHashes *set.Set[string], commits []*models.Commit) { 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()) { - passedAncestor = true - } - if commit.Status != models.StatusPushed && commit.Status != models.StatusUnpushed { + if commit.IsTODO() { continue } - if passedAncestor { + + if unmergedCommitHashes == nil || unmergedCommitHashes.Includes(commit.Hash()) { + if unpushedCommitHashes != nil && unpushedCommitHashes.Includes(commit.Hash()) { + commits[i].Status = models.StatusUnpushed + } else { + commits[i].Status = models.StatusPushed + } + } else { commits[i].Status = models.StatusMerged } } } -func ignoringWarnings(commandOutput string) string { - trimmedOutput := strings.TrimSpace(commandOutput) - split := strings.Split(trimmedOutput, "\n") - // need to get last line in case the first line is a warning about how the error is ambiguous. - // At some point we should find a way to make it unambiguous - lastLine := split[len(split)-1] - - return lastLine -} - -// getFirstPushedCommit returns the first commit hash which has been pushed to the ref's upstream. -// all commits above this are deemed unpushed and marked as such. -func (self *CommitLoader) getFirstPushedCommit(ref models.Ref) (string, error) { - if ref == nil { - return "", nil - } - - output, err := self.cmd.New( - NewGitCmd("merge-base"). - Arg(ref.FullRefName()). - Arg(ref.RefName() + "@{u}"). +func (self *CommitLoader) getReachableHashes(refName string, notRefNames []string) *set.Set[string] { + output, _, err := self.cmd.New( + NewGitCmd("rev-list"). + Arg(refName). + Arg(lo.Map(notRefNames, func(name string, _ int) string { + return "^" + name + })...). ToArgv(), ). DontLog(). - RunWithOutput() + RunWithOutputs() if err != nil { - return "", err + return set.New[string]() } - return ignoringWarnings(output), nil + return set.NewFromSlice(utils.SplitLines(output)) } // getLog gets the git log. diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index cbf170e5b..7f9873b0b 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/go-errors/errors" + "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" @@ -43,7 +44,7 @@ func TestGetCommits(t *testing.T) { logOrder: "topo-order", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -54,7 +55,7 @@ func TestGetCommits(t *testing.T) { logOrder: "topo-order", opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -67,7 +68,7 @@ func TestGetCommits(t *testing.T) { mainBranches: []string{"master", "main", "develop"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}", "^refs/remotes/origin/master", "^refs/remotes/origin/main"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", 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%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil). // here it's testing which of the configured main branches have an upstream @@ -77,8 +78,9 @@ func TestGetCommits(t *testing.T) { ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "develop@{u}"}, "", errors.New("error")). // this one doesn't, so it checks origin instead ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/develop"}, "", errors.New("error")). // doesn't exist there, either, so it checks for a local branch ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/heads/develop"}, "", errors.New("error")). // no local branch either - // 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), + // here it's seeing which of our commits are not on any of the main branches yet + ExpectGitArgs([]string{"rev-list", "HEAD", "^refs/remotes/origin/master", "^refs/remotes/origin/main"}, + "0eea75e8c631fba6b58135697835d58ba4c18dbc\nb21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164\ne94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c\nd8084cd558925eb7c9c38afeed5725c21653ab90\n65f910ebd85283b5cce9bf67d03d3f1a9ea3813a\n", nil), expectedCommitOpts: []models.NewCommitOpts{ { @@ -197,13 +199,13 @@ func TestGetCommits(t *testing.T) { expectedError: nil, }, { - testName: "should not call merge-base for mainBranches if none exist", + testName: "should not call rev-list for mainBranches if none exist", logOrder: "topo-order", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, mainBranches: []string{"master", "main"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", 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%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist; neither does @@ -233,13 +235,13 @@ func TestGetCommits(t *testing.T) { expectedError: nil, }, { - testName: "should call merge-base for all main branches that exist", + testName: "should call rev-list with all main branches that exist", logOrder: "topo-order", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, mainBranches: []string{"master", "main", "develop", "1.0-hotfixes"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}", "^refs/remotes/origin/master", "^refs/remotes/origin/develop", "^refs/remotes/origin/1.0-hotfixes"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", 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%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist @@ -249,8 +251,8 @@ func TestGetCommits(t *testing.T) { ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/heads/main"}, "", errors.New("error")). ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "develop@{u}"}, "refs/remotes/origin/develop", nil). ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "1.0-hotfixes@{u}"}, "refs/remotes/origin/1.0-hotfixes", nil). - // 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), + // here it's seeing which of our commits are not on any of the main branches yet + ExpectGitArgs([]string{"rev-list", "HEAD", "^refs/remotes/origin/master", "^refs/remotes/origin/develop", "^refs/remotes/origin/1.0-hotfixes"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", nil), expectedCommitOpts: []models.NewCommitOpts{ { @@ -275,7 +277,7 @@ func TestGetCommits(t *testing.T) { logOrder: "default", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -286,7 +288,7 @@ func TestGetCommits(t *testing.T) { logOrder: "default", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, FilterPath: "src"}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--follow", "--name-status", "--no-show-signature", "--", "src"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -536,11 +538,12 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { } } -func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { +func TestCommitLoader_setCommitStatuses(t *testing.T) { type scenario struct { testName string commitOpts []models.NewCommitOpts - ancestor string + unmergedCommits []string + unpushedCommits []string expectedCommitOpts []models.NewCommitOpts } @@ -548,13 +551,13 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { { testName: "basic", 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}, + {Hash: "12345", Name: "1", Action: models.ActionNone}, + {Hash: "67890", Name: "2", Action: models.ActionNone}, + {Hash: "abcde", Name: "3", Action: models.ActionNone}, }, - ancestor: "67890", + unmergedCommits: []string{"12345"}, expectedCommitOpts: []models.NewCommitOpts{ - {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, + {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusPushed}, {Hash: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusMerged}, {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusMerged}, }, @@ -562,11 +565,12 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { { testName: "with update-ref", 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}, + {Hash: "12345", Name: "1", Action: models.ActionNone}, + {Hash: "", Name: "", Action: todo.UpdateRef}, + {Hash: "abcde", Name: "3", Action: models.ActionNone}, }, - ancestor: "deadbeef", + unmergedCommits: []string{"12345", "abcde"}, + unpushedCommits: []string{"12345"}, expectedCommitOpts: []models.NewCommitOpts{ {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, {Hash: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone}, @@ -581,7 +585,7 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { commits := lo.Map(scenario.commitOpts, func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) - setCommitMergedStatuses(scenario.ancestor, commits) + setCommitStatuses(set.NewFromSlice(scenario.unpushedCommits), set.NewFromSlice(scenario.unmergedCommits), commits) expectedCommits := lo.Map(scenario.expectedCommitOpts, func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) assert.Equal(t, expectedCommits, commits)