Make double-click detection more precise (#6123)

Lazygit decided whether two clicks form a double click based on the
times it handled them. This can lead to false results: if the first
click takes the UI thread more then 500ms to handle, then the second
click is not detected as a double click even when it happened quickly
after the first. Conversely, if two clicks happen a second apart while
the main loop is busy, they are handled quickly once the main loop
unblocks, and got detected as a double-click.

I never actually ran into either of these problems in practice, and we
try hard to prevent lazygit from blocking the UI for longer than
necessary. I only noticed this because an integration test that performs
a double click occasionally fails when the CI runner is under heavy
load.

Fix this by recording the times at which the events were received, and
use those for double-click detection.

pkg/gocui/double_click_test.go

@@ -2,6 +2,7 @@ package gocui
 
 import (
 	"testing"
+	"time"
 
 	"github.com/gdamore/tcell/v3"
 	"github.com/stretchr/testify/assert"
@@ -66,3 +67,39 @@ func TestASwallowedClickIsNoHalfOfADoubleClick(t *testing.T) {
 
 	assert.Equal(t, []bool{false}, doubleClicks)
 }
+
+func TestDoubleClicksAreTimedByWhenTheClicksWereRead(t *testing.T) {
+	t.Cleanup(resetMouseState)
+	resetMouseState()
+	g := newTestGui(t)
+	view, _ := g.SetView("list", 0, 0, 20, 10, 0)
+	doubleClicks := []bool{}
+	g.SetViewClickBinding(&ViewMouseBinding{
+		ViewName: "list",
+		Key:      MouseLeft,
+		Handler: func(opts ViewMouseBindingOpts) error {
+			doubleClicks = append(doubleClicks, opts.IsDoubleClick)
+			return nil
+		},
+	})
+
+	click := func(readAt time.Time) {
+		for _, buttons := range []tcell.ButtonMask{tcell.ButtonPrimary, tcell.ButtonNone} {
+			tev := tcell.NewEventMouse(view.x0+1, view.y0+1, buttons, tcell.ModNone)
+			tev.SetEventTime(readAt)
+			event := gocuiEventFromTcellEvent(tev)
+			assert.NoError(t, g.onKey(&event))
+		}
+	}
+
+	// The clicks are handled right after each other, long after they were read,
+	// as they are when they wait in the queue while the main loop is busy.
+	readAt := time.Now().Add(-time.Minute)
+	click(readAt)
+	click(readAt.Add(DOUBLE_CLICK_THRESHOLD + time.Millisecond))
+	click(readAt.Add(DOUBLE_CLICK_THRESHOLD + 100*time.Millisecond))
+
+	assert.Equal(t,
+		[]bool{false, false, true},
+		doubleClicks)
+}

pkg/gocui/gui.go

@@ -1825,7 +1825,7 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
 		if IsMouseKey(ev.Key) {
 			mouseOpts = ViewMouseBindingOpts{
 				X: newX, Y: newY, Key: ev.Key.KeyName(),
-				IsDoubleClick: g.isDoubleClick(newX, newY, ev.Key.KeyName(), v),
+				IsDoubleClick: g.isDoubleClick(newX, newY, ev.Key.KeyName(), v, ev.When),
 			}
 
 			// Dispatch bindings that opt into firing while a popup panel is focused
@@ -1835,7 +1835,7 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
 				return err
 			}
 			if matched {
-				g.recordClickInfo(newX, newY, ev.Key.KeyName(), v)
+				g.recordClickInfo(newX, newY, ev.Key.KeyName(), v, ev.When)
 				return nil
 			}
 		}
@@ -1899,7 +1899,7 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
 		}
 
 		if IsMouseKey(ev.Key) {
-			g.recordClickInfo(newX, newY, ev.Key.KeyName(), v)
+			g.recordClickInfo(newX, newY, ev.Key.KeyName(), v, ev.When)
 			matched, err := g.execMouseKeybindings(v, ev, mouseOpts, false)
 			if err != nil {
 				return err
@@ -1933,8 +1933,10 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
 }
 
 // isDoubleClick reports whether this click follows one just like it, closely
-// enough in time to count as a double click.
-func (g *Gui) isDoubleClick(x, y int, key KeyName, v *View) bool {
+// enough in time to count as a double click. It goes by when the clicks were
+// read. The second click may wait in the event queue while the first one is
+// handled, and that wait doesn't count.
+func (g *Gui) isDoubleClick(x, y int, key KeyName, v *View, when time.Time) bool {
 	return g.lastClick != nil &&
 		!IsMouseScrollKey(key) &&
 		key != MouseRelease &&
@@ -1942,13 +1944,13 @@ func (g *Gui) isDoubleClick(x, y int, key KeyName, v *View) bool {
 		y == g.lastClick.y &&
 		key == g.lastClick.key &&
 		v.Name() == g.lastClick.viewName &&
-		time.Now().Before(g.lastClick.time.Add(DOUBLE_CLICK_THRESHOLD))
+		when.Before(g.lastClick.time.Add(DOUBLE_CLICK_THRESHOLD))
 }
 
 // recordClickInfo remembers this click as the one a following click is compared
 // against. Only the clicks that reach a binding are recorded, so a click the
 // client rejects leaves double-click detection where it was.
-func (g *Gui) recordClickInfo(x, y int, key KeyName, v *View) {
+func (g *Gui) recordClickInfo(x, y int, key KeyName, v *View, when time.Time) {
 	if IsMouseScrollKey(key) {
 		g.lastClick = nil
 		return
@@ -1965,7 +1967,7 @@ func (g *Gui) recordClickInfo(x, y int, key KeyName, v *View) {
 		y:        y,
 		key:      key,
 		viewName: v.Name(),
-		time:     time.Now(),
+		time:     when,
 	}
 }
 

pkg/gocui/tcell_driver.go

@@ -5,6 +5,8 @@
 package gocui
 
 import (
+	"time"
+
 	"github.com/gdamore/tcell/v3"
 	"github.com/gdamore/tcell/v3/vt"
 )
@@ -163,6 +165,8 @@ type gocuiEventType uint8
 //
 //	The 'Mod', 'Key' and 'Ch' fields are valid if 'Type' is 'eventKey'.
 //	The 'MouseX' and 'MouseY' fields are valid if 'Type' is 'eventMouse'.
+//	The 'When' field is valid if 'Type' is 'eventMouse'. It is the time the
+//	  input was read, which can be well before the main loop handles the event.
 //	The 'Width' and 'Height' fields are valid if 'Type' is 'eventResize'.
 //	The 'Focused' field is valid if 'Type' is 'eventFocus'.
 //	The 'Start' field is valid if 'Type' is 'eventPaste'. It is true for the
@@ -176,6 +180,7 @@ type GocuiEvent struct {
 	Err     error
 	MouseX  int
 	MouseY  int
+	When    time.Time
 	Focused bool
 	Start   bool
 	N       int
@@ -217,20 +222,18 @@ var (
 
 // this wrapper struct has public keys so we can easily serialize/deserialize to JSON
 type TcellKeyEventWrapper struct {
-	Timestamp int64
-	Mod       tcell.ModMask
-	Key       tcell.Key
-	Ch        string
+	Mod tcell.ModMask
+	Key tcell.Key
+	Ch  string
 
 	task Task // see GocuiEvent.task
 }
 
-func NewTcellKeyEventWrapper(event *tcell.EventKey, timestamp int64) *TcellKeyEventWrapper {
+func NewTcellKeyEventWrapper(event *tcell.EventKey) *TcellKeyEventWrapper {
 	return &TcellKeyEventWrapper{
-		Timestamp: timestamp,
-		Mod:       event.Modifiers(),
-		Key:       event.Key(),
-		Ch:        event.Str(),
+		Mod: event.Modifiers(),
+		Key: event.Key(),
+		Ch:  event.Str(),
 	}
 }
 
@@ -239,43 +242,43 @@ func (wrapper TcellKeyEventWrapper) toTcellEvent() tcell.Event {
 }
 
 type TcellMouseEventWrapper struct {
-	Timestamp  int64
 	X          int
 	Y          int
 	ButtonMask tcell.ButtonMask
 	ModMask    tcell.ModMask
+	When       time.Time
 
 	task Task // see GocuiEvent.task
 }
 
-func NewTcellMouseEventWrapper(event *tcell.EventMouse, timestamp int64) *TcellMouseEventWrapper {
+func NewTcellMouseEventWrapper(event *tcell.EventMouse) *TcellMouseEventWrapper {
 	x, y := event.Position()
 	return &TcellMouseEventWrapper{
-		Timestamp:  timestamp,
 		X:          x,
 		Y:          y,
 		ButtonMask: event.Buttons(),
 		ModMask:    event.Modifiers(),
+		When:       event.When(),
 	}
 }
 
 func (wrapper TcellMouseEventWrapper) toTcellEvent() tcell.Event {
-	return tcell.NewEventMouse(wrapper.X, wrapper.Y, wrapper.ButtonMask, wrapper.ModMask)
+	event := tcell.NewEventMouse(wrapper.X, wrapper.Y, wrapper.ButtonMask, wrapper.ModMask)
+	event.SetEventTime(wrapper.When)
+	return event
 }
 
 type TcellResizeEventWrapper struct {
-	Timestamp int64
-	Width     int
-	Height    int
+	Width  int
+	Height int
 }
 
-func NewTcellResizeEventWrapper(event *tcell.EventResize, timestamp int64) *TcellResizeEventWrapper {
+func NewTcellResizeEventWrapper(event *tcell.EventResize) *TcellResizeEventWrapper {
 	w, h := event.Size()
 
 	return &TcellResizeEventWrapper{
-		Timestamp: timestamp,
-		Width:     w,
-		Height:    h,
+		Width:  w,
+		Height: h,
 	}
 }
 
@@ -284,16 +287,14 @@ func (wrapper TcellResizeEventWrapper) toTcellEvent() tcell.Event {
 }
 
 type TcellFocusEventWrapper struct {
-	Timestamp int64
-	Focused   bool
+	Focused bool
 
 	task Task // see GocuiEvent.task
 }
 
-func NewTcellFocusEventWrapper(event *tcell.EventFocus, timestamp int64) *TcellFocusEventWrapper {
+func NewTcellFocusEventWrapper(event *tcell.EventFocus) *TcellFocusEventWrapper {
 	return &TcellFocusEventWrapper{
-		Timestamp: timestamp,
-		Focused:   event.Focused,
+		Focused: event.Focused,
 	}
 }
 
@@ -454,6 +455,7 @@ func gocuiEventFromTcellEvent(tev tcell.Event) GocuiEvent {
 			MouseX: x,
 			MouseY: y,
 			Key:    NewKey(mouseKey, "", mouseMod),
+			When:   tev.When(),
 		}
 	case *tcell.EventFocus:
 		return GocuiEvent{

pkg/gui/gui_driver.go

@@ -43,7 +43,6 @@ func (self *GuiDriver) PressKeysRapidly(keyStrs ...string) {
 
 		self.gui.g.ReplayKeyEvent(gocui.NewTcellKeyEventWrapper(
 			tcell.NewEventKey(tcell.Key(key.KeyName()), key.Str(), tcell.ModMask(key.Mod())),
-			0,
 		))
 	}
 
@@ -63,6 +62,16 @@ func (self *GuiDriver) ClickWithModifier(x, y int, modifier gocui.Modifier) {
 	self.replayMouseEvent(x, y, tcell.ButtonNone, modifier)
 }
 
+func (self *GuiDriver) DoubleClick(x, y int) {
+	self.CheckAllToastsAcknowledged()
+
+	for range 2 {
+		self.replayMouseEventWithoutWaiting(x, y, tcell.ButtonPrimary, gocui.ModNone)
+		self.replayMouseEventWithoutWaiting(x, y, tcell.ButtonNone, gocui.ModNone)
+	}
+	self.waitTillIdle()
+}
+
 func (self *GuiDriver) ClickAndHold(x, y int) {
 	self.CheckAllToastsAcknowledged()
 	self.replayMouseEvent(x, y, tcell.ButtonPrimary, gocui.ModNone)
@@ -105,7 +114,6 @@ func (self *GuiDriver) replayMouseEventWithoutWaiting(
 ) {
 	self.gui.g.ReplayMouseEvent(gocui.NewTcellMouseEventWrapper(
 		tcell.NewEventMouse(x, y, buttons, tcell.ModMask(modifier)),
-		0,
 	))
 }
 
@@ -115,11 +123,9 @@ func (self *GuiDriver) replayMouseEventWithoutWaiting(
 func (self *GuiDriver) replayFocusIn() {
 	self.gui.g.ReplayFocusEvent(gocui.NewTcellFocusEventWrapper(
 		tcell.NewEventFocus(false),
-		0,
 	))
 	self.gui.g.ReplayFocusEvent(gocui.NewTcellFocusEventWrapper(
 		tcell.NewEventFocus(true),
-		0,
 	))
 }
 
@@ -138,12 +144,10 @@ func (self *GuiDriver) FocusInAndClick(x, y int) {
 	self.replayFocusIn()
 	self.gui.g.ReplayMouseEvent(gocui.NewTcellMouseEventWrapper(
 		tcell.NewEventMouse(x, y, tcell.ButtonPrimary, 0),
-		0,
 	))
 	self.waitTillIdle()
 	self.gui.g.ReplayMouseEvent(gocui.NewTcellMouseEventWrapper(
 		tcell.NewEventMouse(x, y, tcell.ButtonNone, 0),
-		0,
 	))
 	self.waitTillIdle()
 }

pkg/integration/components/test_driver.go

@@ -61,6 +61,12 @@ func (self *TestDriver) click(x, y int) {
 	self.Wait(self.inputDelay)
 }
 
+func (self *TestDriver) doubleClick(x, y int) {
+	self.SetCaption(fmt.Sprintf("Double-clicking %d, %d", x, y))
+	self.gui.DoubleClick(x, y)
+	self.Wait(self.inputDelay)
+}
+
 func (self *TestDriver) clickWithModifier(x, y int, modifier gocui.Modifier, what string) {
 	self.SetCaption(fmt.Sprintf("%s-clicking %d, %d", what, x, y))
 	self.gui.ClickWithModifier(x, y, modifier)

pkg/integration/components/test_test.go

@@ -51,6 +51,10 @@ func (self *fakeGuiDriver) ClickWithModifier(x, y int, modifier gocui.Modifier)
 	self.clickedCoordinates = append(self.clickedCoordinates, coordinate{x: x, y: y})
 }
 
+func (self *fakeGuiDriver) DoubleClick(x, y int) {
+	self.clickedCoordinates = append(self.clickedCoordinates, coordinate{x: x, y: y})
+}
+
 func (self *fakeGuiDriver) ClickAndHold(x, y int) {
 	self.heldCoordinates = append(self.heldCoordinates, coordinate{x: x, y: y})
 }
@@ -207,6 +211,7 @@ func TestViewDriverPointerCoordinates(t *testing.T) {
 		AltClick(2, 3).
 		ShiftClick(4, 5).
 		FocusInAndClick(3, 4).
+		DoubleClick(6, 7).
 		ClickAndHold(5, 6).
 		MouseMove(7, 8).
 		MouseMoveToBottom(9).
@@ -214,12 +219,12 @@ func TestViewDriverPointerCoordinates(t *testing.T) {
 		ScrollWheelDown()
 
 	assert.Equal(t,
-		[]coordinate{{12, 23}, {13, 24}, {15, 26}, {14, 25}},
+		[]coordinate{{12, 23}, {13, 24}, {15, 26}, {14, 25}, {17, 28}},
 		guiDriver.clickedCoordinates)
 	assert.Equal(t, []coordinate{{16, 27}}, guiDriver.heldCoordinates)
 	assert.Equal(t, []coordinate{{18, 29}, {20, 30}, {51, 62}}, guiDriver.movedCoordinates)
 	assert.Equal(t, []coordinate{{11, 21}}, guiDriver.scrolledCoordinates)
-	assert.Equal(t, 9, guiDriver.onUIThreadCallCount)
+	assert.Equal(t, 10, guiDriver.onUIThreadCallCount)
 }
 
 func TestFailingFixture(t *testing.T) {

pkg/integration/components/view_driver.go

@@ -735,6 +735,14 @@ func (self *ViewDriver) Click(x, y int) *ViewDriver {
 	return self
 }
 
+func (self *ViewDriver) DoubleClick(x, y int) *ViewDriver {
+	offsetX, offsetY, _ := self.viewGeometry()
+
+	self.t.doubleClick(offsetX+1+x, offsetY+1+y)
+
+	return self
+}
+
 // AltClick and ShiftClick click with a modifier held down. Both modifiers are
 // bound to the same gestures, because no single one of them reaches lazygit in
 // every terminal.

pkg/integration/tests/main_view/enter_and_double_click_focus_file_diff.go

@@ -32,8 +32,7 @@ var EnterAndDoubleClickFocusFileDiff = NewIntegrationTest(NewIntegrationTestArgs
 
 		t.Views().Files().
 			IsFocused().
-			Click(3, 0).
-			Click(3, 0)
+			DoubleClick(3, 0)
 		t.Views().Main().
 			IsFocused().
 			SelectedLines(Contains("-base")).
@@ -60,8 +59,7 @@ var EnterAndDoubleClickFocusFileDiff = NewIntegrationTest(NewIntegrationTestArgs
 
 		t.Views().CommitFiles().
 			IsFocused().
-			Click(3, 0).
-			Click(3, 0)
+			DoubleClick(3, 0)
 		t.Views().Main().
 			IsFocused().
 			SelectedLines(Contains("+base"))

pkg/integration/types/types.go

@@ -31,6 +31,10 @@ type GuiDriver interface {
 	// Click with a keyboard modifier held down, for the gestures that only exist
 	// as a modified click.
 	ClickWithModifier(int, int, gocui.Modifier)
+	// Two clicks in immediate succession, waiting for lazygit to become idle only
+	// after the second one. If it waited after the first one too, the second click
+	// could come too late to count as a double click.
+	DoubleClick(int, int)
 	ClickAndHold(int, int)
 	MouseMove(int, int)
 	MouseRelease(int, int)