Skip to content

Commit

Permalink
Enforce single-item selection in various actions
Browse files Browse the repository at this point in the history 
We want to show an error when the user tries to invoke an action that expects only
a single item to be selected.

We're using the GetDisabledReason field to enforce this (as well as DisabledReason
on menu items).

I've created a ListControllerTrait to store some shared convenience functions for this.
  • Loading branch information
@jesseduffield
jesseduffield committed Jan 15, 2024
1 parent 0017ff0 commit 8a8072a
Show file tree
Hide file tree
Showing 43 changed files with 849 additions and 748 deletions.
8 changes: 8 additions & 0 deletions pkg/gui/context/traits/list_cursor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,18 @@ func (self *ListCursor) CancelRangeSelect() {
self.rangeSelectMode = RangeSelectModeNone
}

// Returns true if we are in range select mode. Note that we may be in range select
// mode and still only selecting a single item. See AreMultipleItemsSelected below.
func (self *ListCursor) IsSelectingRange() bool {
return self.rangeSelectMode != RangeSelectModeNone
}

// Returns true if we are in range select mode and selecting multiple items
func (self *ListCursor) AreMultipleItemsSelected() bool {
startIdx, endIdx := self.GetSelectionRange()
return startIdx != endIdx
}

func (self *ListCursor) GetSelectionRange() (int, int) {
if self.IsSelectingRange() {
return utils.MinMax(self.selectedIdx, self.rangeStartIdx)
Expand Down
73 changes: 35 additions & 38 deletions pkg/gui/controllers/basic_commits_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,50 +22,61 @@ type ContainsCommits interface {

type BasicCommitsController struct {
baseController
*ListControllerTrait[*models.Commit]
c *ControllerCommon
context ContainsCommits
}

func NewBasicCommitsController(controllerCommon *ControllerCommon, context ContainsCommits) *BasicCommitsController {
func NewBasicCommitsController(c *ControllerCommon, context ContainsCommits) *BasicCommitsController {
return &BasicCommitsController{
baseController: baseController{},
c: controllerCommon,
c: c,
context: context,
ListControllerTrait: NewListControllerTrait[*models.Commit](
c,
context,
context.GetSelected,
),
}
}

func (self *BasicCommitsController) GetKeybindings(opts types.KeybindingsOpts) []*types.Binding {
bindings := []*types.Binding{
{
Key: opts.GetKey(opts.Config.Commits.CheckoutCommit),
Handler: self.checkSelected(self.checkout),
Description: self.c.Tr.CheckoutCommit,
Key: opts.GetKey(opts.Config.Commits.CheckoutCommit),
Handler: self.withItem(self.checkout),
GetDisabledReason: self.require(self.singleItemSelected()),
Description: self.c.Tr.CheckoutCommit,
},
{
Key: opts.GetKey(opts.Config.Commits.CopyCommitAttributeToClipboard),
Handler: self.checkSelected(self.copyCommitAttribute),
Description: self.c.Tr.CopyCommitAttributeToClipboard,
OpensMenu: true,
Key: opts.GetKey(opts.Config.Commits.CopyCommitAttributeToClipboard),
Handler: self.withItem(self.copyCommitAttribute),
GetDisabledReason: self.require(self.singleItemSelected()),
Description: self.c.Tr.CopyCommitAttributeToClipboard,
OpensMenu: true,
},
{
Key: opts.GetKey(opts.Config.Commits.OpenInBrowser),
Handler: self.checkSelected(self.openInBrowser),
Description: self.c.Tr.OpenCommitInBrowser,
Key: opts.GetKey(opts.Config.Commits.OpenInBrowser),
Handler: self.withItem(self.openInBrowser),
GetDisabledReason: self.require(self.singleItemSelected()),
Description: self.c.Tr.OpenCommitInBrowser,
},
{
Key: opts.GetKey(opts.Config.Universal.New),
Handler: self.checkSelected(self.newBranch),
Description: self.c.Tr.CreateNewBranchFromCommit,
Key: opts.GetKey(opts.Config.Universal.New),
Handler: self.withItem(self.newBranch),
GetDisabledReason: self.require(self.singleItemSelected()),
Description: self.c.Tr.CreateNewBranchFromCommit,
},
{
Key: opts.GetKey(opts.Config.Commits.ViewResetOptions),
Handler: self.checkSelected(self.createResetMenu),
Description: self.c.Tr.ViewResetOptions,
OpensMenu: true,
Key: opts.GetKey(opts.Config.Commits.ViewResetOptions),
Handler: self.withItem(self.createResetMenu),
GetDisabledReason: self.require(self.singleItemSelected()),
Description: self.c.Tr.ViewResetOptions,
OpensMenu: true,
},
{
Key: opts.GetKey(opts.Config.Commits.CherryPickCopy),
Handler: self.checkSelected(self.copyRange),
Handler: self.withItem(self.copyRange),
Description: self.c.Tr.CherryPickCopy,
},
{
Expand All @@ -74,30 +85,16 @@ func (self *BasicCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
Description: self.c.Tr.ResetCherryPick,
},
{
Key: opts.GetKey(opts.Config.Universal.OpenDiffTool),
Handler: self.checkSelected(self.openDiffTool),
Description: self.c.Tr.OpenDiffTool,
Key: opts.GetKey(opts.Config.Universal.OpenDiffTool),
Handler: self.withItem(self.openDiffTool),
GetDisabledReason: self.require(self.singleItemSelected()),
Description: self.c.Tr.OpenDiffTool,
},
}

return bindings
}

func (self *BasicCommitsController) checkSelected(callback func(*models.Commit) error) func() error {
return func() error {
commit := self.context.GetSelected()
if commit == nil {
return nil
}

return callback(commit)
}
}

func (self *BasicCommitsController) Context() types.Context {
return self.context
}

func (self *BasicCommitsController) copyCommitAttribute(commit *models.Commit) error {
return self.c.Menu(types.CreateMenuOptions{
Title: self.c.Tr.Actions.CopyCommitAttributeToClipboard,
Expand Down
57 changes: 32 additions & 25 deletions pkg/gui/controllers/bisect_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,31 @@ import (

type BisectController struct {
baseController
*ListControllerTrait[*models.Commit]
c *ControllerCommon
}

var _ types.IController = &BisectController{}

func NewBisectController(
common *ControllerCommon,
c *ControllerCommon,
) *BisectController {
return &BisectController{
baseController: baseController{},
c: common,
c: c,
ListControllerTrait: NewListControllerTrait[*models.Commit](
c,
c.Contexts().LocalCommits,
c.Contexts().LocalCommits.GetSelected,
),
}
}

func (self *BisectController) GetKeybindings(opts types.KeybindingsOpts) []*types.Binding {
bindings := []*types.Binding{
{
Key: opts.GetKey(opts.Config.Commits.ViewBisectOptions),
Handler: opts.Guards.OutsideFilterMode(self.checkSelected(self.openMenu)),
Handler: opts.Guards.OutsideFilterMode(self.withItem(self.openMenu)),
Description: self.c.Tr.ViewBisectOptions,
OpensMenu: true,
},
Expand Down Expand Up @@ -70,9 +76,19 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c
// If we have a current sha already, then we always want to use that one. If
// not, we're still picking the initial commits before we really start, so
// use the selected commit in that case.
shaToMark := lo.Ternary(info.GetCurrentSha() != "", info.GetCurrentSha(), commit.Sha)

bisecting := info.GetCurrentSha() != ""
shaToMark := lo.Ternary(bisecting, info.GetCurrentSha(), commit.Sha)
shortShaToMark := utils.ShortSha(shaToMark)

// For marking a commit as bad, when we're not already bisecting, we require
// a single item selected, but once we are bisecting, it doesn't matter because
// the action applies to the HEAD commit rather than the selected commit.
var singleItemIfNotBisecting *types.DisabledReason
if !bisecting {
singleItemIfNotBisecting = self.require(self.singleItemSelected())()
}

menuItems := []*types.MenuItem{
{
Label: fmt.Sprintf(self.c.Tr.Bisect.Mark, shortShaToMark, info.NewTerm()),
Expand All @@ -84,7 +100,8 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c

return self.afterMark(selectCurrentAfter, waitToReselect)
},
Key: 'b',
DisabledReason: singleItemIfNotBisecting,
Key: 'b',
},
{
Label: fmt.Sprintf(self.c.Tr.Bisect.Mark, shortShaToMark, info.OldTerm()),
Expand All @@ -96,7 +113,8 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c

return self.afterMark(selectCurrentAfter, waitToReselect)
},
Key: 'g',
DisabledReason: singleItemIfNotBisecting,
Key: 'g',
},
{
Label: fmt.Sprintf(self.c.Tr.Bisect.SkipCurrent, shortShaToMark),
Expand All @@ -108,7 +126,8 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c

return self.afterMark(selectCurrentAfter, waitToReselect)
},
Key: 's',
DisabledReason: singleItemIfNotBisecting,
Key: 's',
},
}
if info.GetCurrentSha() != "" && info.GetCurrentSha() != commit.Sha {
Expand All @@ -122,7 +141,8 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c

return self.afterMark(selectCurrentAfter, waitToReselect)
},
Key: 'S',
DisabledReason: self.require(self.singleItemSelected())(),
Key: 'S',
}))
}
menuItems = append(menuItems, lo.ToPtr(types.MenuItem{
Expand Down Expand Up @@ -157,7 +177,8 @@ func (self *BisectController) openStartBisectMenu(info *git_commands.BisectInfo,

return self.c.Helpers().Bisect.PostBisectCommandRefresh()
},
Key: 'b',
DisabledReason: self.require(self.singleItemSelected())(),
Key: 'b',
},
{
Label: fmt.Sprintf(self.c.Tr.Bisect.MarkStart, commit.ShortSha(), info.OldTerm()),
Expand All @@ -173,7 +194,8 @@ func (self *BisectController) openStartBisectMenu(info *git_commands.BisectInfo,

return self.c.Helpers().Bisect.PostBisectCommandRefresh()
},
Key: 'g',
DisabledReason: self.require(self.singleItemSelected())(),
Key: 'g',
},
{
Label: self.c.Tr.Bisect.ChooseTerms,
Expand Down Expand Up @@ -273,21 +295,6 @@ func (self *BisectController) selectCurrentBisectCommit() {
}
}

func (self *BisectController) checkSelected(callback func(*models.Commit) error) func() error {
return func() error {
commit := self.context().GetSelected()
if commit == nil {
return nil
}

return callback(commit)
}
}

func (self *BisectController) Context() types.Context {
return self.context()
}

func (self *BisectController) context() *context.LocalCommitsContext {
return self.c.Contexts().LocalCommits
}
Loading

0 comments on commit 8a8072a

Please sign in to comment.