diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index d4082fcf6..d5d6b585f 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,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)") diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 2edbe960d..9a85aba0c 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 { @@ -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 { diff --git a/pkg/gocui/suspend_test.go b/pkg/gocui/suspend_test.go index ded220bea..28d51ffac 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 { @@ -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/tcell_driver.go b/pkg/gocui/tcell_driver.go index 312d6d5a2..ab36becff 100644 --- a/pkg/gocui/tcell_driver.go +++ b/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 } } 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 diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 6d7c8a168..35198e59e 100644 --- a/pkg/gocui/view.go +++ b/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 { diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 6754c36fe..b6dd6f401 100644 --- a/pkg/gocui/view_test.go +++ b/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. diff --git a/pkg/gui/controllers/helpers/diff_line_selection.go b/pkg/gui/controllers/helpers/diff_line_selection.go index 4ace36554..5d84cf3bc 100644 --- a/pkg/gui/controllers/helpers/diff_line_selection.go +++ b/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. // diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index a26d476a5..d0a51cd28 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( @@ -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 diff --git a/pkg/integration/tests/main_view/select_hunk_on_focusing_main_view.go b/pkg/integration/tests/main_view/select_hunk_on_focusing_main_view.go index caf5419a7..00e413530 100644 --- a/pkg/integration/tests/main_view/select_hunk_on_focusing_main_view.go +++ b/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"),