better handling of click events in list views

9 files changed, 102 insertions, 168 deletions

diff --git a/pkg/gui/branches_panel.go b/pkg/gui/branches_panel.go
index 970e383c4..790f24bd4 100644
--- a/pkg/gui/branches_panel.go
+++ b/pkg/gui/branches_panel.go
@@ -21,14 +21,6 @@ func (gui *Gui) getSelectedBranch() *commands.Branch {
 	return gui.State.Branches[selectedLine]
 }
 
-func (gui *Gui) handleBranchesClick(g *gocui.Gui, v *gocui.View) error {
-	itemCount := len(gui.State.Branches)
-	handleSelect := gui.handleBranchSelect
-	selectedLine := &gui.State.Panels.Branches.SelectedLine
-
-	return gui.handleClick(v, itemCount, selectedLine, handleSelect)
-}
-
 // may want to standardise how these select methods work
 func (gui *Gui) handleBranchSelect(g *gocui.Gui, v *gocui.View) error {
 	if gui.popupPanelFocused() {
diff --git a/pkg/gui/commits_panel.go b/pkg/gui/commits_panel.go
index 16d45ecc6..c29935f14 100644
--- a/pkg/gui/commits_panel.go
+++ b/pkg/gui/commits_panel.go
@@ -23,27 +23,6 @@ func (gui *Gui) getSelectedCommit(g *gocui.Gui) *commands.Commit {
 	return gui.State.Commits[selectedLine]
 }
 
-func (gui *Gui) handleCommitsClick(g *gocui.Gui, v *gocui.View) error {
-	if gui.popupPanelFocused() {
-		return nil
-	}
-
-	prevSelectedLineIdx := gui.State.Panels.Commits.SelectedLine
-	newSelectedLineIdx := v.SelectedLineIdx()
-
-	if newSelectedLineIdx > len(gui.State.Commits)-1 {
-		return gui.handleCommitSelect(gui.g, v)
-	}
-
-	gui.State.Panels.Commits.SelectedLine = newSelectedLineIdx
-
-	if prevSelectedLineIdx == newSelectedLineIdx && gui.currentViewName() == v.Name() {
-		return gui.handleSwitchToCommitFilesPanel(gui.g, v)
-	} else {
-		return gui.handleCommitSelect(gui.g, v)
-	}
-}
-
 func (gui *Gui) handleCommitSelect(g *gocui.Gui, v *gocui.View) error {
 	if gui.popupPanelFocused() {
 		return nil
diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go
index deffad322..f9983f583 100644
--- a/pkg/gui/files_panel.go
+++ b/pkg/gui/files_panel.go
@@ -27,27 +27,6 @@ func (gui *Gui) getSelectedFile(g *gocui.Gui) (*commands.File, error) {
 	return gui.State.Files[selectedLine], nil
 }
 
-func (gui *Gui) handleFilesClick(g *gocui.Gui, v *gocui.View) error {
-	if gui.popupPanelFocused() {
-		return nil
-	}
-
-	prevSelectedLineIdx := gui.State.Panels.Files.SelectedLine
-	newSelectedLineIdx := v.SelectedLineIdx()
-
-	if newSelectedLineIdx > len(gui.State.Files)-1 {
-		return gui.selectFile(false)
-	}
-
-	gui.State.Panels.Files.SelectedLine = newSelectedLineIdx
-
-	if prevSelectedLineIdx == newSelectedLineIdx && gui.currentViewName() == v.Name() {
-		return gui.handleFilePress(gui.g, v)
-	} else {
-		return gui.selectFile(true)
-	}
-}
-
 func (gui *Gui) handleFileSelect(g *gocui.Gui, v *gocui.View) error {
 	return gui.selectFile(false)
 }
diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go
index 85d7c5faf..79972837b 100644
--- a/pkg/gui/keybindings.go
+++ b/pkg/gui/keybindings.go
@@ -1007,33 +1007,6 @@ func (gui *Gui) GetInitialKeybindings() []*Binding {
 			Handler:     gui.handlePopFileSnapshot,
 			Description: gui.Tr.SLocalize("Undo"),
 		},
-		// click handlers
-		{
-			ViewName: "menu",
-			Key:      gocui.MouseLeft,
-			Modifier: gocui.ModNone,
-			Handler:  gui.handleMenuClick,
-		},
-		{
-			ViewName: "files",
-			Key:      gocui.MouseLeft,
-			Modifier: gocui.ModNone,
-			Handler:  gui.handleFilesClick,
-		},
-		{
-			ViewName: "branches",
-			Contexts: []string{"local-branches"},
-			Key:      gocui.MouseLeft,
-			Modifier: gocui.ModNone,
-			Handler:  gui.handleBranchesClick,
-		},
-		{
-			ViewName: "branches",
-			Contexts: []string{"remotes"},
-			Key:      gocui.MouseLeft,
-			Modifier: gocui.ModNone,
-			Handler:  gui.handleRemotesClick,
-		},
 		{
 			ViewName: "branches",
 			Contexts: []string{"remotes"},
@@ -1098,12 +1071,6 @@ func (gui *Gui) GetInitialKeybindings() []*Binding {
 			Description: gui.Tr.SLocalize("setUpstream"),
 		},
 		{
-			ViewName: "commits",
-			Key:      gocui.MouseLeft,
-			Modifier: gocui.ModNone,
-			Handler:  gui.handleCommitsClick,
-		},
-		{
 			ViewName: "stash",
 			Key:      gocui.MouseLeft,
 			Modifier: gocui.ModNone,
@@ -1146,6 +1113,7 @@ func (gui *Gui) GetInitialKeybindings() []*Binding {
 			{ViewName: listView.viewName, Contexts: []string{listView.context}, Key: 'j', Modifier: gocui.ModNone, Handler: listView.handleNextLine},
 			{ViewName: listView.viewName, Contexts: []string{listView.context}, Key: gocui.KeyArrowDown, Modifier: gocui.ModNone, Handler: listView.handleNextLine},
 			{ViewName: listView.viewName, Contexts: []string{listView.context}, Key: gocui.MouseWheelDown, Modifier: gocui.ModNone, Handler: listView.handleNextLine},
+			{ViewName: listView.viewName, Contexts: []string{listView.context}, Key: gocui.MouseLeft, Modifier: gocui.ModNone, Handler: listView.handleClick},
 		}...)
 	}
 
diff --git a/pkg/gui/list_view.go b/pkg/gui/list_view.go
index bdfa33f1c..141d0c8d1 100644
--- a/pkg/gui/list_view.go
+++ b/pkg/gui/list_view.go
@@ -3,13 +3,15 @@ package gui
 import "github.com/jesseduffield/gocui"
 
 type listView struct {
-	viewName          string
-	context           string
-	getItemsLength    func() int
-	getSelectedLine   func() *int
-	handleItemSelect  func(g *gocui.Gui, v *gocui.View) error
-	gui               *Gui
-	rendersToMainView bool
+	viewName                string
+	context                 string
+	getItemsLength          func() int
+	getSelectedLineIdxPtr   func() *int
+	handleFocus             func(g *gocui.Gui, v *gocui.View) error
+	handleItemSelect        func(g *gocui.Gui, v *gocui.View) error
+	handleClickSelectedItem func(g *gocui.Gui, v *gocui.View) error
+	gui                     *Gui
+	rendersToMainView       bool
 }
 
 func (lv *listView) handlePrevLine(g *gocui.Gui, v *gocui.View) error {
@@ -25,7 +27,7 @@ func (lv *listView) handleLineChange(change int) error {
 		return nil
 	}
 
-	lv.gui.changeSelectedLine(lv.getSelectedLine(), lv.getItemsLength(), change)
+	lv.gui.changeSelectedLine(lv.getSelectedLineIdxPtr(), lv.getItemsLength(), change)
 
 	if lv.rendersToMainView {
 		if err := lv.gui.resetOrigin(lv.gui.getMainView()); err != nil {
@@ -39,74 +41,107 @@ func (lv *listView) handleLineChange(change int) error {
 	return lv.handleItemSelect(lv.gui.g, view)
 }
 
+func (lv *listView) handleClick(g *gocui.Gui, v *gocui.View) error {
+	if !lv.gui.isPopupPanel(lv.viewName) && lv.gui.popupPanelFocused() {
+		return nil
+	}
+
+	selectedLineIdxPtr := lv.getSelectedLineIdxPtr()
+	prevSelectedLineIdx := *selectedLineIdxPtr
+	newSelectedLineIdx := v.SelectedLineIdx()
+
+	if newSelectedLineIdx > lv.getItemsLength()-1 {
+		return lv.handleFocus(lv.gui.g, v)
+	}
+
+	*selectedLineIdxPtr = newSelectedLineIdx
+
+	if prevSelectedLineIdx == newSelectedLineIdx && lv.gui.currentViewName() == lv.viewName && lv.handleClickSelectedItem != nil {
+		return lv.handleClickSelectedItem(lv.gui.g, v)
+	}
+	return lv.handleItemSelect(lv.gui.g, v)
+}
+
 func (gui *Gui) getListViews() []*listView {
 	return []*listView{
 		{
-			viewName:          "menu",
-			getItemsLength:    func() int { return gui.getMenuView().LinesHeight() },
-			getSelectedLine:   func() *int { return &gui.State.Panels.Menu.SelectedLine },
-			handleItemSelect:  gui.handleMenuSelect,
+			viewName:              "menu",
+			getItemsLength:        func() int { return gui.getMenuView().LinesHeight() },
+			getSelectedLineIdxPtr: func() *int { return &gui.State.Panels.Menu.SelectedLine },
+			handleFocus:           gui.handleMenuSelect,
+			// need to add a layer of indirection here because the callback changes during runtime
+			handleItemSelect:  gui.wrappedHandler(func() error { return gui.State.Panels.Menu.OnPress(gui.g, nil) }),
 			gui:               gui,
 			rendersToMainView: false,
 		},
 		{
-			viewName:          "files",
-			getItemsLength:    func() int { return len(gui.State.Files) },
-			getSelectedLine:   func() *int { return &gui.State.Panels.Files.SelectedLine },
-			handleItemSelect:  gui.handleFileSelect,
-			gui:               gui,
-			rendersToMainView: true,
+			viewName:                "files",
+			getItemsLength:          func() int { return len(gui.State.Files) },
+			getSelectedLineIdxPtr:   func() *int { return &gui.State.Panels.Files.SelectedLine },
+			handleFocus:             gui.wrappedHandler(func() error { return gui.selectFile(true) }),
+			handleItemSelect:        gui.wrappedHandler(func() error { return gui.selectFile(true) }),
+			handleClickSelectedItem: gui.handleFilePress,
+			gui:                     gui,
+			rendersToMainView:       true,
 		},
 		{
-			viewName:          "branches",
-			context:           "local-branches",
-			getItemsLength:    func() int { return len(gui.State.Branches) },
-			getSelectedLine:   func() *int { return &gui.State.Panels.Branches.SelectedLine },
-			handleItemSelect:  gui.handleBranchSelect,
-			gui:               gui,
-			rendersToMainView: true,
+			viewName:              "branches",
+			context:               "local-branches",
+			getItemsLength:        func() int { return len(gui.State.Branches) },
+			getSelectedLineIdxPtr: func() *int { return &gui.State.Panels.Branches.SelectedLine },
+			handleFocus:           gui.handleBranchSelect,
+			handleItemSelect:      gui.handleBranchSelect,
+			gui:                   gui,
+			rendersToMainView:     true,
 		},
 		{
-			viewName:          "branches",
-			context:           "remotes",
-			getItemsLength:    func() int { return len(gui.State.Remotes) },
-			getSelectedLine:   func() *int { return &gui.State.Panels.Remotes.SelectedLine },
-			handleItemSelect:  gui.handleRemoteSelect,
-			gui:               gui,
-			rendersToMainView: true,
+			viewName:                "branches",
+			context:                 "remotes",
+			getItemsLength:          func() int { return len(gui.State.Remotes) },
+			getSelectedLineIdxPtr:   func() *int { return &gui.State.Panels.Remotes.SelectedLine },
+			handleFocus:             gui.wrappedHandler(gui.renderRemotesWithSelection),
+			handleItemSelect:        gui.handleRemoteSelect,
+			handleClickSelectedItem: gui.handleRemoteEnter,
+			gui:                     gui,
+			rendersToMainView:       true,
 		},
 		{
-			viewName:          "branches",
-			context:           "remote-branches",
-			getItemsLength:    func() int { return len(gui.State.RemoteBranches) },
-			getSelectedLine:   func() *int { return &gui.State.Panels.RemoteBranches.SelectedLine },
-			handleItemSelect:  gui.handleRemoteBranchSelect,
-			gui:               gui,
-			rendersToMainView: true,
+			viewName:              "branches",
+			context:               "remote-branches",
+			getItemsLength:        func() int { return len(gui.State.RemoteBranches) },
+			getSelectedLineIdxPtr: func() *int { return &gui.State.Panels.RemoteBranches.SelectedLine },
+			handleFocus:           gui.handleRemoteBranchSelect,
+			handleItemSelect:      gui.handleRemoteBranchSelect,
+			gui:                   gui,
+			rendersToMainView:     true,
 		},
 		{
-			viewName:          "commits",
-			getItemsLength:    func() int { return len(gui.State.Commits) },
-			getSelectedLine:   func() *int { return &gui.State.Panels.Commits.SelectedLine },
-			handleItemSelect:  gui.handleCommitSelect,
-			gui:               gui,
-			rendersToMainView: true,
+			viewName:                "commits",
+			getItemsLength:          func() int { return len(gui.State.Commits) },
+			getSelectedLineIdxPtr:   func() *int { return &gui.State.Panels.Commits.SelectedLine },
+			handleFocus:             gui.handleCommitSelect,
+			handleItemSelect:        gui.handleCommitSelect,
+			handleClickSelectedItem: gui.handleSwitchToCommitFilesPanel,
+			gui:                     gui,
+			rendersToMainView:       true,
 		},
 		{
-			viewName:          "stash",
-			getItemsLength:    func() int { return len(gui.State.StashEntries) },
-			getSelectedLine:   func() *int { return &gui.State.Panels.Stash.SelectedLine },
-			handleItemSelect:  gui.handleStashEntrySelect,
-			gui:               gui,
-			rendersToMainView: true,
+			viewName:              "stash",
+			getItemsLength:        func() int { return len(gui.State.StashEntries) },
+			getSelectedLineIdxPtr: func() *int { return &gui.State.Panels.Stash.SelectedLine },
+			handleFocus:           gui.handleStashEntrySelect,
+			handleItemSelect:      gui.handleStashEntrySelect,
+			gui:                   gui,
+			rendersToMainView:     true,
 		},
 		{
-			viewName:          "commitFiles",
-			getItemsLength:    func() int { return len(gui.State.CommitFiles) },
-			getSelectedLine:   func() *int { return &gui.State.Panels.CommitFiles.SelectedLine },
-			handleItemSelect:  gui.handleCommitFileSelect,
-			gui:               gui,
-			rendersToMainView: true,
+			viewName:              "commitFiles",
+			getItemsLength:        func() int { return len(gui.State.CommitFiles) },
+			getSelectedLineIdxPtr: func() *int { return &gui.State.Panels.CommitFiles.SelectedLine },
+			handleFocus:           gui.handleCommitFileSelect,
+			handleItemSelect:      gui.handleCommitFileSelect,
+			gui:                   gui,
+			rendersToMainView:     true,
 		},
 	}
 }
diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go
index a7e3f2e14..141925953 100644
--- a/pkg/gui/menu_panel.go
+++ b/pkg/gui/menu_panel.go
@@ -89,15 +89,3 @@ func (gui *Gui) createMenu(title string, items interface{}, itemCount int, handl
 	})
 	return nil
 }
-
-func (gui *Gui) handleMenuClick(g *gocui.Gui, v *gocui.View) error {
-	itemCount := gui.State.MenuItemCount
-	handleSelect := gui.handleMenuSelect
-	selectedLine := &gui.State.Panels.Menu.SelectedLine
-
-	if err := gui.handleClick(v, itemCount, selectedLine, handleSelect); err != nil {
-		return err
-	}
-
-	return gui.State.Panels.Menu.OnPress(g, v)
-}
diff --git a/pkg/gui/remote_branches_panel.go b/pkg/gui/remote_branches_panel.go
index e4dfefa26..af9835999 100644
--- a/pkg/gui/remote_branches_panel.go
+++ b/pkg/gui/remote_branches_panel.go
@@ -21,14 +21,6 @@ func (gui *Gui) getSelectedRemoteBranch() *commands.RemoteBranch {
 	return gui.State.RemoteBranches[selectedLine]
 }
 
-func (gui *Gui) handleRemoteBranchesClick(g *gocui.Gui, v *gocui.View) error {
-	itemCount := len(gui.State.RemoteBranches)
-	handleSelect := gui.handleRemoteBranchSelect
-	selectedLine := &gui.State.Panels.RemoteBranches.SelectedLine
-
-	return gui.handleClick(v, itemCount, selectedLine, handleSelect)
-}
-
 func (gui *Gui) handleRemoteBranchSelect(g *gocui.Gui, v *gocui.View) error {
 	if gui.popupPanelFocused() {
 		return nil
diff --git a/pkg/gui/remotes_panel.go b/pkg/gui/remotes_panel.go
index a562202b3..76b174e89 100644
--- a/pkg/gui/remotes_panel.go
+++ b/pkg/gui/remotes_panel.go
@@ -21,14 +21,6 @@ func (gui *Gui) getSelectedRemote() *commands.Remote {
 	return gui.State.Remotes[selectedLine]
 }
 
-func (gui *Gui) handleRemotesClick(g *gocui.Gui, v *gocui.View) error {
-	itemCount := len(gui.State.Remotes)
-	handleSelect := gui.handleRemoteSelect
-	selectedLine := &gui.State.Panels.Remotes.SelectedLine
-
-	return gui.handleClick(v, itemCount, selectedLine, handleSelect)
-}
-
 func (gui *Gui) handleRemoteSelect(g *gocui.Gui, v *gocui.View) error {
 	if gui.popupPanelFocused() {
 		return nil
diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go
index 8c72252cb..e671165a3 100644
--- a/pkg/gui/view_helpers.go
+++ b/pkg/gui/view_helpers.go
@@ -463,3 +463,12 @@ func (gui *Gui) handleClick(v *gocui.View, itemCount int, selectedLine *int, han
 
 	return handleSelect(gui.g, v)
 }
+
+// often gocui wants functions in the form `func(g *gocui.Gui, v *gocui.View) error`
+// but sometimes we just have a function that returns an error, so this is a
+// convenience wrapper to give gocui what it wants.
+func (gui *Gui) wrappedHandler(f func() error) func(g *gocui.Gui, v *gocui.View) error {
+	return func(g *gocui.Gui, v *gocui.View) error {
+		return f()
+	}
+}