PR 10 landed independently of the explorer removal and gained visible feedback for non-suspending editors. Capture its actual dependency, six-commit shape, pending terminal matrix, and the separately deferred click-selection behavior.
Co-authored-by: GitHub Copilot <noreply@github.com>
Four problems, the one root cause behind three of them, the prep refactor the
focus rule needed, and the two gaps left open. Also corrects a claim made
while the round was being scoped, that a diff renderer is re-laid-out
whenever the view width changes.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects and their fixes, the testing setup that reproduces them, and
the rebase traps the round turned up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Record the four review comments the interactive pass came with, what was
decided for each, and where their fixups landed — three of them belong to
PRs 5 and 6 rather than to PR 7.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The staged-side branch grew out of it, PR 7 was rebuilt on top, and the
selection-visibility rule became a question about content rather than about
which kind of task rendered it.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
It sits below PR 7, which is rebased onto it and simpler for it: which side a
pane shows is now a property of the pane, and the focus-follow rule is
symmetric.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seventeen deviations, of which the ones that bind later work are the split
seam (PR 8's panels satisfy the extension interface), the stale timing fact
about what a refresh has done by the time it returns, and the restore's new
Done hook.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things bind later work: a restore keeps the task reading to the end of
its input, because a rendering that has to be parsed as a diff can only be
parsed whole, and identities match through a key in which every kind of
content line collapses together - which is what lets a change that becomes
a context line still count as the same place, and a hunk header not.
The interactive pass on PR 5 found nothing to fix; refining the behaviour
of a few special cases can wait until the remaining PRs are written.
Toggling "ignore whitespace" re-renders the diff just like a context-size
change or a renderer switch does, so it belongs in PR 6 as a third
consumer. It is the first one whose anchor can vanish along with its hunk
or its whole file, which invalidates the candidate walk's reason for
stopping at the first change line; the walk goes unbounded instead, so the
restore lands on the nearest surviving line wherever in the diff that is.
And once staging happens in the main view, master's refusal to ignore
whitespace while staging no longer catches it - parked as an open question
for PR 7.
The selection commit is split in four, comments no longer point at the
staging view, and the selection commands are described and enabled only
where they apply.
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>
