Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 38 additions & 60 deletions pkg/commands/git_commands/commit_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -59,8 +60,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)
Expand Down Expand Up @@ -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 {
// 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()
Expand All @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -549,52 +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(refName string) (string, error) {
output, err := self.cmd.New(
NewGitCmd("merge-base").
func (self *CommitLoader) getReachableHashes(refName string, notRefNames []string) *set.Set[string] {
output, _, err := self.cmd.New(
NewGitCmd("rev-list").
Arg(refName).
Arg(strings.TrimPrefix(refName, "refs/heads/") + "@{u}").
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.
Expand Down
68 changes: 36 additions & 32 deletions pkg/commands/git_commands/commit_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -41,9 +42,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{"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{},
Expand All @@ -52,9 +53,9 @@ 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{"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{},
Expand All @@ -63,11 +64,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{"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
Expand All @@ -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{
{
Expand Down Expand Up @@ -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: "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{"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
Expand Down Expand Up @@ -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: "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{"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
Expand All @@ -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{
{
Expand All @@ -273,9 +275,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{"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{},
Expand All @@ -284,9 +286,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{"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{},
Expand Down Expand Up @@ -536,37 +538,39 @@ 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
}

scenarios := []scenario{
{
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},
},
},
{
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},
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions pkg/commands/git_commands/main_branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 1 addition & 6 deletions pkg/gui/types/ref.go → pkg/commands/models/ref.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package types
package models

type Ref interface {
FullRefName() string
Expand All @@ -7,8 +7,3 @@ type Ref interface {
ParentRefName() string
Description() string
}

type RefRange struct {
From Ref
To Ref
}
2 changes: 1 addition & 1 deletion pkg/gui/context/branches_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/commit_files_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading