Better local branch delete confirmation

Currently we try to delete a branch normally, and if git returns an error and
its output contains the text "branch -D", then we prompt the user to force
delete, and try again using -D. Besides just being ugly, this has the
disadvantage that git's logic to decide whether a branch is merged is not very
good; it only considers a branch merged if it is either reachable from the
current head, or from its own upstream. In many cases I want to delete a branch
that has been merged to master, but I don't have master checked out, so the
current branch is really irrelevant, and it should rather (or in addition) check
whether the branch is reachable from one of the main branches. The problem is
that git doesn't know what those are.

But lazygit does, so make the check on our side, prompt the user if necessary,
and always use -D. This is both cleaner, and works better.

See this mailing list discussion for more:
https://lore.kernel.org/git/[email protected]/

Author: stefanhaller

pkg/commands/git_commands/branch.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/mgutz/str"
@@ -260,3 +261,26 @@ func (self *BranchCommands) AllBranchesLogCmdObj() oscommands.ICmdObj {
 
 	return self.cmd.New(str.ToArgv(candidates[i])).DontLog()
 }
+
+func (self *BranchCommands) IsBranchMerged(branch *models.Branch, mainBranches *MainBranches) (bool, error) {
+	branchesToCheckAgainst := []string{"HEAD"}
+	if branch.RemoteBranchStoredLocally() {
+		branchesToCheckAgainst = append(branchesToCheckAgainst, fmt.Sprintf("%s@{upstream}", branch.Name))
+	}
+	branchesToCheckAgainst = append(branchesToCheckAgainst, mainBranches.Get()...)
+
+	cmdArgs := NewGitCmd("rev-list").
+		Arg("--max-count=1").
+		Arg(branch.Name).
+		Arg(lo.Map(branchesToCheckAgainst, func(branch string, _ int) string {
+			return fmt.Sprintf("^%s", branch)
+		})...).
+		ToArgv()
+
+	stdout, _, err := self.cmd.New(cmdArgs).RunWithOutputs()
+	if err != nil {
+		return false, err
+	}
+
+	return stdout == "", nil
+}

pkg/gui/controllers.go

@@ -107,7 +107,7 @@ func (gui *Gui) resetHelpersAndControllers() {
 		Files:           helpers.NewFilesHelper(helperCommon),
 		WorkingTree:     helpers.NewWorkingTreeHelper(helperCommon, refsHelper, commitsHelper, gpgHelper),
 		Tags:            helpers.NewTagsHelper(helperCommon, commitsHelper),
-		BranchesHelper:  helpers.NewBranchesHelper(helperCommon),
+		BranchesHelper:  helpers.NewBranchesHelper(helperCommon, worktreeHelper),
 		GPG:             helpers.NewGpgHelper(helperCommon),
 		MergeAndRebase:  rebaseHelper,
 		MergeConflicts:  mergeConflictsHelper,

pkg/gui/controllers/branches_controller.go

@@ -3,7 +3,6 @@ package controllers
 import (
 	"errors"
 	"fmt"
-	"strings"
 
 	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
@@ -521,92 +520,14 @@ func (self *BranchesController) createNewBranchWithName(newBranchName string) er
 	return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, KeepBranchSelectionIndex: true})
 }
 
-func (self *BranchesController) checkedOutByOtherWorktree(branch *models.Branch) bool {
-	return git_commands.CheckedOutByOtherWorktree(branch, self.c.Model().Worktrees)
-}
-
-func (self *BranchesController) promptWorktreeBranchDelete(selectedBranch *models.Branch) error {
-	worktree, ok := self.worktreeForBranch(selectedBranch)
-	if !ok {
-		self.c.Log.Error("promptWorktreeBranchDelete out of sync with list of worktrees")
-		return nil
-	}
-
-	title := utils.ResolvePlaceholderString(self.c.Tr.BranchCheckedOutByWorktree, map[string]string{
-		"worktreeName": worktree.Name,
-		"branchName":   selectedBranch.Name,
-	})
-	return self.c.Menu(types.CreateMenuOptions{
-		Title: title,
-		Items: []*types.MenuItem{
-			{
-				Label: self.c.Tr.SwitchToWorktree,
-				OnPress: func() error {
-					return self.c.Helpers().Worktree.Switch(worktree, context.LOCAL_BRANCHES_CONTEXT_KEY)
-				},
-			},
-			{
-				Label:   self.c.Tr.DetachWorktree,
-				Tooltip: self.c.Tr.DetachWorktreeTooltip,
-				OnPress: func() error {
-					return self.c.Helpers().Worktree.Detach(worktree)
-				},
-			},
-			{
-				Label: self.c.Tr.RemoveWorktree,
-				OnPress: func() error {
-					return self.c.Helpers().Worktree.Remove(worktree, false)
-				},
-			},
-		},
-	})
-}
-
 func (self *BranchesController) localDelete(branch *models.Branch) error {
-	if self.checkedOutByOtherWorktree(branch) {
-		return self.promptWorktreeBranchDelete(branch)
-	}
-
-	return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(_ gocui.Task) error {
-		self.c.LogAction(self.c.Tr.Actions.DeleteLocalBranch)
-		err := self.c.Git().Branch.LocalDelete(branch.Name, false)
-		if err != nil && strings.Contains(err.Error(), "git branch -D ") {
-			return self.forceDelete(branch)
-		}
-		if err != nil {
-			return err
-		}
-		return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}})
-	})
+	return self.c.Helpers().BranchesHelper.ConfirmLocalDelete(branch)
 }
 
 func (self *BranchesController) remoteDelete(branch *models.Branch) error {
 	return self.c.Helpers().BranchesHelper.ConfirmDeleteRemote(branch.UpstreamRemote, branch.UpstreamBranch)
 }
 
