Skip to content
Closed
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
10 changes: 10 additions & 0 deletions pkg/commands/git_commands/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ func (self *BranchCommands) LocalDelete(branch string, force bool) error {
return self.cmd.New(cmdArgs).Run()
}

// LocalDelete delete many branches locally
func (self *BranchCommands) LocalDeleteMany(branches []string, force bool) error {
cmdArgs := NewGitCmd("branch").
ArgIfElse(force, "-D", "-d").
Arg(branches...).
ToArgv()

return self.cmd.New(cmdArgs).Run()
}

// Checkout checks out a branch (or commit), with --force if you set the force arg to true
type CheckoutOptions struct {
Force bool
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/user_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ type KeybindingUniversalConfig struct {
Confirm string `yaml:"confirm"`
ConfirmInEditor string `yaml:"confirmInEditor"`
Remove string `yaml:"remove"`
RemoveMany string `yaml:"removeMany"`
New string `yaml:"new"`
Edit string `yaml:"edit"`
OpenFile string `yaml:"openFile"`
Expand Down Expand Up @@ -829,6 +830,7 @@ func GetDefaultConfig() *UserConfig {
Confirm: "<enter>",
ConfirmInEditor: "<a-enter>",
Remove: "d",
RemoveMany: "<c-d>",
New: "n",
Edit: "e",
OpenFile: "o",
Expand Down
54 changes: 54 additions & 0 deletions pkg/gui/controllers/branches_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ func (self *BranchesController) GetKeybindings(opts types.KeybindingsOpts) []*ty
OpensMenu: true,
DisplayOnScreen: true,
},
{
Key: opts.GetKey(opts.Config.Universal.RemoveMany),
Handler: self.withItem(self.deleteMultiple),
GetDisabledReason: self.require(self.itemsSelected(self.branchesAreReal)),
Copy link

Choose a reason for hiding this comment

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

IMO introducing a new "RemoveMany" command might not be strictly useful. I think we should also enable deleting multiple branches using multi-select

Copy link
Owner

Choose a reason for hiding this comment

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

I agree: there's no need for a new keybinding here; we just need to do something different if the user presses the usual 'remove' key when multiple branches are selected

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree as well, but it's not easy at all (the "just" in your comment suggests that it might be).

The bespoke extra command that's added in this PR deletes only local branches; this is easy to do, and if it's an extra command, it could be justifiable if the command name and tooltip makes this very clear (which this PR doesn't). However, if we include the multi-select functionality in the normal delete menu, then I'd expect this to work the same as for a single selection:

  • it should offer to delete only the local branches, or only the remote branches, or both (the latter two should be disabled if not all of the selected branches have upstream branches)
  • it should prompt for confirmation if some of the local branches aren't fully merged

As an intermediate solution that might be good enough for now, and will be a lot easier, we could simply disable the remoteDeleteItem and deleteBothItem in case there's a multiselection, with a DisabledReason saying that this is not supported for multiselections.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, turns out that it's not that hard (sometimes you have to try it to find out). Here's a PR: #4073

Basically, the main change is that whenever we make a decision, we replace that with either lo.Every or lo.Some on the selected branches. The most tricky part was deleting several remote branches: we have to group them by remote and then loop over the remotes and issue one push --delete call with the selected branches for that remote.

Description: self.c.Tr.Delete,
Tooltip: self.c.Tr.BranchDeleteTooltip,
OpensMenu: true,
DisplayOnScreen: true,
},
{
Key: opts.GetKey(opts.Config.Branches.RebaseBranch),
Handler: opts.Guards.OutsideFilterMode(self.withItem(self.rebase)),
Expand Down Expand Up @@ -524,6 +533,10 @@ func (self *BranchesController) localDelete(branch *models.Branch) error {
return self.c.Helpers().BranchesHelper.ConfirmLocalDelete(branch)
}

func (self *BranchesController) localDeleteMany(branches []*models.Branch) error {
return self.c.Helpers().BranchesHelper.ConfirmLocalDeleteMany(branches)
}

func (self *BranchesController) remoteDelete(branch *models.Branch) error {
return self.c.Helpers().BranchesHelper.ConfirmDeleteRemote(branch.UpstreamRemote, branch.UpstreamBranch)
}
Expand All @@ -532,6 +545,37 @@ func (self *BranchesController) localAndRemoteDelete(branch *models.Branch) erro
return self.c.Helpers().BranchesHelper.ConfirmLocalAndRemoteDelete(branch)
}

func (self *BranchesController) deleteMultiple(branch *models.Branch) error {
checkedOutBranch := self.c.Helpers().Refs.GetCheckedOutRef()
selectedBranches, _, _ := self.context().GetSelectedItems()

localDeleteItems := &types.MenuItem{
Label: self.c.Tr.DeleteLocalBranch,
Key: 'c',
OnPress: func() error {
return self.localDeleteMany(selectedBranches)
},
}

for _, branch := range selectedBranches {
if checkedOutBranch.Name == branch.Name {
localDeleteItems.DisabledReason = &types.DisabledReason{Text: self.c.Tr.CantDeleteCheckOutBranch}
}
}

menuTitle := utils.ResolvePlaceholderString(
self.c.Tr.DeleteBranchTitle,
map[string]string{
"selectedBranchName": branch.Name,
},
)

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

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

Expand Down Expand Up @@ -787,6 +831,16 @@ func (self *BranchesController) branchIsReal(branch *models.Branch) *types.Disab
return nil
}

func (self *BranchesController) branchesAreReal(branches []*models.Branch) *types.DisabledReason {
for _, branch := range branches {
if !branch.IsRealBranch() {
return &types.DisabledReason{Text: self.c.Tr.SelectedItemIsNotABranch}
}
}

return nil
}

func (self *BranchesController) notMergingIntoYourself(branch *models.Branch) *types.DisabledReason {
selectedBranchName := branch.Name
checkedOutBranch := self.c.Helpers().Refs.GetCheckedOutRef().Name
Expand Down
19 changes: 19 additions & 0 deletions pkg/gui/controllers/helpers/branches_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,25 @@ func NewBranchesHelper(c *HelperCommon, worktreeHelper *WorktreeHelper) *Branche
}
}

func (self *BranchesHelper) ConfirmLocalDeleteMany(branches []*models.Branch) error {
branchNames := make([]string, len(branches))

for i, branch := range branches {
branchNames[i] = branch.Name
}
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.LocalDeleteMany(branchNames, true); err != nil {
return err
}
return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}})
})
}

return doDelete()
}

func (self *BranchesHelper) ConfirmLocalDelete(branch *models.Branch) error {
if self.checkedOutByOtherWorktree(branch) {
return self.promptWorktreeBranchDelete(branch)
Expand Down