Git limits its tree diff by the pathspec before it looks for renames, so
a directory only ever gets one end of a rename whose other end is outside
it. Nothing is left to pair up, and the file turns into an addition or a
deletion that the commit doesn't contain.
Pass the other end along with the directory. This is bounded by the
number of renames that cross the directory's boundary, so it costs
nothing at all for the vast majority of commits.
The files and commit files panels each had their own copy of this, one of
which used to be missing the previous path of a rename. Growing them
apart again is the last thing we want, since the next commit needs to
teach both of them about renames that cross a directory boundary.
The files panel version only returned paths for the filtered case, and
left it to WorktreeFileDiffCmdObj to derive the rest from the node; now
that all callers pass the paths in, that command doesn't need to know
about renames at all.
Restricting the diff to the files that a filter leaves visible drops the
delete-side entry of a staged rename, so git shows the file as an
addition instead. Its commit files counterpart already passes both paths;
this brings the files panel in line.
Pathspec limiting happens before rename detection in git's tree diff, so
filtering the diff to a directory hides the delete-side entry of a rename
whose other end is outside that directory. Git then has nothing to pair
up, and reports a file moved into the directory as an addition and one
moved out of it as a deletion.
Selecting a directory is supposed to filter the commit's diff down, never
to change it, so both are wrong.
When several files have conflicts, resolving one of them makes it vanish
from the files panel as soon as it is auto-staged, and it only comes back
once the last conflict is resolved and the filter turns off again. By
then it sits among all the other changed files of the merge, so it is
hard to find the ones whose resulting diff you still wanted to check.
So remember which files had conflicts while the conflicted-files filter
is on, and keep showing them once they are resolved. This is the general
solution that 39513d244d called for; that commit only helped for the
case of a single conflicted file.
The consequence is that the selection no longer moves on to the next
conflicted file when one is resolved: it stays on the file you just
resolved, which shows you its diff right away.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The next commit needs to know which files have conflicts, not just how
many of them there are.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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).
It existed for the incremental re-render: a shorter render left the previous
one's view lines in the tail (deliberately, to avoid a blank frame), and this
cleared them once the new content was fully read. Async renders now build
off-screen and swap in whole, so refreshViewLinesIfNeeded truncates the view
lines to the buffer and no tail can form. All the call at end-of-input still
did was discard every wrapped line and force the whole buffer to be re-wrapped
on the next draw, which is pure work on a large diff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When a main view re-renders content different from what it last showed, the
scroll resets to the top. That reset fired synchronously when the task started —
but with the off-screen render the previous content stays displayed until the
swap, so resetting the origin up front scrolled that still-visible content to the
top before the new content replaced it: a distracting jump when switching commits
(or any item) while scrolled down.
Defer the reset to the first paint that reveals the new content, so the previous
content stays at its scroll until the new content takes its place, and then the
new content appears at the top. Swap and reset happen in one hop on the UI
thread, so no draw can land between them and show the new content at the old
scroll. A same-content re-render keeps its scroll. The "loading..." indicator
path also resets the origin now, since it clears the previous content to show the
message and must put it at the top.
The reset moves out of NewTask into the read loop, keying off the flag that
already records whether the render's content is new. NewTask still decides,
from the same command-key comparison as before and under the same lock. It has
to be that flag rather than per-task state, because a task can be stopped and
replaced before it ever paints — a background refresh landing just after the
user clicked a different item, which is the ordering a VS Code terminal
produces, since it delivers the focus-in event (and so the refresh) before the
click. The replacement renders the same content and so sets nothing of its own,
and the click's reset would be lost with the task that owed it.
The manager's onNewKey callback is renamed resetOrigin to match its now-decoupled
timing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When a task is stopped to make way for a newer one, stopping closes
opts.Stop, and the scanner goroutine then closes lineChan. The read loop's
select between those two channels is therefore non-deterministic: it can
land on the closed lineChan (ok == false) instead of the opts.Stop case,
sending a stopped task into the end-of-input branch.
There it runs the full finalize — swapping its half-read off-screen buffer
in, clamping the origin to the truncated content, and clearing the loading
flag — all of which corrupt what the incoming task is about to render. The
most visible symptom is a brief frame of truncated content with the scroll
yanked to the top, seen when re-renders overlap rapidly (e.g. the periodic
background refresh re-rendering a main view faster than it can load, very
easy to hit under LAZYGIT_SLOW_RENDER).
The underlying bug predates the off-screen render (the EOF branch always
clamped the origin via onEndOfInput), but that change made it far worse by
also swapping a truncated buffer into the display. Fix it at the source: in
the EOF branch, check whether we were stopped and, if so, bail out like the
explicit stop case, leaving the view entirely to the task that replaces us.
There's no test because the bug is the non-deterministic select itself:
any test would have to win a coin flip to observe it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A cmd/pty re-render used to overwrite the displayed buffer from the top
down as lines arrived, relying on keeping the previous render's view-line
tail to avoid a blank frame. That left the view showing a mixture of old
and new content while loading, and any reader (draw, clicks, the
view-line mapping) could observe a half-written buffer at the wrong
scroll.
Instead, build the new content in a second, off-screen viewBuffer: until
the task has read enough to paint, writes go there and the displayed
buffer — and so everything every reader sees — is left untouched. Once the
task reaches its first-paint point (InitialRefreshAfter, or EOF for short
content) it swaps the off-screen buffer in atomically, so the view jumps
straight from the previous render to the new one with no intermediate
frame. Subsequent lines append to the now-displayed buffer.
Swapping at the first-paint point means the displayed buffer is only a
viewport tall when it appears and then grows as the rest streams in toward
the count needed for an accurate scrollbar. The scrollbar is sized from the
displayed buffer's height, so left to itself the thumb would shrink and
snap back during that growth (most visibly: the files panel's periodic
refresh making the thumb jump while scrolled down). The total height the
scrollbar needs is a strictly later quantity than the viewport-fill paint,
so no single early swap can have both right. FreezeScrollbarHeight therefore
records the view's height when a load begins and the scrollbar is held there
— growing only if the new content turns out taller — until the load ends; a
synchronous render superseding the load releases it. This mirrors the layout
clamp, which already ignores the partial content height while a view loads.
With the swap doing a wholesale replace, refreshViewLinesIfNeeded can
truncate the view lines to the current buffer: there is no longer a
half-loaded shorter buffer whose tail we must keep showing, so a stale
tail never forms. clear()/Reset() abandon any in-progress off-screen
render so a synchronous SetContent after a stopped task writes to the
display.
The swap holds writeMutex for now; it could later move to the main thread.
Flicker behaviour still needs interactive verification (LAZYGIT_SLOW_RENDER
+ a real diff renderer).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A render that takes more than 200ms to produce its first line takes the view
over to say "loading...", which clears the buffer it was showing. That is
worth doing when the content coming is different — the view is showing
something the user has moved on from, and saying so beats leaving it there
silently. It is pure flicker when the content isn't changing: the view is
already showing exactly what the render is about to put back, and a slow
re-render of unchanged content is common (a background refresh over a repo
with submodules that have uncommitted changes, say).
So track whether the render in flight has content the view isn't already
showing, and only let the indicator take over when it does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
write, writeCells, makeWriteable, parseInput and
autoRenderHyperlinksInCurrentLine produced cells into v.buf; move them onto
viewBuffer so they can write into any buffer, not just the displayed one.
The display-side effects that don't belong to content production —
tainting, clearing hover, updating search positions — stay behind in the
View.write wrapper, which delegates the actual writing to v.buf.write(v).
Render config the writer needs (Editable, colors, width, tab width,
hyperlink auto-render) is read from the passed View. Behaviour-preserving:
the wrapper still always targets v.buf.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fields that make up a view's content and the act of writing to it —
the cell buffer (lines), the write cursor (wx/wy), the escape-sequence
decoder (ei) and the held-newline flag (pendingNewline) — were loose
fields on View. Bundle them into a viewBuffer struct that View holds by
pointer. This is a behaviour-preserving prep refactor: every access just
goes through v.buf now. It sets up rendering into a second, off-screen
viewBuffer that can be swapped in atomically, so an async re-render never
exposes a half-written buffer to readers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
refreshMainViews reset the scroll position of every other main view at the
very top, before moveMainContextPairToTop runs its CopyContent. CopyContent
copies the previously-shown view's content into the now-visible one to avoid a
blank frame during the async re-render — but because the reset ran first, it
had already zeroed the origin of that soon-to-be-copied source view. The
placeholder therefore always appeared scrolled to the top, jumping away from
wherever the screen actually was, on every cross-pair transition.
Move the reset to after the copy. The end state is unchanged (each other main
view still ends at origin 0, and the destination always re-renders), but the
brief placeholder now stays at the source view's real scroll position until
the real content paints.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The layout scrolls a view up if its origin is past the bottom of its
content, to avoid showing blank space (e.g. after a resize). But it measures
content height by the lines loaded so far, and command/pty tasks load
asynchronously. So when a view is re-rendered while scrolled down, the layout
would yank it to the top because only a fraction of the content has been read
yet, then leave it there once loading finished.
Track whether a command task is actively reading (set synchronously when the
task is created, so a layout pass in between sees it; cleared at EOF, but not
when stopped, since that means a newer task is taking over) and skip the
scroll-up clamp for such views. onEndOfInput already re-clamps once loading
completes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A task's read loop processes one LinesToRead request at a time. The initial
request has a large line count and no Then callback; if the content is shorter
than that, the loop hits EOF on the initial request and breaks out, abandoning
any further requests still sitting in the readLines channel. So a ReadToEnd
call that races a still-loading-but-shorter-than-its-initial-read view has its
Then silently dropped: it isn't fired immediately (the channel was non-nil at
call time) and it's never dequeued.
On EOF, drain the queued requests and fire their Then callbacks before
breaking out, since reaching EOF trivially satisfies any "read more" request.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hyperlinkAt (the click path) and onMouseMove/findHyperlinkAt (hover) read
v.viewLines without holding writeMutex, unlike every other reader. They run
on the event-handling goroutine, so a re-render on the task goroutine can
shrink or rebuild viewLines between the bounds check and the indexing,
causing an out-of-range panic (observed: "index out of range [60] with
length 0" while hovering during a diff re-render).
Take writeMutex for the duration, like the other viewLines readers do, so
the check and the access see the same slice.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading a view's internal buffer belongs on the view itself, next to
findHyperlinkAt, rather than in the event loop; and the view is where the
lock that guards that buffer can be taken.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re-rendering a diff into a main view is asynchronous and lazy: the read
loop fills the view a screenful at a time and refreshes as it goes. When
debugging scroll-restore and flicker behaviour, the individual frames go by
too fast to see. Setting LAZYGIT_SLOW_RENDER=<milliseconds> sleeps that long
after each line is written, stretching the load out so the frames become
visible. It has no effect when unset, so it's safe to leave in.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Several methods assigned v.ox and v.oy directly: SetOrigin, CopyContent,
the wrap/autoscroll branches in draw, FocusPoint, and
Scroll{Up,Down,Left,Right}. Funnelling them all through SetOriginX and
SetOriginY gives a single place to observe (or set a breakpoint on)
every change to a view's scroll position, which makes debugging scroll
behaviour much easier.
This means those call sites now also get the setters' `< 0` clamps, but
that is behaviour-preserving in every case: each assigned value is
already >= 0. calculateNewOrigin never returns a negative number;
CopyContent copies origins that are themselves always >= 0; and the draw
and scroll writes are all guarded (or fed only non-negative amounts) so
the result can't go below zero.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
taskKey is written on the goroutine NewTask spawns, under taskIDMutex, but
GetTaskKey read it without the lock — and the string renders in
tasks_adapter.go call that from the UI thread while a previous task's
goroutine may be writing. A Go string is a two-word value, so a torn read
can pair one string's pointer with another's length and index out of
bounds, not merely return the wrong key.
Take the lock in GetTaskKey, and read the field directly at the one call
site that already holds it.
No test: the failure needs two goroutines to interleave inside a
two-word assignment, which nothing can schedule deterministically.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When a rebase (or multi-commit cherry-pick or revert) stops with a
conflict, it is often useful to look at the diff of the "<-- CONFLICT"
commit to double-check that the conflict resolution matches the diff of
the original commit. To make that easier, select that commit
automatically.
This doesn't change anything, we just pin down the selection behavior
around conflicts; we are going to change that behavior, and these tests
will make it obvious how when they change in the next commit.
When there is a single conflicting file left to be resolved, lazygit
dismisses the conflicted-files-only filter when the file no longer has
conflict markers. However, the selection moved to the top, which is
annoying because very often it is useful to look at that file's
resulting diff once more to confirm that conflicts were resolved
correctly, and finding it again can be cumbersome when there are many
changed files. So keep it selected.
Of course, this only helps for the last (or only) conflicted files; when
there are multiple, a resolved file disappears from the panel until all
are resolved, which makes it hard to double-check the resulting diffs.
Doing it afterwards is not easy because you'd have to remember which
files were conflicting. This needs a different solution, but for the
special case of only a single conflicting file this is already a big
improvement.
When using lazygit in a multi-tab terminal it is useful to see if the
lazygit tab is currently active; ghostty does a very good job at dimming
down the inactive tabs, but VS Code's builtin terminal does not, so
indicate this on our side by removing the green highlight from panel
frames and tab titles, and showing the selection as inactive like we do
for a side panel when the main view is focused.
A terminal that supports focus reporting answers with the state it is
already in when we turn reporting on, so at startup we were told that we
had gained focus that we never lost, and refreshed everything a second
time on top of the refresh that loading the repo had just started. The
two ran at once, each with its own `git status`, which made both of them
slower than the one refresh needed to be.
Keep track of what the reports say, then, and pass on only the ones that
change it. Assuming that we start out focused costs us nothing when we
don't: that same first report says so, so a lazygit started in a window
that isn't in front knows it from the start.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Finding out which of the files are worktrees of ours had its own answer
to where this repo's worktrees are, walking the directory that git keeps
them in. The worktrees panel asks git itself, and that is the better
answer: it is the one git gives for the same question elsewhere in the
app, and it doesn't need to know where git records what.
The model that panel fills is all the files need, so mark them from it.
That takes the work out of the file loader, whose other two callers were
paying for it without wanting it, and it costs no git call at all: both
models are written on the UI thread, so whichever of the two refreshes
lands second marks the files against the other's fresh data.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
We show this with a worktree icon (which is only shown when nerd fonts
are used, so turn these on), but also we strip the trailing `/` that
"git status" reports, so that it shows as a file rather than a directory
with a bogus file in it.
The reason for adding the test is that we are going to touch the logic
that determines whether an item in the Files panel is a linked worktree,
and this guards against regressing.
The worktrees were loaded and written by the branches refresh whenever
both were in scope, because the branches view shows worktrees against
branches: refreshing them separately rendered that view twice, once
with worktrees that were still stale.
Ordering the two is enough for that, and it leaves each scope owning
its own model again. The worktrees refresh now runs first and queues
its model write before it reports being done, so a branches refresh
that waits for it queues its own write behind that one, and renders
once with both. The worktrees scope only renders the branches view
itself when nobody else is going to.
As a side effect the two loads now run concurrently, where the branches
refresh used to load the worktrees after its own branches.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Everything in performRefresh is meant to read as "if this scope was
asked for, refresh it", with the scopes that always change together
expanded into each other up front so that each check can name a single
one. The commits and the branches were the exception: one condition
asking for either of them refreshed both, so what that block does only
followed from reading it together with the expansion at the top of the
function. The rebase commits hung off the same condition as an else,
even though it is the commits refresh they are an alternative to.
Expand those two into each other like the other pairs, and give each of
them a check of its own. They now capture their inputs separately,
which is what every other scope has always done.
The reflog stays with the branches rather than getting a check of its
own, because sorting the branches by recency needs it loaded first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It's overkill for most purposes, but I'd say it doesn't hurt to have the
extra resolution available in the raw JSON data for the few cases where
it's useful. Have "lazygit -l" print them with milli-second resolution;
that seems to be a good middle ground.
Besides the misleading error about submodules, the command crashes when
it runs before the first files refresh has come in: the file tree
doesn't exist yet at that point, and staging all of a tree that isn't
there dereferences a nil root node. That is easy to hit in a big repo,
where `git status` takes a moment while the panel sits there empty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The stage-all command acts on the whole file tree, and nothing stops it
from doing that when the tree is empty. It ends up in the branch that
explains why a submodule couldn't be staged, which has nothing to do
with what the user did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every one of these did by hand what focusing the list now does on its
own: five hand-added scroll requests, and four origin resets that paired
a "select the first item" with a "and show the top of the list".
The scroll that the commits refresh performed when it found the selected
commit at a new index goes too. It is now unconditional for a foreground
refresh, and deliberately absent for a background one: when an agent
commits in another window, we would rather see the new commits arrive
than have the view yank itself back to the commit we had selected.
The one origin reset that stays is the one in ReApplyFilter, which runs
as part of a refresh and so can't rely on the refresh scrolling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ever since scrolling the selection into view became opt-in, we have been
fixing the same class of regression by hand, five times so far: a
controller moves the selection somewhere new, doesn't say that it wants
the view to follow, and the selection ends up off screen. The decision
needs facts from two places — whether the selection went somewhere new is
known to the list, whether the scroll position is the caller's to manage
is known to the caller — and asking every caller for both is what keeps
going wrong. The callers that get it wrong are usually not even the ones
that moved the selection: they are pass-throughs like postRefreshUpdate,
which can't know what a refresh did to the selection.
So default to scrolling, and let the two callers that maintain the scroll
position themselves say so.
The one case where scrolling is always wrong is a refresh that no user
action is behind: a background poll, or a reload of state on window
focus, after a subprocess, or after a repo switch. Those must leave the
viewport wherever the user last scrolled it to — that is what made the
scrolling opt-in in the first place. Both are already marked in
RefreshOptions, so the refresh can decide it once, centrally, instead of
each caller judging it.
A user action that ends in a foreground refresh does now yank the view
back to the selection if the user had scrolled away from it. That's a
behaviour change, and there may be actions where it turns out to be
unwelcome; those we can fix individually, and it beats the ones that
don't scroll today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
This is the other place that manages its own scroll position: while a
drag extends the selection to a line below the viewport, the view stays
put, and the drag autoscroller scrolls it one line at a time for as long
as the pointer stays there. Making the scroll automatic would centre the
selection instead, i.e. jump the view rather than scroll it.
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>
go-gh reads gh's config file once per process and answers from that
snapshot for the rest of the process's life. gh rewrites the file
whenever the active account changes, and stores the active account's
token either in it or in the system keyring, depending on the account.
A lazygit that has been running for a while therefore consults a
snapshot that no longer describes reality: it either keeps using a
token for an account that is no longer active, or, when the snapshot
was taken while a keyring-backed account was active, finds no token at
all and silently stops showing pull requests until it is restarted.
Asking gh resolves the token afresh on every refresh, from whichever of
the environment, the keyring or the config file currently holds it.
go-gh's lookup stays behind as a fallback for setups without the gh
binary, where it still picks up GH_TOKEN and friends.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Quitting with confirmOnQuit set hung for three seconds and printed
"cannot kill child process", but only with a clean working tree. Closing
the confirmation pops the context before running its handler, so the
files panel is re-focused and re-renders the main view, and only then
does the handler return ErrQuit. With no changed files that render is a
string task, whose whole body is one hop to the UI thread — a hop that
is never served, because the handler's ErrQuit has meanwhile brought the
main loop down. The task can't finish, so the ViewBufferManager.Close
that follows waits for it until it times out. (With changed files it's a
command task instead, and every blocking point in one of those selects
on the stop channel, so Close gets through.)
A wait for the UI thread now ends when the loop does. That also covers
the command task's own hops, which are stopped only in between them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing dequeues user events once MainLoop has returned, so a worker
blocked in OnUIThreadAndWait is blocked for good. The assertion records
that; the next commit makes the wait give up instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Right now the function always returns nil, but this will change later in
this branch, so handle errors properly. Without that, the first capture
that assigns env.git would not run, leave env.git nil, and subsequent
code would crash.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every caller passes an f that unconditionally returns nil, so f's error
return has never carried anything: the value is dead weight, and it
occupies the one channel the wait itself needs to report that it couldn't
run f at all. Drop it, so that the error the wait returns can only ever
mean that.
Work that can fail hands its error back through a captured variable, the
way the background fetch already hands back four values, which keeps the
two outcomes distinguishable at a call site that has both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Git only writes the space after a marker when there is a label to write
after it, and the label can be empty: `git checkout -m` with the diff3
conflict style, for instance, has no name for the common ancestor, so it
writes a bare "|||||||" line.
Ask git for the attribute of every conflicted file whenever we load the
file status, so that we recognize the markers it actually wrote. Files
that are set up this way are precisely the ones whose regular content
tends to contain marker-looking lines, so matching a run of at least
seven characters instead is not an option: we'd take the file's own
content for markers and then never consider its conflicts resolved.
One `git check-attr` call covers all conflicted files at once; asking per
file would take seconds when hundreds of files are conflicted, and it
would hurt worst on Windows, where spawning a process is expensive.
Because the lookup rides along with the file status, it costs nothing
when there are no conflicts, and editing .gitattributes during a merge
takes effect on the next refresh.
When a file's conflict markers aren't seven characters long we don't
recognize them at all. Two things go wrong: we consider the file's
conflicts resolved, so we stage it and offer to continue the merge a
moment after stopping at it; and pressing enter on it shows its diff
instead of the merge conflicts view, leaving no way to resolve it in
lazygit.
Git doesn't always write conflict markers of seven characters: the
conflict-marker-size gitattribute overrides that per file, and it is set
for good reasons — for file types whose regular content tends to contain
marker-looking lines, such as documentation about merging, or test
scripts. We hard-code seven characters everywhere we look for markers,
so none of that works.
Prepare for honoring the attribute by threading the marker size through
everything that recognizes a marker, carried on the file model. Nothing
fills it in yet, so we still use git's default size of seven everywhere,
and matching is unchanged: a marker consists of exactly that many marker
characters, and all but the "=======" one are followed by a space and a
label.
Creating and applying a stash both touch every changed file, so in a
large repo they can take long enough to be noticeable — and running them
on the UI thread meant the confirmation popup stayed on screen, frozen,
for the whole operation. Run them on a worker instead, with a spinner,
and keep blocking input for their duration so that the type-ahead
guarantee the refresh used to provide still holds.
Dropping stays on the UI thread: it only rewrites the stash reflog, so
it's fast no matter how big the stashes are.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collapsing the range before kicking off the refresh paints the new
selection against the list as it was before the drop, so for a frame the
entries that were just dropped are still on screen (and, with
gui.shrinkSidePanelsToContent, the panel is still at its old size).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pushing the files context right after kicking off the refresh moves the
focus (and, with gui.shrinkSidePanelsToContent, resizes the panels) a
frame before the refreshed stash and files lists arrive. Doing it from
Then puts it in the same frame as the data it belongs to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stashing and popping change both the stash list and the files list.
With each scope updating the UI as soon as its own refresh is done, the
two panels visibly change at different times; with
gui.shrinkSidePanelsToContent that also means their sizes change at
different times than their contents.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running lazygit in a .git dir got you told you were in a bare repo,
which you weren't: the worktree was sitting right there, one directory
up. git's own convention is that a git dir called .git belongs to the
directory holding it — that's how `git worktree list` names the main
worktree — so ask that directory, and if it is a worktree, open the repo
we were really being asked about.
The git dirs that aren't called .git keep the answer they had. A linked
worktree's and a submodule's do have a worktree, but nothing we look at
says where, so we would be guessing; a bare repo's has none to find.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entering a submodule clears GIT_DIR and GIT_WORK_TREE, as it must: they
say where the superproject is. But the stack we push the superproject
onto so that escape brings us back only held its path, and for a repo
opened with --git-dir/--work-tree the path leads nowhere — git can't
find a repo there. Escaping out of a submodule of a dotfile repo failed
with "not a git repository", or, if some unrelated repo happened to lie
above the work tree, quietly switched to that one instead.
Push the environment onto the stack along with the path, taken from the
repo paths rather than from the process env, so that it also covers a
repo we worked the location out for ourselves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
git finds a repo by looking for a .git in the directory a command runs
in. Lazygit runs its commands in the work tree, so that normally works —
but not when the git dir lives somewhere else entirely, which is what
core.worktree and --work-tree are for. Lazygit chdir'd into such a work
tree and then ran commands that couldn't see any repo from there, so
opening a repo with core.worktree set panicked on startup. It only
worked with --git-dir because that leaves GIT_DIR in the environment for
every command to inherit.
Work out at startup whether git can find the repo from its work tree,
and when it can't, put GIT_DIR and GIT_WORK_TREE on every command the
repo's builder produces. As with the working directory the builder pins
(527124d0e0), these also go into the process env — subprocesses don't
come through the builder — but the commands don't read them from there,
because the process env belongs to whichever repo we have switched to
since.
Working out whether git can find the repo means asking git, rather than
reading the .git file, whose contents can spell the same directory
differently than git does. The extra query is skipped for a repo whose
git dir is simply its .git directory, which is nearly all of them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GIT_DIR and GIT_WORK_TREE tell git where our repo is, and every command
we run inherits them — including the ones we point at a submodule or
another worktree. git resolves those against our repo instead, and says
nothing about it: with GIT_DIR set, `git -C mysub log -1` reports the
superproject's commit. So opening lazygit with --git-dir/--work-tree
quietly broke resolving submodule conflicts, stashing and resetting a
submodule, and detaching another worktree; the worktree list came back
claiming every worktree shared our git dir.
Drop the two variables from the commands that address another repo.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reset told git to change directory with -C while runInParentModule does
it by setting the command's working directory, but they were computing
the same directory for the same reason. Use the helper, so that there is
one place that knows what running in a nested submodule's parent means.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Its working directory resolves against the process rather than against
the repo the command builder pins commands to, which is only safe
because nothing but foreground commands come through here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
git makes `rev-parse --show-toplevel` fatal when there's no work tree,
so asking for it together with everything else meant we never got an
answer at all for a bare repo: GetRepoPaths returned an error, nobody
ever saw IsBareRepo() == true, and lazygit either died with a stack
trace or decided we weren't in a repository. That's what you got for
opening it in a directory holding a bare repo and a .git file pointing
at it, which is a normal way to keep a repo and its worktrees together.
Ask again without --show-toplevel when the first query fails: the other
queries work fine without a work tree, so if they now succeed we know
we're in a bare repo, and the existing prompt offering to open a recent
repo does its job. If they fail too we're not in a repo at all, and the
first error already says so.
--is-bare-repository is gone from the query: a work tree implies
core.bare is false, so it could only ever come back false there, and
what matters to us is whether there is a work tree to show, which is
what we now go by.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"does not yet support" reads as a promise that it will, but it's quite
likely that it never will.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
onNewRepo also runs when switching repos, and a failure there leaves us
in the repo we came from — with a nil GitCommand, which nothing else is
prepared for. Only assign once we have one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RepoPath() is meant to be the same as WorktreePath() when we're in the
main worktree, but we derived it from the git dir's location instead.
That is only the same thing when the git dir lives inside the work tree.
With core.worktree, --work-tree, or a .git file pointing at a repo dir
that isn't called .git, it lands on a directory that isn't a worktree at
all, and the repo name we show follows it there.
A worktree that has the repo's common git dir to itself is the main
worktree, so use its path. That subsumes the submodule case, whose git
dir lives under the superproject's .git/modules but is still the
submodule's own common dir; --show-superproject-working-tree is now only
needed for a linked worktree of a submodule.
The existing bare repo test asserted a git output that can't occur (a
work tree and --is-bare-repository=true at once), but the rest of it is
the shape of a repo opened with --git-dir/--work-tree, where the new
repo path is the correct one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When the work tree lives somewhere else entirely — set up with
core.worktree or --work-tree — we're still in the main worktree, so
RepoPath() should be the work tree, as its own doc comment says. Instead
we derive it from the git dir's location, which lands somewhere that
isn't a worktree at all, and the repo name follows it.
The ACTUAL lines are indented as they will be once the EXPECTED ones
replace them, rather than as gofumpt wants them while the comment
markers are still splitting the struct's alignment. That leaves this one
file not gofumpt-clean until the next commit, in exchange for a diff
there that shows only the lines that actually change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`git worktree list` reports the main worktree as the common git dir with
a trailing "/.git" removed, which equals the working tree only when the
git dir sits inside it. In a submodule, a bare repo, or a repo using
core.worktree it doesn't, so comparing the reported path against the
working tree path matches nothing: no worktree is recognized as current
or as main. Most visibly, inside a submodule lazygit claimed we were in
a linked worktree named after the submodule, and offered to remove that
"worktree".
Comparing git dirs identifies a worktree unambiguously, so use that.
A worktree whose directory is gone has no git dir to compare, and there
we still have nothing better than its path.
The submodule tests were asserting the linked-worktree suffix in the
status view; it is gone now, and the repo name still says which
submodule we're in.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A submodule's git dir doesn't live inside its working tree, and `git
worktree list` reports it by its git dir. Lazygit compares that against
the working tree path, so it recognizes neither the current nor the main
worktree, and the UI ends up claiming we're in a linked worktree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The scenarios describe their repo by its paths but leave the git dirs
empty, which no repo has. Unused for now; the loader is about to want
them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PipeCommands ran every command in its own goroutine, each doing
Start/read-stderr/Wait, with nothing ordering one goroutine's Start
against another's Wait. That ordering matters: StdoutPipe registers the
parent's read end in cmd.parentIOPipes, and Cmd.Wait closes those
descriptors when it returns. The next command's Stdin is that very
*os.File, and exec passes a user-supplied *os.File through untouched, so
Start hands the child whatever the fd happens to be at that moment. If
the producer finished and got reaped before the consumer's goroutine
reached Start, that fd was already closed, File.Fd() returned -1, and
the child was started with fd 0 closed -- reading nothing at all.
The only caller is the pre-2.35 fallback in SaveStagedChanges, which
pipes `git stash show -p` into `git apply -R`. Losing that race left
git apply with an empty patch, so it failed with "unrecognized input",
the following `git stash drop` never ran, and the user was left with a
stray stash entry. This turned up as a flaky stash/stash_staged on the
git 2.32.0 CI job; the newer-git jobs take the `git stash push --staged`
path and never reach this code.
Starting every command up front removes the race, and collecting stderr
into buffers lets exec's own copying goroutines do the work. That also
fixes two lesser problems in the same function: finalErrors was appended
to from several goroutines without synchronization, and a failed Start
was only logged, so a pipeline that never ran reported success.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The commit list a filtering mode change leaves behind has nothing to do
with the one that was showing, so the scroll position it inherits says
nothing about where the selection ended up, and the selection can land
anywhere off screen. PostRefreshUpdate only moves the cursor within the
existing scroll position, so ask for the scroll separately, the way the
commits refresh does when it moves the selection itself.
Exiting filtering mode looked like it worked, but only by accident: the
commits refresh recognizes the commit that was selected before it ran,
selects it again at its new index, and scrolls because the index moved.
That does nothing for the case where the commit is gone from the list, or
for entering filtering mode, where we select the first commit ourselves.
Entering or leaving filtering mode switched the screen mode and the
focused panel immediately, then reloaded the commit list in the
background. The result was an unfiltered list presented in the layout
that says "you are filtering", with nothing to say that anything was
still happening — and in a big repo that state can last seconds.
Before we stopped blocking the UI thread on refreshes, the reload
happened before any of it, so the two always agreed; the price was a
frozen UI for the duration.
Do neither: reload on a worker, so the UI stays live, and hold back
everything the user can see of the change until the new lists are ready,
so they still land together in one frame. A waiting status says what is
going on in the meantime, and blocking input means the keys pressed while
it runs arrive after the change rather than acting on a list that is
about to be replaced.
Setting a filter and clearing it are the same transition in opposite
directions: mutate the mode, bring the screen mode in line with it,
reload the views that depend on the filter, and put the selection
somewhere sensible in the reloaded commit list. They were implemented
twice, once in the filtering menu and once in ModeHelper, which is how
the two came to repaint the commit list in different ways.
Derive the screen mode and the panel switch from whether a filter is
active after the change, so both directions fall out of the same code,
and give ModeHelper the entry points for both. The filtering menu is
left with nothing but the menu.
Blocking keyboard input and hiding the working tree state mode are two
separate concerns; they were fused into one helper because every caller
so far wanted both. A caller that blocks input for something other than a
rebase would then hide the "Rebasing" indicator for the duration of its
operation, which has nothing to do with it.
Make it an explicit option instead, so blocking input on its own doesn't
imply anything about the modes on display.
The mode it suppresses is active for any working tree state, not just a
rebase: merging, cherry-picking and reverting show through the same
indicator. Name it after what it hides.
Whether a graph can be drawn was read from the filtering mode, while the
graph itself is drawn over the commit list in the model. Those two only
agree once the list has been reloaded for the new mode, and a filtering
mode change reloads the list in the background, so in between we can be
asked to draw a graph over a list the graph makes no sense for.
That is not just cosmetic. Commits in a filtered list are almost never
each other's parents, so no pipe ever terminates: the pipe set grows by
one per row and every continuing pipe rescans it, which is cubic in the
length of the list. Escaping out of filtering mode with a filtered list
of 13000 commits — as you get once the 300 commit limit has been lifted,
which happens for good as soon as the selection passes COMMIT_THRESHOLD
— wedges the UI thread for around twenty minutes.
Record whether the list was loaded with a filter, right where the list
itself is stored, and decide from that. The graph now also stays up while
the pre-change list is still on display, rather than vanishing a moment
before the list it belongs to.
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.
The pty teardown in Close runs on a background goroutine that doesn't
get to finish when lazygit is quitting: the process exits milliseconds
after the view buffer managers are closed. The job objects still cover
the clients -- KILL_ON_JOB_CLOSE reaps them when the process's handles
are rundown at exit -- but nothing reaps the conhost, so on Windows
builds whose conhost fails to run down on its own, quitting leaks one
conhost per live pty.
This is not a rare timing window: a diff longer than what has been
read keeps its git process (and thus its pty and conhost) running for
the entire time it is displayed, so that scrolling can read more.
Quitting while looking at a long diff is therefore the common case,
and with an external differ configured it leaks a conhost on affected
builds on almost every quit.
Fix this by having the gui's shutdown path wait synchronously for the
in-flight teardowns after closing the view buffer managers. A quit
signal makes the teardowns skip the conhost rundown wait -- the
conhost serves nothing once its clients are dead, and the exit must
not stall for its sake -- so the wait normally completes in
milliseconds, keeping quit as fast as before; a 2-second cap protects
the exit path even if a teardown wedges.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A pseudoconsole's conhost.exe is spawned by CreatePseudoConsole as a
child of lazygit, so it is not part of the job object that the pty
teardown kills. That is normally fine: a healthy conhost runs itself
down once the reference handle is closed and its clients are gone. But
conhost builds before the ConPTY overhaul that shipped with Windows 11
24H2 (confirmed on 23H2, build 22631) fail to complete the rundown
when a client attached after the close event was delivered and was
then killed -- the fate of exactly the clients the job kill exists for
-- and such a conhost lingers forever with no clients, at a rate of
about one per five fast commit navigations. These builds remain
widespread: all of Windows 10 (whose ESU tail runs into 2028, and
whose hardware often cannot run Windows 11 at all) plus pre-24H2
Windows 11 fleets.
Since Windows offers no way to obtain the conhost's pid or handle from
the HPCON, identify it by diffing lazygit's direct conhost children
around the CreatePseudoConsole call, serialized by a mutex so that two
concurrently starting ptys can't confuse each other's diff, and open a
handle immediately so that pid reuse is harmless. The teardown then
gives conhost a second to exit on its own before terminating it; on
healthy builds the wait succeeds and the reap never fires. If the
conhost can't be identified unambiguously, we simply don't reap, which
is no worse than before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stopping a pty task on Windows relies on ClosePseudoConsole, which
delivers CTRL_CLOSE_EVENT to the console's attached clients. But only
to those attached at that moment: when the user flicks quickly through
commits, a task is often stopped within the first few milliseconds of
its life, before the child has attached to the pseudoconsole. Such a
child misses the event and survives, running the entire diff to
completion in the background (spawning one external differ per changed
file) and keeping its conhost.exe alive; rapid navigation accumulates
these git/difft/conhost trees, and they outlive lazygit. Grandchildren
are affected too: git for Windows runs commands through a two-level
git.exe wrapper, so a single task has several attach windows, and a
grandchild spawned while the console is going down is orphaned even
when its parent got the event and exited.
Fix this by putting the child into a job object before it runs its
first instruction (created suspended, assigned, then resumed), so that
every descendant is in the job from the start; the teardown in Close
terminates the job right after initiating the pseudoconsole close.
There is no point in a grace period between the two: the close event
is not a graceful signal -- git and the common diff tools leave it to
the default handler, which calls ExitProcess at an arbitrary point --
so clients that received it are already dying, and the kill exists for
those that missed it. Killing at an arbitrary point cannot leak a
stale index.lock, because pty-rendered commands no longer take that
lock (see withPtyGitConfig in pkg/gui/pty.go).
The pseudoconsole close runs on its own goroutine because the kill
must not wait for it: on builds where ClosePseudoConsole blocks until
the console host exits (pre-24H2), the host keeps running as long as a
surviving client does, and that client only goes away through the job
kill; sequencing the kill after a blocking close would deadlock in
exactly the case the kill exists for.
KILL_ON_JOB_CLOSE doubles as a safety net: if lazygit exits without
running the teardown, the OS closes the job handle and reaps the tree.
In a harness that mimicked the stop path with randomized 0-120ms stop
delays, 3 of 30 process trees survived as orphans before this change;
none survive with it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The task stop path terminates the still-running command by pulling its
*os.Process out of the Cmd interface and applying one global strategy
(TerminateProcessGracefully) to it. That shape can't accommodate the
upcoming fix for orphaned process trees on Windows: there, stopping a
pty task requires terminating the entire process tree via a job object
whose handle lives with the pty, not with the process. And the two Cmd
implementations genuinely need different strategies anyway: a
process-group kill (the likely future fix for #5675 on Unix) is only
safe for pty children, which run as session leaders, while plain
commands share lazygit's own process group.
So let each Cmd implementation decide how to terminate itself, and drop
GetProcess, which had no other callers. No change in behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
At the end of a diff against the worktree, git re-reads and refreshes
the index and writes it back if it found stale stat information
(diff.autoRefreshIndex, on by default). It holds index.lock for the
whole refresh; GIT_OPTIONAL_LOCKS does not cover this lock, and the
window scales with the size of the repository (~150ms for a 6k-file
repository with a warm stat cache).
On Windows, a pty task that is stopped because the user moved on
terminates its git process at an arbitrary point: tearing down the
pseudoconsole delivers CTRL_CLOSE_EVENT, which git leaves to the
default handler, which simply calls ExitProcess. If that lands inside
the refresh, a stale index.lock is left behind and the next git
command chokes on it. This is the same problem that 98801da106 fixed
by no longer killing git processes; the ConPTY support added in 0.63
reintroduced it through the close event.
Disable the automatic refresh for pty-rendered commands. They can
afford it: the refresh only persists refreshed stat information, and
lazygit's foreground git status refreshes -- which never run in a pty
and are never killed -- already write that back on every user action
and on terminal focus-in. The cost is that while the on-disk stat
cache is stale, an external differ is invoked even for files whose
stat information changed but whose content didn't, showing them as
empty diffs; this heals with the next foreground refresh, which also
re-renders the view.
Unix keeps the refresh: a stopped pty child gets SIGTERM there, and
git's signal handlers remove its lock files, so the lock window is
harmless. The rawGit renderer keeps it too: its tasks don't run in a
pty and are never killed on Windows -- they either run to completion
or die on a broken pipe mid-output, before the refresh begins.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
We already excluded the most commonly used commit trailers from being
auto-wrapped when typing or rewording a commit message, but this was
limited to two hard-coded ones ("Signed-off-by:" and "Co-authored-by:").
Extend this mechanism to use a heuristic to prevent more trailers from
wrapping; the heuristic kicks in for any "Key: Value" line if Key
contains a dash, or the value looks like a URL (so that it also catches
things like "Bug: https://my-bug-tracker/345").
To avoid mistaking a "Key: Value"-looking line in the message body for a
trailer, only apply the heuristic in the last paragraph of the message,
i.e. the block of lines at the end that is separated from the body by a
blank line. Each line there is judged on its own, so a line that isn't
recognized as a trailer still wraps without affecting the real trailers
next to it.
Having the cache in state.yml causes this file to be rewritten every
60s, which is annoying if you have a lazygit running in the background
somewhere without even realizing it, and it keeps overwriting the
foreground lazygit's newer command shell history and recent repos list
with its stale data. State.yml should only contain things that change in
response to user actions, not periodically.
In the branches list we show the checks icon (✓, ✗ etc) instead of the
gihub icon for branches that are open and have a state. It is a little
confusing, because the ✓ in front of the name means something very
different than the ✓ after it, but the checks status is just too useful
to see in the list.
In the main view we show it as a compact status before the PR title,
with a hyperlink that takes you directly to the checks tab in Github.
The branch controller should decide which pull request to show, not how its
header is styled and linked. Move the existing formatter and state badge next
to the branch presentation helpers so subsequent header changes stay in one
layer.
GitHub exposes a combined status for the head commit without requiring
individual check contexts. Include that rollup in the existing request
and startup cache so every consumer sees the same state without making a
second network request.
The fetch currently combines transport, JSON decoding, and model
conversion, which makes response changes difficult to verify without
exercising the network. Put the deterministic work behind a small parser
so later payload changes can be covered with raw GraphQL fixtures.
For a long time lazygit has used the term "custom pager" to refer to
what's really a "diff renderer". A pager is a program that allows you to
view output page by page (hence the name), e.g. less; lazygit's custom
diff renderers are not pagers. It used the term only because the feature
is implemented using git's GIT_PAGER env var, but that's an
implementation detail.
Rename the 'git.pagers' config to 'git.diffRenderers', and restructure
its elements while we're at it to make things clearer:
- Add a 'type' field to explicitly specify which type of diff renderer
it is (the two fundamentally different ones are 'stdinFilter' and
'extDiff').
- Add a third type, 'rawGit', which has an 'args' field that makes it
easy to use 'git --color-words' as a custom renderer
- Unify the old 'pager' and 'externalDiffCommand' fields to a single
'command' field for both types
Existing config files are migrated automatically.
