Keep mouse gesture modifiers stable

Bindings match modifiers exactly, so a modified press must not turn into
an unmodified drag or release halfway through the gesture. Capture the
press-time modifiers once and carry them until the button is released.

This also makes unbound modified clicks no-ops instead of silently
invoking plain-click behavior.

pkg/gocui/gui.go

@@ -1847,11 +1847,21 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
 				break
 			}
 		}
-		if ev.Key.KeyName() == MouseLeft && ev.Key.Mod()&ModMotion == 0 {
+		// Bindings match modifiers exactly, so a gesture with a keyboard modifier held
+		// is a gesture of its own, and the bindings that act on a plain click pass it
+		// by. It therefore has to leave the view as it found it: the cursor stays where
+		// the selection is (a list view draws its selection at the cursor), and no
+		// mouse capture begins for a drag that no binding will extend. ModMotion comes
+		// from the mouse rather than the keyboard, and every drag carries it, so it is
+		// masked out here.
+		gestureIsModified := ev.Key.Mod()&^ModMotion != ModNone
+
+		if ev.Key.KeyName() == MouseLeft && ev.Key.Mod() == ModNone {
 			g.captureMouse(v)
 		}
 
-		if !IsMouseScrollKey(ev.Key.KeyName()) && ev.Key.KeyName() != MouseRelease {
+		if !IsMouseScrollKey(ev.Key.KeyName()) && ev.Key.KeyName() != MouseRelease &&
+			!gestureIsModified {
 			cursorX, cursorY := newCx, newCy
 			// A captured drag can report positions outside the view; keep the
 			// view cursor inside its bounds in that case. Handlers still get

pkg/gocui/modified_click_test.go

@@ -0,0 +1,46 @@
+package gocui
+
+import (
+	"testing"
+
+	"github.com/gdamore/tcell/v3"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAModifiedClickNoBindingWantsLeavesTheViewAlone(t *testing.T) {
+	t.Cleanup(resetMouseState)
+	resetMouseState()
+	g := newTestGui(t)
+	view, _ := g.SetView("list", 0, 0, 20, 10, 0)
+
+	clicks := 0
+	g.SetViewClickBinding(&ViewMouseBinding{
+		ViewName: "list",
+		Key:      MouseLeft,
+		Handler: func(ViewMouseBindingOpts) error {
+			clicks++
+			return nil
+		},
+	})
+
+	click := func(y int, modifier tcell.ModMask) {
+		for _, button := range []tcell.ButtonMask{tcell.ButtonPrimary, tcell.ButtonNone} {
+			event := gocuiEventFromTcellEvent(
+				tcell.NewEventMouse(view.x0+1, y, button, modifier))
+			assert.NoError(t, g.onKey(&event))
+		}
+	}
+
+	// A plain click is the binding's, and moves the cursor it acts on.
+	click(view.y0+4, tcell.ModNone)
+	assert.Equal(t, 1, clicks)
+	assert.Equal(t, 3, view.CursorY())
+
+	// An alt-click is nobody's here, bindings matching modifiers exactly. It has to
+	// leave the cursor where the selection is, or a list view would draw its
+	// selection on a line its owner never selected.
+	click(view.y0+8, tcell.ModAlt)
+	assert.Equal(t, 1, clicks)
+	assert.Equal(t, 3, view.CursorY())
+	assert.Nil(t, g.mouseCapture, "and no drag begins for it either")
+}

pkg/gocui/tcell_driver.go

@@ -209,6 +209,7 @@ const (
 
 var (
 	lastMouseKey tcell.ButtonMask = tcell.ButtonNone
+	lastMouseMod tcell.ModMask    = tcell.ModNone
 	dragState                     = NOT_DRAGGING
 	lastX                         = 0
 	lastY                         = 0
@@ -377,6 +378,12 @@ func gocuiEventFromTcellEvent(tev tcell.Event) GocuiEvent {
 		if button != tcell.ButtonNone && lastMouseKey == tcell.ButtonNone {
 			newButtonPress = true
 			lastMouseKey = button
+			// The keyboard modifiers held at press time apply to the whole gesture:
+			// the press, every drag event, and the release. Snapshotting them here
+			// keeps a modified press from producing events that match unmodified
+			// bindings, and ignores modifier changes while the button is held.
+			lastMouseMod = tev.Modifiers()
+			mouseMod = Modifier(lastMouseMod)
 			switch button {
 			case tcell.ButtonPrimary:
 				mouseKey = MouseLeft
@@ -402,7 +409,8 @@ func gocuiEventFromTcellEvent(tev tcell.Event) GocuiEvent {
 				case tcell.ButtonMiddle:
 				default:
 				}
-				mouseMod = ModNone
+				mouseMod = Modifier(lastMouseMod)
+				lastMouseMod = tcell.ModNone
 				lastMouseKey = tcell.ButtonNone
 			}
 		default:
@@ -433,10 +441,10 @@ func gocuiEventFromTcellEvent(tev tcell.Event) GocuiEvent {
 				// reaches drag bindings instead of being delivered with the
 				// default MouseRelease key.
 				dragState = DRAGGING
-				mouseMod = ModMotion
+				mouseMod = Modifier(lastMouseMod) | ModMotion
 				mouseKey = MouseLeft
 			case DRAGGING:
-				mouseMod = ModMotion
+				mouseMod = Modifier(lastMouseMod) | ModMotion
 				mouseKey = MouseLeft
 			}
 		}

pkg/gocui/tcell_driver_test.go

@@ -36,21 +36,55 @@ func TestMouseReleaseAfterDragIsMouseEvent(t *testing.T) {
 	assert.Equal(t, MouseRelease, releaseEvent.Key.KeyName())
 }
 
-func TestMouseReleaseDoesNotKeepPressModifiers(t *testing.T) {
+func TestWholeGestureCarriesPressModifiers(t *testing.T) {
 	t.Cleanup(resetMouseState)
 	resetMouseState()
 
-	gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 2, tcell.ButtonPrimary, tcell.ModAlt))
-	gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 3, tcell.ButtonPrimary, tcell.ModAlt))
+	pressEvent := gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 2, tcell.ButtonPrimary, tcell.ModAlt))
+	dragEvent := gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 3, tcell.ButtonPrimary, tcell.ModAlt))
 	releaseEvent := gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 3, tcell.ButtonNone, tcell.ModAlt))
 
