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>
A handful of default keybindings differ by platform (e.g. word-wise
cursor movement in text inputs uses alt on macOS but ctrl elsewhere).
Lazygit chooses these based on the OS it runs on, but that's the wrong
signal when the OS isn't where the user is actually typing: someone
running lazygit in a Linux container that they access over ssh from a
Mac gets the Linux bindings, when they'd rather have the Mac ones.
Remapping each binding by hand via config is tedious, so add a single
LAZYGIT_KEYBINDING_PLATFORM override.
An unrecognized value falls back to the real OS rather than to the
non-darwin default bindings, since the latter would be an arbitrary
choice.
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>
RenameYamlKey can only rename a key in place, under the same parent. To
migrate a keybinding from one section to another we need to relocate the
key to a different parent mapping, which is a move, not a rename.
MoveYamlKey creates intermediate maps at the destination as needed and
prunes any maps left empty behind the key, so a section that held only
the moved key doesn't linger as an empty mapping in the user's config.
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>
The redesigned worktree-creation flow never asks the user to type a path
from scratch; instead it offers candidate parent directories. Until a repo
has any linked worktrees to learn from, there's nothing to offer, so let
users seed that list with a configured default location.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ConPTY compresses runs of default-colored spaces into ECH + CUF
(\x1b[NX\x1b[NC) instead of emitting them literally. ECH is still a
no-op for us — our buffer is built sequentially and has nothing to
erase — but CUF has to materialize as N visible space cells so the
gap actually appears, otherwise content the child wrote with leading
indentation slides left against the preceding cell.
The view's cursorForward branch reuses the same machinery as tab
expansion: substitute the trigger byte for a space and let the
repeatCount path emit the cells under the parser-tracked SGR. The
existing notifyCellsWritten plumbing then advances screenCol over
the gap, keeping subsequent CUP targets aligned.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConPTY compresses runs of default-colored spaces into ECH + CUF
(\x1b[NX\x1b[NC) rather than emitting them literally. Both currently
fall through the parser's swallow path, so the gap they describe
collapses entirely and content that the child wrote with leading
indentation ends up slid left against the previous cell.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConPTY presents its child's output as a screen buffer and uses CUP /
CUD / CNL / VPA to skip over blank rows rather than emitting LFs. The
previous behaviour swallowed all of those and the visible content
collapsed together. Now the escape parser tracks the screen-relative
cursor row, and any CSI that moves the cursor past the current row
emits a cursorDown instruction that the view turns into the matching
number of empty lines.
Column tracking is deliberately omitted: doing it correctly would mean
duplicating the view's grapheme-cluster width math in the parser, and
ConPTY in practice positions to column 1 after a CR-equivalent, which
the existing wx-reset path already handles. ConPTY-internal scrolling
needs no special handling either: it only emits cursor-positioning
escapes within the first, un-scrolled screenful — once its screen
scrolls it switches to plain linefeeds, which the view advances on
directly regardless of the tracked cursor.
Backward cursor moves are silently dropped — the view's buffer is
append-style and can't undo earlier writes. The exception is cursor-home
(CUP to row 1): ConPTY emits it at the start of every screen, so rather
than drop it we re-anchor the row tracking to the current write position.
Without that, a view not rewound in lockstep with ConPTY's screen (the
command log, which streams pty output without a rewind) accumulates
drift, and every later absolute CUP becomes a dropped backward move that
collapses the rows ConPTY positioned with.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConPTY presents its child's stdout as a screen buffer and uses CUP
(`\x1b[<row>;<col>H`) to skip over blank rows rather than emitting LFs
for them. Our escape interpreter swallows CUP via the catch-all
"valid CSI final byte we don't implement" branch, so the blank rows
the child put between non-blank ones disappear and the surrounding
lines collapse together — which is what makes the delta-rendered diff
in the screenshot look like its blank lines and section breaks were
removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The per-platform getCmdHandlerPty split existed because the Unix side
had creack/pty and the Windows side had nothing — so it fell back to a
non-pty handler. Now that oscommands.StartPty provides a pty on both
platforms, the two files collapse into one cross-platform
implementation and the stub is gone.
cmdHandler grows a 'wait' field because the pty path on Windows spawns
via CreateProcess and never runs exec.Cmd.Start — so cmd.Wait wouldn't
work there. Non-pty handlers set wait = cmd.Wait; pty handlers set it
to the wait closure StartPty returns.
Co-Authored-By: Claude Opus 4.7 (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>
Update spawned a goroutine per call that then sent on the user-events
channel, so multiple Update calls from the same goroutine could be
reordered by the scheduler — the doc comment even admitted "the order in
which the user events will be handled is not guaranteed." That
non-determinism is a latent source of flaky rendering: code that queues a
model update and then a render in source order could see them run in the
opposite order.
Send on the channel directly instead, so same-goroutine calls arrive in
source order. The send is non-blocking and panics on a full channel
rather than blocking (a blocked send from the UI goroutine would deadlock
against itself) or silently reordering; the buffer is sized generously so
this is unreachable in normal use. UpdateAsync is now identical to Update
and unused, so it's removed along with the shared updateAsyncAux helper.
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.
NewTask was incrementing newTaskID and reading taskID inside the
spawned goroutine, so for two NewTask calls in quick succession the
assignment was determined by goroutine scheduling order rather than
call order. When the goroutines reordered, the first NewTask call
could end up with the higher taskID and "win" the staleness check,
superseding the second call's task even though the caller intended
the second to be the latest.
Worse, the staleness check ran after onNewKey, so a goroutine destined
to bail as stale would still reset the view buffer first, potentially
wiping the winning task's already-written output.
Take newTaskID++ synchronously in NewTask so taskIDs follow call order,
and move the first staleness check ahead of onNewKey so a stale task
doesn't side-effect the view before exiting.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
This reverts commit eb988395e6, which removed the wrapper on the
grounds that it no longer had anything to do but delegate. We're about
to give it a job again: hosting a git-specific default environment
variable on every command. Restore the scaffolding first, as a pure
behaviour-preserving step, so the behaviour change that follows is
minimal.
`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>
Pressing `b` on a conflict is meant to pick both sides. With the diff3
conflict style, git also renders the common ancestor between the two
sides, and `b` currently keeps that ancestor section too, which is
wrong: the common base is neither side of the merge and must not end up
in the resolved file.
This test captures the current (buggy) behaviour so the follow-up fix
has a clear before/after.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TestNewCmdTaskInstantStop is flaky: it closes the stop channel from
within start() and asserts the stopped task touched nothing. But Go's
select picks uniformly at random among ready cases, so when opts.Stop
and a data channel are both ready the loop can pick the data channel,
call beforeStart() (which clears the view) and write the prefix before
bailing. In production a task that's already been superseded thereby
clobbers the output the incoming task is about to render.
Check stop with a non-blocking select before each blocking select, so
the stop signal wins whenever it's already closed (Go has no built-in
priority select; this is the idiomatic substitute). The selects keep
their own stop case for liveness, to unblock when stop closes while
parked waiting for data.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After the previous commit, the escape interpreter still had five paths
that returned an error from parseOne, which view.go handles by rendering
whatever bytes it had accumulated as literal cells. Each of these is a
case where silently consuming the sequence is strictly better than
leaking garbage.
- ';' as the first CSI byte: '\x1b[;5H' is a valid sequence (row
defaults to 1) but we errored on the leading ';'.
- Intermediate bytes in CSI ('\x1b[0 q' = DECSCUSR): the sequence ends
in a final byte we don't implement, so consume and drop.
- Malformed SGR params (empty slot like '\x1b[1;;m'): if outputCSI
fails mid-parse, reset state instead of re-emitting the sequence.
- OSC 8 that isn't actually OSC 8 ('\x1b]8x...'): treat as an OSC we
don't understand and skip to its terminator rather than error-
resetting mid-sequence, which used to leave the rest of the OSC body
to be printed as text.
- The sanity-check overflow paths (too many params, param too long)
now switch to a 'discard until final byte' state rather than
returning the accumulated bytes.
A new stateCSIDiscard centralizes the 'consume bytes until the CSI
final' behavior used by both the intermediate-byte and overflow paths.
errCSITooLong and errOSCParseError are gone with their only callers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A text-mode escape interpreter can't do anything meaningful with cursor
positioning, DEC private modes, or terminal resets — but it must still
consume them, not print them as literal text. Before this change, any
sequence outside SGR / EL / OSC-8 errored out of parseOne, and view.go
rendered the unparsed bytes as visible cells. On Windows this would show
up as junk at the start of main-panel output once we add PTY support
using ConPTY, because ConPTY's session-init stream is full of such
sequences.
Three additions to the state machine:
- stateEscape: a single byte in 0x30–0x7E after ESC (e.g. ESC c = RIS)
is a complete Fs/Fp sequence per ECMA-48; consume and reset.
- stateCSI: accept the DEC private-mode prefix bytes (<, =, >, ?), and
accept a CSI final byte (0x40–0x7E) immediately after [ as the end of
a zero-param sequence.
- stateParams: accept any CSI final byte we don't implement as the end
of the sequence rather than a parse error.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The escape interpreter errors on anything outside the handful of
sequences it understands (SGR, EL, OSC 8 hyperlinks), and view.go then
renders the unparsed bytes as text cells.
Co-Authored-By: Claude Opus 4.7 (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.
Exercises the path the live reload relies on: a per-repo lazygit.yml
sets a different side panel layout, and switching between repos
re-applies each one's own layout (the new-repo path for the cloned repo,
the cached-repo path on switching back).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
Cover the three things gui.sidePanels enables: reordering the panels
(swapping branches and commits, checked via their jump keys), hiding a
panel (omitting stash, checked by cycling past the last panel and wrapping
to the first), and promoting a tab to its own panel (worktrees becomes a
top-level panel reachable by a jump key, and the files panel's remaining
tabs cycle straight to submodules).
The tests drive focus with explicit jump keys rather than ViewDriver.Focus,
which assumes the default panel layout.
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>
This adds the user-facing surface for configuring the side panels: their
order, which ones are visible, and how tabs are grouped into panels. Each
entry is either a single panel name or a list of names sharing one panel
as tabs, mirroring how the Keybinding type accepts a scalar or a sequence;
the JSON schema restricts the names to the known set so editors can offer
completion and catch typos. The default reproduces today's layout exactly.
Validation rejects unknown or duplicated names, and requires the files,
branches, and commits panels to always be present: a lot of code focuses
those directly (e.g. after resolving a conflict or popping a stash), so
allowing them to be hidden would let that code focus a hidden panel.
Nothing reads the option yet; the layout still uses the hard-coded order.
Wiring follows in a later commit so the inert surface (and its generated
docs and schema) can be reviewed on its own.
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>
The unit tests only assert the arguments lazygit constructs; they can't
catch cmd.exe's own quote-stripping, which is where #5560 actually
manifested. This test builds a small editor executable, places it and
the file it opens at paths containing spaces, runs it through real
cmd.exe via NewShell, and checks the editor received the intended args.
It runs only on Windows.
Co-Authored-By: Antoine Gaudreau Simard <a.simard@multidev.net>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lazygit builds a shell command by interpolating Quote'd arguments into a
template and running the result via `cmd /c`. Several things were wrong
on Windows:
- Quote emitted bash-style `\"…\"` quoting, which cmd.exe doesn't
understand. Making it usable at all previously required a fragile
round-trip through str.ToArgv and re-escaping.
- The assembled command line was handed to `cmd /c` without `/s`, so
cmd's default rules stripped the wrong quotes once the line contained
more than two of them (e.g. a quoted editor path at a location with
spaces, plus a quoted filename that also contains spaces).
- Shell metacharacters were escaped with `^` (`&` → `^&`, etc.), which
neutralised command chaining, pipes, redirection and `%VAR%` expansion
in custom commands.
Quote now emits the standard Windows convention directly, and NewShell
hands cmd.exe the fully-assembled line verbatim via SysProcAttr.CmdLine,
wrapped as `cmd /s /c "<command>"`. The /s flag strips exactly the outer
quote pair we add, leaving each argument's own quoting intact. With the
`^` escaping gone, metacharacters in a custom command reach cmd as the
author intended; this also removes the spurious `^` reported in #3092.
Fixes#5560Fixes#2427Fixes#4147
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On Windows, NewShell escapes shell metacharacters (`&`, `|`, `<`, `>`,
`%`) with `^` and splits the command into separate arguments. The
operators in a custom command therefore never reach cmd as operators,
so command chaining (`&&`), pipes, redirection and `%VAR%` expansion all
silently break (#2427, #4147, #5113; the stray `^` is also what #3092
reports).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On Windows, Quote wraps arguments in bash-style `\"…\"` and rewrites
embedded double quotes as `"'"'"`. Neither convention is understood by
cmd.exe or CommandLineToArgvW, so commands built from quoted arguments
are mis-parsed once they contain quotes or spaces (#5560).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The integration README still described the raw `go run cmd/integration_test`
and `go test` invocations, which are easy to get wrong (the headless go-test
command in particular) and don't match how we actually run the tests. Rewrite
the running/debugging/sandbox instructions around the justfile's e2e recipes
instead, and point at the optional zsh completion script. Also switch the
test-list regeneration hint to `just generate`, matching the rest of our docs.
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.
