From 15532b74b95d4f18a08ae59d1476a640b11e577a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 11:06:08 +0200 Subject: [PATCH 1/9] Cleanup: remove error return value from Gui.SetRune, Gui.draw() et al These always returned nil. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/gocui/flush_test.go | 10 +-- pkg/gocui/gui.go | 124 ++++++++++++-------------------------- pkg/gocui/suspend_test.go | 2 +- 3 files changed, 46 insertions(+), 90 deletions(-) diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index d4082fcf6..b4f42afc4 100644 --- a/pkg/gocui/flush_test.go +++ b/pkg/gocui/flush_test.go @@ -64,8 +64,8 @@ func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) { assert.True(t, status.IsTainted(), "status view should be tainted after SetContent") assert.False(t, main.IsTainted(), "main view should not be tainted (was not modified)") - // flushContentOnly should succeed and clear status tainted flag - assert.NoError(t, g.flushContentOnly(g.views)) + // flushContentOnly should clear status tainted flag + g.flushContentOnly(g.views) assert.False(t, status.IsTainted(), "status view should not be tainted after flushContentOnly") assert.False(t, main.IsTainted(), "main view should not be tainted after flushContentOnly") @@ -76,7 +76,7 @@ func TestFlushContentOnly_WritesCorrectContent(t *testing.T) { status, _ := setupViews(t, g) status.SetContent("Fetching |") - assert.NoError(t, g.flushContentOnly(g.views)) + g.flushContentOnly(g.views) assert.Equal(t, "Fetching |", status.Buffer()) } @@ -231,7 +231,7 @@ func TestFlushContentOnly_DoesNotOverdrawHigherZViews(t *testing.T) { assert.False(t, popup.IsTainted(), "popup should not be tainted") // flushContentOnly is what spinner ticks ultimately invoke. - assert.NoError(t, g.flushContentOnly(g.views)) + g.flushContentOnly(g.views) assert.Equal(t, "P", cellAt(21, 9), "popup region must still show popup content after flushContentOnly; "+ @@ -279,7 +279,7 @@ func TestFlushContentOnly_RedrawsTransitivelyOverlappingViews(t *testing.T) { assert.False(t, b.IsTainted()) assert.False(t, c.IsTainted()) - assert.NoError(t, g.flushContentOnly(g.views)) + g.flushContentOnly(g.views) // a redrawn (direct). assert.Equal(t, "X", cellAt(5, 5), "a should be redrawn (tainted)") diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 144fae11f..652a50ab7 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -413,13 +413,12 @@ func (g *Gui) Size() (x, y int) { // corner of the terminal. It checks if the position is valid and applies // the given colors. // Should only be used if you know that the given rune is not part of a grapheme cluster. -func (g *Gui) SetRune(x, y int, ch rune, fgColor, bgColor Attribute) error { +func (g *Gui) SetRune(x, y int, ch rune, fgColor, bgColor Attribute) { if x < 0 || y < 0 || x >= g.maxX || y >= g.maxY { // swallowing error because it's not that big of a deal - return nil + return } tcellSetCell(x, y, string(ch), fgColor, bgColor, g.outputMode) - return nil } // SetView creates a new view with its top-left corner at (x0, y0) @@ -1195,7 +1194,8 @@ func (g *Gui) processEvent() error { contentOnly = contentOnly && remainingContentOnly if contentOnly { - return g.flushContentOnly(g.views) + g.flushContentOnly(g.views) + return nil } return g.flush() } @@ -1296,7 +1296,7 @@ func (g *Gui) onResize() { } // drawFrameEdges draws the horizontal and vertical edges of a view. -func (g *Gui) drawFrameEdges(v *View, fgColor, bgColor Attribute) error { +func (g *Gui) drawFrameEdges(v *View, fgColor, bgColor Attribute) { runeH, runeV := '─', '│' if len(v.FrameRunes) >= 2 { runeH, runeV = v.FrameRunes[0], v.FrameRunes[1] @@ -1307,14 +1307,10 @@ func (g *Gui) drawFrameEdges(v *View, fgColor, bgColor Attribute) error { continue } if v.y0 > -1 && v.y0 < g.maxY { - if err := g.SetRune(x, v.y0, runeH, fgColor, bgColor); err != nil { - return err - } + g.SetRune(x, v.y0, runeH, fgColor, bgColor) } if v.y1 > -1 && v.y1 < g.maxY { - if err := g.SetRune(x, v.y1, runeH, fgColor, bgColor); err != nil { - return err - } + g.SetRune(x, v.y1, runeH, fgColor, bgColor) } } @@ -1324,19 +1320,14 @@ func (g *Gui) drawFrameEdges(v *View, fgColor, bgColor Attribute) error { continue } if v.x0 > -1 && v.x0 < g.maxX { - if err := g.SetRune(v.x0, y, runeV, fgColor, bgColor); err != nil { - return err - } + g.SetRune(v.x0, y, runeV, fgColor, bgColor) } if v.x1 > -1 && v.x1 < g.maxX { runeToPrint := calcScrollbarRune(showScrollbar, realScrollbarStart, realScrollbarEnd, y, runeV) - if err := g.SetRune(v.x1, y, runeToPrint, fgColor, bgColor); err != nil { - return err - } + g.SetRune(v.x1, y, runeToPrint, fgColor, bgColor) } } - return nil } func calcScrollbarRune( @@ -1436,17 +1427,13 @@ func corner(v *View, directions byte) rune { } // drawFrameCorners draws the corners of the view. -func (g *Gui) drawFrameCorners(v *View, fgColor, bgColor Attribute) error { +func (g *Gui) drawFrameCorners(v *View, fgColor, bgColor Attribute) { if v.y0 == v.y1 { if !g.SupportOverlaps && v.x0 >= 0 && v.x1 >= 0 && v.y0 >= 0 && v.x0 < g.maxX && v.x1 < g.maxX && v.y0 < g.maxY { - if err := g.SetRune(v.x0, v.y0, '╶', fgColor, bgColor); err != nil { - return err - } - if err := g.SetRune(v.x1, v.y0, '╴', fgColor, bgColor); err != nil { - return err - } + g.SetRune(v.x0, v.y0, '╶', fgColor, bgColor) + g.SetRune(v.x1, v.y0, '╴', fgColor, bgColor) } - return nil + return } runeTL, runeTR, runeBL, runeBR := '┌', '┐', '└', '┘' @@ -1467,18 +1454,15 @@ func (g *Gui) drawFrameCorners(v *View, fgColor, bgColor Attribute) error { for _, c := range corners { if c.x >= 0 && c.y >= 0 && c.x < g.maxX && c.y < g.maxY { - if err := g.SetRune(c.x, c.y, c.ch, fgColor, bgColor); err != nil { - return err - } + g.SetRune(c.x, c.y, c.ch, fgColor, bgColor) } } - return nil } // drawTitle draws the title of the view. -func (g *Gui) drawTitle(v *View, fgColor, bgColor Attribute) error { +func (g *Gui) drawTitle(v *View, fgColor, bgColor Attribute) { if v.y0 < 0 || v.y0 >= g.maxY { - return nil + return } tabs := v.Tabs @@ -1514,9 +1498,7 @@ func (g *Gui) drawTitle(v *View, fgColor, bgColor Attribute) error { x := v.x0 + 2 for _, ch := range prefix { - if err := g.SetRune(x, v.y0, ch, fgColor, bgColor); err != nil { - return err - } + g.SetRune(x, v.y0, ch, fgColor, bgColor) x += uniseg.StringWidth(string(ch)) } for i, ch := range str { @@ -1539,64 +1521,55 @@ func (g *Gui) drawTitle(v *View, fgColor, bgColor Attribute) error { currentFgColor &= ^AttrBold } } - if err := g.SetRune(x, v.y0, ch, currentFgColor, currentBgColor); err != nil { - return err - } + g.SetRune(x, v.y0, ch, currentFgColor, currentBgColor) x += uniseg.StringWidth(string(ch)) } - return nil } // drawSubtitle draws the subtitle of the view. -func (g *Gui) drawSubtitle(v *View, fgColor, bgColor Attribute) error { +func (g *Gui) drawSubtitle(v *View, fgColor, bgColor Attribute) { if v.y0 < 0 || v.y0 >= g.maxY { - return nil + return } start := v.x1 - 5 - uniseg.StringWidth(v.Subtitle) if start < v.x0 { - return nil + return } x := start for _, ch := range v.Subtitle { if x >= v.x1 { break } - if err := g.SetRune(x, v.y0, ch, fgColor, bgColor); err != nil { - return err - } + g.SetRune(x, v.y0, ch, fgColor, bgColor) x += uniseg.StringWidth(string(ch)) } - return nil } // drawListFooter draws the footer of a list view, showing something like '1 of 10' -func (g *Gui) drawListFooter(v *View, fgColor, bgColor Attribute) error { +func (g *Gui) drawListFooter(v *View, fgColor, bgColor Attribute) { if len(v.buf.lines) == 0 { - return nil + return } message := v.Footer if v.y1 < 0 || v.y1 >= g.maxY { - return nil + return } start := v.x1 - 1 - uniseg.StringWidth(message) if start < v.x0 { - return nil + return } x := start for _, ch := range message { if x >= v.x1 { break } - if err := g.SetRune(x, v.y1, ch, fgColor, bgColor); err != nil { - return err - } + g.SetRune(x, v.y1, ch, fgColor, bgColor) x += uniseg.StringWidth(string(ch)) } - return nil } // flush updates the gui, re-drawing frames and buffers. @@ -1624,9 +1597,7 @@ func (g *Gui) flush() error { } } for _, v := range g.views { - if err := g.draw(v); err != nil { - return err - } + g.draw(v) } Screen.Show() @@ -1637,20 +1608,17 @@ func (g *Gui) flush() error { // tcell's cell-level dirty tracking ensures only // actually-changed cells are emitted to the terminal. // Will also redraw any views that overlap tainted views -func (g *Gui) flushContentOnly(views []*View) error { +func (g *Gui) flushContentOnly(views []*View) { // The screen must not be touched while suspended (see Suspend). if g.isSuspended() { - return nil + return } for _, v := range viewsToRedrawContentOnly(views) { - if err := g.draw(v); err != nil { - return err - } + g.draw(v) } Screen.Show() - return nil } func viewsToRedrawContentOnly(views []*View) []*View { @@ -1690,8 +1658,8 @@ func (g *Gui) ForceLayoutAndRedraw() error { // Redraws only tainted views outside of the normal main // loop, without a layout pass. Useful during longer operations that block the // main thread, e.g. to update a spinner in a status view. -func (g *Gui) ForceFlushViewsContentOnly(views []*View) error { - return g.flushContentOnly(views) +func (g *Gui) ForceFlushViewsContentOnly(views []*View) { + g.flushContentOnly(views) } // hasFocus reports whether a view is drawn as focused. Views that are embedded @@ -1709,9 +1677,9 @@ func outermostView(v *View) *View { } // draw manages the cursor and calls the draw function of a view. -func (g *Gui) draw(v *View) error { +func (g *Gui) draw(v *View) { if !v.Visible || v.y1 < v.y0 || v.x1 < v.x0 { - return nil + return } if g.Cursor { @@ -1750,30 +1718,18 @@ func (g *Gui) draw(v *View) error { } } - if err := g.drawFrameEdges(v, frameColor, bgColor); err != nil { - return err - } - if err := g.drawFrameCorners(v, frameColor, bgColor); err != nil { - return err - } + g.drawFrameEdges(v, frameColor, bgColor) + g.drawFrameCorners(v, frameColor, bgColor) if v.Title != "" || len(v.Tabs) > 0 { - if err := g.drawTitle(v, fgColor, bgColor); err != nil { - return err - } + g.drawTitle(v, fgColor, bgColor) } if v.Subtitle != "" { - if err := g.drawSubtitle(v, fgColor, bgColor); err != nil { - return err - } + g.drawSubtitle(v, fgColor, bgColor) } if v.Footer != "" && g.ShowListFooter { - if err := g.drawListFooter(v, fgColor, bgColor); err != nil { - return err - } + g.drawListFooter(v, fgColor, bgColor) } } - - return nil } // onKey manages key-press events. A keybinding handler is called when diff --git a/pkg/gocui/suspend_test.go b/pkg/gocui/suspend_test.go index ded220bea..5a7a070d6 100644 --- a/pkg/gocui/suspend_test.go +++ b/pkg/gocui/suspend_test.go @@ -18,7 +18,7 @@ func TestFlushIsNoOpWhileSuspended(t *testing.T) { flush func(g *Gui) error }{ {"flush", func(g *Gui) error { return g.flush() }}, - {"flushContentOnly", func(g *Gui) error { return g.flushContentOnly(g.views) }}, + {"flushContentOnly", func(g *Gui) error { g.flushContentOnly(g.views); return nil }}, } for _, tc := range tests { From dc6ea4d515ebd255ddcbd2476d0de9586245caa1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 11:34:06 +0200 Subject: [PATCH 2/9] Remove unused per-line highlighting View.SetHighlight has no production callers. --- pkg/gocui/view.go | 38 +++++++------------------------------- 1 file changed, 7 insertions(+), 31 deletions(-) diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index a43ee1815..463a31a8f 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -87,11 +87,11 @@ type View struct { tainted bool // firstDirtyLine is the index of the lowest line in `lines` that has been - // written to or highlighted since viewLines was last refreshed, and whose - // cached wrapping (lineType.wrappedCells) may therefore be stale. Lines - // below it are unchanged and can reuse their cached wrapping instead of - // being re-wrapped, which keeps refreshViewLinesIfNeeded cheap while - // scrolling appends new lines to a long buffer. + // written to since viewLines was last refreshed, and whose cached wrapping + // (lineType.wrappedCells) may therefore be stale. Lines below it are + // unchanged and can reuse their cached wrapping instead of being + // re-wrapped, which keeps refreshViewLinesIfNeeded cheap while scrolling + // appends new lines to a long buffer. firstDirtyLine int // the last position that the mouse was hovering over; nil if the mouse is outside of @@ -1908,8 +1908,8 @@ func (v *View) refreshViewLinesIfNeeded() { // Reuse the previously wrapped result for lines that haven't changed // since the last refresh (i.e. below firstDirtyLine) and were wrapped at // the current width. Wrapping is expensive and this loop runs on every - // scroll event, so only the lines that were actually just read (or - // re-highlighted) should be wrapped afresh. + // scroll event, so only the lines that were actually just read should + // be wrapped afresh. if line.wrappedCells == nil || line.wrappedColumns != wrap || i >= v.firstDirtyLine { line.wrappedCells = lineWrap(line.cells, wrap) line.wrappedColumns = wrap @@ -2303,30 +2303,6 @@ func applySelTextColor(fgColor, selTextColor Attribute) Attribute { return fgColor | selTextColor&AttrStyleBits } -// SetHighlight toggles highlighting of separate lines, for custom lists -// or multiple selection in views. -func (v *View) SetHighlight(y int, on bool) { - if y < 0 || y >= len(v.buf.lines) { - return - } - - cells := make([]cell, 0, len(v.buf.lines[y].cells)) - for _, c := range v.buf.lines[y].cells { - if on { - c.bgColor = v.SelBgColor - c.fgColor = v.SelFgColor - } else { - c.bgColor = v.BgColor - c.fgColor = v.FgColor - } - cells = append(cells, c) - } - v.tainted = true - v.firstDirtyLine = min(v.firstDirtyLine, y) - v.buf.lines[y].cells = cells - v.clearHover() -} - func lineWrap(line []cell, columns int) [][]cell { if columns == 0 { return [][]cell{line} From 23c0b40fce58cdff25145a5fba42faa27b71f004 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 11:20:28 +0200 Subject: [PATCH 3/9] Separate redraws from view-line invalidation A view can need repainting even when its cached wrapping is still valid. Track that state independently so content-only flushes do not overload tainted, whose only job is to request a viewLines rebuild. --- pkg/gocui/gui.go | 8 ++++---- pkg/gocui/view.go | 26 +++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 652a50ab7..eeaa87c45 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -1604,10 +1604,10 @@ func (g *Gui) flush() error { return nil } -// Redraws only tainted views and skips the layout pass. +// Redraws only dirty views and skips the layout pass. // tcell's cell-level dirty tracking ensures only // actually-changed cells are emitted to the terminal. -// Will also redraw any views that overlap tainted views +// Will also redraw any views that overlap dirty views. func (g *Gui) flushContentOnly(views []*View) { // The screen must not be touched while suspended (see Suspend). if g.isSuspended() { @@ -1625,7 +1625,7 @@ func viewsToRedrawContentOnly(views []*View) []*View { redrawIndexes := set.New[int]() for i, v := range views { - if !v.IsTainted() && !redrawIndexes.Includes(i) { + if !v.NeedsRedraw() && !redrawIndexes.Includes(i) { continue } @@ -1655,7 +1655,7 @@ func (g *Gui) ForceLayoutAndRedraw() error { return g.flush() } -// Redraws only tainted views outside of the normal main +// Redraws only dirty views outside of the normal main // loop, without a layout pass. Useful during longer operations that block the // main thread, e.g. to update a spinner in a status view. func (g *Gui) ForceFlushViewsContentOnly(views []*View) { diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 463a31a8f..fa5eb934d 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -86,6 +86,11 @@ type View struct { // tained is true if the viewLines must be updated tainted bool + // needsRedraw is true if the view's current state has not been drawn to the + // screen yet. A tainted view always needs a redraw, but draw-only state can + // require one without invalidating viewLines. + needsRedraw bool + // firstDirtyLine is the index of the lowest line in `lines` that has been // written to since viewLines was last refreshed, and whose cached wrapping // (lineType.wrappedCells) may therefore be stale. Lines below it are @@ -274,11 +279,18 @@ type pos struct { // a view whose size has changed, whose content is the same but has to be wrapped // afresh, call RewrapContent instead. func (v *View) clearViewLines() { - v.tainted = true + v.markViewLinesDirty() v.viewLines = nil v.clearHover() } +// markViewLinesDirty records that the cached viewLines no longer represent the +// view's buffer or wrapping, so both rebuilding and redrawing are required. +func (v *View) markViewLinesDirty() { + v.tainted = true + v.needsRedraw = true +} + // RewrapContent wraps the view's content for the size the view has now, and puts // the positions into that content — the scroll offset, the cursor, a range's // anchor — back on the lines they were on. They are all view lines, which count @@ -793,6 +805,7 @@ func NewView(name string, x0, y0, x1, y1 int, mode OutputMode) *View { Frame: true, Editor: DefaultEditor, tainted: true, + needsRedraw: true, outMode: mode, buf: &viewBuffer{ei: newEscapeInterpreter(mode)}, searcher: &searcher{}, @@ -1177,7 +1190,7 @@ func (v *View) write(p []byte) { return } - v.tainted = true + v.markViewLinesDirty() // write only ever touches lines from v.buf.wy onwards, so any cached wrapping // below that stays valid. v.firstDirtyLine = min(v.firstDirtyLine, v.buf.wy) @@ -1612,7 +1625,7 @@ func (v *View) SwapInOffscreenRender() { } v.buf = v.offscreen v.offscreen = nil - v.tainted = true + v.markViewLinesDirty() v.clearHover() } @@ -1776,6 +1789,12 @@ func (v *View) IsTainted() bool { return v.tainted } +func (v *View) NeedsRedraw() bool { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + return v.needsRedraw +} + // draw re-draws the view's contents. func (v *View) draw(isWindowFocused bool) { v.writeMutex.Lock() @@ -1784,6 +1803,7 @@ func (v *View) draw(isWindowFocused bool) { if !v.Visible { return } + defer func() { v.needsRedraw = false }() v.clearRunes() From ef5f0dccfb850c322219a609e29b6be3b38f1bf1 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 09:05:49 +0200 Subject: [PATCH 4/9] Let mouse bindings work behind focused popups Mouse events on views behind a popup are normally swallowed before their bindings can run. Add an explicit early-dispatch opt-in for actions that should remain live there, matching the phase where hyperlink clicks already run. --- pkg/gocui/double_click_test.go | 34 ++++++++++++++++ pkg/gocui/gui.go | 72 ++++++++++++++++++++++++--------- pkg/gocui/mouse_binding_test.go | 49 ++++++++++++++++++++++ 3 files changed, 135 insertions(+), 20 deletions(-) create mode 100644 pkg/gocui/mouse_binding_test.go diff --git a/pkg/gocui/double_click_test.go b/pkg/gocui/double_click_test.go index 9c73da1e9..f6a14f620 100644 --- a/pkg/gocui/double_click_test.go +++ b/pkg/gocui/double_click_test.go @@ -32,3 +32,37 @@ func TestMouseReleaseDoesNotBreakDoubleClickDetection(t *testing.T) { assert.Equal(t, []bool{false, true}, doubleClicks) } + +func TestASwallowedClickIsNoHalfOfADoubleClick(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 + }, + }) + + press := gocuiEventFromTcellEvent( + tcell.NewEventMouse(view.x0+1, view.y0+1, tcell.ButtonPrimary, tcell.ModNone)) + release := gocuiEventFromTcellEvent( + tcell.NewEventMouse(view.x0+1, view.y0+1, tcell.ButtonNone, tcell.ModNone)) + + // A click the client rejects, as it does for one landing behind a popup panel. + g.ShouldHandleMouseEvent = func(*View, KeyName) bool { return false } + assert.NoError(t, g.onKey(&press)) + assert.NoError(t, g.onKey(&release)) + assert.Empty(t, doubleClicks) + + // The same spot clicked again once clicks are accepted. It is a click of its + // own, not the second half of the one that nothing acted on. + g.ShouldHandleMouseEvent = func(*View, KeyName) bool { return true } + assert.NoError(t, g.onKey(&press)) + + assert.Equal(t, []bool{false}, doubleClicks) +} diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index eeaa87c45..9f46887f3 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -91,6 +91,14 @@ type ViewMouseBinding struct { // must be a mouse key Key KeyName + + // If true, this binding is dispatched before ShouldHandleMouseEvent is + // consulted, so it fires even when a popup panel is focused and the click + // lands on a view other than that panel (which is normally swallowed). This + // is the same early phase that hyperlink clicks are handled in; use it for + // clicks that must stay live behind a popup, e.g. opening a diff line in the + // editor from the main view behind the commit-message panel. + HandleWhenPopupPanelFocused bool } type ViewMouseBindingOpts struct { @@ -1813,6 +1821,25 @@ func (g *Gui) onKey(ev *GocuiEvent) error { } } + var mouseOpts ViewMouseBindingOpts + if IsMouseKey(ev.Key) { + mouseOpts = ViewMouseBindingOpts{ + X: newX, Y: newY, Key: ev.Key.KeyName(), + IsDoubleClick: g.isDoubleClick(newX, newY, ev.Key.KeyName(), v), + } + + // Dispatch bindings that opt into firing while a popup panel is focused + // before the gate below gets a chance to reject the click. + matched, err := g.execMouseKeybindings(v, ev, mouseOpts, true) + if err != nil { + return err + } + if matched { + g.recordClickInfo(newX, newY, ev.Key.KeyName(), v) + return nil + } + } + if g.ShouldHandleMouseEvent != nil { if !g.ShouldHandleMouseEvent(v, ev.Key.KeyName()) { // Give clients a chance to reject clicks, for example clicks in inactive views @@ -1862,9 +1889,8 @@ func (g *Gui) onKey(ev *GocuiEvent) error { } if IsMouseKey(ev.Key) { - isDoubleClick := g.recordClickInfo(newX, newY, ev.Key.KeyName(), v) - opts := ViewMouseBindingOpts{X: newX, Y: newY, Key: ev.Key.KeyName(), IsDoubleClick: isDoubleClick} - matched, err := g.execMouseKeybindings(v, ev, opts) + g.recordClickInfo(newX, newY, ev.Key.KeyName(), v) + matched, err := g.execMouseKeybindings(v, ev, mouseOpts, false) if err != nil { return err } @@ -1896,43 +1922,49 @@ func (g *Gui) onKey(ev *GocuiEvent) error { return nil } -// remember the information for this click, and return true if it was a double click -func (g *Gui) recordClickInfo(x, y int, key KeyName, v *View) bool { +// 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 { + return g.lastClick != nil && + !IsMouseScrollKey(key) && + key != MouseRelease && + x == g.lastClick.x && + y == g.lastClick.y && + key == g.lastClick.key && + v.Name() == g.lastClick.viewName && + time.Now().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) { if IsMouseScrollKey(key) { g.lastClick = nil - return false + return } // A release ends a gesture but is not a click of its own; it must leave // the click info of the press that started it alone, or no double click // could ever be detected. if key == MouseRelease { - return false + return } - clickInfo := &clickInfo{ + g.lastClick = &clickInfo{ x: x, y: y, key: key, viewName: v.Name(), time: time.Now(), } - - isDoubleClick := g.lastClick != nil && - clickInfo.x == g.lastClick.x && - clickInfo.y == g.lastClick.y && - clickInfo.key == g.lastClick.key && - clickInfo.viewName == g.lastClick.viewName && - clickInfo.time.Before(g.lastClick.time.Add(DOUBLE_CLICK_THRESHOLD)) - - g.lastClick = clickInfo - return isDoubleClick } -func (g *Gui) execMouseKeybindings(view *View, ev *GocuiEvent, opts ViewMouseBindingOpts) (bool, error) { +func (g *Gui) execMouseKeybindings(view *View, ev *GocuiEvent, opts ViewMouseBindingOpts, handleWhenPopupPanelFocused bool) (bool, error) { isMatch := func(binding *ViewMouseBinding) bool { return binding.ViewName == view.Name() && ev.Key.KeyName() == binding.Key && - ev.Key.Mod() == binding.Modifier + ev.Key.Mod() == binding.Modifier && + binding.HandleWhenPopupPanelFocused == handleWhenPopupPanelFocused } // first pass looks for ones that match the focused view diff --git a/pkg/gocui/mouse_binding_test.go b/pkg/gocui/mouse_binding_test.go new file mode 100644 index 000000000..7b522559a --- /dev/null +++ b/pkg/gocui/mouse_binding_test.go @@ -0,0 +1,49 @@ +package gocui + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOnlyBindingsThatOptedInFireBehindAFocusedPopup(t *testing.T) { + g := newTestGui(t) + _, _ = g.SetView("main", 0, 0, 20, 10, 0) + + fired := []string{} + g.SetViewClickBinding(&ViewMouseBinding{ + ViewName: "main", + Key: MouseLeft, + Modifier: ModAlt, + HandleWhenPopupPanelFocused: true, + Handler: func(ViewMouseBindingOpts) error { + fired = append(fired, "opted in") + return nil + }, + }) + g.SetViewClickBinding(&ViewMouseBinding{ + ViewName: "main", + Key: MouseLeft, + Handler: func(ViewMouseBindingOpts) error { + fired = append(fired, "ordinary") + return nil + }, + }) + + // This is how a client reports that a popup panel has the focus and the click + // landed on a view behind it. + g.ShouldHandleMouseEvent = func(*View, KeyName) bool { return false } + + assert.NoError(t, g.onKey(&GocuiEvent{ + Type: eventMouse, MouseX: 3, MouseY: 4, + Key: NewKey(MouseLeft, "", ModAlt), + })) + assert.Equal(t, []string{"opted in"}, fired) + + assert.NoError(t, g.onKey(&GocuiEvent{ + Type: eventMouse, MouseX: 3, MouseY: 4, + Key: NewKey(MouseLeft, "", ModNone), + })) + assert.Equal(t, []string{"opted in"}, fired, + "a binding that didn't opt in must still be swallowed") +} From 345191a16ff8310449564e8684fd560409e30206 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 10:11:43 +0200 Subject: [PATCH 5/9] Share diff-line editing with mouse actions The selected-line keybinding and a modified click need the same path from a rendered diff row to the editor. Give that operation an explicit view-line argument before adding the mouse gesture. --- pkg/gui/controllers/main_view_controller.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index ef54d149e..4cf3299c7 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -966,8 +966,11 @@ func (self *MainViewController) editLine() error { if !view.Highlight { return nil } + return self.editDiffLine(view.SelectedLineIdx()) +} - info, ok := self.c.Helpers().DiffLine.GetDiffLineInfo(view, view.SelectedLineIdx()) +func (self *MainViewController) editDiffLine(viewLine int) error { + info, ok := self.c.Helpers().DiffLine.GetDiffLineInfo(self.context.GetView(), viewLine) if !ok { return nil } From 9fdefc418dfe592eeed081ba5942eba1f486e62e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 10:13:36 +0200 Subject: [PATCH 6/9] 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 9f46887f3..f4f87a281 100644 --- a/pkg/gocui/gui.go +++ b/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 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 From 6990c0688014575e313905d1c13f45edb003a097 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 10:14:32 +0200 Subject: [PATCH 7/9] Open a clicked diff line in the editor Clickable renderer gutters are small and unavailable on some diff rows. Make the whole row an editor target without changing focus or selection, including while a popup is focused. Use both Alt and Shift because terminal mouse protocols do not deliver either modifier consistently across Ghostty, iTerm2, and VS Code. --- docs-master/Config.md | 2 + pkg/gui/controllers/main_view_controller.go | 18 +++++ pkg/gui/gui_driver.go | 30 +++++---- pkg/integration/components/test_driver.go | 7 ++ pkg/integration/components/test_test.go | 12 +++- pkg/integration/components/view_driver.go | 19 ++++++ .../tests/main_view/edit_clicked_diff_line.go | 66 +++++++++++++++++++ .../no_selection_over_a_conflict_hint.go | 15 +++++ pkg/integration/tests/test_list.go | 1 + pkg/integration/types/types.go | 3 + 10 files changed, 160 insertions(+), 13 deletions(-) create mode 100644 pkg/integration/tests/main_view/edit_clicked_diff_line.go diff --git a/docs-master/Config.md b/docs-master/Config.md index 8b2f07c9c..ee1c7ff43 100644 --- a/docs-master/Config.md +++ b/docs-master/Config.md @@ -950,6 +950,8 @@ It is used, for example, when pasting a commit message into the commit message p There are two commands for opening files, `o` for "open" and `e` for "edit". `o` acts as if the file was double-clicked in the Finder/Explorer, so it also works for non-text files, whereas `e` opens the file in an editor. `e` can also jump to the right line in the file when you invoke it from a focused diff. +You can also open a line in your editor with the mouse: alt-click or shift-click it. Both modifiers do the same thing, because some terminals only support one or the other. The click leaves the focus and the selection where they are, so it works while you are reading a diff from another panel, or while a popup is open. + To tell lazygit which editor to use for the `e` command, the easiest way to do that is to provide an editPreset config, e.g. ```yaml diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index 4cf3299c7..f040b401c 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -241,6 +241,20 @@ func (self *MainViewController) GetMouseKeybindings(opts types.KeybindingsOpts) Key: gocui.MouseRelease, Handler: self.onDragRelease, }, + { + ViewName: self.context.GetViewName(), + Key: gocui.MouseLeft, + Modifier: gocui.ModAlt, + Handler: self.editClickedLine, + HandleWhenPopupPanelFocused: true, + }, + { + ViewName: self.context.GetViewName(), + Key: gocui.MouseLeft, + Modifier: gocui.ModShift, + Handler: self.editClickedLine, + HandleWhenPopupPanelFocused: true, + }, } } @@ -518,6 +532,10 @@ func (self *MainViewController) onClickInAlreadyFocusedView(opts gocui.ViewMouse return nil } +func (self *MainViewController) editClickedLine(opts gocui.ViewMouseBindingOpts) error { + return self.editDiffLine(opts.Y) +} + func (self *MainViewController) onClickInOtherViewOfMainViewPair(opts gocui.ViewMouseBindingOpts) error { // Carry the select mode over from the pane we're leaving, so that clicking into // the other pane keeps hunk mode even the first time we enter it — its own mode diff --git a/pkg/gui/gui_driver.go b/pkg/gui/gui_driver.go index 07a0b7d4c..af261b453 100644 --- a/pkg/gui/gui_driver.go +++ b/pkg/gui/gui_driver.go @@ -51,34 +51,40 @@ func (self *GuiDriver) PressKeysRapidly(keyStrs ...string) { } func (self *GuiDriver) Click(x, y int) { + self.ClickWithModifier(x, y, gocui.ModNone) +} + +// ClickWithModifier clicks with a keyboard modifier held down for the whole +// gesture, as a terminal reports it. +func (self *GuiDriver) ClickWithModifier(x, y int, modifier gocui.Modifier) { self.CheckAllToastsAcknowledged() - self.replayMouseEvent(x, y, tcell.ButtonPrimary) - self.replayMouseEvent(x, y, tcell.ButtonNone) + self.replayMouseEvent(x, y, tcell.ButtonPrimary, modifier) + self.replayMouseEvent(x, y, tcell.ButtonNone, modifier) } func (self *GuiDriver) ClickAndHold(x, y int) { self.CheckAllToastsAcknowledged() - self.replayMouseEvent(x, y, tcell.ButtonPrimary) + self.replayMouseEvent(x, y, tcell.ButtonPrimary, gocui.ModNone) } // MouseMove reports the mouse at a new position with the left button still // held down, i.e. a drag movement. (No test needs pointer motion without a // button held, so that variant doesn't exist.) func (self *GuiDriver) MouseMove(x, y int) { - self.replayMouseEvent(x, y, tcell.ButtonPrimary) + self.replayMouseEvent(x, y, tcell.ButtonPrimary, gocui.ModNone) } func (self *GuiDriver) ScrollWheelDown(x, y int) { - self.replayMouseEvent(x, y, tcell.WheelDown) + self.replayMouseEvent(x, y, tcell.WheelDown, gocui.ModNone) } func (self *GuiDriver) MouseRelease(x, y int) { - self.replayMouseEvent(x, y, tcell.ButtonNone) + self.replayMouseEvent(x, y, tcell.ButtonNone, gocui.ModNone) } func (self *GuiDriver) MouseReleaseWithoutWaiting(x, y int) { - self.replayMouseEventWithoutWaiting(x, y, tcell.ButtonNone) + self.replayMouseEventWithoutWaiting(x, y, tcell.ButtonNone, gocui.ModNone) } func (self *GuiDriver) WaitUntilIdle() { @@ -89,14 +95,16 @@ func (self *GuiDriver) OnUIThreadAndWait(f func()) { _ = self.gui.g.OnUIThreadAndWait(f) } -func (self *GuiDriver) replayMouseEvent(x, y int, buttons tcell.ButtonMask) { - self.replayMouseEventWithoutWaiting(x, y, buttons) +func (self *GuiDriver) replayMouseEvent(x, y int, buttons tcell.ButtonMask, modifier gocui.Modifier) { + self.replayMouseEventWithoutWaiting(x, y, buttons, modifier) self.waitTillIdle() } -func (self *GuiDriver) replayMouseEventWithoutWaiting(x, y int, buttons tcell.ButtonMask) { +func (self *GuiDriver) replayMouseEventWithoutWaiting( + x, y int, buttons tcell.ButtonMask, modifier gocui.Modifier, +) { self.gui.g.ReplayMouseEvent(gocui.NewTcellMouseEventWrapper( - tcell.NewEventMouse(x, y, buttons, 0), + tcell.NewEventMouse(x, y, buttons, tcell.ModMask(modifier)), 0, )) } diff --git a/pkg/integration/components/test_driver.go b/pkg/integration/components/test_driver.go index bd5bbfc24..825ce5688 100644 --- a/pkg/integration/components/test_driver.go +++ b/pkg/integration/components/test_driver.go @@ -6,6 +6,7 @@ import ( "time" "github.com/jesseduffield/lazygit/pkg/config" + "github.com/jesseduffield/lazygit/pkg/gocui" integrationTypes "github.com/jesseduffield/lazygit/pkg/integration/types" ) @@ -60,6 +61,12 @@ func (self *TestDriver) click(x, y int) { 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) + self.Wait(self.inputDelay) +} + func (self *TestDriver) clickAndHold(x, y int) { self.SetCaption(fmt.Sprintf("Clicking and holding %d, %d", x, y)) self.mouseX, self.mouseY = x, y diff --git a/pkg/integration/components/test_test.go b/pkg/integration/components/test_test.go index 3b608e698..ef77ddaec 100644 --- a/pkg/integration/components/test_test.go +++ b/pkg/integration/components/test_test.go @@ -47,6 +47,10 @@ func (self *fakeGuiDriver) Click(x, y int) { self.clickedCoordinates = append(self.clickedCoordinates, coordinate{x: x, y: y}) } +func (self *fakeGuiDriver) ClickWithModifier(x, y int, modifier gocui.Modifier) { + 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}) } @@ -200,6 +204,8 @@ func TestViewDriverPointerCoordinates(t *testing.T) { viewDriver. Click(1, 2). + AltClick(2, 3). + ShiftClick(4, 5). FocusInAndClick(3, 4). ClickAndHold(5, 6). MouseMove(7, 8). @@ -207,11 +213,13 @@ func TestViewDriverPointerCoordinates(t *testing.T) { MouseMoveToView(targetViewDriver, 10, 11). ScrollWheelDown() - assert.Equal(t, []coordinate{{12, 23}, {14, 25}}, guiDriver.clickedCoordinates) + assert.Equal(t, + []coordinate{{12, 23}, {13, 24}, {15, 26}, {14, 25}}, + 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, 7, guiDriver.onUIThreadCallCount) + assert.Equal(t, 9, guiDriver.onUIThreadCallCount) } func TestFailingFixture(t *testing.T) { diff --git a/pkg/integration/components/view_driver.go b/pkg/integration/components/view_driver.go index 4650f335c..818385ca1 100644 --- a/pkg/integration/components/view_driver.go +++ b/pkg/integration/components/view_driver.go @@ -726,6 +726,25 @@ func (self *ViewDriver) Click(x, y int) *ViewDriver { 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. +func (self *ViewDriver) AltClick(x, y int) *ViewDriver { + offsetX, offsetY, _ := self.viewGeometry() + + self.t.clickWithModifier(offsetX+1+x, offsetY+1+y, gocui.ModAlt, "Alt") + + return self +} + +func (self *ViewDriver) ShiftClick(x, y int) *ViewDriver { + offsetX, offsetY, _ := self.viewGeometry() + + self.t.clickWithModifier(offsetX+1+x, offsetY+1+y, gocui.ModShift, "Shift") + + return self +} + func (self *ViewDriver) FocusInAndClick(x, y int) *ViewDriver { offsetX, offsetY, _ := self.viewGeometry() diff --git a/pkg/integration/tests/main_view/edit_clicked_diff_line.go b/pkg/integration/tests/main_view/edit_clicked_diff_line.go new file mode 100644 index 000000000..792fd09c8 --- /dev/null +++ b/pkg/integration/tests/main_view/edit_clicked_diff_line.go @@ -0,0 +1,66 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var EditClickedDiffLine = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Alt- or shift-click a line of the main view's diff to open it in the editor, without focusing the view", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + config.GetUserConfig().OS.EditAtLine = "echo {{filename}}:{{line}} > edit-command" + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\nfour\nfive\n") + shell.Commit("one") + + shell.UpdateFile("file1", "one\ntwo\nTHREE\nfour\nfive\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused() + + // The click points at the line itself, so the main view can stay unfocused + // and unselected. You read a diff where it is and click into it. + t.Views().Main(). + AltClick(0, 8). + Tap(func() { + t.FileSystem().FileContent("edit-command", Contains("/repo/file1:3\n")) + }). + SelectionIsHidden() + + t.Views().Files(). + IsFocused() + + // Shift-click is bound to the same thing, since neither modifier reaches + // lazygit in every terminal. A context line names a line of the file like any + // other row. + t.Views().Main(). + ShiftClick(0, 9). + Tap(func() { + t.FileSystem().FileContent("edit-command", Contains("/repo/file1:4\n")) + }) + + // A popup taking the focus swallows clicks on the views behind it. This one + // stays live, so a diff can still be read and clicked into while a popup is + // up. + t.Views().Files(). + Press(keys.Universal.Remove) + + t.Views().Menu(). + IsFocused() + + t.Views().Main(). + AltClick(0, 6). + Tap(func() { + t.FileSystem().FileContent("edit-command", Contains("/repo/file1:2\n")) + }) + + t.Views().Menu(). + IsFocused(). + Press(keys.Universal.Return) + }, +}) diff --git a/pkg/integration/tests/main_view/no_selection_over_a_conflict_hint.go b/pkg/integration/tests/main_view/no_selection_over_a_conflict_hint.go index 45cd19ce2..d4748365a 100644 --- a/pkg/integration/tests/main_view/no_selection_over_a_conflict_hint.go +++ b/pkg/integration/tests/main_view/no_selection_over_a_conflict_hint.go @@ -11,6 +11,7 @@ var NoSelectionOverAConflictHint = NewIntegrationTest(NewIntegrationTestArgs{ Skip: false, SetupConfig: func(config *config.AppConfig) { config.GetUserConfig().Gui.ShowFileTree = false + config.GetUserConfig().OS.EditAtLine = "echo {{filename}}:{{line}} > edit-command" }, SetupRepo: func(shell *Shell) { shell.RunShellCommand(`echo 1 > foo && echo 1 > bar`) @@ -47,6 +48,20 @@ var NoSelectionOverAConflictHint = NewIntegrationTest(NewIntegrationTestArgs{ PressPrimaryAction(). Tap(func() { t.ExpectToast(Contains("There is nothing to select here")) + }). + // A pane with nothing to select still has lines to point at. A modified + // click names its own line rather than acting on the selection, so it + // opens the file there. The file is in the working tree for a conflict + // like this one, holding the modified side; you may want to copy a piece + // of it elsewhere before resolving the conflict by deleting the file. + // + // The cursor moves where a plain click points even here, so the assertion + // below says which row the modified click then lands on. + Click(0, 17). + SelectedLine(Contains("+2")). + AltClick(0, 17). + Tap(func() { + t.FileSystem().FileContent("edit-command", Contains("/repo/bar:1\n")) }) }, }) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 997aa28db..55a647aa1 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -391,6 +391,7 @@ var tests = []*components.IntegrationTest{ main_view.DiscardLinesFromACommit, main_view.DragRangeWithAutoscroll, main_view.DragSelectsDiffLineRange, + main_view.EditClickedDiffLine, main_view.EditHistoricalDiffLine, main_view.EditHunkInFocusedDiff, main_view.EditSelectedDiffLine, diff --git a/pkg/integration/types/types.go b/pkg/integration/types/types.go index 325f4ea38..b6655aebc 100644 --- a/pkg/integration/types/types.go +++ b/pkg/integration/types/types.go @@ -28,6 +28,9 @@ type GuiDriver interface { // user typing faster than lazygit processes the input. PressKeysRapidly(...string) Click(int, int) + // Click with a keyboard modifier held down, for the gestures that only exist + // as a modified click. + ClickWithModifier(int, int, gocui.Modifier) ClickAndHold(int, int) MouseMove(int, int) MouseRelease(int, int) From 7aff6d7eae04569fee3c3f54fcee05280b4cada4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 10:19:07 +0200 Subject: [PATCH 8/9] Give views a transient line flash Actions that hand control to another application need visible acknowledgement without moving a view's cursor or replacing renderer colors. Reverse the narrow selection bar independently of selection state, and clear transient flashes whenever the terminal UI suspends. --- pkg/gocui/flush_test.go | 17 +++++++++++++++++ pkg/gocui/gui.go | 3 +++ pkg/gocui/suspend_test.go | 11 +++++++++++ pkg/gocui/view.go | 27 +++++++++++++++++++++++++++ pkg/gocui/view_test.go | 26 ++++++++++++++++++++++++++ 5 files changed, 84 insertions(+) diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index b4f42afc4..d5d6b585f 100644 --- a/pkg/gocui/flush_test.go +++ b/pkg/gocui/flush_test.go @@ -81,6 +81,23 @@ func TestFlushContentOnly_WritesCorrectContent(t *testing.T) { assert.Equal(t, "Fetching |", status.Buffer()) } +func TestForceFlushViewsContentOnlyDrawsLineFlash(t *testing.T) { + g := newTestGui(t) + _, main := setupViews(t, g) + main.Highlight = true + main.SelBgColor = ColorBlue + main.SelectedLineColorWidth = 2 + main.FocusPoint(0, 0, false) + + main.SetLineFlash(0) + g.ForceFlushViewsContentOnly(g.Views()) + + for x := main.x0 + 1; x <= main.x0+2; x++ { + _, style, _ := Screen.Get(x, main.y0+1) + assert.True(t, style.HasReverse(), "selection-bar cell at x=%d should flash", x) + } +} + func TestProcessEvent_ContentOnlyEvent_SkipsTaintedCheck(t *testing.T) { g := newTestGui(t) status, main := setupViews(t, g) diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index f4f87a281..465a7b48e 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -2149,6 +2149,9 @@ func (g *Gui) Suspend() error { return errors.New("Already suspended") } + for _, view := range g.views { + view.ClearLineFlash() + } g.suspended = true if err := g.screen.Suspend(); err != nil { diff --git a/pkg/gocui/suspend_test.go b/pkg/gocui/suspend_test.go index 5a7a070d6..28d51ffac 100644 --- a/pkg/gocui/suspend_test.go +++ b/pkg/gocui/suspend_test.go @@ -67,3 +67,14 @@ func TestResumeSchedulesRedraw(t *testing.T) { assert.Equal(t, eventResize, ev.Type, "resuming must schedule a redraw; without one the screen stays blank until the next event arrives") } + +func TestSuspendClearsLineFlashes(t *testing.T) { + g := newTestGui(t) + v, err := g.SetView("main", 0, 0, 20, 10, 0) + assert.ErrorIs(t, err, ErrUnknownView) + v.SetLineFlash(3) + + assert.NoError(t, g.Suspend()) + assert.Equal(t, -1, v.lineFlashY) + assert.NoError(t, g.Resume()) +} diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index fa5eb934d..b195768dc 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -80,6 +80,10 @@ type View struct { // a user starts a range select and then moves the cursor up. rangeSelectStartY int + // The view line whose selection-width bar is temporarily reversed. A value + // of -1 means that no line is flashing. + lineFlashY int + // readBuffer is used for storing unread bytes readBuffer []byte @@ -811,6 +815,7 @@ func NewView(name string, x0, y0, x1, y1 int, mode OutputMode) *View { searcher: &searcher{}, TextArea: &TextArea{}, rangeSelectStartY: -1, + lineFlashY: -1, TabWidth: 4, } @@ -970,6 +975,10 @@ func (v *View) setCharacter(x, y int, ch string, fgColor, bgColor Attribute, isW fgColor |= AttrUnderline } + if v.lineFlashY == v.oy+y && (v.SelectedLineColorWidth == 0 || x < v.SelectedLineColorWidth) { + fgColor ^= AttrReverse + } + // Don't display empty characters if ch == "" { ch = " " @@ -2323,6 +2332,24 @@ func applySelTextColor(fgColor, selTextColor Attribute) Attribute { return fgColor | selTextColor&AttrStyleBits } +// SetLineFlash temporarily marks a view line without moving or changing the +// selection. The caller owns the lifetime and clears it with ClearLineFlash. +func (v *View) SetLineFlash(viewLine int) { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + v.lineFlashY = viewLine + v.needsRedraw = true +} + +func (v *View) ClearLineFlash() { + v.writeMutex.Lock() + defer v.writeMutex.Unlock() + + v.lineFlashY = -1 + v.needsRedraw = true +} + func lineWrap(line []cell, columns int) [][]cell { if columns == 0 { return [][]cell{line} diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 0ce688aa6..5731d853e 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -1113,6 +1113,32 @@ func TestSelectedLinesOfWrappedContent(t *testing.T) { assert.Equal(t, []string{"a line that wraps"}, v.SelectedLines()) } +func TestLineFlashReversesTheSelectionBarWithoutChangingSelection(t *testing.T) { + WithSimulationScreen(t, 14, 6) + + v := NewView("name", 0, 0, 11, 5, OutputNormal) + v.Highlight = true + v.SelBgColor = ColorBlue + v.SelectedLineColorWidth = 2 + v.writeString("one\ntwo\nthree\n") + v.FocusPoint(0, 1, false) + v.SetLineFlash(1) + v.draw(true) + + for x := 1; x <= 2; x++ { + _, style, _ := Screen.Get(x, 2) + assert.True(t, style.HasReverse(), "selection-bar cell at (%d, 2) should flash", x) + } + _, style, _ := Screen.Get(3, 2) + assert.False(t, style.HasReverse(), "the flash should stop after the selection bar") + assert.Equal(t, "two", v.SelectedLine(), "flashing should not change the selection") + + v.ClearLineFlash() + v.draw(true) + _, style, _ = Screen.Get(1, 2) + assert.False(t, style.HasReverse(), "clearing should remove the flash") +} + // Resizing a view throws away the wrapping of its content and wraps it again for // the new width, which moves every line of it to a different view line. The // positions into the view count view lines, so they all have to come along. From 150dfadf9bb60277951eb40793e34f472c995815 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 20 Aug 2026 10:20:23 +0200 Subject: [PATCH 9/9] Acknowledge editor clicks in the diff Non-suspending graphical editors can take a moment to reach the foreground, leaving a modified click with no visible response. Briefly reverse the clicked row's selection bar after launching the editor so the registered action is apparent. Arm the flash only for a resolved diff row, keep newer clicks safe from stale timers, and rely on suspension to clear the transient state before terminal editors take over. --- pkg/gui/controllers/main_view_controller.go | 53 +++++++++++++++++++-- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index f040b401c..3f0a1e849 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -1,6 +1,8 @@ package controllers import ( + "time" + "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" @@ -15,10 +17,13 @@ type MainViewController struct { context *context.MainContext otherContext *context.MainContext - dragAutoscroller *helpers.DragAutoscroller - draggingWithMouse bool + dragAutoscroller *helpers.DragAutoscroller + draggingWithMouse bool + lineFlashGeneration uint64 } +const editedLineFlashDuration = 200 * time.Millisecond + var _ types.IController = &MainViewController{} func NewMainViewController( @@ -533,7 +538,42 @@ func (self *MainViewController) onClickInAlreadyFocusedView(opts gocui.ViewMouse } func (self *MainViewController) editClickedLine(opts gocui.ViewMouseBindingOpts) error { - return self.editDiffLine(opts.Y) + var flashGeneration uint64 + err := self.editDiffLine(opts.Y, func() { + self.lineFlashGeneration++ + flashGeneration = self.lineFlashGeneration + self.context.GetView().SetLineFlash(self.lineToFlash(opts.Y)) + self.c.GocuiGui().ForceFlushViewsContentOnly(self.c.GocuiGui().Views()) + }) + if flashGeneration != 0 { + time.AfterFunc(editedLineFlashDuration, func() { + self.c.OnUIThreadContentOnlyBackground(func() error { + if self.lineFlashGeneration == flashGeneration { + self.context.GetView().ClearLineFlash() + } + return nil + }) + }) + } + return err +} + +// lineToFlash returns the view line to flash for an edit of the line clicked at the +// given one. The editor is sent to where that line begins, so a long line wrapped over +// several view lines is flashed at the first of them. If the editor wraps the line too, +// its cursor ends up on the flashed line. When the line begins above the top of the +// viewport, the clicked line is flashed, as the only part of the line on screen. +func (self *MainViewController) lineToFlash(clickedViewLine int) int { + view := self.context.GetView() + bufferLine, ok := view.BufferLineForViewLine(clickedViewLine) + if !ok { + return clickedViewLine + } + firstViewLine, ok := view.ViewLineForBufferLine(bufferLine) + if !ok || firstViewLine < view.OriginY() { + return clickedViewLine + } + return firstViewLine } func (self *MainViewController) onClickInOtherViewOfMainViewPair(opts gocui.ViewMouseBindingOpts) error { @@ -984,14 +1024,17 @@ func (self *MainViewController) editLine() error { if !view.Highlight { return nil } - return self.editDiffLine(view.SelectedLineIdx()) + return self.editDiffLine(view.SelectedLineIdx(), nil) } -func (self *MainViewController) editDiffLine(viewLine int) error { +func (self *MainViewController) editDiffLine(viewLine int, beforeEdit func()) error { info, ok := self.c.Helpers().DiffLine.GetDiffLineInfo(self.context.GetView(), viewLine) if !ok { return nil } + if beforeEdit != nil { + beforeEdit() + } // A file-header row points at the file as a whole rather than at a line in it, so // it opens the file without jumping anywhere — as pressing edit on a file in a side