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

Add a menu item to delete both local and remote branch at once #3916

Merged
merged 1 commit into from
Sep 28, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion pkg/gui/controllers/branches_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,10 @@ func (self *BranchesController) remoteDelete(branch *models.Branch) error {
return self.c.Helpers().BranchesHelper.ConfirmDeleteRemote(branch.UpstreamRemote, branch.UpstreamBranch)
}

func (self *BranchesController) localAndRemoteDelete(branch *models.Branch) error {
return self.c.Helpers().BranchesHelper.ConfirmLocalAndRemoteDelete(branch)
}

func (self *BranchesController) delete(branch *models.Branch) error {
checkedOutBranch := self.c.Helpers().Refs.GetCheckedOutRef()

Expand All @@ -553,6 +557,19 @@ func (self *BranchesController) delete(branch *models.Branch) error {
remoteDeleteItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.UpstreamNotSetError}
}

deleteBothItem := &types.MenuItem{
Label: self.c.Tr.DeleteLocalAndRemoteBranch,
Key: 'b',
OnPress: func() error {
return self.localAndRemoteDelete(branch)
},
}
if checkedOutBranch.Name == branch.Name {
deleteBothItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.CantDeleteCheckOutBranch}
} else if !branch.IsTrackingRemote() || branch.UpstreamGone {
deleteBothItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.UpstreamNotSetError}
}

menuTitle := utils.ResolvePlaceholderString(
self.c.Tr.DeleteBranchTitle,
map[string]string{
Expand All @@ -562,7 +579,7 @@ func (self *BranchesController) delete(branch *models.Branch) error {

return self.c.Menu(types.CreateMenuOptions{
Title: menuTitle,
Items: []*types.MenuItem{localDeleteItem, remoteDeleteItem},
Items: []*types.MenuItem{localDeleteItem, remoteDeleteItem, deleteBothItem},
})
}

Expand Down
53 changes: 53 additions & 0 deletions pkg/gui/controllers/helpers/branches_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,59 @@ func (self *BranchesHelper) ConfirmDeleteRemote(remoteName string, branchName st
return nil
}

func (self *BranchesHelper) ConfirmLocalAndRemoteDelete(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
}

prompt := utils.ResolvePlaceholderString(
self.c.Tr.DeleteLocalAndRemoteBranchPrompt,
map[string]string{
"localBranchName": branch.Name,
"remoteBranchName": branch.UpstreamBranch,
"remoteName": branch.UpstreamRemote,
},
)

if !isMerged {
prompt += "\n\n" + utils.ResolvePlaceholderString(
self.c.Tr.ForceDeleteBranchMessage,
map[string]string{
"selectedBranchName": branch.Name,
},
)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to put up only a single confirmation ("are you sure you want to delete both branches"). If the local branch is not merged, I append the warning about this below in the same popup. I like this better than putting up a separate popup for this after the first one, but I can change it if you disagree.


self.c.Confirm(types.ConfirmOpts{
Title: self.c.Tr.DeleteLocalAndRemoteBranch,
Prompt: prompt,
HandleConfirm: func() error {
return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func(task gocui.Task) error {
// Delete the remote branch first so that we keep the local one
// in case of failure
self.c.LogAction(self.c.Tr.Actions.DeleteRemoteBranch)
if err := self.c.Git().Remote.DeleteRemoteBranch(task, branch.UpstreamRemote, branch.Name); err != nil {
return err
}

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, types.REMOTES}})
})
},
})

return nil
}

