Mouse clicks on a view other than the focused popup panel are normally
swallowed by the ShouldHandleMouseEvent gate, so a registered click
handler can't run while a modal is up. Hyperlink clicks already dodge
this by being handled in an earlier phase; generalize that to ordinary
mouse bindings via a HandleWhenPopupPanelFocused flag, dispatched before
the gate. No binding sets it yet, so behavior is unchanged.
This is what lets a click on the main view stay live behind a popup
(e.g. opening a diff line in the editor while the commit-message panel is
in front), the way the wheel already scrolls it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The terminal-allocation audit settled the protocol's OSC number on 1717 (unused
by every surveyed terminal — see diff-line-metadata-osc-spec.md), retiring the
456 placeholder. Rename the host side to match: the gocui carrier that accumulates
and reads back the sequence, the parser, and the handshake env var the pager
subprocess is given (now EMIT_OSC1717_METADATA). Flip the design notes and spec
from 'rename pending' to done.
The delta and difftastic emitters are renamed in their own repos.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the OSC number from the 456 placeholder to 1717. There is no central
registry for OSC numbers, so the audit reduces to: pick a high, distinctive
number that no real terminal *acts on* — an unknown OSC is skipped harmlessly,
but a recognized one can fire a visible side-effect (OSC 555 flashes foot, OSC
777 raises a desktop notification), and the metadata flows through real
terminals whenever the pager runs outside a host. Audited the live OSC
allocations of xterm, VTE, kitty, foot, WezTerm, iTerm2, Windows Terminal,
Ghostty, VS Code, ConEmu and urxvt; 1717 collides with none and sits in the
empty 1400-5000 band (only iTerm2's 1337 is nearby).
Write the spec as a standalone draft to circulate to pager developers for
feedback — separate from the internal session notes, with motivation, the v1
wire format, the env handshake, semantics, emit rules (including the per-row
wrapping correction and the side-by-side two-records-per-row case), and the
known v2 candidates (both-numbers-always, the difftastic token-vs-line
mismatch). The prototype code still emits the 456 placeholder; the 456->1717
rename across delta/difftastic/gocui/lazygit and the EMIT_OSC<n>_METADATA env
var is a tracked follow-up, not part of this commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Record session 8 — preserving scroll/selection when cycling pagers, as
the seventh consumer of the identity-based restore. Captures why both the
plain-pager (GIT_PAGER, line-number kept) and externalDiffCommand
(command changes, reset to top) cases needed it, the "main"-window-only
scope, the graceful no-op fallback, and that the side-by-side<->inline
structural win awaits the §17.4 row+column resolver.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cycling pagers re-renders the diff into the main view. Until now that
either lost the scroll position outright — an entry with its own
externalDiffCommand changes the actual git command, so the re-render
reset the view to the top — or kept it only by raw line number, which a
plain pager swap got for free because the git command was unchanged.
Raw line number is the wrong anchor: two pagers can structure the same
diff very differently (side-by-side vs inline), so the same screen line
means something different afterwards. Reuse the identity-based restore
(PreserveDiffPositionOnRerender, already driving the -U context-size
consumer) to re-anchor on the same patch line instead. That both keeps
the position meaningful when the structure changes and covers the
externalDiffCommand case the line-number approach couldn't.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The same pager-level wrapping gap found in difftastic (§10.8) was fixed in delta
too. Delta only wraps in side-by-side mode, so the bug was SxS-only; each wrapped
continuation row now re-emits the record of the primary line it continues,
without advancing delta's line-number counters (so subsequent lines keep correct
numbers). Update §9.3, §10.8, and the §17.1 correction note accordingly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Testing the difftastic prototype in lazygit exposed that the "wrapped
continuation rows carry no attachment" convention is a bug whenever the pager
itself wraps a long line (difftastic side-by-side, delta with wrap-max-lines):
each wrapped row is a distinct host buffer line, so e/enter and hunk navigation
break on the un-tagged continuation rows. The fix -- emit the line's record on
every output row, not just the first -- was applied to difftastic; delta needs
the same. Record the underlying distinction (terminal/host wrapping vs
pager wrapping) and state the spec rule positively (§10.8).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
difftastic is now prototyped as an OSC-metadata emitter in both its modes
(side-by-side and inline), the one pager mechanism #1 categorically cannot
serve. The notes capture what was built and, more importantly, what it reveals
about the v1 format: it holds, but difftastic's token-granular structural model
surfaces a model mismatch the unified-diff pagers hid -- an aligned row whose
old side has no novel tokens is faithfully a context cell plus an addition cell,
with no deletion record for the old side. Also resolves the "how many regions
per row" open question (two, not N) and amplifies the §17.3 "context/added carry
no old-line" limitation, since difftastic's old/new line numbers diverge with no
constant offset.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The OSC-metadata prototype now extends to difftastic (the categorical #2-only
pager), checked out at /Users/stk/Stk/Dev/Builds/difftastic.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A parallel session added per-line OSC metadata to delta's side-by-side mode
(in the delta repo, not lazygit) and recorded its findings here as §17, with
the §15 roadmap updated to mark step 3 done.
The headline, which feeds the OSC spec draft: side-by-side needs no change to
the v1 wire format — `type` already implies the column (a→right/new, d→left/
old) and context is symmetric, so no side discriminator is warranted. This
corrects §15's earlier guess that side-by-side would likely add payload, and
leaves the OSC number as the spec's only remaining open item. Also flagged for
productionization: the host reader becomes row+column→identity (a changed row
carries two records), which the §16 consumers haven't yet been exercised
against.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hunk/file navigation (#4) and -U-context-change scroll preservation (#5) are
built; document the design, decisions (main-view-only scope, offset-preserving
placement, file nav landing on the file top, the visibility guard) and the
pending interactive sign-off.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add file and change-block ("hunk") navigation to the focused main view,
mirroring the staging view's hunk keys: `<left>`/`<right>` jump to the
previous/next hunk and `n`/`N` to the next/previous file. A "hunk" here is
lazygit's notion — a run of consecutive added/deleted lines separated by
context, not a git `@@` section — matching what the staging view jumps
between.
This is a consumer of the diff-line primitive in its forward direction:
resolve each rendered row's patch identity, then scan for the next/previous
change block (by the line type) or file boundary (by the path changing). The
file scan lands on the top of the neighbouring file even when a restructuring
pager leaves the header rows untagged, by backing up over them from the
file's first identifiable row — which is impossible without the per-line
metadata once the pager stops emitting a parseable unified diff. The
boundary arithmetic is pulled out into pure functions and unit-tested.
The anchor is the selected line if a selection is showing, else the top
visible line. With a selection we move it to the target and scroll it into
view, like the staging view; with none we stay in scroll mode, bringing the
target to the top without creating a selection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Increasing or decreasing the diff context size (the `{`/`}` keybindings)
re-renders the diff with a different `git diff -U<n>` command. Because the
command key changes, the render reset the main view to the top — losing the
spot the user was reading, which is exactly the spot the context change is
about.
Preserve it instead, reusing the identity-based restore built for the escape
path. This is its sibling consumer: capture the lines around the anchor (the
selection, or the top visible line when there's none) as restore candidates,
and after the re-render land on the nearest one that survived, put back at the
same screen row. Prefer the anchor line itself, falling back outward only when
it didn't survive: a context line vanishes when the context size shrinks,
whereas additions and deletions always survive, so expansion stops at the
first change line in each direction and the candidate list always contains a
survivor. Landing on the nearest survivor keeps scrolling to a minimum, and a
context line that is still in the patch stays put (or stays selected).
This generalizes the shared restore helper to take a prioritized candidate
list (the escape path passes a single candidate). It covers the focused main
view and every side panel's diff, since they all render into the same "main"
view through the same path. A showing selection is re-established on the landed
line; otherwise the view stays in scroll mode.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The escape restore's mechanism — set a RenderRestore that scans the
re-rendering content for a target patch identity and, once it loads,
positions the view on the matched row — is about to gain a second caller:
preserving a diff view's scroll/selection when its -U context size changes
re-renders it (the sibling consumer of the diff-line primitive). That caller
positions the row differently (put it back where it was, rather than scroll to
and select it), so split the positioning out behind a `place` callback and
keep the scan/swap machinery shared.
Behaviour-preserving: RestoreFocusedMainViewOnEscape passes the same
FocusPoint-and-select closure the inline Apply used.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a main view re-renders content different from what it last showed, the
scroll resets to the top. That reset fired synchronously when the task started —
but with the off-screen render the previous content stays displayed until the
swap, so resetting the origin up front scrolled that still-visible content to the
top before the new content replaced it: a distracting jump when switching commits
(or any item) while scrolled down.
Defer the reset to the task's first paint (the swap), alongside the restore that
already runs there: the previous content stays at its scroll until the new content
takes its place, then the new content appears at the top. A same-content re-render
keeps its scroll (no reset); a restore places the scroll itself. The "loading..."
indicator path also resets the origin now, since it clears the previous content to
show the message and must put it at the top.
The reset moves out of NewTask (it no longer needs the task key or the pending
restore for this) into the read loop, driven by LinesToRead.ResetOrigin, which the
cmd/pty wrappers set from the key comparison the reset used to do. The manager's
onNewKey callback is renamed resetOrigin to match its now-decoupled timing.
onMouseMove (and findHyperlinkAt, which it calls) read v.viewLines without
holding writeMutex, unlike every other reader. They run on the event-handling
goroutine, so a re-render on the task goroutine can shrink or rebuild viewLines
between onMouseMove's bounds check and findHyperlinkAt's indexing, causing an
out-of-range panic (observed: "index out of range [60] with length 0" while
hovering during a diff re-render).
Take writeMutex for the duration, like the other viewLines readers do, so the
check and the access see the same slice. Pre-existing, but the off-screen
re-render rebuilds viewLines on the task goroutine more often, widening the window.
§14: the escape restore now anchors on a patch identity scanned from the loading
re-render (items 1+3 of the part-3 plan, which collapsed into one mechanism), the
partial §12.2 routing fix, and the analysis for the deferred pieces (the (b)
no-clobber lever's interaction with the entry origin reset, NormalSecondary
routing, the hyperlink-backend match limitation, the O(n^2) scan). Records that
interactive sign-off — including the still-pending session-5 scrollbar/stopped-task
checks — remains, since the agent couldn't drive the TUI.
Diving into staging from a focused main view records a snapshot so escape can
return there. But it was recorded only on the half we entered (unstaged), and
staging the last unstaged hunk moves the selection to the staged half
(RefreshStagingPanel pushes StagingSecondary when the unstaged state goes empty).
Escaping from there found no snapshot and fell back to the files panel, instead
of returning to the focused main view.
Record the snapshot on both staging halves at entry, and clear both on escape.
Escaping from either half now returns to the focused main view, and the
identity-based restore lands on the line the explorer ended up selecting — which,
with the last unstaged hunk gone, is shown in the main view (the file now has only
staged changes, so no split). Routing the split-and-tabbed-to-staged case to the
secondary focused main view is left for follow-up; see focused-main-view-notes.md
§14.3.
Escaping a patch explorer (staging / patch building) back to the focused main
view it was entered from used to replay a numeric scroll position and selection
index captured on the way in. But the reason to escape after staging or dropping
a hunk is that the content changed, so a saved index points at the wrong line —
and the host auto-advances the explorer's selection to a still-valid line anyway,
which is the line the user actually cares about returning to.
Restore by *patch identity* instead. On escape, read the (file, type, source
line) the explorer currently has selected, then have the main view's re-render
land on the row that matches it: scan the incoming content as it loads (the
inverse of the diff-line primitive), and once the matching row plus a screenful
below it have loaded, swap the off-screen render in and scroll to / select that
row in one step. FocusPoint with scrollIntoView centres the row only if it's
off-screen, so the common unchanged-content escape — where the row is already
where it was — doesn't move at all. If the line is gone (the content really
changed), nothing is forced.
This generalizes the scroll restore from a fixed origin to a predicate
(RenderRestore: FirstPaintReady decides when the saved position is reachable,
Apply re-establishes it), folding the separate selection restore into the same
first paint — so it no longer rides a post-load callback that could fire early.
The restore also now survives task replacement, which the numeric version did
not: a periodic refresh can stop the escape's re-render before it first-paints.
The pending restore is held on the buffer manager and is *not* cleared when a
task starts, so the replacement task picks it up. It is not gated on the command
key — staging the last unstaged hunk re-renders `git diff` as `git diff --cached`,
a different command, yet the line to land on is still in the new content — but
validates itself: the scan finds the target line only when the content still
contains it, so applying it to a different item is a harmless no-op. A task
clears it once it has applied it (found or not), so it lives for exactly one
re-render. Because the restore is anchored on content identity and is idempotent,
"survive replacement" and "restore by identity" are one mechanism, not two.
With the identity in hand the snapshot no longer needs the captured scroll/index;
they're derived from the explorer's live selection.
The escape restore (next commit) finds the row in a re-rendering focused main
view that matches a target patch identity, and it has to do so while the content
is still loading — i.e. against the off-screen buffer, before it is swapped in,
since the displayed buffer still shows the previous render.
Add the two primitives that scan needs:
- OffscreenDiffLineContents exposes the per-line diff material (text, metadata,
hyperlink) of the rows read so far into the off-screen render, so the resolver
built last commit can run against the incoming content.
- ViewLineForBufferLine maps a matched (unwrapped) buffer line back to the first
view line that renders it — the inverse of BufferLineForViewLine — so the
restore can scroll to and select that line once the render is swapped in.
The diff-line primitive (recover a rendered row's patch-space identity) is about
to gain a second, inverse consumer: the escape restore scans a focused main
view's rows as it re-renders, looking for the row that matches a target patch
identity. That scan runs over the *loading* off-screen buffer, not the displayed
view, so the resolver can't be tied to the displayed view's per-view-line readers.
Pull the three backends (OSC metadata, buffer parse, lazygit-edit hyperlink) onto
a single buffer-agnostic resolver that takes a snapshot of a diff's per-line
content — text, metadata and hyperlink per unwrapped buffer line — and the line
to resolve. The forward consumers (click/enter/edit/PR) feed it a snapshot of the
displayed buffer (gocui.DiffLineContents) after mapping the wrapped view line to
its buffer line; the upcoming scan will feed it the off-screen buffer's loaded
rows. Behavior is unchanged.
When a task is stopped to make way for a newer one, stopping closes
opts.Stop, and the scanner goroutine then closes lineChan. The read loop's
select between those two channels is therefore non-deterministic: it can
land on the closed lineChan (ok == false) instead of the opts.Stop case,
sending a stopped task into the end-of-input branch.
There it runs the full finalize — swapping its half-read off-screen buffer
in, applying the saved scroll, clamping the origin to the truncated
content, and clearing the loading flag — all of which corrupt what the
incoming task is about to render. The most visible symptom is a brief frame
of truncated content with the scroll yanked to the top, seen when re-renders
overlap rapidly (e.g. the periodic background refresh re-rendering a main
view faster than it can load, very easy to hit under LAZYGIT_SLOW_RENDER).
The underlying bug predates the off-screen render (the EOF branch always
clamped the origin via onEndOfInput), but that change made it far worse by
also swapping a truncated buffer into the display. Fix it at the source: in
the EOF branch, check whether we were stopped and, if so, bail out like the
explicit stop case, leaving the view entirely to the task that replaces us.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A cmd/pty re-render used to overwrite the displayed buffer from the top
down as lines arrived, relying on keeping the previous render's view-line
tail to avoid a blank frame. That left the view showing a mixture of old
and new content while loading, and any reader (draw, the diff-line
mapping, clicks) could observe a half-written buffer at the wrong scroll —
the §11 Race A flicker and the §8 stale-tail mapping both came from this.
Instead, build the new content in a second, off-screen viewBuffer: until
the task has read enough to paint, writes go there and the displayed
buffer — and so everything every reader sees — is left untouched. Once the
task reaches its first-paint point (InitialRefreshAfter, or EOF for short
content) it swaps the off-screen buffer in atomically and applies the
saved scroll in the same step, so the view jumps straight from the
previous render to the new one with no intermediate frame. Subsequent
lines append to the now-displayed buffer.
Swapping at the first-paint point means the displayed buffer is only a
viewport tall when it appears and then grows as the rest streams in toward
the count needed for an accurate scrollbar. The scrollbar is sized from the
displayed buffer's height, so left to itself the thumb would shrink and
snap back during that growth (most visibly: the files panel's periodic
refresh making the thumb jump while scrolled down). The total height the
scrollbar needs is a strictly later quantity than the viewport-fill paint,
so no single early swap can have both right. FreezeScrollbarHeight therefore
records the view's height when a load begins and the scrollbar is held there
— growing only if the new content turns out taller — until the load ends; a
synchronous render superseding the load releases it. This mirrors the layout
clamp, which already ignores the partial content height while a view loads.
With the swap doing a wholesale replace, refreshViewLinesIfNeeded can
truncate the view lines to the current buffer: there is no longer a
half-loaded shorter buffer whose tail we must keep showing, so the stale
tail (§8) never forms. clear()/Reset() abandon any in-progress off-screen
render so a synchronous SetContent after a stopped task writes to the
display.
This replaces the holdViewLines and freshViewLineCount patches reverted in
the previous two commits with one mechanism. The swap holds writeMutex for
now; it could later move to the main thread. Flicker behaviour still needs
interactive verification (LAZYGIT_SLOW_RENDER + a real pager).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts the freshViewLineCount stale-tail guard (§8), restoring the
demonstrate-the-bug test state. The upcoming off-screen render rebuilds
the displayed buffer wholesale on swap and lets refreshViewLinesIfNeeded
truncate, so the stale tail never forms — a cleaner fix than tracking a
fresh-count. Removing the guard on its own keeps that change focused; the
bug it guarded against is re-fixed by the truncation in the next commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts the holdViewLines flicker patch (Race A). It is about to be
superseded by rendering a re-render into an off-screen buffer and swapping
it in atomically, which keeps the displayed buffer (and so every reader)
untouched until the new content is ready — a cleaner mechanism than
suppressing the view-line rebuild. Removing it on its own keeps that
upcoming change focused.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
write, writeCells, makeWriteable, parseInput and
autoRenderHyperlinksInCurrentLine produced cells into v.buf; move them onto
viewBuffer so they can write into any buffer, not just the displayed one.
The display-side effects that don't belong to content production —
tainting, clearing hover, updating search positions — stay behind in the
View.write wrapper, which delegates the actual writing to v.buf.write(v).
Render config the writer needs (Editable, colors, width, tab width,
hyperlink auto-render) is read from the passed View. Behaviour-preserving:
the wrapper still always targets v.buf.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The fields that make up a view's content and the act of writing to it —
the cell buffer (lines), the write cursor (wx/wy), the escape-sequence
decoder (ei) and the held-newline flag (pendingNewline) — were loose
fields on View. Bundle them into a viewBuffer struct that View holds by
pointer. This is a behaviour-preserving prep refactor: every access just
goes through v.buf now. It sets up rendering into a second, off-screen
viewBuffer that can be swapped in atomically, so an async re-render never
exposes a half-written buffer to readers.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When escaping to a focused main view scrolled down, the re-render keeps
the previous content as a placeholder and the task scrolls to the saved
position at its first paint. But the task's writes mark the view tainted,
so any layout pass landing between the first write and the first paint
rebuilds the view lines from the half-loaded buffer and draws them at the
placeholder's scroll — a brief frame of the wrong content at the wrong
scroll before it snaps into place. Intermittent, and only visible when the
load is slow enough for a layout pass to fall in that window.
Give the view a hold: while set, refreshViewLinesIfNeeded keeps the
current view lines instead of rebuilding from the buffer, so the view goes
on drawing the coherent placeholder. The re-render task sets it only when
restoring a scroll position (so the normal load-from-top case is
untouched) and releases it at its first paint, which applies the saved
scroll in the same step — so the loaded content appears at the restored
scroll with no intermediate frame. While held, the displayed view lines
need not match the loading buffer, so the view-line→buffer-line mapping
reports no result.
This needs interactive verification (LAZYGIT_SLOW_RENDER + a real pager);
see focused-main-view-notes.md §13.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Escaping to a focused main view restored the selection by scheduling, on
the next UI tick, a ReadToEnd whose callback re-selected the saved line.
But ReadToEnd fires its callback synchronously when the manager has no
live read channel, and the re-render task triggered by the push creates
that channel later, inside its own goroutine (after stopping the previous
task). If the UI tick won that race, the restore ran before any content
was loaded, FocusPoint no-oped against the unloaded line, and the
selection was silently dropped — intermittently, and more often under
load.
Thread the restore through the task instead: a thenForNextTask hook on
the buffer manager, folded into the next cmd/pty task's initial-read Then,
mirroring scrollToOriginYForNextTask. It runs once the task has read
enough to place the selection, and can't fire before the task exists. The
scroll restore already applies at the task's first paint, which precedes
the initial read's end, so the origin is in place when the selection is
restored.
This needs interactive verification (LAZYGIT_SLOW_RENDER + a real pager);
see focused-main-view-notes.md §13.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin the two escape-restore races to concrete mechanisms: Race A (layout
draws reveal partial content at the placeholder scroll before the task's
first paint) and Race B (the selection restore can fire before the
re-render task's readLines channel is live). Classify both as bounded
interleavings, sitting over the one fundamental constraint (no flicker-free
first paint of the target scroll without buffering that screenful), and
record the prescribed fixes and their implications for part 3. Documents
why faithful repro is the interactive app, not the headless harness.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Track how many leading viewLines entries the most recent refresh built
from the current buffer (freshViewLineCount) and bound the view-line→
buffer-line mapping on it. The entries past that count are the stale tail
refreshViewLinesIfNeeded leaves in place for flicker-avoidance; they
belong to a previous, longer render and must not be mapped. The old
in-range guard was insufficient: with wrapping a stale entry's buffer
index can still be in range of the shrunk buffer. Within the fresh range
every entry was just built from the current buffer, so its index is
guaranteed in range and the guard is no longer needed.
This is the §8 correctness fix the identity-based escape restore depends
on, since that read scans the view buffer while it is still loading.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
refreshViewLinesIfNeeded keeps the previous render's view lines in the
tail when the new render is shorter, so the view can go on showing old
content without flicker until the new content catches up. The mapping
readers guard only against a buffer index that has gone out of range of
the shrunk buffer — but with wrapping, a stale tail entry's index can
still be in range, so the guard passes and a view line that no longer
exists is mapped onto the wrong buffer line. See diff-line-metadata-notes.md §8.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HyperLinkInLine, DiffLineMetadataInLine and BufferLineForViewLine each
repeated the same preamble: take the lock, refresh the view lines, range-
check the view line, and guard against a stale viewLines entry pointing
past a shrunk buffer. They all need that mapping to stay consistent with
the buffer they then read, so the logic belongs in one place. Extract it
into bufferLineForViewLine and have all three call it. Behavior-preserving.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Capture a design discussion (no code yet; implementation is a future session):
- The escape-from-staging restore should anchor on the explorer view's current
patch identity at escape time, not a saved numeric scroll/index, since staging
or dropping hunks changes the content. This is the inverse direction of the
diff-line-metadata primitive (identity -> rendered row) and the same operation
the -U scroll-preservation consumer needs; record it as consumer #6 and split
the consumer list into forward (1-4) and inverse (5-6) directions.
- Record the escape-routing cases the current prototype gets wrong (staging the
last hunk should land in the staged half; <tab> between staged/unstaged; the
empty-view and custom-patch-builder cases).
- Decide to solve the new restore mechanism, the §11 timing races, and the
BufferLineForViewLine staleness trap together in the prototype rather than
defer them to productionization (you can't plan around unsolved entangled
mechanisms), with a dependency-first attack order (§8 fix, characterize the
races, then the predicate-scroll restore).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The emitter (delta), carrier (gocui per-cell attachment), and consumer (the
GetDiffLineInfo metadata backend + env-var handshake) are now built for the
normal unified case and verified end-to-end — including in the running app with
delta's default mode, where clicking/enter/e/G resolve via #2 and deletions get
the correct side. Update §9 from "in progress" to a "built & verified" record
(what was built, how it was verified) mirroring §8 for #1, and mark the build
order (§7) accordingly. The remaining step-5 deliverables — finalizing/publishing
the spec and the production plan — and side-by-side/difftastic are still open.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add mechanism #2 to GetDiffLineInfo: when a patched pager annotated each diff
line with OSC 456 metadata, read it back as the first backend, ahead of the
buffer parser (#1) and the lazygit-edit hyperlink. It is strictly higher
fidelity -- it carries the side explicitly, so it serves the renderings #1
cannot parse (delta's default mode, --line-numbers, diff-so-fancy) and conveys
deletions, which the hyperlink can't.
The host advertises the protocol versions it understands by setting
EMIT_OSC456_METADATA on the pager subprocess; a pager that doesn't understand
it ignores the variable, so this is safe to set unconditionally.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A patched pager (delta) prefixes each diff line with an OSC 456 sequence
carrying that line's patch-space identity (see diff-line-metadata-notes.md),
so the host can map a rendered row back to (file, type, new-line, old-line)
without re-parsing -- the only way to recover the side for renderings that
drop the +/- markers (delta's default mode).
Recognize it in the escape interpreter and stamp the payload onto each cell,
mirroring how OSC-8 hyperlinks are handled, exposing it via
DiffLineMetadataInLine. To do so, generalize the OSC dispatch to accumulate
the (possibly multi-digit) OSC number before branching, rather than matching
the single character '8'; the OSC-8 path is unchanged for well-formed input.
Unlike a hyperlink, the metadata sequence is never closed -- the pager
re-emits one per line -- so clear it at each line boundary to keep it from
bleeding onto a following line that has none (e.g. a hunk header).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mechanism #2's emitter side is now prototyped on this branch (delta, normal
unified mode only) and the bytes are verified. Capture what that settled, since
the spec is meant to be published for pager-developer feedback:
- The single per-line emit point in delta and which fields are reachable there
(the gotcha: delta only maintains its line-number counters with --line-numbers
on, so the patch tracks its own), and why a dedicated additive emitter beats
reusing LineNumbersData.
- The pinned v1 wire format (positional, file last so it may contain ';',
empty old-line unless deleted) and the EMIT_OSC456_METADATA env-var handshake.
- Deferred items: the OSC-number terminal audit, wrapped continuation rows, and
header-row attachments.
Also resolves the §6 wire-format open question accordingly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Record what the #1 prototype settled. The two open questions #1 touches are now
answered (the deleted-line new-line convention is patch.LineNumberOfLine's, and a
deletion carries both line numbers; multi-file diffs split on "diff --git" and
the section parses 1:1 with patch.Parse). Add a §8 capturing what landed, how it
was verified, and the implications for #2.
Two coverage corrections came out of verifying against real pager output, both
worth pinning before the spec is written: delta --color-only qualifies for #1
only *without* line numbers (the gutter pushes the +/- marker off column 0, so a
naive parse is confidently wrong — handled by an integrity check that falls back,
not by teaching the host delta's gutter), and diff-so-fancy strips the +/- markers
entirely, so it's a #2 case, not #1.
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 diff-line parser needs to walk the unwrapped diff buffer upward from a
clicked/selected row, but the row index it's handed is a view line index (which
counts wrapped lines). Expose the existing internal mapping (viewLines[y].linesY)
so callers can translate, with the same lock and stale-tail guard that
HyperLinkInLine uses against a concurrent shorter re-render.
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>
