Merge pull request #227 from levineuwirth/git-status-stage1
feat(git): Stage 1 — *git-status* and *git-diff*, no wire change
This commit is contained in:
GitHub
commit
b867f642b6
34
COHERENCE.md
34
COHERENCE.md
|
|
@ -1306,6 +1306,17 @@ Primitive-by-primitive against the list above:
|
|||
that found it (§25). All four remain in `lsp.lua`; per §25 the
|
||||
symbols are authoritative and the `ad41cf1` line numbers have drifted.
|
||||
|
||||
**Updated again: FIVE, and the fifth is the first outside
|
||||
`lsp.lua`.** Git Stage 1's `*git-status*`
|
||||
(`builtin/runtime/git.lua`) is the concrete evidence P5 asked for
|
||||
that the primitive generalizes past its first consumer — the
|
||||
remediation here was always adoption, not construction. It also
|
||||
added the primitive's one extension: an optional **`keys`** table on
|
||||
the open spec, installed once with the panel's buffer and compared
|
||||
(not re-bound) on reopen, because `Keymap::bind` refuses duplicates
|
||||
and an async consumer re-opens on every refresh. `*buffer-list*` and
|
||||
project-search remain the un-migrated hand-rolled pair.
|
||||
|
||||
**`*lsp*` is the only one of the four with a working refresh** — it is
|
||||
the only one supplying `on_refresh`. `g` is bound on all four
|
||||
unconditionally by `bind_local_keymap`, so the other three carry a
|
||||
|
|
@ -1435,10 +1446,25 @@ What does not:
|
|||
|
||||
- **Code actions apply the first action blindly** — no picker (a
|
||||
roadmap "dark matter" item still true at audit).
|
||||
- **There is no Git integration at all** — no status, stage, diff,
|
||||
blame, or gutter markers anywhere in the tree (gutter git riders and
|
||||
the `ResourceOffer` diff/blame family are named deferrals). The Git
|
||||
affordance list above has nothing to attach to yet.
|
||||
- **Git integration reaches status and diff, and no further.** Stage 1
|
||||
(`docs/git-integration-framing.md`) ships `*git-status*` — a
|
||||
`listview` panel over `git status --porcelain=v2 --branch -z`, with
|
||||
RET visiting the file and `d` showing its file-level diff. There is
|
||||
still **no stage, revert, blame, or gutter marker** anywhere in the
|
||||
tree; gutter git riders need new `DecorationKind` variants (Stage 2,
|
||||
which must be scheduled alone), and the `ResourceOffer` diff/blame
|
||||
family remains a named deferral. The Git affordance list above now has
|
||||
something to attach to; the affordances themselves are unbuilt, and
|
||||
the menu's context vocabulary (`src/menu.rs`) has no `git` context to
|
||||
host them.
|
||||
|
||||
The original audit said "there is no Git integration at all … anywhere
|
||||
in the tree", and that was **literally false when it was written**:
|
||||
`tests/fixtures/pmacs-magit/` is a tracked, installable package that
|
||||
spawns git and parses porcelain v2, with a 32-test acceptance suite
|
||||
(`tests/m8_6_acceptance.rs`). The **product** gap it described was
|
||||
real; the sentence overstated it, and the framing that found the
|
||||
overstatement is the one that closed the gap.
|
||||
- No test run/debug affordances (DAP is a future arc,
|
||||
`docs/dap-debugging-framing.md`).
|
||||
- No missing-tool guidance affordances (§1.2 — the diagnostic that
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -25,6 +25,7 @@
|
|||
-- rows = { { text = "src/foo.rs:12:4", item = <any> }, ... },
|
||||
-- on_visit = function(item) ... end, -- RET/SPC (optional)
|
||||
-- on_refresh = function() return rows end, -- g (optional)
|
||||
-- keys = { d = "git.diff-file" }, -- extra buffer-local keys
|
||||
-- }
|
||||
|
||||
pmacs.listview = pmacs.listview or {}
|
||||
|
|
@ -263,19 +264,193 @@ local function seat_cursor(p, line)
|
|||
end
|
||||
end
|
||||
|
||||
-- The primitive's own key surface, named ONCE so the binder below and
|
||||
-- the `keys` validator consult the same list. Previously this was a
|
||||
-- sequence of `bind(...)` calls and the set existed nowhere as data,
|
||||
-- which is why the git framing had to quote it from the source
|
||||
-- (docs/git-integration-framing.md Q#G-7).
|
||||
local FIXED_KEYS = {
|
||||
{ "RET", "listview.visit" },
|
||||
{ "SPC", "listview.visit" },
|
||||
{ "n", "cursor.down" },
|
||||
{ "<down>", "cursor.down" },
|
||||
{ "p", "cursor.up" },
|
||||
{ "<up>", "cursor.up" },
|
||||
{ "TAB", "listview.toggle" },
|
||||
{ "g", "listview.refresh" },
|
||||
{ "q", "listview.quit" },
|
||||
}
|
||||
|
||||
local function bind_local_keymap(buf)
|
||||
local function bind(seq, command)
|
||||
pmacs.keymap.bind { scope = "buffer", buffer = buf, sequence = seq, command = command }
|
||||
for _, entry in ipairs(FIXED_KEYS) do
|
||||
pmacs.keymap.bind {
|
||||
scope = "buffer", buffer = buf, sequence = entry[1], command = entry[2],
|
||||
}
|
||||
end
|
||||
bind("RET", "listview.visit")
|
||||
bind("SPC", "listview.visit")
|
||||
bind("n", "cursor.down")
|
||||
bind("<down>", "cursor.down")
|
||||
bind("p", "cursor.up")
|
||||
bind("<up>", "cursor.up")
|
||||
bind("TAB", "listview.toggle")
|
||||
bind("g", "listview.refresh")
|
||||
bind("q", "listview.quit")
|
||||
end
|
||||
|
||||
-- ---------------------------------------------------------------------
|
||||
-- Consumer-supplied keys (Q#G-7)
|
||||
-- ---------------------------------------------------------------------
|
||||
--
|
||||
-- An optional `keys = { <sequence> = <command name> }` on the open
|
||||
-- spec, bound through the SAME `pmacs.keymap.bind { scope = "buffer" }`
|
||||
-- path as the fixed set above. It exists because a consumer cannot
|
||||
-- safely bind its own key from outside: `open` disambiguates a name
|
||||
-- collision to `<2>`, so the name a consumer passed is not necessarily
|
||||
-- the buffer it got, and this module is the only place the handle is
|
||||
-- known. No key is intercepted anywhere — COHERENCE.md §6's shadow
|
||||
-- count is unchanged by this.
|
||||
--
|
||||
-- INSTALL-ONCE, MATCH-ON-REOPEN. `Keymap::bind` refuses duplicates
|
||||
-- (`KeymapError::DuplicateBinding`, "Refuse rather than silently
|
||||
-- overwrite", src/keymap_tree.rs), and a consumer built on the async
|
||||
-- completion model calls `open` again on EVERY refresh. So keys are
|
||||
-- installed when the buffer is created and a later `open` for a live
|
||||
-- panel does not re-bind — it COMPARES, and errors on divergence.
|
||||
-- Silently keeping the old binding would hand the consumer a key that
|
||||
-- does something other than what it just asked for, which is the dead-
|
||||
-- or-lying-key defect this module already condemns for `g`.
|
||||
|
||||
-- A key sequence's whitespace-separated chord tokens. That is exactly
|
||||
-- how `parse_sequence` (src/key.rs) splits one, so a prefix relation
|
||||
-- computed here is the same relation the trie would find.
|
||||
local function chords_of(sequence)
|
||||
local out = {}
|
||||
for token in sequence:gmatch("%S+") do out[#out + 1] = token end
|
||||
return out
|
||||
end
|
||||
|
||||
-- True when one chord list is a STRICT prefix of the other. Either
|
||||
-- direction is a conflict: `Keymap` refuses both turning a leaf into a
|
||||
-- submap (`WouldExtendLeaf`) and shadowing a submap with a leaf
|
||||
-- (`WouldShadowSubmap`), and a `keys` table must not be able to reach
|
||||
-- either.
|
||||
local function prefix_conflict(a, b)
|
||||
local short, long = a, b
|
||||
if #a > #b then short, long = b, a end
|
||||
if #short == 0 or #short == #long then return false end
|
||||
for i = 1, #short do
|
||||
if short[i] ~= long[i] then return false end
|
||||
end
|
||||
return true
|
||||
end
|
||||
|
||||
-- Normalize `keys` into a sorted array of `{ sequence, command }`.
|
||||
-- Sorted so the comparison on reopen and every error message are
|
||||
-- deterministic (`pairs` order is not).
|
||||
local function normalized_keys(keys)
|
||||
if keys == nil then return {} end
|
||||
if type(keys) ~= "table" then
|
||||
error(string.format(
|
||||
"listview: `keys` must be a table of sequence -> command name; got %s",
|
||||
type(keys)))
|
||||
end
|
||||
local out = {}
|
||||
for sequence, command in pairs(keys) do
|
||||
if type(sequence) ~= "string" or sequence == "" then
|
||||
error("listview: every `keys` entry must be keyed by a non-empty key sequence")
|
||||
end
|
||||
if type(command) ~= "string" or command == "" then
|
||||
error(string.format(
|
||||
"listview: `keys[%q]` must be a command NAME (a non-empty string); got %s",
|
||||
sequence, type(command)))
|
||||
end
|
||||
out[#out + 1] = { sequence = sequence, command = command }
|
||||
end
|
||||
table.sort(out, function(a, b) return a.sequence < b.sequence end)
|
||||
return out
|
||||
end
|
||||
|
||||
-- A FIRST-PASS collision check, for a better message than the keymap's.
|
||||
--
|
||||
-- It compares RAW TOKENS, and that is deliberately not sufficient: the
|
||||
-- key parser canonicalizes aliases before it ever reaches the trie
|
||||
-- (`parse_key_code`, src/key.rs, uppercases and folds `RET`/`RETURN`/
|
||||
-- `ENTER`, `SPC`/`SPACE`, `ESC`/`ESCAPE`, `BS`/`BACKSPACE`,
|
||||
-- `DEL`/`DELETE`), so `keys = { RETURN = ... }` is a collision this
|
||||
-- function cannot see.
|
||||
--
|
||||
-- **`Keymap::bind` is the authority, and `ensure_panel` tears the panel
|
||||
-- down when it refuses.** That is not a fallback for a check that
|
||||
-- happens to be weak --- it is the only version that cannot go stale. A
|
||||
-- Lua-side canonicalizer would be a second copy of `parse_key_code`'s
|
||||
-- alias table, and the day the Rust one gains a name the Lua one would
|
||||
-- silently stop seeing that alias, reintroducing exactly this bug for
|
||||
-- it. (There is also no way to canonicalize an arbitrary sequence from
|
||||
-- Lua today: `display_sequence` is reachable only through
|
||||
-- `describe.key` and `keymap.list`, which both require the sequence to
|
||||
-- be BOUND already.)
|
||||
--
|
||||
-- So what this buys is diagnosis, not safety: a named "that is the
|
||||
-- panel's own `g`" instead of a raw `DuplicateBinding`.
|
||||
local function check_key_collisions(entries)
|
||||
for i, entry in ipairs(entries) do
|
||||
local mine = chords_of(entry.sequence)
|
||||
for _, fixed in ipairs(FIXED_KEYS) do
|
||||
if entry.sequence == fixed[1] then
|
||||
error(string.format(
|
||||
"listview: `keys` may not rebind %q --- it is part of the panel's "
|
||||
.. "own key surface (RET SPC n <down> p <up> TAB g q), bound to %q",
|
||||
entry.sequence, fixed[2]))
|
||||
end
|
||||
if prefix_conflict(mine, chords_of(fixed[1])) then
|
||||
error(string.format(
|
||||
"listview: `keys` entry %q conflicts with the panel's own %q --- "
|
||||
.. "one is a prefix of the other, which the keymap refuses rather "
|
||||
.. "than turning a binding into a submap",
|
||||
entry.sequence, fixed[1]))
|
||||
end
|
||||
end
|
||||
for j = i + 1, #entries do
|
||||
if prefix_conflict(mine, chords_of(entries[j].sequence)) then
|
||||
error(string.format(
|
||||
"listview: `keys` entries %q and %q conflict --- one is a prefix "
|
||||
.. "of the other", entry.sequence, entries[j].sequence))
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
-- Bind the entries, naming which one the keymap refused.
|
||||
--
|
||||
-- It does NOT roll back the keys it already bound: its caller owns
|
||||
-- teardown, and the caller's teardown is killing the whole buffer,
|
||||
-- which takes the buffer's entire keymap scope with it
|
||||
-- (`after_buffer_removed` -> `KeymapStack::remove_buffer`). Unbinding
|
||||
-- here as well would be a second, weaker cleanup mechanism for the same
|
||||
-- failure --- and the weaker one is what let a half-built panel survive.
|
||||
local function install_keys(buf, entries)
|
||||
for _, entry in ipairs(entries) do
|
||||
local ok, err = pcall(pmacs.keymap.bind, {
|
||||
scope = "buffer", buffer = buf,
|
||||
sequence = entry.sequence, command = entry.command,
|
||||
})
|
||||
if not ok then
|
||||
error(string.format(
|
||||
"listview: cannot bind %q to %q: %s",
|
||||
entry.sequence, entry.command, tostring(err)))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
local function keys_match(a, b)
|
||||
if #a ~= #b then return false end
|
||||
for i = 1, #a do
|
||||
if a[i].sequence ~= b[i].sequence or a[i].command ~= b[i].command then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
end
|
||||
|
||||
local function render_keys(entries)
|
||||
if #entries == 0 then return "none" end
|
||||
local parts = {}
|
||||
for i, entry in ipairs(entries) do
|
||||
parts[i] = string.format("%s=%s", entry.sequence, entry.command)
|
||||
end
|
||||
return table.concat(parts, " ")
|
||||
end
|
||||
|
||||
-- Build the persistent panel record for `name`. A user-killed panel
|
||||
|
|
@ -293,9 +468,23 @@ end
|
|||
-- collision disambiguates `<2>`..`<99>`, and exhausting the limit raises
|
||||
-- rather than adopting --- the rule terminal.lua:300-305 states and
|
||||
-- dired.lua:476-504 already implements.
|
||||
local function ensure_panel(name)
|
||||
local function ensure_panel(name, key_entries)
|
||||
local p = panel_for_requested_name(name)
|
||||
if p then return p end
|
||||
if p then
|
||||
-- Match-on-reopen (Q#G-7). A live panel keeps the keys it was
|
||||
-- created with; a DIFFERENT table is a consumer asking for
|
||||
-- something it will not get, so it is an error rather than a
|
||||
-- silently ignored request.
|
||||
if not keys_match(p.keys, key_entries) then
|
||||
error(string.format(
|
||||
"listview: %s is already open with keys [%s]; this open asks for "
|
||||
.. "[%s]. Keys are installed once with the panel's buffer, so the "
|
||||
.. "second table would be silently ignored --- close the panel "
|
||||
.. "first, or pass the same keys",
|
||||
name, render_keys(p.keys), render_keys(key_entries)))
|
||||
end
|
||||
return p
|
||||
end
|
||||
|
||||
local actual = name
|
||||
if find_buffer_by_name(actual) then
|
||||
|
|
@ -315,29 +504,64 @@ local function ensure_panel(name)
|
|||
|
||||
local buf = pmacs.buffer.create(actual)
|
||||
p = { requested_name = name, buffer = buf, line_to_item = {},
|
||||
line_to_row = {}, collapsed = {}, rows = {}, visible = 0 }
|
||||
panels[#panels + 1] = p
|
||||
-- Read-only (Q#P3): every non-bypass edit is rejected, with a NAMED
|
||||
-- error. Kept beside the rope lock, not replaced by it: the layering
|
||||
-- at terminal.lua:351-366 --- the rope lock protects the daemon copy,
|
||||
-- this and the round-trip mark protect a semantic frontend's own
|
||||
-- mirror, and neither substitutes for the other. The intercept lives
|
||||
-- as long as the buffer; no teardown (the buffer-list precedent for
|
||||
-- its keymap).
|
||||
pmacs.buffer.add_intercept(buf, function()
|
||||
error(actual .. " is read-only")
|
||||
line_to_row = {}, collapsed = {}, rows = {}, visible = 0,
|
||||
keys = key_entries }
|
||||
-- ALL-OR-NOTHING from here. Everything below mutates a buffer that
|
||||
-- does not yet belong to a panel, and `install_keys` can genuinely
|
||||
-- fail: the raw-token preflight cannot see an alias spelling of a
|
||||
-- fixed key (`RETURN` for `RET`), so `Keymap::bind` is the first thing
|
||||
-- to notice, and by then the buffer exists, carries a read-only
|
||||
-- intercept and a round-trip mark, and holds the fixed keymap.
|
||||
--
|
||||
-- Leaving it behind is worse than it sounds: it is read-only, it is in
|
||||
-- no `panels` record so nothing owns or can reach it, and the next
|
||||
-- `open` for the same name finds it by name and disambiguates itself
|
||||
-- to `<2>` --- so a rejected `keys` table silently renames the panel.
|
||||
local built, err = pcall(function()
|
||||
-- Read-only (Q#P3): every non-bypass edit is rejected, with a NAMED
|
||||
-- error. Kept beside the rope lock, not replaced by it: the layering
|
||||
-- at terminal.lua:351-366 --- the rope lock protects the daemon copy,
|
||||
-- this and the round-trip mark protect a semantic frontend's own
|
||||
-- mirror, and neither substitutes for the other. The intercept lives
|
||||
-- as long as the buffer; no teardown (the buffer-list precedent for
|
||||
-- its keymap).
|
||||
pmacs.buffer.add_intercept(buf, function()
|
||||
error(actual .. " is read-only")
|
||||
end)
|
||||
-- Q#P6: semantic frontends must round-trip keys while this panel
|
||||
-- is focused (RET = visit, not an optimistic newline).
|
||||
pmacs.buffer.set_round_trip_input(buf, true)
|
||||
bind_local_keymap(buf)
|
||||
install_keys(buf, key_entries)
|
||||
end)
|
||||
-- Q#P6: semantic frontends must round-trip keys while this panel
|
||||
-- is focused (RET = visit, not an optimistic newline).
|
||||
pmacs.buffer.set_round_trip_input(buf, true)
|
||||
bind_local_keymap(buf)
|
||||
if not built then
|
||||
-- `kill` is the whole teardown, not a convenience: it removes the
|
||||
-- buffer AND, through `after_buffer_removed`, prunes the buffer's
|
||||
-- keymap scope, its config locals and its folds. Unbinding key by
|
||||
-- key would leave the buffer itself --- which is the defect.
|
||||
pcall(pmacs.buffer.kill, buf)
|
||||
-- Level 0: re-raise the inner message verbatim rather than stacking
|
||||
-- this line's position onto it.
|
||||
error(err, 0)
|
||||
end
|
||||
-- Registered LAST, deliberately: a failure above must leave no record
|
||||
-- claiming keys it did not bind. Nothing above needs the panel to be
|
||||
-- in `panels` --- the intercept, the round-trip mark and the keymap
|
||||
-- all address the buffer directly.
|
||||
panels[#panels + 1] = p
|
||||
return p
|
||||
end
|
||||
|
||||
function pmacs.listview.open(spec)
|
||||
assert(type(spec) == "table" and type(spec.name) == "string",
|
||||
"listview.open: spec.name (string) required")
|
||||
local p = ensure_panel(spec.name)
|
||||
-- The cheap checks first, so the common mistakes are named before
|
||||
-- anything is created. The ones this pass cannot see --- alias
|
||||
-- spellings --- are caught by `Keymap::bind` inside `ensure_panel`,
|
||||
-- which tears the panel down rather than leaving it half-built.
|
||||
local key_entries = normalized_keys(spec.keys)
|
||||
check_key_collisions(key_entries)
|
||||
local p = ensure_panel(spec.name, key_entries)
|
||||
p.header = spec.header or spec.name
|
||||
p.on_visit = spec.on_visit
|
||||
p.on_refresh = spec.on_refresh
|
||||
|
|
|
|||
|
|
@ -367,6 +367,444 @@ also removed: this branch's "R8 NEEDS A LANE" investigation block, and
|
|||
durable facts are in the retired registry row and the handoff §6
|
||||
census.
|
||||
|
||||
## Git integration Stage 1 — PR #227 OPEN
|
||||
|
||||
**PR #227** — https://github.com/levineuwirth/pmacs/pull/227. Opened
|
||||
2026-08-09 at `4002734`, after the framing was approved at revision 5
|
||||
and the full gate suite went green. **UNBLOCKED 2026-08-10**: `main` was
|
||||
merged in (72 commits) and the destination capture #231 provides is
|
||||
adopted below. Re-gated 12/12 green on the merged tree.
|
||||
|
||||
**One cross-lane break the merge surfaced**, recorded because it was
|
||||
invisible until the suites ran: #232 made `purpose` **required** on
|
||||
`pmacs.process.spawn`, and this module's spawn lives on this branch, so
|
||||
it was never among the 11 call sites #232 updated — every git test
|
||||
failed at once. Each of the three spawns now carries its own purpose,
|
||||
and deliberately **not** the label, which is the copy #232's ruling was
|
||||
made against: all three are labelled `git`, and only the purpose
|
||||
distinguishes "resolving which repository contains `<dir>`" from
|
||||
"reading the working tree status of `<root>`" from "diffing `<path>`
|
||||
against HEAD".
|
||||
|
||||
**Review round 1 found three blockers. Two are fixed; the third is why
|
||||
this lane is blocked.**
|
||||
|
||||
- **P1 fixed (`ffe5ae2`) — concurrent `status` opens were ordered by
|
||||
`rev-parse` completion, not by invocation.** The generation was
|
||||
minted on arrival, inside `start_status`, so the **slowest root
|
||||
lookup won** rather than the newest invocation. It is now reserved at
|
||||
the command and carried through the root-resolution callback, which
|
||||
drops a superseded result **before any effect** — no spawn, no
|
||||
`state.root` write, and no status message either, since a message
|
||||
from a replaced invocation is as wrong as a panel from one.
|
||||
- **P2 fixed (`6c1631e`) — a refused `keys` bind left a live, unowned
|
||||
buffer.** The preflight compared **raw tokens** while `parse_key_code`
|
||||
folds `RET`/`RETURN`/`ENTER` (and `SPC`/`SPACE`, `ESC`/`ESCAPE`,
|
||||
`BS`/`BACKSPACE`, `DEL`/`DELETE`, case-insensitively) onto one chord,
|
||||
so an alias spelling passed preflight and failed at bind time —
|
||||
*after* buffer creation and intercept install. The partial rollback
|
||||
removed only new keys, so later opens silently got `<2>`.
|
||||
|
||||
**Fixed by full teardown, not by canonicalizing the preflight**, and
|
||||
the reasoning is worth keeping: a Lua canonicalizer would be a second
|
||||
copy of the Rust alias table and would go stale the day that table
|
||||
gains a name, reintroducing this exact bug for the new alias.
|
||||
`Keymap::bind` is the authority because it *is* what decides. There
|
||||
is also **no Lua-reachable canonicalization** to build on —
|
||||
`display_sequence` escapes only through `describe.key` and
|
||||
`keymap.list`, both of which require the sequence to be bound
|
||||
already. Verified; no binding was added for this.
|
||||
- **P1a FIXED — the block is lifted.** The async completions displayed
|
||||
UI without capturing the initiating frontend, so a result surfaced in
|
||||
whichever frontend was active when git exited. `commit_to` was the
|
||||
right mechanism and was **not Lua-reachable** outside a directory
|
||||
open, so the capture half shipped as **#231** (`0e4c58d`) and this
|
||||
lane adopts it now that `main` carries it:
|
||||
|
||||
- **Captured at invocation** in all four entrances — `git.status()`,
|
||||
`_on_refresh` (the `g` keypress), `_deliver_root`'s hand-off, and
|
||||
the `git.diff-file` command — and threaded on the request table
|
||||
exactly as the generation and root already were. The ambient
|
||||
frontend was the one input still being read late.
|
||||
- **Committed under the profile the surface actually takes**:
|
||||
`"panel"` for `*git-status*`, `"document"` for `*git-diff*` (Q#DC-2).
|
||||
- **`set_status` moved INSIDE the status commit.** A failure message
|
||||
announcing a panel that the commit then refuses is the same
|
||||
misrouting in its most confusing form, so rows and message are now
|
||||
computed first and emitted together.
|
||||
- **Witnessed by `g6_25`**, and the first version of that test was
|
||||
**worthless**: `panel_text` finds `*git-status*` by NAME, which is
|
||||
global, so a render into the wrong frontend satisfied it and the
|
||||
test passed its own mutation. Rewritten per frontend
|
||||
(`side_window_for`, and each view's active window) it fails both
|
||||
bites — removing the status commit grows a `*git-status*` panel in
|
||||
the competitor, removing the diff commit hands it the document
|
||||
window.
|
||||
|
||||
**The second citation written here in round 1 was wrong** — `:854`
|
||||
pointed at `local unstaged = …` inside `diff_plan`, not at a display
|
||||
call. The site was always `show_diff_buffer`'s `pmacs.window.display`.
|
||||
Corrected rather than silently re-numbered, because a stale pointer in
|
||||
a block whose whole purpose is "do not touch these two lines" is worse
|
||||
than none.
|
||||
|
||||
**Re-gated at `6c1631e`:** all 11 steps green, acceptance now 27 tests.
|
||||
Both fixes mutation-verified.
|
||||
|
||||
**Review round 2 found three more P2s, all the SAME SHAPE as the round-1
|
||||
P1: module-level mutable state read or written at CONTINUATION time
|
||||
instead of captured at INVOCATION time.** All three fixed here; the P1a
|
||||
block above is unchanged and still the reason this lane cannot merge.
|
||||
The fourth recurrence is why the last fix generalizes the rule instead
|
||||
of adding another counter, and why the census below exists.
|
||||
|
||||
- **P2 fixed (`3eca5e8`) — an unborn-repository diff could switch
|
||||
repositories mid-plan.** `run_diff_plan`'s `next_step` read
|
||||
`state.root` each time it started a step, and an unborn `AM`/`AD` row
|
||||
produces a **two-step** plan. A concurrent `git.status` for another
|
||||
repository reassigns `state.root` from its own root-resolution
|
||||
callback, so a diff started in A ran its second step with B as cwd and
|
||||
A's path — git there matches nothing, so the unstaged half silently
|
||||
rendered `(no changes)` instead of the worktree delta it exists to
|
||||
show. The root is now captured at the keypress and threaded through as
|
||||
a parameter; `state.root` is not read inside the plan at all.
|
||||
- **P2 fixed (`842ec61`) — a repository root containing a newline was
|
||||
truncated.** `rev-parse --show-toplevel` was parsed with `first_line`,
|
||||
so a root at `/tmp/a\nb` became `/tmp/a` and every command after it ran
|
||||
with a nonexistent cwd. Fixed with a **separate** helper,
|
||||
`strip_output_terminator`, at that one call site. `first_line` is
|
||||
deliberately untouched: its other three callers all feed the
|
||||
**single-line status band**, where truncating is right, so folding the
|
||||
two together would fix one caller and break three.
|
||||
|
||||
**A third instance of the shape was found in the same pass and reported
|
||||
rather than fixed silently:** the diff path had no generation counter at
|
||||
all. Review independently raised it as **P2 #3**, and it is fixed below.
|
||||
|
||||
- **P2 fixed (`723afa7`) — concurrent `d` requests were
|
||||
last-writer-wins.** `git.diff-file` started a plan with **no request
|
||||
generation** while every plan writes the **singleton** `*git-diff*`
|
||||
buffer through `show_diff_buffer`. `d` on A, then `d` on B before A
|
||||
finishes: if A completed last, A's diff replaced B's. Reachable
|
||||
without contrivance — `d` renders into the document window only at
|
||||
completion, so the panel is still focused for a second press.
|
||||
|
||||
**Fixed by giving the rule ONE implementation instead of a fourth
|
||||
hand-rolled counter.** `new_channel()` hands out a ticket at the
|
||||
command and answers "is this still the request in force?" at the
|
||||
completion; `state.generation` and `reserve_generation` are gone.
|
||||
|
||||
**Two channels, not one, and that is a design decision.** A single
|
||||
module-wide counter would make `d` cancel an in-flight `g` and vice
|
||||
versa. The status panel and the diff view are independent things a
|
||||
user asks for, so each gets its own ordering; what is shared is the
|
||||
**mechanism**, not the counter. A channel spans a whole **request**
|
||||
rather than one process — a status open is `rev-parse` then `status` —
|
||||
so `_deliver_root` and `_deliver_status` correctly share one ticket
|
||||
while a diff plan gets its own. `g6_23` asserts the separation: two
|
||||
`d` presses leave `_generation()` untouched.
|
||||
|
||||
The plan is restructured into the request shape the other two
|
||||
continuations already use: `step_done`'s closure becomes
|
||||
`pmacs.git._deliver_diff(request, step, res)`, exposed for the same
|
||||
reason `_deliver_status` and `_deliver_root` are.
|
||||
|
||||
**The async-continuation census, so the next round is not another
|
||||
instance hunt.** `git.lua` has **exactly three** async continuations,
|
||||
plus one dispatcher and one synchronous impostor:
|
||||
|
||||
| continuation | invocation-time ticket? | needs one? | shared state written |
|
||||
|---|---|---|---|
|
||||
| `_deliver_root` (`rev-parse`) | yes, `status_requests`, reserved in `git.status()` | yes | `state.root`, `state.buffer`, spawns the status |
|
||||
| `_deliver_status` (`git status`) | yes, `status_requests`, reserved at the command or the `g` keypress | yes | `state.branch`, `.rows`, `.display`, `.failure`, `.buffer`, the panel, the cursor |
|
||||
| `_deliver_diff` (`git diff`, 1–2 steps) | **now yes**, `diff_requests`, reserved at the `d` keypress | yes | `state.diff_buffer`, the `*git-diff*` contents, the status band |
|
||||
| `process.after-tick` pump | no | **no** — it is the dispatcher, not a request; it owns only the module-local `pump` table and calls each `on_done` once | `pump` |
|
||||
| `run_git`'s spawn-failure path | n/a | **no** — it calls `on_done` **synchronously**, at invocation time, and that `on_done` carries the ticket anyway | none |
|
||||
|
||||
Everything else that looks like a callback is synchronous at the
|
||||
keypress: `on_visit`/`on_refresh` on the listview spec, the `*git-diff*`
|
||||
read-only intercept, and the two `pmacs.command.define` bodies.
|
||||
|
||||
`state.diff_buffer` is deliberately still read at continuation time and
|
||||
that is correct: "do I already have a live diff buffer?" is a question
|
||||
about *now*, not about the invocation. It is the state `_deliver_diff`
|
||||
guards, not a second instance of the bug.
|
||||
|
||||
**Re-gated at `723afa7`:** all steps green, acceptance now 30 tests. All
|
||||
three round-2 fixes mutation-verified — `g6_22` fails on the second
|
||||
spawned diff argv when the `state.root` read is restored; `g6_14c`
|
||||
resolves `<tmp>/nl` instead of `<tmp>/nl\nroot` when `first_line` is;
|
||||
and `g6_23` fails when the ticket check is removed, at its **real**
|
||||
half, the older plan having overwritten the newer one's patch before the
|
||||
driven delivery was reached.
|
||||
|
||||
**Review round 3 found a FOURTH instance of the byte/lifetime shape, and
|
||||
it was one byte inside round 2's own fix.**
|
||||
|
||||
- **P2 fixed (`39ad43d`) — a repository root ENDING IN A CARRIAGE RETURN
|
||||
was truncated.** `strip_output_terminator` stripped `\r?\n$`, and `\r`
|
||||
is as legal a byte in a POSIX directory name as `\n` is. For a root
|
||||
named `trailing\r`, `git rev-parse --show-toplevel` prints
|
||||
`…/trailing` `0d` `0a` — the path's own CR, then git's LF terminator —
|
||||
and a pattern tolerant of an optional preceding carriage return cannot
|
||||
tell those apart, so it took both. The root resolved as `…/trailing`
|
||||
and every command after it ran with that as its `-C` and cwd: a
|
||||
directory that does not exist. Now **exactly one trailing `\n`** is
|
||||
removed, by an explicit last-byte test rather than an anchored pattern
|
||||
— both of this function's bugs lived in a pattern.
|
||||
|
||||
**`-z` was CHECKED against the installed git, not assumed, and must
|
||||
NOT be used.** `git rev-parse` has no `-z` option at all on git 2.55:
|
||||
it is absent from the manual, `--parseopt -z` errors with "unknown
|
||||
switch", and in ordinary mode `rev-parse` treats `-z` as an
|
||||
unrecognized **flag argument** and echoes a literal `-z\n` onto stdout
|
||||
**ahead of** the toplevel — exit code 0, corrupted output, silent.
|
||||
`--show-toplevel` applies no C quoting either, not even under
|
||||
`core.quotePath=true`. So there is no unambiguous representation to
|
||||
prefer over a correct strip, and removing the one byte git appended is
|
||||
the whole of the right answer.
|
||||
|
||||
`first_line` is untouched again, for the reason `842ec61` recorded: its
|
||||
three callers all feed the single-line status band.
|
||||
|
||||
**Re-gated at `39ad43d`:** all steps green, acceptance now 31 tests.
|
||||
`g6_14d` is end to end — real directories, the real `git`, asserted on
|
||||
the cwd of the spawn the module actually made — and covers both
|
||||
`trailing\r` and `nl\nand-trailing\r`, the second because the two hazards
|
||||
compose and neither fix may mask the other. `g6_14c` now shares that
|
||||
chain through `assert_root_resolves_whole` rather than keeping a second
|
||||
copy of it. Mutation-verified: restoring `\r?\n$` fails `g6_14d` at
|
||||
`<tmp>/trailing` against `<tmp>/trailing\r` while `g6_14c` still passes,
|
||||
which is exactly the byte separating the two fixes.
|
||||
|
||||
**CI round: a DETERMINISTIC macOS failure, in the FIXTURE rather than in
|
||||
the product.** Both macOS legs of the matrix (LuaJIT and Lua 5.4) failed
|
||||
`g6_2` identically at
|
||||
https://github.com/levineuwirth/pmacs/actions/runs/31324683235 while
|
||||
Linux stayed green.
|
||||
|
||||
- **DURABLE PORTABILITY FACT, and this project will hit it again:
|
||||
macOS cannot hold a non-UTF-8 filename.** APFS and HFS+ validate
|
||||
pathnames as UTF-8 and reject an invalid one at the syscall with
|
||||
**errno 92, `EILSEQ`, "Illegal byte sequence"**. Linux's VFS treats a
|
||||
filename as opaque bytes and accepts it. So `std::fs::write` on
|
||||
`bad\xFF.txt` is a Linux-only fixture, and any test that builds one is
|
||||
red on macOS by construction, not by flake.
|
||||
|
||||
It goes further than creation: the name cannot be reached *around* the
|
||||
filesystem either. Putting it only in the index (`update-index
|
||||
--index-info` plus `write-tree`, never touching the worktree) does not
|
||||
help, because `git status` lstats every index entry and on macOS that
|
||||
lstat fails with `EILSEQ` rather than `ENOENT` — which git reports on
|
||||
stderr and **skips**, so the row would be absent rather than
|
||||
unrepresentable. **There is no macOS arrangement in which real `git
|
||||
status` names a non-UTF-8 path at all.**
|
||||
|
||||
- **Fixed (`4b82d1e`) by splitting the coverage along the line the
|
||||
platform actually draws, NOT by `#[cfg]`-skipping the behaviour.**
|
||||
A behaviour that vanishes on one platform is how a boundary stops
|
||||
being tested; the behaviour now runs everywhere and only the
|
||||
*provenance* is gated.
|
||||
|
||||
| test | what it witnesses | where it runs |
|
||||
|---|---|---|
|
||||
| `g6_2` | parse + display, driven from the **payload bytes** — no repository, no filesystem | every platform |
|
||||
| `g6_2b` | RET and `d` **refusing with a message**, over a real repository, with the row delivered through `_deliver_status` | every platform |
|
||||
| `g6_2c` | that real `git` emits those bytes at all | **Linux only**, loudly named and commented |
|
||||
|
||||
The gestures are exercised against a **real** repository, panel,
|
||||
keymap and dispatch; only the row bytes are supplied, through the same
|
||||
`_deliver_status` seam `g6_17` and `g6_21` already use because a
|
||||
chosen completion is not otherwise expressible. `g6_2b` also gained
|
||||
the **rename-ORIGIN** case, which the old single test never had: `d`
|
||||
passes the origin to `git diff` as an argument too, so a check written
|
||||
on `row.path` alone would let it through.
|
||||
|
||||
The one link no payload can witness — that the spawn pipe carries
|
||||
bytes rather than text — is **structural**: `event_to_lua` in
|
||||
`src/lua_bindings/mod.rs` builds the stdout chunk with
|
||||
`lua.create_string(bytes)`, and `git.lua` only concatenates chunks.
|
||||
|
||||
- **New fixture mechanism: `lua_bytes` / `z_payload_bytes`.** A `-z`
|
||||
payload whose paths are not UTF-8 **cannot be spelled as a Rust
|
||||
`&str`**, so it is assembled as raw bytes and handed to Lua as one
|
||||
literal, with every non-printable byte spelled as a **three-digit**
|
||||
decimal escape. Three digits always: Lua's decimal escape consumes up
|
||||
to three, so a shorter one swallows the digit after it — the same
|
||||
hazard the `{:?}`-on-NUL note above records, removed rather than
|
||||
worked around.
|
||||
|
||||
- **Verified here vs. reasoned about.** Verified locally: the full gate
|
||||
suite green; 33/33 under **both** LuaJIT and Lua 5.4; the two portable
|
||||
tests still green with `g6_2c` compiled out (a stand-in for the macOS
|
||||
build, with no dead-code warnings left behind); three mutations each
|
||||
caught by `g6_2b` — removing the RET check, removing the `d` check,
|
||||
and removing only the origin clause. Reasoned about, not executed: the
|
||||
macOS `EILSEQ` behaviour itself and the `lstat`-vs-`ENOENT` argument
|
||||
above. What is *no longer* reasoned about is the important part —
|
||||
after this change nothing macOS runs depends on a filesystem accepting
|
||||
such a name.
|
||||
|
||||
- **CONFIRMED ON THE REAL MATRIX.** Run
|
||||
https://github.com/levineuwirth/pmacs/actions/runs/31330601204 at
|
||||
`e816812`: **all 14 jobs green**, including
|
||||
`Test (macos-latest / luajit)` and `Test (macos-latest / lua54)` — the
|
||||
two that were red. So the macOS half is now OBSERVED rather than
|
||||
reasoned about; what stays reasoned about is only the *explanation*
|
||||
(`EILSEQ`, and the `lstat`-vs-`ENOENT` argument for why `g6_2c` cannot
|
||||
be made portable), which nothing in CI can confirm or refute.
|
||||
|
||||
- **LATENT SIBLING, out of scope and NOT red today:**
|
||||
`tests/gpu_invocation_acceptance.rs:621` writes
|
||||
`OsString::from_vec(vec![b'r', b'a', b'w', 0xff])` to disk. It sits
|
||||
inside `#[cfg(feature = "crdt")] mod crdt`, and the `crdt-test` job is
|
||||
**ubuntu-only**, so it never runs on macOS. It would fail the same way
|
||||
the day that job gains a macOS leg. No other test in the tree builds a
|
||||
platform-hostile path: `g6_14c`/`g6_14d`'s `nl\nroot` and `trailing\r`
|
||||
are valid UTF-8 and legal on APFS, which is why they were green on
|
||||
macOS all along.
|
||||
|
||||
**Review round 4: a COPY was being reported as a RENAME.**
|
||||
|
||||
- **Fixed at presentation, not in `kind`.** Porcelain v2's `2` record
|
||||
covers renames **and** copies — `<Xscore>` leads with `R` or `C` —
|
||||
and the parser already retained `score`. The diff header now reads
|
||||
that byte and says `copied from` or `renamed from`; nothing new is
|
||||
parsed.
|
||||
|
||||
**`kind` stays `"rename"` for both, deliberately.** Every *behaviour*
|
||||
keyed on it is identical, including the two-path
|
||||
`git diff HEAD -- <orig> <current>`, which is correct for a copy as
|
||||
much as for a rename. Splitting the kind would force every consumer
|
||||
present and future to spell `kind == "rename" or kind == "copy"`, and
|
||||
an arm forgotten anywhere silently drops copies back to the one-path
|
||||
diff — the exact regression the fix exists to avoid. Consumers
|
||||
checked, and there are few: `diff_plan` (the only `kind == "rename"`
|
||||
branch in the tree), `status_line_text` (keys off `row.orig`, not
|
||||
`kind`), `g6_1`'s corpus assertion and `g6_8`'s unborn-unreachability
|
||||
assertion. `score` has no other reader anywhere.
|
||||
|
||||
- **Read from `score`, not from `row.x`.** The score field names
|
||||
rename-vs-copy whichever side detected the change; `X` carries the
|
||||
letter only for an index-side one, a worktree-side detection leaving
|
||||
`X` a `.`.
|
||||
|
||||
- **The status ROW is unchanged, and that is a decision.** Its `XY`
|
||||
prefix already reads `R.` against `C.`, out of the same byte, in the
|
||||
porcelain vocabulary every other row is read in — so the distinction
|
||||
is already on screen and a second vocabulary beside it would be the
|
||||
wider surface for no new fact. `g6_4b` asserts both prefixes, so the
|
||||
claim is checked rather than asserted here.
|
||||
|
||||
- **Parser-level coverage, stated plainly rather than implied.** The
|
||||
copy ROW is supplied through `_deliver_status`, the seam
|
||||
`g6_2b`/`g6_17`/`g6_21` already use.
|
||||
|
||||
**Scope corrected in review round 5.** This entry claimed real `git`
|
||||
emits no `2 C` record at all, measured under
|
||||
`-c status.renames=copies`. **The measurement was real; the claim
|
||||
drawn from it was too broad.** `git-status(1)` documents `C` as
|
||||
"copied (if config option status.renames is set to `copies`)", so
|
||||
git does emit it. What the test establishes is that **this fixture**
|
||||
— whose copy source is unchanged — yields `1 A.`. That is enough to
|
||||
justify crafting the row and nothing more. Everything downstream is real: repository,
|
||||
panel, `d` dispatch, spawned `git diff`, rendered buffer. Both
|
||||
crafted rows name paths that exist in the fixture, so each drives a
|
||||
real two-path diff.
|
||||
|
||||
- **Unborn `HEAD` needed nothing, confirmed rather than assumed.**
|
||||
`diff_plan`'s rename branch is inside `if not unborn`, and `g6_8`
|
||||
already pins that no `2` record can occur there — over
|
||||
`kind == 'rename'`, which under this choice covers copies too.
|
||||
|
||||
**Re-gated:** all steps green, acceptance now **34 tests**. Three
|
||||
mutations, each caught: header always `renamed` fails only the copy
|
||||
half; header always `copied` fails only the rename half; dropping
|
||||
`row.orig` from the steps fails the argv equality. The two `--lib`
|
||||
failures seen on an earlier run (`composition_overhead_under_ten_percent`,
|
||||
`setsid_escapee_…`, plus two perf tests) were **machine load from
|
||||
sibling worktrees** — load average 15–27 — and pass in isolation and on
|
||||
a re-run; nothing in this change touches `src/`.
|
||||
|
||||
**Written with the lane's first commit, before the PR exists** — the
|
||||
standing correction from #171 and #215. This session it was missed on
|
||||
#224 and again on #225, both caught by review; writing it now is the
|
||||
only thing that stops a third.
|
||||
|
||||
**Branch `git-status-stage1`**, base `githubsucks/main` @ `4bc55e8`
|
||||
(the #225 merge). **`githubsucks/git-status-stage1` is the
|
||||
authoritative tip** — the ref, not a SHA. Recover with
|
||||
`git fetch githubsucks && git checkout git-status-stage1`.
|
||||
|
||||
- **Framing `docs/git-integration-framing.md`, revision 5, APPROVED
|
||||
2026-08-09** after four review rounds. Every round found something
|
||||
the previous one had asserted without reading; the doc records which.
|
||||
- **Scope:** `*git-status*` (a `listview` panel over
|
||||
`git --no-optional-locks -C <dir> status --porcelain=v2 --branch -z`)
|
||||
and `*git-diff*` (plain generated text, file-level, no hunk model).
|
||||
Plus **one additive `listview` change**: an optional `keys` table,
|
||||
install-once with match-on-reopen, because `Keymap::bind` refuses
|
||||
duplicates and the refresh path re-opens.
|
||||
- **NO WIRE CHANGE**, and that is load-bearing for scheduling:
|
||||
`PROTOCOL_VERSION` is a strict serialization point, so this lane can
|
||||
run concurrently with other work. **Stage 2 (gutter markers) needs
|
||||
new `DecorationKind` variants and must be scheduled alone.**
|
||||
- **Known negative coherence impact (§9):** git runs as a spawned
|
||||
process, and spawned processes do not appear in `*workers*` — that is
|
||||
`async.lua`'s job list. This adds a fifth unattributable background
|
||||
thing. Labelled honestly; a label is not attribution.
|
||||
- **Verification plan** in framing §6. The load-bearing cases: an `AM`
|
||||
unborn fixture (two labelled patches), untracked diff rendering on
|
||||
**exit 1** (`--no-index` implies `--exit-code`), two successive
|
||||
refreshes not raising `DuplicateBinding`, and a non-UTF-8 path that
|
||||
parses and displays but **refuses** its gestures at the
|
||||
`String`-typed binding boundary.
|
||||
- **Gates:** `scripts/gate --acceptance git_status_stage1_acceptance`.
|
||||
|
||||
**Implemented.** `builtin/runtime/git.lua` (new, loaded after
|
||||
`linewrap.lua`), the `keys` extension in `builtin/runtime/listview.lua`,
|
||||
one chunk-load line in `src/editor.rs`, and
|
||||
`tests/git_status_stage1_acceptance.rs` (25 tests, one per §6 bullet).
|
||||
No `pmacs-protocol` change, no `PROTOCOL_VERSION` change, no
|
||||
`DecorationKind` change — the no-wire property held.
|
||||
|
||||
Five things worth carrying, all found by biting the suite rather than by
|
||||
reading:
|
||||
|
||||
- **`listview.open`'s `seat_cursor` walks DOWN from wherever the cursor
|
||||
is**, on the premise that a fresh `switch_active_buffer` zeroed it.
|
||||
Re-opening an already-displayed panel — which is exactly what the
|
||||
async completion model does — zeroes nothing, so the walk lands one
|
||||
row *below* the previous cursor. The completion handler seats
|
||||
unconditionally from line 0 instead of trusting `open`.
|
||||
- **A selection test that inserts ONE row above the selection is
|
||||
vacuous**, because that accidental off-by-one lands on the right row.
|
||||
The fixture inserts two.
|
||||
- **`{:?}` on a Rust string containing NUL cannot build a `-z` fixture.**
|
||||
Debug renders NUL as `\0`, and Lua's decimal escape swallows the
|
||||
digits after it — so `\0` before a `1` record becomes
|
||||
`string.char(1)` and the record merges into its predecessor. One test
|
||||
passed while parsing nothing: the merged text landed in
|
||||
`# branch.head`, the panel header rendered it, and a `contains`
|
||||
assertion on the panel text was satisfied by the header. Payloads are
|
||||
joined in Lua with `string.char(0)`.
|
||||
- **A path may contain a newline, so a panel row must escape it.**
|
||||
Parsing the bytes correctly and then writing them raw into a
|
||||
one-row-per-line buffer desynchronizes every line-to-row map — and the
|
||||
rope is UTF-8 by project invariant, so non-UTF-8 path bytes cannot go
|
||||
in at all. Rows render `\xNN` escapes; the raw bytes stay on the
|
||||
record, where the refusal check reads them.
|
||||
- **Untracked rows sort AFTER every tracked row** in porcelain v2, so an
|
||||
untracked file cannot be used to reorder a list above a selection.
|
||||
|
||||
Two deliberate deviations from the framing's letter, both narrow:
|
||||
`--no-color` on every diff invocation (a user with `color.ui = always`
|
||||
would otherwise get escape sequences in a buffer with no ANSI parser
|
||||
behind it), and `pmacs.git._program`, a module-local that the
|
||||
missing-binary witness points at a name not on `PATH`. There is no other
|
||||
in-process route to that branch: Rust's `Command` resolves the program
|
||||
against the **parent** process's `PATH`, so a child `env` cannot hide
|
||||
git, and `std::env::set_var` is `unsafe` in edition 2024.
|
||||
## Destination capture (Q#JR14 generalization) — PR #231 OPEN, revision 9, cleared to merge
|
||||
|
||||
**PR #231** — https://github.com/levineuwirth/pmacs/pull/231. #227
|
||||
|
|
@ -1499,6 +1937,7 @@ authoritative tip** — the ref, not a SHA. Recover with
|
|||
— added in the second round — a **rename of either** the build or the
|
||||
sweep step each fail the suite.
|
||||
|
||||
|
||||
## QoL arc retirement — PR #224 OPEN (docs only)
|
||||
|
||||
**PR #224** — https://github.com/levineuwirth/pmacs/pull/224. Written
|
||||
|
|
|
|||
|
|
@ -0,0 +1,708 @@
|
|||
# Git integration — Stage 1: seeing what changed
|
||||
|
||||
**Status: revision 5, APPROVED 2026-08-09. Implementation may
|
||||
proceed.**
|
||||
|
||||
**Revision 5 completes the unborn-repository policy, which revision 4
|
||||
wrote as three disjoint rows when a single file can be in two states at
|
||||
once.** `AM` — staged, then edited again — is not exotic; it is what a
|
||||
first commit looks like halfway through. The states below were
|
||||
**enumerated from a real unborn repository**, not reasoned about, and
|
||||
one of them settles a case by ruling it out entirely.
|
||||
|
||||
**Revision 4 fixes two contracts that would have failed in ordinary
|
||||
use, both verified against real behaviour rather than reasoned about:**
|
||||
re-binding `d` on every refresh (keymap binds refuse duplicates, so
|
||||
*every successful refresh* would have errored), and two git exit states
|
||||
the failure predicate got wrong. Measured, not assumed — the exit codes
|
||||
below were produced in a scratch repository.
|
||||
|
||||
**Revision 3 pins four Stage 1 contracts revision 2 left loose, and two
|
||||
of those were again claims I made without reading the code I was
|
||||
crediting.** I attributed selection preservation to `listview` and
|
||||
`d` to its key surface; neither is true, and both were checkable in the
|
||||
file I had already cited. The pattern is worth naming since it has now
|
||||
recurred across three revisions: **I cite a file, then describe what I
|
||||
expect it to contain.**
|
||||
|
||||
**Revision 2 answers four blockers, two of which were factual errors in
|
||||
revision 1 that scouting should have caught and did not.** I read
|
||||
`ProjectKind::Git`'s name instead of its doc comment, and I quoted
|
||||
`COHERENCE.md` §15's "no Git integration anywhere in the tree" without
|
||||
checking whether it was still true of the tree. It is not.
|
||||
|
||||
---
|
||||
|
||||
## 1. Why this, and why now
|
||||
|
||||
`COHERENCE.md` §15 is blunt about it:
|
||||
|
||||
> **There is no Git integration at all** — no status, stage, diff,
|
||||
> blame, or gutter markers anywhere in the tree (gutter git riders and
|
||||
> the `ResourceOffer` diff/blame family are named deferrals). The Git
|
||||
> affordance list above has nothing to attach to yet.
|
||||
|
||||
**That sentence is literally false about the tree, and revision 1
|
||||
repeated it without checking.** `tests/fixtures/pmacs-magit/` is a
|
||||
tracked, installable package — 1,914 lines across four modules, with
|
||||
`status.lua` spawning git through `pmacs.process.spawn` and parsing
|
||||
**`--porcelain=v2 --branch`** into structured sections, plus a 662-line
|
||||
acceptance suite (`tests/m8_6_acceptance.rs`, 32 tests) covering
|
||||
status, refresh, staging, commit, push and branch behaviour.
|
||||
|
||||
**The PRODUCT gap is real and unchanged** — none of that is bundled
|
||||
runtime, so a user who installs pmacs gets no git integration. But
|
||||
"nothing to attach to" understates what exists to *learn from*, and
|
||||
§15's wording should be corrected when this lands.
|
||||
|
||||
For a **daily driver**, this is the largest remaining gap. Not because
|
||||
git is the most architecturally interesting thing missing — §7
|
||||
workspaces and §9 worker identity are both deeper — but because it is
|
||||
the one a user touches *every working hour*, and pmacs currently makes
|
||||
them leave the editor to answer "what have I changed?".
|
||||
|
||||
That is the criterion this lane is chosen against: **frequency of use
|
||||
per day**, not depth of model.
|
||||
|
||||
## 2. Ground truth — what already exists
|
||||
|
||||
Scouted, not assumed:
|
||||
|
||||
- **`ProjectKind::Git` is NOT general repository detection**, and
|
||||
revision 1 said it was. Its doc comment is explicit: *"A bare git
|
||||
repository (no language marker found inside)"* (`src/project.rs:89`).
|
||||
Markers are ordered and a language marker beside `.git` **wins**
|
||||
(`src/project.rs:10`), so a normal Rust repository reports
|
||||
`kind = "rust"` and would have been invisible to a lane that gated on
|
||||
`kind == "git"`. That gate would have failed on this very repository.
|
||||
|
||||
**The rule this lane uses instead: never ask pmacs whether it is a
|
||||
git repo.** Run git in the **active file's directory** and let git
|
||||
resolve its own worktree — `git -C <dir> rev-parse --show-toplevel`
|
||||
establishes the root, and a non-zero exit *is* the "not a repository"
|
||||
answer. Git's own resolution handles submodules, worktrees, `GIT_DIR`
|
||||
and `.git` files; a marker walk reimplements a subset of that and
|
||||
gets it subtly wrong.
|
||||
- **`pmacs.process.spawn` / `events_take` / `terminate` / `forget`**
|
||||
is the working model for running an external tool asynchronously;
|
||||
`builtin/runtime/compile.lua` is a full worked example, including
|
||||
spawn-failure handling and exit markers.
|
||||
- **`pmacs.listview.open`** is a real primitive with existing adopters
|
||||
(`*references*`, `*lsp*`), carrying optional `depth`/`id`,
|
||||
primitive-owned collapse, and selection re-seated by id. `COHERENCE.md`
|
||||
P5 says the remaining work there is **adoption, not construction** —
|
||||
a `*git-status*` panel is exactly that.
|
||||
- **Gutter signs exist in both frontends** — the TUI's leading-column
|
||||
glyph (`src/diag.rs`) and the GPU's `GUTTER_SIGN_X` bars
|
||||
(`pmacs-gpu/src/main.rs:420`).
|
||||
- **A tested porcelain-v2 parser exists as a package fixture** (above).
|
||||
Its `status.lua` deliberately separates **pure `parse_*` functions
|
||||
that take a string and return structure** from the spawning around
|
||||
them — which is the shape that makes a parser testable without a
|
||||
repository, and it is already proven by 32 tests.
|
||||
|
||||
And the constraint that shapes the staging:
|
||||
|
||||
- **`DecorationKind` is a CLOSED enum on the wire**
|
||||
(`pmacs-protocol/src/message.rs:1472`): four diagnostic severities,
|
||||
`Selection`, `SearchMatch`, `SearchMatchActive`, `CurrentLine`.
|
||||
**Gutter markers for git hunks therefore require new variants, which
|
||||
is a protocol version bump.** The gutter signs that exist are keyed
|
||||
on `diagnostic_severity_rank` and have no notion of anything else.
|
||||
|
||||
## 3. The staging, and why the line falls where it does
|
||||
|
||||
**Stage 1 (this lane): read-only, panel-based, NO WIRE CHANGE.**
|
||||
|
||||
- `*git-status*` — a `listview` panel over
|
||||
`git status --porcelain=v2 --branch -z` (Q#G-6), rows visiting the
|
||||
file at RET, refreshed by `g` under the completion model in Q#G-1.
|
||||
- `*git-diff*` — the diff for the **file** under point (Q#G-7), in a
|
||||
generated buffer rendered as **plain text** (no `diff` grammar
|
||||
exists). **No hunk model** — hunks are Stage 2's concern.
|
||||
|
||||
**Stage 2 (separate lane): gutter markers.** Needs new
|
||||
`DecorationKind` variants and a `PROTOCOL_VERSION` bump, plus both
|
||||
frontends' gutter renderers learning a second rider family.
|
||||
|
||||
**Stage 3+ (unscheduled): staging, commit, blame.** Staging and commit
|
||||
are where an editor becomes a git *client*; blame is a lower-frequency
|
||||
read. Neither belongs in front of the two above.
|
||||
|
||||
**The line is drawn at the wire on purpose, and it is a scheduling
|
||||
decision as much as a design one.** Parallel lanes are about to start,
|
||||
and `PROTOCOL_VERSION` is a strict serialization point — two lanes
|
||||
bumping it collide, and this session already recorded what that costs
|
||||
(eight broken version assertions on CI from a single bump). Stage 1
|
||||
touching no wire is what lets it run **concurrently** with other work.
|
||||
Stage 2 must be scheduled alone.
|
||||
|
||||
## 4. Coherence impact (§20)
|
||||
|
||||
Required by `CLAUDE.md` for coherence-affecting work, and this is
|
||||
coherence-affecting — it is §15's named gap.
|
||||
|
||||
- **Journey steps touched:** none directly. Git is not currently a
|
||||
journey step; the golden journey runs open → edit → build → test →
|
||||
navigate. This lane does **not** add a step, and I would rather say
|
||||
so than inflate the claim.
|
||||
- **§15 contextual affordances — the direct target.** The audit's git
|
||||
affordance list ("a Git change stage/revert/diff") has *nothing to
|
||||
attach to*. Stage 1 creates the thing to attach to; the affordances
|
||||
themselves follow it, and the menu's context vocabulary
|
||||
(`src/menu.rs:44`) would need a `git` context to host them — **out of
|
||||
scope here**, named so it is not forgotten.
|
||||
- **§14 workbench primitives — adoption, which is the stated need.**
|
||||
`*git-status*` becomes the **fifth** `listview` call site and the
|
||||
first outside the LSP panels, which is the concrete evidence P5 asks
|
||||
for that the primitive generalizes past its first consumer.
|
||||
- **Interaction islands (§6): none added, and this is a real
|
||||
constraint.** The panel gets no hardcoded key interception; it uses
|
||||
`listview`'s existing key handling. §6 records six such shadows and
|
||||
calls them "weak, and growing" — this lane must not make it seven.
|
||||
- **Config registry adoption:** at least one setting
|
||||
(`git.enabled`, Q#G-4), defined through `pmacs.config.define` like
|
||||
`ui.line-wrap` and the zoom settings, not a bare Lua global.
|
||||
- **Background-work attribution (§9): NEGATIVE, and named as such.**
|
||||
Git runs as a spawned process, and spawned processes do **not** appear
|
||||
in `*workers*` — that view is `async.lua`'s job list; processes live
|
||||
under `pmacs.process.list` (Q#G-5). This lane therefore adds a fifth
|
||||
thing running in the background with no single place to see it. The
|
||||
process is labelled honestly, which is better than anonymous, but
|
||||
**a label is not attribution and this document does not pretend
|
||||
otherwise.** Accepted because these are short-lived reads; it would
|
||||
not be acceptable for Stage 3's push/pull.
|
||||
|
||||
## 5. Open questions
|
||||
|
||||
### Q#G-1 — is the status panel a snapshot or a live view?
|
||||
|
||||
A snapshot is a command that opens a panel; a live view refreshes on
|
||||
buffer save, on focus, or on a filesystem watch.
|
||||
|
||||
*My vote: **snapshot, refreshed explicitly***, with `g` re-running
|
||||
inside the panel. Live refresh needs a watch mechanism, an invalidation
|
||||
rule, and a §9 story for the recurring work — all real arcs. A snapshot
|
||||
is honest, useful the first day, and does not pretend to a currency it
|
||||
cannot maintain.
|
||||
|
||||
**But "explicit refresh" does not fit `listview` unmodified, and
|
||||
revision 1 missed that.** `listview.refresh` is synchronous:
|
||||
|
||||
```lua
|
||||
local rows = check_ids(p.on_refresh() or {}) -- listview.lua:402
|
||||
```
|
||||
|
||||
The result is consumed immediately. `pmacs.process.spawn` cannot return
|
||||
rows there — it returns a process id whose output is drained later. So
|
||||
revision 1's "adopt `listview`" would have produced exactly one of the
|
||||
two failures the reviewer named: a reimplemented list, or a `g` that
|
||||
silently does nothing. The primitive's own docs already call a dead `g`
|
||||
out as a defect it must not repeat (`listview.lua:416`).
|
||||
|
||||
**The completion model, specified.** `on_refresh` stays synchronous and
|
||||
honest:
|
||||
|
||||
1. **`on_refresh` returns the CURRENT rows immediately**, with a
|
||||
`refreshing…` marker row appended, and *kicks off* the spawn. `g` is
|
||||
therefore never a no-op — it always re-renders and always shows that
|
||||
work started.
|
||||
2. **On exit, the completion handler re-opens the panel** via
|
||||
`listview.open` with the same `name` — **and re-seats the selection
|
||||
itself.**
|
||||
|
||||
Revision 2 credited that to the primitive and was wrong.
|
||||
`listview.open` **resets collapse** (`p.collapsed = {}`) and
|
||||
**always seats line 1** (`seat_cursor(p, 1)`,
|
||||
`builtin/runtime/listview.lua:337-378`). The `listview.lua:82` note
|
||||
I cited is about **name** disambiguation to `<2>`, not selection.
|
||||
Only `listview.refresh` preserves a selection, and that is the
|
||||
synchronous path this model cannot use.
|
||||
|
||||
So the contract is explicit and owned here: **capture the selected
|
||||
row's git id (its current path) before re-opening, and after
|
||||
re-opening move to the line whose row carries that id**, computed
|
||||
from the handler's own rows array via `pmacs.editor.move_to_line`.
|
||||
If the id is gone from the new status — the commonest case, since a
|
||||
file that stopped being modified drops out — seat line 1 and say
|
||||
nothing; that is the correct answer, not a failure.
|
||||
|
||||
**Collapse state is moot in Stage 1** because the rows are flat: no
|
||||
`depth`, so nothing to collapse. Stage 2 or a sectioned view would
|
||||
have to revisit this, and would then face the same reset.
|
||||
3. **Concurrent refresh is suppressed by a generation counter.** A
|
||||
second `g` while one is in flight bumps the generation; the older
|
||||
completion sees a stale generation and **discards its rows** rather
|
||||
than racing. It does not terminate the first process — reaping is
|
||||
`pmacs.process.forget`'s job and killing git mid-read buys nothing.
|
||||
4. **Failure is a row, not a silence.** Non-zero exit or spawn failure
|
||||
renders a row carrying the exit code and the first stderr line, plus
|
||||
a status message. §1.2's silence asymmetry.
|
||||
5. **Panel lifetime.** If the panel's buffer is gone when the process
|
||||
exits, the handler drops the result. `compile.lua:252` already
|
||||
handles the buffer-killed case for its own slot; the same shape.
|
||||
|
||||
**The alternative — extending `listview` with an async contract — is
|
||||
the more correct long-term answer** and is deliberately not taken here:
|
||||
it changes a primitive with four existing adopters, and doing that from
|
||||
inside its fifth adopter's lane is how a primitive acquires a consumer's
|
||||
idiosyncrasies. **If review prefers it, it belongs in its own lane
|
||||
before this one.**
|
||||
|
||||
### Q#G-0 — what is the relationship to `pmacs-magit`? **(new in rev 2)**
|
||||
|
||||
The reviewer's framing of the choice is right: adopt, replace, or
|
||||
declare it out-of-product precedent. Doing none of those and quietly
|
||||
writing a second parser is the option that must not happen.
|
||||
|
||||
*My vote: **port its pure `parse_*` functions and its test corpus into
|
||||
the bundled runtime; leave the fixture itself untouched.***
|
||||
|
||||
- **The record TOKENIZER is deliberately rewritten, not ported.** The
|
||||
fixture parses **newline-delimited** v2; Stage 1 reads **`-z`**, and
|
||||
those are different grammars — under `-z` a record's fields are
|
||||
NUL-terminated and a rename carries its two paths as separate fields
|
||||
rather than tab-joined. Saying "port the parser" would have been
|
||||
wrong; what ports is the **separation** (pure `parse_*` functions
|
||||
over a string, testable with no repository) and the **case coverage**
|
||||
its 32 tests encode. The tokenizer underneath is new, and its
|
||||
correctness rests on this lane's own corpus.
|
||||
- **Port, not import.** The fixture's purpose is to prove the *package
|
||||
system* can host this. If bundled code became its dependency, it
|
||||
would stop demonstrating an independent package and `m8_6` would test
|
||||
less than it claims.
|
||||
- **The duplication is therefore deliberate**, and it is the one place
|
||||
this framing accepts two copies of a rule after a session spent
|
||||
removing them. The justification is that they answer different
|
||||
questions — one is product behaviour, one is package-system
|
||||
capability — and coupling them weakens the second. **If review
|
||||
prefers the coupling, that is a defensible call and I will take it**;
|
||||
what I will not do is leave the duplication unstated.
|
||||
- **It also settles Q#G-2's format**: the existing, tested parser is
|
||||
**porcelain v2**, so Stage 1 is v2. Revision 1 said v1 for no reason
|
||||
beyond familiarity.
|
||||
|
||||
### Q#G-2 — `git` the binary, or a library?
|
||||
|
||||
*My vote: **the binary**, via `pmacs.process.spawn`. `compile.lua` is
|
||||
the worked precedent, the daemon already spawns external tools, and a
|
||||
git library is a dependency with a much larger surface than "run one
|
||||
command and parse porcelain". `--porcelain=v2` is explicitly a stable
|
||||
machine format; that is what it is for.
|
||||
|
||||
**Named risk:** no `git` on `PATH`. §1.2's *silence asymmetry* says the
|
||||
failure must be **surfaced with guidance**, not swallowed — the same
|
||||
lesson #204 landed for a missing language server.
|
||||
|
||||
### Q#G-6 — the status data contract **(new in rev 2)**
|
||||
|
||||
Revision 1 said "`--porcelain=v1`" and proposed "a path with a space"
|
||||
as the parsing witness. **Both were inadequate.** Porcelain without
|
||||
`-z` emits paths in git's **C quoting** for anything non-ASCII or
|
||||
containing special characters, and rename/copy records carry *two*
|
||||
paths whose separation is positional. A single space-in-path fixture
|
||||
proves none of that.
|
||||
|
||||
*My vote: the exact invocation*
|
||||
|
||||
```
|
||||
git --no-optional-locks -C <dir> status --porcelain=v2 --branch -z
|
||||
```
|
||||
|
||||
**`--no-optional-locks` is part of the contract, not a nicety.**
|
||||
`git status` is **not strictly read-only**: it may refresh and write
|
||||
the index, and git's own documentation recommends this flag for
|
||||
background scripts precisely so a background reader does not contend
|
||||
for `index.lock` with the user's real git commands
|
||||
(<https://git-scm.com/docs/git-status>). This lane runs status
|
||||
*asynchronously, from an editor, while the user may be running git in a
|
||||
terminal* — the exact scenario the flag exists for. Revision 2 called
|
||||
the lane "read-only" and that was wrong about the mechanism.
|
||||
|
||||
It is **witnessed structurally** — the assembled argv is asserted to
|
||||
carry the flag — because observing a lock that was *not* taken is not
|
||||
something a test can do directly. Verified accepted by the git in use
|
||||
here.
|
||||
|
||||
The rest: `--porcelain=v2 --branch -z`, also verified accepted. NUL delimiting removes C quoting from
|
||||
the problem **entirely** rather than obliging a hand-written unquoter,
|
||||
and it makes the two-path rename record unambiguous: the paths are
|
||||
separate NUL-terminated fields rather than tab-joined inside one.
|
||||
|
||||
The rename/copy identity rule to pin: a `2` record carries the current
|
||||
path **and** its origin, and the panel must show which file it is now
|
||||
while remembering where it came from — a row whose id is the current
|
||||
path, since that is what RET visits.
|
||||
|
||||
**Witness corpus, not one case:** modified, added, deleted, untracked,
|
||||
**renamed (both paths)**, **copied**, a path with a space, a path with
|
||||
a newline, and a non-UTF-8 path. The last two are exactly what `-z`
|
||||
buys and what a quoted parser gets wrong.
|
||||
|
||||
### Q#G-7 — the diff gesture **(new in rev 2)**
|
||||
|
||||
Revision 1 wrote "the diff for the file or hunk under point" while also
|
||||
committing RET to visiting the file. **RET cannot do both, there is no
|
||||
second binding proposed, and no hunk model exists anywhere in the
|
||||
tree.**
|
||||
|
||||
*My vote:*
|
||||
|
||||
- **RET visits the file** — unchanged, and the behaviour a list of
|
||||
files should have.
|
||||
- **A named command, `git.diff-file`, bound to `d` inside the panel.**
|
||||
|
||||
**`d` is not on `listview`'s key surface**, and revision 2 said it
|
||||
was. The bound set is exactly `RET SPC n <down> p <up> TAB g q`
|
||||
(`builtin/runtime/listview.lua:266-279`), bound buffer-locally inside
|
||||
the primitive, which is the only place the panel's buffer handle is
|
||||
known. **Looking the buffer up by name from outside is unsafe** —
|
||||
`listview` deliberately disambiguates a collision to `<2>`, so the
|
||||
name a consumer passed is not necessarily the buffer it got.
|
||||
|
||||
*My vote: **a `keys` table on the open spec***, e.g.
|
||||
`keys = { d = "git.diff-file" }`, bound through the same
|
||||
`bind_local_keymap` that already binds the fixed set. It is additive,
|
||||
general to any adopter, keeps binding where the buffer is known, and
|
||||
adds **no** interception — the §6 constraint holds.
|
||||
|
||||
**The registration lifecycle, which revision 3 omitted and which
|
||||
would have broken the refresh path it depends on.** `Keymap::bind`
|
||||
**refuses duplicates** — `KeymapError::DuplicateBinding`, *"Refuse
|
||||
rather than silently overwrite"* (`src/keymap_tree.rs:75`) — and the
|
||||
completion model calls `listview.open` again on **every** refresh. A
|
||||
naive `keys` implementation therefore errors on the second open, so
|
||||
**every successful refresh would have failed while re-binding `d`.**
|
||||
|
||||
The contract:
|
||||
|
||||
1. **Keys are installed once, when the panel's buffer is created**,
|
||||
and stored on the panel.
|
||||
2. **A later `open` for a live panel does not re-bind.** It
|
||||
**compares** the supplied `keys` against the stored table and
|
||||
**errors on divergence** rather than ignoring it. Silently keeping
|
||||
the old binding would give the consumer a key that does something
|
||||
other than what it just asked for — a dead or lying key, which is
|
||||
the defect `listview` already condemns for `g`.
|
||||
3. **Collisions are rejected at install time**, against both the
|
||||
fixed set (`RET SPC n <down> p <up> TAB g q`) and any
|
||||
**prefix conflict** — `Keymap` has a separate error for turning a
|
||||
leaf into a submap, and a `keys` table must not be able to reach
|
||||
it.
|
||||
|
||||
(The alternative, idempotent re-registration, is tolerable but
|
||||
strictly weaker: it makes a consumer that changes its keys mid-session
|
||||
silently wrong instead of loudly wrong.)
|
||||
|
||||
**This IS a `listview` modification, and revision 2's "no listview
|
||||
modification" was false.** I distinguish it from the async-contract
|
||||
change I deferred: that one alters *when* an existing callback's
|
||||
result is consumed for four existing adopters; this adds an optional
|
||||
field that changes nothing for a spec that omits it. **If review
|
||||
judges any primitive change out of an adopter's lane, the alternative
|
||||
is `listview.open` returning the panel buffer** so the consumer binds
|
||||
its own key — smaller still, but it pushes binding to every adopter.
|
||||
- **No hunk model in Stage 1.** Hunks are precisely what gutter markers
|
||||
need, and that is Stage 2's protocol work. Introducing a half hunk
|
||||
model here to serve one gesture would prejudge Stage 2's design from
|
||||
the wrong side.
|
||||
|
||||
**And what `d` actually SHOWS, which revision 2 left unstated.** "File,
|
||||
not hunk" is a scope, not a contract. A porcelain-v2 row carries an
|
||||
**XY** pair — X staged (index vs HEAD), Y unstaged (worktree vs index)
|
||||
— and the three plausible diffs answer three different questions:
|
||||
`git diff` shows only Y, `--cached` only X, and neither shows an
|
||||
untracked file at all.
|
||||
|
||||
*My vote: **`d` answers the lane's own question — "what have I
|
||||
changed?" — against `HEAD`:***
|
||||
|
||||
| row | `d` runs | why |
|
||||
|---|---|---|
|
||||
| staged, unstaged, or both | `git diff HEAD -- <path>` | one view of the total change; splitting X from Y is a staging UI, which is Stage 3 |
|
||||
| deleted | `git diff HEAD -- <path>` | shows the deletion; no special case needed |
|
||||
| renamed / copied | `git diff HEAD -- <orig> <current>` | v2 gives both paths; passing both is what lets rename detection render it as a rename rather than an unrelated add+delete |
|
||||
| **untracked** | `git diff --no-index -- /dev/null <path>` | **a normal diff shows nothing at all** for an untracked file. Without this case `d` is silently dead on the rows a user is most likely to press it on |
|
||||
| non-UTF-8 path | *refuses, with a message* | see Q#G-8 |
|
||||
|
||||
The `HEAD` choice is deliberate and is the one thing here I would most
|
||||
expect review to push back on: it is the right default for *reading*
|
||||
what changed, and the wrong one for *staging*, which is why it is
|
||||
correct for Stage 1 and will need revisiting when Stage 3 arrives.
|
||||
|
||||
**The exit-state contract, which revision 3 got wrong in two ways.**
|
||||
"Non-zero exit renders a failure row" is not correct for `git diff`.
|
||||
Both cases below were measured in a scratch repository, not inferred:
|
||||
|
||||
**(a) `--no-index` implies `--exit-code`.** It exits **1 when it
|
||||
successfully finds differences** — measured: `exit=1` for an untracked
|
||||
file against `/dev/null`. Under revision 3's predicate, *every*
|
||||
untracked diff — the case `--no-index` exists to serve — would have
|
||||
rendered a failure row instead of the diff it just produced.
|
||||
|
||||
So for the untracked path the success predicate is **exit ∈ {0, 1}**,
|
||||
rendering whatever came out; **exit ≥ 2 is a real failure**. That
|
||||
asymmetry is confined to the `--no-index` invocation and does not leak
|
||||
to the others, where non-zero still means failure.
|
||||
|
||||
**(b) An unborn repository has no `HEAD`.** Measured:
|
||||
`git diff HEAD -- <path>` exits **128** with `fatal: bad revision
|
||||
'HEAD'`. This is not an edge case — it is a freshly `git init`-ed
|
||||
repository with the first files staged, which is exactly when someone
|
||||
opens a status panel to see what they are about to commit.
|
||||
|
||||
*Policy: **detect once, then split**.*
|
||||
|
||||
**Detection needs no extra subprocess.** `--branch` already reports
|
||||
`# branch.oid (initial)` when `HEAD` is unborn — observed in the
|
||||
output this lane already parses. Revision 4 proposed a separate
|
||||
`git rev-parse --verify --quiet HEAD`; that is a second process for a
|
||||
fact the first one hands over.
|
||||
|
||||
**The reachable states, enumerated from a real unborn repository** —
|
||||
`git init`, stage three files, then edit one, delete one, and `git mv`
|
||||
one:
|
||||
|
||||
```
|
||||
# branch.oid (initial)
|
||||
1 AD ... ad.txt
|
||||
1 AM ... am.txt
|
||||
1 A. ... r_new.txt <- the `git mv`
|
||||
? untracked.txt
|
||||
```
|
||||
|
||||
Two findings fall straight out:
|
||||
|
||||
- **`AM` and `AD` are ordinary and carry BOTH states**, which is
|
||||
exactly the gap: `--cached` alone loses the worktree delta, plain
|
||||
`git diff` alone loses the staged base.
|
||||
- **Rename and copy CANNOT occur under an unborn `HEAD`.** The
|
||||
`git mv` produced `1 A. … r_new.txt` — an ordinary add of the new
|
||||
path, **not** a `2` record. With no `HEAD` there is nothing to
|
||||
rename *from*, so the rename/copy row class is unreachable here and
|
||||
needs no unborn policy. That is a case closed by evidence rather than
|
||||
handled speculatively.
|
||||
|
||||
| unborn row | `d` renders |
|
||||
|---|---|
|
||||
| `A.` staged only | one patch: `git diff --cached -- <path>` |
|
||||
| **`AM` staged + edited** | **two labelled patches** — *staged* `git diff --cached -- <path>`, then *unstaged* `git diff -- <path>` |
|
||||
| **`AD` staged + deleted** | **two labelled patches**, same pair; the second renders the deletion |
|
||||
| `.M` / `.D` unstaged only | one patch: `git diff -- <path>` |
|
||||
| `?` untracked | `git diff --no-index -- /dev/null <path>` (exit ∈ {0,1}) |
|
||||
| rename / copy | **unreachable** — see above |
|
||||
|
||||
All four `--cached` / plain invocations above were run against that
|
||||
repository and render the expected patches.
|
||||
|
||||
**The split is unborn-only, and that asymmetry is deliberate.** Once
|
||||
`HEAD` exists, `git diff HEAD` gives one total — which is the lane's
|
||||
question — and splitting it would be a staging UI (Stage 3). The split
|
||||
appears here only because there is no `HEAD` to total *against*.
|
||||
|
||||
The generated buffer carries a **header naming what it is showing**:
|
||||
*"no commits yet — split view: staged (index) above, unstaged
|
||||
(worktree) below"*. Revision 4's wording ("showing staged changes")
|
||||
would have described a single total-against-`HEAD` diff, which is
|
||||
precisely what this is not. A diff that silently answers a different
|
||||
question than the one asked is worse than one that says so — and a
|
||||
header that misdescribes a split view is the same failure in smaller
|
||||
type.
|
||||
|
||||
### Q#G-8 — non-UTF-8 paths: an honest boundary **(new in rev 3)**
|
||||
|
||||
Revision 2 listed a non-UTF-8 path in the witness corpus as though it
|
||||
were an end-to-end case. **It cannot be**, and the boundary is in the
|
||||
bindings: `pmacs.process.spawn` takes `args: Vec<String>`
|
||||
(`src/lua_bindings/mod.rs:8683`) and `pmacs.buffer.find_or_open` takes
|
||||
`path: String` (`:3564`). Both are Rust `String`, i.e. UTF-8 by
|
||||
construction. A path that is valid bytes but not valid UTF-8 can be
|
||||
*read* from git's `-z` output and *displayed*, but it cannot be passed
|
||||
back to `spawn` for a diff, nor opened.
|
||||
|
||||
*My vote: **parse it, show it, and refuse the gesture with a
|
||||
message***:
|
||||
|
||||
- the row **appears** in the panel, so the user is not lied to about
|
||||
what is modified;
|
||||
- **RET and `d` on that row report** that the path is not representable
|
||||
and do nothing else — a witnessed refusal, not a stack trace or a
|
||||
silent no-op;
|
||||
- **it is removed from the end-to-end promise.** The witness is
|
||||
parser-and-display **plus the refusal**, and the framing does not
|
||||
claim visiting works.
|
||||
|
||||
Making it work end-to-end means `OsString`/bytes through two binding
|
||||
boundaries — a real change to the Lua API surface, and not this lane's.
|
||||
|
||||
### Q#G-3 — what does the diff view render into?
|
||||
|
||||
*My vote: **a generated buffer**, reusing the generated-buffer
|
||||
immutability work (Stage 1 merged; that lane's Stage 2 is queued).
|
||||
Diff output is read-only text and that machinery exists.
|
||||
|
||||
**RESOLVED in rev 2 — there is no bundled `diff` grammar.**
|
||||
`BUILTIN_LANGUAGES` (`src/syntax.rs`) has no `diff` entry; checked, not
|
||||
assumed. **Stage 1 renders plain generated text**, and diff
|
||||
highlighting is later work needing a grammar first.
|
||||
|
||||
### Q#G-4 — what is configurable?
|
||||
|
||||
*My vote: **one setting to start** — `git.enabled` (boolean, default
|
||||
`true`), through the config registry. Resist more until there is use
|
||||
evidence; §11's grade is "partial (foundation only)" and adding five
|
||||
speculative settings is how a registry becomes noise.
|
||||
|
||||
### Q#G-5 — §9 attribution — **RESOLVED, and the answer is negative**
|
||||
|
||||
Revision 1 deferred this to implementation. That was wrong: it is
|
||||
answerable by reading, and deferring it would have meant discovering a
|
||||
known coherence cost *after* committing to the design.
|
||||
|
||||
**A spawned git process does not appear in `*workers*` at all.** That
|
||||
buffer is `builtin/runtime/async.lua`'s (`:490`) and lists **async
|
||||
jobs**; spawned processes live separately under `pmacs.process.list`.
|
||||
They are two of the four disjoint activity views §9 grades as
|
||||
"mechanism without identity".
|
||||
|
||||
So, stated plainly rather than dressed up:
|
||||
|
||||
- **This lane adds a fifth thing that runs in the background and is not
|
||||
attributable from one place.** That is a **negative** coherence impact
|
||||
against §9, and it is the honest cost of shipping git status before
|
||||
worker identity exists.
|
||||
- **Labelling the process is still required** — a clear label under
|
||||
`pmacs.process.list` is strictly better than an anonymous `git`. But
|
||||
**a label does not solve attribution**, and this document does not
|
||||
claim it does. The claim is only: do not make it worse than it has to
|
||||
be.
|
||||
- **The mitigation is bounded in time, not in kind.** These are
|
||||
short-lived reads, not long-running jobs; a `git status` that has not
|
||||
finished is a bug, not a background task a user needs to supervise.
|
||||
That is why the cost is acceptable *now* and would not be for
|
||||
Stage 3's push/pull.
|
||||
|
||||
## 6. Verification
|
||||
|
||||
- **Parsing, against a corpus rather than a case (Q#G-6):** modified,
|
||||
added, deleted, untracked, **renamed with both paths**, **copied**, a
|
||||
path with a space, and **a path with a newline** — the last is what
|
||||
`-z` buys, and a parser that passes only the space case is the one
|
||||
that ships broken.
|
||||
- **A non-UTF-8 path is parsed and displayed, and its gestures refuse
|
||||
with a message** (Q#G-8) — a witnessed refusal at the binding
|
||||
boundary, **not** an end-to-end visit.
|
||||
- **The argv carries `--no-optional-locks`** (Q#G-6), asserted
|
||||
structurally. A lock not taken cannot be observed directly, so the
|
||||
invocation is what gets pinned.
|
||||
- **`d` is witnessed on every row class** (Q#G-7): staged, unstaged,
|
||||
both, deleted, renamed, and **untracked** — the last because a normal
|
||||
`git diff` shows nothing there, so a missing `--no-index` case makes
|
||||
`d` silently dead exactly where it is most used.
|
||||
- **A copy is reported as a COPY, not a rename** (Q#G-7). Porcelain v2
|
||||
folds both into the one `2` record, so `kind` stays `"rename"` for
|
||||
both — every *behaviour* keyed on it is the same — and the
|
||||
distinction is made where it is a distinction: the diff header reads
|
||||
the `<Xscore>` field's leading `R`/`C` and says which one happened.
|
||||
The status row is left alone, because its `XY` prefix already reads
|
||||
`R.` against `C.`. Both classes are asserted, and so is the **argv**:
|
||||
the two-path `git diff HEAD -- <orig> <current>` is right for a copy
|
||||
and a rename alike, so a fix to what the user is *told* must not
|
||||
reach what runs. **Parser-level, deliberately** — the copy ROW is
|
||||
supplied through `_deliver_status` while the repository, the panel,
|
||||
the `d` dispatch and the spawned diff around it are real.
|
||||
|
||||
**The reason, narrowed after review.** This bullet used to say real
|
||||
`git` emits no `2 C` record "even under `status.renames=copies`".
|
||||
**That is too strong, and git's own documentation contradicts it** —
|
||||
`git-status(1)` lists `C` as *"copied (if config option
|
||||
status.renames is set to `copies`)"*. What the test measures is
|
||||
narrower: **for its fixture, whose copy source is left unchanged**,
|
||||
git reports `1 A.`. That is a fact about the fixture, and it is
|
||||
sufficient reason to craft the row — a weaker and true justification
|
||||
in place of a stronger false one. No mechanism is claimed for why an
|
||||
unchanged source is not offered as a candidate; that was never
|
||||
established.
|
||||
- **The untracked diff renders on exit 1**, not a failure row (Q#G-7a)
|
||||
— the case `--exit-code` semantics would otherwise break, and the
|
||||
one most likely to be "fixed" later by someone who reads exit 1 as an
|
||||
error.
|
||||
- **An unborn repository is witnessed end to end**, and the fixture is
|
||||
**`AM`** specifically — staged then edited again, the shape a first
|
||||
commit actually has partway through. `git init`, stage, edit, open
|
||||
the panel, press `d`, and get **two labelled patches** with the
|
||||
split-view header — not `fatal: bad revision 'HEAD'`, and not a
|
||||
single `--cached` patch that silently drops the worktree edit.
|
||||
**`AD` rides the same fixture**, since one repository can hold both.
|
||||
- **Unborn detection reads `# branch.oid (initial)`** from the status
|
||||
output already being parsed — asserted, so nobody later reintroduces
|
||||
a second `rev-parse` process for a fact already in hand.
|
||||
- **Rename/copy under an unborn `HEAD` is asserted UNREACHABLE**: the
|
||||
fixture `git mv`s a staged-but-uncommitted file and the parser sees a
|
||||
`1 A.` record, never a `2`. Pinned so a future reader does not
|
||||
"fix" the missing unborn rename policy by inventing one.
|
||||
- **Re-binding across a refresh does not error** (Q#G-7): two
|
||||
successive refreshes on a live panel, asserting `d` still works and
|
||||
no `DuplicateBinding` surfaced. This is the one that would have
|
||||
broken on every refresh.
|
||||
- **A `keys` table colliding with the fixed set is rejected at install
|
||||
time**, as is a prefix conflict.
|
||||
- **Selection is re-seated by the completion handler** (Q#G-1), across
|
||||
a refresh that reorders rows, and **falls back to line 1 without
|
||||
complaint when the selected path drops out of status** — the common
|
||||
case, not an error.
|
||||
- **The pure `parse_*` functions are tested without a repository**,
|
||||
which is the shape `pmacs-magit/status.lua` already proves works and
|
||||
the reason to port that separation rather than invent one.
|
||||
- **A repository fixture built with real `git`**, in a tempdir, and
|
||||
**bounded with `set_search_boundary`** — R8 was retired two commits
|
||||
ago and is precisely what happens when a fixture lets project
|
||||
detection escape into the developer's environment.
|
||||
- **The root rule is witnessed on a repository whose `ProjectKind` is
|
||||
NOT `Git`** — i.e. an ordinary language project with a `.git` beside
|
||||
its manifest. That is the case revision 1's `kind == "git"` gate
|
||||
would have failed, and this repository is one.
|
||||
- **Missing `git` on `PATH` is witnessed**, not assumed (Q#G-2), and
|
||||
surfaces guidance rather than silence.
|
||||
- **`g` is never a no-op** (Q#G-1): it re-renders and marks that work
|
||||
started, even mid-flight. A dead `g` is a defect `listview` already
|
||||
names.
|
||||
- **Concurrent refresh discards the stale generation** rather than
|
||||
racing — asserted by driving two refreshes and completing them out of
|
||||
order.
|
||||
- **Failure renders a row**, carrying exit code and stderr.
|
||||
- **The panel is a `listview` adopter**, asserted structurally, so a
|
||||
future re-implementation of list behaviour inside git code fails the
|
||||
test rather than passing review.
|
||||
- **No new interaction island** — `d` is bound buffer-locally through
|
||||
`listview`'s own binding path (Q#G-7), not a hardcoded interception.
|
||||
§6 stays at six shadows.
|
||||
|
||||
Gates via `scripts/gate --acceptance <the new suite>`.
|
||||
|
||||
**What this will NOT prove:** that background git work is attributable
|
||||
(Q#G-5 — it is not, by construction), or that the parser handles
|
||||
porcelain versions other than v2.
|
||||
|
||||
## 7. Not in scope
|
||||
|
||||
Gutter markers and any `DecorationKind`/`PROTOCOL_VERSION` change
|
||||
(Stage 2 — must be scheduled alone). Staging, commit, push, pull,
|
||||
branch operations, merge-conflict resolution. Blame. A `git` context in
|
||||
the menu vocabulary. Any git *library* dependency. Live refresh
|
||||
(Q#G-1). Fixing §9's worker identity — this lane makes it marginally
|
||||
worse and says so (Q#G-5). Any hunk model (Q#G-7). Modifying the
|
||||
`listview` primitive to carry an async contract — the better long-term
|
||||
answer, but it belongs in its own lane before this one, not inside its
|
||||
fifth adopter (Q#G-1). Changing `tests/fixtures/pmacs-magit/` or
|
||||
`tests/m8_6_acceptance.rs` (Q#G-0).
|
||||
|
||||
**A `listview` change IS in scope after all** (Q#G-7): an optional
|
||||
`keys` table on the open spec. Revision 2 said no primitive
|
||||
modification; that was false, because `d` cannot be bound from outside
|
||||
the primitive safely. The async-contract change stays out.
|
||||
|
||||
**One correction this lane should carry when it lands:** `COHERENCE.md`
|
||||
§15's "no Git integration anywhere in the tree" is literally false —
|
||||
`tests/fixtures/pmacs-magit/` exists. The *product* gap it describes is
|
||||
real; the sentence needs narrowing to say so.
|
||||
|
|
@ -231,12 +231,33 @@ all share one keymap:
|
|||
| `RET` / `SPC` | `listview.visit` — act on the item under the cursor |
|
||||
| `n` / `<down>` | `cursor.down` |
|
||||
| `p` / `<up>` | `cursor.up` |
|
||||
| `TAB` | `listview.toggle` — collapse/expand the tree node under the cursor; a panel with no tree rows delegates to `buffer.tab` |
|
||||
| `g` | `listview.refresh` — re-run the data source and re-render |
|
||||
| `q` | `listview.quit` — restore the buffer that was active before the panel opened |
|
||||
|
||||
(`TAB` arrived with the tree primitive and this table had not recorded
|
||||
it. Noted rather than quietly added: the omission predates the git lane
|
||||
that found it.)
|
||||
|
||||
Panels currently built on this: `*references*`, `*outline*`,
|
||||
`*lsp-help*` (hover docs). Header text always spells out the same
|
||||
`RET`/`n`/`p`/`g`/`q` legend inline.
|
||||
`*lsp-help*` (hover docs), `*lsp*` (`lsp.status`), and `*git-status*`
|
||||
(`git.status`). Header text always spells out the panel's own legend
|
||||
inline.
|
||||
|
||||
A panel may add keys of its own through an optional `keys` table on the
|
||||
open spec, bound through the same buffer-local path — so they are
|
||||
inspectable by `describe-key` and rebindable from `init.lua`, exactly
|
||||
like the fixed set. They are installed once with the panel's buffer and
|
||||
may not collide with the fixed set, nor prefix it. One panel uses this
|
||||
today:
|
||||
|
||||
| Buffer | Key | Command |
|
||||
|---|---|---|
|
||||
| `*git-status*` (`git.status`) | `d` | `git.diff-file` — the diff for the file under the cursor, into `*git-diff*` |
|
||||
|
||||
`git.status` gets **no global chord**: an opening key is a
|
||||
command-surface decision the Stage 1 framing did not make, so the entry
|
||||
point is `M-x git.status`.
|
||||
|
||||
`*buffer-list*` (`editor.list-buffers`, `C-x C-b`) uses its own
|
||||
keymap, layered on the same idiom, in `builtin/commands/default.lua`:
|
||||
|
|
|
|||
|
|
@ -797,6 +797,20 @@ impl EditorState {
|
|||
include_str!("../builtin/runtime/linewrap.lua"),
|
||||
)
|
||||
.expect("load linewrap builtin chunk");
|
||||
// Git integration Stage 1 (docs/git-integration-framing.md):
|
||||
// `*git-status*` and `*git-diff*`. Loaded after `listview.lua`,
|
||||
// whose `open` (and whose new optional `keys` table) it drives,
|
||||
// and after `window.lua`, which owns `window.panel-height` — the
|
||||
// setting a `display = "panel"` listview resolves. It binds no
|
||||
// global key: an opening chord is a command-surface decision and
|
||||
// the framing did not make one, so the entry point is
|
||||
// `M-x git.status`.
|
||||
lua_host
|
||||
.eval(
|
||||
Some("@pmacs/builtin/runtime/git.lua"),
|
||||
include_str!("../builtin/runtime/git.lua"),
|
||||
)
|
||||
.expect("load git builtin chunk");
|
||||
// T M7.11 bundled-package bootstrap. Through M7.10 the REPL
|
||||
// was loaded directly via `eval(include_str!(...))`; the
|
||||
// M7.11 deliverable migrates it to the package system so it
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
Loading…
Reference in New Issue