[SQUASHED] edit-diff-line-with-modified-click

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,11 +76,28 @@ 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())
 }
 
+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)
@@ -231,7 +248,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 +296,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)")

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 {
@@ -391,13 +399,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)
@@ -1127,7 +1134,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()
 }
@@ -1221,7 +1229,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]
@@ -1232,14 +1240,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)
 		}
 	}
 
@@ -1249,19 +1253,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(
@@ -1361,17 +1360,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 := '┌', '┐', '└', '┘'
@@ -1392,18 +1387,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
@@ -1439,9 +1431,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 {
@@ -1464,64 +1454,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.
@@ -1549,40 +1530,35 @@ func (g *Gui) flush() error {
 		}
 	}
 	for _, v := range g.views {
-		if err := g.draw(v); err != nil {
-			return err
-		}
+		g.draw(v)
 	}
 
 	Screen.Show()
 	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
-func (g *Gui) flushContentOnly(views []*View) error {
+// 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() {
-		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 {
 	redrawIndexes := set.New[int]()
 
 	for i, v := range views {
-		if !v.IsTainted() && !redrawIndexes.Includes(i) {
+		if !v.NeedsRedraw() && !redrawIndexes.Includes(i) {
 			continue
 		}
 
@@ -1612,17 +1588,17 @@ 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) error {
-	return g.flushContentOnly(views)
+func (g *Gui) ForceFlushViewsContentOnly(views []*View) {
+	g.flushContentOnly(views)
 }
 
 // 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 {
@@ -1661,30 +1637,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
@@ -1768,6 +1732,22 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
 			}
 		}
 
+		var mouseOpts ViewMouseBindingOpts
+		if IsMouseKey(ev.Key) {
+			isDoubleClick := g.recordClickInfo(newX, newY, ev.Key.KeyName(), v)
+			mouseOpts = ViewMouseBindingOpts{X: newX, Y: newY, Key: ev.Key.KeyName(), IsDoubleClick: isDoubleClick}
+
+			// 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 {
+				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
@@ -1817,9 +1797,7 @@ 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)
+			matched, err := g.execMouseKeybindings(v, ev, mouseOpts, false)
 			if err != nil {
 				return err
 			}
@@ -1883,11 +1861,12 @@ func (g *Gui) recordClickInfo(x, y int, key KeyName, v *View) bool {
 	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
@@ -2056,6 +2035,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 {

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 {
@@ -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())
+}

pkg/gocui/tcell_driver.go

@@ -202,6 +202,7 @@ const (
 
 var (
 	lastMouseKey tcell.ButtonMask = tcell.ButtonNone
+	lastMouseMod tcell.ModMask    = tcell.ModNone
 	dragState                     = NOT_DRAGGING
 	lastX                         = 0
 	lastY                         = 0
@@ -370,6 +371,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
@@ -395,7 +402,8 @@ func gocuiEventFromTcellEvent(tev tcell.Event) GocuiEvent {
 				case tcell.ButtonMiddle:
 				default:
 				}
-				mouseMod = ModNone
+				mouseMod = Modifier(lastMouseMod)
+				lastMouseMod = tcell.ModNone
 				lastMouseKey = tcell.ButtonNone
 			}
 		default:
@@ -426,10 +434,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

pkg/gocui/view.go

@@ -79,12 +79,21 @@ 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
 
 	// 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 or highlighted since viewLines was last refreshed, and whose
 	// cached wrapping (lineType.wrappedCells) may therefore be stale. Lines
@@ -265,11 +274,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
@@ -581,6 +597,12 @@ func (v *View) SetRangeSelectStart(rangeSelectStartY int) {
 	v.rangeSelectStartY = rangeSelectStartY
 }
 
+// RangeSelectStartY returns the view line the range selection is anchored on,
+// or -1 when there is no range.
+func (v *View) RangeSelectStartY() int {
+	return v.rangeSelectStartY
+}
+
 func (v *View) CancelRangeSelect() {
 	v.rangeSelectStartY = -1
 }
@@ -713,11 +735,13 @@ 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{},
 		TextArea:          &TextArea{},
 		rangeSelectStartY: -1,
+		lineFlashY:        -1,
 		TabWidth:          4,
 	}
 
@@ -874,6 +898,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 = " "
@@ -1066,7 +1094,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)
@@ -1489,7 +1517,7 @@ func (v *View) SwapInOffscreenRender() {
 	}
 	v.buf = v.offscreen
 	v.offscreen = nil
-	v.tainted = true
+	v.markViewLinesDirty()
 	v.clearHover()
 }
 
@@ -1646,6 +1674,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()
@@ -1654,6 +1688,7 @@ func (v *View) draw(isWindowFocused bool) {
 	if !v.Visible {
 		return
 	}
+	defer func() { v.needsRedraw = false }()
 
 	v.clearRunes()
 
@@ -2153,28 +2188,22 @@ func indexFunc(r rune) bool {
 	return r == ' ' || r == 0
 }
 
-// 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
-	}
+// 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()
 
-	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()
+	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 {

pkg/gocui/view_test.go

@@ -960,6 +960,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.

pkg/gui/controllers/helpers/diff_line_selection.go

@@ -116,6 +116,17 @@ func (self *DiffLineHelper) SelectChangeBlock(
 	self.ShowSelectionAtLine(view, start, scrollIntoView)
 }
 
+// SelectedHunkBounds returns the change block selected in hunk mode. The range
+// anchor stays on the block's far end when a click moves the cursor before its
+// handler runs, so it still identifies the selected block.
+func (self *DiffLineHelper) SelectedHunkBounds(view *gocui.View) (int, int, bool) {
+	anchor := view.RangeSelectStartY()
+	if anchor < 0 {
+		return 0, 0, false
+	}
+	return self.ChangeBlockBounds(view, anchor)
+}
+
 // RefreshInclusionGutter updates the marks drawn over the diff in the main pane, which
 // say which of its lines are in the custom patch being built from it.
 //

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(
@@ -226,6 +231,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,
+		},
 	}
 }
 
@@ -466,6 +485,27 @@ func (self *MainViewController) onClickInAlreadyFocusedView(opts gocui.ViewMouse
 	return nil
 }
 
+func (self *MainViewController) editClickedLine(opts gocui.ViewMouseBindingOpts) error {
+	var flashGeneration uint64
+	err := self.editDiffLine(opts.Y, func() {
+		self.lineFlashGeneration++
+		flashGeneration = self.lineFlashGeneration
+		self.context.GetView().SetLineFlash(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
+}
+
 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
@@ -563,10 +603,9 @@ func (self *MainViewController) handleDragAutoscroll(viewLine int) bool {
 }
 
 // selectClickedDiffLine sets the focused main view's selection from a click at the
-// given view line. In hunk mode a click on a change line keeps hunk mode and selects
-// that whole block, so clicking from hunk to hunk stays ready to act on one; a click
-// on context drops to a single line, as does any click when we weren't in hunk mode —
-// the click points at that line precisely, e.g. to edit it.
+// given view line. In hunk mode, clicking inside the selected block collapses it to
+// that line; clicking a change line outside it keeps hunk mode and selects that block.
+// A click on context, or any click outside hunk mode, selects just that line too.
 func (self *MainViewController) selectClickedDiffLine(viewLine int) {
 	if !self.isDiffView() {
 		return
@@ -575,10 +614,17 @@ func (self *MainViewController) selectClickedDiffLine(viewLine int) {
 	// Remember where the click landed so that a drag that follows anchors its range
 	// there, even when this click selects a whole hunk.
 	self.context.SetDragAnchorViewLine(viewLine)
-	if self.diffSelectState().Mode == types.DiffSelectModeHunk &&
-		self.c.Helpers().DiffLine.IsChangeLine(view, viewLine) {
-		self.selectHunkAround(viewLine, false)
-		return
+	if self.diffSelectState().Mode == types.DiffSelectModeHunk {
+		if start, end, ok := self.c.Helpers().DiffLine.SelectedHunkBounds(view); ok &&
+			viewLine >= start && viewLine <= end {
+			self.context.ResetDiffSelectMode()
+			self.c.Helpers().DiffLine.ShowSelectionAtLine(view, viewLine, false)
+			return
+		}
+		if self.c.Helpers().DiffLine.IsChangeLine(view, viewLine) {
+			self.selectHunkAround(viewLine, false)
+			return
+		}
 	}
 	self.context.ResetDiffSelectMode()
 	self.c.Helpers().DiffLine.ShowSelectionAtLine(view, viewLine, false)
@@ -878,11 +924,17 @@ func (self *MainViewController) editLine() error {
 	if !view.Highlight {
 		return nil
 	}
+	return self.editDiffLine(view.SelectedLineIdx(), nil)
+}
 
-	info, ok := self.c.Helpers().DiffLine.GetDiffLineInfo(view, view.SelectedLineIdx())
+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

pkg/integration/tests/main_view/select_hunk_on_focusing_main_view.go

@@ -46,7 +46,19 @@ var SelectHunkOnFocusingMainView = NewIntegrationTest(NewIntegrationTestArgs{
 				Contains("-nine"),
 				Contains("+NINE"),
 			).
-			// A click on a context line points at it precisely, so it stays a single line.
+			// A click inside the selected block collapses hunk mode to that line.
+			Click(0, 15).
+			SelectedLines(
+				Contains("+NINE"),
+			).
+			// Switch back to hunk mode so the context click below proves that it gives
+			// hunk mode up, rather than merely keeping line mode.
+			Press(keys.Main.ToggleSelectHunk).
+			SelectedLines(
+				Contains("-nine"),
+				Contains("+NINE"),
+			).
+			// A click on a context line points at it precisely, so it selects that line.
 			Click(0, 12).
 			SelectedLines(
 				Contains(" seven"),