Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly request force-pushing in a triangular workflow #3528

Merged
merged 6 commits into from May 19, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/Custom_Command_Keybindings.md
Expand Up @@ -305,7 +305,7 @@ SelectedWorktree
CheckedOutBranch
```

To see what fields are available on e.g. the `SelectedFile`, see [here](https://github.com/jesseduffield/lazygit/blob/master/pkg/commands/models/file.go) (all the modelling lives in the same directory). Note that the custom commands feature does not guarantee backwards compatibility (until we hit Lazygit version 1.0 of course) which means a field you're accessing on an object may no longer be available from one release to the next. Typically however, all you'll need is `{{.SelectedFile.Name}}`, `{{.SelectedLocalCommit.Hash}}` and `{{.SelectedLocalBranch.Name}}`. In the future we will likely introduce a tighter interface that exposes a limited set of fields for each model.
To see what fields are available on e.g. the `SelectedFile`, see [here](https://github.com/jesseduffield/lazygit/blob/master/pkg/gui/services/custom_commands/models.go) (all the modelling lives in the same file).

## Keybinding collisions

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/git.go
Expand Up @@ -134,7 +134,7 @@ func NewGitCommandAux(
worktreeCommands := git_commands.NewWorktreeCommands(gitCommon)
blameCommands := git_commands.NewBlameCommands(gitCommon)

branchLoader := git_commands.NewBranchLoader(cmn, cmd, branchCommands.CurrentBranchInfo, configCommands)
branchLoader := git_commands.NewBranchLoader(cmn, gitCommon, cmd, branchCommands.CurrentBranchInfo, configCommands)
commitFileLoader := git_commands.NewCommitFileLoader(cmn, cmd)
commitLoader := git_commands.NewCommitLoader(cmn, cmd, statusCommands.RebaseMode, gitCommon)
reflogCommitLoader := git_commands.NewReflogCommitLoader(cmn, cmd)
Expand Down
51 changes: 32 additions & 19 deletions pkg/commands/git_commands/branch_loader.go
Expand Up @@ -40,19 +40,22 @@ type BranchInfo struct {
// BranchLoader returns a list of Branch objects for the current repo
type BranchLoader struct {
*common.Common
*GitCommon
cmd oscommands.ICmdObjBuilder
getCurrentBranchInfo func() (BranchInfo, error)
config BranchLoaderConfigCommands
}

func NewBranchLoader(
cmn *common.Common,
gitCommon *GitCommon,
cmd oscommands.ICmdObjBuilder,
getCurrentBranchInfo func() (BranchInfo, error),
config BranchLoaderConfigCommands,
) *BranchLoader {
return &BranchLoader{
Common: cmn,
GitCommon: gitCommon,
cmd: cmd,
getCurrentBranchInfo: getCurrentBranchInfo,
config: config,
Expand All @@ -61,7 +64,7 @@ func NewBranchLoader(

// Load the list of branches for the current repo
func (self *BranchLoader) Load(reflogCommits []*models.Commit) ([]*models.Branch, error) {
branches := self.obtainBranches()
branches := self.obtainBranches(self.version.IsAtLeast(2, 22, 0))

if self.AppState.LocalBranchSortOrder == "recency" {
reflogBranches := self.obtainReflogBranches(reflogCommits)
Expand Down Expand Up @@ -124,7 +127,7 @@ func (self *BranchLoader) Load(reflogCommits []*models.Commit) ([]*models.Branch
return branches, nil
}

func (self *BranchLoader) obtainBranches() []*models.Branch {
func (self *BranchLoader) obtainBranches(canUsePushTrack bool) []*models.Branch {
output, err := self.getRawBranches()
if err != nil {
panic(err)
Expand All @@ -147,7 +150,7 @@ func (self *BranchLoader) obtainBranches() []*models.Branch {
}

storeCommitDateAsRecency := self.AppState.LocalBranchSortOrder != "recency"
return obtainBranch(split, storeCommitDateAsRecency), true
return obtainBranch(split, storeCommitDateAsRecency, canUsePushTrack), true
})
}

Expand Down Expand Up @@ -183,23 +186,31 @@ var branchFields = []string{
"refname:short",
"upstream:short",
"upstream:track",
"push:track",
"subject",
"objectname",
"committerdate:unix",
}

// Obtain branch information from parsed line output of getRawBranches()
func obtainBranch(split []string, storeCommitDateAsRecency bool) *models.Branch {
func obtainBranch(split []string, storeCommitDateAsRecency bool, canUsePushTrack bool) *models.Branch {
headMarker := split[0]
fullName := split[1]
upstreamName := split[2]
track := split[3]
subject := split[4]
commitHash := split[5]
commitDate := split[6]
pushTrack := split[4]
subject := split[5]
commitHash := split[6]
commitDate := split[7]

name := strings.TrimPrefix(fullName, "heads/")
pushables, pullables, gone := parseUpstreamInfo(upstreamName, track)
aheadForPull, behindForPull, gone := parseUpstreamInfo(upstreamName, track)
var aheadForPush, behindForPush string
if canUsePushTrack {
aheadForPush, behindForPush, _ = parseUpstreamInfo(upstreamName, pushTrack)
} else {
aheadForPush, behindForPush = aheadForPull, behindForPull
}

recency := ""
if storeCommitDateAsRecency {
Expand All @@ -209,14 +220,16 @@ func obtainBranch(split []string, storeCommitDateAsRecency bool) *models.Branch
}

return &models.Branch{
Name: name,
Recency: recency,
Pushables: pushables,
Pullables: pullables,
UpstreamGone: gone,
Head: headMarker == "*",
Subject: subject,
CommitHash: commitHash,
Name: name,
Recency: recency,
AheadForPull: aheadForPull,
BehindForPull: behindForPull,
AheadForPush: aheadForPush,
BehindForPush: behindForPush,
UpstreamGone: gone,
Head: headMarker == "*",
Subject: subject,
CommitHash: commitHash,
}
}

Expand All @@ -232,10 +245,10 @@ func parseUpstreamInfo(upstreamName string, track string) (string, string, bool)
return "?", "?", true
}

pushables := parseDifference(track, `ahead (\d+)`)
pullables := parseDifference(track, `behind (\d+)`)
ahead := parseDifference(track, `ahead (\d+)`)
behind := parseDifference(track, `behind (\d+)`)

return pushables, pullables, false
return ahead, behind, false
}

func parseDifference(track string, regexStr string) string {
Expand Down
102 changes: 57 additions & 45 deletions pkg/commands/git_commands/branch_loader_test.go
Expand Up @@ -25,89 +25,101 @@ func TestObtainBranch(t *testing.T) {
scenarios := []scenario{
{
testName: "TrimHeads",
input: []string{"", "heads/a_branch", "", "", "subject", "123", timeStamp},
input: []string{"", "heads/a_branch", "", "", "", "subject", "123", timeStamp},
storeCommitDateAsRecency: false,
expectedBranch: &models.Branch{
Name: "a_branch",
Pushables: "?",
Pullables: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
Name: "a_branch",
AheadForPull: "?",
BehindForPull: "?",
AheadForPush: "?",
BehindForPush: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
},
},
{
testName: "NoUpstream",
input: []string{"", "a_branch", "", "", "subject", "123", timeStamp},
input: []string{"", "a_branch", "", "", "", "subject", "123", timeStamp},
storeCommitDateAsRecency: false,
expectedBranch: &models.Branch{
Name: "a_branch",
Pushables: "?",
Pullables: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
Name: "a_branch",
AheadForPull: "?",
BehindForPull: "?",
AheadForPush: "?",
BehindForPush: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
},
},
{
testName: "IsHead",
input: []string{"*", "a_branch", "", "", "subject", "123", timeStamp},
input: []string{"*", "a_branch", "", "", "", "subject", "123", timeStamp},
storeCommitDateAsRecency: false,
expectedBranch: &models.Branch{
Name: "a_branch",
Pushables: "?",
Pullables: "?",
Head: true,
Subject: "subject",
CommitHash: "123",
Name: "a_branch",
AheadForPull: "?",
BehindForPull: "?",
AheadForPush: "?",
BehindForPush: "?",
Head: true,
Subject: "subject",
CommitHash: "123",
},
},
{
testName: "IsBehindAndAhead",
input: []string{"", "a_branch", "a_remote/a_branch", "[behind 2, ahead 3]", "subject", "123", timeStamp},
input: []string{"", "a_branch", "a_remote/a_branch", "[behind 2, ahead 3]", "[behind 2, ahead 3]", "subject", "123", timeStamp},
storeCommitDateAsRecency: false,
expectedBranch: &models.Branch{
Name: "a_branch",
Pushables: "3",
Pullables: "2",
Head: false,
Subject: "subject",
CommitHash: "123",
Name: "a_branch",
AheadForPull: "3",
BehindForPull: "2",
AheadForPush: "3",
BehindForPush: "2",
Head: false,
Subject: "subject",
CommitHash: "123",
},
},
{
testName: "RemoteBranchIsGone",
input: []string{"", "a_branch", "a_remote/a_branch", "[gone]", "subject", "123", timeStamp},
input: []string{"", "a_branch", "a_remote/a_branch", "[gone]", "[gone]", "subject", "123", timeStamp},
storeCommitDateAsRecency: false,
expectedBranch: &models.Branch{
Name: "a_branch",
UpstreamGone: true,
Pushables: "?",
Pullables: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
Name: "a_branch",
UpstreamGone: true,
AheadForPull: "?",
BehindForPull: "?",
AheadForPush: "?",
BehindForPush: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
},
},
{
testName: "WithCommitDateAsRecency",
input: []string{"", "a_branch", "", "", "subject", "123", timeStamp},
input: []string{"", "a_branch", "", "", "", "subject", "123", timeStamp},
storeCommitDateAsRecency: true,
expectedBranch: &models.Branch{
Name: "a_branch",
Recency: "2h",
Pushables: "?",
Pullables: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
Name: "a_branch",
Recency: "2h",
AheadForPull: "?",
BehindForPull: "?",
AheadForPush: "?",
BehindForPush: "?",
Head: false,
Subject: "subject",
CommitHash: "123",
},
},
}

for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
branch := obtainBranch(s.input, s.storeCommitDateAsRecency)
branch := obtainBranch(s.input, s.storeCommitDateAsRecency, true)
assert.EqualValues(t, s.expectedBranch, branch)
})
}
Expand Down
30 changes: 19 additions & 11 deletions pkg/commands/models/branch.go
Expand Up @@ -10,10 +10,14 @@ type Branch struct {
DisplayName string
// indicator of when the branch was last checked out e.g. '2d', '3m'
Recency string
// how many commits ahead we are from the remote branch (how many commits we can push)
Pushables string
// how many commits ahead we are from the remote branch (how many commits we can push, assuming we push to our tracked remote branch)
AheadForPull string
// how many commits behind we are from the remote branch (how many commits we can pull)
Pullables string
BehindForPull string
// how many commits ahead we are from the branch we're pushing to (which might not be the same as our upstream branch in a triangular workflow)
AheadForPush string
// how many commits behind we are from the branch we're pushing to (which might not be the same as our upstream branch in a triangular workflow)
BehindForPush string
// whether the remote branch is 'gone' i.e. we're tracking a remote branch that has been deleted
UpstreamGone bool
// whether this is the current branch. Exactly one branch should have this be true
Expand Down Expand Up @@ -80,26 +84,30 @@ func (b *Branch) IsTrackingRemote() bool {
// we know that the remote branch is not stored locally based on our pushable/pullable
// count being question marks.
func (b *Branch) RemoteBranchStoredLocally() bool {
return b.IsTrackingRemote() && b.Pushables != "?" && b.Pullables != "?"
return b.IsTrackingRemote() && b.AheadForPull != "?" && b.BehindForPull != "?"
}

func (b *Branch) RemoteBranchNotStoredLocally() bool {
return b.IsTrackingRemote() && b.Pushables == "?" && b.Pullables == "?"
return b.IsTrackingRemote() && b.AheadForPull == "?" && b.BehindForPull == "?"
}

func (b *Branch) MatchesUpstream() bool {
return b.RemoteBranchStoredLocally() && b.Pushables == "0" && b.Pullables == "0"
return b.RemoteBranchStoredLocally() && b.AheadForPull == "0" && b.BehindForPull == "0"
}

func (b *Branch) HasCommitsToPush() bool {
return b.RemoteBranchStoredLocally() && b.Pushables != "0"
func (b *Branch) IsAheadForPull() bool {
return b.RemoteBranchStoredLocally() && b.AheadForPull != "0"
}

func (b *Branch) HasCommitsToPull() bool {
return b.RemoteBranchStoredLocally() && b.Pullables != "0"
func (b *Branch) IsBehindForPull() bool {
return b.RemoteBranchStoredLocally() && b.BehindForPull != "0"
}

func (b *Branch) IsBehindForPush() bool {
return b.BehindForPush != "" && b.BehindForPush != "0"
}

// for when we're in a detached head state
func (b *Branch) IsRealBranch() bool {
return b.Pushables != "" && b.Pullables != ""
return b.AheadForPull != "" && b.BehindForPull != ""
}
2 changes: 1 addition & 1 deletion pkg/gui/controllers/branches_controller.go
Expand Up @@ -620,7 +620,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error {
if !branch.RemoteBranchStoredLocally() {
return errors.New(self.c.Tr.FwdNoLocalUpstream)
}
if branch.HasCommitsToPush() {
if branch.IsAheadForPull() {
return errors.New(self.c.Tr.FwdCommitsToPush)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/gui/controllers/sync_controller.go
Expand Up @@ -87,10 +87,10 @@ func (self *SyncController) branchCheckedOut(f func(*models.Branch) error) func(
}

func (self *SyncController) push(currentBranch *models.Branch) error {
// if we have pullables we'll ask if the user wants to force push
// if we are behind our upstream branch we'll ask if the user wants to force push
if currentBranch.IsTrackingRemote() {
opts := pushOpts{}
if currentBranch.HasCommitsToPull() {
if currentBranch.IsBehindForPush() {
return self.requestToForcePush(currentBranch, opts)
} else {
return self.pushAux(currentBranch, opts)
Expand Down
8 changes: 4 additions & 4 deletions pkg/gui/presentation/branches.go
Expand Up @@ -196,11 +196,11 @@ func BranchStatus(
}

result := ""
if branch.HasCommitsToPush() {
result = fmt.Sprintf("↑%s", branch.Pushables)
if branch.IsAheadForPull() {
result = fmt.Sprintf("↑%s", branch.AheadForPull)
}
if branch.HasCommitsToPull() {
result = fmt.Sprintf("%s↓%s", result, branch.Pullables)
if branch.IsBehindForPull() {
result = fmt.Sprintf("%s↓%s", result, branch.BehindForPull)
}

return result
Expand Down