+	assert.Equal(t, eventMouse, pressEvent.Type)
+	assert.Equal(t, MouseLeft, pressEvent.Key.KeyName())
+	assert.Equal(t, ModAlt, pressEvent.Key.Mod())
+	assert.Equal(t, eventMouse, dragEvent.Type)
+	assert.Equal(t, MouseLeft, dragEvent.Key.KeyName())
+	assert.Equal(t, ModAlt|ModMotion, dragEvent.Key.Mod())
 	assert.Equal(t, eventMouse, releaseEvent.Type)
 	assert.Equal(t, MouseRelease, releaseEvent.Key.KeyName())
+	assert.Equal(t, ModAlt, releaseEvent.Key.Mod())
+}
+
+func TestModifierChangesWhileButtonHeldAreIgnored(t *testing.T) {
+	t.Cleanup(resetMouseState)
+	resetMouseState()
+
+	gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 2, tcell.ButtonPrimary, tcell.ModNone))
+	dragEvent := gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 3, tcell.ButtonPrimary, tcell.ModAlt))
+	releaseEvent := gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 3, tcell.ButtonNone, tcell.ModAlt))
+
+	assert.Equal(t, ModMotion, dragEvent.Key.Mod())
 	assert.Equal(t, ModNone, releaseEvent.Key.Mod())
 }
 
+func TestModifiedClickWithoutDragCarriesModifierOnPressAndRelease(t *testing.T) {
+	t.Cleanup(resetMouseState)
+	resetMouseState()
+
+	pressEvent := gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 2, tcell.ButtonPrimary, tcell.ModShift))
+	releaseEvent := gocuiEventFromTcellEvent(tcell.NewEventMouse(1, 2, tcell.ButtonNone, tcell.ModShift))
+
+	assert.Equal(t, eventMouse, pressEvent.Type)
+	assert.Equal(t, MouseLeft, pressEvent.Key.KeyName())
+	assert.Equal(t, ModShift, pressEvent.Key.Mod())
+	assert.Equal(t, eventMouse, releaseEvent.Type)
+	assert.Equal(t, MouseRelease, releaseEvent.Key.KeyName())
+	assert.Equal(t, ModShift, releaseEvent.Key.Mod())
+}
+
 func resetMouseState() {
 	lastMouseKey = tcell.ButtonNone
+	lastMouseMod = tcell.ModNone
 	dragState = NOT_DRAGGING
 	lastX = 0
 	lastY = 0