For the busy query to be usable as a repo-switch guard it has to be
false while the ongoing background routines run, or a switch would be
refused every time a background fetch or files refresh happened to be in
flight. Mark that work as background so it's excluded from the query.
The background routine dispatch in goEvery becomes OnWorkerBackground,
and the auto-fetch waiting status renders its spinner through the
background variants. Within a refresh, the background flag (which
Refresh already carries as options.Background, and which the files path
already threaded) is now threaded through every place that enqueues a
task: the async scope workers, the model-write bounces
(onUIThreadUnlessRepoChanged), refreshView, the staging bounce, the
Then dispatch, and the branch-loader's behind-count worker. Two
single-caller chains reached by a background files refresh get the flag
too: MergeConflictsHelper.EscapeMerge and BranchesHelper.
AutoForwardBranches (whose follow-up refresh must stay background when
triggered by the background fetch).
Nothing gates on the busy query yet, so this is behavior-preserving;
background tasks still count as busy for the test idle-listener, which
looks at every task regardless of the background flag.
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>
DispatchSwitchTo wrapped its whole body in WithWaitingStatus, so the
switch ran on a worker: it chdirs, reassigns gui.git, and swaps gui.State
(in resetState), all of which the UI thread also reads. The generation
guard prevents the refresh-in-flight logical corruption but not this
pointer data race on gui.State.
Run the switch synchronously on the UI thread instead. Every caller is
already a UI-thread handler except NewWorktreeCheckout, which must create
the worktree (git work) on a worker first; it now dispatches only the
switch via OnUIThread. The heavy data loading still happens
asynchronously via the refresh that onNewRepo triggers, so the
synchronous part is small (a couple of git rev-parse plus direnv).
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>
At the INITIAL startup stage two branch refreshes happen: an immediate
one sorted by whatever reflog we have (empty, so not by recency), and an
async one that loads the reflog first and re-sorts by recency. Until now
the async one was spawned first and the immediate one ran afterwards;
this inverts that so the immediate refresh runs before the async one is
spawned.
With RefreshingBranchesMutex still in place this is behavior-preserving
(the mutex serializes the two either way). It's a preparatory step for
replacing that mutex with a branch-load sequence guard: running the
immediate refresh first establishes a happens-before relation between
the two loads' sequence numbers, so the recency-sorted one is guaranteed
the higher sequence.
This also lets refreshReflogCommitsConsideringStartup fold into
refreshReflogAndBranches, whose two-phase logic is now all in one place.
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>
refreshBranches now loads the branches (and worktrees) on the worker and
writes Model.Branches, the pull-requests map, Model.Worktrees, and the
restored branch selection in an onUIThreadUnlessRepoChanged bounce. The
selection restore and rebuildPullRequestsMap run in the bounce so they
see the branches we just wrote; the LocalCommits re-render (for branch
head visualization) moves into the same bounce.
refreshStatus is adjusted to read the checked-out branch and the linked
worktree name inside its bounce rather than on the worker: both derive
from models (Branches, Worktrees) that are now written via bounces, so
reading them on the worker would format the status from stale values —
which showed up as the status line dropping the "(worktree)" suffix right
after entering a submodule or switching worktrees. The git work
(WorkingTreeState) stays on the worker.
Two callers that read the branches model right after a SYNC branches
refresh move their reads into Then:
- BranchesHelper.PostFetchRefresh: AutoForwardBranches reads Model.Branches,
so it runs in Then (preserving that a fetch error is still returned to
the caller and that background auto-forward errors aren't surfaced as a
popup).
- BranchesController rename: the re-select-by-name loop runs in Then.
RefreshingBranchesMutex is left in place for the mutex cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshCommitsWithLimit now loads the commits, working-tree state and
bisect info on the worker and writes them all — Model.Commits,
Model.BisectInfo, Model.WorkingTreeStateAtLastCommitRefresh,
Model.CheckedOutBranch, the authors, and the restored commit selection —
in a single onUIThreadUnlessRepoChanged bounce. The selection restore
(SelectHeadCommit / KeepCommitSelectionByHash) has to run in the bounce
because it reads the freshly-loaded commits; the FocusLine scroll is
enqueued from within the bounce so it still runs after refreshView's
re-render, as before.
refForLog no longer writes Model.BisectInfo as a side effect; it returns
the bisect info it read, and the bounce writes it, keeping that model
write on the UI thread. No caller reads Model.BisectInfo synchronously
after a refresh (the bisect controller reads Git().Bisect.GetInfo()
directly), so this is safe.
refreshCommitsAndCommitFiles's post-refresh re-init of the commit files
context depends on that restored selection, so it reads the selection in
a bounce and dispatches the commit-files git work back to a worker.
LocalCommitsMutex / AuthorsMutex are left in place for the mutex cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshGithubPullRequests and setGithubPullRequests now do their network
work on the worker and write Model.PullRequests / PullRequestsMap in an
onUIThreadUnlessRepoChanged bounce (the "no github remotes" and "no base
remote" early-returns clear them the same way). rebuildPullRequestsMap
moves into the bounce so the map is built from Model.Branches and
Model.Remotes as they stand on the UI thread — after those scopes'
refreshes have applied their own bounces — rather than from whatever the
worker happened to see.
The remaining worker-side reads of Model.Branches (to pick which upstream
branches to query) are the same not-yet-addressed worker-read race that
applies to the other bounced scopes.
RefreshingPullRequestsMutex is left in place for the mutex cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshStatus computes the status line on the calling goroutine (as
before) but now writes it to the status view in an
onUIThreadUnlessRepoChanged bounce rather than calling SetViewContent
directly from the worker. RefreshingStatusMutex is left in place for now;
it only guards the compute phase between concurrent callers and comes out
in the mutex cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshReflogCommits now does the git fetch on the worker and computes
the new ReflogCommits / FilteredReflogCommits values (still reading the
existing slices for the incremental prepend), then writes them in an
onUIThreadUnlessRepoChanged bounce. The freshly-computed reflog is still
returned for the branches load, so recency sorting is unaffected by the
write now landing on the UI thread (see the previous commit).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BranchLoader.Load reads the reflog commits to sort branches by recency.
Today it reads them straight from Model.ReflogCommits, which works
because in the recency path the reflog refresh writes that field
synchronously just before the branches refresh reads it (same goroutine,
sequential).
An upcoming commit bounces the reflog model write onto the UI thread, at
which point Model.ReflogCommits wouldn't be updated yet when branches
runs — branches would sort by the previous refresh's reflog. To decouple
the branches load from *when* that write lands, pass the reflog commits
to refreshBranches explicitly: refreshReflogCommits now returns the
commits it loaded, and the recency path hands them straight to
refreshBranches. The non-recency path (branches and reflog run
concurrently, as before) keeps passing Model.ReflogCommits.
Pure refactor: behavior is identical, since the value passed is exactly
what Load read from the model before.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshRemotes now loads the remotes on the worker and writes
Model.Remotes, rebuilds the pull-requests map, and updates the selected
remote's RemoteBranches inside an onUIThreadUnlessRepoChanged bounce.
RemotesController.addAndCheckoutRemote read Model.Remotes right after its
SYNC REMOTES refresh to select the newly-added remote; since that write
now bounces, the selection (and the follow-up fetch) move into Then so
they run against the post-refresh model rather than the stale one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshSubCommitsWithLimit now loads the sub-commits on the worker and
writes Model.SubCommits (and folds their authors into Model.Authors via
RefreshAuthors) inside an onUIThreadUnlessRepoChanged bounce.
SubCommitsMutex and AuthorsMutex are left in place: the former is shared
with setSubCommits, the latter with the commits refresh's RefreshAuthors
call, so both come out only once those other writers are on the UI thread
too.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshRebaseCommits now computes the merged rebasing commits and working
tree state on the worker and writes Model.Commits /
WorkingTreeStateAtLastCommitRefresh in an onUIThreadUnlessRepoChanged
bounce. LocalCommitsMutex is left in place for now; it's shared with the
commits and branches refreshes and comes out once they're all bounced.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshWorktrees now writes Model.Worktrees in an
onUIThreadUnlessRepoChanged bounce. loadWorktrees becomes a pure loader
that returns the worktrees instead of writing them, since it's shared
with refreshBranches; refreshWorktrees bounces the result, and the
branches call site writes it directly for now (that write moves into
refreshBranches's own bounce when that scope is migrated).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshStashEntries now loads the stash entries on the worker and writes
Model.StashEntries in an onUIThreadUnlessRepoChanged bounce.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshTags now captures the repo generation, loads the tags on the
worker, and writes Model.Tags in an onUIThreadUnlessRepoChanged bounce
rather than directly from the worker goroutine.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshCommitFilesContext now enqueues the Model.CommitFiles write and
CommitFileTreeViewModel.SetTree() call via OnUIThread, instead of running
them directly on the worker goroutine that drives async refreshes. This is
what makes moving SwitchToDiffFilesController's post-refresh work into Then
(previous commit) actually necessary, rather than just future-proofing.
Same repo-switch hazard as the FILES bounce, closed the same way: it
captures the repo generation before the git work and bounces through
onUIThreadUnlessRepoChanged, so the write is dropped if the user switched
repos while it was in flight.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SwitchToDiffFilesController.enter calls SelectPath and Context.Push
right after a (SYNC, by default) COMMIT_FILES refresh. This works today
because the model write currently happens synchronously in the worker
before Refresh's wg.Wait() returns, but an upcoming commit will bounce
that write onto the UI thread instead, at which point wg.Wait() no
longer guarantees it's been applied, and SelectPath would operate on a
stale tree.
Move both calls into Then ahead of that change, for the same reason as
the earlier FILES-scope commit: Then is already queued via OnUIThread,
so this is behavior-preserving on its own.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FileTreeViewModel.RWMutex is removed along with the
withFileTreeViewModelMutex wrapper in FilesController that RLocked it:
every writer (the bounce closure, previous commit) and every reader (key
handlers, disabled-reason callbacks) now runs on the UI thread, so the
mutex is redundant.
RefreshingFilesMutex is removed entirely, including its last use in
repos_helper's DispatchSwitchTo. That use predates the bounce and was
never about FilesController's optimistic-rendering concern; it serialized
a repo switch's onNewRepo() against an in-flight FILES refresh for the
repo being switched away from, so that a slow refresh from the old repo
couldn't write into the freshly-reset model for the new one. Bouncing the
write already broke that guarantee on its own terms — the mutex's critical
section never covered the bounced closure's actual execution, only the
(now-removed) code that enqueued it — so by this point it was only still
locked here without protecting anything real; the previous commit's
repo-generation guard is what now actually closes that race, making this
lock fully redundant rather than just relocated.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshStateFiles now does its git work on the worker and enqueues a
single OnUIThread closure that writes Model.Submodules, Model.Files, and
the FileTreeViewModel state together, instead of writing them directly
from the worker goroutine. refreshStateSubmoduleConfigs becomes a pure
getter (returns the configs; no model write) so the result can be
threaded into that same bounce.
The STAGING handler wraps RefreshStagingPanel in OnUIThread after
fileWg.Wait() so it sees the post-bounce file model rather than the stale
pre-refresh one — without this it would race the files bounce queued just
above it.
Bouncing the write opens a hazard the old synchronous write didn't have:
if the user switches repos while this refresh is in flight, the queued
closure would fire after resetState has replaced the model with a fresh
one for the new repo, silently overwriting it with the previous repo's
files. Guard against this with a repo generation: resetState bumps a
counter on every switch, refreshStateFiles captures it before its git
work, and onUIThreadUnlessRepoChanged drops the bounce if the generation
has moved on. This one helper is the general mechanism the remaining
scopes' bounces will use too; the same guard covers the rebase-continue
prompt, which reads Model.Files right after.
A generation counter, not a comparison of the *Model pointer: switching
away from and back to a repo reuses that repo's cached state (the same
Model pointer), which a pointer comparison would wrongly accept even
though the in-flight data is stale.
PromptToContinueRebase's Then callback (previous commit) now gets an
explanatory comment, since this is the commit that makes it necessary.
The explicit locking around these writes (RefreshingFilesMutex in
refreshFilesAndSubmodules, FileTreeViewModel.RWMutex around the write in
refreshStateFiles) is left in place for now even though it's becoming
redundant, to keep this commit focused on the bounce itself; it's removed
next.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PromptToContinueRebase and WithEnsureCommittableFiles both read
Model.Files right after a SYNC FILES refresh. This works today because
the model write currently happens synchronously in the worker before
Refresh's wg.Wait() returns, but an upcoming commit will bounce that
write onto the UI thread instead, at which point wg.Wait() no longer
guarantees it's been applied.
Move both reads into Then ahead of that change. Then is already queued
via OnUIThread (previous commit), so this is a behavior-preserving
refactor on its own: the model is fully written by the time Then runs
either way, whether that write is still synchronous or gets bounced
later.
As part of restructuring WithEnsureCommittableFiles, prepareFilesForCommit
and syncRefresh are inlined into their single call sites.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This is preparation for upcoming commits that will bounce refresh-scope
model updates (e.g. Model.Files) onto the UI thread by enqueuing the
write via OnUIThread instead of applying it directly on the worker
goroutine. Once that lands, a Then callback that reads the model must
run after that queued write has been processed, not synchronously at
wg.Wait() time — at that point the workers have returned, but a bounce
they queued may not have been processed yet.
Queuing Then via OnUIThread here, ahead of that change, guarantees the
right ordering once it lands: a bounce queued earlier in the same
refresh is already sitting in the channel by the time wg.Wait()
returns, so Then enqueued after it will always be processed after, and
see the post-refresh model.
The signature change to func() error lets Then propagate errors
through gocui's normal error handler (the same path key-handler errors
take).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GetIsRefreshingFiles() is never called anywhere in the codebase, so the
flag serves no purpose. Remove it from Gui, StateAccessor, and
IStateAccessor, and drop the two SetIsRefreshingFiles calls in
refreshFilesAndSubmodules that maintained it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pushing a tag triggers no refresh, so it used to redraw the tags view
by hand to remove the "Pushing" inline status. WithInlineStatus now
always re-renders after clearing the operation, so this is redundant.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Operations that show an inline status ("Pushing", "Fast-forwarding",
"Fetching", …) removed it by relying on the async refresh they trigger
to redraw the view after the item operation had been cleared. That
ordering was never guaranteed: the item operation is cleared on the
worker once the operation's function returns, while the refresh redraws
the item from the UI thread whenever its (asynchronous) git work
happens to finish. If the refresh redrew before the clear, the status
was left on screen with no later redraw to remove it, so the branch (or
tag/remote) stayed stuck showing e.g. "Pushing" indefinitely even though
the operation had completed. This is timing-dependent, which is why it
surfaced as rare, hard-to-reproduce reports and as flaky CI failures.
Fix it by re-rendering in stop() right after clearing the operation,
and by making these refreshes synchronous rather than async. Because a
synchronous refresh has already updated the model and queued its own
redraw by the time stop() runs, and UI-thread callbacks run in order,
the redraw we queue here runs last and draws the up-to-date model with
the status removed. An async refresh couldn't give that guarantee: its
model update might not have landed yet, so the redraw could briefly
flash the pre-operation status.
Pull refreshes through the shared CheckMergeOrRebaseAndSelectHeadCommit,
so that helper becomes synchronous too; its only other caller,
RegularMerge, thereby also refreshes synchronously, which is fine: a
synchronous on-worker refresh is what we want anyway.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resetting to a commit/branch/tag from the reset menu ran inline on the UI
thread with no spinner; a hard reset to a distant commit can take a while
and blocks the UI meanwhile. Run it on a worker with a waiting status. The
undo/redo callers of ResetToRef already wrap it this way.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Setting a single commit to "edit" ran the interactive rebase inline on the
UI thread with no spinner, while its sibling startInteractiveRebaseWithEdit
(used when editing multiple commits or quick-starting a rebase) already runs
on a worker with a waiting status. Make the direct path match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The interactive-rebase item in the rebase-onto-ref menu ran inline on the UI
thread with no spinner, unlike its two siblings in the same menu (simple
rebase and rebase onto base branch), which already run on a worker with a
waiting status. Make it consistent.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The regular and squash merges from the merge menu ran inline on the UI
thread, freezing it with no spinner while git worked. Run them on a worker
with a waiting status, like the rebase entry points already do.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Continuing, skipping, or aborting a merge/rebase from the options menu ran
the git command inline on the UI thread, freezing the UI with no spinner
while it worked (a continue can replay many commits). Run the
non-subprocess path on a worker with a waiting status instead, matching how
the other merge/rebase entry points already behave.
The auto-skip recursion in CheckMergeOrRebaseWithRefreshOptions must not
start its own worker: it already runs on the caller's thread (the worker of
the enclosing waiting status, or the UI thread for the synchronous callers).
Route it through genericMergeCommandImpl with the waiting status suppressed
so its behavior is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When loading the files of a commit we passed --no-renames, so a rename
showed up as a separate delete and add rather than a single R entry.
That made it impossible to work with a rename that also modifies the
file: the modifications were spread across a full deletion and a full
addition instead of appearing as the handful of lines that actually
changed. The staging view already shows renames and lets you stage
their hunks, so there was no good reason for the patch builder to
differ; the flag was only there because the commit-file parser couldn't
cope with the rename record format.
Switch the commit-file loader and the per-file diff to --find-renames,
teach the parser about the rename record (a status followed by two
paths), and carry the previous path through the patch builder so the
diff for a rename is loaded with both paths, which is what makes git
emit the rename in the first place.
A whole-file selection keeps the rename in the header, so the rename
moves or is discarded together with the file's contents. A partial
selection instead strips the rename metadata and points the header at
the new path, so applying the patch only changes the contents and
leaves the rename in place; the blob index line is kept so that a 3-way
apply can still fall back to a blob merge.
Discarding a renamed file from a commit now discards both the new and
the old path, so the new file is removed and the old one is restored.
Changing the rename similarity threshold refreshes the commit files
panel too, not just the files panel, so that a rename can turn into a
delete and add or back. It is disabled while building a patch, however,
because the patch builder caches each file's diff by path and would
desync if a rename changed into a delete and add underneath it.
Finally, copying a file's diff from the commit files panel now passes
both paths for a rename, so the copied diff shows the rename instead of
a new-file add.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Accordion mode expands the focused side panel, but when that panel has
little content (an empty Files panel, a Branches panel with only master)
it just fills the extra height with blank space. The same waste happens
for any panel that gets more height than it has content to show.
When this option is enabled, each side panel is sized to its own content
(plus a blank line, so it's clear there's nothing more below) rather than
to an equal share of the height. The height a small panel gives up flows
to the panels that have more content than fits; those grow up to their
content and then scroll, weighted toward the focused panel in accordion
mode so the two features compose. Only when every panel fits with room to
spare is the leftover shared out equally, regardless of focus: enlarging
the focused panel there would reveal no more content and would only make
the panels jump around as the focus moves.
The option is independent of expandFocusedSidePanel and off by default.
The status panel, and the stash panel when unfocused, keep their fixed
one-line height as before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The height thresholds that decide between the proportional layout and
the squashed layout (and, within the squashed layout, between 3-row and
1-row unfocused panels) were hard-coded constants tuned for the fixed
set of five side panels. Now that the panels are configurable, a layout
with fewer panels has less to fit, yet was still forced into the
squashed layout at the same height as five panels would be.
Scale the thresholds down in proportion to the panel count so a smaller
layout keeps using the proportional layout at smaller heights. Only ever
scale down: raising the thresholds for more panels would make them
squash sooner, which works against the reason someone adds panels in the
first place (they want to see them).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pressing `d` on a worktree only ever removed the worktree, leaving its branch
behind even though deleting it too is often what you want. Turn the confirmation
into a menu: "Remove worktree", "Remove worktree and delete branch", and "Remove
worktree and delete local and remote branch". The branch-deleting items come
after the plain removal (they do more harm if picked by accident); both are
greyed out for a detached-HEAD worktree, and the local-and-remote one is also
greyed when the branch has no upstream. The plain menu pick is the confirmation,
so the standalone "remove worktree?" prompt is gone (and its now-dead
translation string with it); the dirty-worktree force prompt and the
unmerged-branch warning still appear when relevant.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Picking "Delete local and remote branch" for a single branch that's checked
out in another worktree used to fail with "Some of the selected branches are
checked out by other worktrees. Select them one by one to delete them." That
message only makes sense for a multi-selection; for a single branch there's no
reason we can't remove the worktree and delete both the local and remote branch
in one go. Route that case through the same worktree menu as the local-only
delete, with labels that spell out that the remote goes too. The multi-select
error stays for actual multi-selections.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When you delete a local branch that's checked out in another worktree, the
menu offered to remove or detach the worktree but then stopped there, leaving
the branch you asked to delete still around. Now both actions delete the branch
afterwards, and the labels say so ("Remove worktree and delete branch" /
"Detach worktree and delete branch") to avoid surprises.
Also drop the "Switch to worktree" item: switching abandons the delete the user
asked for, and it's already reachable by checking out the branch or via the
worktrees panel. And drop the now-redundant "remove worktree?" confirmation:
the explicit menu pick is the confirmation (the dirty-worktree force prompt and
the unmerged-branch warning still appear when relevant).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split the actual worktree removal out of the confirmation in Remove into a
non-confirming helper, and give both Remove and Detach an optional `then`
continuation that runs after a successful removal in place of the default
refresh. Upcoming flows need to delete the worktree's branch once the worktree
is out of the way; threading a continuation through (rather than the caller
firing branch deletion independently) keeps it ordered after the git command
that actually frees the branch. No behavior change yet.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pull the merged-check-and-force-warning step and the actual git deletion
out of ConfirmLocalDelete and ConfirmLocalAndRemoteDelete into helpers, so
that the upcoming worktree-aware delete flows can reuse them instead of
duplicating the logic. No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lazygit runs git directly rather than through a shell, so a literal "~"
reaches `git worktree add` unexpanded and git creates a directory named
"~" instead of using the home directory.
Expand the tilde ourselves, both for paths typed into the "Other"
location prompt and for the worktree.defaultPath config value.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The command was renamed from "View worktree options" to "New worktree",
but its keybinding config key was still 'worktrees.viewWorktreeOptions'.
That name no longer matches the command, and the 'worktrees' section made
little sense: it held a single binding that isn't even used in the
worktrees panel (that panel uses universal.new), only in the branches,
remotes, tags, commits, and stash panels. Other keybinding sections are
named after the panel they're local to; this one wasn't local to any.
Move it to universal.newWorktree, which describes the action and drops the
spurious section, and migrate existing configs automatically.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The old 'n' flow opened a "normal vs detached" menu (the same meaningless
gate the 'w' flow used to have), then asked for a base ref, a path typed from
scratch, and a branch name in three separate prompts.
Replace it with a single picker prompt titled "New worktree for branch",
suggesting local branches not already checked out anywhere, plus remote
branches that don't yet have a local branch of the same name. The entered
value is classified on confirm: an existing local branch checks out into a
new worktree, a remote branch creates a new local tracking branch, and
anything else creates a new branch off the current ref. All three then feed
the same location menu the 'w' flow uses, so paths are chosen from candidates
rather than typed blind. Picking a remote or new branch needs no separate
name prompt — the picker value already is the name. Checked-out branches are
filtered from the suggestions, and a verbatim type-in of one is rejected with
an error.
createWorktree now takes the context to switch focus to once the worktree is
created, so 'n' lands back in the worktrees panel while 'w' still lands in
the branches panel.
This deletes the old NewWorktree / NewWorktreeCheckout core and the now-
orphaned i18n (CreateWorktreeFrom, CreateWorktreeFromDetached, NewWorktreeBase,
NewBranchNameLeaveBlank), completing the migration started for 'w'.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The old flow forced an up-front "normal vs detached" menu (meaningless for
commits, tags and stashes), then asked the user to type a worktree path from
scratch — easy to get wrong, and ambiguous about what relative paths resolve
against. It also offered the same two actions everywhere regardless of what
was selected.
Replace it with per-context "Worktree" menus whose items imply the intent
(new branch + worktree, worktree for an existing branch, detached worktree),
each feeding a shared name -> location -> create pipeline. The location menu
offers candidate parent directories as absolute paths instead of a blank
field, and "Worktree for a branch" is disabled (with a reason) when that
branch is already checked out somewhere, rather than failing after the fact.
Each ref/commit panel binds 'w' in its own controller and calls the matching
typed entry point on the worktree helper, so which menu opens is decided
statically by the call site rather than by dispatching on a ref's dynamic
type. The three commit panels share one menu through BasicCommitsController;
there is no longer a shared worktree-options controller.
The worktrees-panel 'n' flow and its old core (NewWorktree /
NewWorktreeCheckout) are left untouched here so nothing is written and then
rewritten; they migrate, and the dead i18n strings get removed, in a
follow-up commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This is the core of "never type a path from scratch": from the repo root,
the configured default path, and the parents of existing worktrees, derive
the ordered list of directories under which a new worktree could be placed.
Pure and unit-tested here; wired into the creation flow in a later commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the StartPty stub with a real ConPTY implementation:
CreatePipe + CreatePseudoConsole + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE
+ CreateProcess. Pagers and external diff tools now get real terminal
behavior instead of being handed pipes.
One Windows-specific quirk worth flagging: ConPTY does not EOF the
output pipe when the child exits; conhost keeps it alive until
ClosePseudoConsole is called explicitly. A background waiter goroutine
calls ClosePseudoConsole as soon as proc.Wait returns, so callers see
EOF on outRead — restoring the Unix master-fd-EOFs-when-slave-closes
semantics they depend on.
The ErrPtyUnsupported sentinel and the no-pty fallback in newPtyTask
are gone now that both platforms have a real implementation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the pty master behind a small interface (Read/Write/Close/Resize),
and push the actual startup into a platform-specific StartPty function
in pkg/commands/oscommands. The Unix implementation still uses
creack/pty; the Windows implementation is a stub that returns
ErrPtyUnsupported, at which point newPtyTask falls back to a plain cmd
task — matching the existing Windows behavior.
The primitive lives in oscommands rather than pkg/gui because the
cmd_obj_runner pty handler (also in oscommands) is going to consume it
too, and tasks → oscommands is the existing dependency direction.
Same observable behavior on every platform; this just carves out a seam
for a real ConPTY implementation on Windows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Windows ConPTY can't attach a child process to a pseudoconsole via
os/exec — Go's stdlib doesn't expose PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE
(golang/go#62708). The ConPTY path has to call CreateProcess directly,
so it can't hand an *exec.Cmd back to the task runner.
Widen NewCmdTask to accept a small Cmd interface satisfied by both
*exec.Cmd (via the ExecCmd adapter) and the Windows ConPTY command type
we're about to add. Change TerminateProcessGracefully to take
*os.Process, which both cmd shapes can provide.
Behavior is unchanged on every platform.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The env var was previously set only on Windows, where the no-op pty
stub was just running the command without a pty and needed to expose
the width to pager scripts another way. With ConPTY coming to Windows
the rationale disappears there, but the env var is documented in
docs/Custom_Pagers.md for pager scripts that can't query the terminal
width directly. Set it on every platform so those scripts remain
portable, regardless of whether a pty is in play.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The prompt offering to continue a rebase/merge is opened from a refresh
and then left to sit until the user acts on it. But the operation can
change out from under it: a coding agent (or the user in another
terminal) might continue or abort it, or advance it to a commit with new
conflicts. The prompt then becomes stale — pressing continue fails with
"no rebase in progress" or acts on the wrong state.
Track whether the prompt is showing, and on each refresh dismiss it if
the operation is no longer in the "resolved, ready to continue" state
that the prompt is offering to act on. This runs on the same refreshes
that would open it (including the background poll and the refresh on
window focus), so the prompt disappears on its own shortly after the
operation moves on.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When conflicts of an in-progress rebase/merge/cherry-pick/revert are
resolved, lazygit pops up a prompt offering to continue it. This is
helpful when you started the operation in lazygit and resolved the
conflicts in your editor. But it's confusing when the operation was
started outside lazygit — e.g. by a coding agent in another terminal
that resolves the conflicts but hasn't continued yet because it's still
running tests or fixing the build. lazygit would then prompt unbidden.
Track whether the in-progress operation was started from within lazygit,
and only show the prompt in that case. We record this right after running
a merge/rebase step (in CheckMergeOrRebaseWithRefreshOptions, the
subprocess branch of genericMergeCommand, and the custom-command
conflict path), and clear it whenever a refresh observes that no
operation is in progress — which also handles an operation that was
finished or aborted externally.
The conflict-resolution tests start their operation by running git
directly (not through lazygit's UI), so they call the new test helper
Common.PretendMergeOrRebaseStartedInLazygit to have lazygit treat the
operation as its own and still get the prompt.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy the outgoing view's content into the target view (the flicker-
prevention step) before queuing the render task, rather than after. The
task writes the fresh content from a worker goroutine, so with the old
order the worker write races the UI-thread copy, and the copy can land
last and clobber the fresh content with stale output.
This is only needed while view writes happen concurrently. Once view
writes are serialized on the UI thread and the view write-mutex goes
away, the synchronous copy always precedes the FIFO-queued write
regardless of order, so the reorder becomes unnecessary. No code comment
is added for it, since that comment would be obsoleted by that work and
likely left behind.
Commit d94f2f05 dropped the GIT_OPTIONAL_LOCKS=0 env var that we used
to set on every git command, and re-added lock suppression only as a
--no-optional-locks flag on the background files refresh. The intent
was sound — a foreground `git status` should persist git's refreshed
stat-cache — but the change was too broad: it stopped suppressing
optional locks for every other command too.
The one that bites is the main-view diff. When a folder containing
submodules is selected, we render `git diff --submodule -- <dir>`, and
`--submodule` makes git run `git status` inside each submodule to
describe its "modified" state. That status now grabs the submodule's
index.lock. It runs as a PTY task on its own goroutine, so it races
any submodule-mutating action the user triggers — e.g. resetting a
submodule runs `git -C <submodule> stash`, which then fails with
"index.lock: File exists". This is what made submodule/reset_folder
flaky. `git status` is in fact the only command that takes the
optional lock, but the env var also covered its use inside `git diff
--submodule`, inside PTY-run commands, and inside git's own submodule
child processes — none of which a per-command flag reaches cleanly.
Invert the polarity to match how it worked before d94f2f05: the git
command builder disables optional locks on every command by default,
and the single command that benefits from taking the lock — the
foreground files refresh — opts back in. This restores the original
contention avoidance (including against the user's terminal git) while
keeping d94f2f05's stat-cache-persistence win for the foreground
refresh.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`b` on a merge conflict is meant to keep both sides. With the diff3
conflict style git additionally renders the common ancestor between the
two sides, and the old ALL selection kept everything between the
outermost markers, dragging that ancestor into the resolved file.
Rename the selection from ALL to BOTH and restrict it to the top and
bottom hunks so the common base is dropped. Without the diff3 style
there is no ancestor section, so the behaviour there is unchanged.
The user-facing keybinding config was already named pickBothHunks; only
the internal enum, handler, translation and log string still said "all".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetOnRenderToMain had grown to handle five distinct rendering cases inline (no
selection, submodule conflict, inline text conflict, non-textual text conflict,
and the normal working-tree diff), which made it hard to follow. Split each case
into its own method so the function reads as a short dispatcher, and pull the
repeated main-view boilerplate into renderToMainWithTask. Pure refactor; no
behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a conflicted submodule is selected, the main view shows the commits
each side added relative to their common ancestor as two indented logs,
labelled current and incoming, so it's clear which commit each side would
resolve to.
The logs aren't truncated (the view scrolls). If a side added no commits
of its own (e.g. it was rewound to an ancestor of the other), its head
commit is shown instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When both sides of a merge moved a submodule's gitlink, git reports it as "UU".
Pressing space used to fall into the submodule no-op guard and pop the confusing
"Nothing to stage..." error, and enter just entered the submodule, which does
nothing to resolve the superproject conflict.
Treat a conflicted submodule like the other non-textual conflicts: both space
and enter now open a picker offering the two candidate commits, "current" and
"incoming", each labelled with its summary. `git checkout --ours/--theirs` is a
no-op on gitlinks, so we resolve by checking the submodule out at the chosen
commit and staging it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
For a non-textual conflict (e.g. DD/AU/UA/UD/DU), pressing space used to run the
normal stage path, which did something unclear: `git add` happens to resolve the
conflict by keeping the file, but that's neither obvious nor symmetric. Route a
single such file to the same Keep/Delete picker that enter opens, so space and
enter agree.
For a range selection that includes one of these conflicts, staging makes no
sense, so disable it with a toast that points the user at resolving them one at
a time. (Entering a range was already disabled with the standard toast.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some merge conflicts can't be resolved by editing markers in the merge view;
they require a dialog that picks one side (the "non-textual" conflicts like
DD/AU/UA/UD/DU). Both `enter` and, soon, `space` need to recognize these, so
pull the test into a shared predicate and rename handleNonInlineConflict to
openConflictResolutionMenu to match.
Restructure EnterFile so the predicate is checked first, ahead of the submodule
and inline-conflict branches. This is its final shape: upcoming commits only add
the submodule case to the predicate, with no further reordering. Behavior is
unchanged here, since the predicate is currently false for submodules.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The status panel already binds the universal edit key to "edit config
file", but that's only reachable while the status panel is focused. Add
a dedicated global binding (alt+shift+c) so the config file can be
opened from anywhere.
The "edit config file" command is about to gain a second, global
keybinding alongside the existing status-panel one. Moving its body into
an action struct (the convention GlobalController already follows for
all its handlers) lets both controllers delegate to one implementation.
OpenFile (`o`) is for opening a file as if it was double-clicked in
Finder/Explorer; this is useful for binary files like PNGs, but never
for text files. You want to edit them, and there's `e` for that.
When the config file changes and lazygit regains focus it reloads the
config, but the side panel window assignments, default views, tab
strips, and z-order were only ever set up on repo entry, so a changed
sidePanels wouldn't take effect until restart. Re-apply it from the
reload path: reassign windows and default views and restore each panel's
default tab.
The focused panel needs care: resetting it to its default tab would
leave the focused tab hidden behind that default tab, so the panel looks
unfocused even though its tab is selected. Re-focus the current context
so its tab stays shown and highlighted; only when the new config hides
the focused panel entirely do we move focus to the default side panel.
Tab strips are already refreshed via configureViewProperties.
Side panel tabs share a window, so which tab is shown is decided by view
z-order rather than the visibility flag (every tab in a window is
'visible'). Tests had no way to assert which tab is actually drawn in
front, which is distinct from which view has keyboard focus. Expose the
window's top view and add an IsActiveTab assertion built on it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lazygit reloads changed config files when its terminal window regains
focus, but the test harness had no way to simulate that focus event, so
the live config-reload path was untestable. Add a focus event to the
replayed-events queue and expose it through the GuiDriver as FocusIn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The tab-assignment loop only ever set a view's tabs; it never cleared them.
That was fine when the groupings were fixed, but with gui.sidePanels a
config reload can turn a tab into a standalone panel, and the old tab strip
would linger on its title. Index the tab strips by view name and assign to
every view, so views that dropped out of a multi-tab panel get their tabs
cleared. No change for a given config; this only matters across a reload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Within a window the visible tab is whichever view sits on top in the
z-order, and onRepoViewReset establishes that z-order from a fixed list
that needn't agree with the configured tab order. After ordering the
views, bring each panel's first configured tab to the top so that, for a
panel whose tabs have been reordered, the configured first tab is the one
shown before the panel is focused. No effect on the default layout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In squashed mode (short terminals) the unfocused side panels each reserve
a row and the focused panel takes whatever is left, so once the unfocused
panels' rows fill the height the focused panel collapses to nothing and
panels below it render off-screen. The fixed floor of 9 was tuned for five
panels; with the panel count now configurable (and promotion allowing up
to ten), grow the floor by one per panel so we show the "not enough space"
view instead of a broken layout. Five panels still floor at 9.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These two views only ever appeared as tabs (of the files and commits
panels), so unlike the other side views they had no title set; the tab
strip supplied their label. Once a tab can be promoted to its own panel
they can appear without a tab strip, so set their titles like the others.
This has no effect in the default layout, where both are always tabs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the hard-coded side panel order, tab groupings, and window
assignments with values resolved from the gui.sidePanels config. The panel
order (SideWindows and the layout boxes), the tab strips (viewTabMap), the
per-context window names, each window's default view, and the jump-label
groups all now come from the config rather than from five separate
hard-coded lists.
A panel's window name is the name of its first tab, and panels not listed
in the config get their own window name so their views stay hidden instead
of overlapping a visible panel. Three small lookups translate config names
into views, tab titles, and contexts; a test keeps them in sync with the
set of valid names. The lookups are split this way (rather than one
resolver) because configureViewProperties runs before the context tree
exists, so the title/view lookups must not depend on it.
The config is applied to a repo's contexts via applySidePanelConfig on
every repo entry, including the cached-repo path: a repo's per-repo config
can differ from the previously visited one's, so each repo's contexts must
be (re)assigned from its own config rather than kept from when they were
first built.
With the default config this reproduces today's layout exactly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The number of side panels is about to become configurable, so a fixed
count of jump-to-panel keys no longer makes sense: a user who configures
six panels shouldn't be forced to also extend jumpToBlock, and one who
hides a panel shouldn't have to trim it. Drop the count check entirely
(individual keys are still validated) and assign keys to panels
positionally, for as many panels as there are keys. Surplus panels go
without a jump key but remain reachable via the next/previous-panel keys.
This also removes the log.Fatal that the count check guarded against.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The jump-label prefixes were assigned to each side view by name, twice
(once for the on case, once for off), so the panel-to-views grouping and
the panel order were baked into 28 positional statements. Express the
grouping once as a slice of view groups and loop over it, deriving each
panel's label from its index. The label lookup is now bounds-checked, so
it no longer assumes exactly as many jump bindings as panels. Behavior is
unchanged; this prepares the grouping to come from config.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The remote-branches context is a transient guest that takes over a host
window when you drill into a remote. SubCommits and CommitFiles already
adopt their parent's window via SetWindowName when shown; RemoteBranches
relied instead on its static window name ("branches") matching the remotes
context's window. That assumption only holds while remotes lives in the
branches panel. Adopt the parent's window like the other transient guests
so remote branches render in the right place once panels are configurable.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The three branches of sidePanelChildren each spelled out the five side
windows by name, so the panel order lived in three places and the
status/stash sizing special-cases were tangled into positional literals.
Map each branch over one `windows` slice instead, and fold the
normal-height special-cases (status's fixed height, stash's
collapse-unless-focused) into a single per-window function. Behavior is
unchanged; this isolates the ordering so it can later come from config.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Starting an interactive rebase (the `edit` command and quick-start) used
to capture the selected commit range by hash before starting the rebase
and restore it afterwards, because new update-ref lines for stacked
branches can shift the commits' positions in the list. The generic
keep-selection-by-hash mechanism now does exactly this for every refresh,
including these, so the bespoke code is redundant.
This relies on the previous commit, which taught the generic matcher to
handle the case where the selected commit turns into a rebase TODO entry
while it's being edited - something the bespoke code handled implicitly
by matching on hash alone.
When restoring the commit selection after a refresh we match by hash and
TODO status. The TODO status is part of the match so that a commit being
reverted or cherry-picked is matched to the real commit rather than to
the rebase TODO entry that shares its hash.
But a selected commit can also change its TODO status across a refresh:
when starting an interactive rebase that stops to edit it, the real
commit becomes a TODO entry. Fall back to matching by hash alone when
there is no exact match, so the selection is still restored in that case.
The next commit relies on this to remove bespoke selection-restoration
code in the local commits controller that matched by hash alone, which
the generic mechanism otherwise wouldn't fully replace.
With the recently added external change detection, it happens more often
now that we refresh the commits list because an agent made a commit in
the background. In this case, if we keep the selection index the same,
it now points at a different commit, making the main view show a
different commit too, which is confusing and annoying. To fix this,
track the selected commit and range anchor by hash before reloading,
then restore those rows if both hashes still exist. This also allows us
to get rid of some bespoke code that did this for the specific cases of
reverting a commit or cherry-picking commits, because those are now
handled by the generic mechanism.
Preparation for the next commit, which selects the newly created commit
after a commit succeeds, while leaving the selection alone on failure.
For now success and failure use the same refresh options, so behavior is
unchanged.
We set GIT_OPTIONAL_LOCKS=0 for every git command we run. That env var
only affects `git status`: it tells git not to take the optional lock it
would otherwise use to write the index back after refreshing the cached
stat information. The intent was to avoid contending for index.lock with
git commands the user runs in a terminal.
The downside is that our `git status` never persists the refreshed
stat-cache. So whenever the working tree's cached stat info goes stale
(e.g. editing files and discarding the changes, or a checkout), every
subsequent status re-hashes the affected files to confirm they're clean,
and stays slow until something else writes the index (such as the user
running `git status` in a terminal).
Fix this by only suppressing optional locks for refreshes that run
unattended in the background; foreground refreshes triggered by a user
action now run a plain `git status` that writes the refreshed index back,
just like the command line does. Background refreshes keep passing
--no-optional-locks so they still can't cause lock contention.
RefreshOptions gains a Background flag that the background routines set,
threaded down to the status command.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a 2-second background poll that calls Status.RefsSnapshot and
compares against the snapshot stored at the end of the last refs-
touching refresh. On a diff, trigger a full refresh — same scope as the
focus-in handler, because once we know something changed externally
we can't be sure what (an agent might have created a worktree or
stashed something alongside the commit we detected).
Refresh runs in SYNC mode because goEvery already serializes iterations
via <-done: a slow refresh delays the next tick naturally instead of
letting work stack. The post-refresh hook from the previous commit
updates the snapshot, so in-app commands don't cause the next poll to
spuriously re-fire.
Disabled in the integration test config, like autoRefresh and autoFetch,
because demo replays make repo changes throughout the run; at 2-second
cadence the resulting full refreshes compete with the demo's own
choreography and push some demos past their 40-second timeout.
Also list the two new config keys in checkForChangedConfigsThatDontAutoReload
so a config edit warns the user that lazygit needs a restart.
Add the storage and snapshot-update half of the external-change-detection
mechanism. RefreshHelper now keeps a mutex-protected snapshot string and exposes
accessors for it; Refresh captures a fresh snapshot at the start of any refresh
whose scope set includes COMMITS or BRANCHES.
We capture before reading the git state, not after. Capturing after would let an
external change that lands between the git state read and the snapshot (say, the
next step of a rebase running in another terminal) leave the stored snapshot
newer than what we actually rendered; the poller would then see no difference
and never refresh again, stranding the UI on the intermediate state. Capturing
first keeps the snapshot from running ahead of the render, so if disk moves
during the refresh the next poll catches it.
No reader of the snapshot exists yet — the polling goroutine that consumes it
comes in a later commit. Keeping the snapshot hook in its own commit isolates
the invariant that the snapshot stays in sync with what the UI has observed,
which is what makes the poller's change-detection predicate work across in-app
commands and focus-in refreshes.
Several downstream conditions in Refresh() relied on multi-scope predicates to
express "if X is in scope, Y also needs refreshing". This makes it hard to add
new code that needs to ask "does this refresh re-read refs?", because the answer
involves mirroring one of those predicates and keeping them in sync forever.
Expand the co-refreshing relationships once, up front, right after the scope set
is built. The downstream conditions then collapse to single-scope checks against
the (now-expanded) set. Behavior is preserved.
Two of the scattered multi-scope conditions are intentionally left as-is because
they express subsumption rather than co-refresh (one branch already does the
work of another internally — expanding would cause double-refresh), and one
expresses mid-function coupling on a flag set inside the COMMITS/BRANCHES block.
Several commands (rewording or amending an earlier commit, custom patch
operations, etc.) are implemented by starting an interactive rebase that stops
at a commit, amending it, and continuing. When no conflict occurs, the user
isn't meant to notice a rebase happened at all.
But a background file refresh can fire while the rebase is mid-flight and render
a dirty working copy of whatever the behind-the-scenes rebase is doing (e.g.
applying a custom patch).
To fix this, we pause the background routines for the duration of any
waiting-status operation — exactly the window in which lazygit is driving the
git operation itself and will refresh once at the end. The boundary is also
right for the conflict case: when a rebase stops on a conflict the operation
returns, the pause releases, and background refreshes resume for the interactive
resolution that follows.
Replace the pauseBackgroundRefreshes bool with a count. The single existing
caller (subprocess suspend/resume) is unaffected, but we're about to add a
second, independent reason to pause — lazygit driving a git operation that the
background routines would otherwise catch mid-flight — and the two scopes can
overlap. A bool can't represent "two things both want refreshes paused"; a count
can.
With more than a couple of pagers, having to cycle forward through all
of them to reach the previous one (or to back out of an accidental press
of `|`) is tedious. Add a second binding that cycles backward.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A reverse-cycle handler is about to need the same re-render-and-toast
logic. Pull it out first so the behavior change that follows only has to
swap the cycle direction.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When cycling pagers, "Selected pager 2 of 3" gives no clue which pager
you landed on; with several configured you have to remember the order.
Include the pager's name in the toast instead.
The name is normally derived from the first word of the pager command,
but that isn't always enough: two entries can share a command but differ
in options (e.g. "delta" and "delta --side-by-side"), and an entry may
have no command at all (the default entry, or when using
useExternalDiffGitConfig). So add an optional `name` field that
overrides the derived name.
The message was also hardcoded in English; localize it while we're here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A submodule that only has dirty or untracked content (no new commit) can't
be staged from the parent repo, but it still shows up as having unstaged
changes. Pressing stage on it therefore briefly flashed as staged and then
reverted, without explaining why nothing was staged.
Detect this case (via `git submodule status`, where a '+' prefix marks a
stageable commit change) in the shared stage/unstage decision: if the only
thing that looks stageable is such a submodule, don't try to stage it.
Instead unstage if there's anything staged to unstage, so the toggle stays
symmetric; otherwise show an error explaining that there's nothing to stage.
Because the decision is shared, this covers both the stage (space) and
stage-all (a) keybindings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This map only feeds the optimistic rendering that makes staging feel
instant; it doesn't affect the eventual status, which git reports after
the refresh. The "MM" entry can never be reached for a regular file: a
file at "MM" has stageable unstaged changes, so pressing space stages it
rather than unstaging, and the unstage path is where this map is used. The
only thing that reaches the unstage path at "MM" is a submodule whose
commit is staged on top of dirty content, so this entry exists purely to
update that submodule instantly instead of waiting for the next git
status.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stage/unstage toggle decides what to do based on whether a node has
unstaged changes: if it does, it stages; otherwise it unstages. For a
submodule this breaks down, because dirty or untracked content inside the
submodule always reports as an unstaged change in the parent repo but can
never be staged from there. Once such a submodule's commit pointer is
staged it sits at "MM", and every subsequent press keeps trying to stage
the unstageable dirty content, so it can never be unstaged.
Treat a submodule's unstaged change as stageable only when its commit
isn't already staged, so that a staged submodule unstages on the next
press regardless of leftover dirty content. Because the decision is now
shared by press and stage-all, this fixes both at once.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pressWithLock (acting on the selection) and toggleStagedAllWithLock (acting
on the whole tree) each independently decided whether to stage or unstage,
ran the optimistic update, and logged the action. That duplicated decision
has already drifted: the tracked-files filter was added to press months
before it was applied to stage-all, and fixes to one have repeatedly had to
be chased into the other.
Extract that shared decision into toggleStaged, leaving each caller to
supply only the git commands it runs (per-path for the selection, bulk
add -A / reset for the whole tree — the latter is required because the tree
root node has an empty path, so a per-path stage wouldn't work). This is a
pure refactor: the two callers' decisions were already equivalent, so
behavior is unchanged. It exists so the next change to the staging logic
only has to be made once.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a user switches into a repo whose .envrc hasn't been approved with
`direnv allow`, the previous behavior was to drop a "blocked" error
popup and leave the user to fix it externally. That meant opening a
terminal, running `direnv allow`, and then either restarting lazygit or
switching repos and back to refresh the env — easy to get wrong, easy
to forget.
When `direnv export json` exits non-zero, follow up with `direnv status
--json` to ask direnv whether the current directory has a not-yet-
allowed .envrc, and if so, get its path. Then show a confirmation popup
with the .envrc contents inline so the user can read what they're
approving. Confirming runs `direnv allow <path>` and re-runs the load
so the new env reaches subprocesses immediately; cancelling leaves the
env unloaded (the same state as before this commit when direnv refused
to load the .envrc).
Using `direnv status --json` instead of parsing the "is blocked"
stderr line means we rely on direnv's structured output rather than
its human-readable error format, which is more stable across versions
and avoids assumptions about output formatting.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a user opens a repo from the recent-repos menu or jumps between
worktrees inside lazygit, only the env vars present at process startup
reach subprocesses. That breaks pre-commit hooks and other tools whose
dependencies are pulled in by a per-repo .envrc — users were left with
read-only operations because the env their shell would normally load via
direnv never made it into lazygit's git invocations.
Shell out to `direnv export json` after each chdir and apply the JSON
delta via os.Setenv/Unsetenv. direnv tracks the previous load in its own
DIRENV_DIFF env var, so the delta also unloads vars from the old repo
when entering one without a matching .envrc. If direnv isn't on PATH the
call is a no-op, so users who don't use direnv pay nothing and users who
do need no config to opt in. Any stderr direnv emits (loading messages,
"blocked .envrc" errors, etc.) goes to the command log.
The integration test puts a fake direnv on PATH and asserts that a value
it exports reaches a custom command after switching repos. Wiring this
up needed runner.go to support `{{actualPath}}` placeholders in
ExtraEnvVars, mirroring the existing support for ExtraCmdArgs, so the
test can prepend a fixture-relative directory to PATH.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
If we return the error here, we don't switch repos, but the chdir
happened already, so this would be an inconsistent state (a lot of
lazygit's code assumes that the current directory is always the worktree
root). Only log the error; failing to record the current directory is
not the end of the world.
Also, it is very unlikely to happen; RecordCurrentDirectory only writes
to a small file, and if this fails, then either there is filesystem
corruption of the disk is full, and in both cases the user likely has
much bigger problems.
Commit 4f0393f97b caused a regression: for operations that use
WithWaitingStatusSync (examples are squashing fixups, moving commits up or down,
cherry-picking, creating fixup commits, and more), the waiting status wouldn't
show during the operation; however, it would show after the operation was done,
and then linger forever.
The cause: since 4f0393f97b, layout sizes the bottom line from the actual
content of the AppStatus view rather than from the status manager. The async
render path keeps the view in sync (it sets the buffer on the first tick and
clears it to "" when the status ends), but the sync path used by
WithWaitingStatusSync did not:
- It called ForceLayoutAndRedraw before writing anything to the view, so layout
saw an empty buffer and left no room; the status never appeared during the
operation.
- When the operation finished it just broke out of the loop, leaving the last
spinner frame in the buffer. Every subsequent layout kept reserving room for
that stale content, so the status stuck around forever.
Fix this by writing the status into the view before the initial layout, and
clearing it again when stopping.
This doesn't make a difference for the behavior, it just looks strange to
include the empty bindings first and then filter them out in the next statement.
If a keybinding that we want to display in the options bar was set to <disabled>
by the user, in pre-0.62 versions we would still display the command, but with
no keybinding. This was arguably not very useful before, but now it actually
crashes because we would now try to display the first key of the slice of
configured keys (crash introduced in 3d18ee8f91). Fix the crash by not showing
those commands at all.
The previous logic only re-rendered the main view when the side panel itself was
focused. When the user pressed `0` to focus the main view, the
Normal/NormalSecondary context becomes Current and the equality check failed, so
cycling pagers had no visible effect. Mirror the pattern from postRefreshUpdate:
when the main view is focused, call HandleRenderToMain on the side panel below
it on the stack (which CurrentSide already returns).
Convert the remaining *Alt/*Alt[12] sibling fields (PrevItem/NextItem,
GotoTop/GotoBottom, PrevBlock/NextBlock, ScrollUpMain/ScrollDownMain,
OptionMenu, ConfirmInEditor, DiffingMenu) so the merge mechanism folds
their values into the corresponding main multi-key binding at config
load. The redundant alt-only Binding registrations across the various
controllers and the global keybindings file are gone: the merged main
field already carries every key, so the for-loop in SetKeybinding
registers them all.
Previously the patch_explorer and merge_conflicts controllers reused
Universal.PrevBlock/NextBlock for moving between hunks (or conflicts) in the
main view, sharing keys with the global side-window cycle. The two operations
are conceptually distinct: cycling side windows is a global navigation gesture,
while next/prev hunk acts on the diff in the main view. Tying them together also
blocks adding <tab>/<backtab> as side- window-cycle keys, because <tab> already
means "toggle panel" in the staging view.
Add Main.PrevHunk/NextHunk to the existing KeybindingMainConfig (which already
groups bindings for the main view across staging, patch building, and merge
conflicts) and switch both controllers to it. The defaults match the active key
set those controllers had before (<left>/<right>/h/l), so the user-visible
behavior is unchanged.
Now that quit accepts multiple keys, the historical quit-alt1 field is
redundant: existing configs that set it should keep working without the user
having to migrate, but the lazygit code shouldn't have to register the alt
binding separately.
Add a merge step that runs after the user config is loaded (and from
NewDummyAppConfig, which the cheatsheet generator and integration tests go
through) folding the alt value into the main key list. Mark QuitAlt1 deprecated
so it disappears from the generated Config.md example, while staying in the JSON
schema with a description so editors can still steer users toward the new form.
Note that instead of marking the alt config as deprecated, we could have added a
migrator that changes users' config files and gets rid of the alt config for
good. I decided not to do that, because this would render the config file
invalid for older versions of lazygit, which would then refuse to start; and
that's annoying when bisecting bugs. We'll keep the deprecated configs in the
code for a year or so, and then add the migrator.
The next commit will fold the remaining ~15 -alt-style fields the same way; the
helper is shaped to keep that mechanical.
CustomCommand.Key and CustomCommandMenuOption.Key are user-configured
keybindings just like the built-in ones. Converting them to the Keybinding type
lets a user assign multiple keys to the same custom command, e.g. `key: [a, b]`,
the same way they would for any other keybinding.
The validator iterates over the elements rather than checking a single string,
the binding registration goes through GetValidatedKeyBindingKeys to register
every alternate, and the existing error messages use .String() so a multi-key
binding renders sensibly.
CustomCommandPrompt.Key (a form field name, not a keybinding) stays a plain
string.
JumpToBlock is special: each of its 5 elements is the binding for one side
window (status / files / branches / commits / stash), not an alternate for a
single command. Change the field from []string to []Keybinding so each window
slot can have alternates of its own.
The schema becomes "an array of 5 keybindings, each itself a string or array of
strings", which falls out cleanly from how the Keybinding type inlines into the
generated schema. Existing configs (a flat array of 5 strings) keep validating
because each element is unmarshalled through Keybinding's scalar-or-sequence
decoder.
Until now every keybinding config field was a plain string. That meant a user
couldn't ask for two keys to invoke a command — the config silently accepted
only one form.
Convert every string-typed field across all 13 KeybindingXxxConfig structs to
Keybinding so the union type extends to every command. Defaults wrap their
single-key value in Keybinding{...} so the generated Config.md still renders one
scalar key per binding.
The alt fields keep their separate Binding registrations for now: this commit
does not yet introduce the merge mechanism that folds them into the main field —
that comes in a follow-up. Consumers previously calling opts.GetKeys on a string
field now call opts.GetKeys on the Keybinding, or take .String() / Keys[0] where
a single value is needed.
Adds a Keybinding.String helper for rendering, schema-generator work that
inlines the Keybinding union into each consuming property, and a unit test
covering the user-facing scalar/sequence YAML forms for quit.
This is a pure refactor in preparation for letting users configure multiple
alternate bindings for a single command. Every Binding still has exactly one
key, so nothing changes visibly: the cheatsheet, the on-screen options bar,
and the keybindings menu all render identically.
When a Binding ends up with multiple keys, the on-screen options bar will
show only the first (to avoid clutter); the cheatsheet will show all of them (in
a later commit). For now both paths take Key[0].
MenuItem.Key is changed in the same way, it also has a slice of keys now.
In this commit we keep the name `Key` in Binding, KeybindingOpts and MenuItem,
instead of renaming them to `Keys` right away, in order to keep the diff a bit
more readable. We'll do the rename separately in the next commit.
Constructing a menu item key from a literal character requires
gocui.NewKeyRune('r'), which is a bit noisy. Add a private menuKey helper in
both the controllers and helpers packages so the common case in either reads as
menuKey('r'). Duplicating the one-liner is cheaper than a cross-package import
dependency and avoids forcing every controller file to qualify the call.
The reason for doing this now is that we are going to change MenuItem.Key to a
slice of keys later in the branch, which means we'd have to add `[]gocui.Key{`
at each call site, making them even more noisy. With the menuKey helper we can
just change its signature and leave all clients unchanged.
For legacy reasons, OptionMenu was set to `<disabled>`, and OptionMenuAlt1 to
`?`. This doesn't make a lot of sense any more; get rid of OptionMenuAlt1 and
bind OptionMenu to `?` by default. This is a breaking change for users who
rebound OptionMenuAlt1 in their config, but it doesn't strike me as very likely,
and it's easy enough to fix.
AutoForwardBranches relies on the worktree model to skip any branch
that's currently checked out in another worktree (so we don't update
its ref behind the worktree's back). The post-fetch refresh wasn't
including the worktrees scope, so any external change to the worktree
list between lazygit's startup and the fetch — a `git worktree add`,
a `git checkout` in a linked worktree, a branch rename — left the
in-memory model stale and the skip check returned false negatives.
Add WORKTREES to the post-fetch refresh scope when auto-forwarding is
enabled. We gate on the config so users with auto-forward disabled
don't pay for an extra `git worktree list` plus per-worktree rev-parse
on every fetch tick.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The post-fetch logic was duplicated in `backgroundFetch` and the manual
fetch handler: refresh a fixed set of views, then auto-forward branches
if the fetch succeeded. The two had already drifted on the refresh
scope; folding them into a single helper makes the duplication go
away and prevents it from drifting again.
Pass the fetch error through so we preserve the previous behaviour of
refreshing unconditionally but only auto-forwarding on success.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The background fetch path already includes PULL_REQUESTS in its
post-fetch refresh scope, but the manual fetch from the files view
doesn't. As far as I can tell that's an oversight from when
PULL_REQUESTS was added — there's no reason the two paths should
differ. Align them so both refresh PRs after fetching.
This also sets up the next commit to extract a shared helper for
the post-fetch refresh.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
If `git worktree list` fails, we want the Worktrees model to fall back
to an empty slice so callers iterating over it stay correct. The error
branch was setting it to `[]`, but the line below unconditionally
overwrote it with the nil `worktrees` value from the failed call.
Use an else branch so the empty-slice fallback actually sticks.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The branches-panel PR icons only worked for github.com remotes. There was no
fundamental reason — the auth library we already vendor (cli/go-gh) supports
enterprise tokens out of the box (GH_ENTERPRISE_TOKEN, gh auth's keyring),
and the user-facing 'services' config has long been the documented way to
tell lazygit "this domain is a github service" for the View-PR-URL feature.
The fetcher just hardcoded github.com in three places:
- a substring check on the remote URL to decide we're "in a github repo",
- the GraphQL endpoint (always api.github.com/graphql), and
- the auth lookup (always against the default host).
Plumb the resolved web domain through instead. Detection now goes through
the hosting_service ("is this remote's provider 'github'?"), which means a
user with services: { 'git.acme.com': 'github:git.acme.com' } configured
gets PR icons on their GHE remotes too.
Replacing the substring check with a provider check also tightens a latent
bug in getGithubRemotes: it previously accepted any remote whose URL parsed
with the default regex, including gitlab and bitbucket — masked today only
by the InGithubRepo gate, but exposed once the gate goes away.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After a successful paste DidPaste is true, hiding the "X commits copied"
indicator but leaving the buffer populated. From the user's perspective
this looks like a clean slate, so a new shift+C should start fresh. It is
important to reset DidPaste first, before populating the buffer with the new
commits, because otherwise each loop iteration would overwrite the previous one
since Add() rebuilds the set via SelectedHashSet() which returns empty while
DidPaste is set.
Now that HandleRender no longer does an implicit Render(), we can use it inside
OnUIThreadContentOnly to save performance, on the assumption that rerendering a
view that contains an inline spinner never changes the layout.
self.c.Render() at the end of HandleRender was there to schedule a gocui Update
tick so the view content modified above would actually get drawn. For UI-thread
callers (the great majority -- keybinding handlers, the layout function itself,
popup resize, etc.) this was unnecessary work, since gocui already runs a
layout/redraw cycle after every event. SimpleContext.HandleRender doesn't call
Render() either, so this aligns the two implementations.
The few callers that drove HandleRender from a worker goroutine and relied on
Render() for the flush were wrapped in OnUIThread in the preceding commits, so
the implicit Render is no longer needed.
Also, Render() being called *before* setFooter() looks like it might have been a
theoretical race; this is no longer an issue now.
The redraw of the selection color (using <space>) would be tied to the
spinner drawing. To reproduce, having a high spinner refresh rate and
toggling a file would see a delay equivalent to the time spinner refresh
rate.
SetSuggestions has two callers: prepareConfirmationPanel calls it directly on
the UI thread, while editors.promptEditor and
SuggestionsContext.RefreshSuggestions call it via AsyncHandler, which runs the
result closure on a worker goroutine. The worker path currently relies on
HandleRender's self.c.Render() to flush the view update. Wrap the body in
OnUIThread so the worker path stays correct when Render() is removed; for the
UI-thread caller the extra bounce is harmless.
refreshBranches runs on a worker goroutine and re-renders the commits view
directly to refresh the branch-head visualization. As with refreshView, this
currently only flushes because HandleRender ends with self.c.Render(). Wrap the
explicit HandleRender call (along with the LocalCommitsMutex pair around it) in
OnUIThread so it keeps working once Render() is removed.
An async refresh dispatches refreshXyz on a worker goroutine, which then calls
refreshView -> PostRefreshUpdate -> HandleRender. Today the final
self.c.Render() inside ListContextTrait.HandleRender is what triggers a UI flush
from the worker. We're going to remove that Render() call, so prepare by
wrapping refreshView's body in OnUIThread.
This moves the entire rendering of the view (and the ReApplyFilter/ReApplySearch
stuff) to the UI thread, not just the layout. I don't expect this to make a
difference in practice, and it is already one step towards my long-term goal of
moving all view rendering to the UI thread (see
https://github.com/jesseduffield/lazygit/issues/2974#issuecomment-1729154768).
Unrelated to this branch, just because we're touching this code: there's little
reason to set the color on the background thread but the text on the UI thread.
Set them both together on the UI thread. Avoids a data race (unlikely to be a
problem in practice, we're talking about a 64-bit int, but still).
In 0d195077e4 we improved the performance of the status bar spinner by
avoiding a layout. This is fine from one spinner tick to the next, but it's a
problem when spinning starts or ends (or in the hypothetical case that the
status text changes in the middle of the operation, which we never do in
lazygit, but theoretically could). In this case a layout is needed so that the
rest of the status bar gets pushed over appropriately (or moves back to the left
when the spinner ends), and also so that the bottom line is shown or hidden
properly for users who set gui.showBottomLine to false.
To fix this, keep track of the status string width and force a layout whenever
it changes. This includes the beginning and end of an operation when it changes
from empty to non-empty or vice versa.
There is currently no observable misbehavior from this bug, but that's only
because we must have a HandleRender call somewhere that forces a full layout
when an operation starts or ends. We will remove the Render() call from
HandleRender at the end of this branch, at which point the misbehavior would be
visible if we didn't fix it here.
With a very slow spinner rate (seconds), you can see that at the beginning of an
operation the bottom line leaves a gap for where the status will go, but the
status (and spinner) is only drawn the first time the spinner ticks. The reason
is that layout looks at GetStatusString to decide how much room to leave, rather
than at the actual content of the AppStatus view; the view content is only set
by renderAppStatus the first time the spinner ticks.
Fix this by making layout look at the actual content of the view so that the
layout is in sync with what is drawn. This also avoids flicker if an operation
is so fast that it finishes before the spinner ticks for the first time,
especially for users who set gui.showBottomLine to false.
This didn't cause a bug so far because switching repos always happens from
within an OnWaitingStatus, so the spinner would take care of calling layout and
draw. However, later in this branch we are going to optimize the spinner so that
it no longer calls layout, at which point this would break, so make sure we
rerender at the point where it's needed.
Modifiers were moved into Key in 22169e22f, but the separate Modifier field
on types.Binding and gocui.keybinding was left behind. The keypress matcher
already compares modifiers via Key.Equals, so the old field is never read on
the dispatch path; it just got passed through SetKeybinding and stored.
Drop it from gocui.keybinding, types.Binding, and the SetKeybinding /
DeleteKeybinding signatures, and remove every now-redundant Modifier:
gocui.ModNone struct field. Mouse bindings keep their own Modifier (on
ViewMouseBinding) since that path still consults it.
The commit graph used '⏣' (U+23E3 BENZENE RING WITH CIRCLE) for merge
commits and '◯' (U+25EF LARGE CIRCLE) for regular commits. Both have
very poor coverage in popular monospace fonts:
- '⏣' lives in the Misc Technical block and is essentially absent from
every common monospace font (Source Code Pro, JetBrains Mono, Fira
Code, Cascadia Code, Hack, Iosevka, Menlo, Consolas, Monaco, IBM
Plex Mono, Ubuntu Mono, Noto Sans Mono, Inconsolata). It is always
drawn from a system fallback font.
- '◯' is the late-addition LARGE CIRCLE codepoint. It is present in
some fonts (Cascadia, Fira Code, Hack, Iosevka, Menlo, Noto Sans
Mono) but missing from many others, including Source Code Pro and
most Nerd Font derivatives based on it.
This is why the graph renders inconsistently across platforms even
when the same monospace font is configured: each OS picks a different
fallback font (Apple Symbols on macOS, Segoe UI Symbol on Windows,
Noto/DejaVu/Symbola on Linux), and the substituted glyphs differ in
shape, weight, and advance width. '◯' is also East Asian Ambiguous
width, so some terminals render it wider than one cell, exaggerating
the misalignment.
Replace the symbols with codepoints from the foundational 1991
Geometric Shapes block, which has far broader font coverage:
- Merge: '◎' U+25CE BULLSEYE -- concentric circles, the visually
closest cousin to the previous benzene-ring glyph.
- Commit: '○' U+25CB WHITE CIRCLE -- the same hollow-circle silhouette
as before, just a more universally available codepoint.
The new symbols are present in the font directly in significantly
more cases; and when fallback is still required, they are universally
well-drawn (unlike '⏣', which many fallback fonts also lack).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The call was meant to guarantee a UTC time zone to make the tests deterministic,
but as the previous commit explained, this only worked on Mac and Linux, not on
Windows. Since we now have a better fix, this workaround is no longer needed.
This changes not only how we store modifiers (inside of Key instead of passing
it separately), but also how we parse keybinding strings: it supports all
combinations of modifiers now (if the terminal supports it, that is).
This bundles the keyName and a rune, so that we don't have to pass these around
separately everywhere. This should make it easier to swap out the rune for a
string when we upgrade to tcell v3.
I copied all files except dot files (.github and .gitignore), the _examples
folder, and go.mod/go.sum.
At some point we may want to copy the files back to the gocui repo when other
clients (e.g. lazydocker) want to use the newer versions of them.
This is so that they look the same no matter what color palette the terminal is
using. (One user complained that the text for the Open state is barely readable,
because they are using a palette that has a very pale green.)
GitHub uses slightly different colors depending on light vs. dark mode;
fortunately they are very close, so hopefully we can ignore this. I picked the
ones for dark mode here, on the assumption that this is more common.
Also, not all terminals support true hex colors; for example, Terminal.app on
macOS doesn't, so it maps the colors to the closest ones in the Xterm-256
palette. This shouldn't be a huge problem, but for some reason it displays draft
PRs as something closer to Cyan than grey, and I don't understand why.
If the repo has multiple remotes, but only one of them is on Github (the others
might for example point to a self-hosted Critic server or something like that),
lazygit would still present a menu to choose the remote for pull requests, but
it would contain only that single entry. That's pointless, pick it automatically
without prompting.
We add some tests while we're at it; these wouldn't have caught the problem,
because they only test getGithubBaseRemote which already takes the filtered
github remotes. It's still better than not having any tests; the real issue
could only have been caught with an integration test, which we don't bother
adding.
If we used to have a base remote (e.g. because there was only one), but now we
don't (e.g. because the user has added a second one, and we don't know yet which
one is the base), stop showing the cached PRs. This is especially important in
the case that the user cancels the prompt, in which case we don't have a base
remote for the rest of the session, but would keep showing the (potentially
stale) cached PRs forever.
This is useful when cancelling out of the commit panel mid-sentence (after
having typed the space for the next word); when entering the commit message
panel again, the space was gone and you had to type it again. Small thing, but
it just seems better to resume the panel in exactly the state that you left it
in. (Which we actually don't do; we don't remember the cursor position, or which
of the subject/description panels was active. That would be a separate
improvement.)
The save path and the load path used to be asymmetric. On save, the textarea
getters applied strings.TrimSpace, which stripped any leading blank lines, a
trailing newline after the cursor, or indentation on the very first line of the
description — all of which are legitimate user content. On load,
SplitCommitMessageAndDescription did TrimSpace on the description as well, and
the preserved message was routed through that same git-format split because
HandleCommitPress passed it as OpenCommitMessagePanel's InitialMessage. The
result: every round-trip through "escape and reopen" silently mutated the
message.
The fix is to treat our own preservation file as its own format, distinct from
git's canonical "summary\n\nbody" format:
- The textarea getters return raw content. strings.TrimSpace moves to the one
place that still needs it: the empty-summary check in HandleCommitConfirm (git
itself strips trailing whitespace and blank lines, so no pre-trim is needed
before -m).
- SplitPreservedCommitMessage / SetPreservedMessageInView split on the single
"\n" our Join uses, without any trimming — truly lossless.
- SplitCommitMessageAndDescription keeps its git-format behavior but replaces
TrimSpace with TrimPrefix("\n"), so it strips only the blank-line separator
and leaves body indentation intact.
- HandleCommitPress now mirrors HandleWIPCommitPress: it no longer passes the
preserved message as InitialMessage. OpenCommitMessagePanel resolves the
preserved content itself, uses it for display via the preservation-format
setter, and stores it as the initial message so the close-time "did the user
change anything?" check still correctly detects a cleared panel.
- GetInitialMessage no longer trims. With raw getters on both sides of the
comparison, trimming here caused spurious non-matches (e.g. for preserved
content with trailing whitespace). The original motivation — matching a
"WIP: " prefix with trailing space — works unchanged.
- UpdateCommitPanelView becomes dead code and is removed; its one remaining
caller (history cycling, always git-format) goes directly through
SetMessageAndDescriptionInView.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This isn't possible with the default quit binding ('q'), but there's an
alternative binding (ctrl-c) that does work while the panel is open; also, users
can rebind quit to something like ctrl-q.
We'll want to call just the preservation step when the app is quitting
with the commit message panel open: at that point the view-hiding and
context-popping in CloseCommitMessagePanel are pointless (the app is
about to exit), and some of the focus-lost side effects could be
surprising during teardown.
There's no reason for this; when opening the panel next time, it will be
populated again. Also, it doesn't make sense to do this only when not preserving
the message.
When no base repository is configured using gh, we have several heuristics for
choosing a likely base repo before prompting the user. One of these was that if
the origin remote exists, we'd choose that. This may be the right choice in some
cases, but in many others it's not; for example, in the common scenario of a
forking setup where the upstream repo's remote is called after its owner, and
the fork remote is called origin, we would choose origin as the base repo, which
results in not showing any PRs. Don't do that, and instead prompt the user for
the base repo in this case.
The assumption is that if a pull request exists on a main branch, it was usually
created by mistake and then closed, and showing it serves no purpose and is only
distracting.
We keep showing open pull requests for main branches though, because this allows
you to notice that there is one that you probably want to close.
This only affects the display (in the branches list and in the main view);
opening the PR in the browser using shift-G is still possible, as is copying its
URL to the clipboard.
When building multi-step custom command forms, some prompts are only
relevant depending on earlier answers. Without conditional logic,
users must dismiss irrelevant prompts manually.
Prompts now accept a `condition` field with a template expression
evaluated against prior form values. Skipped prompts default to
an empty string.
The template expression is a string pre- and suffixed with double curly
braces - {{}}.
Form keys can be reused, a guard ensures that skipped prompts do not
reset already set form keys with an empty string. This allows the
conditional flow to remind a user to set a key that was left empty
because additional conditions want that key to be set. This removes the
need to have additional if checks in the command that uses the form
keys.
This makes it easier to recover them if they were deleted accidentally.
We only do this for local branches for now, we don't bother for remote branches;
it would be trickier there, because the branch on the server might not actually
point to the same hash as our local remote tracking branch does.
For esthetic reasons, checking out a branch (or other ref) blocks the UI until
the refresh is done, so it's important that the refresh doesn't do unnecessary
work. Refreshing pull requests is unnecessary (but costly, when waiting for it)
when a branch is checked out that already existed locally. However, it is
required when checking out a remote branch for the first time, so that the PR
icon appears immediately when there is one.
For the branches panel we might consider unifying it with the existing `o`
command for creating a PR: it could check if there is a PR already, and open it
if so, or create a new one if not.
However, I also want the command in the local commits panel for the checked out
branch, and there's no existing "Create PR" command there; and the `o` command
opens the selected commit in the browser, so it's unrelated.
The only information that this carries is whether an entry is a real branch or a
detached head. Detached heads can only be at the top, and they are easy to tell
apart from the other branches by their name, so the icon is not really very
useful.
We are going to show PR icons in this column.
Not a terribly important feature for users, probably, but it is useful for
developers when testing a new feature with or without nerd fonts; and it does go
against or policy of all configs being hot-reloadable.
Note that switching from nerd fonts version 2 to 3 is *not* supported without
restarting, but I find this less important.
Set the sort order's default from the former foldersFirst to mixed, so this is a
change in behavior. I find this useful because it now matches git's order, so if
you look at the diff of a commit, the TOC at the top has the same order as the
file tree you see when entering the commit.
When you rebase a branch and there are conflicts, lazygit asks you to continue
the rebase when it detects that all conflicts have been resolved. However, it is
common to make additional changes beyond just fixing the conflicts (e.g. to fix
build failures), and when doing that while the "Continue the rebase?" prompt is
showing, lazygit detects that too and asks you if you want to stage those newly
modified files too. This is all well and good (and can be disabled for those who
don't like it); however, lazygit would treat out-of-date submodules as unstaged
changes and would offer to stage those as well, and this is pretty bad and
almost never what you want. Fix this by excluding submodules from that second
check.
Previously, when showing subcommits of a branch or tag, scrolling down, and then
going out and entering subcommits again, it would select the first commit but
keep the previous scroll position, so that the selection was not visible. Always
scroll to the top to fix this.
This is probably the less severe case, but it could still be an issue for people
who have many modified top-level files they want to discard, and have
showRootItemInFileTree set to false; they could select all those files by
pressing 'v' and '>'.
When switching to a different repo, and then back to the original one, searching
would no longer work. The reason is that our contexts set callbacks on their
views; when switching to a different repo we instantiate a new set of contexts,
so they will overwrite the views' callbacks with their new ones, but when
switching back to the original repo, we reuse the old contexts because they are
still in memory, but they won't set their callbacks again since they only do
this on construction.
To fix this, replace the view-local callbacks with a global one on the gui that
takes the view as an argument, so that the callback can look up the associated
context dynamically.
Can be used for doing additional click handling in list views.
Like the GetOnDoubleClick hook we should try to find a better design for this
than putting it in HasKeybindings and BaseContext, since it is only used by list
contexts.
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
We have some documentation for the corresponding setters in IBaseContext, but
that's part of the controller infrastructure and not client facing. For somebody
implementing a new view, this is where they will probably look for what methods
they can override.
When this was originally introduced, it handled single clicks on a list entry
(treating them similar to a double-click by checking whether the click was on
the selected entry). Arguably it should have been called OnDoubleClick back then
already; but when we later changed it to do actual double-click detection (see
37197b8e9a), we should have renamed the methods.
If the showDivergenceFromBaseBranch config is not none, the expanded branches
panel would no longer show the remote branch and subject; the reason was that we
padded the name all the way to the right edge of the view so that the divergence
information is right-aligned.
To fix this, we simply don't right-align the divergence in half or full screen
mode. This is a bit unfortunate, but it's hard to do any better, because we
don't know the width of our column in advance. One thing I tried is to make the
divergence a separate column in that case (with an explicit right-alignment set
on it via getColumnAlignments; however, this makes it harder to properly
truncate the name when the window is too narrow.
When a branch is checked out by another worktree, show the worktree
name in the label, e.g. "(worktree cosmos2)" instead of just
"(worktree)", so you can immediately see which worktree holds it.
Actually, we simply enable them in any panel, even those that can never show a
diff. Since the commands show a toast, that's enough feedback to understand what
happened; the change will take effect the next time you switch to a panel that
does show diffs.
Same for ( and ) to change the rename threshold.
This fixes two problems:
- if the previously focused view (behind the panel) was a list view, it would
look like the click would select a different row, because gocui would still
set the view's cursor position, which is used to draw the highlighted row
- it was still possible to click on tab headers, and this would dismiss the
panel
I got confused when I investigated the bug that was fixed in the previous
commit, so hopefully these will be helpful for the next person reading this
code.
The symptom of the bug was that confirmation panels with wrapped text were one
character too wide, which could sometimes result in them being one line too
heigh. One concrete example is the "Discard file changes" confirmation that
appears when pressing `d` in the commit files panel.
I got very confused when investigating this bug, and the reason is that
getPopupPanelDimensionsAux took two parameters panelWidth and panelHeight, but
the second was actually contentHeight. Rename it to contentHeight, and change
the first one to also be the content width rather than the panel width. This is
easier for clients, and less confusing.
This commit implements the ability to cycle backward through different
all branches log visualization modes, complementing the existing forward
cycling functionality. Users can now press 'A' (Shift+a) to cycle in
reverse through the available log graph views, improving navigation
efficiency when they overshoot their desired view.
Changes:
- Added RotateAllBranchesLogIdxBackward() method to BranchCommands for
backward rotation through log command candidates
- Introduced AllBranchesLogGraphReverse keybinding configuration with
default key 'A' (uppercase)
- Implemented switchToOrRotateAllBranchesLogsBackward() handler in
StatusController that mirrors forward cycling logic
- Added English translation for "Show/cycle all branch logs (reverse)"
The implementation uses modulo arithmetic with proper handling of
negative indices to ensure seamless backward cycling through the
available log visualization options.
The worktrees tab now displays the checked-out branch (or detached HEAD
state) for each worktree, making it much easier to see which worktree
holds which branch.
This is useful when you need to "unlock" a branch that's occupied by
another worktree: you can now clearly see which worktrees are on a
branch vs detached, without having to select each one individually.
Also rename the "(main)" label to "(main worktree)" to avoid confusion
with the common "main" branch name, and move it after the branch column.
Change working tree files and commit files panels to use filtering
(reducing the list) instead of search (highlighting matches). This
matches the behavior of other filterable views.
The text filter matches against the full file path, not just the
filename, which is more useful for navigating large directory trees.
When toggling a directory for a custom patch while a text filter is
active, only the visible filtered files in the directory are affected,
consistent with how staging a directory in the files panel works.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Without this check, the selection was being reset to 0 whenever
ReApplyFilter was called for the current filter context, even when
the user wasn't actively typing in the search prompt (e.g. when the
model updates in the background). This was causing unexpected cursor
jumps.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When opening a filter prompt, reset PrevSearchIndex to -1 to avoid
stale search state from a previous search/filter session.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This avoids a naming collision with GetFilter from the
IFilterableContext interface, which will be implemented by
FileTreeViewModel in the next commit.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
I always press 'd' in the patch building view, expecting that I can do
exactly what I can do in the staging view, to find out I need to go
space -> ctrl+p -> d and I think it's time to honour the muscle memory
and support this convenience keybinding.
We don't add the commands to the menu when we're rebasing, so it's pointless to
check this in the handlers.
I guess alternatively we could add all commands always, and strike out the ones
that are not applicable using DisabledReasons. But we don't usually do this in
menus (except the global keybindings menu).
Only mention resetting the patch when there actually is one. This way users have
to read less text in the normal case, and the added note hopefully stands out
more if there is one. Also, separate the note from the previous text by a blank
line.
"Remove" can be confusing when a deleted file is selected; in this cases it
actually "un-removes" it. "Discard" hopefully makes it clearer that we are
talking about the change to a file, and not the file itself.
If the bottom line is hidden (by setting gui.showBottomLine to false in the
config), don't show the "Fetching..." status because it changes the layout,
which should happen only in response to user actions.
However, keep the status for the first fetch after startup, and right after
switching repos, because in these cases it is useful to see when the fetch is
done.
Tweak regexp such that a trailing ":" is not included in the file path
extracted from a lazygit-edit URL.
Previously, when matching a URL containing the ":" separator but no line
number, such as `lazygit-edit:///path/to/file.ext:`, the trailing
separator would be included in the matched file path, so lazygit would
open the non-existent file `/path/to/file.ext:`. Notably, such urls are
created when using delta with the hyperlink feature, as suggested in
https://github.com/jesseduffield/lazygit/blob/master/docs/Custom_Pagers.md#delta,
and clicking a file path rather than a line number.
This is a regression introduced with #5135. I didn't notice it because I am
using `gui.scrollOffBehavior: jump`, in which case the problem wouldn't appear;
it only happened with `scrollOffBehavior: margin` (which is the default). In
that case, if you used the arrow keys (or j/k) to move the selection so that the
view would start to scroll, empty space would appear.
This brings in https://github.com/jesseduffield/gocui/pull/98 with the following
fix:
Fix rendering of CRLF sequence ('\r\n')
The FirstGraphemeCluster call returns this as a single character; we want to
treat it the same way as a single \n.
This would be a problem if e.g. a progress bar used \r repeatedly to paint over
the same line, and then printed a \n to move on to the next line; the last pair
of \r and \n was swallowed.
Another scenario where this was a problem was if you stream output of a command
to the log, and the command used \r\n as line feeds. This happens for example
for a background fetch that fails with an error; in that case we print the
combined output (stdout plus stderr) to the log after the command finished, and
for some reason it uses \r\n in that case (I can't actually explain why; when I
do `git fetch --all | xxd` I see only bare \n characters). All output would
appear on one line then.
Also, filter out escape sequences for character set designation; there's nothing
useful we can do with them. In practice, the only one that you are likely to see
is `ESC ( B`, which is sent as part of tput sgr0, which is sometimes used in
scripts to reset all graphics attributes to defaults.
I've optimised for muscle memory backwards compatibility here:
- Outside interactive rebase: press 'f' then instead of a confirmation
panel, a menu appears where you can choose to keep the selected commit's
message
- Inside interactive rebase: press 'f' then press 'c' to see the menu
for keeping the message, where if you press 'c' again it will retain the
current message. so 'fcc' is the chord to press.
We're also now showing the -C flag (which is what enables the behaviour)
against the todo.
I've picked the 'c' keybinding because 'C' was taken and it corresponds
to the flag. Previously that showed a warning about a change in
keybinding for cherry picking but it's been ages since we've made that
change so I'm happy to retire it.
Not used yet, we pass an empty string everywhere, to match the previous
behavior. Just extracting this into a separate commit to make the next one
smaller.
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
It's been ages since we changed the key, users should hopefully be used to it by
now, and we want to reuse the key for something else later in the branch.
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
On very large screens they would get ridiculously wide.
The limits we chose here are a little arbitrary:
- 80 for confirmations and prompts
- 90 for menus
- auto-wrap width plus 25 for the commit message editor (or 100 if auto-wrap is
off)
To reproduce: open the keybindings menu ('?'), press '>' to select the last
line, esc to close it, open it again. The view would appear still scrolled down,
so the selected first line was out of view. Even worse, if you open a different,
shorter menu (e.g. the "View upstream reset options..." menu in the Files
panel), you'd only see the Cancel item, the other ones were scrolled out of
view.
This is a regression introduced with efd4298b5e (#5134).
Since the reflog can get very long, this saves some memory but especially some
UI thread lag. In one of my repos I had over 11'000 reflog entries (I guess I
should prune them more regularly...), and rendering them took ~600ms; since this
happens on the UI thread, there was an annoying stall for half a second after
every background fetch, for example.
This fixes a bug in ListContextTrait.FocusLine whereby the view would go blank
when scrolling by page (using ',' or '.') in views that have
renderOnlyVisibleLines set to true but refreshViewportOnChange set to false.
Currently we don't have any such views; the only ones who use
renderOnlyVisibleLines are commits and subcommits, and they also use
refreshViewportOnChange. However, we are going to add one in the next commit,
and eventually it might be a good idea to convert all our list views to that by
default, and get rid of the renderOnlyVisibleLines flag.
Move SetContentLineCount into OverwriteLinesAndClearEverythingElse. Calling it
separately beforehand is not concurrency safe; we need both to happen
when the view's writeMutex is locked.
We have this logic to avoid constantly rerendering the main view when hitting
up-arrow when you are already at the top of the list. That's good, but we do
want to scroll the selection into view if it is outside and you hit up or down,
no matter if it changed.
It is possible to scroll the selection out of view using the mouse wheel; after
doing this, it would sometimes scroll into view by itself again, for example
when a background fetch occurred. In the files panel this would even happen
every 10s with every regular files refresh.
Fix this by adding a scrollIntoView parameter to HandleFocus, which is false by
default, and is only set to true from controllers that change the selection.
We want to do this whenever we switch branches; it wasn't done consistently
though. There are many different ways to switch branches, and only some of these
would reset the selection of all three panels (branches, commits, and reflog).
We move the selection down by the number of commits that were reverted (to keep
the same commits selected). However, this only happens after refreshing, which
has rendered the main view with the wrong commit, so we need to render it again
after moving the selection.
There are many other callers of MoveSelection in LocalCommitsController, but all
of them happen before the refresh. Revert is special because it needs to move
the selection after refreshing, e.g. when reverting the only commit of a branch.
