From 7f238de1ea32769648ecb698724ed4e36f347a0a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 10:13:36 +0200 Subject: [PATCH] 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 | 14 ++++++++-- pkg/gocui/modified_click_test.go | 46 ++++++++++++++++++++++++++++++++ pkg/gocui/tcell_driver.go | 14 +++++++--- pkg/gocui/tcell_driver_test.go | 40 ++++++++++++++++++++++++--- 4 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 pkg/gocui/modified_click_test.go diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 5f955f96e..f361f7302 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -1841,11 +1841,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 diff --git a/pkg/gocui/modified_click_test.go b/pkg/gocui/modified_click_test.go new file mode 100644 index 000000000..5b84f9244 --- /dev/null +++ b/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") +} diff --git a/pkg/gocui/tcell_driver.go b/pkg/gocui/tcell_driver.go index e7f3997af..d8e97b9b6 100644 --- a/pkg/gocui/tcell_driver.go +++ b/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 } } diff --git a/pkg/gocui/tcell_driver_test.go b/pkg/gocui/tcell_driver_test.go index 9038e73ce..99a9f8966 100644 --- a/pkg/gocui/tcell_driver_test.go +++ b/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