Probe git for the handshake like the other renderers, not for a record.
git announces itself for exactly the formats it describes, so asked with
the renderer's own arguments the handshake already answers the question,
and on empty input at that. Looking for a record was over-specified and
made git the one renderer probed differently for no reason.
The two probes that now both stand a diff up from two empty temp files
share that setup.
git now emits the diff line metadata records itself, for the word-diff
formats -- the ones whose output can't be read back from its own text,
which is the same reason we need records out of delta and difftastic. So
a rawGit renderer configured with --color-words is no longer a diff we
have to give up on: it names every row it shows, as a patched pager does.
Two things stood in the way. The probe only knew how to ask a stdin
filter or an external diff driver and reported false for anything else,
so git's own records were never looked for; and the focused main view
treated a rawGit renderer with arguments as unresolvable by definition,
re-rendering it raw whatever the probe said.
So probe git the way we will run it, with the renderer's own arguments,
and look for an actual record rather than the handshake the other probes
settle for. git annotates only some of its formats, so an installed git
that doesn't speak the protocol and arguments that select no word diff
both leave us without records, and looking for a record answers both at
once. Arguments that aren't a word diff (-U10, --stat) therefore keep
rendering raw when focused, as they did before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pager authors reviewing the protocol asked to drop the EMIT_ prefix: the
OSC number already names the protocol, so EMIT_ was redundant. Rename the
handshake variable to OSC1717_METADATA in the spec and in the two places
lazygit advertises it (the pager PTY and the metadata probe).
Use osCommand.GetTempDir() (lazygit's own per-session temp dir, which it
creates and cleans up) as the parent for the custom-patch diff trees, instead
of the OS default — so it honors the configured temp dir and is cleaned up with
the rest of lazygit's temp files.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The secondary pane showed the custom patch with a bespoke in-memory render
(PatchBuilder.RenderAggregatedPatch / FormatView). That had two problems: it
could never be fed to a pager the way the main view's diff is (a stdin pager
might have worked, but an external diff tool like difftastic, which diffs two
files rather than a unified diff, could not), and its hand-rolled hunk/context
handling differed subtly from git's.
Materialize the patch instead as two real file trees under a temp dir — a/ holds
each patched file's "from"-side content, b/ that content with the patch applied —
and render it with `git diff --no-index`, reusing the exact pager wiring the main
view uses (stdin pager, external diff, or git's own colour as the raw fallback).
`--no-index` honors both GIT_PAGER and --ext-diff, so every pager type now renders
the custom patch like any other diff, and git computes the context, fixing the
quirks. Because the secondary is now an async diff task, the post-removal
selection reveal (which rides a task's restore) finally takes effect.
The trees are named a/b so that with --no-prefix the diff shows the real
repo-relative paths; added files are seeded empty in a/ so they pair up and show
their real paths rather than git's directory-comparison "added in b" form. The
patch builder owns the temp dir's lifetime (created on Start, removed on Reset)
and bumps a generation counter on every change, so the trees are rebuilt only
when the patch actually changes — covering the focused-main view and the old
explorer alike, without rebuilding on mere navigation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The focused-main-view rework made the secondary pane actionable, but the old
patch-building explorer's secondary was inert, so its actions were never thought
through. Pressing space there routed through the same toggle handler as the main
pane, resolving the selection against the secondary's diff and mapping it to
patch-builder indices by line number. But the secondary shows the *aggregated*
custom patch, which renumbers included additions whenever an earlier addition in
the same hunk is excluded (Transform recomputes each hunk's +start). So the
shifted number resolved to the wrong line in the original diff — often adding an
unrelated line instead of removing the selected one.
Resolve the secondary selection by its *ordinal* among the change lines shown
instead: the custom-patch view renders exactly the included change lines in
order, so the k-th change line of a file is that file's k-th included change line
(PatchBuilder.IncludedChangeLineIndices), independent of the renumbering. Space
in the secondary now only ever removes, mirroring how space in the staging view's
staged pane unstages.
Discarding from the commit (the remove key) makes no sense in the custom-patch
preview — it would act on lines shown only as the patch, and space already
removes them — so it's disabled there.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Prep for extending the focused main view's raw-diff fallback beyond the files
panel. ShowCmdObj, ShowFileDiffCmdObj, ShowStashEntryCmdObj and DiffCmdObj gain
the ignoreExternalDiff arg WorktreeFileDiffCmdObj already has — forcing git's own
coloured diff regardless of a configured external diff command. All callers pass
false, so behaviour is unchanged.
Also lift the pty-vs-command task choice out of the files controller into
types.NewMainViewDiffTask(WithPrefix), so the commit/stash/patch-building panels
(whose diffs are built in the diff helper, which can't reach the staging helper)
can select the same raw-fallback task.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The focused main view is the staging surface, so it has to resolve the diff it
shows to patch-space. A pager that restructures the diff without emitting our
metadata (stock delta-default, plain difftastic, `cat -n`) produces output the
buffer parser can't read, which would leave its diff unstageable.
When such a diff is focused, re-render it raw — git's own colour, no pager — the
same content a no-pager setup shows, which the buffer parser handles. Browsing
keeps the pretty pager output; only focusing to act switches to raw.
Whether the pager is usable is decided by probing it: run it on empty input and
look for the version-only OSC 1717 handshake a metadata-aware pager emits first.
This is a pager-level, content-independent fact (a binary file, which has no
change lines under any pager, can't mislead it) and it's known before we render,
so we never render pretty only to discover mid-flight that we should have rendered
raw. The verdict is cached per pager (reset when the pager changes). No PTY is
needed — git needs a terminal to decide to invoke a pager, but the pager emits the
handshake regardless. A git-config external diff driver (useExternalDiffGitConfig)
is chosen per file via .gitattributes and a single diff can mix drivers, so
there's no one pager to probe; it's treated as unsupported (always raw).
Bypassing the pager needs two things, since a pager reaches the diff by two
routes: an external diff command (suppressed in the cmd via the new
ignoreExternalDiff arg, keeping git's colour, unlike plain) and a stdin pager
(GIT_PAGER, applied by the pty task — so the raw render uses a plain command
task instead).
This wires the files panel; the commit/stash/patch-building panels follow.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The patch builder works in patch-line-index space, which the patch
explorer can supply directly from its parsed-patch state. The focused
main view can't: it only knows a selection as diff-line metadata
identities (file line number + deletion?), and those indices differ
between the raw diff and however a pager renders it.
Add the conversion at the boundary so the main view stays in identity
space: PatchLineIndicesForLines maps identities to indices (for
toggling) and IncludedLineIdentities reports the included change lines
as identities (for the inclusion gutter to match rendered rows against).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The focused main view's click/enter/e/G handlers all need the same thing: given
a rendered diff row, the patch-space line it corresponds to. Until now that came
solely from delta's lazygit-edit:// hyperlinks, which only carry a path and a
single line number — no side. That's lossy: for a deletion the number is the old
line, but the consumers fed it into new-file lookups, and two consecutive
deletions (which share a new-file line number) couldn't be told apart at all.
Replace GetFileAndLineForClickedDiffLine with GetDiffLineInfo, returning the
fuller (file, type, new-line, old-line) record from diff-line-metadata-notes.md.
This is mechanism #1: parse the decolorized view buffer — walk up to the file's
"diff --git" section, reuse patch.Parse on it (splitting multi-file commit diffs
on the "diff --git" boundaries), and read the type and line numbers off the patch
arithmetic. It serves the structure-preserving renderings — no pager, git diff
--color, and delta --color-only without line numbers — with no external
dependency.
To avoid trusting a mis-parse, the parser bails when a hunk's body no longer
matches its header (Patch.IsWellFormed). That's what happens when a pager keeps
the diff/hunk headers but restructures the body: delta's line-number gutters push
the +/- marker off the start of each line, so every body line reads as context.
Such renderings fall through to the next backend rather than yielding a confident
wrong answer. (diff-so-fancy goes further and rewrites the headers too, so it
fails even earlier, on the missing "diff --git".)
GetDiffLineInfo is a seam with swappable backends: the buffer parser first, then
the old hyperlink reader as a fallback for renderings the parser can't handle
(delta's default mode, or delta with line-number gutters). The future #2 OSC
per-cell metadata reader plugs in ahead of both, behind the same record shape.
Wire the consumers to the record per that doc's field mapping:
- dive into staging/patch building lands on the exact patch line, looking a
deletion up by its old-file line number (PatchLineForOldLineNumber) so the
two-deletions case resolves correctly;
- `e` edits at the new-file line;
- `G` anchors the PR link on the left (old) side for a deletion, the right (new)
side otherwise.
The hyperlink fallback can't convey the side, so it reports DiffLineOther, which
the consumers treat as a non-deletion — i.e. exactly today's behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The patch package can already map a patch line index to its new-file line
number and back (LineNumberOfLine / PatchLineForLineNumber). The diff-line
parser being built on top of this needs the old-file equivalents: a deletion
has no distinct new-file position (two consecutive deletions share one), so to
land on the exact deletion when diving into staging we have to look it up by
old-file line number.
Add OldLineNumberOfLine and PatchLineForOldLineNumber as direct mirrors of the
new-file functions, counting DELETION+CONTEXT lines instead of ADDITION+CONTEXT.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was already possible, but only when a file was selected, and it woudln't
always land on the right line when a pager was used. Now it's also possible to
do this for directories, and it jumps to the right line.
At the moment this is a hack that relies on delta's hyperlinks, so it only works
on lines that have hyperlinks (added and context).
The implementation is very hacky for other reasons too (e.g. the addition of the
weirdly named ClickedViewRealLineIdx to OnFocusOpts).
Ask git for the attribute of every conflicted file whenever we load the
file status, so that we recognize the markers it actually wrote. Files
that are set up this way are precisely the ones whose regular content
tends to contain marker-looking lines, so matching a run of at least
seven characters instead is not an option: we'd take the file's own
content for markers and then never consider its conflicts resolved.
One `git check-attr` call covers all conflicted files at once; asking per
file would take seconds when hundreds of files are conflicted, and it
would hurt worst on Windows, where spawning a process is expensive.
Because the lookup rides along with the file status, it costs nothing
when there are no conflicts, and editing .gitattributes during a merge
takes effect on the next refresh.
Git doesn't always write conflict markers of seven characters: the
conflict-marker-size gitattribute overrides that per file, and it is set
for good reasons — for file types whose regular content tends to contain
marker-looking lines, such as documentation about merging, or test
scripts. We hard-code seven characters everywhere we look for markers,
so none of that works.
Prepare for honoring the attribute by threading the marker size through
everything that recognizes a marker, carried on the file model. Nothing
fills it in yet, so we still use git's default size of seven everywhere,
and matching is unchanged: a marker consists of exactly that many marker
characters, and all but the "=======" one are followed by a space and a
label.
Running lazygit in a .git dir got you told you were in a bare repo,
which you weren't: the worktree was sitting right there, one directory
up. git's own convention is that a git dir called .git belongs to the
directory holding it — that's how `git worktree list` names the main
worktree — so ask that directory, and if it is a worktree, open the repo
we were really being asked about.
The git dirs that aren't called .git keep the answer they had. A linked
worktree's and a submodule's do have a worktree, but nothing we look at
says where, so we would be guessing; a bare repo's has none to find.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
git finds a repo by looking for a .git in the directory a command runs
in. Lazygit runs its commands in the work tree, so that normally works —
but not when the git dir lives somewhere else entirely, which is what
core.worktree and --work-tree are for. Lazygit chdir'd into such a work
tree and then ran commands that couldn't see any repo from there, so
opening a repo with core.worktree set panicked on startup. It only
worked with --git-dir because that leaves GIT_DIR in the environment for
every command to inherit.
Work out at startup whether git can find the repo from its work tree,
and when it can't, put GIT_DIR and GIT_WORK_TREE on every command the
repo's builder produces. As with the working directory the builder pins
(527124d0e0), these also go into the process env — subprocesses don't
come through the builder — but the commands don't read them from there,
because the process env belongs to whichever repo we have switched to
since.
Working out whether git can find the repo means asking git, rather than
reading the .git file, whose contents can spell the same directory
differently than git does. The extra query is skipped for a repo whose
git dir is simply its .git directory, which is nearly all of them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GIT_DIR and GIT_WORK_TREE tell git where our repo is, and every command
we run inherits them — including the ones we point at a submodule or
another worktree. git resolves those against our repo instead, and says
nothing about it: with GIT_DIR set, `git -C mysub log -1` reports the
superproject's commit. So opening lazygit with --git-dir/--work-tree
quietly broke resolving submodule conflicts, stashing and resetting a
submodule, and detaching another worktree; the worktree list came back
claiming every worktree shared our git dir.
Drop the two variables from the commands that address another repo.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reset told git to change directory with -C while runInParentModule does
it by setting the command's working directory, but they were computing
the same directory for the same reason. Use the helper, so that there is
one place that knows what running in a nested submodule's parent means.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Its working directory resolves against the process rather than against
the repo the command builder pins commands to, which is only safe
because nothing but foreground commands come through here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
git makes `rev-parse --show-toplevel` fatal when there's no work tree,
so asking for it together with everything else meant we never got an
answer at all for a bare repo: GetRepoPaths returned an error, nobody
ever saw IsBareRepo() == true, and lazygit either died with a stack
trace or decided we weren't in a repository. That's what you got for
opening it in a directory holding a bare repo and a .git file pointing
at it, which is a normal way to keep a repo and its worktrees together.
Ask again without --show-toplevel when the first query fails: the other
queries work fine without a work tree, so if they now succeed we know
we're in a bare repo, and the existing prompt offering to open a recent
repo does its job. If they fail too we're not in a repo at all, and the
first error already says so.
--is-bare-repository is gone from the query: a work tree implies
core.bare is false, so it could only ever come back false there, and
what matters to us is whether there is a work tree to show, which is
what we now go by.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RepoPath() is meant to be the same as WorktreePath() when we're in the
main worktree, but we derived it from the git dir's location instead.
That is only the same thing when the git dir lives inside the work tree.
With core.worktree, --work-tree, or a .git file pointing at a repo dir
that isn't called .git, it lands on a directory that isn't a worktree at
all, and the repo name we show follows it there.
A worktree that has the repo's common git dir to itself is the main
worktree, so use its path. That subsumes the submodule case, whose git
dir lives under the superproject's .git/modules but is still the
submodule's own common dir; --show-superproject-working-tree is now only
needed for a linked worktree of a submodule.
The existing bare repo test asserted a git output that can't occur (a
work tree and --is-bare-repository=true at once), but the rest of it is
the shape of a repo opened with --git-dir/--work-tree, where the new
repo path is the correct one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When the work tree lives somewhere else entirely — set up with
core.worktree or --work-tree — we're still in the main worktree, so
RepoPath() should be the work tree, as its own doc comment says. Instead
we derive it from the git dir's location, which lands somewhere that
isn't a worktree at all, and the repo name follows it.
The ACTUAL lines are indented as they will be once the EXPECTED ones
replace them, rather than as gofumpt wants them while the comment
markers are still splitting the struct's alignment. That leaves this one
file not gofumpt-clean until the next commit, in exchange for a diff
there that shows only the lines that actually change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`git worktree list` reports the main worktree as the common git dir with
a trailing "/.git" removed, which equals the working tree only when the
git dir sits inside it. In a submodule, a bare repo, or a repo using
core.worktree it doesn't, so comparing the reported path against the
working tree path matches nothing: no worktree is recognized as current
or as main. Most visibly, inside a submodule lazygit claimed we were in
a linked worktree named after the submodule, and offered to remove that
"worktree".
Comparing git dirs identifies a worktree unambiguously, so use that.
A worktree whose directory is gone has no git dir to compare, and there
we still have nothing better than its path.
The submodule tests were asserting the linked-worktree suffix in the
status view; it is gone now, and the repo name still says which
submodule we're in.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A submodule's git dir doesn't live inside its working tree, and `git
worktree list` reports it by its git dir. Lazygit compares that against
the working tree path, so it recognizes neither the current nor the main
worktree, and the UI ends up claiming we're in a linked worktree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The scenarios describe their repo by its paths but leave the git dirs
empty, which no repo has. Unused for now; the loader is about to want
them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PipeCommands ran every command in its own goroutine, each doing
Start/read-stderr/Wait, with nothing ordering one goroutine's Start
against another's Wait. That ordering matters: StdoutPipe registers the
parent's read end in cmd.parentIOPipes, and Cmd.Wait closes those
descriptors when it returns. The next command's Stdin is that very
*os.File, and exec passes a user-supplied *os.File through untouched, so
Start hands the child whatever the fd happens to be at that moment. If
the producer finished and got reaped before the consumer's goroutine
reached Start, that fd was already closed, File.Fd() returned -1, and
the child was started with fd 0 closed -- reading nothing at all.
The only caller is the pre-2.35 fallback in SaveStagedChanges, which
pipes `git stash show -p` into `git apply -R`. Losing that race left
git apply with an empty patch, so it failed with "unrecognized input",
the following `git stash drop` never ran, and the user was left with a
stray stash entry. This turned up as a flaky stash/stash_staged on the
git 2.32.0 CI job; the newer-git jobs take the `git stash push --staged`
path and never reach this code.
Starting every command up front removes the race, and collecting stderr
into buffers lets exec's own copying goroutines do the work. That also
fixes two lesser problems in the same function: finalErrors was appended
to from several goroutines without synchronization, and a failed Start
was only logged, so a pipeline that never ran reported success.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pty teardown in Close runs on a background goroutine that doesn't
get to finish when lazygit is quitting: the process exits milliseconds
after the view buffer managers are closed. The job objects still cover
the clients -- KILL_ON_JOB_CLOSE reaps them when the process's handles
are rundown at exit -- but nothing reaps the conhost, so on Windows
builds whose conhost fails to run down on its own, quitting leaks one
conhost per live pty.
This is not a rare timing window: a diff longer than what has been
read keeps its git process (and thus its pty and conhost) running for
the entire time it is displayed, so that scrolling can read more.
Quitting while looking at a long diff is therefore the common case,
and with an external differ configured it leaks a conhost on affected
builds on almost every quit.
Fix this by having the gui's shutdown path wait synchronously for the
in-flight teardowns after closing the view buffer managers. A quit
signal makes the teardowns skip the conhost rundown wait -- the
conhost serves nothing once its clients are dead, and the exit must
not stall for its sake -- so the wait normally completes in
milliseconds, keeping quit as fast as before; a 2-second cap protects
the exit path even if a teardown wedges.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A pseudoconsole's conhost.exe is spawned by CreatePseudoConsole as a
child of lazygit, so it is not part of the job object that the pty
teardown kills. That is normally fine: a healthy conhost runs itself
down once the reference handle is closed and its clients are gone. But
conhost builds before the ConPTY overhaul that shipped with Windows 11
24H2 (confirmed on 23H2, build 22631) fail to complete the rundown
when a client attached after the close event was delivered and was
then killed -- the fate of exactly the clients the job kill exists for
-- and such a conhost lingers forever with no clients, at a rate of
about one per five fast commit navigations. These builds remain
widespread: all of Windows 10 (whose ESU tail runs into 2028, and
whose hardware often cannot run Windows 11 at all) plus pre-24H2
Windows 11 fleets.
Since Windows offers no way to obtain the conhost's pid or handle from
the HPCON, identify it by diffing lazygit's direct conhost children
around the CreatePseudoConsole call, serialized by a mutex so that two
concurrently starting ptys can't confuse each other's diff, and open a
handle immediately so that pid reuse is harmless. The teardown then
gives conhost a second to exit on its own before terminating it; on
healthy builds the wait succeeds and the reap never fires. If the
conhost can't be identified unambiguously, we simply don't reap, which
is no worse than before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stopping a pty task on Windows relies on ClosePseudoConsole, which
delivers CTRL_CLOSE_EVENT to the console's attached clients. But only
to those attached at that moment: when the user flicks quickly through
commits, a task is often stopped within the first few milliseconds of
its life, before the child has attached to the pseudoconsole. Such a
child misses the event and survives, running the entire diff to
completion in the background (spawning one external differ per changed
file) and keeping its conhost.exe alive; rapid navigation accumulates
these git/difft/conhost trees, and they outlive lazygit. Grandchildren
are affected too: git for Windows runs commands through a two-level
git.exe wrapper, so a single task has several attach windows, and a
grandchild spawned while the console is going down is orphaned even
when its parent got the event and exited.
Fix this by putting the child into a job object before it runs its
first instruction (created suspended, assigned, then resumed), so that
every descendant is in the job from the start; the teardown in Close
terminates the job right after initiating the pseudoconsole close.
There is no point in a grace period between the two: the close event
is not a graceful signal -- git and the common diff tools leave it to
the default handler, which calls ExitProcess at an arbitrary point --
so clients that received it are already dying, and the kill exists for
those that missed it. Killing at an arbitrary point cannot leak a
stale index.lock, because pty-rendered commands no longer take that
lock (see withPtyGitConfig in pkg/gui/pty.go).
The pseudoconsole close runs on its own goroutine because the kill
must not wait for it: on builds where ClosePseudoConsole blocks until
the console host exits (pre-24H2), the host keeps running as long as a
surviving client does, and that client only goes away through the job
kill; sequencing the kill after a blocking close would deadlock in
exactly the case the kill exists for.
KILL_ON_JOB_CLOSE doubles as a safety net: if lazygit exits without
running the teardown, the OS closes the job handle and reaps the tree.
In a harness that mimicked the stop path with randomized 0-120ms stop
delays, 3 of 30 process trees survived as orphans before this change;
none survive with it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub exposes a combined status for the head commit without requiring
individual check contexts. Include that rollup in the existing request
and startup cache so every consumer sees the same state without making a
second network request.
The fetch currently combines transport, JSON decoding, and model
conversion, which makes response changes difficult to verify without
exercising the network. Put the deterministic work behind a small parser
so later payload changes can be covered with raw GraphQL fixtures.
For a long time lazygit has used the term "custom pager" to refer to
what's really a "diff renderer". A pager is a program that allows you to
view output page by page (hence the name), e.g. less; lazygit's custom
diff renderers are not pagers. It used the term only because the feature
is implemented using git's GIT_PAGER env var, but that's an
implementation detail.
Rename the 'git.pagers' config to 'git.diffRenderers', and restructure
its elements while we're at it to make things clearer:
- Add a 'type' field to explicitly specify which type of diff renderer
it is (the two fundamentally different ones are 'stdinFilter' and
'extDiff').
- Add a third type, 'rawGit', which has an 'args' field that makes it
easy to use 'git --color-words' as a custom renderer
- Unify the old 'pager' and 'externalDiffCommand' fields to a single
'command' field for both types
Existing config files are migrated automatically.
Let the todo-move primitives take a distance instead of hardcoding a
single row, by iterating the one-row move in memory. Dropping a commit
several rows away thus rewrites the todo file once and, outside of an
interactive rebase, runs a single rebase rather than one per row.
CreatePseudoConsole and ResizePseudoConsole reject zero dimensions with
E_INVALIDARG, but we legitimately request them: the pty is sized after
the main view, and that view is zero-sized while hidden, e.g. in
full-screen mode with a side panel focused. Entering that mode while a
custom pager is configured therefore made StartPty fail (degrading to
unpaged output now that the fallback works), and resizing a live pty
from onResize would fail layout. The Unix pty accepts zero sizes, so
the clamp lives in the Windows implementation only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CreatePseudoConsole rejects zero dimensions with E_INVALIDARG, so
starting a pty sized after a hidden (and thus zero-sized) view fails.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cached git config runs its `git config` reads through raw
exec.Command calls, outside the pinned git command builder, so they
followed the process working directory. A cache miss on a stale
instance — one still in use by a refresh that crossed a repo switch —
would therefore read the new repo's local config while computing data
for the old one. Give the cache a directory, set once by NewGitCommand
right after it determines the repo paths (the object is created fresh
for every repo switch, so no cross-repo cache invalidation is needed),
and run every config command there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The refresh workers read a few files at paths relative to the process
working directory: the submodule config read of .gitmodules, the files
refresh's check for conflict markers, and the submodule stash's
existence check. Git commands are pinned to the repo their instance was
created for, but these Go file reads still followed the cwd, so a
background refresh crossing a repo switch would read the new repo's
files while computing data for the old one. Join them with the worktree
root of the instance they belong to. (Most git-state file reads —
working tree state, rebase todos, bisect info — already resolve
against RepoPaths and need no change.)
This also fixes the submodule stash's existence check for nested
submodules: it stat'ed submodule.Path, which is relative to the parent
module, against the repo root — now it uses the submodule's full path,
matching the stash command right below it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lazygit changes the process working directory when switching repos, but
work that is still in flight for the previous repo can keep spawning
git commands after the switch — most notably a background refresh. Its
model writes are already dropped by the repo generation guard, but its
git commands would now run against the new repo. That is wasted work at
best; at worst it surfaces spurious error popups (the behind-base-
branch computation failing with "no such ref" when the old repo's main
branch doesn't exist in the new one) and pollutes caches belonging to
the old repo's reusable state (e.g. MainBranches' existing-branches
cache), which the user sees when switching back.
Give the git command builder the directory of the repo it was created
for, and pin every command it produces to that directory. The pinned
directory and the process cwd are identical until a switch happens
(NewGitCommand chdirs to the worktree path right before creating the
builder), so nothing changes in the steady state; the pin only takes
effect for commands built through a previous repo's GitCommand instance
after a switch, which now keep addressing the repo they were built for.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deleting a nested submodule (and updating its URL) chdir'd the whole
process into the parent module, ran its git commands there, and chdir'd
back. Only those commands need to run there, and a process-wide chdir
leaks the parent module's directory into any command another goroutine
spawns during that window (e.g. a background refresh's). Set the
directory on the commands themselves instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The command log is supposed to show only commands initiated by the user;
these are commands that we run to get information for rendering, so they
pollute the log and are confusing.
The http.Client used for fetching pull requests had no timeout, so on a
network that silently drops packets a request could stay in flight
until the OS-level TCP timeouts kick in, which can take many minutes.
The fetch has no visible status, so nothing tells the user it is still
running; bounding it keeps the refresh's worst case short, and the next
refresh simply tries again.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The custom-patch git operations (move/pull/delete patch, and their rebase
continuations) run on worker goroutines and call PatchBuilder.Reset when
they've consumed the patch, clearing To and the fileInfoMap. Meanwhile the
UI thread reads that state every layout — the options bar and the mode
indicator both call Active() — so the reset raced the render.
Add a mutex. The map's entries are only ever touched on the UI thread, so
the lock only has to serialize the To field and the fileInfoMap pointer:
readers snapshot the pointer under the lock and iterate the local, and
getFileInfo drops the lock across its git diff I/O rather than holding it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
runAndStreamAux reads the stdout buffer (and, when output is suppressed,
the combinedOutput buffer) for its error message after handler.wait()
returns, but the goroutine that fills those buffers by draining the
command's output isn't awaited, so the reads raced its final writes.
Own the goroutine here rather than letting the onRun callbacks spawn it,
and join it before reading the buffers. The pty reader reaches EOF on its
own once the process exits, but the non-pty pipe never does, so its
handler now closes the read end to unblock the reader; the pipe is
synchronous, so by the time the command has exited all of its output has
already been read and nothing is lost. This also plugs the goroutine that
the non-pty streaming path previously leaked on every command.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
runAndStreamAux funnels a command's stdout and stderr into a single
cmdWriter (the command-log panel, or a buffer when output is suppressed)
from two separate goroutines: stderr through the MultiWriter set on
cmd.Stderr, and stdout through the onRun callback. Those goroutines
wrote the shared writer without any synchronization, racing on the
prefixWriter's prefixWritten flag and interleaving the two streams.
Wrap the writer so its writes are serialized.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
winPty.Close could block indefinitely, and it is called while holding
the global PtyMutex and while the task's onDone sync.Once is
executing, so blocking there wedges the task's entire cleanup chain:
the next NewTask call blocks on <-notifyStopped while holding
waitingMutex, every later task for that view queues up behind it, and
onResize blocks on PtyMutex — a full UI freeze. (Reported by a user
via go-deadlock's 30s watchdog; a regression from the ConPTY support
introduced for v0.63.0.)
ClosePseudoConsole is what blocks; before Windows 11 24H2 it can do
so in two ways. It flushes the client's pending output into the out
pipe, but a stopped task's scanner goroutine has already quit
draining, so with a client that's still producing output the flush
never completes; this can also wedge the background waiter's
closeHpc, which runs with the pipes deliberately left open. And it
waits for the console host to exit, but closing only delivers
CTRL_CLOSE_EVENT to the attached client without terminating it, so a
client that keeps running (git still computing an expensive diff, a
pager waiting for input) keeps the host alive arbitrarily long.
Run the teardown on a background goroutine so Close returns
immediately no matter which of these strikes, and within it close our
pipe ends before the pseudoconsole, without taking p.mu: breaking the
pipes fails a pending flush fast, which also unblocks a waiter
already stuck in one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
