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/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/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 144fae11f..465a7b48e 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 { @@ -413,13 +421,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 +1202,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 +1304,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 +1315,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 +1328,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 +1435,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 +1462,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 +1506,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 +1529,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,40 +1605,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 } @@ -1687,11 +1663,11 @@ 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) } // hasFocus reports whether a view is drawn as focused. Views that are embedded @@ -1709,9 +1685,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 +1726,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 @@ -1857,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 @@ -1864,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 @@ -1906,9 +1899,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 } @@ -1940,43 +1932,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 @@ -2151,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/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/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") +} 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 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 diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index a43ee1815..b195768dc 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -80,18 +80,27 @@ 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 - // 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 @@ -274,11 +283,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,11 +809,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, } @@ -957,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 = " " @@ -1177,7 +1199,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 +1634,7 @@ func (v *View) SwapInOffscreenRender() { } v.buf = v.offscreen v.offscreen = nil - v.tainted = true + v.markViewLinesDirty() v.clearHover() } @@ -1776,6 +1798,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 +1812,7 @@ func (v *View) draw(isWindowFocused bool) { if !v.Visible { return } + defer func() { v.needsRedraw = false }() v.clearRunes() @@ -1908,8 +1937,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,28 +2332,22 @@ 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 - } +// 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 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. diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index ef54d149e..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( @@ -241,6 +246,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 +537,45 @@ 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(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 { // 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 @@ -966,11 +1024,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/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)