-func (self *BranchesController) forceDelete(branch *models.Branch) error {
-	title := self.c.Tr.ForceDeleteBranchTitle
-	message := utils.ResolvePlaceholderString(
-		self.c.Tr.ForceDeleteBranchMessage,
-		map[string]string{
-			"selectedBranchName": branch.Name,
-		},
-	)
-
-	self.c.Confirm(types.ConfirmOpts{
-		Title:  title,
-		Prompt: message,
-		HandleConfirm: func() error {
-			if err := self.c.Git().Branch.LocalDelete(branch.Name, true); err != nil {
-				return err
-			}
-			return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}})
-		},
-	})
-
-	return nil
-}
-
 func (self *BranchesController) delete(branch *models.Branch) error {
 	checkedOutBranch := self.c.Helpers().Refs.GetCheckedOutRef()
 

pkg/gui/controllers/helpers/branches_helper.go

@@ -4,20 +4,68 @@ import (
 	"strings"
 
 	"github.com/jesseduffield/gocui"
+	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
+	"github.com/jesseduffield/lazygit/pkg/gui/context"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
 type BranchesHelper struct {
-	c *HelperCommon
+	c              *HelperCommon
+	worktreeHelper *WorktreeHelper
 }
 
-func NewBranchesHelper(c *HelperCommon) *BranchesHelper {
+func NewBranchesHelper(c *HelperCommon, worktreeHelper *WorktreeHelper) *BranchesHelper {
 	return &BranchesHelper{
-		c: c,
+		c:              c,
+		worktreeHelper: worktreeHelper,
 	}
 }
 
+func (self *BranchesHelper) ConfirmLocalDelete(branch *models.Branch) error {
+	if self.checkedOutByOtherWorktree(branch) {
+		return self.promptWorktreeBranchDelete(branch)
+	}
+
+	isMerged, err := self.c.Git().Branch.IsBranchMerged(branch, self.c.Model().MainBranches)
+	if err != nil {
+		return err
+	}
+
+	doDelete := func() error {
+		return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(_ gocui.Task) error {
+			self.c.LogAction(self.c.Tr.Actions.DeleteLocalBranch)
+			if err := self.c.Git().Branch.LocalDelete(branch.Name, true); err != nil {
+				return err
+			}
+			return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}})
+		})
+	}
+
+	if isMerged {
+		return doDelete()
+	}
+
+	title := self.c.Tr.ForceDeleteBranchTitle
+	message := utils.ResolvePlaceholderString(
+		self.c.Tr.ForceDeleteBranchMessage,
+		map[string]string{
+			"selectedBranchName": branch.Name,
+		},
+	)
+
+	self.c.Confirm(types.ConfirmOpts{
+		Title:  title,
+		Prompt: message,
+		HandleConfirm: func() error {
+			return doDelete()
+		},
+	})
+
+	return nil
+}
+
 func (self *BranchesHelper) ConfirmDeleteRemote(remoteName string, branchName string) error {
 	title := utils.ResolvePlaceholderString(
 		self.c.Tr.DeleteBranchTitle,
@@ -52,3 +100,48 @@ func (self *BranchesHelper) ConfirmDeleteRemote(remoteName string, branchName st
 func ShortBranchName(fullBranchName string) string {
 	return strings.TrimPrefix(strings.TrimPrefix(fullBranchName, "refs/heads/"), "refs/remotes/")
 }
+
+func (self *BranchesHelper) checkedOutByOtherWorktree(branch *models.Branch) bool {
+	return git_commands.CheckedOutByOtherWorktree(branch, self.c.Model().Worktrees)
+}
+
+func (self *BranchesHelper) worktreeForBranch(branch *models.Branch) (*models.Worktree, bool) {
+	return git_commands.WorktreeForBranch(branch, self.c.Model().Worktrees)
+}
+
+func (self *BranchesHelper) promptWorktreeBranchDelete(selectedBranch *models.Branch) error {
+	worktree, ok := self.worktreeForBranch(selectedBranch)
+	if !ok {
+		self.c.Log.Error("promptWorktreeBranchDelete out of sync with list of worktrees")
+		return nil
+	}
+
+	title := utils.ResolvePlaceholderString(self.c.Tr.BranchCheckedOutByWorktree, map[string]string{
+		"worktreeName": worktree.Name,
+		"branchName":   selectedBranch.Name,
+	})
+	return self.c.Menu(types.CreateMenuOptions{
+		Title: title,
+		Items: []*types.MenuItem{
+			{
+				Label: self.c.Tr.SwitchToWorktree,
+				OnPress: func() error {
+					return self.worktreeHelper.Switch(worktree, context.LOCAL_BRANCHES_CONTEXT_KEY)
+				},
+			},
+			{
+				Label:   self.c.Tr.DetachWorktree,
+				Tooltip: self.c.Tr.DetachWorktreeTooltip,
+				OnPress: func() error {
+					return self.worktreeHelper.Detach(worktree)
+				},
+			},
+			{
+				Label: self.c.Tr.RemoveWorktree,
+				OnPress: func() error {
+					return self.worktreeHelper.Remove(worktree, false)
+				},
+			},
+		},
+	})
+}