Drag autoscrolling deliberately keeps rendering after the test action
has returned, so view bounds can change while the test goroutine
prepares its next mouse event. Snapshot the geometry on the event loop
before translating view-relative coordinates to avoid data races. This
hasn't been a problem so far, but only because we were lucky; the added
test assertions later in this branch would cause consistent race
detector failures without this fix.
Since scrolling the selection into view became opt-in, five places have
had to be fixed by hand after the fact, none of them with a test. Cover
them now: making the scrolling automatic has to keep all five working,
and once it does, the hand-added scroll calls can go.
Two of them assert that the selection is visible rather than on an exact
scroll position, because the panel they look at changes height along the
way (filtering mode switches to half screen), or because what matters is
only that the commit we jumped to can be seen.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The one behaviour that made scrolling the selection into view opt-in in
the first place — a background refresh must not yank the view back to a
selection the user scrolled away from — has never been covered by a test.
It's about to become the one case that the automatic scrolling has to
suppress, so cover it first.
Getting there needs two things from the test harness: mouse wheel events,
which are the only way to scroll a list panel without moving the
selection, and a way to trigger a background refresh. The periodic
routine that issues it is turned off in tests, and turning it on would
mean waiting for its timer and hoping it fires while we're looking, so
drive the refresh directly instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
We are about to make list panels scroll their selection into view
automatically. Page up and down are one of the few places that manage
the scroll position themselves, keeping the selection at the edge of the
viewport rather than in its middle, and nothing covers that today.
Asserting on it needs an exact scroll position assertion; only
OriginYAtLeast existed so far.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When the pointer reaches the edge of the view during a drag (or leaves
the view entirely, which mouse capture makes possible), keep scrolling
and extending the selection: slowly on the innermost edge row, faster
on the outermost row, and very fast beyond. Scrolling starts after a
short delay so that a drag merely passing near the edge doesn't scroll.
When the view loses focus mid-drag (e.g. because a popup appeared),
cancel the autoscroll and the mouse capture.
Add press/move/release primitives next to the existing Click. The test
driver remembers the last reported position so a release doesn't have
to repeat the coordinates, and RepeatMouseMove lets a test verify that
a held-button motion event within the same cell has no effect.
When clicking in the commits view of lazygit running in an unfocused VS
Code window, VS Code first sends us the focus-in event and then the
mouse-click. The focus-in refresh captures the selection when it starts,
then we handle the mouse click and you briefly see the clicked row
getting selected, but then the selection flashes back to the original
row as the refresh restores it when done.
The test driver waits for lazygit to become idle after every keypress, so
tests could never exercise what happens when a key arrives while the
previous key's processing is still in flight — for example while the
refresh triggered by the previous key hasn't updated the model yet. Real
users type faster than that all the time.
PressRapidly injects all its keys back to back and waits for idle only
once at the end, so the second and later keys are queued before the first
one's processing has finished. The next commit uses this to demonstrate a
bug in exactly that scenario.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Side panel tabs share a window, so which tab is shown is decided by view
z-order rather than the visibility flag (every tab in a window is
'visible'). Tests had no way to assert which tab is actually drawn in
front, which is distinct from which view has keyboard focus. Expose the
window's top view and add an IsActiveTab assertion built on it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JumpToBlock is special: each of its 5 elements is the binding for one side
window (status / files / branches / commits / stash), not an alternate for a
single command. Change the field from []string to []Keybinding so each window
slot can have alternates of its own.
The schema becomes "an array of 5 keybindings, each itself a string or array of
strings", which falls out cleanly from how the Keybinding type inlines into the
generated schema. Existing configs (a flat array of 5 strings) keep validating
because each element is unmarshalled through Keybinding's scalar-or-sequence
decoder.
Until now every keybinding config field was a plain string. That meant a user
couldn't ask for two keys to invoke a command — the config silently accepted
only one form.
Convert every string-typed field across all 13 KeybindingXxxConfig structs to
Keybinding so the union type extends to every command. Defaults wrap their
single-key value in Keybinding{...} so the generated Config.md still renders one
scalar key per binding.
The alt fields keep their separate Binding registrations for now: this commit
does not yet introduce the merge mechanism that folds them into the main field —
that comes in a follow-up. Consumers previously calling opts.GetKeys on a string
field now call opts.GetKeys on the Keybinding, or take .String() / Keys[0] where
a single value is needed.
Adds a Keybinding.String helper for rendering, schema-generator work that
inlines the Keybinding union into each consuming property, and a unit test
covering the user-facing scalar/sequence YAML forms for quit.
I copied all files except dot files (.github and .gitignore), the _examples
folder, and go.mod/go.sum.
At some point we may want to copy the files back to the gocui repo when other
clients (e.g. lazydocker) want to use the newer versions of them.
Original commit message of the gocui change:
This fixes View.Size, Width and Height to be the correct (outer) size of a view
including its frame, and InnerSize/InnerWidth/InnerHeight to be the usable
client area exluding the frame. Previously, Size was actually the InnerSize (and
a lot of client code used it as such, so these need to be changed to InnerSize).
InnerSize, on the other hand, was *one* less than Size (not two, as you would
have expected), and in many cases this was made up for at call sites by adding 1
(e.g. in calcRealScrollbarStartEnd, parseInput, and many other places in the
lazygit code).
There are still some weird things left that I didn't address here:
- a view's lower-right coordinates (x1/y1) are one less than you would expect.
For example, a view with a 2x2 client area like this:
╭──╮
│ab│
│cd│
╰──╯
in the top-left corner of the screen (x0 and y0 both zero) has x1/xy at 3, not
4 as would be more natural.
- a view without a frame has its coordinates extended by 1 on all sides; to
illustrate, the same 2x2 view as before but without a frame, sitting in the
top-left corder of the screen, has coordinates x0=-1, y0=-1, x1=2, y1=2. This
is highly confusing and unexpected.
I left these as they are because they would be even more of a breaking change,
and also because they don't have quite as much of an impact on general app code.
Previously if we marked a line with IsSelected() we would check if it was selected, but
we would not check if other lines were unexpectedly selected. Now, if you use IsSelected(),
we ensure that _only_ the lines you marked as such are the selected lines.
We're not fully standardising here: different contexts can store their range state however
they like. What we are standardising on is that now the view is always responsible for
highlighting the selected lines, meaning the context/controller needs to tell the view
where the range start is.
Two convenient benefits from this change:
1) we no longer need bespoke code in integration tests for asserting on selected lines because
we can just ask the view
2) line selection in staging/patch-building/merge-conflicts views now look the same as in
list views i.e. the highlight applies to the whole line (including trailing space)
I also noticed a bug with merge conflicts not rendering the selection on focus though I suspect
it wasn't a bug with any real consequences when the view wasn't displaying the selection.
I'm going to scrap the selectedRangeBgColor config and just let it use the single line
background color. Hopefully nobody cares, but there's really no need for an extra config.
A better refactor would be to allow matchers to assert against either a string or a slice of cells, so that I could have
the same ergonomics that I have elsewhere, but this is a start.
When we use the one panel for the entire commit message, its tricky to have a keybinding both for adding a newline and submitting.
By having two panels: one for the summary line and one for the description, we allow for 'enter' to submit the message when done from the summary panel,
and 'enter' to add a newline when done from the description panel. Alt-enter, for those who can use that key combo, also works for submitting the message
from the description panel. For those who can't use that key combo, and don't want to remap the keybinding, they can hit tab to go back to the summary panel
and then 'enter' to submit the message.
We have some awkwardness in that both contexts (i.e. panels) need to appear and disappear in tandem and we don't have a great way of handling that concept,
so we just push both contexts one after the other, and likewise remove both contexts when we escape.
Stefan Haller
AzraelSecandStefan Haller
Yaroslav HalchenkoandStefan Haller
Jesse Duffield
Simon Whitaker
SeanandJesse Duffield