Correctly request force-pushing in a triangular workflow (#3528)

- **PR Description**

Some people push to a different branch (or even remote) than they pull
from. One example is described in #3437. Our logic of when to request a
force push is not appropriate for these workflows: we check the
configured upstream branch for divergence, but that's the one you pull
from. We should instead check the push-to branch for divergence.

Fixes #3437.

Author: stefanhaller

docs/Custom_Command_Keybindings.md

@@ -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
 

pkg/commands/git.go

@@ -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)

pkg/commands/git_commands/branch_loader.go

@@ -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,
@@ -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)
@@ -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)
@@ -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
 	})
 }
 
@@ -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 {
@@ -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,
 	}
 }
 
@@ -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 {

pkg/commands/git_commands/branch_loader_test.go

@@ -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)
 		})
 	}

pkg/commands/models/branch.go

@@ -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
@@ -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 != ""
 }

pkg/gui/controllers/branches_controller.go

@@ -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)
 	}
 

pkg/gui/controllers/sync_controller.go

@@ -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)

pkg/gui/presentation/branches.go

@@ -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