From 4ca47482ec45ca2e9aa7b86f623681d7d8038c7c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 7 Aug 2026 19:41:45 +0200 Subject: [PATCH] Plan for git being able to emit the metadata itself 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. --- focused-main-view-production-plan.md | 78 +++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 13 deletions(-) diff --git a/focused-main-view-production-plan.md b/focused-main-view-production-plan.md index d408e3305..9b9bc100d 100644 --- a/focused-main-view-production-plan.md +++ b/focused-main-view-production-plan.md @@ -360,9 +360,16 @@ Commits: full reasoning). Refs: 836f768cb, af98be48d. 3. **Advertise the protocol to diff renderers** — set `OSC1717=V1` in the environment of renderer/ext-diff invocations (pty task env + ext-diff cmd - env); which route applies now follows from the entry's `type` (#5870) — - no need for `rawGit` (git itself never emits the protocol). Ref: - 9975a8fac + 665149b11 (final name: `OSC1717`). + env); which route applies now follows from the entry's `type` (#5870). + **Including `rawGit`:** git itself emits the records for its word-diff + formats, so the advertisement must be set **before** `newPtyTask`'s + early return for `rawGit` (that route needs no pty, since only a pager + needs a terminal to be spawned — but git still needs to be asked). Put + it next to `LAZYGIT_COLUMNS`, which sits there for the same reason. + Getting this wrong is silent: the renderer's output simply carries no + records, and every consumer falls back as if the renderer were + non-conforming. Ref: 9975a8fac + 665149b11 (final name: `OSC1717`), + "Advertise the metadata protocol to git as well, not only to a pager". Cross-repo note: the reference emitters live on `osc-1717-metadata` branches in `/Users/stk/Stk/Dev/Builds/{delta,difftastic,diff-so-fancy}`; nothing is @@ -372,6 +379,20 @@ before verifying. lazygit must remain fully functional without any conforming renderer (buffer-parse + PR 7's raw fallback guarantee this). Interactive verification of this PR needs locally built patched renderers. +**git is one of those emitters, and lazygit ships its side regardless of +whether git ever takes the patch.** The emitter lives on branch `osc-1717` +in `/Users/stk/Stk/Dev/Builds/git` (4 commits, word diffs only, unproposed +as of 2026-08-07); it may never be accepted, in which case the fallback is a +maintained fork for the users who want it. That doesn't change what lazygit +does, and nothing here is conditional on the outcome: the probe asks the +installed git what it can do, so a stock git answers "no records" and every +consumer degrades to exactly the behaviour of a non-conforming renderer — +the raw fallback when focused, buffer-parse otherwise. A git that speaks the +protocol turns `--color-words` from a browse-only renderer into a fully +usable one. Write the PR descriptions so that they don't promise a git +feature that doesn't exist upstream: describe the protocol and the probe, +not "works with git --color-words". + ### PR 5 — Select, navigate, edit and copy diff lines in the focused main view The focused main view (already reachable via `0`/click on master) gains a @@ -610,14 +631,23 @@ Commits: git's 7-arg convention on two empty temp files; env `OSC1717=V1`, greps for the handshake; verdict cached per renderer signature. `extDiff` with empty `command` (git's `diff.external`, formerly - `useExternalDiffGitConfig`) → always raw when focused. **New case, - decide at implementation:** `rawGit` entries (new in #5870) never - conform, so skip the probe; entries without restructuring `args` are - already raw (fallback is a no-op), while args like `--color-words` - produce output buffer-parse can't handle (`IsWellFormed` rejects it) — - these need the raw fallback when focused, whether by a static - args-present rule or by the well-formedness gate; surface the choice to - the user if it's not clear-cut; + `useExternalDiffGitConfig`) → always raw when focused. **`rawGit` is + probed like any other renderer** (resolved 2026-08-07; was "decide at + implementation"): an entry *with* `args` is run as + `git diff --no-index ` on the same two empty temp files, and its + handshake read the same way. git announces itself for exactly the + formats it describes, so asked with the entry's own arguments the + handshake is a faithful answer — `--color-words` announces, a unified + diff says nothing at all. Don't special-case git by looking for a + per-line record instead: that was tried and reverted as over-specified. + Entries with no `args` skip the probe, being already raw (the fallback + would be a no-op). That leaves **one rule for every renderer type** — + raw when focused iff the diff needs records and the renderer won't + supply them, i.e. `mainViewDiffNeedsMetadata() && !probe`, where "needs + records" is any custom renderer *or* git with word-diff args. The + `IsWellFormed`/static-args alternatives are dropped. The probe's cache + signature must include the `rawGit` args, or editing them in the config + reuses a stale verdict; `DiffMainViewShouldRenderRaw` read by every diff panel's render-to-main; `ignoreExternalDiff` threaded through the diff-cmd builders (`--no-ext-diff`, keep color); `types.NewMainViewDiffTask` routes raw @@ -936,8 +966,15 @@ The remaining rows are agreed as keep/defer: 5. **PR titles**: drafts in §4 — the user finalizes wording at PR-open time (they're the release-notes lines). 6. **Cross-repo timing** (outside this plan): circulating the OSC 1717 spec, - upstreaming the three renderer patches. lazygit ships fully functional - without them; revisit pitching once PRs 1–8 exist as evidence. + upstreaming the three renderer patches and the git one. lazygit ships + fully functional without any of them; revisit pitching once PRs 1–8 exist + as evidence. git's own patch may never be accepted — a maintained fork is + the accepted fallback (PR 4 cross-repo note), and no PR here waits on the + outcome. +7. ~~**PR 7:** how should `rawGit` entries with restructuring args decide + the raw fallback?~~ Resolved 2026-08-07: probe them like any other + renderer, since git announces itself for exactly the formats it + describes. See PR 7 commit 10. ## 10. Progress @@ -975,3 +1012,18 @@ Log: decisions on `DiffRendererConfigManager.GetDiffRendererType()` instead of querying the pager/ext-diff fields individually (PR 4 commit 3, PR 7 commit 10), and `rawGit` entries are a new case for PR 7's fallback. +- **2026-08-07:** git can emit the records itself, so `rawGit` entries with + word-diff args are now first-class conforming renderers rather than a + case to give up on. The emitter is 4 commits on branch `osc-1717` in the + git repo (word diffs only; byte-identical output when `OSC1717` is + unset), unproposed upstream and shipping either way — see PR 4's + cross-repo note. Resolved the "decide at implementation" question in PR 7 + commit 10 in favour of probing `rawGit` like anything else, which + collapses the fallback to one rule for all renderer types. Prototyped on + the branch in "Support git's own word diff as a metadata-emitting diff + renderer" (+ its fixup) and "Advertise the metadata protocol to git as + well, not only to a pager"; verified interactively: focus selects a + change line, staging works, the selection advances after staging. Two + traps found the hard way, both recorded above — the advertisement must + precede `newPtyTask`'s no-pty early return, and the probe's cache + signature must include the args.