The two findings from the interactive pass were folded back, which took the
loading-indicator gate to before the off-screen render — where the blanking
would otherwise arrive — and made the amend! for the origin reset
unnecessary, since the flag it needs now already exists by then.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deviation 6 still described the TaskOpts field as the shape PR 6 should
build on while deviation 8 said it was gone, so a reader hitting 6 first
would follow a design that no longer exists. Deviation 6 now describes the
final shape and what it means for PR 6, and PR 6's own commit 1 says the
same.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The regression it found retires the PR 6 note from deviation 6, and the
race fix establishes where unrelated bugs found during this work go.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of them constrain later work: TaskOpts.ResetOrigin replaces the planned
LinesToRead.ResetOrigin that PR 6 was going to extend, and the screenColMax
gap PR 1 had to fix is still live on the prototype branch that later PRs
transcribe from.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan assumed git would never emit the records, so PR 4 skipped the
advertisement for rawGit renderers and PR 7 listed "how should rawGit
entries decide the raw fallback" as undecided, with a static args-present
rule or a well-formedness gate as the candidates. git can emit them now,
which answers that question better than either candidate: probe a rawGit
entry like any other renderer and the fallback collapses to a single rule
for every renderer type.
Record the two traps as well, since neither is visible from the outside --
the advertisement has to precede newPtyTask's no-pty early return, and the
probe's cache signature has to include the args.
Say explicitly that none of this waits on git upstream. The probe asks the
installed git what it can do, so a stock git degrades to exactly the
behaviour of a non-conforming renderer, and a fork covers the users who
want the feature if the patch is never accepted.
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.
A rawGit diff renderer needs no pty -- git renders the diff itself, and
only a pager needs a terminal to be spawned at all -- so newPtyTask hands
that case straight to newCmdTask. But the OSC1717 advertisement was set
forty lines further down, past that early return, so git was never asked
to annotate its output and the word-diff renderer we just started
trusting emitted no records.
Set it before the branch, next to LAZYGIT_COLUMNS, which is there for the
same reason. Nothing else changes: a renderer that doesn't know the
variable ignores it, and git says nothing for the formats it doesn't
annotate, which is every format a rawGit renderer without word-diff
arguments produces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
We stopped updating these notes once the real work moved to
focused-main-view-notes.md and the production plan, but two things in them
have since become actively misleading rather than merely stale: they say in
five places that the protocol is content-lines-only, and they use the env
var name from two renames ago.
Mark the document historical, name the spec as the authority, and correct
§11's outcome banner in place rather than rewriting it, so the reasoning
that led to dropping `f`/`h` is still on record next to the reason it was
reversed -- which is the very cost that banner judged acceptable.
Don't touch the `EMIT_OSC1717_METADATA` spellings further down: they are
what those prototypes were built with, and the status block now says so.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The prototype was rebased onto master past #5854 (gocui mouse gestures)
and #5870 (diff-renderer config rework), so the SHAs quoted in the plan
now resolve only on the pre-rebase copy at
fold-staging-functionality-into-main-view-plan; note that in §1 and in a
dated log in §10.
PR 3 is done: #5870 landed the rename with a bigger config restructure
than planned (per-entry 'type' field, unified 'command', 'rawGit' type,
DiffRendererConfigManager). Rewrite the PR 3 section as a summary of
what landed and its consequences, key the affected decisions in PRs 4-7
off GetDiffRendererType(), add rawGit as a new case for PR 7's raw
fallback, and resolve the open question about the 'pager:' field name.
PR 10 commit 3 is superseded: the rebase dropped the press-only
modifier fix, and #5854's gesture model makes press-only insufficient
anyway; the re-implemented prototype commit carries the press-time
modifiers through the whole gesture, and the plan now points at that
shape.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A click is reported on the button press, but the press never set the
event's modifier, so a modified click (alt/shift/ctrl+click) reached
handlers stripped of its modifier, indistinguishable from a plain
click. An earlier fix for this ("Carry the keyboard modifier on mouse
click events") was lost while rebasing onto master's mouse gesture
handling (#5854) — and that work also changes what the right shape is:
it isn't enough for the modifier to ride the press alone.
Drag events and the release are delivered to the view that owns the
gesture, and bindings match modifiers exactly. If the press carried alt
but the drags carried plain ModMotion, an alt-press that no binding
consumed would start matching unmodified drag bindings mid-gesture
(drag-select), and the release of a modified gesture would look like a
plain one. So snapshot the modifiers at press time and stamp them on
every event of the gesture: the press, each drag event (combined with
ModMotion), and the release; modifier changes while the button is held
don't alter the gesture. TestMouseReleaseDoesNotKeepPressModifiers
asserted the opposite for the release and is inverted accordingly.
As before, this means a modified click that nothing binds is a no-op
rather than silently acting as a plain click.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review clarified that "deferred" in the gap table meant "not addressed
while prototyping", not "optional". Five items are mandatory:
- IsSingleHunkForWholeFile (PR 5 commit 3): regression vs master
otherwise. Single-file-ness comes from the side panel selection; the
single-block computation's lean is patch-space over the raw diff
fetched at focus, with ReadToEnd-on-focus as the accepted fallback.
- Nav targets beyond the loaded content (PR 5 commit 6): ReadToEnd and
re-scan like openSearch; free if the above reads to end on focus.
Covers the jump-to-file menu too.
- Copy (PR 5 commit 9): reframed — copy the corresponding raw diff
lines, never the renderer's output. Dissolves the can't-tell-if-the-
renderer-preserves-the-prefix-column problem instead of solving it.
- Deleted-file MD-vs-D (new PR 7 commit 5): staging a deleted file's
entire content stages the file deletion itself.
- Renderer switch shifting the inclusion checkmarks mid-build (new
PR 8 commit 10): recompute from the new buffer at render completion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A parallel session committed the zero-width OSC-record carrier
(fe8022827) and diagnosed the secondary patch pane's removal as broken
under difftastic: the ordinal bridge assumes displayed change lines
match the patch's change lines in order and multiplicity (difftastic
reorders d/a per hunk and collapses modification rows), and an external
diff tool over the temp trees emits the literal a/-b/ paths in its
records, so the patch-builder file lookup no-ops. PR 8 commit 7 now
prescribes the identity bridge (all payloads per row, matched against
identities from the raw temp-tree diff, k-th match -> included[k]) and
records the path-leak as an open sub-item to decide with the user; PR 4
commit 1 references the now-committed carrier, which the bridge relies
on for the d half of collapsed modification rows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A rendered row can carry several diff-metadata records back-to-back with
nothing painted in between: difftastic's per-hunk banner emits the
file's f immediately followed by the hunk's h, and a modification row
collapsed to a single column emits its d immediately followed by its a
(spec 6.1/6.2). The escape interpreter kept only one accumulating
payload and reset it when the next record started, so every record but
the last was silently dropped -- the banner resolved as a bare hunk
header, and staging a collapsed modification row staged only the
addition half.
Orphan an unconsumed payload instead of dropping it: when a new record
starts (or the line ends) before any cell consumed the current payload,
hand it to the write loop, which materializes it as a content-less
zero-width carrier cell -- the same trick finishLine already used for
delta's metadata-only blank lines, now generalized. Carrier cells hold
their place in left-to-right payload order and are invisible: drawing
paints a transient space that the next cell immediately overwrites at
the same x, and their zero width keeps wrap accounting unchanged.
DiffLineMetadataPayloads thus reports every record of such a row, so
ChangeLinesInViewRange stages both halves of a collapsed modification
row; DiffLineMetadataInLine reports the first (the f of a banner, the d
of a collapsed row), matching the two-column convention.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two corrections from review:
- "Pager" is retired in favor of "diff renderer" throughout (the spec
already made this change); a new mini PR 3 renames the pagers config
to diffRenderers with migration and docs, and the remaining PRs are
renumbered accordingly.
- The locked-decision list wrongly said concurrency stays mutex-based;
the main-thread-mutation rework has landed on master and the
prototype is rebased on top of it. The plan now states the landed
threading contract that all PRs must honor, and PR 1 gained a
re-validate-against-master preamble. Its reset-after-CopyContent
commit was verified still needed (master's CopyContent still copies
the source origin); the two locking fixes were reshaped onto the
viewLines readers master actually has, since the prototype's
HyperLinkInLine guard belongs to the dropped hyperlink backend.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The prototype is complete and signed off; this is the plan for
re-implementing it as a stack of clean PRs off master. It divides the
work into ten PRs (grouped for release-notes value as much as for
technical cohesion), outlines the commits of each, records the scope
decisions made in the planning session (panels removed, enter/dive
gesture dropped, stacked PRs within one release, both extras in scope,
nav/preserve as early standalone PRs), lists everything from the
prototype that must NOT be ported, and carries the sign-off matrix,
separate-lists compatibility seams, and known-gap dispositions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The spec regained file-header (f) and hunk-header (h) records: f never
carries a line number, h always does (the first line of the hunk it
heads). Accept them in the OSC metadata backend, so a conforming pager's
header rows resolve to the same DiffLineFileHeader/DiffLineHunkHeader
identities the buffer parser already reports for raw diffs.
With header rows located, next/previous file navigation and the
jump-to-file menu land on a file's header row, and header rows become
usable scroll-restore anchors. The consumers need a few adjustments:
- File navigation used to reach a file's top by backing up over the
untagged rows above its first located row. With tagged headers that
overshoots onto the blank separator row above the file header, so drop
the back-up (backUpOverHeader) and land on the first located row
itself: the header for any conforming source, or the first content
line under a pager that leaves its headers untagged — an accepted
degradation for non-conforming pagers, now that the spec makes f/h
mandatory.
- SamePatchLine now requires headers to match headers of the same kind.
A hunk header shares its line number with the hunk's first content
line (and a file header shares "0" with a deleted file's hunk header),
so without this a position restore aiming at one could land on the
other. This also applied to raw diffs before, but headers used to be
unlikely restore targets; now that navigation deliberately lands on
them, the ambiguity would bite.
- Editing a file-header row opens the file without jumping to a line,
like pressing edit on a whole file in a side panel. (It used to open
at line 1 for raw diffs, where headers resolved already.)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
n / N step through the files of a multi-file diff one at a time, which
is tedious when the diff spans many files. Add `f`, which pops up a menu
listing every file in the diff — in the order they appear, as
repo-relative paths — so you can jump straight to one.
Picking a file reuses the exact landing logic n / N use (navigate to the
file's first row), computed from the same backUpOverHeader machinery
AdjacentFile uses, so the menu and the step keys agree on where each
file begins and land identically.
The menu title and the keybinding description are hard-coded English for
now; this is a prototype.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The focused main view now plays the role the staging panel used to,
so the working-tree commands the staging panel offered — commit (and
its variants) and find-base-commit-for-fixup — need to be reachable
there as well.
Gate them to when the focused main view actually shows the working-tree
diff (DiffMainViewTypeStaging), which is exactly the role the staging
panel filled. Over a commit's or stash's diff these commands would
operate on the working tree, unrelated to what's on screen, so the keys
are a no-op there and the bindings don't clutter its keybinding menu.
The gate is re-checked on each press rather than captured at
registration time, since one keybinding set serves the main view over
every panel. That requires reading the panel beneath the main view from
GetKeybindings (to decide whether to show the descriptions), which runs
for off-stack contexts too — at startup and during cheatsheet
generation — so a panic-safe IsInStack guards the NextInStack lookup.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Records the two rename-handling gaps the rebase onto f84ada494 leaves
for productionization: the previousPath="" conflict resolutions in the
focused-main-view patch code, and the failing renamed_file_whole e2e
(a view-rendering regression in the prototype's patch package, not a
build regression and not the previousPath issue).
Co-Authored-By: Claude Opus 4.8 (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).
The inclusion gutter (the ✓ markers on the commit diff showing which lines are
in the custom patch) is painted on the Normal pane but is an affordance of the
whole focused-main-view pair. It was gated on the Normal pane specifically being
current, so tabbing to the secondary (custom-patch) pane hid it — even though
both panes are visible and you're still building the patch.
Show it whenever either pane of the focused main view holds focus, finding the
side panel beneath whichever pane is current. GetOnFocusLost now re-evaluates the
gutter (rather than unconditionally hiding) so it persists across a pane switch
but still hides when focus leaves the pair; the new context is already current by
then, so it decides correctly and doesn't flicker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Staging a hunk from the focused main view advances the selection to the next
hunk, because staging removes the acted-on lines from the diff so the preserved
change-line ordinal lands on the next change. Toggling a hunk into a custom
patch left the selection sitting on the just-toggled hunk instead — the toggle
doesn't change the diff (only the inclusion set), so the same ordinal lands back
where it was, and you had to navigate by hand to build a patch hunk by hunk.
Give the toggle the same feel by advancing the reveal ordinal past the toggled
change lines (RevealSelectionAfterStaging gains an advanceBy arg, fed the toggled
change-line count). Staging and removal still pass 0 (their lines are consumed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
Discarding lines from a commit via the focused main view installed its own
revealSelectionAfterPrimaryAction before the rebase. That rebase rewrites
the commit, so the selection-preserve net now re-establishes the selection
as the diff re-renders — with the same anchor (the selection's first line),
making this install redundant. Files-discard keeps its own reveal: its diff
command is stable, so the net never fires there, and it has staging's
focus-follow to the secondary pane besides.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dropping a hunk with `d` re-establishes the focused main view's selection
on the next surviving change, which feels great. But other operations that
rewrite the commit under the focused main view — moving a custom patch out
into the index, undoing right after a discard or a patch move — don't run
through the focused-main-view action handlers, so nothing was preserving
the selection. The stale gocui selection was left painted over the new
content, often as a large, now-meaningless range.
Rather than teach every such command to capture and restore the selection
(move-patch, undo, redo, and any future one), the focused main view now
preserves it itself, by its change-line ordinal, as the diff re-renders —
the command-agnostic counterpart of revealSelectionAfterPrimaryAction. The
diff side panels call it from their render-to-main before triggering the
render, so the restore rides the re-render.
It stands down unless the focused main view is current and shows a
selection, no precise restore is already pending (escape / post-stage
reveal / context-size place the selection more precisely), and the diff
command is actually changing. That last gate matters: a plain background
refresh re-renders the same commit's diff unchanged, and there the
selection — range and all — must be left alone; only a command change
(e.g. a rebase rewriting the commit's hash) means the content moved.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SHAs go stale on every history rewrite of this throwaway branch; describe
commits by what they did instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The driver fix is a no-op for the old staging panel, not a tightening: that
panel anchors its drag at the click on mouse-down, so it never exposed the
bug.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Click-and-drag in the focused main view turns a hunk selection into a range,
but it was anchored at the change block's far end — where selecting a hunk
leaves the range anchor — rather than the clicked line. And on a context
line, where the click leaves no range anchor at all, dragging just moved the
single selected line instead of opening a range. Remember the line each
mouse-down lands on (the click can show a whole hunk, so it can't be read
back from the view) and, as the drag proceeds, anchor the range there while
the cursor end follows the mouse as gocui already moves it. Works for a click
that focuses the view and for one while it's already focused, on change and
context lines alike.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When the diff re-renders for a new pager (or a changed context size),
PreserveDiffPositionOnRerender restored the cursor by patch identity but left
the selection's other end — the range anchor — pinned to its old view line. A
pager that restructures the diff (delta side-by-side is the clearest case)
then left the selection spanning the wrong range of patch lines. Remember the
far end by patch identity too and put it back the same way, so the selection
covers the same lines however the new pager lays them out. One mechanism now
restores both ends, covering hunk and range selections alike; a single-line
selection still needs only the cursor.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clicking the staged/unstaged pane you weren't focused on selected a single
line the first time, even in hunk mode, because that pane's select mode was
still its default until it had been focused once (tabbing to it and back was
the workaround). Seed the clicked pane's mode from the one we're leaving so
the very first click there behaves like every later one.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Once in hunk mode, clicking another change line reset the selection to a
single line, so you had to press `a` again for each block you wanted to
stage (a behaviour the main view inherited from the staging and patch-
building panels). Preserve hunk mode across clicks instead: a click on a
change line re-selects that whole block, while a click on context — or any
click when we weren't in hunk mode — drops to a single line, where the click
points precisely (e.g. to edit it).
The two click handlers shared an identical selection body, so unify them
into one helper and make the change in a single place.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Focusing the main view by keyboard already selects a whole change block in
hunk mode; a click, though, only ever placed a single-line selection, so the
common "click the diff to stage this block" gesture still needed a follow-up
`a`. Now a click on a change line selects that line's block too. A click on
context still selects just that line, since the click points at it precisely
(e.g. to edit it with `e`).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
