diff --git a/pkg/commands/git_commands/branch.go b/pkg/commands/git_commands/branch.go index c63087a80..99229b12b 100644 --- a/pkg/commands/git_commands/branch.go +++ b/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 +} diff --git a/pkg/gui/controllers.go b/pkg/gui/controllers.go index c784b4a1d..5f38b0443 100644 --- a/pkg/gui/controllers.go +++ b/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, diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index c7cba1302..93b3b93e1 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/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() diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 635b94f20..68976f7a3 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/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) + }, + }, + }, + }) +} diff --git a/pkg/integration/tests/branch/delete.go b/pkg/integration/tests/branch/delete.go index f81eb0609..aab872957 100644 --- a/pkg/integration/tests/branch/delete.go +++ b/pkg/integration/tests/branch/delete.go @@ -15,27 +15,43 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{ CloneIntoRemote("origin"). EmptyCommit("blah"). NewBranch("branch-one"). + EmptyCommit("on branch-one 01"). PushBranchAndSetUpstream("origin", "branch-one"). + EmptyCommit("on branch-one 02"). + Checkout("master"). + Merge("branch-one"). // branch-one is contained in master, so no delete confirmation NewBranch("branch-two"). - PushBranchAndSetUpstream("origin", "branch-two"). - EmptyCommit("deletion blocker"). - NewBranch("branch-three") + EmptyCommit("on branch-two 01"). + PushBranchAndSetUpstream("origin", "branch-two"). // branch-two is contained in its own upstream, so no delete confirmation either + NewBranchFrom("branch-three", "master"). + EmptyCommit("on branch-three 01"). + NewBranch("current-head"). // branch-three is contained in the current head, so no delete confirmation + EmptyCommit("on current-head"). + NewBranchFrom("branch-four", "master"). + EmptyCommit("on branch-four 01"). + PushBranchAndSetUpstream("origin", "branch-four"). + EmptyCommit("on branch-four 02"). // branch-four is not contained in any of these, so we get a delete confirmation + Checkout("current-head") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Branches(). Focus(). Lines( - MatchesRegexp(`\*.*branch-three`).IsSelected(), - MatchesRegexp(`branch-two`), - MatchesRegexp(`branch-one`), - MatchesRegexp(`master`), + Contains("current-head").IsSelected(), + Contains("branch-four ↑1"), + Contains("branch-three"), + Contains("branch-two ✓"), + Contains("master"), + Contains("branch-one ↑1"), ). + + // Deleting the current branch is not possible Press(keys.Universal.Remove). Tap(func() { t.ExpectPopup(). Menu(). Tooltip(Contains("You cannot delete the checked out branch!")). - Title(Equals("Delete branch 'branch-three'?")). + Title(Equals("Delete branch 'current-head'?")). Select(Contains("Delete local branch")). Confirm(). Tap(func() { @@ -43,8 +59,51 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{ }). Cancel() }). + + // Delete branch-four. This is the only branch that is not fully merged, so we get + // a confirmation popup. SelectNextItem(). Press(keys.Universal.Remove). + Tap(func() { + t.ExpectPopup(). + Menu(). + Title(Equals("Delete branch 'branch-four'?")). + Select(Contains("Delete local branch")). + Confirm() + t.ExpectPopup(). + Confirmation(). + Title(Equals("Force delete branch")). + Content(Equals("'branch-four' is not fully merged. Are you sure you want to delete it?")). + Confirm() + }). + Lines( + Contains("current-head"), + Contains("branch-three").IsSelected(), + Contains("branch-two ✓"), + Contains("master"), + Contains("branch-one ↑1"), + ). + + // Delete branch-three. This branch is contained in the current head, so this just works + // without any confirmation. + Press(keys.Universal.Remove). + Tap(func() { + t.ExpectPopup(). + Menu(). + Title(Equals("Delete branch 'branch-three'?")). + Select(Contains("Delete local branch")). + Confirm() + }). + Lines( + Contains("current-head"), + Contains("branch-two ✓").IsSelected(), + Contains("master"), + Contains("branch-one ↑1"), + ). + + // Delete branch-two. This branch is contained in its own upstream, so this just works + // without any confirmation. + Press(keys.Universal.Remove). Tap(func() { t.ExpectPopup(). Menu(). @@ -52,18 +111,14 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{ Select(Contains("Delete local branch")). Confirm() }). - Tap(func() { - t.ExpectPopup(). - Confirmation(). - Title(Equals("Force delete branch")). - Content(Equals("'branch-two' is not fully merged. Are you sure you want to delete it?")). - Confirm() - }). Lines( - MatchesRegexp(`\*.*branch-three`), - MatchesRegexp(`branch-one`).IsSelected(), - MatchesRegexp(`master`), + Contains("current-head"), + Contains("master").IsSelected(), + Contains("branch-one ↑1"), ). + + // Delete remote branch of branch-one. We only get the normal remote branch confirmation for this one. + SelectNextItem(). Press(keys.Universal.Remove). Tap(func() { t.ExpectPopup(). @@ -87,7 +142,10 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{ t.Views(). RemoteBranches(). - Lines(Equals("branch-two")). + Lines( + Equals("branch-four"), + Equals("branch-two"), + ). Press(keys.Universal.Return) t.Views(). @@ -95,10 +153,13 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{ Focus() }). Lines( - MatchesRegexp(`\*.*branch-three`), - MatchesRegexp(`branch-one \(upstream gone\)`).IsSelected(), - MatchesRegexp(`master`), + Contains("current-head"), + Contains("master"), + Contains("branch-one (upstream gone)").IsSelected(), ). + + // Delete local branch of branch-one. Even though its upstream is gone, we don't get a confirmation + // because it is contained in master. Press(keys.Universal.Remove). Tap(func() { t.ExpectPopup(). @@ -108,8 +169,8 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{ Confirm() }). Lines( - MatchesRegexp(`\*.*branch-three`), - MatchesRegexp(`master`).IsSelected(), + Contains("current-head"), + Contains("master").IsSelected(), ) }, })