diff --git a/COHERENCE.md b/COHERENCE.md index c12acfc..fc27ce9 100644 --- a/COHERENCE.md +++ b/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 diff --git a/builtin/runtime/git.lua b/builtin/runtime/git.lua new file mode 100644 index 0000000..fcefabd --- /dev/null +++ b/builtin/runtime/git.lua @@ -0,0 +1,1234 @@ +-- git.lua --- Git integration Stage 1: read-only status and diff. +-- Framing: docs/git-integration-framing.md (revision 5). +-- +-- Two surfaces and nothing else: +-- +-- *git-status* a `pmacs.listview` panel over +-- `git --no-optional-locks -C status +-- --porcelain=v2 --branch -z`. RET visits the file, +-- `d` shows its diff, `g` refreshes. +-- *git-diff* the diff for the FILE under point, in a generated +-- buffer rendered as plain text. There is no bundled +-- `diff` grammar (checked: `BUILTIN_LANGUAGES` in +-- src/syntax.rs has no entry), and there is no hunk +-- model anywhere in the tree --- hunks are what gutter +-- markers need, and that is Stage 2's protocol work. +-- +-- NO WIRE CHANGE. Stage 2 (gutter markers) needs new `DecorationKind` +-- variants and a `PROTOCOL_VERSION` bump, and must be scheduled alone. +-- +-- Three facts this module is built on, each measured rather than +-- reasoned about: +-- +-- * `ProjectKind::Git` means a BARE repository ("no language marker +-- found inside", src/project.rs), and a language marker beside +-- `.git` WINS --- so pmacs reports `kind = "rust"` for its own +-- repository. This module therefore never asks pmacs whether +-- something is a git repo: it runs `git -C rev-parse +-- --show-toplevel` and lets a non-zero exit be the answer. Git's own +-- resolution handles submodules, worktrees, `GIT_DIR` and `.git` +-- files; a marker walk reimplements a subset and gets it wrong. +-- * `git diff --no-index` implies `--exit-code`: it exits 1 when it +-- SUCCESSFULLY finds differences. For the untracked path the success +-- predicate is therefore exit in {0, 1}; only >= 2 is a failure. +-- That asymmetry is confined to `--no-index`. +-- * An unborn `HEAD` makes `git diff HEAD` exit 128 with +-- `fatal: bad revision 'HEAD'`. It is detected from +-- `# branch.oid (initial)` in the `--branch` output this module +-- already parses --- NOT from a second `rev-parse` process for a +-- fact the first one hands over. +-- +-- Background-work attribution is a NEGATIVE (COHERENCE.md §9): a +-- spawned process does not appear in `*workers*` at all --- that buffer +-- is `async.lua`'s job list, while processes live under +-- `pmacs.process.list`. Every spawn here is labelled, which is strictly +-- better than an anonymous `git`, but a label is not attribution and +-- this module does not pretend otherwise. Accepted because these are +-- short-lived reads. + +pmacs.git = pmacs.git or {} + +local STATUS_PANEL = "*git-status*" +local DIFF_BUFFER = "*git-diff*" + +--- The git program name. +--- +--- A module-local rather than a setting: Q#G-4 defines exactly one +--- setting and says to resist more until there is use evidence. It is +--- reachable from Lua because the missing-binary path (Q#G-2's named +--- risk) has to be WITNESSED, and there is no other way to reach it in +--- process: 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, which this project +--- forbids. Pointing this at a name that is not on `PATH` produces +--- exactly the ENOENT a missing git produces. +pmacs.git._program = "git" + +--- The most recent spawn's `{ command, args, cwd, label }`. +--- +--- Public because the `--no-optional-locks` contract is witnessed +--- STRUCTURALLY: a lock that was not taken cannot be observed directly, +--- so the assembled invocation is what gets pinned (Q#G-6). +pmacs.git._last_spawn = nil + +--- The last few spawns' argv, oldest first. +--- +--- A bounded diagnostic ring rather than an unbounded log. It exists +--- because one of this module's contracts is about a process that must +--- NOT be spawned: unborn `HEAD` is detected from `# branch.oid +--- (initial)` in output already in hand, and nobody should later +--- reintroduce a `rev-parse --verify HEAD` for it. "Which processes did +--- that open run?" is not answerable from `_last_spawn` alone. +pmacs.git._spawn_log = {} + +local SPAWN_LOG_LIMIT = 16 + +-- --------------------------------------------------------------------- +-- Configuration (Q#G-4) +-- --------------------------------------------------------------------- + +pmacs.config.define { + name = "git.enabled", + description = "Whether the Git commands (*git-status*, *git-diff*) run git at all. Turning this off makes them report that they are disabled rather than spawning anything.", + type = "boolean", + default = true, + mutability = "live", +} + +local function git_enabled() + local ok, value = pcall(pmacs.config.get, "git.enabled") + if not ok then return true end + return value ~= false +end + +-- --------------------------------------------------------------------- +-- Text safety at the binding boundary (Q#G-8) +-- --------------------------------------------------------------------- +-- +-- Git hands back PATH BYTES. `pmacs.process.spawn` takes +-- `args: Vec` and `pmacs.buffer.find_or_open` takes +-- `path: String` --- both Rust `String`, i.e. UTF-8 by construction --- +-- and the rope is UTF-8 by project invariant, so a path that is valid +-- bytes but not valid UTF-8 can be READ and DISPLAYED and cannot be +-- passed back for a diff, nor opened. The honest boundary is: parse it, +-- show it (escaped, since the raw bytes cannot enter a rope), and +-- REFUSE the gesture with a message. + +-- Length of the valid UTF-8 sequence starting at byte `i`, or nil. +-- Rejects overlongs, surrogates and anything past U+10FFFF, so +-- "displayable" means the same thing here as it does to Rust. +local function utf8_seq_len(s, i) + local b1 = s:byte(i) + if not b1 then return nil end + if b1 < 0x80 then return 1 end + local n, cp + if b1 >= 0xC2 and b1 <= 0xDF then + n, cp = 2, b1 - 0xC0 + elseif b1 >= 0xE0 and b1 <= 0xEF then + n, cp = 3, b1 - 0xE0 + elseif b1 >= 0xF0 and b1 <= 0xF4 then + n, cp = 4, b1 - 0xF0 + else + return nil + end + if i + n - 1 > #s then return nil end + for k = 1, n - 1 do + local b = s:byte(i + k) + if b < 0x80 or b > 0xBF then return nil end + cp = cp * 64 + (b - 0x80) + end + if n == 3 and cp < 0x800 then return nil end + if n == 4 and cp < 0x10000 then return nil end + if cp >= 0xD800 and cp <= 0xDFFF then return nil end + if cp > 0x10FFFF then return nil end + return n +end + +--- True when `s` is valid UTF-8, i.e. when it can cross the binding +--- boundary at all. +function pmacs.git.is_text(s) + if type(s) ~= "string" then return false end + local i, n = 1, #s + while i <= n do + local len = utf8_seq_len(s, i) + if not len then return false end + i = i + len + end + return true +end + +--- `s` rendered for a ONE-LINE panel row: invalid UTF-8 bytes and every +--- control byte become `\xNN`. +--- +--- Escaping controls is not cosmetic. A path may contain a newline --- +--- that is exactly what `-z` buys and what a quoted parser gets wrong +--- --- and a raw newline in a row would split it across two lines and +--- desynchronize every line-to-row mapping in the panel. +function pmacs.git.display_path(s) + if type(s) ~= "string" then return "" end + local out = {} + local i, n = 1, #s + while i <= n do + local len = utf8_seq_len(s, i) + local b = s:byte(i) + if len and not (len == 1 and (b < 0x20 or b == 0x7F)) then + out[#out + 1] = s:sub(i, i + len - 1) + i = i + len + else + out[#out + 1] = string.format("\\x%02X", b) + i = i + 1 + end + end + return table.concat(out) +end + +--- `s` with invalid UTF-8 bytes replaced by U+FFFD, controls left +--- alone. For MULTI-LINE bodies (a patch), where newlines and tabs are +--- content rather than a hazard. +local function utf8_clean(s) + if pmacs.git.is_text(s) then return s end + local out = {} + local i, n = 1, #s + while i <= n do + local len = utf8_seq_len(s, i) + if len then + out[#out + 1] = s:sub(i, i + len - 1) + i = i + len + else + out[#out + 1] = "\239\191\189" -- U+FFFD + i = i + 1 + end + end + return table.concat(out) +end + +-- --------------------------------------------------------------------- +-- Running git (Q#G-2) +-- --------------------------------------------------------------------- + +--- The argv for a git invocation rooted at `root`. +--- +--- Every call site goes through here, so `--no-optional-locks` cannot +--- be dropped by one of them. The flag is part of the contract, not a +--- nicety (Q#G-6): `git status` is not strictly read-only --- it may +--- refresh and write the index --- and this module runs it +--- asynchronously from an editor while the user may be running git in a +--- terminal, which is the exact scenario the flag exists for. +function pmacs.git.argv(root, rest) + local args = { "--no-optional-locks" } + if root then + args[#args + 1] = "-C" + args[#args + 1] = root + end + for _, a in ipairs(rest) do args[#args + 1] = a end + return args +end + +-- proc raw id -> { procid, out, err, on_done } +local pump = {} + +-- Spawn git and call `on_done { ok, code, kind, stdout, stderr }` once +-- it terminates. A spawn failure calls `on_done` too, with +-- `spawn_error` set --- §1.2's silence asymmetry: the failure must be +-- surfaced with guidance, never swallowed. +local function run_git(label, purpose, root, rest, on_done) + local spec = { + label = label, + -- Required since worker identity Stage 1 (#232), and deliberately + -- NOT the label. `label` identifies the process --- "git status" --- + -- while this says what the run is FOR, which is what someone reading + -- `*workers*` or the statusline activity indicator needs: three of + -- this module's spawns are all "git", and only the purpose tells + -- them apart. Each call site writes its own; copying the label + -- across is the failure that ruling was made against. + purpose = purpose, + command = pmacs.git._program, + args = pmacs.git.argv(root, rest), + stdin = "null", + } + if root then spec.cwd = root end + pmacs.git._last_spawn = { + command = spec.command, args = spec.args, cwd = spec.cwd, label = spec.label, + purpose = spec.purpose, + } + local log = pmacs.git._spawn_log + log[#log + 1] = spec.args + while #log > SPAWN_LOG_LIMIT do table.remove(log, 1) end + local ok, proc = pcall(pmacs.process.spawn, spec) + if not ok then + on_done { + ok = false, kind = "spawn_failed", spawn_error = tostring(proc), + stdout = "", stderr = "", + } + return nil + end + pump[proc:raw()] = { procid = proc, out = {}, err = {}, on_done = on_done } + return proc +end + +pmacs.hook.add("process.after-tick", function() + for raw, entry in pairs(pump) do + for _, ev in ipairs(pmacs.process.events_take(entry.procid)) do + local kind = ev.kind + if kind == "stdout" then + entry.out[#entry.out + 1] = ev.bytes + elseif kind == "stderr" then + entry.err[#entry.err + 1] = ev.bytes + elseif kind == "exited" or kind == "signaled" or kind == "crashed" then + -- The supervisor drains all remaining output BEFORE pushing the + -- terminal event (`final_drain_runtime`, src/process.rs), so one + -- pass in event order captures everything. + pump[raw] = nil + pcall(pmacs.process.forget, entry.procid) + local result = { + ok = (kind == "exited"), + code = (kind == "exited") and (ev.code or 0) or nil, + kind = kind, + signal = ev.signal, + error = ev.error, + stdout = table.concat(entry.out), + stderr = table.concat(entry.err), + } + local called, err = pcall(entry.on_done, result) + if not called then + pmacs.editor.set_status("git: " .. tostring(err)) + end + end + end + end +end) + +--- The first line of `text`, trimmed, or `""`. +--- +--- For text bound for the ONE-LINE STATUS BAND, and only for that: a +--- spawn error, a stderr detail, an error string. A status message that +--- carried a newline would corrupt the row layout of whatever is +--- rendering it, so truncating to the first line is the right answer +--- there. +local function first_line(text) + local line = (text or ""):match("^[^\r\n]*") or "" + return (line:gsub("%s+$", "")) +end + +--- `text` with git's final output terminator removed, and NOTHING else. +--- +--- The counterpart to `first_line`, deliberately a SECOND function +--- rather than a change to it, because the two answer opposite +--- questions and each has callers that the other's answer would break. +--- This one is for COMMAND OUTPUT that must survive whole: a POSIX path +--- may legally contain a newline, so `git rev-parse --show-toplevel` +--- prints one for a repository rooted at `/tmp/a\nb`, and taking the +--- first line there truncates the root to `/tmp/a` --- after which every +--- command this module runs has a wrong or nonexistent cwd. `first_line` +--- has three other callers, all of them status-band text, and folding +--- the two together would fix this one and break those. +--- +--- Exactly ONE trailing `\n` is stripped, and NOTHING may ride along +--- with it --- not a carriage return, not a second newline. A carriage +--- return is as legal a POSIX path byte as a newline is, so a repository +--- rooted at `/tmp/a\r` makes git print `/tmp/a` `0d` `0a`: the path's +--- own CR, then git's LF terminator. A strip tolerant of `\r?\n$` cannot +--- tell those two bytes apart and takes both, resolving the root as +--- `/tmp/a` --- the same defect this function was written to fix, one +--- byte over. A second newline is output, not a terminator, for the same +--- reason. +--- +--- There is no unambiguous output representation to prefer instead, +--- which was CHECKED against git 2.55 rather than assumed: `-z` is not +--- an option of `git rev-parse` at all. 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 --- so asking for it +--- would corrupt the very output it was meant to disambiguate, silently +--- and with exit code 0. `--show-toplevel` applies no C quoting either, +--- not even under `core.quotePath=true`. Removing the one byte git +--- appended is therefore the whole of the correct answer. +--- +--- Written as an explicit last-byte test rather than a pattern: an +--- anchored Lua pattern is where both of this function's bugs lived. +local function strip_output_terminator(text) + text = text or "" + if text:sub(-1) == "\n" then return text:sub(1, -2) end + return text +end + +--- A one-line description of why a git invocation failed. +local function failure_reason(res) + if res.spawn_error then + return string.format( + "cannot run %q (%s) --- is git installed and on PATH?", + pmacs.git._program, first_line(res.spawn_error)) + end + local detail = first_line(utf8_clean(res.stderr)) + if res.kind == "signaled" then + return string.format("git was killed by %s%s", + res.signal or "a signal", detail ~= "" and (": " .. detail) or "") + end + if res.kind == "crashed" then + return string.format("git crashed: %s", res.error or detail) + end + return string.format("git exited with code %d%s", + res.code or -1, detail ~= "" and (": " .. detail) or "") +end + +-- --------------------------------------------------------------------- +-- Porcelain v2 parsing (Q#G-6) +-- --------------------------------------------------------------------- +-- +-- The SEPARATION here --- pure `parse_*` functions that take a string +-- and return structure, testable with no repository --- is ported from +-- `tests/fixtures/pmacs-magit/status.lua`, whose 32-test suite proves +-- the shape works. The record TOKENIZER is deliberately NOT ported: the +-- fixture reads newline-delimited v2 and this reads `-z`, and those are +-- different grammars. Under `-z` a record's fields are NUL-terminated, +-- so a rename carries its two paths as SEPARATE fields rather than +-- tab-joined inside one, and C quoting is removed from the problem +-- entirely rather than obliging a hand-written unquoter. +-- +-- The fixture is left untouched (Q#G-0): its purpose is to prove the +-- PACKAGE SYSTEM can host this, and bundled code becoming its +-- dependency would make `tests/m8_6_acceptance.rs` test less than it +-- claims. The duplication is deliberate and stated rather than quiet. + +-- Split NUL-terminated fields. A trailing empty fragment after the last +-- NUL is dropped; an empty payload yields no fields. +local function nul_fields(text) + local out = {} + if type(text) ~= "string" or text == "" then return out end + local i = 1 + while true do + local nul = text:find("\0", i, true) + if not nul then + if i <= #text then out[#out + 1] = text:sub(i) end + break + end + out[#out + 1] = text:sub(i, nul - 1) + i = nul + 1 + end + return out +end + +--- Parse `git status --porcelain=v2 --branch -z` output. +--- +--- Returns `{ branch = {...}, rows = {...} }` where each row is +--- +--- { kind = "ordinary"|"rename"|"unmerged"|"untracked"|"ignored", +--- xy, x, y, path, orig, score } +--- +--- `path` is the CURRENT path --- what the panel shows and what RET +--- visits --- and `orig` remembers where a rename or copy came from. +--- Both are raw bytes; nothing here assumes they are text. +--- +--- **`kind = "rename"` covers copies too, deliberately.** A `2` record +--- is porcelain-v2's ONE two-path record and every behaviour keyed on +--- it is the same for both classes --- notably the two-path +--- `git diff HEAD -- `. What differs is only what the +--- user is TOLD, and that fact is already carried: `score` leads with +--- `R` or `C` (and `xy` carries the same letter on whichever side +--- detected it). See `is_copy` for where the two are told apart. +function pmacs.git.parse_status(text) + local branch = { unborn = false } + local rows = {} + local fields = nul_fields(text) + local i = 1 + while i <= #fields do + local field = fields[i] + local tag = field:sub(1, 1) + if tag == "#" then + local key, value = field:match("^# (%S+) (.*)$") + if key == "branch.oid" then + branch.oid = value + -- Unborn HEAD, from the output already being parsed. A second + -- `rev-parse --verify HEAD` would be a whole extra process for + -- a fact this line hands over. + branch.unborn = (value == "(initial)") + elseif key == "branch.head" then + branch.head = value + elseif key == "branch.upstream" then + branch.upstream = value + elseif key == "branch.ab" then + branch.ahead = tonumber(value:match("^%+(%d+)") or "") + branch.behind = tonumber(value:match("%-(%d+)$") or "") + end + elseif tag == "1" then + -- 1 + local xy, path = field:match("^1 (%S%S) %S+ %S+ %S+ %S+ %S+ %S+ (.+)$") + if xy then + rows[#rows + 1] = { + kind = "ordinary", xy = xy, x = xy:sub(1, 1), y = xy:sub(2, 2), path = path, + } + end + elseif tag == "2" then + -- 2 \0 + -- + -- The origin is the NEXT field, not a tab-joined suffix. That is + -- the whole reason this tokenizer is not the fixture's. + local xy, score, path = + field:match("^2 (%S%S) %S+ %S+ %S+ %S+ %S+ %S+ (%S+) (.+)$") + if xy then + i = i + 1 + rows[#rows + 1] = { + kind = "rename", xy = xy, x = xy:sub(1, 1), y = xy:sub(2, 2), + path = path, orig = fields[i], score = score, + } + end + elseif tag == "u" then + -- u

+ local xy, path = + field:match("^u (%S%S) %S+ %S+ %S+ %S+ %S+ %S+ %S+ %S+ (.+)$") + if xy then + rows[#rows + 1] = { + kind = "unmerged", xy = xy, x = xy:sub(1, 1), y = xy:sub(2, 2), path = path, + } + end + elseif tag == "?" then + local path = field:match("^%? (.+)$") + if path then + rows[#rows + 1] = { kind = "untracked", xy = "??", x = "?", y = "?", path = path } + end + elseif tag == "!" then + local path = field:match("^! (.+)$") + if path then + rows[#rows + 1] = { kind = "ignored", xy = "!!", x = "!", y = "!", path = path } + end + end + i = i + 1 + end + return { branch = branch, rows = rows } +end + +-- --------------------------------------------------------------------- +-- Request channels: "the newest INVOCATION wins" +-- --------------------------------------------------------------------- +-- +-- EVERY defect review found on this module was one shape: module-level +-- mutable state read or written at CONTINUATION time without an +-- invocation-time ticket. So the rule gets one implementation instead of +-- a bespoke counter per call site. +-- +-- A channel hands out a ticket when the user ASKS for something and +-- answers "is this still the request in force?" when a subprocess +-- finally replies. A continuation holding a stale ticket must discard +-- BEFORE ANY EFFECT --- no spawn, no shared-state write, and no status +-- message either, since a message from a replaced invocation is as wrong +-- as a panel from one. +-- +-- There are TWO channels, and that is deliberate rather than an +-- oversight: a single module-wide counter would make pressing `d` cancel +-- an in-flight `g`, and vice versa. The status panel and the diff view +-- are independent things a user can ask for, so each gets its own "newest +-- wins" ordering. What is shared is the MECHANISM, not the counter. +-- +-- A channel spans a whole request, not one process: a status open is +-- `rev-parse` then `status`, and a diff is one or two `git diff` runs. +-- Every stage of one request carries the ticket reserved at the command. +local function new_channel() + local ch = { current = 0 } + --- Claim the newest ticket, and return it. Called at the point a user + --- ASKS for something, never at the point some subprocess answers --- + --- minting on arrival makes the SLOWEST subprocess win instead of the + --- newest invocation, which is exactly the bug this exists to stop. + function ch.reserve() + ch.current = ch.current + 1 + return ch.current + end + --- True while `ticket` is still the request in force. + function ch.is_current(ticket) + return ticket == ch.current + end + return ch +end + +local status_requests = new_channel() +local diff_requests = new_channel() + +-- --------------------------------------------------------------------- +-- Panel state +-- --------------------------------------------------------------------- + +-- `display` maps a panel DATA LINE (1-based; the header is line 0) to +-- the git row rendered there. It is this module's own copy because +-- listview's line map is private, and `d` needs the row under the +-- cursor. `rows` is the same information as an array, for re-seating. +local state = { + root = nil, + branch = nil, + rows = {}, + display = {}, + buffer = nil, + diff_buffer = nil, + failure = nil, +} + +-- A copy and a rename are already TOLD APART here, and by the field +-- that tells every other row class apart: the `XY` prefix reads `R.` +-- for one and `C.` for the other, out of the same byte `score` leads +-- with. So this renders both the same way on purpose --- `<-` reads +-- "came from", which is true of a copy --- rather than growing a second +-- vocabulary beside the porcelain codes the whole panel is built on. +local function status_line_text(row) + local shown = pmacs.git.display_path(row.path) + if row.orig then + shown = string.format("%s <- %s", shown, pmacs.git.display_path(row.orig)) + end + return string.format("%s %s", row.xy, shown) +end + +local function status_header() + -- A failed run has no branch and no rows, and rendering "0 changes" + -- above a failure row would be a small lie in the one place the panel + -- most needs to be honest. + if state.failure then + return "git: status failed g retry q quit" + end + local branch = state.branch or {} + local where + if branch.unborn then + where = string.format("%s (no commits yet)", branch.head or "HEAD") + elseif branch.head == "(detached)" or branch.head == nil then + where = string.format("detached at %s", (branch.oid or "?"):sub(1, 8)) + else + where = branch.head + end + local n = #state.rows + return string.format( + "git: %s --- %d change%s RET visit d diff n/p move g refresh q quit", + where, n, n == 1 and "" or "s") +end + +-- Build the listview rows and (re)build `state.display` alongside them, +-- so the two can never drift. +local function listview_rows(extra_text) + state.failure = nil + state.display = {} + local out = {} + for _, row in ipairs(state.rows) do + out[#out + 1] = { text = status_line_text(row), item = row } + state.display[#out] = row + end + if extra_text then + out[#out + 1] = { text = extra_text } + end + return out +end + +-- Row-level failure (Q#G-1 item 4): a failure is a ROW, not a silence. +local function failure_rows(reason) + state.failure = reason + state.rows = {} + state.display = {} + return { { text = "! " .. reason } } +end + +-- --------------------------------------------------------------------- +-- Visiting (RET) +-- --------------------------------------------------------------------- + +local function join_root(path) + if path:sub(1, 1) == "/" then return path end + return (state.root or ".") .. "/" .. path +end + +local function refuse_unrepresentable(row) + pmacs.editor.set_status(string.format( + "git: %s is not valid UTF-8, so pmacs cannot open or diff it", + pmacs.git.display_path(row.path))) +end + +local function visit_row(row) + if type(row) ~= "table" or not row.path then return end + if not pmacs.git.is_text(row.path) then + refuse_unrepresentable(row) + return + end + local target = join_root(row.path) + pmacs.editor.push_jump() + -- A visit FROM a panel lands in the DOCUMENT target and leaves the + -- panel where it is (Q#BP11b) --- `display_file`, never the raw + -- switch, which would clobber the panel with the source. + local ok, err = pcall(pmacs.window.display_file, target, { select = true }) + if not ok then + pmacs.editor.jump_back() + pmacs.editor.set_status(string.format("git: cannot open %s: %s", + pmacs.git.display_path(row.path), first_line(tostring(err)))) + end +end + +-- --------------------------------------------------------------------- +-- The status refresh (Q#G-1) +-- --------------------------------------------------------------------- + +local function open_status_panel(rows) + pmacs.listview.open { + name = STATUS_PANEL, + header = status_header(), + rows = rows, + -- `d` is not on listview's key surface (RET SPC n p TAB + -- g q), and it cannot be bound from outside the primitive safely: + -- a name collision disambiguates to `<2>`, so the name passed here + -- is not necessarily the buffer that came back. The `keys` table + -- binds it through the primitive's own buffer-local path, so no key + -- is intercepted and COHERENCE.md §6 stays at six shadows. + keys = { d = "git.diff-file" }, + on_visit = visit_row, + on_refresh = function() return pmacs.git._on_refresh() end, + } + -- `listview.open` takes `select = true`, so the panel is the active + -- buffer here. Captured rather than looked up by name, because the + -- name may have been disambiguated. + state.buffer = pmacs.window.buffer() +end + +-- --------------------------------------------------------------------- +-- The destination boundary (Q#JR14, Q#DC-1) +-- --------------------------------------------------------------------- +-- +-- Every continuation in this module renders a tick or more after the +-- keypress that asked for it, and until #231 there was nothing it could +-- render *to* except ambient state: whichever frontend happened to be +-- active when git exited. Run `M-x git.status` in frontend A, let B +-- become active while `git status` runs, and A's panel opened in B. +-- +-- So the destination is CAPTURED AT INVOCATION and threaded through, +-- exactly as this module already threads the generation, the root and +-- the row --- and for the same reason. `state.root` has a comment +-- explaining why reading module-level state per step is wrong; the +-- ambient frontend is that same argument one layer down, and it was the +-- one thing still being read late. + +--- Run `body` against the destination captured at invocation. +--- +--- Returns false when the commit is REFUSED --- the captured window or +--- buffer is gone, or the frontend can no longer satisfy `profile`. +--- A refusal drops the render, which is the same answer the +--- `expect_buffer` rule already gives when the panel a refresh belongs +--- to has been killed: a result whose destination no longer exists is +--- not a result to force somewhere else. `commit_to` refuses BEFORE the +--- body runs, so a refusal is mutation-free and there is no partial +--- render to undo. +--- +--- Deliberately not `pcall`-wrapped. A refusal returns `(false, reason)` +--- and is handled here; anything that RAISES is a defect in this module +--- (a fabricated destination, an unknown profile) and must not be +--- swallowed into a silent no-op. +local function commit_ui(dest, profile, body) + local ok, reason = pmacs.window.commit_to(dest, body, profile) + if ok == false then return false, reason end + return true +end + +--- The status-refresh ticket currently in force. +--- +--- Exposed alongside `_deliver_status` below, and for the same reason: +--- the discard rule is about a completion arriving LATE, and a caller +--- cannot construct a stale request without knowing what "current" +--- means. +function pmacs.git._generation() + return status_requests.current +end + +--- The diff ticket currently in force. `_generation`'s counterpart, on +--- the other channel; see `new_channel` for why they are two. +function pmacs.git._diff_generation() + return diff_requests.current +end + +--- Deliver a completed `git status`. +--- +--- Exposed because concurrent refresh is asserted by DRIVING two +--- refreshes and completing them out of order, which no arrangement of +--- real subprocess timing can guarantee. +function pmacs.git._deliver_status(request, res) + -- Generation (Q#G-1 item 3): a second `g` while one is in flight + -- bumps the ticket, and the older completion DISCARDS its rows rather + -- than racing. It does not terminate the first process --- reaping is + -- `process.forget`'s job and killing git mid-read buys nothing. + if not status_requests.is_current(request.generation) then return end + -- Panel lifetime (Q#G-1 item 5): if the buffer this refresh belongs + -- to is gone, drop the result. A FIRST open carries no expectation. + if request.expect_buffer ~= nil then + local live, valid = pcall(request.expect_buffer.is_valid, request.expect_buffer) + if not (live and valid) then return end + end + + -- Rows and the status line are COMPUTED here and EMITTED inside the + -- commit below. Splitting them is the point: `set_status` is a UI + -- mutation, and a failure message announcing a panel that the commit + -- then refuses to open is the misrouting this boundary exists to + -- prevent, in its most confusing form. + local rows, message + if res.ok and res.code == 0 then + local parsed = pmacs.git.parse_status(res.stdout) + state.branch = parsed.branch + state.rows = parsed.rows + rows = listview_rows(nil) + else + local reason = failure_reason(res) + state.branch = state.branch or {} + rows = failure_rows(reason) + message = "git status: " .. reason + end + + -- The PANEL profile: `*git-status*` is a bottom-panel surface + -- (`listview.open` defaults `display` to `"panel"`), so the + -- stale-intent checks that guard replacing a document window do not + -- apply --- but only while the placement really is a panel, which is + -- what the profile's own refusal keeps true for the extent of this + -- body (Q#DC-2). + commit_ui(request.dest, "panel", function() + if message then pmacs.editor.set_status(message) end + open_status_panel(rows) + -- `listview.open` resets collapse and does NOT preserve selection --- + -- only `listview.refresh` does, and that is the synchronous path this + -- model cannot use. So re-seating is owned here. + -- + -- And `open`'s own `seat_cursor(p, 1)` cannot be relied on either: it + -- walks DOWN from wherever the cursor is, on the premise that a fresh + -- `switch_active_buffer` zeroed it. Re-opening a panel that is + -- already displayed does not zero anything, so that walk would land + -- one row below the previous cursor instead of on row 1. The handler + -- therefore seats unconditionally, from line 0. + local target = 1 + if request.selected_path then + for i, row in ipairs(state.rows) do + if row.path == request.selected_path then + target = i + break + end + end + end + -- If the captured path is gone --- the commonest case, since a file + -- that stopped being modified drops out of status --- `target` stays + -- 1 and nothing is said about it. That is the correct answer, not a + -- failure. + pmacs.editor.clear_selection() + pmacs.editor.set_view_top(0) + pmacs.editor.move_to_line(0) + -- Walked with `move_down` rather than a single `move_to_line(target)` + -- because motion is what drags the viewport along; a bare cursor set + -- would leave a long status list scrolled to the top with the cursor + -- off screen. This is `listview.refresh`'s own idiom. + for _ = 1, target do pmacs.editor.move_down() end + end) +end + +-- The path of the row under the cursor right now, or nil. +local function selected_path() + local row = state.display[pmacs.editor.cursor_line()] + return row and row.path or nil +end + +--- Spawn `git status` under an ALREADY-RESERVED generation. +--- +--- The generation is a parameter rather than something minted here, +--- because this runs from the root-resolution callback: two `git.status` +--- invocations against different repositories resolve their roots +--- concurrently, and if the generation were minted on arrival then the +--- invocation whose `rev-parse` returned LAST would claim the newest +--- generation and replace the newer request. Reserving at the command +--- and carrying it through is what makes the ordering the user's, not +--- the filesystem's. +local function start_status(root, expect_buffer, want_selection, generation, dest) + local request = { + generation = generation, + expect_buffer = expect_buffer, + selected_path = want_selection and selected_path() or nil, + -- A parameter for the same reason `generation` is: it belongs to + -- the invocation, and this function can run from a root-resolution + -- callback that is already a tick removed from it. + dest = dest, + } + run_git("git status", + "reading the working tree status of " .. root .. " for the *git-status* panel", + root, + { "status", "--porcelain=v2", "--branch", "-z" }, + function(res) pmacs.git._deliver_status(request, res) end) +end + +--- `g` inside the panel. +--- +--- `on_refresh` stays SYNCHRONOUS and honest: it returns the current +--- rows immediately with a marker appended and KICKS OFF the spawn, so +--- `g` always re-renders and always shows that work started. A `g` that +--- silently does nothing is a defect this primitive already names. +function pmacs.git._on_refresh() + if not git_enabled() then + return listview_rows("(git.enabled is false --- nothing was run)") + end + if not state.root then + return listview_rows("(no repository --- run M-x git.status)") + end + -- Reserved HERE, at the keypress, for the same reason `git.status` + -- reserves at the command: `g` needs no root lookup, so this is + -- already the moment of invocation. The destination is captured on + -- the same line of reasoning --- `g` is pressed IN the panel, so the + -- frontend showing it is the one this refresh belongs to, and that is + -- true now and possibly not true when git exits. + start_status(state.root, state.buffer, true, status_requests.reserve(), + pmacs.window.capture_destination()) + return listview_rows("(refreshing...)") +end + +-- --------------------------------------------------------------------- +-- Entry point +-- --------------------------------------------------------------------- + +local function directory_of(path) + local dir = path:match("^(.*)/[^/]*$") + if dir == nil or dir == "" then return "/" end + return dir +end + +--- The directory a status run should resolve its repository from. +--- +--- The ACTIVE FILE's directory, falling back to the daemon's working +--- directory when the active buffer is pathless (a dired listing, the +--- scratch buffer). Deliberately not a project-marker walk: git resolves +--- its own worktree, and `ProjectKind` cannot answer the question at all. +local function active_directory() + local buf = pmacs.window.buffer() + if buf then + local ok, path = pcall(function() return buf:path() end) + if ok and type(path) == "string" and path ~= "" then + return directory_of(path) + end + end + local ok, id = pcall(pmacs.instance.identity) + if ok and type(id) == "table" and type(id.working_directory) == "string" then + return id.working_directory + end + return nil +end + +--- Deliver a completed root lookup for the invocation in `request`. +--- +--- Exposed for the same reason `_deliver_status` is: the contract is +--- about completions arriving in an order the CALLER did not choose, and +--- no arrangement of real subprocess timing can guarantee that two +--- `rev-parse` runs finish in a chosen order. +function pmacs.git._deliver_root(request, res) + -- A root lookup that returns after a newer invocation has superseded + -- it must not proceed: not to a status spawn, not to `state.root`, and + -- not even to a status-line message. Everything below this line is an + -- effect belonging to an invocation the user has already replaced. + if not status_requests.is_current(request.generation) then return end + + if not (res.ok and res.code == 0) then + if res.spawn_error then + pmacs.editor.set_status("git: " .. failure_reason(res)) + else + pmacs.editor.set_status( + string.format("git: %s is not inside a repository", request.dir)) + end + return + end + -- The WHOLE output, minus its terminator --- never the first line, and + -- never a byte more than git appended. A repository root may contain a + -- newline and may END in a carriage return, and losing either here + -- would point every command that follows at a directory that does not + -- exist. + local root = strip_output_terminator(res.stdout) + if root == "" then + pmacs.editor.set_status("git: rev-parse returned no worktree root") + return + end + state.root = root + -- A fresh open carries no buffer expectation, so a panel the user + -- killed earlier does not make this run drop its own first result. + state.buffer = nil + -- The generation reserved at the command, NOT a fresh one --- and the + -- destination captured there, for the same reason. This callback runs + -- after `git rev-parse` has exited, so capturing here would read the + -- ambient frontend a round trip late and reintroduce exactly the + -- misrouting the capture exists to close. + start_status(root, nil, false, request.generation, request.dest) +end + +--- Open (or re-open) `*git-status*` for the repository containing the +--- active file. +function pmacs.git.status() + if not git_enabled() then + pmacs.editor.set_status("git: disabled by the `git.enabled` setting") + return + end + local dir = active_directory() + if not dir then + pmacs.editor.set_status("git: no directory to resolve a repository from") + return + end + -- Reserved AFTER the early returns and BEFORE the spawn: an + -- invocation that starts no work must not invalidate one that is + -- already in flight, and an invocation that does start work must own + -- the newest generation from that moment on. + -- Captured alongside the generation, at the same moment and for the + -- same reason: this is the last instant at which "the frontend the + -- user asked from" is knowable without guessing. + local request = { + generation = status_requests.reserve(), + dir = dir, + dest = pmacs.window.capture_destination(), + } + -- The root rule (Q#G-2): ask git, and let a non-zero exit BE the + -- "not a repository" answer. `-C ` with no root of our own. + run_git("git rev-parse", + "resolving which Git repository contains " .. dir, + nil, { "-C", dir, "rev-parse", "--show-toplevel" }, + function(res) pmacs.git._deliver_root(request, res) end) +end + +pmacs.command.define { + name = "git.status", + description = "Show the working tree's Git status in a *git-status* panel.", + fn = pmacs.git.status, +} + +-- --------------------------------------------------------------------- +-- The diff gesture (Q#G-7) +-- --------------------------------------------------------------------- +-- +-- RET visits the file --- the behaviour a list of files should have --- +-- so the diff needs its own key, and `d` is it. +-- +-- What `d` shows answers the lane's own question, "what have I +-- changed?", against HEAD. A porcelain-v2 row carries an XY pair (X +-- staged, Y unstaged) 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. One view of the TOTAL change +-- is right for reading; splitting X from Y is a staging UI, which is +-- Stage 3. +-- +-- `--no-color` because this renders as plain text and a user with +-- `color.ui = always` would otherwise get escape sequences in a buffer +-- with no ANSI parser behind it. + +local SPLIT_HEADER = + "no commits yet --- split view: staged (index) above, unstaged (worktree) below" + +-- True when a `2` record is a COPY rather than a rename. +-- +-- Read from `score`, not from `row.x`. The `` field names +-- rename-vs-copy whichever side detected the change, while `X` carries +-- the letter only for an index-side one --- a worktree-side detection +-- puts it in `Y` and leaves `X` a `.`. Absent or malformed, this says +-- "not a copy", so the header falls back to the commoner of the two +-- rather than to a claim it cannot support. +local function is_copy(row) + return (row.score or ""):sub(1, 1) == "C" +end + +-- The invocations `d` runs for `row`, as +-- `{ { label = string|nil, args = {...}, no_index = bool }, ... }`, +-- plus the header describing what the result shows. +local function diff_plan(row, unborn) + local path = row.path + if row.kind == "untracked" or row.kind == "ignored" then + -- A normal diff shows NOTHING for an untracked file. Without this + -- case `d` is silently dead on the rows a user is most likely to + -- press it on. + return { + header = "untracked --- shown against /dev/null", + steps = { { args = { "diff", "--no-color", "--no-index", "--", "/dev/null", path }, + no_index = true } }, + } + end + if not unborn then + if row.kind == "rename" and row.orig then + -- Both paths, which is what lets rename detection render this as + -- a rename rather than an unrelated add plus delete. Identical for + -- a copy, which is why the two share a `kind` --- only the WORD + -- differs, because a copy left the origin where it was and saying + -- "renamed" of it states a different fact about the user's tree. + return { + header = string.format("against HEAD (%s from %s)", + is_copy(row) and "copied" or "renamed", + pmacs.git.display_path(row.orig)), + steps = { { args = { "diff", "--no-color", "HEAD", "--", row.orig, path } } }, + } + end + return { + header = "against HEAD", + steps = { { args = { "diff", "--no-color", "HEAD", "--", path } } }, + } + end + -- Unborn HEAD: there is nothing to total AGAINST, so the split + -- appears here and only here. `AM` and `AD` carry BOTH states at + -- once, which is exactly the gap: `--cached` alone loses the worktree + -- delta and plain `git diff` alone loses the staged base. + local staged = row.x ~= "." and row.x ~= " " + local unstaged = row.y ~= "." and row.y ~= " " + local cached = { label = "staged (index)", + args = { "diff", "--no-color", "--cached", "--", path } } + local worktree = { label = "unstaged (worktree)", + args = { "diff", "--no-color", "--", path } } + if staged and unstaged then + return { header = SPLIT_HEADER, steps = { cached, worktree } } + end + if staged then + return { header = "no commits yet --- staged (index) only", steps = { cached } } + end + return { header = "no commits yet --- unstaged (worktree) only", steps = { worktree } } +end + +local function diff_step_ok(step, res) + if not res.ok then return false end + -- `--no-index` implies `--exit-code`: exit 1 means it SUCCESSFULLY + -- found differences, which is the whole point of running it. Under a + -- plain "non-zero is failure" predicate every untracked diff would + -- render a failure row instead of the diff it just produced. The + -- asymmetry is confined to this invocation. + if step.no_index then return (res.code or 0) <= 1 end + return (res.code or 0) == 0 +end + +-- Ownership is the HANDLE this module holds, never a name match --- +-- listview's Q#GB13 rule and dired's F7 rule, for the same reason. +-- `pmacs.buffer.create` takes any caller-chosen name, so a user may +-- already have a buffer called `*git-diff*`; adopting it would clobber +-- their bytes and then lock the rope. A fresh create leaves theirs +-- untouched, and this module writes only to the handle it made. +local function show_diff_buffer(title, body) + local buf = state.diff_buffer + local live = buf ~= nil and select(2, pcall(buf.is_valid, buf)) == true + if not live then + buf = pmacs.buffer.create(DIFF_BUFFER) + pmacs.buffer.add_intercept(buf, function() + error(DIFF_BUFFER .. " is read-only") + end) + pmacs.buffer.set_round_trip_input(buf, true) + state.diff_buffer = buf + end + pmacs.buffer.set_generated_contents(buf, title .. "\n\n" .. body) + -- The DOCUMENT target, so the status panel it was invoked from stays + -- visible beside it. + pcall(pmacs.window.display, buf, { select = true }) +end + +-- Start the request's next step, or render what the finished ones +-- produced. +-- +-- Everything this needs lives on `request`, captured at the keypress: +-- the diff ticket, the row, the plan, and the ROOT. `state.root` is +-- deliberately not read anywhere in here. An unborn `AM`/`AD` row +-- produces a TWO-STEP plan, and `state.root` is module-level mutable +-- state that a concurrent `git.status` against another repository +-- reassigns from its own root-resolution callback --- so reading it per +-- step would let one plan's second step run in a different repository +-- than its first, carrying the first repository's path. +local function advance_diff(request) + request.index = request.index + 1 + local step = request.plan.steps[request.index] + if not step then + local body = table.concat(request.pieces, "\n") + if body:gsub("%s", "") == "" then + body = "(no differences)" + end + -- The DOCUMENT profile, and the contrast with the status channel is + -- the whole of Q#DC-2: `*git-diff*` REPLACES a document window, so + -- every stale-intent check applies. If the window the `d` was + -- pressed from now holds a different buffer, this diff is answering + -- a question about a view the user has already left, and the commit + -- is refused rather than allowed to overwrite it. + commit_ui(request.dest, "document", function() + show_diff_buffer(string.format("git diff --- %s\n%s", + pmacs.git.display_path(request.row.path), request.plan.header), body) + end) + return + end + run_git("git diff", + "diffing " .. pmacs.git.display_path(request.row.path) .. " against HEAD for *git-diff*", + request.root, step.args, + function(res) pmacs.git._deliver_diff(request, step, res) end) +end + +--- Deliver a completed diff STEP for the plan in `request`. +--- +--- Exposed for the same reason `_deliver_status` and `_deliver_root` +--- are: the contract is about completions arriving in an order the +--- CALLER did not choose, and no arrangement of real subprocess timing +--- can guarantee that two `git diff` runs finish in a chosen order. +function pmacs.git._deliver_diff(request, step, res) + -- Superseded by a newer `d`: discard BEFORE ANY EFFECT. `*git-diff*` + -- is a singleton buffer, so without this a slow first request + -- overwrites a fast second one --- the newest invocation loses to the + -- slowest subprocess, which is the same defect the status channel had. + -- + -- This is the ONE place a diff plan re-enters from a continuation, so + -- one check covers all of it: no further spawn (`advance_diff` is + -- never reached), no buffer write, and no status message either, since + -- a status line from a replaced invocation is as wrong as a buffer + -- from one. The in-flight process is not terminated --- reaping is + -- `process.forget`'s job, exactly as on the status channel. + if not diff_requests.is_current(request.generation) then return end + if not diff_step_ok(step, res) then + local reason = failure_reason(res) + -- The failure render is a render: same destination, same profile, + -- same refusal. Announcing a diff failure into whatever frontend + -- happens to be active would be the identical misrouting as + -- announcing a success there. + commit_ui(request.dest, "document", function() + show_diff_buffer(string.format("git diff --- %s", + pmacs.git.display_path(request.row.path)), reason) + pmacs.editor.set_status("git diff: " .. reason) + end) + return + end + local text = utf8_clean(res.stdout) + if step.label then + request.pieces[#request.pieces + 1] = string.format("=== %s ===\n%s", step.label, + text ~= "" and text or "(no changes)\n") + else + request.pieces[#request.pieces + 1] = text + end + advance_diff(request) +end + +-- Run `plan`'s steps in order under an ALREADY-RESERVED diff ticket, +-- then render. `generation` is a parameter for the same reason +-- `start_status`'s is: it belongs to the keypress, not to this call. +local function run_diff_plan(row, plan, root, generation, dest) + advance_diff { + generation = generation, row = row, plan = plan, root = root, + pieces = {}, index = 0, dest = dest, + } +end + +pmacs.command.define { + name = "git.diff-file", + description = "Show the diff for the file under the cursor in *git-status*.", + fn = function() + -- Bound buffer-locally on the panel, so `d` can only reach this + -- there; the identity check is for the `M-x` path. Compared against + -- the CAPTURED handle, never a name lookup: listview disambiguates + -- a collision to `<2>`, so the name is not the identity. + local active = pmacs.window.buffer() + if not (state.buffer and active and active == state.buffer) then + pmacs.editor.set_status("git: no *git-status* row here") + return + end + local row = state.display[pmacs.editor.cursor_line()] + if not (type(row) == "table" and row.path) then + pmacs.editor.set_status("git: no file on this line") + return + end + if not pmacs.git.is_text(row.path) + or (row.orig and not pmacs.git.is_text(row.orig)) then + refuse_unrepresentable(row) + return + end + if not git_enabled() then + pmacs.editor.set_status("git: disabled by the `git.enabled` setting") + return + end + -- Everything the plan runs on is captured HERE, at the keypress, and + -- threaded through every step: the ticket, reserved AFTER the early + -- returns above so a `d` that starts no work cannot supersede one + -- that is already in flight; the root; the unborn flag; and the + -- DESTINATION. The first three describe the repository the user is + -- looking at right now and are replaced wholesale by a `git.status` + -- against another one; the fourth describes the window they are + -- looking at it IN, which nothing in this module replaces and + -- nothing outside it announces. + run_diff_plan(row, diff_plan(row, (state.branch or {}).unborn == true), + state.root, diff_requests.reserve(), pmacs.window.capture_destination()) + end, +} diff --git a/builtin/runtime/listview.lua b/builtin/runtime/listview.lua index a10cf57..8e23d1d 100644 --- a/builtin/runtime/listview.lua +++ b/builtin/runtime/listview.lua @@ -25,6 +25,7 @@ -- rows = { { text = "src/foo.rs:12:4", item = }, ... }, -- 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" }, + { "", "cursor.down" }, + { "p", "cursor.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("", "cursor.down") - bind("p", "cursor.up") - bind("", "cursor.up") - bind("TAB", "listview.toggle") - bind("g", "listview.refresh") - bind("q", "listview.quit") +end + +-- --------------------------------------------------------------------- +-- Consumer-supplied keys (Q#G-7) +-- --------------------------------------------------------------------- +-- +-- An optional `keys = { = }` 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 p 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 diff --git a/docs/active-work.md b/docs/active-work.md index 53df4de..bebf9b8 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -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 ``" from +"reading the working tree status of ``" from "diffing `` +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 `/nl` instead of `/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 +`/trailing` against `/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 — `` 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 -- `, 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 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 diff --git a/docs/git-integration-framing.md b/docs/git-integration-framing.md new file mode 100644 index 0000000..95c8e03 --- /dev/null +++ b/docs/git-integration-framing.md @@ -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 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 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 +(). 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 p 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 p 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 -- ` | one view of the total change; splitting X from Y is a staging UI, which is Stage 3 | +| deleted | `git diff HEAD -- ` | shows the deletion; no special case needed | +| renamed / copied | `git diff HEAD -- ` | 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 ` | **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 -- ` 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 -- ` | +| **`AM` staged + edited** | **two labelled patches** — *staged* `git diff --cached -- `, then *unstaged* `git diff -- ` | +| **`AD` staged + deleted** | **two labelled patches**, same pair; the second renders the deletion | +| `.M` / `.D` unstaged only | one patch: `git diff -- ` | +| `?` untracked | `git diff --no-index -- /dev/null ` (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` +(`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 `` 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 -- ` 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 `. + +**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. diff --git a/docs/keybindings.md b/docs/keybindings.md index b08e4f4..181c637 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -231,12 +231,33 @@ all share one keymap: | `RET` / `SPC` | `listview.visit` — act on the item under the cursor | | `n` / `` | `cursor.down` | | `p` / `` | `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`: diff --git a/src/editor.rs b/src/editor.rs index 63f617d..458751b 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -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 diff --git a/tests/git_status_stage1_acceptance.rs b/tests/git_status_stage1_acceptance.rs new file mode 100644 index 0000000..aeea222 --- /dev/null +++ b/tests/git_status_stage1_acceptance.rs @@ -0,0 +1,2682 @@ +//! Git integration Stage 1 acceptance — +//! `docs/git-integration-framing.md` §6, one test per verification +//! bullet. +//! +//! Dispatch-driven wherever a key is claimed: `RET`, `d`, `g`, `n`/`p` +//! and `q` are exercised through `dispatch_key`, never through +//! `pmacs.command.invoke`, so a dead binding fails these tests instead +//! of passing review. +//! +//! Repository fixtures are built with the REAL `git` in a tempdir, and +//! every one of them **bounds project detection** with +//! `pmacs.project.set_search_boundary`. That is not tidiness: R8 was a +//! fixture whose marker walk escaped into the developer's own +//! environment, retired two commits before this branch's base, and a +//! tempdir under `/tmp` is exactly the shape that reaches it. +//! +//! The pure `parse_*` half runs with **no repository at all** — the +//! separation `tests/fixtures/pmacs-magit/status.lua` already proves +//! works, and the reason this lane ported that separation rather than +//! inventing one (Q#G-0). + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; +use pmacs::editor::EditorState; +use pmacs::protocol::{CellSize, FrontendId}; +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { + KeyEvent { + code, + modifiers: mods, + kind: KeyEventKind::Press, + state: KeyEventState::empty(), + } +} + +fn press(s: &mut EditorState, code: KeyCode) { + s.dispatch_key(FrontendId::LOCAL, key(code, KeyModifiers::NONE)); +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +fn status(s: &EditorState) -> String { + s.core.borrow().status.clone() +} + +fn errors_text(s: &EditorState) -> String { + s.lua_host.errors_buffer_text() +} + +fn active_name(s: &EditorState) -> String { + eval( + s, + "return pmacs.describe.buffer(pmacs.window.buffer()).name", + ) +} + +/// Text of the buffer named `name`, or `""` when absent. +fn named_text(s: &EditorState, name: &str) -> String { + let b: mlua::String = eval( + s, + &format!( + "for _, id in ipairs(pmacs.buffer.list()) do\n\ + if pmacs.describe.buffer(id).name == {name:?} then\n\ + return id:slice(0, id:len())\n\ + end\n\ + end\n\ + return \"\"" + ), + ); + String::from_utf8_lossy(&b.as_bytes()).into_owned() +} + +fn panel_text(s: &EditorState) -> String { + named_text(s, "*git-status*") +} + +fn diff_text(s: &EditorState) -> String { + named_text(s, "*git-diff*") +} + +/// Fresh editor with LSP spawning disabled and frame geometry declared. +/// +/// Geometry is not optional here: a listview opens into the PANEL by +/// default since bottom-panel Stage 3, and a panel is derived-hidden +/// while the frontend's frame size is unknown — so focus would fall +/// back to the document window and every assertion below would read the +/// wrong buffer. +fn editor() -> EditorState { + let s = EditorState::new_with_roots(&crate::iso::roots()); + exec(&s, "pmacs.lsp.config = {}"); + s.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(24, 80)); + s +} + +/// Drive frames until `pred` holds, pumping the process supervisor — +/// the production `process.after-tick` path this module's pump rides. +fn pump_until( + s: &mut EditorState, + timeout_ms: u64, + mut pred: impl FnMut(&EditorState) -> bool, +) -> bool { + let stop = Instant::now() + Duration::from_millis(timeout_ms); + loop { + if pred(s) { + return true; + } + if Instant::now() >= stop { + return false; + } + s.tick_processes(); + std::thread::sleep(Duration::from_millis(5)); + } +} + +// --------------------------------------------------------------------------- +// Repository fixtures (real `git`, in a tempdir) +// --------------------------------------------------------------------------- + +fn git(root: &Path, args: &[&str]) -> String { + let out = std::process::Command::new("git") + .current_dir(root) + .args(args) + .output() + .unwrap_or_else(|e| panic!("running git {args:?}: {e}")); + assert!( + out.status.success(), + "git {args:?} failed: {}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).into_owned() +} + +/// `git init` plus the identity every commit needs, with the ambient +/// user's own config kept out of it. +fn init_repo(root: &Path) { + git(root, &["init", "-q", "-b", "main", "."]); + git(root, &["config", "user.email", "gate@example.invalid"]); + git(root, &["config", "user.name", "Gate"]); + // A developer's `commit.gpgsign = true` would make every fixture + // commit prompt or fail; a repo-local `false` outranks it. + git(root, &["config", "commit.gpgsign", "false"]); +} + +fn write(root: &Path, rel: &str, body: &str) { + let p = root.join(rel); + if let Some(parent) = p.parent() { + std::fs::create_dir_all(parent).expect("mkdir -p"); + } + std::fs::write(&p, body).expect("write fixture file"); +} + +/// A repository with **one row of every class `d` must answer**: +/// staged, unstaged, both, deleted, renamed, and untracked. Plus a +/// `Cargo.toml`, so `pmacs.project.detect` reports `rust` and NOT +/// `git` — the case a `kind == "git"` gate would have failed, and the +/// reason this module never asks pmacs whether something is a repo. +fn mixed_repo(root: &Path) { + init_repo(root); + write(root, "Cargo.toml", "[package]\nname = \"fixture\"\n"); + // Two tracked files that stay CLEAN, so they are absent from status + // until a test dirties them. `g6_11` needs to insert **two** rows + // above its selection: inserting one is satisfied by the accidental + // off-by-one that a missing re-seat produces, which is how that test + // was vacuous in its first form. + write(root, "a1.txt", "a1 base\n"); + write(root, "a2.txt", "a2 base\n"); + write(root, "staged.txt", "staged base\n"); + write(root, "unstaged.txt", "unstaged base\n"); + write(root, "both.txt", "both base\n"); + write(root, "deleted.txt", "deleted base\n"); + // Long enough that rename detection scores it at 100%. + write( + root, + "renamed_from.txt", + "a line of content long enough for rename detection to score it\n", + ); + git(root, &["add", "-A"]); + git(root, &["commit", "-qm", "init"]); + + write(root, "staged.txt", "staged base\nstaged edit\n"); + git(root, &["add", "staged.txt"]); + write(root, "unstaged.txt", "unstaged base\nworktree edit\n"); + write(root, "both.txt", "both base\nstaged edit\n"); + git(root, &["add", "both.txt"]); + write(root, "both.txt", "both base\nstaged edit\nworktree edit\n"); + std::fs::remove_file(root.join("deleted.txt")).expect("rm deleted.txt"); + git(root, &["mv", "renamed_from.txt", "renamed_to.txt"]); + write(root, "untracked.txt", "untracked body\n"); +} + +/// A repository holding **a real rename and a real copy**: `orig.txt` +/// is `git mv`d to `moved.txt`, and `copy_src.txt` is copied to a +/// staged `copy_dst.txt`. +/// +/// Both are real on disk, and that is what the copy fixture is FOR: +/// **with this fixture's source left unchanged**, git classifies the +/// copy as an ordinary `1 A.` add (measured in `g6_4b`; not a claim +/// about git in general), so the `2 C.` row has to be supplied — but +/// because both of its paths exist here, the two-path diff that row +/// drives is a real invocation rendering a real patch. +fn rename_and_copy_repo(root: &Path) { + init_repo(root); + write(root, "Cargo.toml", "[package]\nname = \"fixture\"\n"); + write( + root, + "orig.txt", + "a line of content long enough for rename detection to score it\n", + ); + write( + root, + "copy_src.txt", + "a line of content long enough for copy detection to score it\n", + ); + git(root, &["add", "-A"]); + git(root, &["commit", "-qm", "init"]); + git(root, &["mv", "orig.txt", "moved.txt"]); + std::fs::copy(root.join("copy_src.txt"), root.join("copy_dst.txt")).expect("cp"); + git(root, &["add", "copy_dst.txt"]); +} + +/// The unborn fixture, enumerated from a real unborn repository rather +/// than reasoned about: `git init`, stage three files, then edit one +/// (`AM`), delete one (`AD`), and `git mv` one — which produces an +/// ordinary `1 A.` add of the new path, never a `2` record. +fn unborn_repo(root: &Path) { + init_repo(root); + write(root, "am.txt", "am base\n"); + write(root, "ad.txt", "ad base\n"); + write(root, "r_orig.txt", "r base\n"); + git(root, &["add", "-A"]); + write(root, "am.txt", "am base\nworktree edit\n"); + std::fs::remove_file(root.join("ad.txt")).expect("rm ad.txt"); + git(root, &["mv", "r_orig.txt", "r_new.txt"]); + write(root, "untracked.txt", "untracked body\n"); +} + +/// Bound project detection to the fixture, open a file inside it so the +/// active buffer has a path there, and run `M-x git.status` to +/// completion. +/// +/// `set_search_boundary` is the R8 discipline: without it, detection +/// walks out of the tempdir and picks up whatever markers the developer +/// happens to have above `/tmp`. +fn open_panel(s: &mut EditorState, root: &Path, seed_file: &str) { + let root_str = root.display().to_string(); + let seed = root.join(seed_file).display().to_string(); + exec( + s, + &format!( + "pmacs.project.set_search_boundary({root_str:?})\n\ + pmacs.buffer.find_or_open({seed:?})" + ), + ); + exec(s, "pmacs.git.status()"); + assert!( + pump_until(s, 15_000, |s| !panel_text(s).is_empty()), + "the status panel must render; status was {:?}", + status(s) + ); +} + +/// The 1-based data line of the first panel row whose text contains +/// `needle`, or `None`. +fn row_line(s: &EditorState, needle: &str) -> Option { + panel_text(s) + .lines() + .enumerate() + .find(|(i, line)| *i > 0 && line.contains(needle)) + .map(|(i, _)| i) +} + +/// Seat the cursor on the row containing `needle` by pressing `n`, +/// which is listview's own binding — so a broken panel keymap fails +/// here rather than being stepped around. +fn seat_on(s: &mut EditorState, needle: &str) { + let target = row_line(s, needle) + .unwrap_or_else(|| panic!("no row matching {needle:?} in:\n{}", panel_text(s))); + let current: i64 = eval(s, "return pmacs.editor.cursor_line()"); + let current = usize::try_from(current).expect("cursor line fits"); + assert!( + current <= target, + "fixture: expected to walk down to {needle:?} (line {target}, cursor {current})" + ); + for _ in current..target { + press(s, KeyCode::Char('n')); + } + let now: i64 = eval(s, "return pmacs.editor.cursor_line()"); + assert_eq!( + usize::try_from(now).expect("cursor line fits"), + target, + "cursor must land on the {needle:?} row" + ); +} + +/// Pump frames for `ms` without waiting on anything — for the +/// assertions whose subject is that NOTHING happens. +fn pump_for(s: &mut EditorState, ms: u64) { + pump_until(s, ms, |_| false); +} + +/// Press `d` and pump until `*git-diff*` names `path_fragment`. +fn press_d_and_wait(s: &mut EditorState, path_fragment: &str) -> String { + let before = diff_text(s); + press(s, KeyCode::Char('d')); + assert!( + pump_until(s, 15_000, |s| { + let now = diff_text(s); + now != before && now.contains(path_fragment) + }), + "d must render a diff naming {path_fragment:?}; buffer was:\n{}\nstatus: {:?}", + diff_text(s), + status(s) + ); + diff_text(s) +} + +/// The all-zero object id porcelain v2 prints for an absent side of a +/// change. Module scope because clippy refuses an item after statements. +const H: &str = "0000000000000000000000000000000000000000"; + +/// A Lua EXPRESSION producing a `-z` status payload from `fields`. +/// +/// Deliberately assembled in Lua with `string.char(0)` rather than as a +/// Rust string interpolated with `{:?}`. Rust's `Debug` renders a NUL +/// as `\0`, and Lua's decimal escape then swallows the digits that +/// follow — so a `\0` immediately before a `1` record becomes +/// `string.char(1)` and that record silently merges into its +/// predecessor. The first draft of this suite did exactly that, and one +/// of its tests PASSED while parsing nothing at all: the merged text +/// landed in the `# branch.head` value, which the panel header then +/// rendered, and a `contains` assertion on the panel was satisfied by +/// the header. +fn z_payload(fields: &[&str]) -> String { + let quoted: Vec = fields.iter().map(|f| format!("{f:?}")).collect(); + format!( + "(table.concat({{ {} }}, string.char(0)) .. string.char(0))", + quoted.join(", ") + ) +} + +/// A path whose bytes are a legal POSIX filename but **not** valid +/// UTF-8 — the Q#G-8 subject. `0xFF` can begin no UTF-8 sequence at all. +const BAD_PATH: &[u8] = b"bad\xFF.txt"; + +/// A second one, for the ORIGIN side of a rename. +const BAD_ORIG: &[u8] = b"was\xFE.txt"; + +/// A Lua string LITERAL for the raw bytes `b`. +/// +/// Every byte outside printable ASCII is spelled as a **three-digit** +/// decimal escape, never fewer digits. Lua's decimal escape consumes up +/// to three digits, so a shorter one silently absorbs whatever digit +/// follows it — `"\0"` written before a `1` becomes the single byte +/// `0x01`. That is the exact swallowing `z_payload` documents above, and +/// a fixed width removes it rather than working around it. +/// +/// This exists because the paths Q#G-8 is about **cannot be spelled as a +/// Rust `&str` at all**: `bad\xFF.txt` is not UTF-8, so the payload has +/// to be assembled from bytes rather than from `&str` fields. +fn lua_bytes(b: &[u8]) -> String { + use std::fmt::Write as _; + + let mut out = String::from("\""); + for &byte in b { + match byte { + b'"' => out.push_str("\\\""), + b'\\' => out.push_str("\\\\"), + 0x20..=0x7E => out.push(char::from(byte)), + _ => write!(out, "\\{byte:03}").expect("writing to a String cannot fail"), + } + } + out.push('"'); + out +} + +/// `z_payload`'s counterpart for rows whose paths are **raw bytes**: the +/// `-z` payload is assembled here as bytes, NUL terminators and all, +/// exactly the way `git status --porcelain=v2 -z` writes them, and +/// handed to Lua as one literal. +fn z_payload_bytes(fields: &[&[u8]]) -> String { + let mut raw = Vec::new(); + for field in fields { + raw.extend_from_slice(field); + raw.push(0); + } + lua_bytes(&raw) +} + +/// One porcelain field: an ASCII `prefix` followed by raw `path` bytes. +fn field(prefix: &str, path: &[u8]) -> Vec { + let mut out = prefix.as_bytes().to_vec(); + out.extend_from_slice(path); + out +} + +fn tempdir() -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().expect("tempdir"); + // Canonicalize: `git rev-parse --show-toplevel` reports a physical + // path, and `/tmp` is a symlink on some machines, so an + // uncanonicalized fixture root would not compare equal to the root + // the module resolves. + let root = std::fs::canonicalize(dir.path()).expect("canonicalize tempdir"); + (dir, root) +} + +// --------------------------------------------------------------------------- +// §6 — parsing, against a corpus rather than a case (Q#G-6) +// --------------------------------------------------------------------------- + +/// The pure parser, with **no repository**, over every row class the +/// framing's witness corpus names: modified, added, deleted, untracked, +/// renamed with BOTH paths, copied, a path with a space, and a path +/// with a **newline**. +/// +/// The newline case is what `-z` buys. A parser that passes only the +/// space case is the one that ships broken: without `-z` git C-quotes +/// such a path, and a hand-written unquoter is what this design exists +/// to avoid. +#[test] +fn g6_1_the_parser_covers_the_whole_v2_row_corpus() { + let s = editor(); + // Field layouts taken from real `git status --porcelain=v2 -z` + // output, not from the documentation: + // 1 + // 2 \0 + let fields = [ + "# branch.oid 16fa4d708a09af0c96212f66395c3e204049534a".to_string(), + "# branch.head main".to_string(), + format!("1 .M N... 100644 100644 100644 {H} {H} modified.txt"), + format!("1 A. N... 000000 100644 100644 {H} {H} added.txt"), + format!("1 .D N... 100644 100644 000000 {H} {H} deleted.txt"), + format!("2 R. N... 100644 100644 100644 {H} {H} R100 renamed_to.txt"), + "renamed_from.txt".to_string(), + format!("2 C. N... 100644 100644 100644 {H} {H} C085 copy_dst.txt"), + "copy_src.txt".to_string(), + format!("1 .M N... 100644 100644 100644 {H} {H} has space.txt"), + format!("1 .M N... 100644 100644 100644 {H} {H} new\nline.txt"), + "? untracked.txt".to_string(), + ]; + let refs: Vec<&str> = fields.iter().map(String::as_str).collect(); + + exec( + &s, + &format!("_G.PARSED = pmacs.git.parse_status({})", z_payload(&refs)), + ); + + // Rows are joined with RS (0x1E), not a newline: one of the paths + // in this corpus CONTAINS a newline, and splitting on `\n` would + // tear that very row in half — turning the case `-z` exists for + // into a test artefact. + let encoded: String = eval( + &s, + "local out = {}\n\ + for _, r in ipairs(_G.PARSED.rows) do\n\ + out[#out + 1] = table.concat({ r.kind, r.xy, r.path, r.orig or '-' }, '|')\n\ + end\n\ + return table.concat(out, string.char(30))", + ); + let rows: Vec<&str> = encoded.split('\u{1e}').collect(); + + assert_eq!( + rows, + vec![ + "ordinary|.M|modified.txt|-", + "ordinary|A.|added.txt|-", + "ordinary|.D|deleted.txt|-", + "rename|R.|renamed_to.txt|renamed_from.txt", + "rename|C.|copy_dst.txt|copy_src.txt", + "ordinary|.M|has space.txt|-", + "ordinary|.M|new\nline.txt|-", + "untracked|??|untracked.txt|-", + ], + "every corpus row must parse, and a rename/copy must carry BOTH \ + paths — the origin is the NEXT NUL-terminated field under -z, \ + not a tab-joined suffix" + ); + + let head: String = eval(&s, "return _G.PARSED.branch.head"); + assert_eq!(head, "main", "the --branch header is parsed too"); +} + +/// A path with a newline must not be able to split a panel row. +/// +/// This rides alongside the parse assertion above rather than instead +/// of it: parsing the byte correctly and then writing it raw into a +/// one-row-per-line buffer desynchronizes every line-to-row mapping in +/// the panel, which no parser test can see. +#[test] +fn g6_1b_a_newline_in_a_path_is_escaped_for_display() { + let s = editor(); + let shown: String = eval(&s, "return pmacs.git.display_path('new\\nline.txt')"); + assert_eq!(shown, "new\\x0Aline.txt"); + assert!( + !shown.contains('\n'), + "a rendered row must occupy exactly one line" + ); +} + +/// Unborn detection reads `# branch.oid (initial)` from the status +/// output already being parsed. Pinned so nobody later reintroduces a +/// second `rev-parse` process for a fact the first one hands over. +#[test] +fn g6_7_unborn_is_read_from_the_branch_oid_header() { + let s = editor(); + let unborn_payload = z_payload(&["# branch.oid (initial)", "# branch.head main"]); + let born_payload = z_payload(&["# branch.oid deadbeef", "# branch.head main"]); + let (unborn, born): (bool, bool) = eval( + &s, + &format!( + "return pmacs.git.parse_status({unborn_payload}).branch.unborn,\n\ + pmacs.git.parse_status({born_payload}).branch.unborn" + ), + ); + assert!(unborn, "`(initial)` is the unborn marker"); + assert!(!born, "a real oid is not"); +} + +// --------------------------------------------------------------------------- +// §6 — the non-UTF-8 boundary (Q#G-8) +// --------------------------------------------------------------------------- +// +// THE CLAIM, unchanged from the framing: a non-UTF-8 path is PARSED and +// DISPLAYED — so the user is not lied to about what is modified — and +// RET and `d` REFUSE it with a message, because `pmacs.process.spawn` +// takes `args: Vec` and `pmacs.buffer.find_or_open` takes +// `path: String`, both Rust `String` and so UTF-8 by construction. The +// witness is parse-and-display PLUS a witnessed refusal — never a stack +// trace and never a silent no-op. +// +// THE SPLIT, and why it is where it is. The first form of this coverage +// was one test that built `bad\xFF.txt` ON DISK with `std::fs::write`. +// It was green on Linux and red on BOTH macOS legs of the matrix: +// APFS/HFS+ validate pathnames as UTF-8 and reject that name at the +// syscall with errno 92, EILSEQ, "Illegal byte sequence". Linux's VFS +// treats a filename as opaque bytes and does not. +// +// So the coverage is split along the line the PLATFORM draws, not along +// a `#[cfg]` that would make the behaviour stop being tested somewhere: +// +// g6_2 parse + display, driven from the PAYLOAD BYTES directly. +// No repository, no filesystem, runs everywhere. This is the +// larger half of the claim, and it never needed a file: git +// hands this module bytes and `parse_status` takes a string. +// g6_2b the gestures refusing, over a REAL repository the platform +// can actually create, with the unrepresentable row delivered +// through `_deliver_status` — the seam g6_17 and g6_21 already +// use, for the same reason: it is the only way to put a chosen +// completion in front of the panel. Runs everywhere. +// g6_2c the one thing a payload cannot witness — that real `git` +// emits these bytes at all. LINUX-ONLY and loudly so; see its +// own comment for exactly what is not covered on macOS and why +// nothing there could cover it. + +/// A non-UTF-8 path is **parsed** and **displayed**, from the payload +/// bytes, with no repository and no filesystem. +/// +/// Portable by construction, which is the point: the bytes git would +/// emit are supplied directly, so the assertion does not depend on a +/// host filesystem being willing to hold such a name. Both sides of a +/// rename are covered, because `d` passes the ORIGIN to `git diff` as an +/// argument too and it crosses the same `Vec` boundary. +#[test] +fn g6_2_a_non_utf8_path_parses_and_displays() { + let s = editor(); + let fields: Vec> = vec![ + b"# branch.oid deadbeef".to_vec(), + b"# branch.head main".to_vec(), + field( + &format!("1 .M N... 100644 100644 100644 {H} {H} "), + BAD_PATH, + ), + field( + &format!("2 R. N... 100644 100644 100644 {H} {H} R100 "), + b"kept.txt", + ), + BAD_ORIG.to_vec(), + ]; + let refs: Vec<&[u8]> = fields.iter().map(Vec::as_slice).collect(); + let payload = z_payload_bytes(&refs); + exec( + &s, + &format!("_G.PARSED = pmacs.git.parse_status({payload})"), + ); + + // Parsed: the bytes survive whole. Compared as BYTES in Rust rather + // than as text, because a lossy conversion anywhere along the way + // would turn `0xFF` into U+FFFD and a text comparison would then + // agree with the corruption. + let path: mlua::String = eval(&s, "return _G.PARSED.rows[1].path"); + assert_eq!( + &*path.as_bytes(), + BAD_PATH, + "the parser must hand back the path BYTES, unmodified" + ); + let orig: mlua::String = eval(&s, "return _G.PARSED.rows[2].orig"); + assert_eq!( + &*orig.as_bytes(), + BAD_ORIG, + "…including a rename's origin, which is the NEXT -z field" + ); + + // Classified: this is the fact both refusals are built on, so it is + // asserted rather than assumed, on each side of the rename. + let (bad, orig_bad, kept_ok): (bool, bool, bool) = eval( + &s, + "return pmacs.git.is_text(_G.PARSED.rows[1].path),\n\ + pmacs.git.is_text(_G.PARSED.rows[2].orig),\n\ + pmacs.git.is_text(_G.PARSED.rows[2].path)", + ); + assert!(!bad, "`bad\\xFF.txt` cannot cross the binding boundary"); + assert!(!orig_bad, "nor can the rename's origin"); + assert!(kept_ok, "…while its representable current path can"); + + // Displayed: escaped, one line, and no raw byte in the result — the + // rope is UTF-8 by project invariant, so an unescaped byte here is + // not a cosmetic defect but a value that cannot be stored at all. + let shown: mlua::String = eval(&s, "return pmacs.git.display_path(_G.PARSED.rows[1].path)"); + assert_eq!( + &*shown.as_bytes(), + b"bad\\xFF.txt", + "the unrepresentable byte is displayed as an escape" + ); + assert!( + shown.as_bytes().iter().all(u8::is_ascii), + "and nothing raw survives into what gets rendered" + ); +} + +/// RET and `d` **refuse** an unrepresentable row, each with a message, +/// over a **real repository** — and an ordinary neighbour still works. +/// +/// The repository, the panel, the keymap and the dispatch are all real; +/// the only thing supplied rather than observed is the ROW BYTES, which +/// arrive through `_deliver_status` at the current generation. That seam +/// is not a shortcut invented here: `g6_17` and `g6_21` already drive +/// completions through it, because a chosen completion is not otherwise +/// expressible. And it is what makes this test portable — no filesystem +/// anywhere has to hold `bad\xFF.txt` for the refusal to be exercised. +#[test] +fn g6_2b_a_non_utf8_row_refuses_both_gestures_with_a_message() { + let (_dir, root) = tempdir(); + init_repo(&root); + write(&root, "ok.txt", "ok base\n"); + git(&root, &["add", "-A"]); + git(&root, &["commit", "-qm", "init"]); + write(&root, "ok.txt", "ok base\nedit\n"); + + let mut s = editor(); + // Real `git`, real root resolution, and `set_search_boundary` bounds + // detection to the fixture (R8's lesson). + open_panel(&mut s, &root, "ok.txt"); + + // Rows in this order deliberately: `seat_on` only ever walks DOWN, + // and the representable control has to be reachable after both + // refusals. + let fields: Vec> = vec![ + b"# branch.oid deadbeef".to_vec(), + b"# branch.head main".to_vec(), + field( + &format!("1 .M N... 100644 100644 100644 {H} {H} "), + BAD_PATH, + ), + field( + &format!("2 R. N... 100644 100644 100644 {H} {H} R100 "), + b"kept.txt", + ), + BAD_ORIG.to_vec(), + field( + &format!("1 .M N... 100644 100644 100644 {H} {H} "), + b"ok.txt", + ), + ]; + let refs: Vec<&[u8]> = fields.iter().map(Vec::as_slice).collect(); + let payload = z_payload_bytes(&refs); + exec( + &s, + &format!( + "pmacs.git._deliver_status(\n\ + {{ generation = pmacs.git._generation(),\n\ + dest = pmacs.window.capture_destination() }},\n\ + {{ ok = true, code = 0, stdout = {payload}, stderr = '' }})" + ), + ); + + // Displayed: the row is there, escaped, so the user is not lied to + // about what is modified. + let text = panel_text(&s); + assert!( + text.contains("bad\\xFF.txt"), + "the unrepresentable path must still appear as a row: {text}" + ); + + // RET refuses. + seat_on(&mut s, "bad\\xFF.txt"); + let before = active_name(&s); + press(&mut s, KeyCode::Enter); + assert!( + status(&s).contains("not valid UTF-8"), + "RET must report the refusal; status was {:?}", + status(&s) + ); + assert_eq!( + active_name(&s), + before, + "and must not have navigated anywhere" + ); + + // `d` refuses too, and renders no diff. + exec(&s, "pmacs.editor.set_status('')"); + press(&mut s, KeyCode::Char('d')); + assert!( + status(&s).contains("not valid UTF-8"), + "d must report the refusal; status was {:?}", + status(&s) + ); + pump_for(&mut s, 300); + assert!( + diff_text(&s).is_empty(), + "no *git-diff* buffer may appear for an unrepresentable path: {:?}", + diff_text(&s) + ); + + // The ORIGIN side of a rename refuses as well. `d` on a rename runs + // `git diff HEAD -- `, so the origin crosses the same + // `Vec` boundary the current path does — and a row whose + // current path is perfectly representable is exactly where a check + // written on `row.path` alone would let it through. + exec(&s, "pmacs.editor.set_status('')"); + seat_on(&mut s, "kept.txt"); + press(&mut s, KeyCode::Char('d')); + assert!( + status(&s).contains("not valid UTF-8"), + "d must refuse a rename whose ORIGIN is unrepresentable; status was {:?}", + status(&s) + ); + pump_for(&mut s, 300); + assert!( + diff_text(&s).is_empty(), + "and render no diff for it: {:?}", + diff_text(&s) + ); + + // The positive controls: an ordinary neighbour in the same panel + // still visits and still diffs, so the refusals above are about the + // path and not about a panel that stopped working. + seat_on(&mut s, "ok.txt"); + press(&mut s, KeyCode::Enter); + assert_eq!( + active_name(&s), + root.join("ok.txt").display().to_string(), + "RET really does navigate when the path is representable; \ + status was {:?}", + status(&s) + ); + refocus_panel(&mut s); + seat_on(&mut s, "ok.txt"); + let diff = press_d_and_wait(&mut s, "ok.txt"); + assert!(diff.contains("diff --git"), "control diff: {diff}"); +} + +/// Real `git` really does emit **raw non-UTF-8 path bytes**, and they +/// reach `parse_status` and the panel unmangled. +/// +/// **LINUX-ONLY, and this gate is the whole point of the split above.** +/// +/// WHAT IS NOT COVERED WHERE, AND WHY NOTHING COULD COVER IT THERE. +/// This is the one part of Q#G-8 that genuinely requires a non-UTF-8 +/// filename to exist on disk, and macOS cannot host one: APFS and HFS+ +/// validate pathnames as UTF-8 and fail the syscall with errno 92, +/// EILSEQ. Nor can the fixture be built around the filesystem by putting +/// the name only in the index (`update-index --index-info` plus +/// `write-tree`): `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, and the test would assert a different thing while +/// looking the same. There is therefore no macOS arrangement in which +/// real `git status` names a non-UTF-8 path at all. +/// +/// What this leaves uncovered on macOS is exactly the PROVENANCE of the +/// bytes — that git emits them — and nothing else. The BEHAVIOUR built +/// on them (parse, display, refuse) is covered on every platform by +/// `g6_2` and `g6_2b`, which is why this gate is narrow enough to be +/// honest. The remaining link, 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 the chunks. +#[cfg(target_os = "linux")] +#[test] +fn g6_2c_real_git_emits_non_utf8_path_bytes_on_a_filesystem_that_allows_the_name() { + use std::os::unix::ffi::OsStrExt; + + let (_dir, root) = tempdir(); + init_repo(&root); + write(&root, "ok.txt", "ok base\n"); + let bad = root.join(std::ffi::OsStr::from_bytes(BAD_PATH)); + std::fs::write(&bad, b"bad base\n").unwrap_or_else(|e| { + panic!( + "this test is gated to the platforms whose filesystem accepts \ + a non-UTF-8 filename; creating {BAD_PATH:?} failed with: {e}" + ) + }); + git(&root, &["add", "-A"]); + git(&root, &["commit", "-qm", "init"]); + std::fs::write(&bad, b"bad base\nedit\n").expect("edit non-utf8 path"); + + let mut s = editor(); + open_panel(&mut s, &root, "ok.txt"); + + let text = panel_text(&s); + assert!( + text.contains("bad\\xFF.txt"), + "real `git status --porcelain=v2 -z` names the path in raw bytes, \ + and they survive the spawn pipe into the panel: {text}" + ); +} + +// --------------------------------------------------------------------------- +// §6 — the invocation (Q#G-6) +// --------------------------------------------------------------------------- + +/// `--no-optional-locks` is asserted **structurally**, on the argv the +/// module really spawned. A lock that was not taken cannot be observed +/// directly, so the invocation is what gets pinned. +/// +/// The flag is part of the contract, not a nicety: `git status` may +/// refresh and write the index, and this module runs it asynchronously +/// from an editor while the user may be running git in a terminal. +#[test] +fn g6_3_every_invocation_carries_no_optional_locks() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + let log: Vec = eval( + &s, + "local out = {}\n\ + for _, args in ipairs(pmacs.git._spawn_log) do\n\ + out[#out + 1] = table.concat(args, ' ')\n\ + end\n\ + return out", + ); + assert!(!log.is_empty(), "the open must have spawned git"); + for argv in &log { + assert!( + argv.starts_with("--no-optional-locks"), + "every git argv must lead with --no-optional-locks; got {argv:?}" + ); + } + assert!( + log.iter() + .any(|a| a.contains("status --porcelain=v2 --branch -z")), + "the status invocation is the pinned one: {log:?}" + ); + + // And the diff path too, which is a separate call site. + seat_on(&mut s, "staged.txt"); + press_d_and_wait(&mut s, "staged.txt"); + let last: Vec = eval(&s, "return pmacs.git._last_spawn.args"); + assert_eq!( + last.first().map(String::as_str), + Some("--no-optional-locks"), + "the diff argv carries it as well: {last:?}" + ); +} + +// --------------------------------------------------------------------------- +// §6 — `d` on every row class (Q#G-7) +// --------------------------------------------------------------------------- + +/// `d` is witnessed on staged, unstaged, both, deleted, renamed **and +/// untracked** rows. +/// +/// Untracked is the load-bearing one: a normal `git diff` shows nothing +/// at all for an untracked file, so a missing `--no-index` case makes +/// `d` silently dead exactly where a user is most likely to press it. +#[test] +fn g6_4_d_answers_every_row_class() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + // Each row class, with a fragment only the RIGHT diff can contain. + // `both.txt` is asserted on BOTH halves: `git diff HEAD` is one view + // of the total change, so a `--cached`-only or plain-only answer + // would lose one of them and still look plausible. + for (needle, expect) in [ + ("staged.txt", &["+staged edit"][..]), + ("unstaged.txt", &["+worktree edit"][..]), + ("both.txt", &["+staged edit", "+worktree edit"][..]), + ("deleted.txt", &["-deleted base"][..]), + ("renamed_to.txt", &["rename from renamed_from.txt"][..]), + ("untracked.txt", &["+untracked body"][..]), + ] { + // `d` displays the diff in the DOCUMENT window, so focus has to + // come back to the panel before the next gesture. + refocus_panel(&mut s); + seat_on(&mut s, needle); + let diff = press_d_and_wait(&mut s, needle); + for fragment in expect { + assert!( + diff.contains(fragment), + "d on the {needle:?} row must render {fragment:?}; got:\n{diff}" + ); + } + assert!( + !diff.contains("exited with code"), + "…and not a failure body: {diff}" + ); + } +} + +/// Focus the `*git-status*` panel and seat the cursor on its first data +/// row, whatever `d` last displayed. +fn refocus_panel(s: &mut EditorState) { + exec( + s, + "for _, id in ipairs(pmacs.buffer.list()) do\n\ + if pmacs.describe.buffer(id).name == '*git-status*' then\n\ + pmacs.window.display(id, { side = 'bottom', select = true })\n\ + end\n\ + end\n\ + pmacs.editor.move_to_line(1)", + ); + assert_eq!( + active_name(s), + "*git-status*", + "the panel must be focused again before the next gesture" + ); +} + +/// A **copy** row's diff header says *copied*, a **rename** row's says +/// *renamed*, and both run the SAME two-path invocation. +/// +/// **This is a parser/presentation test, not end-to-end copy coverage.** +/// Porcelain v2 folds renames and copies into one `2` record whose +/// `` field leads with `R` or `C`. +/// +/// **Scope of the premise, narrowed after review.** An earlier version +/// of this comment said real `git` "will not emit a `2 C` record — not +/// 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 premise below actually MEASURES is narrower and is all it +/// claims: **for THIS fixture — a copy whose source is left unchanged — +/// git reports `1 A.` and emits no `2 C` record**, under +/// `-c status.renames=copies`. It is measured rather than recalled, but +/// it is a fact about this fixture, not about `git` in general. No +/// mechanism is asserted here for *why* an unchanged source is not +/// offered as a copy candidate; that was not established. +/// +/// So the copy ROW is supplied as payload bytes through +/// `_deliver_status` — the seam `g6_2b`, `g6_17` and `g6_21` already use +/// — because this fixture cannot produce one, which is a weaker and +/// true reason than the one first given. +/// +/// Everything downstream of the row is real: the repository, the panel, +/// the `d` dispatch, the spawned `git diff`, and the rendered buffer. +/// Both crafted rows name paths that EXIST in the fixture, so each one's +/// two-path diff really runs and really renders. +/// +/// BOTH classes are asserted. A header that said "copied" for every `2` +/// record would satisfy the copy half on its own, so the rename half is +/// what makes this a distinction rather than a relabelling. The argv is +/// asserted for both as well: the two-path `git diff HEAD -- +/// ` is correct for a copy and a rename alike, so a fix to what +/// the user is TOLD must not reach what the module DOES. +#[test] +fn g6_4b_a_copy_says_copied_and_a_rename_says_renamed() { + let (_dir, root) = tempdir(); + rename_and_copy_repo(&root); + + let mut s = editor(); + open_panel(&mut s, &root, "copy_src.txt"); + + // The premise, measured — and note what it does and does not say. + // For THIS fixture, whose copy source is left unchanged, git reports + // `1 A.` and emits no `2 C` record even when asked for copy + // detection explicitly. That is a fact about this fixture. `git` + // DOES emit `C` in general — `git-status(1)` documents it as + // "copied (if config option status.renames is set to `copies`)" — + // so this assertion must not be read as proving otherwise. + // Pinned so a future reader can see WHY the row below is crafted, + // instead of taking it on trust. + let raw = git( + &root, + &[ + "--no-optional-locks", + "-c", + "status.renames=copies", + "status", + "--porcelain=v2", + "--branch", + "-z", + ], + ); + assert!( + !raw.contains("2 C"), + "fixture premise: for THIS fixture (copy source unchanged), git \ + emits no `2 C` record even under status.renames=copies. This is \ + a claim about the fixture, not about git in general — git does \ + document `C` as copied under that setting. It emitted:\n{raw:?}" + ); + assert!( + panel_text(&s).contains("A. copy_dst.txt"), + "…and the panel shows the real run's ordinary ADD: {}", + panel_text(&s) + ); + + // Now the crafted pair, delivered at the current generation. + let fields = [ + "# branch.oid 16fa4d708a09af0c96212f66395c3e204049534a".to_string(), + "# branch.head main".to_string(), + format!("2 R. N... 100644 100644 100644 {H} {H} R100 moved.txt"), + "orig.txt".to_string(), + format!("2 C. N... 100644 100644 100644 {H} {H} C100 copy_dst.txt"), + "copy_src.txt".to_string(), + ]; + let refs: Vec<&str> = fields.iter().map(String::as_str).collect(); + exec( + &s, + &format!( + "pmacs.git._deliver_status(\n\ + {{ generation = pmacs.git._generation(),\n\ + dest = pmacs.window.capture_destination() }},\n\ + {{ ok = true, code = 0, stdout = {}, stderr = '' }})", + z_payload(&refs) + ), + ); + + // The ROW rendering is deliberately the same for both, because the + // `XY` prefix ALREADY tells them apart — `R.` against `C.`, out of + // the same byte the score leads with, in the porcelain vocabulary + // every other row in the panel is read in. Pinned so the decision + // not to widen row rendering is a checked claim rather than a note. + let text = panel_text(&s); + assert!( + text.contains("R. moved.txt <- orig.txt"), + "the rename row keeps its `R.` prefix and both paths: {text}" + ); + assert!( + text.contains("C. copy_dst.txt <- copy_src.txt"), + "and the copy row is distinguished by its `C.` prefix: {text}" + ); + + assert_two_path_diff(&mut s, &root, "moved.txt", "orig.txt", "renamed", "copied"); + refocus_panel(&mut s); + assert_two_path_diff( + &mut s, + &root, + "copy_dst.txt", + "copy_src.txt", + "copied", + "renamed", + ); +} + +/// Press `d` on the `path` row and assert its header says +/// `" from "` and never `" from"` — over the +/// two-path `git diff HEAD -- `, asserted argv and all. +/// +/// One helper for both classes on purpose: a copy and a rename differ +/// in exactly one word, and everything else about them — the +/// invocation, the patch, the buffer — has to stay identical, which is +/// easiest to keep honest when the same code asserts it twice. +fn assert_two_path_diff( + s: &mut EditorState, + root: &Path, + path: &str, + orig: &str, + said: &str, + not_said: &str, +) { + seat_on(s, path); + let diff = press_d_and_wait(s, path); + assert!( + diff.contains(&format!("against HEAD ({said} from {orig})")), + "the {path:?} row's header must say {said:?} of {orig:?} — a copy \ + left its origin where it was, and saying \"renamed\" of it states \ + a different fact about the user's tree: {diff}" + ); + assert!( + !diff.contains(&format!("{not_said} from")), + "…and must never say {not_said:?}: {diff}" + ); + assert!( + !diff.contains("exited with code"), + "…over a diff that really ran: {diff}" + ); + + let last: Vec = eval(s, "return pmacs.git._last_spawn.args"); + let want: Vec = [ + "--no-optional-locks", + "-C", + &root.display().to_string(), + "diff", + "--no-color", + "HEAD", + "--", + orig, + path, + ] + .iter() + .map(|a| (*a).to_string()) + .collect(); + assert_eq!( + last, want, + "and the two-path invocation is the SAME for both classes — \ + passing both paths is what makes git render the relationship \ + rather than an unrelated add, so a fix to what the user is TOLD \ + must not reach what the module DOES" + ); +} + +/// The untracked diff renders **on exit 1**, not a failure row. +/// +/// `git diff --no-index` implies `--exit-code`: it exits 1 when it +/// SUCCESSFULLY finds differences. Under a plain "non-zero is failure" +/// predicate every untracked diff — the case `--no-index` exists to +/// serve — would render a failure instead of the diff it just produced. +/// +/// Measured rather than assumed: the same invocation run by hand +/// against a scratch repository exits 1 and prints the patch. +#[test] +fn g6_5_the_untracked_diff_renders_on_exit_one() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + seat_on(&mut s, "untracked.txt"); + let diff = press_d_and_wait(&mut s, "untracked.txt"); + + // The invocation really was the `--no-index` one… + let last: Vec = eval(&s, "return pmacs.git._last_spawn.args"); + assert!( + last.iter().any(|a| a == "--no-index") && last.iter().any(|a| a == "/dev/null"), + "the untracked row must diff against /dev/null: {last:?}" + ); + // …and the same argv really does exit 1 here, so this test is about + // the predicate rather than about a git that happens to exit 0. + let out = std::process::Command::new("git") + .current_dir(&root) + .args([ + "--no-optional-locks", + "diff", + "--no-index", + "--", + "/dev/null", + "untracked.txt", + ]) + .output() + .expect("run the same invocation by hand"); + assert_eq!( + out.status.code(), + Some(1), + "fixture premise: --no-index exits 1 when it finds differences" + ); + + assert!( + diff.contains("+untracked body"), + "the patch must be rendered: {diff}" + ); + assert!( + !diff.contains("exited with code 1"), + "exit 1 must NOT be read as a failure here: {diff}" + ); +} + +// --------------------------------------------------------------------------- +// §6 — the unborn repository (Q#G-7b) +// --------------------------------------------------------------------------- + +/// An unborn repository, end to end, with an **`AM`** fixture — 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. +#[test] +fn g6_6_an_unborn_am_row_renders_two_labelled_patches() { + let (_dir, root) = tempdir(); + unborn_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "am.txt"); + + let text = panel_text(&s); + assert!( + text.contains("no commits yet"), + "the header names the unborn state: {text}" + ); + assert!(text.contains("AM am.txt"), "the AM row is present: {text}"); + + seat_on(&mut s, "am.txt"); + let diff = press_d_and_wait(&mut s, "am.txt"); + + assert!( + !diff.contains("bad revision"), + "`git diff HEAD` must never be attempted here: {diff}" + ); + assert!( + diff.contains( + "no commits yet --- split view: staged (index) above, unstaged (worktree) below" + ), + "the header must describe a SPLIT view, not a total against HEAD: {diff}" + ); + assert!( + diff.contains("=== staged (index) ===") && diff.contains("=== unstaged (worktree) ==="), + "two labelled patches: {diff}" + ); + assert!( + diff.contains("+am base"), + "the staged half carries the index patch: {diff}" + ); + assert!( + diff.contains("+worktree edit"), + "the unstaged half carries the worktree delta — the half a \ + --cached-only answer silently drops: {diff}" + ); +} + +/// `AD` rides the same fixture, since one repository can hold both, and +/// its second patch renders the deletion. +#[test] +fn g6_6b_an_unborn_ad_row_renders_the_deletion_in_its_second_patch() { + let (_dir, root) = tempdir(); + unborn_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "am.txt"); + seat_on(&mut s, "ad.txt"); + let diff = press_d_and_wait(&mut s, "ad.txt"); + + assert!( + diff.contains("=== staged (index) ===") && diff.contains("=== unstaged (worktree) ==="), + "two labelled patches for AD too: {diff}" + ); + assert!( + diff.contains("deleted file mode"), + "the unstaged half renders the deletion: {diff}" + ); +} + +/// A single-state unborn row takes ONE patch, and says which one. +/// +/// Without this the split above could be an unconditional two-command +/// answer that happens to look right, and the `A.` row would carry a +/// header describing a view it is not. +#[test] +fn g6_6c_a_single_state_unborn_row_takes_one_patch_and_says_so() { + let (_dir, root) = tempdir(); + unborn_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "am.txt"); + seat_on(&mut s, "r_new.txt"); + let diff = press_d_and_wait(&mut s, "r_new.txt"); + + assert!( + diff.contains("no commits yet --- staged (index) only"), + "the header must name the single state it shows: {diff}" + ); + assert!( + !diff.contains("=== unstaged (worktree) ==="), + "and there is no second patch to label: {diff}" + ); +} + +/// A two-step plan runs **every** step against the repository the user +/// was looking at when they pressed `d`. +/// +/// An unborn `AM` row is the only shape that makes this observable: it +/// produces a **two-step** plan (staged patch, then unstaged patch), and +/// the second step used to be spawned from the first one's completion +/// callback against whatever `state.root` held **by then**. So a +/// `git.status` for another repository, whose root lookup lands while +/// the first patch is still in flight, moved the plan's second step into +/// a different repository — carrying the first repository's path, which +/// git there resolves to nothing at all. +/// +/// Driven through `_deliver_root` for the same reason `g6_21` is: no +/// arrangement of real subprocess timing can guarantee that the +/// interleaving happens, and a test that merely hoped for it would pass +/// on the broken code most of the time. Nothing is pumped between the +/// keypress and the reassignment, so step 1 is genuinely in flight. +/// +/// The argv assertion is the load-bearing one. A test that checked only +/// the FIRST step — or only that a diff rendered — passes on the broken +/// code, since step 1 is spawned synchronously from the keypress and +/// step 2 against the wrong repository merely produces an empty patch. +#[test] +fn g6_22_a_two_step_plan_keeps_the_root_it_started_with() { + let (_dir_a, root_a) = tempdir(); + unborn_repo(&root_a); + // An unrelated repository, with no `am.txt` in it: a step that + // escaped into B would find nothing and render "(no changes)". + let (_dir_b, root_b) = tempdir(); + mixed_repo(&root_b); + + let mut s = editor(); + open_panel(&mut s, &root_a, "am.txt"); + seat_on(&mut s, "am.txt"); + + let a = root_a.display().to_string(); + let b = root_b.display().to_string(); + + // `d` spawns step 1 synchronously against A… + press(&mut s, KeyCode::Char('d')); + // …and now, before a single frame is pumped, a `git.status` for B + // resolves its root and reassigns `state.root`. + exec( + &s, + &format!( + "pmacs.git._deliver_root(\n\ + {{ generation = pmacs.git._generation(), dir = {b:?},\n\ + dest = pmacs.window.capture_destination() }},\n\ + {{ ok = true, code = 0, stdout = {b:?}, stderr = '' }})" + ), + ); + assert!( + pump_until(&mut s, 15_000, |s| diff_text(s) + .contains("=== unstaged (worktree) ===")), + "the plan must run to completion; diff was:\n{}\nstatus: {:?}", + diff_text(&s), + status(&s) + ); + + let diffs: Vec = eval( + &s, + "local out = {}\n\ + for _, args in ipairs(pmacs.git._spawn_log) do\n\ + for _, a in ipairs(args) do\n\ + if a == 'diff' then\n\ + out[#out + 1] = table.concat(args, ' ')\n\ + break\n\ + end\n\ + end\n\ + end\n\ + return out", + ); + assert_eq!( + diffs.len(), + 2, + "premise: the AM row's plan really is two steps: {diffs:?}" + ); + for argv in &diffs { + assert!( + argv.contains(&format!("-C {a}")), + "every step of one plan runs against the captured root: {argv:?}" + ); + assert!( + !argv.contains(&b), + "and none of them may follow `state.root` into another \ + repository: {argv:?}" + ); + } + + // The user-visible half: the second patch is still A's worktree + // delta, not the empty answer B would have given. + let diff = diff_text(&s); + assert!( + diff.contains("+worktree edit"), + "the unstaged half must still carry A's worktree delta: {diff}" + ); +} + +/// Rename/copy under an unborn `HEAD` is **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. +#[test] +fn g6_8_rename_under_an_unborn_head_is_unreachable() { + let (_dir, root) = tempdir(); + unborn_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "am.txt"); + + let text = panel_text(&s); + assert!( + text.contains("A. r_new.txt"), + "the `git mv` produces an ordinary ADD of the new path: {text}" + ); + assert!( + !text.contains("<-"), + "and no rename row exists at all — with no HEAD there is nothing \ + to rename FROM: {text}" + ); + // Asserted against the REAL status output too, so a display-only + // change cannot make this pass. + let raw = git( + &root, + &[ + "--no-optional-locks", + "status", + "--porcelain=v2", + "--branch", + "-z", + ], + ); + exec(&s, &format!("_G.RAW = {raw:?}")); + let has_rename: bool = eval( + &s, + "for _, r in ipairs(pmacs.git.parse_status(_G.RAW).rows) do\n\ + if r.kind == 'rename' then return true end\n\ + end\n\ + return false", + ); + assert!( + !has_rename, + "the real unborn status output contains no `2` record" + ); +} + +// --------------------------------------------------------------------------- +// §6 — the `keys` lifecycle (Q#G-7) +// --------------------------------------------------------------------------- + +/// Two successive refreshes on a live panel: `d` still works and no +/// `DuplicateBinding` surfaces. +/// +/// This is the one that would have broken on **every** refresh. +/// `Keymap::bind` refuses duplicates and the completion model calls +/// `listview.open` again each time, so a naive `keys` implementation +/// errors on the second open. +#[test] +fn g6_9_two_refreshes_keep_d_working_and_raise_nothing() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + for round in 1..=2 { + let before: i64 = eval(&s, "return pmacs.git._generation()"); + press(&mut s, KeyCode::Char('g')); + assert!( + pump_until(&mut s, 15_000, |s| !panel_text(s).contains("refreshing")), + "refresh {round} must complete; panel:\n{}", + panel_text(&s) + ); + let after: i64 = eval(&s, "return pmacs.git._generation()"); + assert_eq!(after, before + 1, "refresh {round} really ran"); + assert!( + !status(&s).contains("already bound"), + "refresh {round} status: {:?}", + status(&s) + ); + } + + let errs = errors_text(&s); + assert!( + !errs.contains("already bound"), + "no DuplicateBinding may reach the errors buffer:\n{errs}" + ); + + // And `d` is still a live binding, not merely un-erroring. + seat_on(&mut s, "staged.txt"); + let diff = press_d_and_wait(&mut s, "staged.txt"); + assert!( + diff.contains("+staged edit"), + "d after two refreshes: {diff}" + ); +} + +/// A `keys` table colliding with the fixed set is rejected **at install +/// time**, as is a prefix conflict — in either direction. +#[test] +fn g6_10_a_colliding_or_prefixing_keys_table_is_rejected() { + let s = editor(); + + let fixed: String = eval( + &s, + "local ok, e = pcall(pmacs.listview.open, {\n\ + name = '*keys-a*', rows = {}, keys = { g = 'git.status' } })\n\ + return tostring(e)", + ); + assert!( + fixed.contains("own key surface"), + "rebinding `g` must be refused by name: {fixed}" + ); + + let extends: String = eval( + &s, + "local ok, e = pcall(pmacs.listview.open, {\n\ + name = '*keys-b*', rows = {}, keys = { ['g x'] = 'git.status' } })\n\ + return tostring(e)", + ); + assert!( + extends.contains("prefix"), + "`g x` would turn the `g` leaf into a submap: {extends}" + ); + + let internal: String = eval( + &s, + "local ok, e = pcall(pmacs.listview.open, {\n\ + name = '*keys-c*', rows = {},\n\ + keys = { ['d'] = 'git.status', ['d x'] = 'git.diff-file' } })\n\ + return tostring(e)", + ); + assert!( + internal.contains("prefix"), + "two `keys` entries may not prefix each other: {internal}" + ); + + // Nothing was created: a rejected table must leave no half-built + // panel behind, which is why validation runs before `ensure_panel`. + let names: Vec = eval( + &s, + "local out = {}\n\ + for _, id in ipairs(pmacs.buffer.list()) do out[#out+1] = pmacs.describe.buffer(id).name end\n\ + return out", + ); + for name in ["*keys-a*", "*keys-b*", "*keys-c*"] { + assert!( + !names.iter().any(|n| n == name), + "{name} must not exist after a rejected open: {names:?}" + ); + } +} + +/// An ALIAS spelling of a fixed key is rejected too — and rejecting it +/// leaves **no orphan buffer**. +/// +/// The raw-token preflight cannot see these: `parse_key_code` +/// (`src/key.rs`) uppercases and then folds `RET`/`RETURN`/`ENTER`, +/// `SPC`/`SPACE`, `TAB` onto the same `KeyCode`, so `keys = { RETURN = +/// … }` compares unequal to every fixed token and sails through, only +/// for `Keymap::bind` to refuse it later — *after* the buffer has been +/// created, made read-only, marked round-trip and given the fixed +/// keymap, and *before* the panel is registered. The buffer then +/// survives owned by nothing, and the next `open` for that name finds it +/// and silently disambiguates itself to `<2>`. +/// +/// So the error message is the weaker half of this test. Asserting only +/// that would pass on the broken code, because the broken code does +/// raise — it just leaves wreckage behind. The buffer count and the +/// undisambiguated reopen are what actually bite. +#[test] +fn g6_10c_an_alias_spelling_is_rejected_and_leaves_no_orphan_buffer() { + let s = editor(); + let before: i64 = eval(&s, "return #pmacs.buffer.list()"); + + // Every alias the parser folds onto a key the panel already owns. + for alias in ["RETURN", "ENTER", "ret", "enter", "SPACE", "space", "tab"] { + let err: String = eval( + &s, + &format!( + "local ok, e = pcall(pmacs.listview.open, {{\n\ + name = '*alias*', rows = {{}}, keys = {{ [{alias:?}] = 'git.status' }} }})\n\ + return tostring(e)" + ), + ); + assert!( + err.contains("listview:"), + "{alias:?} must be refused by the primitive, with its own \ + message rather than a bare keymap error: {err}" + ); + let now: i64 = eval(&s, "return #pmacs.buffer.list()"); + assert_eq!( + now, before, + "refusing {alias:?} must leave no buffer behind (this is the \ + half that bites: the broken code raises too)" + ); + } + + // …and the name is genuinely still free: a subsequent legitimate + // open gets the plain name, not `*alias*<2>`. + exec( + &s, + "pmacs.listview.open { name = '*alias*', rows = { { text = 'x', item = 1 } },\n\ + keys = { d = 'git.diff-file' } }", + ); + assert_eq!( + active_name(&s), + "*alias*", + "a rejected `keys` table must not have consumed the panel's name" + ); +} + +/// Reopening a live panel with a DIFFERENT `keys` table errors rather +/// than silently keeping the old binding. +/// +/// Silently keeping it would hand the consumer a key that does +/// something other than what it just asked for — a dead or lying key, +/// which is the defect this primitive already condemns for `g`. +#[test] +fn g6_10b_reopening_with_different_keys_errors_instead_of_lying() { + let s = editor(); + exec( + &s, + "pmacs.listview.open { name = '*keys-d*', rows = {}, keys = { d = 'git.status' } }", + ); + let err: String = eval( + &s, + "local ok, e = pcall(pmacs.listview.open, {\n\ + name = '*keys-d*', rows = {}, keys = { d = 'git.diff-file' } })\n\ + return tostring(e)", + ); + assert!( + err.contains("already open with keys"), + "divergence must be reported, not ignored: {err}" + ); + // The same table reopens fine — otherwise the refresh path itself + // would be broken and this rule would be unusable. + exec( + &s, + "pmacs.listview.open { name = '*keys-d*', rows = {}, keys = { d = 'git.status' } }", + ); +} + +/// No new interaction island (COHERENCE.md §6): `d` is bound +/// **buffer-locally** through listview's own binding path, so +/// `describe-key` reports the truth and the key is rebindable from +/// `init.lua` — the observable difference between the keymap idiom and +/// a hardcoded shadow. +#[test] +fn g6_20_d_is_an_ordinary_buffer_local_binding() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + let (command, scope): (String, String) = eval( + &s, + "local info = pmacs.describe.key('d')\n\ + return info.command, info.scope", + ); + assert_eq!(command, "git.diff-file"); + assert!( + scope.starts_with("buffer"), + "`d` must live at BUFFER scope, not global: {scope}" + ); + + // Away from the panel it is not bound at all, which is what "no + // island" means: nothing intercepts `d` anywhere else. + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*plain*'))", + ); + let elsewhere: Option = eval( + &s, + "local info = pmacs.describe.key('d')\n\ + return info and info.command or nil", + ); + assert_ne!( + elsewhere.as_deref(), + Some("git.diff-file"), + "`d` must not be bound outside the panel" + ); +} + +// --------------------------------------------------------------------------- +// §6 — refresh semantics (Q#G-1) +// --------------------------------------------------------------------------- + +/// `g` is **never a no-op**: it re-renders and marks that work started, +/// even mid-flight. A dead `g` is a defect this primitive already names. +#[test] +fn g6_16_g_marks_that_work_started_immediately() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + assert!( + !panel_text(&s).contains("refreshing"), + "precondition: settled" + ); + + // No pumping: the marker must be there the instant `g` returns. + press(&mut s, KeyCode::Char('g')); + assert!( + panel_text(&s).contains("(refreshing...)"), + "g must re-render with a marker before any output arrives:\n{}", + panel_text(&s) + ); + // And again mid-flight, rather than being swallowed as "already + // running". + press(&mut s, KeyCode::Char('g')); + assert!( + panel_text(&s).contains("(refreshing...)"), + "a second g mid-flight still re-renders:\n{}", + panel_text(&s) + ); + + assert!( + pump_until(&mut s, 15_000, |s| !panel_text(s).contains("refreshing")), + "both refreshes settle" + ); +} + +/// Concurrent refresh **discards the stale generation** rather than +/// racing. +/// +/// Driven by completing two refreshes out of order, which no +/// arrangement of real subprocess timing can guarantee — so the +/// completion handler is called directly, with a request carrying the +/// older generation. The positive control at the end is what makes the +/// discard attributable to the generation and not to the payload. +#[test] +fn g6_17_a_stale_completion_discards_its_rows() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + let row = format!("1 .M N... 100644 100644 100644 {H} {H} SENTINEL.txt"); + let payload = z_payload(&["# branch.oid deadbeef", "# branch.head main", &row]); + + // A completion from one generation ago. + exec( + &s, + &format!( + "local current = pmacs.git._generation()\n\ + pmacs.git._deliver_status(\n\ + {{ generation = current - 1,\n\ + dest = pmacs.window.capture_destination() }},\n\ + {{ ok = true, code = 0, stdout = {payload}, stderr = '' }})" + ), + ); + assert!( + !panel_text(&s).contains("SENTINEL.txt"), + "a stale completion must not reach the panel:\n{}", + panel_text(&s) + ); + + // The positive control: the same payload at the CURRENT generation + // does land, so the discard above was about the generation. + // + // Asserted on a ROW, not on the panel as a whole. The panel text + // includes the header, and a malformed payload can put arbitrary + // text there through `# branch.head` — which is precisely how the + // first draft of this test passed while parsing nothing. + exec( + &s, + &format!( + "pmacs.git._deliver_status(\n\ + {{ generation = pmacs.git._generation(),\n\ + dest = pmacs.window.capture_destination() }},\n\ + {{ ok = true, code = 0, stdout = {payload}, stderr = '' }})" + ), + ); + assert_eq!( + row_line(&s, "SENTINEL.txt"), + Some(1), + "the current generation must land as a ROW:\n{}", + panel_text(&s) + ); +} + +/// Two `git.status` invocations against DIFFERENT repositories, where +/// the **first** invocation's root lookup completes **second**. The +/// second invocation must win. +/// +/// This is the ordering `g6_17` cannot see. That test drives the STATUS +/// completions out of order, and the generation each of those carries +/// was already fixed; this one drives the **root lookups** out of order, +/// which is where the generation used to be minted. `git.status` started +/// an unversioned `rev-parse` and the generation was claimed later, from +/// the callback — so whichever `rev-parse` returned last claimed the +/// newest generation and replaced the newer request. The counter that is +/// supposed to make the newest INVOCATION win instead made the slowest +/// SUBPROCESS win. +/// +/// Driven through `_deliver_root` for the same reason `g6_17` uses +/// `_deliver_status`: no arrangement of real subprocess timing can +/// guarantee that two `rev-parse` runs finish in a chosen order, and a +/// test that merely hoped for the bad order would pass on the broken +/// code roughly half the time. +/// +/// The assertion is on the argv of the last spawn, because the contract +/// is precisely that the superseded lookup "must not proceed to spawn a +/// status" — and that is observable without pumping, so the real +/// `rev-parse` children still in flight cannot muddy it. +#[test] +fn g6_21_a_superseded_root_lookup_does_not_spawn_its_status() { + let (_dir_a, root_a) = tempdir(); + mixed_repo(&root_a); + let (_dir_b, root_b) = tempdir(); + mixed_repo(&root_b); + + let mut s = editor(); + // Invocation 1 (repo A), then invocation 2 (repo B). Neither is + // pumped, so both root lookups are genuinely in flight and each has + // reserved its generation at the command. + for root in [&root_a, &root_b] { + let root_str = root.display().to_string(); + let seed = root.join("staged.txt").display().to_string(); + exec( + &s, + &format!( + "pmacs.project.set_search_boundary({root_str:?})\n\ + pmacs.buffer.find_or_open({seed:?})" + ), + ); + exec(&s, "pmacs.git.status()"); + } + + let gen_b: i64 = eval(&s, "return pmacs.git._generation()"); + let gen_a = gen_b - 1; + assert!( + gen_a >= 1, + "premise: each invocation reserved its own generation at the \ + command, so the two differ" + ); + + let a = root_a.display().to_string(); + let b = root_b.display().to_string(); + + // The NEWER invocation's root lands first… + exec( + &s, + &format!( + "pmacs.git._deliver_root(\n\ + {{ generation = {gen_b}, dir = {b:?} }},\n\ + {{ ok = true, code = 0, stdout = {b:?}, stderr = '' }})" + ), + ); + let after_b: Vec = eval(&s, "return pmacs.git._last_spawn.args"); + assert!( + after_b.contains(&b), + "premise: the newer invocation spawned its status against B: {after_b:?}" + ); + let statuses_after_b: i64 = eval( + &s, + "local n = 0\n\ + for _, args in ipairs(pmacs.git._spawn_log) do\n\ + for _, a in ipairs(args) do if a == 'status' then n = n + 1 end end\n\ + end\n\ + return n", + ); + + // …and the OLDER invocation's root lands second, superseded. + exec( + &s, + &format!( + "pmacs.git._deliver_root(\n\ + {{ generation = {gen_a}, dir = {a:?} }},\n\ + {{ ok = true, code = 0, stdout = {a:?}, stderr = '' }})" + ), + ); + + let after_a: Vec = eval(&s, "return pmacs.git._last_spawn.args"); + assert!( + !after_a.contains(&a), + "the superseded root lookup must NOT spawn a status against A; \ + last argv was {after_a:?}" + ); + assert_eq!( + after_a, after_b, + "…so the last spawn is still the newer invocation's" + ); + let statuses_after_a: i64 = eval( + &s, + "local n = 0\n\ + for _, args in ipairs(pmacs.git._spawn_log) do\n\ + for _, a in ipairs(args) do if a == 'status' then n = n + 1 end end\n\ + end\n\ + return n", + ); + assert_eq!( + statuses_after_a, statuses_after_b, + "and it spawned nothing at all — one status invocation, not two" + ); + + // The user-visible half: pumping settles on B's repository, whatever + // order the two real `rev-parse` children happen to finish in. + assert!( + pump_until(&mut s, 15_000, |s| !panel_text(s).is_empty() + && !panel_text(s).contains("refreshing")), + "the winning invocation's panel must render; status was {:?}", + status(&s) + ); + let last: Vec = eval(&s, "return pmacs.git._last_spawn.args"); + assert!( + last.contains(&b) && !last.contains(&a), + "the settled panel belongs to the second invocation: {last:?}" + ); +} + +/// Two `d` requests in flight: the **newer** one wins, even when the +/// older one completes **last**. +/// +/// `*git-diff*` is a singleton buffer, so a diff plan that finishes +/// after a newer one would otherwise overwrite it — the newest +/// invocation losing to the slowest subprocess, which is exactly the +/// defect the status channel was fixed for. The diff channel now +/// reserves its own ticket at the keypress and discards a superseded +/// completion **before any effect**. +/// +/// Two halves, and both are needed: +/// +/// * the **real** half presses `d` twice with nothing pumped between, +/// so two plans are genuinely in flight and each really did reserve +/// its own ticket at the command; +/// * the **driven** half then completes the OLDER request, after the +/// newer one has already rendered. That ordering is the whole +/// contract and no arrangement of real subprocess timing can produce +/// it on demand — both diffs take milliseconds, and the first one +/// spawned normally finishes first, which is the order that passes on +/// the broken code. Same reason `g6_17` and `g6_21` drive their own +/// completions. +/// +/// The positive control at the end is what makes the discard +/// attributable to the ticket rather than to the payload. +#[test] +fn g6_23_a_superseded_diff_does_not_replace_the_newer_one() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + // Request A, then request B, with no frame pumped between them. `d` + // renders into the DOCUMENT window only at completion, so the panel + // is still focused for the second press. + let status_gen_before: i64 = eval(&s, "return pmacs.git._generation()"); + seat_on(&mut s, "staged.txt"); + press(&mut s, KeyCode::Char('d')); + let gen_a: i64 = eval(&s, "return pmacs.git._diff_generation()"); + seat_on(&mut s, "unstaged.txt"); + press(&mut s, KeyCode::Char('d')); + let gen_b: i64 = eval(&s, "return pmacs.git._diff_generation()"); + assert_eq!( + gen_b, + gen_a + 1, + "premise: each `d` reserves its own ticket at the keypress" + ); + // …and the diff channel is its OWN: two `d` presses must not have + // touched the status channel, which a single module-wide counter + // would have done — making `d` cancel an in-flight `g`. + let status_gen_after: i64 = eval(&s, "return pmacs.git._generation()"); + assert_eq!( + status_gen_before, status_gen_after, + "a diff must not consume the status channel's ticket" + ); + + assert!( + pump_until(&mut s, 15_000, |s| diff_text(s).contains("+worktree edit")), + "the newest request must render; diff was:\n{}\nstatus: {:?}", + diff_text(&s), + status(&s) + ); + let settled = diff_text(&s); + assert!( + settled.contains("git diff --- unstaged.txt"), + "…and it is the row `d` was last pressed on: {settled}" + ); + + // The older request finally answers. It must change nothing. + let stale = format!( + "pmacs.git._deliver_diff(\n\ + {{ generation = {gen_a}, row = {{ path = 'staged.txt' }},\n\ + plan = {{ header = 'against HEAD', steps = {{}} }},\n\ + root = '/', pieces = {{}}, index = 0,\n\ + dest = pmacs.window.capture_destination() }},\n\ + {{}},\n\ + {{ ok = true, code = 0, stdout = 'STALE-DIFF-SENTINEL\\n', stderr = '' }})" + ); + exec(&s, &stale); + assert_eq!( + diff_text(&s), + settled, + "a superseded diff must not replace the newer one" + ); + + // …and it must not reach the status band either, which is the half a + // buffer-only check would miss. + exec(&s, "pmacs.editor.set_status('')"); + exec( + &s, + &format!( + "pmacs.git._deliver_diff(\n\ + {{ generation = {gen_a}, row = {{ path = 'staged.txt' }},\n\ + plan = {{ header = 'against HEAD', steps = {{}} }},\n\ + root = '/', pieces = {{}}, index = 0,\n\ + dest = pmacs.window.capture_destination() }},\n\ + {{}},\n\ + {{ ok = true, code = 128, stdout = '',\n\ + stderr = 'fatal: STALE-FAILURE' }})" + ), + ); + assert_eq!( + status(&s), + "", + "a superseded FAILURE is as wrong as a superseded patch" + ); + assert_eq!( + diff_text(&s), + settled, + "and it wrote no failure body either" + ); + + // The positive control: the same delivery at the CURRENT ticket does + // land, so the two discards above were about the ticket. + exec( + &s, + &stale.replace( + &format!("generation = {gen_a}"), + &format!("generation = {gen_b}"), + ), + ); + let now = diff_text(&s); + assert!( + now.contains("STALE-DIFF-SENTINEL"), + "the current ticket must be delivered: {now}" + ); +} + +/// Selection is re-seated **by the completion handler**, across a +/// refresh that reorders rows. +/// +/// `listview.open` resets collapse and always seats line 1; it does not +/// preserve selection. Only `listview.refresh` does, and that is the +/// synchronous path this model cannot use — so the re-seating is owned +/// here, and this test is what says so. +#[test] +fn g6_11_selection_follows_the_path_across_a_reordering_refresh() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + seat_on(&mut s, "unstaged.txt"); + let line_before: i64 = eval(&s, "return pmacs.editor.cursor_line()"); + + // Dirty TWO tracked files that sort before it, so its line moves by + // two. Both details are load-bearing, and both were found by biting + // this test: + // + // * TRACKED, not a new untracked file — porcelain v2 emits every + // `1`/`2` record before any `?` record, so an untracked addition + // lands at the end and moves nothing at all; + // * TWO, not one — dropping the handler's re-seating entirely + // leaves the cursor exactly one row lower than it was, which a + // one-row insertion happens to match. + write(&root, "a1.txt", "a1 base\nedit\n"); + write(&root, "a2.txt", "a2 base\nedit\n"); + press(&mut s, KeyCode::Char('g')); + assert!( + pump_until(&mut s, 15_000, |s| !panel_text(s).contains("refreshing")), + "refresh must settle" + ); + + let line_after: i64 = eval(&s, "return pmacs.editor.cursor_line()"); + let row = panel_text(&s) + .lines() + .nth(usize::try_from(line_after).expect("line fits")) + .unwrap_or_default() + .to_string(); + assert!( + row.contains("unstaged.txt"), + "selection follows the PATH, not the line; landed on {row:?} in:\n{}", + panel_text(&s) + ); + assert_ne!( + line_before, line_after, + "fixture: the inserted row must have moved the selected one" + ); +} + +/// …and falls back to line 1 **without complaint** when the selected +/// path drops out of status — the common case, not an error, since a +/// file that stopped being modified simply leaves the list. +#[test] +fn g6_11b_a_vanished_selection_seats_line_one_silently() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + seat_on(&mut s, "untracked.txt"); + std::fs::remove_file(root.join("untracked.txt")).expect("rm untracked.txt"); + exec(&s, "pmacs.editor.set_status('')"); + press(&mut s, KeyCode::Char('g')); + assert!( + pump_until(&mut s, 15_000, |s| { + !panel_text(s).contains("refreshing") && !panel_text(s).contains("untracked.txt") + }), + "the refresh must settle without the vanished row:\n{}", + panel_text(&s) + ); + + let line: i64 = eval(&s, "return pmacs.editor.cursor_line()"); + assert_eq!(line, 1, "the cursor seats on the first data row"); + assert_eq!( + status(&s), + "", + "and nothing is reported — this is the correct answer, not a failure" + ); +} + +// --------------------------------------------------------------------------- +// §6 — the root rule (Q#G-2) and failure surfaces (§1.2) +// --------------------------------------------------------------------------- + +/// The root rule is witnessed on a repository whose `ProjectKind` is +/// **not** `Git`. +/// +/// `ProjectKind::Git` means a BARE repository — "no language marker +/// found inside" — and a language marker beside `.git` wins, so an +/// ordinary Rust project reports `kind = "rust"`. A lane gating on +/// `kind == "git"` would have been invisible on it, and on this +/// repository too. +#[test] +fn g6_14_the_root_rule_works_where_project_kind_is_not_git() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let root_str = root.display().to_string(); + + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + let kind: String = eval( + &s, + &format!("return pmacs.project.detect({root_str:?}).kind"), + ); + assert_eq!( + kind, "rust", + "premise: the fixture's ProjectKind is NOT `git`, because \ + Cargo.toml sits beside .git" + ); + assert!( + panel_text(&s).contains("staged.txt"), + "and the panel resolved its root through git anyway:\n{}", + panel_text(&s) + ); + // The root really is the repository's, so a row's REPOSITORY-relative + // path resolves to a real file. Porcelain paths are relative to the + // repository root, not to the directory git ran in, so a root that + // was merely "some ancestor" would open nothing. + seat_on(&mut s, "unstaged.txt"); + press(&mut s, KeyCode::Enter); + let visited = active_name(&s); + assert_eq!( + visited, + root.join("unstaged.txt").display().to_string(), + "RET opens the file the row names, resolved against the git root; \ + status was {:?}", + status(&s) + ); +} + +/// A repository rooted at a directory literally named `leaf` resolves +/// **whole**, and the status command really runs there. +/// +/// End to end, not at the parser: the directory really is created, the +/// real `git` really resolves it, and the assertion is on the cwd of the +/// spawn the module actually made. `_last_spawn` is the status +/// invocation here, since `rev-parse` runs first and carries no cwd of +/// its own. A root that lost a byte would still SPAWN — with a `-C` and +/// a cwd naming a directory that does not exist — so the panel is +/// checked for a failure row as well as for real ones. +/// +/// `open_panel` binds `pmacs.project.set_search_boundary` to the fixture +/// (R8's lesson), which matters most here: these leaf names are exactly +/// the shape that makes a detection walk out of the tempdir hard to +/// read when it goes wrong. +fn assert_root_resolves_whole(leaf: &str) { + let (_dir, base) = tempdir(); + let root = base.join(leaf); + std::fs::create_dir_all(&root).unwrap_or_else(|e| { + panic!("a root named {leaf:?} must be creatable — every byte in it is a legal POSIX path byte: {e}") + }); + mixed_repo(&root); + + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + let cwd: String = eval(&s, "return pmacs.git._last_spawn.cwd"); + assert_eq!( + cwd, + root.display().to_string(), + "the resolved root must be the WHOLE path, every byte of {leaf:?} included" + ); + + let text = panel_text(&s); + assert!( + !text.contains("exited with code"), + "…so the status command ran somewhere that exists: {text}" + ); + assert!( + text.contains("staged.txt"), + "…and produced real rows: {text}" + ); + + // And the root is usable for the gestures built on it: a + // repository-relative row path resolves against it to a real file. + seat_on(&mut s, "unstaged.txt"); + press(&mut s, KeyCode::Enter); + assert_eq!( + active_name(&s), + root.join("unstaged.txt").display().to_string(), + "RET resolves against the untruncated root; status was {:?}", + status(&s) + ); +} + +/// A repository root containing a **newline** resolves **whole**, and +/// the status command really runs there. +/// +/// A newline is a legal byte in a POSIX path — the fixture builds one +/// and `git rev-parse --show-toplevel` prints it, terminator and all — +/// so parsing that output with a first-line match truncates +/// `/tmp/…/nl\nroot` to `/tmp/…/nl`, and every command this module runs +/// afterwards gets a `-C` and a cwd naming a directory that does not +/// exist. The right answer is to strip git's final terminator and +/// nothing else. +/// +/// It rides beside the one-line-status rule rather than replacing it: +/// the helper this uses is deliberately **separate** from `first_line`, +/// whose other three callers all feed the single-line status band and +/// would be corrupted by a multi-line message. +#[test] +fn g6_14c_a_root_containing_a_newline_is_not_truncated() { + assert_root_resolves_whole("nl\nroot"); +} + +/// A repository root ending in a **carriage return** resolves whole too +/// — the byte the newline fix's own strip still ate. +/// +/// `\r` is as legal in a POSIX directory name as `\n` is, and it is the +/// byte that makes `\r?\n$` ambiguous: for a root named `trailing\r`, +/// git prints `…/trailing` `0d` `0a`, where the `0d` is the PATH and +/// only the `0a` is the terminator. A strip tolerant of an optional +/// preceding carriage return cannot tell those apart and takes both, +/// resolving the root as `…/trailing` — a directory that does not +/// exist. Only git's final `\n` may be removed. +/// +/// The second case sends the two hazards in together, because a root may +/// hold both and neither fix may mask the other: an embedded newline +/// (which forbids a first-line read) ahead of a trailing carriage return +/// (which forbids an over-eager terminator strip). +#[test] +fn g6_14d_a_root_ending_in_a_carriage_return_is_not_truncated() { + assert_root_resolves_whole("trailing\r"); + assert_root_resolves_whole("nl\nand-trailing\r"); +} + +/// A directory outside any repository reports it, rather than opening +/// an empty panel or saying nothing. +#[test] +fn g6_14b_a_directory_outside_a_repository_is_reported() { + let (_dir, root) = tempdir(); + write(&root, "loose.txt", "not in a repo\n"); + let root_str = root.display().to_string(); + let seed = root.join("loose.txt").display().to_string(); + + let mut s = editor(); + exec( + &s, + &format!( + "pmacs.project.set_search_boundary({root_str:?})\n\ + pmacs.buffer.find_or_open({seed:?})" + ), + ); + exec(&s, "pmacs.git.status()"); + assert!( + pump_until(&mut s, 15_000, |s| status(s) + .contains("not inside a repository")), + "the non-zero rev-parse exit IS the answer; status was {:?}", + status(&s) + ); + assert!( + panel_text(&s).is_empty(), + "and no panel is opened: {:?}", + panel_text(&s) + ); +} + +/// A missing `git` on `PATH` is witnessed, and surfaces **guidance** +/// rather than silence (§1.2's asymmetry; the same lesson #204 landed +/// for a missing language server). +/// +/// Reached by pointing `pmacs.git._program` at a name that is not on +/// `PATH`, which produces exactly the ENOENT an absent git produces. +/// There is no other in-process route: 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 +/// — which this project forbids. +#[test] +fn g6_15_a_missing_git_binary_surfaces_guidance() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let root_str = root.display().to_string(); + let seed = root.join("staged.txt").display().to_string(); + + let mut s = editor(); + exec( + &s, + &format!( + "pmacs.project.set_search_boundary({root_str:?})\n\ + pmacs.buffer.find_or_open({seed:?})\n\ + pmacs.git._program = 'pmacs-no-such-git-binary'" + ), + ); + exec(&s, "pmacs.git.status()"); + // The spawn fails synchronously, so the guidance is already there; + // pumping anyway proves nothing later overwrites it with silence. + pump_for(&mut s, 200); + + let reported = status(&s); + assert!( + reported.contains("pmacs-no-such-git-binary") && reported.contains("PATH"), + "the failure must name the program and point at PATH; got {reported:?}" + ); + assert!( + panel_text(&s).is_empty(), + "and no panel is opened on a failed root resolution: {:?}", + panel_text(&s) + ); +} + +/// A failing `git status` renders a **row**, carrying the exit code and +/// the first line of stderr, plus a status message. +/// +/// Driven by removing `.git` under a live panel and pressing `g`: the +/// panel already holds its root, so the refresh really does run and +/// really does fail. +#[test] +fn g6_18_a_failing_status_renders_a_row_with_code_and_stderr() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + std::fs::remove_dir_all(root.join(".git")).expect("rm -rf .git"); + press(&mut s, KeyCode::Char('g')); + assert!( + pump_until(&mut s, 15_000, |s| panel_text(s) + .contains("exited with code")), + "the failure must become a ROW, not a silence; panel:\n{}\nstatus: {:?}", + panel_text(&s), + status(&s) + ); + + let text = panel_text(&s); + assert!( + text.contains("exited with code 128"), + "the row carries the exit code: {text}" + ); + assert!( + text.contains("not a git repository"), + "…and the first stderr line: {text}" + ); + assert!( + status(&s).contains("git status:"), + "…and a status message rides with it: {:?}", + status(&s) + ); +} + +// --------------------------------------------------------------------------- +// §6 — structural claims +// --------------------------------------------------------------------------- + +/// 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. +/// +/// Structural alone would not be enough — a comparison of two +/// authorities does not catch a misrouted consumer — so the behavioural +/// half rides with it: `n`, `p` and `q` are listview's own bindings and +/// are driven through `dispatch_key`. +#[test] +fn g6_19_the_panel_is_a_listview_adopter() { + const GIT: &str = include_str!("../builtin/runtime/git.lua"); + + assert!( + GIT.contains("pmacs.listview.open"), + "the panel must go through the primitive" + ); + let creates: Vec<&str> = GIT + .lines() + .filter(|l| !l.trim_start().starts_with("--")) + .filter(|l| l.contains("pmacs.buffer.create")) + .collect(); + assert_eq!( + creates.len(), + 1, + "the ONLY buffer this module creates itself is *git-diff*; the \ + status panel is listview's. Found: {creates:?}" + ); + assert!( + creates[0].contains("DIFF_BUFFER"), + "and it is the diff buffer: {creates:?}" + ); + let binds: Vec<&str> = GIT + .lines() + .filter(|l| !l.trim_start().starts_with("--")) + .filter(|l| l.contains("pmacs.keymap.bind")) + .collect(); + assert!( + binds.is_empty(), + "`d` is bound through listview's `keys` table, never by this \ + module reaching for the keymap itself: {binds:?}" + ); + // No second `rev-parse` for unborn detection: that fact comes from + // the status output already being parsed. Checked on CODE lines + // only — the module's own comment explains the rule and names the + // process it forbids, and a substring sweep would trip on it. + let verifies: Vec<&str> = GIT + .lines() + .filter(|l| !l.trim_start().starts_with("--")) + .filter(|l| l.contains("--verify")) + .collect(); + assert!( + verifies.is_empty(), + "unborn detection must not reintroduce `rev-parse --verify HEAD`: {verifies:?}" + ); + assert!( + GIT.contains("(initial)"), + "…it reads `# branch.oid (initial)` instead" + ); + + let (_dir, root) = tempdir(); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + let start: i64 = eval(&s, "return pmacs.editor.cursor_line()"); + press(&mut s, KeyCode::Char('n')); + let down: i64 = eval(&s, "return pmacs.editor.cursor_line()"); + assert_eq!(down, start + 1, "`n` is listview's own motion"); + press(&mut s, KeyCode::Char('p')); + let up: i64 = eval(&s, "return pmacs.editor.cursor_line()"); + assert_eq!(up, start, "`p` too"); + press(&mut s, KeyCode::Char('q')); + assert_ne!( + active_name(&s), + "*git-status*", + "`q` leaves the panel, through listview's quit" + ); +} + +/// `git.enabled` is a real config-registry setting (Q#G-4), and turning +/// it off means nothing is spawned — reported, not silently inert. +#[test] +fn g6_config_git_enabled_is_registry_defined_and_honoured() { + let (_dir, root) = tempdir(); + mixed_repo(&root); + let root_str = root.display().to_string(); + let seed = root.join("staged.txt").display().to_string(); + + let mut s = editor(); + let (default, kind, described): (bool, String, bool) = eval( + &s, + "local d = pmacs.config.describe('git.enabled')\n\ + return pmacs.config.get('git.enabled'), d.type, #d.description > 0", + ); + assert!(default, "default is true"); + assert_eq!(kind, "boolean"); + assert!(described, "the registry carries a real description"); + + exec( + &s, + &format!( + "pmacs.project.set_search_boundary({root_str:?})\n\ + pmacs.buffer.find_or_open({seed:?})\n\ + pmacs.config.set('git.enabled', false)\n\ + pmacs.git._spawn_log = {{}}" + ), + ); + exec(&s, "pmacs.git.status()"); + pump_for(&mut s, 300); + assert!( + status(&s).contains("git.enabled"), + "the refusal names the setting; got {:?}", + status(&s) + ); + let spawned: i64 = eval(&s, "return #pmacs.git._spawn_log"); + assert_eq!(spawned, 0, "and nothing was spawned"); + assert!(panel_text(&s).is_empty(), "and no panel opened"); +} + +// --------------------------------------------------------------------------- +// Q#G-9 — the continuation lands where it was ASKED FROM +// --------------------------------------------------------------------------- + +/// The frontend that competes for ambient authority while git runs. +/// +/// Same shape and same id as `destination_capture_acceptance`'s, so the +/// two suites describe one mechanism rather than two conventions. +const COMPETITOR: FrontendId = FrontendId(7); + +/// The buffer name in `fid`'s own side (panel) slot, or `None` when that +/// frontend has no panel. +/// +/// **Per-frontend on purpose.** `panel_text` finds `*git-status*` by +/// NAME, which is global — it answers "does this buffer exist and what +/// is in it", not "which frontend is showing it". A name lookup is +/// therefore satisfied by a render into the wrong frontend, and an +/// earlier draft of `g6_25` passed its own mutation for exactly that +/// reason. +fn panel_buffer_name(s: &EditorState, fid: FrontendId) -> Option { + let core = s.core.borrow(); + let side = core.side_window_for(fid)?; + let buffer_id = core.windows.get(&side)?.buffer_id; + let reg = core.registry.borrow(); + Some(reg.get(buffer_id).ok()?.name().to_string()) +} + +/// The buffer name in `fid`'s active window. +fn active_name_in(s: &EditorState, fid: FrontendId) -> Option { + let core = s.core.borrow(); + let win = core.views.get(&fid)?.active; + let buffer_id = core.windows.get(&win)?.buffer_id; + let reg = core.registry.borrow(); + Some(reg.get(buffer_id).ok()?.name().to_string()) +} + +/// Register a second frontend with its own single-window layout. +fn attach_frontend(s: &EditorState) -> pmacs::window::WindowId { + use pmacs::window::{FrontendView, Layout, Window, WindowId}; + let win = WindowId::next(); + let mut core = s.core.borrow_mut(); + let buffer_id = core.active_buffer_id(); + let text_view = { + let reg = core.registry.borrow(); + pmacs::text_view::TextView::new(reg.get(buffer_id).expect("buffer")) + }; + core.windows + .insert(win, Window::new(win, buffer_id, text_view)); + core.register_frontend_view( + COMPETITOR, + FrontendView { + layout: Layout::single(win), + active: win, + fold_projection: true, + panel_capable: true, + frame_geometry: None, + panel_hidden: false, + }, + ); + win +} + +/// **The defect this lane's review found, pinned on both channels.** +/// +/// `git status` and `git diff` settle a tick or more after the keypress. +/// Before the destination capture, both rendered through whichever +/// frontend was ambient *at completion* — so running `M-x git.status` in +/// frontend A and letting B become active while git ran opened A's panel +/// in B. The generation and root were already captured at invocation; +/// the frontend was the one thing still read late. +/// +/// Driven through `_deliver_status` / `_deliver_diff` — the seams the +/// concurrency tests already use — because the switch has to happen +/// *while the work is in flight*, and no arrangement of real subprocess +/// timing can produce that moment on demand. +/// +/// **Falsified by dropping either `commit_ui` call**: the render then +/// follows the ambient frontend and the competitor's window changes. +/// Asserting only that the capturing frontend got the buffer would pass +/// on a commit that wrote to *both*, so the competitor is asserted +/// unchanged as well. +#[test] +fn g6_25_a_completion_lands_in_the_frontend_that_asked() { + let (_td, root) = tempdir(); + init_repo(&root); + mixed_repo(&root); + let mut s = editor(); + open_panel(&mut s, &root, "staged.txt"); + + let other_win = attach_frontend(&s); + let other_before = { + let core = s.core.borrow(); + core.windows.get(&other_win).map(|w| w.buffer_id) + }; + + // Captured while LOCAL is the acting frontend — the invocation. + exec(&s, "saved_dest = pmacs.window.capture_destination()"); + + // The competitor takes ambient authority while git is "running". + s.core.borrow_mut().active_frontend = COMPETITOR; + + let row = format!("1 .M N... 100644 100644 100644 {H} {H} SENTINEL.txt"); + let payload = z_payload(&["# branch.oid deadbeef", "# branch.head main", &row]); + exec( + &s, + &format!( + "pmacs.git._deliver_status(\n\ + {{ generation = pmacs.git._generation(), dest = saved_dest }},\n\ + {{ ok = true, code = 0, stdout = {payload}, stderr = '' }})" + ), + ); + + // Asserted PER FRONTEND. `panel_text` alone would pass on a render + // into the competitor, because it finds the buffer by name. + assert_eq!( + panel_buffer_name(&s, FrontendId::LOCAL).as_deref(), + Some("*git-status*"), + "the panel must be in the capturing frontend's own side slot" + ); + assert_eq!( + panel_buffer_name(&s, COMPETITOR), + None, + "the competing frontend must not have grown a panel" + ); + assert_eq!( + { + let core = s.core.borrow(); + core.windows.get(&other_win).map(|w| w.buffer_id) + }, + other_before, + "…and its document window is untouched too" + ); + + s.core.borrow_mut().active_frontend = FrontendId::LOCAL; + assert!( + panel_text(&s).contains("SENTINEL.txt"), + "the rows must have rendered:\n{}", + panel_text(&s) + ); + + // The diff channel, same shape, DOCUMENT profile. An empty remaining + // plan makes `advance_diff` render on this very delivery. + let root_str = root.display().to_string(); + exec(&s, "saved_dest = pmacs.window.capture_destination()"); + s.core.borrow_mut().active_frontend = COMPETITOR; + let other_before_diff = { + let core = s.core.borrow(); + core.windows.get(&other_win).map(|w| w.buffer_id) + }; + exec( + &s, + &format!( + "pmacs.git._deliver_diff(\n\ + {{ generation = pmacs.git._diff_generation(),\n\ + row = {{ path = 'SENTINEL.txt' }},\n\ + plan = {{ steps = {{}}, header = 'hdr' }},\n\ + pieces = {{}}, index = 0, root = {root_str:?},\n\ + dest = saved_dest }},\n\ + {{ }},\n\ + {{ ok = true, code = 0, stdout = 'diff body', stderr = '' }})" + ), + ); + + // The diff takes a DOCUMENT window, so the discriminating question + // is which frontend's active window now holds `*git-diff*`. + assert_ne!( + active_name_in(&s, COMPETITOR).as_deref(), + Some("*git-diff*"), + "the diff must not have taken the competing frontend's window" + ); + assert_eq!( + { + let core = s.core.borrow(); + core.windows.get(&other_win).map(|w| w.buffer_id) + }, + other_before_diff, + "…and that window's buffer is unchanged" + ); + assert_eq!( + active_name_in(&s, FrontendId::LOCAL).as_deref(), + Some("*git-diff*"), + "the diff must land in the capturing frontend's own window" + ); + s.core.borrow_mut().active_frontend = FrontendId::LOCAL; + assert!( + diff_text(&s).contains("diff body"), + "…carrying the body it was given:\n{}", + diff_text(&s) + ); +} + +// Isolated bootstrap storage roots: an integration test is compiled +// without `cfg(test)`, so a raw `EditorState::new()` would read the +// developer's real `init.lua` and write into their real data root. +#[path = "common/iso.rs"] +mod iso;