func ShortBranchName(fullBranchName string) string {
return strings.TrimPrefix(strings.TrimPrefix(fullBranchName, "refs/heads/"), "refs/remotes/")
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/i18n/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ type TranslationSet struct {
DeleteLocalBranch string
DeleteRemoteBranchOption string
DeleteRemoteBranchPrompt string
DeleteLocalAndRemoteBranchPrompt string
ForceDeleteBranchTitle string
ForceDeleteBranchMessage string
RebaseBranch string
Expand Down Expand Up @@ -473,6 +474,7 @@ type TranslationSet struct {
RemoveRemotePrompt string
DeleteRemoteBranch string
DeleteRemoteBranchTooltip string
DeleteLocalAndRemoteBranch string
SetAsUpstream string
SetAsUpstreamTooltip string
SetUpstream string
Expand Down Expand Up @@ -1086,6 +1088,7 @@ func EnglishTranslationSet() *TranslationSet {
DeleteLocalBranch: "Delete local branch",
DeleteRemoteBranchOption: "Delete remote branch",
DeleteRemoteBranchPrompt: "Are you sure you want to delete the remote branch '{{.selectedBranchName}}' from '{{.upstream}}'?",
DeleteLocalAndRemoteBranchPrompt: "Are you sure you want to delete both '{{.localBranchName}}' from your machine, and '{{.remoteBranchName}}' from '{{.remoteName}}'?",
ForceDeleteBranchTitle: "Force delete branch",
ForceDeleteBranchMessage: "'{{.selectedBranchName}}' is not fully merged. Are you sure you want to delete it?",
RebaseBranch: "Rebase",
Expand Down Expand Up @@ -1462,6 +1465,7 @@ func EnglishTranslationSet() *TranslationSet {
RemoveRemotePrompt: "Are you sure you want to remove remote?",
DeleteRemoteBranch: "Delete remote branch",
DeleteRemoteBranchTooltip: "Delete the remote branch from the remote.",
DeleteLocalAndRemoteBranch: "Delete local and remote branch",
SetAsUpstream: "Set as upstream",
SetAsUpstreamTooltip: "Set the selected remote branch as the upstream of the checked-out branch.",
SetUpstream: "Set upstream of selected branch",
Expand Down
66 changes: 65 additions & 1 deletion pkg/integration/tests/branch/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,22 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{
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
NewBranchFrom("branch-five", "master").
EmptyCommit("on branch-five 01").
PushBranchAndSetUpstream("origin", "branch-five"). // branch-five is contained in its own upstream
NewBranchFrom("branch-six", "master").
EmptyCommit("on branch-six 01").
PushBranchAndSetUpstream("origin", "branch-six").
EmptyCommit("on branch-six 02"). // branch-six 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(
Contains("current-head").IsSelected(),
Contains("branch-six ↑1"),
Contains("branch-five ✓"),
Contains("branch-four ↑1"),
Contains("branch-three"),
Contains("branch-two ✓"),
Expand All @@ -62,7 +71,7 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{

// Delete branch-four. This is the only branch that is not fully merged, so we get
// a confirmation popup.
SelectNextItem().
NavigateToLine(Contains("branch-four")).
Press(keys.Universal.Remove).
Tap(func() {
t.ExpectPopup().
Expand All @@ -78,6 +87,8 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{
}).
Lines(
Contains("current-head"),
Contains("branch-six ↑1"),
Contains("branch-five ✓"),
Contains("branch-three").IsSelected(),
Contains("branch-two ✓"),
Contains("master"),
Expand All @@ -96,6 +107,8 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{
}).
Lines(
Contains("current-head"),
Contains("branch-six ↑1"),
Contains("branch-five ✓"),
Contains("branch-two ✓").IsSelected(),
Contains("master"),
Contains("branch-one ↑1"),
Expand All @@ -113,6 +126,8 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{
}).
Lines(
Contains("current-head"),
Contains("branch-six ↑1"),
Contains("branch-five ✓"),
Contains("master").IsSelected(),
Contains("branch-one ↑1"),
).
Expand Down Expand Up @@ -143,7 +158,9 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{
t.Views().
RemoteBranches().
Lines(
Equals("branch-five"),
Equals("branch-four"),
Equals("branch-six"),
Equals("branch-two"),
).
Press(keys.Universal.Return)
Expand All @@ -154,6 +171,8 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{
}).
Lines(
Contains("current-head"),
Contains("branch-six ↑1"),
Contains("branch-five ✓"),
Contains("master"),
Contains("branch-one (upstream gone)").IsSelected(),
).
Expand All @@ -168,6 +187,51 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{
Select(Contains("Delete local branch")).
Confirm()
}).
Lines(
Contains("current-head"),
Contains("branch-six ↑1"),
Contains("branch-five ✓"),
Contains("master").IsSelected(),
).

// Delete both local and remote branch of branch-six. We get the force-delete warning because it is not fully merged.
NavigateToLine(Contains("branch-six")).
Press(keys.Universal.Remove).
Tap(func() {
t.ExpectPopup().
Menu().
Title(Equals("Delete branch 'branch-six'?")).
Select(Contains("Delete local and remote branch")).
Confirm()
t.ExpectPopup().
Confirmation().
Title(Equals("Delete local and remote branch")).
Content(Contains("Are you sure you want to delete both 'branch-six' from your machine, and 'branch-six' from 'origin'?").
Contains("'branch-six' is not fully merged. Are you sure you want to delete it?")).
Confirm()
}).
Lines(
Contains("current-head"),
Contains("branch-five ✓").IsSelected(),
Contains("master"),
).

// Delete both local and remote branch of branch-five. We get the same popups, but the confirmation
// doesn't contain the force-delete warning.
Press(keys.Universal.Remove).
Tap(func() {
t.ExpectPopup().
Menu().
Title(Equals("Delete branch 'branch-five'?")).
Select(Contains("Delete local and remote branch")).
Confirm()
t.ExpectPopup().
Confirmation().
Title(Equals("Delete local and remote branch")).
Content(Equals("Are you sure you want to delete both 'branch-five' from your machine, and 'branch-five' from 'origin'?").
DoesNotContain("not fully merged")).
Confirm()
}).
Lines(
Contains("current-head"),
Contains("master").IsSelected(),
Expand Down
Loading