The pane beside a commit's diff showed the patch being built from it by
assembling the text itself. That text could not be handed to a diff
renderer the way a diff can: a stdin filter might have coped, but a tool
that diffs two files could not. Its idea of how much context to show
around a hunk was also its own rather than git's.
Materialize the patch instead: write each of its files as it is before the
patch into one tree and as it is after into another, and let git diff the
two trees. The patch becomes a diff of real files, rendered by whatever
renders the rest of them, with git's own context around it. Its lines can
then be pointed at; taking them back out of the patch will need that.
The trees are named a and b, so that with git's own prefixes suppressed
the paths read like an ordinary diff's over the repo's own paths. They are
written when the patch changes rather than when it is shown, the patch
builder counting its own versions for that, and they go away with the
patch.
A renamed file is materialized under the name the patch expects to find it
under. Where the patch carries the rename, that is the name the file had
before, so the rename comes out as a rename. git names the trees
themselves in the two rename lines, having only the two paths to go by.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A patch built from what is on screen has to show what is in it, over the
diff those lines were taken from — which may be a diff renderer's
rendering of it, whose bytes are none of ours to touch.
So the marks are drawn in a gutter over the content, from the lines the
patch holds rather than from where they were drawn last. They are worked
out again whenever a pane's content settles and whenever the patch
itself changes. Working them out as the content settles keeps them right
across a renderer switch, a change of context size, or a walk through
the commits.
They are shown whenever the main view shows the diff the patch is built
from, whether or not it has the focus. The pane beside the diff previews
the patch all the while, and marks that came and went with the focus
would look like a bug while browsing. A diff of any other commit gets
none.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The five panels that show a commit's diff now offer on it what the patch
explorer offers on the one file it can show at a time: space takes the
selected lines into the custom patch being built, or back out of it again,
and d takes them out of the commit itself.
So a patch can be built straight from the diff already on screen, over
several files at once and without entering the commit's files first — and
from a stash entry or a reflog entry too, whose diffs the explorer could
reach but which had no patch preview of their own to show for it.
The two arrive together because they are one answer to what a commit's diff
offers, and because d needs exactly what space builds: a patch of the
selection, which the rebase then removes from the commit. Which of the two
space is doing is decided by the first selected line, as toggling a file in
the commit files panel is, and the selection then moves past the lines
dealt with — they are still in the diff, a patch leaving the commit alone,
so a second press would otherwise take them straight back out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Which line of a file a diff row is, in the terms a patch is built in, is
asked in two places already — staging a selection and editing a hunk — and
is about to be asked in a third, of a commit's diff rather than the working
tree's. The rule is the same one everywhere: an addition is where it sits in
the new version of the file, a deletion where it sat in the old one.
So the row is asked for its identity and the patch package for the lines
with those identities, instead of walking the diff working the identities
out again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Some merge conflicts can only be resolved by picking a side. The files
panel explains those rather than diffing them, and where one side deleted
the file and the other modified it, git's diff of that modification is
shown below the explanation. The focused main view took those change lines
for a diff of its own. It drew a selection over them and offered to stage
hunks of a file whose conflict staging can't resolve.
So have a render say whether it holds the diff the panel offers in the
main view, and put a selection only on one that does. Every diff render
already goes through NewMainViewDiffTask, so it says so for itself; the
custom patch preview, assembled as text rather than run as a command, says
so through NewMainViewDiffStringTask. Establishing a selection asks the
pane the same question rather than looking for change lines itself.
The selection has been wrong over this content since "Show a selection in
the focused main view" introduced it, and the fix belongs there. It lands
here instead because a render had no way to say what it holds until "Show
git's own diff when the renderer's can't be acted on" gave every diff
render one constructor to go through.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A diff renderer may lay a diff out however it likes: line numbers in a
gutter, the +/- column replaced by colour, the two sides in columns. Once
it has, we can only tell which line of which file a row shows if the
renderer says so. Under a renderer that doesn't say, the main view holds
a diff that can be read but not staged, edited or copied from. That is no
good now that the main view is where you stage.
So focusing it brings git's own diff instead, and every re-render while it
stays focused keeps to that, so staging a hunk doesn't flip back. Browsing
is untouched: you see what the renderer produced until you focus the view
to act on it.
Whether the renderer says anything is settled by asking it rather than by
watching it work: run it on empty input and see whether it announces the
protocol. Announcing is a property of the renderer, so the answer is known
before we render anything, and a diff with no lines to describe can't fool
it. Watching would have to see a diff go by first, and a binary file's diff
holds nothing that would tell the two cases apart. The answer is remembered
until the renderer changes. git itself is asked the same question, with the
renderer's own arguments, since it announces itself for exactly the formats
whose output can't be read back as a diff.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The remove key now works on a diff selection the way it does in the staging
view: on the unstaged side it throws the selected lines away, which it asks
about first, and on the staged side it takes them out of the index, which is
unstaging and needs no warning.
It goes through the same path as staging, so that everything around the
action behaves identically — the selection lands on the change that took the
place of the discarded one, and the focus follows the side it acted on.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each side of a file's diff has a pane of its own, but a pane is only shown
while its side has something in it: staging the last unstaged change takes
the upper pane away, and unstaging the last staged one takes the lower one
away. The focus follows into the pane that is left, and the selection has to
be waiting there when it arrives — on the lines just acted on, which is where
they are now, unless they were discarded rather than moved, in which case on
what is left of the file. The pane being moved to shows no selection until
the restore places one, so that the selection it was left with the last time
it was used doesn't appear for a frame.
Whether the acted-on side still holds anything is a question the model can't
answer: a refresh only queues its update, and by the time it lands the
re-render this has to ride is already under way. So the answer is worked out
from what we just did — the files we changed report whether the selection
covered all of their changes, and the ones it didn't touch are as the model
describes them.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Configured to always split the diff, a pane is shown whether or not its side
of the file holds anything, so emptying the side the focus is on no longer
takes the pane away — but it does take away everything there was to do there,
which is the question the focus is really asking.
So the render says which of the panes it is giving something to act on,
rather than the focus reading that off which panes are shown.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Space in the focused main view now acts on the selected lines the way it
does in the staging view: over the working tree's unstaged changes it puts
them into the index, and over the staged ones it takes them back out. Which
of the two it does is a property of the pane, each side of the diff having
one of its own. Nothing has to be entered first, and a selection reaching
across several files of a directory's diff is applied as one patch per file.
The rows on screen are only a picture of the diff, so the patch is built
from the diff itself: each selected row's identity — which file, which line
of it, and whether it is a deletion — is looked for in the file's own diff,
and the lines that match are the ones the patch includes. Matching by
position rather than by counting rows is what tells the two halves of a
modified line apart, since the deletion and the addition replacing it sit at
the same place in the new file.
Panels other than the working tree offer no action on their diff yet, so
space says nothing and does nothing there.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The main view shows a diff renderer's picture of a diff, and that is not
what you want on your clipboard: a renderer may drop the +/- column, move
the line numbers into a gutter, group a hunk's deletions before its
additions, or leave lines out altogether. So copying takes the lines from
the diff itself, locating them by the identity of the selected rows.
Only the panel that produced the diff can produce it again, though: the
working tree's staged or unstaged side, a commit's, a stash entry's. The
new seam is there for that. It asks per file, so that copying three lines
of a commit's diff doesn't fetch the whole of it, and it will grow the
actions on a selection as staging and patch building arrive.
The clipboard gets the run of diff lines from the first selected line of
a file to the last, so that lines the renderer hid come along and the
result still reads as a diff. Headers count as selected lines too. A hunk
header names the first line of its hunk, so it is looked for the way a
line of the file is. A file header names no line at all, and a rendering
may draw it over any number of rows, so a selection touching one of them
takes the whole header.
As in the patch explorers, a selection that is all additions or all
deletions loses its +/- column, ready to be pasted into code. One that
reaches into a header keeps the columns, and what comes out is a patch
fragment rather than lines of code.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The main section shows one pane or two, which was enough while the second
pane only ever accompanied the first. It is about to have to show the second
one by itself: the working tree's staged changes are moving there for good,
and a file with nothing but staged changes has only that side to show — it
should have the whole section rather than sit under an empty pane.
So which panes are shown becomes a three-way answer, derived from which of
them the render has content for.
Behaviour-preserving: nothing renders into the secondary pane alone yet.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The mode of a diff view's selection lives on the context of the pane showing
it, and until now only the controllers that drive the selection needed to
name that pane, all of which have it as the concrete context it is.
A side panel is about to be handed the pane a command was invoked in, so
that it can act on what is selected there — and it can't be handed a
context, the interface it is handed one through being declared where a
context's concrete type isn't available. So the state moves to where such an
interface can speak of it, and the panes gain one: a context that has a diff
selection.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The focused main view could be scrolled but not pointed at: there was no
way to say "this line" or "these lines", which is what every line-level
interaction needs — editing a line, copying part of a diff, and, later,
staging. So a diff main view now always carries a selection while focused,
starting at the first change already on screen so that focusing doesn't
move the view.
The mode of the selection lives on the main context (a single line, a
range from a fixed anchor, or the change block around the cursor); the
selected line and the range anchor stay in the view itself, whose native
range select draws them, so there is no new highlight machinery. The modes
and their keys are the ones the staging view has, including that the arrow
keys step from block to block in hunk mode.
A pane holds something to select only when what it is showing is a diff
with changes in it, which the render works out once its content is final: a
placeholder message is not a diff, and neither is a diff with nothing in it
at all, such as a binary file's or an empty commit's. Whether a selection
is then drawn there follows from the context stack, as it does for every
other view.
A click in the focused main view now has a meaning again: it selects the
line it points at.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
With both implementations gone, nothing is left that lets a side panel
handle a click in the focused main view, so the mechanism for attaching
one to a context can go as well.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The focused main view is about to show a selection, but only where there
are diff lines to act on: a branch's commit log or the status dashboard
has nothing to select. Rather than have the main view guess from the
rendered content, let the side panels say so, since each of them knows
what it renders.
The classification is finer than a yes/no because acting on a selection
means different things per panel — staging into the working tree for the
files panel, taking lines into a custom patch for the commit panels — and
those actions want the same one answer as this. Only whether a panel
shows a diff at all is read for now.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
To act on the line the user is pointing at in a diff view — to stage it,
to open it in an editor, to keep the cursor on it while the diff is
regenerated — we need to know which file it belongs to and where it sits
in that file. Only the diff the view was rendered from knows that, and
by the time it's on screen all we have is text.
So parse it back: the view's contents are (usually) a unified diff, and
running them through the patch parser recovers each row's file, kind and
line numbers. "Usually" is why this goes behind a seam, and why it
answers "I don't know" rather than guessing: a diff renderer is free to
restructure what it prints, and a wrong answer here means acting on the
wrong line of the wrong file.
Parsing a whole buffer is a separate entry point from parsing a single
line, because a caller resolving every row of a large diff must not
re-parse a file's section once per line of it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The task runs a command with its output shown through the configured
diff renderer. A pty is one way of getting the output to the renderer,
and is about to become one of two, so the task can no longer be named
after it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Each column of a menu is padded to the width of its widest entry, so a
single long name in the first two columns of the recent repos menu
pushes the path column off the right edge for every entry. Anyone whose
worktree directories are named after their branches hits this: with a
90 column menu and one 42 character name, the path column starts at
column 86 of 88.
Truncate both names to 30 characters, and put the ones that got
truncated into the item's tooltip, so that the full text is still on
screen for the selected entry. Filtering keeps matching the full names
and the full path, which the columns no longer show in their entirety.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A view drew a selection because something told it to, from four places on
three different schedules: a context being focused, a context losing focus,
a context being activated over another one, and a list being re-rendered.
Whether the flags ended up describing the state of the app depended on
which of those had run last, and the last one to run was often none of
them: a refresh only re-focuses the view that has the focus, so a list
whose contents changed underneath an unfocused panel kept whichever
highlight it happened to have.
Derive both flags instead, in one place, from the two things they mean: a
view shows a selection while its context is on the stack and has something
to select, and the context the user is in shows an active one where the
ones behind it show inactive ones. Nothing else needs to say anything about
highlighting, so nothing else can leave a view saying something untrue
about where the focus is.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Whether a view draws a selection is about to be derived in one place from
the context stack, which needs to ask any context — list or not — whether
there is something for a selection to sit on. Name the existing flag after
that question, and let a list context answer it from its length.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The only menu that ever asked for it was the keybindings menu, which now
filters as you type and doesn't use the prompt at all. That leaves every
filterable context with the same prompt, so the whole hook can go, and
with it the two implementations that only existed to satisfy it.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The filter input is where the keyboard points for as long as such a menu
is open, so that the first printable key can go straight into it. The menu
still gets every key the input doesn't take, because the input view is
embedded in the menu view, and the two are drawn as one focused panel.
Which keys the input takes changes once there is a filter: until then
printable keys still drive the menu, so that the configured navigation
keys work as usual, and afterwards they are all filter text. A menu item's
own keys are never bound in such a menu, because typing one has to reach
the filter rather than execute the item.
Escape gives up the filter and leaves the menu open; the next one closes
it. The filter prompt behind '/' is gone from these menus: the row already
does that job, and a second filter would only be confusing.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The row is reserved for as long as such a menu is open, even while it is
still hidden, so that it can appear without moving the menu. That costs
two rows of the popup, which is why the screen has to be a little taller
before a menu is worth showing at all.
The prompt in front of the input field is dropped when the row gets too
narrow to type in, and the keybindings menu says what '@' does when it
still fits.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing shows or positions them yet. The row is two views because the
input field has to start after the "Filter:" prompt, and a gocui view is
a rectangle: the frame view draws the row and the prompt, the field sits
inside it.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Originally I thought we'd benefit from this change in this branch; turns
out that we didn't after all, because we changed the approach, but it's
a nice cleanup anyway, so we include it here.
This fixes two problems:
1. the list selection updated before the list content did, which caused
a bit of a wobble effect in the list
2. the main view would first update to a different commit's diff and
then back to the one that is being moved, resulting in a very ugly
flicker especially when moving the todo multiple times with auto repeat
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>
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>
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.
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.
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.
A refresh from the UI thread returns immediately and applies its model
and view updates as queued UI-thread callbacks. A key pressed before
those have run is handled against the stale, pre-refresh state. For most
keys that's harmless, but some handlers turn that state into git
commands: pressing space twice in quick succession in the staging panel
builds the second patch from the already-applied diff and fails with
'patch does not apply', because the refresh after the first press is
what moves the selection to the next stageable hunk.
Notably, this is not just a regression of the recent change that made
UI-thread refreshes non-blocking; the window was merely much narrower
before. A blocking refresh parked the UI thread while the scopes'
bounces were queued, and the event loop drains pending keyboard input
with priority over queued user events, so a key pressed during the
blocked window still beat the queued state updates. The guarantee that
the next keypress sees post-refresh state had already ended when the
scopes' state updates moved from worker-side mutex-guarded writes to
UI-thread bounces.
Fix it with the input-blocking mechanism we already use for commit
surgery, exposed as a new RefreshBlockingInput entry point: it begins
blocking events synchronously in the calling handler, and ends the
block from a callback that the finishing step queues behind the
refresh's own updates. Keys pressed while the refresh is in flight are
buffered and replayed, in order, against the fully refreshed state;
since a replayed key's handler re-enters this same path, a burst of
keypresses applies sequentially, each one seeing the previous one's
refresh. Unlike the old blocking refreshes, this doesn't freeze the UI
thread: rendering, spinners, resizing, and mouse scrolling keep working
while input is withheld.
Blocking input is opt-in per call site rather than the default for all
UI-thread refreshes, because most refreshes (the focus-in and startup
refreshes, say) don't produce state that the next keypress depends on,
and blocking on them would delay typing for no reason. It should also be
limited to quick, narrow-scoped refreshes: a full refresh, or any scope
that pulls in COMMITS, can take very long in large repos and should
usually not hold up input.
The staging panel's stage/discard/edit-hunk refreshes use it now.
The refreshes on focus-in, right after a repo switch, and after
returning from a subprocess are full foreground refreshes, so their
tasks kept Busy() true for as long as the slowest scope took — and any
switch attempt in that window was refused with the "can't switch"
toast. The focus-in one is particularly annoying: focusing lazygit is
often precisely what the user does in order to switch repos, and right
after regaining focus is when a refresh takes longest.
Blocking the switch bought nothing there. The refusal exists for user
operations, whose follow-up work (e.g. a Then callback reading the
model) isn't covered by the switch-safety guards; but these refreshes
merely reload state, and a refresh by itself is now switch-safe: its
git commands run against the repo it was started for, and the
generation guard drops its updates when the repo changed.
We can't just mark them Background, because that flag also decides
whether the files refresh lets git take optional locks to persist its
refreshed stat cache — worth doing for an attended refresh, and the
focus-in refresh (typically running right after external changes) is
the case that profits most. So split the two meanings: a new
DontBlockRepoSwitch option dispatches the refresh's tasks as background
tasks (excluded from Busy()) while keeping the attended optional-locks
behavior. Combining it with Then panics, since Then is not
generation-guarded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The file-path suggestions trie is rebuilt asynchronously and then read by
the suggestions search, which runs on an AsyncHandler worker. It lived in
Model().FilesTrie, so that worker read the (UI-thread-only) model. Move
it to an atomic pointer on the SuggestionsHelper instead: it's the only
place that uses it, the helper is recreated per repo (so the cache still
resets on a repo switch), and an atomic pointer is safe to store from the
build and load from the search worker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PopupMutex guarded CurrentPopupOpts against a popup being created on a
worker goroutine while the UI thread deactivated it, or reset it on a
repo switch. Now that popup and menu creation is bounced onto the UI
thread, every access to CurrentPopupOpts — create, deactivate, and the
reset-on-switch (which already runs on the UI thread) — happens on the
one goroutine, so the mutex protects nothing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nothing calls it anymore now that the commit-surgery operations run on a
worker with input blocked. Remove the helper, its bespoke synchronous
spinner loop (renderAppStatusSync/setAppStatusContent), the popup-handler
plumbing, and the interface method.
That loop was also the only thing suppressing the yellow "Rebasing" mode
indicator (and its reset button) while lazygit drives a rebase itself.
Move that suppression to WithWaitingStatusBlockingInput so it applies to
every input-blocking commit-surgery op — including the ones that already
ran on a worker (edit, drop, and so on) and previously let the indicator
flash on mid-operation. It's cleared after the refresh, so an operation
that legitimately leaves a rebase in progress still shows the mode.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bracket gocui's BeginBlockingEvents/EndBlockingEvents around a
worker operation that shows a waiting status. The block is begun
synchronously on the UI thread, before the operation is dispatched to a
worker, so no keypress can slip through in between; it ends via
OnUIThread once the operation and its refresh have applied their UI
updates, so the replayed keys act on the refreshed state.
This composes what the retiring WithWaitingStatusSync did — show a
status and block input — but on a worker, so the UI keeps rendering
(spinner animates, model updates land) instead of freezing. Callers
follow in subsequent commits.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With sync vs async now derived from the calling thread, the Mode field
and its SYNC/ASYNC constants no longer carry any information: Refresh is
always async, RefreshFromWorker always sync. Drop the field, the type,
and the Mode argument at every call site, and reduce the debug log's
mode name to a plain sync/async derived from calledFromWorker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BLOCK_UI ran the whole refresh on the UI thread and parked it in a
wg.Wait for the duration, so the UI (and its spinner) froze while the
git work ran. Blocking the UI was never the point — the point was to
apply all the scopes' updates in one frame instead of a per-scope
cascade — and if we genuinely wanted to block input it should span the
whole operation, not just its refresh, which needs a gocui-level
mechanism we don't have.
So drop the mode and add a BatchUIUpdates option that achieves the
"one frame" effect without blocking: each scope's UI-thread bounce is
collected into a shared refreshBounceBatch during the refresh, and once
every scope has finished they're all applied inside a single OnUIThread
task. gocui drains every queued event before it redraws, so one task
means one repaint. The refresh itself now runs SYNC — on a worker when
issued from one (checkout, move-to-new-branch, the rebase-edit result
handling), so the UI thread stays live and the spinner keeps animating.
The batch needs a mutex because the scopes add concurrently from their
worker goroutines, and a closed flag so that any bounces enqueued after
the flush starts — the nested ones a flushed bounce produces in turn,
e.g. scrolling the selection into view — are dispatched immediately as
ordinary follow-ups rather than collected into a batch that nothing
will drain.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When scrolling a lazy-loaded view (a diff in the main view, the command
log, etc.), we top up the view's line buffer by reading more lines from
the still-running task. This was driven by asking the task to read a
fixed number of *additional* lines on every scroll event, which had two
problems:
- It was decoupled from the scroll position. Scrolling down, back up,
and down again re-read lines that had already been read, so the buffer
crept towards the end of the input regardless of where the user
actually scrolled.
- A single wheel notch only bought a single notch worth of runway, so
fast scrolling constantly outran the reader and had to wait for the
next read (and re-render) on every notch.
Make ReadLines take an absolute target total instead of a delta: the
task tracks how many lines it has read and only reads the shortfall, so
requests are idempotent. Callers now ask to fill the viewport at the
current scroll position plus a few screenfuls of read-ahead, which gives
scrolling enough runway to stay smooth.
The four call sites all wanted the same "fill this view" computation, so
consolidate them into a single ReadLinesToFillView helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every refresh scope needs two ambient values to bounce its model and
view updates back to the UI thread safely: the background flag (which
picks the dispatch variant that doesn't count towards lazygit being
busy) and the repo generation that guards the bounce against a repo
switch. These were threaded separately — background as a parameter on
every refreshXxx function, generation re-read from the model inside each
one. Bundle them into a single refreshEnv passed through instead, so the
guard has a home to grow into (the next commit needs the generation in
refreshView, which currently has no access to it).
Capturing the generation once, at the start of the refresh, is also more
correct than the previous per-function re-read. The baseline should
reflect the repo whose inputs the refresh snapshotted (all captured up
front on the UI thread), not whenever each scope's worker happens to
wake. With the per-function read, a background refresh whose worker woke
after a repo switch would read the new generation and let its bounce
through, writing data computed from the old repo's inputs into the new
repo; capturing up front makes that bounce drop instead.
No behavior change for foreground refreshes, where the UI thread is held
for the whole refresh and the generation can't move under it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A commits refresh does its git work on a worker and then reads the model,
the contexts, and the modes for that work directly from there:
LocalCommits.GetSelectionRangeAndMode/GetLimitCommits/GetShowWholeGitGraph,
Model.Commits/MainBranches/HashPool, the filtering path/author. Those are
owned by the UI thread, which is concurrently running the cursor and render
code, so the reads race it — the dominant, confirmed source of the
commits-scope flakes (the startup ClampSelection vs GetSelectionRangeAndMode
race, for one).
Gather them into an immutable capturedCommitState on the UI thread, before
the git work is dispatched, and have refreshCommitsWithLimit compute from
that snapshot. UI-thread callers capture inline; worker callers can't (a
SYNC/BLOCK_UI refresh parks the UI thread at wg.Wait, so hopping from a
scope sub-worker would deadlock), so the capture is lifted out of the scope
worker into the refresh orchestration, and worker callers announce
themselves with a new RefreshFromWorker entry point that hops the capture to
the UI thread and blocks for it (OnUIThreadAndWait). BLOCK_UI runs the whole
refresh on the UI thread regardless of the caller, so it captures inline
too.
Every refresh issued from a worker that reaches the commits (or branches,
which pulls in commits) scope is converted: the fast-forward, branch/tag
delete, worktree remove/detach, push, reword-via-rebase, author edits,
custom-command, hard-reset-with-autostash, reset-to-ref, fetch-and-checkout,
gpg-stream, post-fetch, and external-change-poller refreshes, plus the
branch checkout and move-commits-to-new-branch refreshes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Operations that check something out (checkout, create branch, move
commits to a new branch, fetch-and-checkout) selected the newly
checked-out branch by calling SelectFirstBranchAndFirstCommit() before
the refresh and passing KeepBranchSelectionIndex so the refresh wouldn't
override it. That set the selection directly, usually from a worker
goroutine (WithWaitingStatus/WithInlineStatus). Now that the refresh's
own selection write is bounced onto the UI thread, the two writes could
land in either order, and under load the refresh's "restore the
previously-selected branch" write would win — leaving the old branch
selected instead of the new one (flaky
move_commits_to_new_branch_from_base_branch).
Replace it with declarative selection intents applied inside the
refresh's own bounce, so the selection is set on the UI thread and
atomically with the list write (no off-thread write, and no BLOCK_UI
needed to avoid a flicker):
- BranchSelection: SelectCheckedOutBranch selects the checked-out branch
(top of the list). The default, KeepBranchSelectionByName, restores
the previously-selected branch by name as before. This replaces the
KeepBranchSelectionIndex bool.
- CommitSelection: SelectHeadCommit (already existed) for the commit.
- SelectTopReflogCommit selects the top reflog entry, since a checkout
adds a new entry there (reflog/checkout relies on this).
SelectFirstBranchAndFirstCommit is gone. The previously-selected branch
is now read at the top of the branches bounce, before the list is
overwritten, so that read moves onto the UI thread too.
fetchAndCheckout's refresh changes from ASYNC to SYNC so its
post-refresh focus switch can run in Then on the UI thread; SYNC keeps
the inline fetch spinner spinning (only BLOCK_UI would freeze it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Repo-switch safety needs to answer, synchronously on the UI thread,
"is any foreground work in flight right now?" so it can refuse a switch
that would run against a repo about to be swapped out. gocui already
tracks a task per OnWorker/Update for the test idle-listener; extend
that.
Tasks gain a background flag: background tasks (the ongoing routines
like auto-fetch, and the refreshes they trigger) don't count towards
busy, because their model writes are already guarded against a
concurrent switch by the repo generation. Add OnWorkerBackground,
UpdateBackground and UpdateContentOnlyBackground (plus the gui-layer
OnUIThreadBackground / OnUIThreadContentOnlyBackground / OnWorkerBackground
on IGuiCommon) so the few background call sites can opt in without
touching the hundreds of foreground callers.
TaskManager.hasBusyForegroundTaskExcept answers the query; Gui.Busy()
wraps it, excluding the event currently being processed (recorded as
currentTask) so a handler asking the question doesn't count itself.
Nothing gates on Busy() yet; this is the mechanism only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This removes the last refresh mutex. RefreshingBranchesMutex wasn't
guarding a data race (Branch.BehindBaseBranch is atomic, and every model
write is now bounced onto the UI thread); it was serializing the two
branch loads that race at the INITIAL startup stage — an immediate one
sorted without the reflog, and an async one that loads the reflog and
sorts by recency — so that the recency-sorted write landed last and won.
That serialization was never a real guarantee, only "very likely": it
relied on the immediate load acquiring the lock before the async load,
which had to load the reflog first.
Instead, each branch load takes a monotonically increasing sequence
number, and its bounce drops the write if a later-started load has
already applied. Combined with the preceding commit (immediate load runs
before the async one is spawned), this is an actual guarantee: the
immediate non-recency load always has a lower sequence than its recency
async partner, so the highest sequence number is always held by a
recency-sorted load, and highest-wins converges on recency ordering —
even if more refreshes fire during the INITIAL window, since each
refresh's async out-sequences its own immediate.
The guard also subsumes what the mutex gave post-startup: a slow, stale
refresh's bounce can no longer clobber a newer refresh's branches.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that every refresh scope writes its model updates on the UI thread
via onUIThreadUnlessRepoChanged, the per-scope mutexes that used to
serialize concurrent worker-goroutine access are redundant:
Model().Commits, .SubCommits, .Authors, the status view content, and
.PullRequests/.PullRequestsMap are all now written only on the UI
thread, and their readers already ran there. setSubCommits only existed
to take the lock, so it's inlined to match refreshSubCommitsWithLimit,
which writes Model().SubCommits directly.
The worker phases still *read* some of these fields (the commit
selection range, MergeRebasingCommits), but those reads race a
concurrent refresh's bounced write regardless of the mutex — the write
happens in the bounce, outside the locked region — so the mutex never
protected them. That residual read race belongs to the broader -race
effort, not to these locks.
RefreshingBranchesMutex is deliberately kept. It is load-bearing for a
reason unrelated to data races: at the INITIAL startup stage two
refreshBranches run concurrently — an immediate one with an empty
reflog (non-recency order) and an async one with the freshly-loaded
reflog (recency order). The mutex serializes them so the recency write's
bounce is enqueued last and wins. Without it the stale non-recency write
can land last, reordering the branches list (caught by the recency-sort
e2e tests: cherry_pick/*, branch/rebase_*).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
