mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-10-05 21:46:49 -04:00
Drop f/h types again
This commit is contained in:
Stefan Haller
+112
-7
@@ -176,7 +176,7 @@ Fields per attachment:
|
||||
| field | presence | meaning |
|
||||
|---|---|---|
|
||||
| `version` | always | self-describing (see §3.3) |
|
||||
| `type` | always | `file-header \| hunk-header \| context \| added \| deleted \| other` |
|
||||
| `type` | always | `context \| added \| deleted` only — header types (`file-header`/`hunk-header`) were considered here but **dropped**; content-lines-only (§11, spec §5.5). (#1's host-side parser still classifies parsed rows internally; that's separate from the #2 wire type.) |
|
||||
| `file` | always | absolute or repo-root-relative path (the host normalizes — pagers may emit whichever is convenient); on **every** attachment so search-left yields a complete answer without scanning back to the file header |
|
||||
| `new-line` | always (content lines) | new-file line number, in the **diff's** new-file space |
|
||||
| `old-line` | **only** when `type = deleted` | old-file line number |
|
||||
@@ -298,10 +298,11 @@ patch-find for added/context, PR `R` anchor).
|
||||
index is just `targetBufferIdx − fileStartIdx`. New/old line numbers and the
|
||||
type then fall straight out of the patch arithmetic. The file path comes from
|
||||
the section's `+++ b/…` line (falling back to `--- a/…`, then `diff --git`).
|
||||
- **Do headers carry line numbers?** `hunk-header` *could* carry old-start /
|
||||
new-start, but hunk boundaries are also derivable from coordinate
|
||||
discontinuities in the content lines, so it may be unnecessary. `file-header`
|
||||
needs no line numbers (the `file` field already attributes every line).
|
||||
- ~~**Do headers carry line numbers?**~~ **RESOLVED — headers carry *nothing*; the
|
||||
spec is content-lines-only (§11 outcome, spec §5.5).** Hunk boundaries are
|
||||
derivable from coordinate discontinuities in the content lines and file
|
||||
boundaries from the `file` field, so header records were dropped entirely rather
|
||||
than carry (or not carry) line numbers.
|
||||
- ~~**difftastic specifics:**~~ **RESOLVED (#2 difftastic prototype, §10).** Two
|
||||
regions per row (one per side-by-side column), not N — token-level novelty is
|
||||
sub-cell colouring, not separate identity (§10.3). Each is emitted at the start
|
||||
@@ -541,8 +542,12 @@ emits V1 when the advertised list contains `V1`.
|
||||
`enter`/hunk-nav break on continuation rows. **Now FIXED in delta too** (§10.8):
|
||||
delta wraps only in side-by-side mode, and each wrapped row now re-emits its
|
||||
primary line's record (no counter advance). difftastic was fixed the same way.
|
||||
- **Header rows** (`@@`, `diff --git`, `---`/`+++`) get no attachment; acting on a
|
||||
header row falls through to #1, then to no-selection.
|
||||
- **Header rows** (`@@`, `diff --git`, `---`/`+++`, decorations) get no attachment;
|
||||
acting on a header row falls through to #1, then to no-selection. This is now the
|
||||
**intended, permanent** design: `f`/`h` header records were prototyped (§11) and
|
||||
then **dropped from the spec** — the protocol is content-lines-only, and the host
|
||||
derives file/hunk structure from content records and backs up over un-annotated
|
||||
header rows (consumer #4 already does this). See §11's outcome banner + spec §5.5.
|
||||
|
||||
### 9.4 What was built, and how it was verified
|
||||
|
||||
@@ -830,3 +835,103 @@ still reports the right new-line). `osc_for_line` is the single chokepoint, so t
|
||||
one change covers both SxS emit paths (the minus/plus precompute and the
|
||||
`paint_zero_lines_side_by_side` context path). Landed as an `amend!` into the
|
||||
delta side-by-side commit, with a unit test (`test_wrapped_rows_reemit_…`).
|
||||
|
||||
---
|
||||
|
||||
## 11. `f`/`h` header-record prototype — built, then dropped from the spec
|
||||
|
||||
> **OUTCOME (decided after this prototype):** `f`/`h` are **removed from the spec
|
||||
> entirely** — the protocol is now **content-lines-only** (`c`/`a`/`d`). This
|
||||
> section is kept as the *evidence* for that decision; it is no longer a
|
||||
> description of the spec. The reasoning: the host derives file/hunk structure
|
||||
> from content records (the `file` field changes → new file; a `new-line`
|
||||
> discontinuity → new hunk), and it needs the "back up over un-annotated header
|
||||
> rows" fallback *regardless* (consumer #4 already does this), so making `f`/`h`
|
||||
> optional would be the worst of both worlds and making them mandatory adds the
|
||||
> real pager-side friction documented below. The one thing lost — files that emit
|
||||
> no content records (pure renames, mode changes, binaries) are invisible to the
|
||||
> identity layer — is acceptable (nothing to act on; still reachable by cursor).
|
||||
> See spec §5.5. **The emitter code is preserved on WIP commits** (delta +
|
||||
> difftastic `prototype-osc-metadata`) for future reference, not folded into the
|
||||
> content-line emitters.
|
||||
|
||||
Extends the #2 emitter to the **structural rows** — `f` (file header) and `h`
|
||||
(hunk header) — in both delta and difftastic, to pressure-test the spec's claim
|
||||
that this is cheap. Both built on `prototype-osc-metadata`, both byte-identical to
|
||||
stock with the env unset (verified by stripping the OSC and `diff`), both with new
|
||||
unit tests; all delta tests (444) and difftastic tests (129 unit + 23 integration)
|
||||
green. Headline: **the claim mostly holds, but two real shapes bent the spec** —
|
||||
delta can't populate `f`'s `new-line`, and difftastic's only header row is a
|
||||
*combined* file+hunk banner. Spec edited (§4.2/§4.3/§5.1/§5.2/§6.4/§9.5/§10).
|
||||
|
||||
### 11.1 delta — `h` trivial, `f` cannot carry its `new-line`
|
||||
|
||||
- **`h` (easy).** At `emit_hunk_header_line`, `initialize_hunk` has just seeded
|
||||
the new-file start *which is the hunk's first line*, so `h`'s `new-line` is in
|
||||
hand exactly when the box is drawn. `osc_for_hunk_header()` reads it; verified
|
||||
`h;8`, `h;41`, `h;1`, and `h;0` for a deleted file (`@@ -1,N +0,0 @@`).
|
||||
- **`f` (the ordering finding the task predicted).** delta boxes the file name
|
||||
when it parses the `+++` line — **before** it has seen the first `@@`. At that
|
||||
moment its counters are 0 (first file) or *stale from the previous file's last
|
||||
hunk*, so the first-hunk line is genuinely not known. Carrying it would require
|
||||
buffering the whole file header until the first `@@`, i.e. abandoning delta's
|
||||
line-by-line streaming. So **delta emits `f` with an empty `new-line`**
|
||||
(`1717;1;f;;;src/foo.go`), and **the spec now relaxes `f`'s `new-line` to
|
||||
optional** (§5.2): the host falls back to the first content record after the
|
||||
header. The path isn't on the emitter yet either (it's learned at the first
|
||||
hunk), so the `StateMachine` supplies it, with the same `plus_file`/`minus_file`
|
||||
selection the hunk header uses (`diff_header_osc()`).
|
||||
- **Multi-row blocks + mode-independence (clean).** The file header is 2 rows
|
||||
(name + underline), the hunk header 3 (box). A `Write` adapter
|
||||
(`OscLinePrefixer` / `write_with_header_osc` in `diff_line_metadata.rs`) injects
|
||||
the record after every newline, so every row of the block carries it — the same
|
||||
rule wrapped content rows follow. And both headers are **full-width decorations
|
||||
rendered by the same code in unified *and* side-by-side** (the column split is
|
||||
content-only), so f/h emission is mode-independent — verified identical byte
|
||||
offsets in `-s`. No SxS-specific work.
|
||||
- **Awkward (minor):** the OSC is threaded as a `&str` through the shared draw
|
||||
helpers (`write_line_of_code_with_optional_path_and_line_number` is shared with
|
||||
ripgrep output, which passes `""`; `write_generic_diff_header_header_line` has
|
||||
four call sites). Small, explicit, no behavior change when empty.
|
||||
|
||||
### 11.2 difftastic — the spec's "no hunk headers" assumption was wrong
|
||||
|
||||
The spec/notes assumed difftastic renders no hunk headers (only a file name), so
|
||||
it would emit "only `f`". **Not so.** difftastic prints **one banner per hunk** —
|
||||
`path --- N/total --- Format` (via `style::header`, in both side-by-side and
|
||||
inline) — that announces the file *and* the hunk (with a hunk counter). So:
|
||||
|
||||
- there is **no standalone file-name row** distinct from the hunk, and
|
||||
- there is **no `@@`-style hunk row** — but there **is** a per-hunk header (the
|
||||
banner). This is the §10.2 token-vs-line model mismatch reappearing at the
|
||||
header level: one row is *both* a file header and a hunk header.
|
||||
|
||||
**Mapping chosen (now the spec's rule for combined headers, §5.1):** first hunk's
|
||||
banner → `f` (the file's entry; `new-line` = the first hunk's first line); each
|
||||
later hunk's banner → `h` (that hunk's first line). **One record per banner row** —
|
||||
*not* an `f` and an `h` before the same cell, because a row-granular action (first
|
||||
record on the row, §7) and a click (nearest record left of the point) would then
|
||||
disagree. A single-hunk file emits one `f` and no `h` (its banner shows no
|
||||
counter) — which *looks* like the predicted "only `f`", but for a different reason.
|
||||
|
||||
- **`new-line` is free (opposite of delta).** difftastic builds the whole diff
|
||||
(`hunks: &[Hunk]`) before rendering, so the banner knows its hunk's aligned
|
||||
lines; `f`/`h` carry a real `new-line`. Verified `f;1` + `h;16` (SxS), `f;1` +
|
||||
`h;15` (inline), whole-file add `f;1`, whole-file delete `f;0`, AST-mode Rust
|
||||
`f;1` with no `@@` rows.
|
||||
- **Multi-row banner handled.** Normally one row, two when the first hunk also
|
||||
shows a rename's old path; `header_banner` prefixes every row (spec §6.4).
|
||||
- **Less code than delta**, same reason as §10.1: line numbers native, path is a
|
||||
parameter, no streaming-order problem.
|
||||
|
||||
### 11.3 Files touched
|
||||
|
||||
- **delta** (`prototype-osc-metadata`): `features/diff_line_metadata.rs`
|
||||
(`osc_for_hunk_header`/`osc_for_file_header`/`header_osc`, `OscLinePrefixer`,
|
||||
`write_with_header_osc`, 4 tests); `handlers/hunk_header.rs` (`h` thread +
|
||||
wrap); `handlers/diff_header.rs` (`diff_header_osc` + `f` thread + wrap);
|
||||
`handlers/mod.rs`, `handlers/grep.rs` (signature follow-through).
|
||||
- **difftastic** (`prototype-osc-metadata`): `display/diff_line_metadata.rs`
|
||||
(`header_banner`, 2 tests); `display/side_by_side.rs` (`hunk_first_new_line`,
|
||||
banner f/h in `print`, `f` in `display_single_column`); `display/inline.rs`
|
||||
(banner f/h, reordered so the first-new-line is known before printing).
|
||||
|
||||
@@ -126,8 +126,8 @@ Raw bytes, for a context line at new-file line 10 of `src/foo.go`:
|
||||
| field | presence | meaning |
|
||||
|---|---|---|
|
||||
| `version` | always | decimal protocol version; `1` for v1. Carried in every record so attachments are self-describing. |
|
||||
| `type` | always | one character — see §5.1. v1 emits `c` (context), `a` (added), `d` (deleted), `f` (file header), `h` (hunk header). |
|
||||
| `new-line` | content & header lines | new-file line number, in the **diff's new-file space** (see §5.2). |
|
||||
| `type` | always | one character — see §5.1. v1 emits `c` (context), `a` (added), `d` (deleted). |
|
||||
| `new-line` | always | new-file line number, in the **diff's new-file space** (see §5.2). |
|
||||
| `old-line` | only `type=d` | old-file line number. **Empty** for `c` and `a`. |
|
||||
| `file` | always | the file path the line belongs to; absolute or repo-root-relative (the host normalizes — emit whichever is convenient). Carried on **every** record so a single record is a complete answer. |
|
||||
|
||||
@@ -144,8 +144,6 @@ path is not carried. A pure rename with no content change emits no records at al
|
||||
| deletion, old line 9, sits at new pos 11 | `1717;1;d;11;9;src/foo.go` |
|
||||
| two consecutive deletions | `…;d;11;9;…` then `…;d;11;10;…` (same `new-line`, different `old-line` — see §5.3) |
|
||||
| whole-file deletion | `1717;1;d;0;9;old/path` (`new-line` 0 — see §5.4) |
|
||||
| file header | `1717;1;f;10;;src/foo.go` (`new-line` = the file's first hunk line — §5.2) |
|
||||
| hunk header | `1717;1;h;10;;src/foo.go` |
|
||||
|
||||
---
|
||||
|
||||
@@ -153,21 +151,17 @@ path is not carried. A pure rename with no content change emits no records at al
|
||||
|
||||
### 5.1 Type
|
||||
|
||||
`type` is one character. v1 defines five, and a conforming pager emits each
|
||||
wherever the corresponding row exists:
|
||||
`type` is one character. v1 defines three, all of them **content-line** types, and
|
||||
a conforming pager emits one before every content line it renders:
|
||||
|
||||
- `c` — context (unchanged) line
|
||||
- `a` — added line
|
||||
- `d` — deleted line
|
||||
- `f` — file header — the row a pager renders to announce a file
|
||||
- `h` — hunk header — the row announcing a hunk (e.g. a `@@ … @@` line)
|
||||
|
||||
`c`/`a`/`d` are the content-line types. `f`/`h` mark the structural rows so the
|
||||
host can place every file and hunk **exactly** — for navigation, and for acting on
|
||||
a header — instead of inferring structure from layout, the one thing it cannot do.
|
||||
They are not optional: a pager emits `f` on each file-header row and `h` on each
|
||||
hunk-header row it renders (a pager that renders no hunk-header rows, e.g.
|
||||
difftastic, simply has none to tag). See §6.4.
|
||||
**There is deliberately no file-header or hunk-header type — see §5.5.** The
|
||||
protocol annotates content lines only; the host recovers file and hunk *structure*
|
||||
from the content records themselves (the `file` field and `new-line`
|
||||
discontinuities), and treats the pager's header/decoration rows as non-actionable.
|
||||
|
||||
A host **must ignore a `type` it does not recognize** (treat the row as
|
||||
non-actionable) rather than reject the record, so the set can grow later without a
|
||||
@@ -186,11 +180,6 @@ specifically on *change* lines). Hence an explicit type.
|
||||
expected to re-map this through its own diff↔worktree adjustment; the pager
|
||||
should emit the number as it appears in the diff it is rendering.
|
||||
- `old-line` is the old-file line number, present **only** for deletions.
|
||||
- On a **header** record (`f`/`h`), `new-line` is the new-file line of the first
|
||||
line of the hunk it heads — for `h` that hunk, for `f` the file's first hunk —
|
||||
and `old-line` is empty. This is what lets `e` ("open in editor") on a header
|
||||
land at the top of what the user is looking at. (A round-trip *through* staging
|
||||
from a header is therefore only approximate, which is an accepted trade.)
|
||||
|
||||
### 5.3 The deleted-line convention (both numbers)
|
||||
|
||||
@@ -219,6 +208,51 @@ A host uses `old-line` to find a deletion's patch line and its old-side
|
||||
A deleted file's lines carry `new-line` = `0` (mirroring git's `@@ -1,N +0,0 @@`);
|
||||
an added file's lines carry the new-file numbers normally and `type=a`.
|
||||
|
||||
### 5.5 Non-goal: header and decoration rows are not annotated
|
||||
|
||||
The protocol covers **content lines only**. A pager's file-header and hunk-header
|
||||
rows — delta's boxed file name and hunk-header box, difftastic's per-hunk banner,
|
||||
diff-so-fancy's `── file ──` rule — carry **no** record, and there is no `f`/`h`
|
||||
(or "file-header"/"hunk-header") type. The host treats every un-annotated row as
|
||||
non-actionable.
|
||||
|
||||
This is deliberate. The host does not need header records to recover diff
|
||||
structure, because the structure is already implicit in the content records:
|
||||
|
||||
- **File boundaries** — the `file` field changes between consecutive content
|
||||
records, so the first content record carrying a new path *is* that file's entry.
|
||||
- **Hunk boundaries** — `new-line` jumps by more than one between consecutive
|
||||
content records of a file (lines were skipped), so a discontinuity marks a new
|
||||
hunk. (Two consecutive deletions share a `new-line` by §5.3, so compute the gap
|
||||
from the last *advancing* line.)
|
||||
|
||||
So file/hunk navigation, "jump to the top of this file/hunk", and the rest are all
|
||||
served by content records plus a trivial scan; the host lands navigation on a
|
||||
hunk/file's first **content** row, backing up over any un-annotated header rows the
|
||||
pager drew above it (a few lines of host code, needed anyway — see below).
|
||||
|
||||
**Why not annotate headers, even optionally?** An earlier draft made `f`/`h`
|
||||
mandatory; prototyping them in delta and difftastic (preserved in the design
|
||||
notes) showed the cost is real and the benefit marginal:
|
||||
|
||||
- It adds genuine pager-side friction. delta draws the file header when it parses
|
||||
the `+++` line, *before* it has seen the first `@@`, so it cannot know the
|
||||
header's hunk line without buffering or abandoning streaming. difftastic has no
|
||||
separate header rows at all — one per-hunk banner is *both* a file and a hunk
|
||||
header — so neither `f` nor `h` maps cleanly onto it. For a protocol whose whole
|
||||
pitch is "emit one OSC per content line," this roughly doubles the conceptual
|
||||
surface for the next pager author.
|
||||
- Making them *optional* is the worst of both: a host can't rely on them, so it
|
||||
must implement the "header row is un-annotated → back up to the nearest content
|
||||
row" fallback regardless — and then maintain two code paths forever. Dropping
|
||||
the types entirely leaves the host **one** path, exercised for every pager.
|
||||
|
||||
The only thing genuinely lost is files that emit **no content records** at all —
|
||||
pure renames, pure mode changes, binary files (§4.2). These become invisible to
|
||||
the identity layer (navigation can't anchor on them). That is acceptable: they
|
||||
have nothing to stage/edit/open, and remain reachable by ordinary cursor movement
|
||||
over the rendered buffer.
|
||||
|
||||
---
|
||||
|
||||
## 6. Emit rules (placement)
|
||||
@@ -279,24 +313,12 @@ Getting this wrong is not theoretical: without per-row records, acting on a
|
||||
wrapped continuation row does nothing, and hunk/file navigation breaks because the
|
||||
untagged rows fragment a wrapped line into one block per visual row.
|
||||
|
||||
### 6.4 Header rows
|
||||
### 6.4 Header and decoration rows — emit nothing
|
||||
|
||||
File-header and hunk-header rows carry records too, and these are **mandatory**:
|
||||
`f` on each file-header row, `h` on each hunk-header row (§5.1). With them the host
|
||||
places every file and hunk exactly — file/hunk navigation lands on the header, and
|
||||
`e` on a header opens at the hunk's first line — with no layout guessing.
|
||||
|
||||
Where a header spans **several rows** — delta boxes a file name in divider/name/
|
||||
divider lines; a pager might draw a rule under a hunk header — the recommendation
|
||||
is that **every row of that block carry the same record** (all three box lines get
|
||||
the file's `f`; a hunk header and its rule get that hunk's `h`), exactly as a
|
||||
wrapped content line re-emits its record on every row (§6.3). That leaves no dead
|
||||
rows in a header block — the user can press `e` anywhere on it and land in the same
|
||||
place. The pager has the final say over what counts as its header block and which
|
||||
row "is" the heading; this is the recommended default, not a hard rule.
|
||||
|
||||
A pager with no hunk-header rows (e.g. difftastic) has no `h` to emit. A row a pager
|
||||
genuinely leaves un-recorded is treated by the host as non-actionable.
|
||||
A pager's header and decoration rows (file headers, hunk headers, dividers,
|
||||
padding) carry **no** record — there is no header type to emit (§5.1, §5.5). The
|
||||
host derives file and hunk structure from the content records and treats every
|
||||
un-annotated row as non-actionable.
|
||||
|
||||
---
|
||||
|
||||
@@ -359,9 +381,15 @@ mapping; recorded as a v2 candidate, not taken (§9).
|
||||
the side for deleted lines, and in side-by-side mode. (delta needed to track
|
||||
its own old/new counters because its line-number counters are dormant unless
|
||||
`--line-numbers` is on; difftastic had them natively. Your mileage may vary.)
|
||||
5. **Is emitting `f`/`h` on header rows a burden?** v1 makes them mandatory (§5.1,
|
||||
§6.4); it should be cheap — the pager already knows the file and the hunk's
|
||||
start line — but tell us if your format makes it awkward.
|
||||
5. **Content-lines-only scope (§5.5) — is anything lost for your pager?** We
|
||||
deliberately dropped header annotations: an earlier draft made file/hunk-header
|
||||
types mandatory, but prototyping them in delta and difftastic showed real
|
||||
pager-side friction (delta draws the file header before it has parsed the first
|
||||
`@@`; difftastic has no separate header rows, only a combined per-hunk banner)
|
||||
for benefit the host can get by deriving structure from content records. If your
|
||||
pager has a structure where the host genuinely *cannot* reconstruct file/hunk
|
||||
boundaries from content records, tell us — that would argue for bringing header
|
||||
types back.
|
||||
|
||||
---
|
||||
|
||||
@@ -373,7 +401,9 @@ OSC `1717`:
|
||||
- **delta** — a dedicated additive emitter that injects only OSC bytes (no change
|
||||
to styling, width, or wrapping); with the env var unset, output is byte-for-byte
|
||||
identical to stock delta. Covers unified and side-by-side modes, including
|
||||
wrapped rows.
|
||||
wrapped rows. (A `f`/`h` header-record variant was also prototyped before headers
|
||||
were dropped from the spec — see §5.5 / the design notes — but is not part of
|
||||
this content-line-only protocol.)
|
||||
- **difftastic** — the categorical case (#1 host-side parsing cannot serve it in
|
||||
either mode). Emits the same v1 format under the same handshake; markedly less
|
||||
code than delta because difftastic carries old/new line numbers natively. Covers
|
||||
|
||||
Reference in New Issue
Block a user