Not only is this nicer code (and more idiomatic at least in this code
base), but it also avoids linter warnings about missing preallocations
(lo.Map does preallocate the result array).
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>
A failing setup step called Shell.fail, which panicked. Tests run as
parallel subtests, so that panic aborted the entire test binary: one bad
fixture cost us the results of all ~500 tests, and the failure was
reported as a stack trace rather than against the test that caused it.
Keep panicking to skip the remaining setup steps -- they would only
produce follow-on failures -- but recover in createFixture and return the
message as that test's error. All three clients already propagate an
error from a test, so they report it the way they report any other
failure.
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>
To spot slow or anomalous tests across CI runs, record each test's run
duration when LAZYGIT_TEST_TIMING is set (to a file path);
run_integration_tests.sh prints them at the end, sorted by slowest
first. CI sets it for all integration jobs.
The harness appends to a file rather than writing to stdout/stderr
because `go test` captures those and only surfaces them with -v, which
would drown the signal in every test's verbose logs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some tests assert that a specific commit subject does or doesn't occur
in the main view; interactive_rebase/outside_rebase_range_select.go is
an example for this, it asserts `t.Views().Main().Content(
DoesNotContain("commit 06"))`. The problem with this kind of assertion
and our test commit naming scheme is that the diff view begins with a
"commit <hash>" line, and when that hash happens to start with "06" the
assertion matched it and failed spuriously. This was usually masked by
our MaxAttempts=2 that we currently use for integration tests (it's
quite unlikely that the commit gets a hash beginning with "06" twice in
a row). However, we want to get to a state where we can set MaxAttempts
to 1, so make this more robust by changing our naming scheme.
When conflicts of an in-progress rebase/merge/cherry-pick/revert are
resolved, lazygit pops up a prompt offering to continue it. This is
helpful when you started the operation in lazygit and resolved the
conflicts in your editor. But it's confusing when the operation was
started outside lazygit — e.g. by a coding agent in another terminal
that resolves the conflicts but hasn't continued yet because it's still
running tests or fixing the build. lazygit would then prompt unbidden.
Track whether the in-progress operation was started from within lazygit,
and only show the prompt in that case. We record this right after running
a merge/rebase step (in CheckMergeOrRebaseWithRefreshOptions, the
subprocess branch of genericMergeCommand, and the custom-command
conflict path), and clear it whenever a refresh observes that no
operation is in progress — which also handles an operation that was
finished or aborted externally.
The conflict-resolution tests start their operation by running git
directly (not through lazygit's UI), so they call the new test helper
Common.PretendMergeOrRebaseStartedInLazygit to have lazygit treat the
operation as its own and still get the prompt.
Co-Authored-By: Claude Opus 4.8 (1M context) <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>
Lazygit reloads changed config files when its terminal window regains
focus, but the test harness had no way to simulate that focus event, so
the live config-reload path was untestable. Add a focus event to the
replayed-events queue and expose it through the GuiDriver as FocusIn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a user opens a repo from the recent-repos menu or jumps between
worktrees inside lazygit, only the env vars present at process startup
reach subprocesses. That breaks pre-commit hooks and other tools whose
dependencies are pulled in by a per-repo .envrc — users were left with
read-only operations because the env their shell would normally load via
direnv never made it into lazygit's git invocations.
Shell out to `direnv export json` after each chdir and apply the JSON
delta via os.Setenv/Unsetenv. direnv tracks the previous load in its own
DIRENV_DIFF env var, so the delta also unloads vars from the old repo
when entering one without a matching .envrc. If direnv isn't on PATH the
call is a no-op, so users who don't use direnv pay nothing and users who
do need no config to opt in. Any stderr direnv emits (loading messages,
"blocked .envrc" errors, etc.) goes to the command log.
The integration test puts a fake direnv on PATH and asserts that a value
it exports reaches a custom command after switching repos. Wiring this
up needed runner.go to support `{{actualPath}}` placeholders in
ExtraEnvVars, mirroring the existing support for ExtraCmdArgs, so the
test can prepend a fixture-relative directory to PATH.
Co-Authored-By: Claude Opus 4.7 (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.
Shell.RunShellCommand was passing os.Environ() to its child, while its
sibling runCommandWithOutputAndEnv has used the minimal
NewTestEnvironment since late 2023 when env isolation was introduced;
the sh path was just missed.
This matters when integration tests run from inside a `git rebase -x`
exec in a linked worktree: git sets GIT_DIR=<main>/.git/worktrees/<name>
for the exec, and it leaks all the way down through bash, just, go test,
and the test process, into every git invocation RunShellCommand spawns.
cmd.Dir becomes irrelevant — git resolves GIT_DIR over cwd-based
discovery, with the work-tree taken from the gitdir file (i.e. the
worktree root). So `git checkout -b conflict` in a test fixture creates
the branch on the real worktree and switches its HEAD, hijacking the
in-progress rebase and trashing the working tree. (In the main worktree
git doesn't set GIT_DIR for rebase exec, which is why the bug was only
visible from linked worktrees.)
Using self.env also incidentally restores GIT_CONFIG_GLOBAL for shell
commands, so commits made via RunShellCommand are now authored by the
test config's CI identity rather than whatever the host's ~/.gitconfig
resolves to.
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.
It works for submodules too.
Also, pass file name and file content explicitly; the existing tests don't care
about these, but when writing tests that do, it makes them easier to understand.
It seems useful to have the flexibility to remap "enter" in confirmations to
"y", but keep "enter" for menus and suggestions (even though we sometimes use
menus as confirmations, but it's still good to give users the choice).
This one doesn't make a difference in practice because we don't remap the key in
tests, but if we would, then this would no longer work correctly. It's just more
correct this way.
So far, confirmations and prompts were handled by the same view, context, and
controller, with a bunch of conditional code based on whether the view is
editable. This was more or less ok so far, since it does save a little bit of
code duplication; however, now we need separate views, because we don't have
dynamic keybindings, but we want to map "confirm" to different keys in
confirmations (the "universal.confirm" user config) and prompts (hard-coded to
enter, because it doesn't make sense to customize it there).
It also allows us to get rid of the conditional code, which is a nice benefit;
and the code duplication is actually not *that* bad.
The test shows that selecting a commit before the rename shows an empty diff,
and selecting the rename itself shows an added file rather than a rename.
When you are in the middle of a rebase, and you cherry-pick a commit which
conflicts, it helps to be clear on whether you are prompted to continue the
cherry-pick or the rebase.
The easiest way to do that is to rename the local branch after pushing.
This shows various levels of brokenness for the reset and rebase to upstream
commands: both menu entries display the wrong upstream branch name in the menu
(the local one rather than the remote one); executing the rebase command works
correctly though, the rebase command uses the right branch name. Resetting
fails, though.
We'll fix this in the next commit.
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.
If a `t.FileSystem().FileContent("file.txt", Equals("bla"))` assertion fails
because the file doesn't exist, the error would say
Expected path 'file.txt' to not exist, but it does
which is very confusing.
Stefan Haller
Karol ZwolakandStefan Haller
phaniumandStefan Haller
kyu08andStefan Haller
BrandonandStefan Haller
Chris McDonnell
AzraelSecandStefan Haller
Anvar UmuralievandJesse Duffield
Nathan BaulchandJesse Duffield
Yaroslav HalchenkoandStefan Haller