Some terminals provide a modifier key that bypasses mouse capture so
that text can be selected with the mouse and copied to the clipboard
(option in iTerm, shift in Ghostty), but others don't (e.g. the
terminals built into VS Code and Zed). In those, selecting text
requires setting gui.mouseEvents to false in the config file and back
again afterwards, which is inconvenient.
Add a global keybinding (% by default) that toggles mouse capture on
the fly. Like the whitespace toggle, it flips the in-memory config
value, so the toggled state lasts until the config file is changed and
reloaded.
The key needs to be free in every panel, and % is; as a plain rune it
also arrives reliably in every terminal on all platforms, and it can't
fire while typing in a text prompt, since editors consume plain runes.
An alt-based binding would have been more mnemonic, but doesn't reach
lazygit in the built-in terminals of VS Code and Zed on macOS (the
very terminals that need this feature) unless the option key is
configured to act as meta; and a ctrl-based binding would burn one of
the few remaining free ctrl keys, which are better saved for future
features that get used more frequently.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The model<->view index conversions were derived from arrays that only
renderLines populated. That made them depend on the list having been
rendered (so a conversion before the first render ignored the non-model
items), and it made them go stale whenever the model changed after a
render: converting an index then returned a wrong result, and once the
model had grown past the last rendered length the conversion indexed a
too-short array and panicked (seen in cherry_pick under -race).
The conversion is a pure function of the current list length and the
current non-model items, and needs none of the rendered display strings.
Compute it directly and drop the cached arrays, so the result is always
consistent with the current model and no longer depends on rendering.
The model<->view index conversions were derived from arrays that only
renderLines populated. That made them depend on the list having been
rendered (so a conversion before the first render ignored the non-model
items), and it made them go stale whenever the model changed after a
render: converting an index then returned a wrong result, and once the
model had grown past the last rendered length the conversion indexed a
too-short array and panicked (seen in cherry_pick under -race).
The conversion is a pure function of the current list length and the
current non-model items, and needs none of the rendered display strings.
Compute it directly and drop the cached arrays, so the result is always
consistent with the current model and no longer depends on rendering.
searchModelCommits converts every commit's index, and building the
non-model items can be O(len) mid-rebase, so it would now be quadratic;
snapshot the non-model items once via modelToViewIndexConverter instead
of rebuilding them per index.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ModelIndexToViewIndex and ViewIndexToModelIndex read conversion arrays
that only renderLines populates. So converting an index before the list
has been rendered ignores the non-model items (e.g. section headers) and
returns a wrong result; the same staleness makes a conversion after the
model has grown index a too-short array and panic (seen in cherry_pick
under -race).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Close read and called stopCurrentTask with no lock, while NewTask's
goroutine assigns it (and constructs the sync.Once it closes over) under
waitingMutex. On shutdown Close runs while a render task spawned by the
last layout is still starting, so the two raced on the field and the
once (three DATA RACE blocks under -race, e.g. cherry_pick).
Read stopCurrentTask once under waitingMutex and call the captured value
instead of re-reading the field, which establishes the happens-before
the once needs. This can't deadlock: no task holds waitingMutex across a
blocking UI-thread hop, so Close can always take it, and a task wedged in
such a hop is still bounded by the existing 3s timeout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some tests assert that a specific commit subject does or doesn't occur
in the main view; interactive_rebase/outside_rebase_range_select.go is
an example for this, it asserts `t.Views().Main().Content(
DoesNotContain("commit 06"))`. The problem with this kind of assertion
and our test commit naming scheme is that the diff view begins with a
"commit <hash>" line, and when that hash happens to start with "06" the
assertion matched it and failed spuriously. This was usually masked by
our MaxAttempts=2 that we currently use for integration tests (it's
quite unlikely that the commit gets a hash beginning with "06" twice in
a row). However, we want to get to a state where we can set MaxAttempts
to 1, so make this more robust by changing our naming scheme.
Some tests assert that a specific commit subject does or doesn't occur
in the main view; interactive_rebase/outside_rebase_range_select.go is
an example for this, it asserts `t.Views().Main().Content(
DoesNotContain("commit 06"))`. The problem with this kind of assertion
and our test commit naming scheme is that the diff view begins with a
"commit <hash>" line, and when that hash happens to start with "06" the
assertion matched it and failed spuriously. This was usually masked by
our MaxAttempts=2 that we currently use for integration tests (it's
quite unlikely that the commit gets a hash beginning with "06" twice in
a row). However, we want to get to a state where we can set MaxAttempts
to 1, so make this more robust by changing our naming scheme.
refreshViewLinesIfNeeded re-wrapped every line of the buffer whenever
the view was tainted. That's cheap for short content, but scrolling a
long diff calls it constantly: adjustDownwardScrollAmount queries
ViewLinesHeight on every scroll event, and each newly-read line taints
the view, so every notch re-wrapped the entire buffer. Wrapping measures
each cell's width (uniseg) and allocates per line, so once you'd scrolled
far enough down the diff, scrolling turned sluggish - the cost grew with
how much had been read. (A CPU profile of scrolling deep in a long diff
put 77% of the time in lineWrap, reached almost entirely via
ViewLinesHeight rather than draw.)
Cache each line's wrapped result on the lineType, keyed by the width it
was wrapped at, and only re-wrap lines that have actually changed since
the last refresh. A firstDirtyLine index, updated in the same three
places that set `tainted` (write, clearViewLines' callers, SetHighlight),
marks the lowest line that might have changed; lines below it with a
matching cached width reuse their cached wrapping. The cache lives on the
line, so it's freed with the line when the view's content is replaced
(e.g. selecting a different commit) - it doesn't accumulate across a
session.
The wrapping cost per scroll now scales with the number of lines just
read, not with the total size of the buffer, so scrolling stays smooth
no matter how far down you are.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reading more lines into a lazy-loaded view (e.g. a diff being scrolled)
never changes the window layout, and after the first screenful it
doesn't even change the visible content - the new lines land below the
viewport, so the only thing that changes on screen is the scrollbar
thumb. Yet each read triggered a full render: a layout pass plus a
redraw of every view. On a slow terminal that full-screen repaint on
every read is a big part of why scrolling through a not-yet-fully-read
diff stutters.
Route the task's refresh through a content-only render instead. It
skips the layout pass and only redraws the views whose content changed,
leaving tcell's cell-level dirty tracking to emit just the cells that
actually differ (in the steady state, the scrollbar column).
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>
LogAction and LogCommand are called from git worker goroutines (every
command a worker runs logs itself, and controllers log an action before
kicking off their worker), where they set the Extras view's Autoscroll
flag and append to GuiLog while the UI thread reads both when it lays
out and draws the view. Bounce the writes onto the UI thread instead.
This doesn't fix any user-visible issue that I know of; labelling it as
"maintenance" rather than "bug" for that reason. It is one of many steps
that gets us closer to running our test suite with `-race`.
LogAction and LogCommand are called from git worker goroutines (every
command a worker runs logs itself, and controllers log an action before
kicking off their worker), where they set the Extras view's Autoscroll
flag and append to GuiLog while the UI thread reads both when it lays out
and draws the view. Bounce the writes onto the UI thread instead.
Use the background variant so the bounce doesn't count towards lazygit
being busy: writing the command log is incidental display work, and a
foreground task would let an in-flight log write refuse a concurrent repo
switch (the same reason view-buffer renders and toasts are backgrounded).
Ordering between successive log calls is preserved by the bounce FIFO.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetStatusString and HasStatus read the statuses slice without holding
the mutex that addStatus and removeStatus take when they mutate it. The
readers run on the spinner-render worker (which polls GetStatusString
every frame) while removeStatus fires from the waiting-status and
toast-expiry goroutines, so the unguarded reads race the concurrent
writes. Take the mutex in the readers too.
This doesn't fix any user-visible issue that I know of; labelling it as
"maintenance" rather than "bug" for that reason. It is one of many steps
that gets us closer to running our test suite with `-race`.
GetStatusString and HasStatus read the statuses slice without holding
the mutex that addStatus and removeStatus take when they mutate it. The
readers run on the spinner-render worker (which polls GetStatusString
every frame) while removeStatus fires from the waiting-status and
toast-expiry goroutines, so the unguarded reads race the concurrent
writes. Take the mutex in the readers too.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refresh workers do their git work on background goroutines and then
mutate the model (`Model().Commits`, `.Branches`, …) and re-render views
directly from those goroutines, racing the UI thread's own cursor and
render code. This has been a long-standing source of flaky integration
tests, and it's what prevents us from running the e2e suite under the
race detector.
This PR removes that class of races by updating refresh state only on
the UI thread, and drops the mutexes that were standing in for that
discipline. It's an internal concurrency change with no intended
difference in normal use (the one small user-facing addition is noted at
the end).
- Each refresh scope does its git work on a worker, then enqueues its
model write onto the UI thread ("bouncing") through a single primitive,
so all model mutations are serialized on the one UI goroutine alongside
the cursor/render code they used to race.
- That primitive is generation-guarded: if you switch repos while a
refresh is in flight, the queued write is dropped instead of being
applied to the new repo.
- The inputs a refresh worker reads (model fields, selection, modes) are
now captured on the UI thread up front, so the worker computes from an
immutable snapshot. Worker-issued refreshes use a dedicated entry point,
and a debug-only assertion checks that the entry point matches the
calling goroutine.
- A few flags written from workers are made atomic rather than bounced.
- All six refresh mutexes are removed as redundant; the branches mutex
is replaced by a small branch-load sequence guard so the recency-sorted
result still wins at startup.
- Repo switching now runs on the UI thread rather than a worker,
removing a race on the shared gui state.
This is one step toward being able to run the test suite under `-race`
in CI — the remaining view-buffer rendering races are left for a
follow-up.
The one user-facing addition: switching repositories while a foreground
git operation is still running is now refused with a toast, instead of
running the operation's remaining commands against the newly-switched
repo.
The model-update bounces already drop themselves when the repo is
switched mid-refresh (onUIThreadUnlessRepoChanged), but three bounces
that touch the UI without writing the model did not: refreshView's
render, the staging-panel refresh, and the stale continue-rebase prompt
dismissal. All three ran unconditionally on the UI thread, so a
background refresh in flight across a repo switch could render the old
repo's data (through a context object belonging to the now-replaced
context tree), or pop the new repo's popup based on the old repo's
prompt state.
Route them through onUIThreadUnlessRepoChanged too, so they're dropped
alongside the model writes they accompany. This also fixes the dismiss
bounce using the raw foreground OnUIThread, which ignored the background
flag every other bounce in a background refresh respects.
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>
The commit's gpg onSuccess runs on a worker when the command output is
streamed, so its ClearPreservedCommitMessage wrote commit-message
context state off the UI thread. Bounce that write through OnUIThread.
GetFilePathSuggestionsFunc builds the trie on a worker (the slow
AllRepoFiles walk) and then assigned Model().FilesTrie and refreshed the
suggestions panel from there, racing the UI thread that reads the trie.
Keep the build on the worker but bounce just the model assignment and
the refresh through OnUIThread.
The discard handler cancelled the commit-files range selection from its
WithWaitingStatus worker. Bounce it through OnUIThread, keeping it after
the successful CheckMergeOrRebase as before.
The three branch-delete handlers and the two worktree-removal
continuations collapsed the Branches/RemoteBranches range selection from
their worker goroutine, racing the UI thread. Wrap each collapse in
OnUIThread, keeping it in the same spot relative to the refresh (FIFO
preserves the collapse-then-refresh order the name-restore depends on).
The pull-patch-into-new-commit handlers closed the commit-message panel
and, on success, pushed the local-commits context from inside the
WithWaitingStatus worker. Close the panel in OnConfirm before
dispatching (UI thread), and bounce the post-rebase context push through
OnUIThread, keeping it on the success path.
The three rebase-onto menu items read Modes().MarkedBaseCommit.GetHash()
(a bare string field) and, on success, cleared it via
ResetMarkedBaseCommit and pushed the commits context — all from the
WithWaitingStatus worker, racing the UI thread. Read the marked base
hash before dispatching, and bounce the post-rebase reset and context
push through OnUIThread, still guarded by the success check so they
don't run on the conflict path.
interactiveRebaseWithFlag and dropMergeCommit ran inside the
WithWaitingStatus worker but read Model().Commits and wrote the
selection (SetSelection(startIdx)) there, racing the UI thread. Thread
the commits slice in from each caller, and hoist the pre-rebase
selection into a UI-thread helper (selectRebaseResultCommit) called
before dispatching — squash/fixup unconditionally, drop only on the
non-merge path, matching the previous action guard.
ResetToRef ran on a worker and wrote the local-commits and reflog
selection directly (SetSelection(0) on both) before its refresh, racing
the UI thread. Fold those into the refresh's selection intents:
SelectHeadCommit for the commits (after a reset HEAD is the top commit,
and mid-interactive-rebase it correctly picks the real head over the
first todo entry) and SelectTopReflogCommit for the reflog. The
now-atomic SetLimitCommits stays where it is.
CheckoutRef and ResetToRef set this flag from their worker goroutine
(to load fewer commits for speed) while the commits refresh reads it on
the UI thread in captureCommitsState to decide how many to load — a data
race. Make it an atomic.Bool so those writes are safe where they are,
rather than routing the flag through a refresh intent. Precedent:
Branch.BehindBaseBranch.
discard reads Model().Commits and the selected commit index from its
WithWaitingStatus worker; read them in HandleConfirm instead.
toggleForPatch reads the commit-files ref name from the worker, and its
startPatchBuilder call reads the context's canRebase and diff range from
there too. Capture the ref name and run startPatchBuilder in
HandleConfirm before dispatching; PatchBuilder.Start only assigns
fields, so moving it off the worker changes no timing.
discard still collapses the range selection from the worker; that write
is a separate concern, left for a follow-up.
ResetSubmodule and fastForward each call a helper that reads the model
from inside their worker: FileForSubmodule reads Model().Files and
worktreeForBranch reads Model().Worktrees, racing the UI thread's model
writes. Hoist both lookups above the worker dispatch.
The two move helpers run inside the WithWaitingStatus worker that
withNewBranchNamePrompt dispatches to, but read Model().Files/Submodules
(to decide whether to auto-stash) and Model().Commits (the unpushed
commits to cherry-pick off the base branch) from there, racing the UI
thread's model writes. Compute mustStash — needed by both paths — at the
top, and the unpushed commits in the off-of-main menu item, on the UI
thread, and pass them into the helpers.
handleReword, amendTo, and the reset/set/add-co-author handlers pass
Model().Commits (and the selected line index) to a git rebase from
inside the WithWaitingStatus worker, racing the UI thread's model
writes. Read them on the UI thread before dispatching.
The author handlers index the full commit list by absolute start/end, so
the range sub-slice withItemsRange hands amendAttribute is not what they
need; capture the full Model().Commits there and thread it through.
These handlers dispatch their rebase to a worker via WithWaitingStatus
but read Model().Commits (and, for move-to-selected-commit, the selected
line index) from inside that worker, racing the UI thread's model
writes. Read them on the UI thread before dispatching and close over the
results.
getPatchCommitIndex stays as-is: moving its call out of the worker makes
its own Model().Commits read UI-thread-bound too, so the identical copy
in patch_building_controller.go needs no matching signature change.
The two pull-patch-into-new-commit handlers still push a context and
close the commit-message panel from the worker; those writes are a
separate concern, left for a follow-up.
With every scope's worker reads now captured on the UI thread and every
worker caller on RefreshFromWorker, the debug entry-point assertion no longer
needs to be scoped to the commits refresh. Move it to the top of
performRefresh so it guards every refresh regardless of which scopes it
touches, and drop the per-scope gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The remaining refresh scopes each still read model, context, and mode state
directly on their worker, racing the UI thread — the same class of race the
commits refresh had:
- files reads Model.Files (to detect resolved conflicts and drive the
auto-stage) and the Files context's ForceShowUntracked;
- reflog reads the existing reflog slices (for the incremental fetch),
Model.HashPool and the filtering path/author;
- branches reads Model.MainBranches and the previous branches (for the
BehindBaseBranch carry-over);
- stash reads the filtering path.
Gather each scope's inputs into an immutable snapshot on the UI thread (via
captureOnUIThread) before dispatching the git work, and have the refresh
compute from the snapshot — for branches, threaded through both the immediate
and the recency-sorted startup loads, which share one snapshot (the
BehindBaseBranch carry-over is identical either way). Status, tags and
worktrees read nothing UI-owned, so they're left alone.
For the snapshots to actually run on the UI thread, the worker callers that
reach these scopes must announce themselves: convert the submodule
operations, the submodule stash-and-reset, and the background files poller
to RefreshFromWorker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GuiRepoState.mergeOrRebaseStartedInLazygit and StartupStage are plain
fields, but they're written and read from worker goroutines: the former
from both the files refresh and the merge/rebase result path (which runs on
a worker for the async callers), the latter from the reflog/branches load as
it transitions the startup stage. Those are data races.
Make both atomic, like Branch.BehindBaseBranch. They're leaf flags, not
mutexes guarding model or view state, so an atomic is the natural fit and
keeps the merge/rebase result path out of this change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These four refreshes each read model, context, and mode state directly on
their worker — the same class of race the commits refresh had:
- remotes reads the selected remote (Contexts().Remotes.GetSelected), needed
to keep the remote-branches selection valid;
- sub-commits reads the SubCommits ref/limit/divergence, the filtering
path/author, and Model.MainBranches/HashPool;
- commit-files reads the diff endpoints (CommitFiles from/to and the diffing
args);
- rebase-commits reads Model.HashPool/Commits.
Give each the same treatment as commits: gather its inputs into an
immutable snapshot on the UI thread (via captureOnUIThread, inline for a
UI-thread refresh, hopped for a worker one) before dispatching the git work,
and have the refresh compute from the snapshot. The commit-files re-init
inside the commits refresh captures its endpoints in the bounce, right after
ReInit sets them, before dispatching to the worker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that every commits-reaching refresh issued from a worker goes through
RefreshFromWorker, guard the choice: in debug builds, panic if a refresh was
issued from the UI thread as RefreshFromWorker or from a worker as Refresh.
The caller's own goroutine is recorded at the top of performRefresh, before
a BLOCK_UI refresh dispatches onto the UI thread, so the check holds for
every mode rather than being fooled by BLOCK_UI. It's scoped to the commits
refresh for now, the only converted scope; once the rest are converted the
guard can move up to cover every refresh unconditionally.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CheckMergeOrRebaseWithRefreshOptions refreshes after a merge/rebase step,
and until now always via the UI-thread Refresh. Most of its callers are on a
worker (the WithWaitingStatus/WithInlineStatus merge, squash-merge, rebase,
pull, amend, drop, and patch-move handlers), so that refresh reads the
commits scope off the UI thread — the race the previous commit addresses for
everything else.
Split it: the default is for worker callers and refreshes via
RefreshFromWorker; a new CheckMergeOrRebaseWithRefreshOptionsFromUIThread is
for the handlers that run the step synchronously on the UI thread
(WithWaitingStatusSync, kept sync so rapid key presses batch): move up/down,
revert, squash-fixups, cherry-pick paste, and patch-discard.
The two share a private impl carrying which thread the caller is on, and the
auto-skip recursion (genericMergeCommandImpl for an empty commit) threads it
through so the follow-up step refreshes on the same thread. The
merge-and-commit refresh in SquashMergeCommitted, also on a worker, moves to
RefreshFromWorker to match.
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>
The next commits move refresh workers to read UI-thread-owned state (the
model, contexts, selection) on the UI thread rather than off it. Two
primitives support that:
- OnUIThreadAndWait runs a function on the main event loop and blocks the
caller until it has run, so a worker can read that state without racing.
OnUIThreadAndWaitBackground is the same for background routines, whose
work must not count towards the program being busy.
- IsUIThread reports whether the caller is on the main event loop, for a
debug-only assertion that a refresh was issued from the thread it claims.
It records the main loop's goroutine id in MainLoop and compares via
goid, so it's promoted from an indirect to a direct dependency.
goid is used only by that debug assertion, never to drive production
control flow.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The PR fetch needs the current branches (for their upstreams) and
remotes to know what to query. It read them from Model().Branches /
Model().Remotes on its own worker, after waiting on branchesAndRemotesWg
for the branches and remotes refreshes to finish.
That wait no longer guarantees fresh data: those refreshes now write the
model in a bounce onto the UI thread, and Done() fires before the bounce
has been processed. So the fetch read the pre-refresh lists — most
visibly, checking out a branch that has a PR wouldn't show that PR until
the next refresh, because the fetch queried the old branch set.
Have refreshBranches / refreshReflogAndBranches / refreshRemotes return
what they loaded, stash it in locals in Refresh, and hand it to the
fetch. The wait on branchesAndRemotesWg orders the fetch after both
loads have stored their slices, so it fetches against exactly the
branches and remotes that were just loaded, with no model read on the
worker. The previous commit guarantees both are always in scope when
pull requests are, so no fallback is needed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pull-request fetch queries GitHub for the tracking branches'
upstreams against the configured remotes. It therefore depends on the
branches and remotes being up to date; a refresh that asks for pull
requests but not for those (e.g. checking out a branch) would fetch
against a stale branch/remote list — for instance missing the PR of the
branch just checked out.
Expand the scope so pull requests always co-refresh branches and
remotes. This also sets up the next commit to hand the freshly-loaded
branches and remotes straight to the fetch, instead of reading them
back from the model (which, now that those writes are bounced onto the
UI thread, would be stale on the worker).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CommitFileTreeViewModel embedded the low-level tree's SetTree, which
rebuilds the node list without touching the cursor. So after a shrinking
rebuild (e.g. moving a patch out into the index removes a file), the
selection index could be left past the end of the tree. GetSelectedItems
then indexes out of range and returns a nil node, which segfaults callers
such as canEditFiles when the options map is rendered during layout.
Override SetTree to ClampSelection after the rebuild. Unlike
FileTreeViewModel we deliberately don't also re-find the selected node by
path: that walk lands on the containing directory when a file is removed
from a dir that then collapses, whereas keeping the clamped index lands
on the sibling file (see discard_old_file_changes).
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>
A toast keeps a foreground spinner task alive for its whole lifetime
(~2-4s): showing one calls renderAppStatus, whose OnWorker loop runs
until the status string clears. With the repo-switch guard in place that
made the guard's own "can't switch, operation in progress" toast keep
Busy() true, so the next escape/switch was refused until the toast
faded — you had to wait it out.
Render toasts in the background, like view-buffer content: a toast is a
transient notification, not lazygit driving an operation, so a switch
during one is fine. A real operation that shows a toast still keeps its
own foreground task busy independently.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Switching repos reassigns gui.git and the process cwd; doing it while a
foreground git operation (rebase/commit/push/…) is mid-flight would run
that operation's remaining commands against the wrong repo. The same
applies while the refresh an operation triggers is still settling: its
model writes are generation-guarded, but the client-side Then/OnUIThread
callbacks that run after it aren't, and shouldn't run against a repo that
changed underneath them.
Refuse the switch (with a toast) whenever gocui reports a busy foreground
task. DispatchSwitchTo carries the guard for the simple callers. The
callers that do work before the switch check up front instead, so a
refused switch doesn't leave that work half-done: worktree creation
checks before creating (its own waiting-status spinner would otherwise
make the query busy and refuse its own switch); submodule-enter and the
recent-repos menu check before mutating the repo-path stack (pushing /
clearing it); and escape-to-parent (SwitchToParentRepo) checks before
popping it, so a refusal doesn't consume the entry and strand the user
with nowhere to escape back to. All then call the unguarded switchTo,
which is safe because their own operation is complete by then.
Stefan HallerandClaude Fable 5
