Compare commits
No commits in common. "main" and "worker-identity-stage1" have entirely different histories.
main
...
worker-ide
34
COHERENCE.md
34
COHERENCE.md
|
|
@ -1306,17 +1306,6 @@ 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
|
||||
|
|
@ -1446,25 +1435,10 @@ What does not:
|
|||
|
||||
- **Code actions apply the first action blindly** — no picker (a
|
||||
roadmap "dark matter" item still true at audit).
|
||||
- **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.
|
||||
- **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.
|
||||
- No test run/debug affordances (DAP is a future arc,
|
||||
`docs/dap-debugging-framing.md`).
|
||||
- No missing-tool guidance affordances (§1.2 — the diagnostic that
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -25,7 +25,6 @@
|
|||
-- rows = { { text = "src/foo.rs:12:4", item = <any> }, ... },
|
||||
-- on_visit = function(item) ... end, -- RET/SPC (optional)
|
||||
-- on_refresh = function() return rows end, -- g (optional)
|
||||
-- keys = { d = "git.diff-file" }, -- extra buffer-local keys
|
||||
-- }
|
||||
|
||||
pmacs.listview = pmacs.listview or {}
|
||||
|
|
@ -264,193 +263,19 @@ local function seat_cursor(p, line)
|
|||
end
|
||||
end
|
||||
|
||||
-- The primitive's own key surface, named ONCE so the binder below and
|
||||
-- the `keys` validator consult the same list. Previously this was a
|
||||
-- sequence of `bind(...)` calls and the set existed nowhere as data,
|
||||
-- which is why the git framing had to quote it from the source
|
||||
-- (docs/git-integration-framing.md Q#G-7).
|
||||
local FIXED_KEYS = {
|
||||
{ "RET", "listview.visit" },
|
||||
{ "SPC", "listview.visit" },
|
||||
{ "n", "cursor.down" },
|
||||
{ "<down>", "cursor.down" },
|
||||
{ "p", "cursor.up" },
|
||||
{ "<up>", "cursor.up" },
|
||||
{ "TAB", "listview.toggle" },
|
||||
{ "g", "listview.refresh" },
|
||||
{ "q", "listview.quit" },
|
||||
}
|
||||
|
||||
local function bind_local_keymap(buf)
|
||||
for _, entry in ipairs(FIXED_KEYS) do
|
||||
pmacs.keymap.bind {
|
||||
scope = "buffer", buffer = buf, sequence = entry[1], command = entry[2],
|
||||
}
|
||||
local function bind(seq, command)
|
||||
pmacs.keymap.bind { scope = "buffer", buffer = buf, sequence = seq, command = command }
|
||||
end
|
||||
end
|
||||
|
||||
-- ---------------------------------------------------------------------
|
||||
-- Consumer-supplied keys (Q#G-7)
|
||||
-- ---------------------------------------------------------------------
|
||||
--
|
||||
-- An optional `keys = { <sequence> = <command name> }` on the open
|
||||
-- spec, bound through the SAME `pmacs.keymap.bind { scope = "buffer" }`
|
||||
-- path as the fixed set above. It exists because a consumer cannot
|
||||
-- safely bind its own key from outside: `open` disambiguates a name
|
||||
-- collision to `<2>`, so the name a consumer passed is not necessarily
|
||||
-- the buffer it got, and this module is the only place the handle is
|
||||
-- known. No key is intercepted anywhere — COHERENCE.md §6's shadow
|
||||
-- count is unchanged by this.
|
||||
--
|
||||
-- INSTALL-ONCE, MATCH-ON-REOPEN. `Keymap::bind` refuses duplicates
|
||||
-- (`KeymapError::DuplicateBinding`, "Refuse rather than silently
|
||||
-- overwrite", src/keymap_tree.rs), and a consumer built on the async
|
||||
-- completion model calls `open` again on EVERY refresh. So keys are
|
||||
-- installed when the buffer is created and a later `open` for a live
|
||||
-- panel does not re-bind — it COMPARES, and errors on divergence.
|
||||
-- Silently keeping the old binding would hand the consumer a key that
|
||||
-- does something other than what it just asked for, which is the dead-
|
||||
-- or-lying-key defect this module already condemns for `g`.
|
||||
|
||||
-- A key sequence's whitespace-separated chord tokens. That is exactly
|
||||
-- how `parse_sequence` (src/key.rs) splits one, so a prefix relation
|
||||
-- computed here is the same relation the trie would find.
|
||||
local function chords_of(sequence)
|
||||
local out = {}
|
||||
for token in sequence:gmatch("%S+") do out[#out + 1] = token end
|
||||
return out
|
||||
end
|
||||
|
||||
-- True when one chord list is a STRICT prefix of the other. Either
|
||||
-- direction is a conflict: `Keymap` refuses both turning a leaf into a
|
||||
-- submap (`WouldExtendLeaf`) and shadowing a submap with a leaf
|
||||
-- (`WouldShadowSubmap`), and a `keys` table must not be able to reach
|
||||
-- either.
|
||||
local function prefix_conflict(a, b)
|
||||
local short, long = a, b
|
||||
if #a > #b then short, long = b, a end
|
||||
if #short == 0 or #short == #long then return false end
|
||||
for i = 1, #short do
|
||||
if short[i] ~= long[i] then return false end
|
||||
end
|
||||
return true
|
||||
end
|
||||
|
||||
-- Normalize `keys` into a sorted array of `{ sequence, command }`.
|
||||
-- Sorted so the comparison on reopen and every error message are
|
||||
-- deterministic (`pairs` order is not).
|
||||
local function normalized_keys(keys)
|
||||
if keys == nil then return {} end
|
||||
if type(keys) ~= "table" then
|
||||
error(string.format(
|
||||
"listview: `keys` must be a table of sequence -> command name; got %s",
|
||||
type(keys)))
|
||||
end
|
||||
local out = {}
|
||||
for sequence, command in pairs(keys) do
|
||||
if type(sequence) ~= "string" or sequence == "" then
|
||||
error("listview: every `keys` entry must be keyed by a non-empty key sequence")
|
||||
end
|
||||
if type(command) ~= "string" or command == "" then
|
||||
error(string.format(
|
||||
"listview: `keys[%q]` must be a command NAME (a non-empty string); got %s",
|
||||
sequence, type(command)))
|
||||
end
|
||||
out[#out + 1] = { sequence = sequence, command = command }
|
||||
end
|
||||
table.sort(out, function(a, b) return a.sequence < b.sequence end)
|
||||
return out
|
||||
end
|
||||
|
||||
-- A FIRST-PASS collision check, for a better message than the keymap's.
|
||||
--
|
||||
-- It compares RAW TOKENS, and that is deliberately not sufficient: the
|
||||
-- key parser canonicalizes aliases before it ever reaches the trie
|
||||
-- (`parse_key_code`, src/key.rs, uppercases and folds `RET`/`RETURN`/
|
||||
-- `ENTER`, `SPC`/`SPACE`, `ESC`/`ESCAPE`, `BS`/`BACKSPACE`,
|
||||
-- `DEL`/`DELETE`), so `keys = { RETURN = ... }` is a collision this
|
||||
-- function cannot see.
|
||||
--
|
||||
-- **`Keymap::bind` is the authority, and `ensure_panel` tears the panel
|
||||
-- down when it refuses.** That is not a fallback for a check that
|
||||
-- happens to be weak --- it is the only version that cannot go stale. A
|
||||
-- Lua-side canonicalizer would be a second copy of `parse_key_code`'s
|
||||
-- alias table, and the day the Rust one gains a name the Lua one would
|
||||
-- silently stop seeing that alias, reintroducing exactly this bug for
|
||||
-- it. (There is also no way to canonicalize an arbitrary sequence from
|
||||
-- Lua today: `display_sequence` is reachable only through
|
||||
-- `describe.key` and `keymap.list`, which both require the sequence to
|
||||
-- be BOUND already.)
|
||||
--
|
||||
-- So what this buys is diagnosis, not safety: a named "that is the
|
||||
-- panel's own `g`" instead of a raw `DuplicateBinding`.
|
||||
local function check_key_collisions(entries)
|
||||
for i, entry in ipairs(entries) do
|
||||
local mine = chords_of(entry.sequence)
|
||||
for _, fixed in ipairs(FIXED_KEYS) do
|
||||
if entry.sequence == fixed[1] then
|
||||
error(string.format(
|
||||
"listview: `keys` may not rebind %q --- it is part of the panel's "
|
||||
.. "own key surface (RET SPC n <down> p <up> TAB g q), bound to %q",
|
||||
entry.sequence, fixed[2]))
|
||||
end
|
||||
if prefix_conflict(mine, chords_of(fixed[1])) then
|
||||
error(string.format(
|
||||
"listview: `keys` entry %q conflicts with the panel's own %q --- "
|
||||
.. "one is a prefix of the other, which the keymap refuses rather "
|
||||
.. "than turning a binding into a submap",
|
||||
entry.sequence, fixed[1]))
|
||||
end
|
||||
end
|
||||
for j = i + 1, #entries do
|
||||
if prefix_conflict(mine, chords_of(entries[j].sequence)) then
|
||||
error(string.format(
|
||||
"listview: `keys` entries %q and %q conflict --- one is a prefix "
|
||||
.. "of the other", entry.sequence, entries[j].sequence))
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
-- Bind the entries, naming which one the keymap refused.
|
||||
--
|
||||
-- It does NOT roll back the keys it already bound: its caller owns
|
||||
-- teardown, and the caller's teardown is killing the whole buffer,
|
||||
-- which takes the buffer's entire keymap scope with it
|
||||
-- (`after_buffer_removed` -> `KeymapStack::remove_buffer`). Unbinding
|
||||
-- here as well would be a second, weaker cleanup mechanism for the same
|
||||
-- failure --- and the weaker one is what let a half-built panel survive.
|
||||
local function install_keys(buf, entries)
|
||||
for _, entry in ipairs(entries) do
|
||||
local ok, err = pcall(pmacs.keymap.bind, {
|
||||
scope = "buffer", buffer = buf,
|
||||
sequence = entry.sequence, command = entry.command,
|
||||
})
|
||||
if not ok then
|
||||
error(string.format(
|
||||
"listview: cannot bind %q to %q: %s",
|
||||
entry.sequence, entry.command, tostring(err)))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
local function keys_match(a, b)
|
||||
if #a ~= #b then return false end
|
||||
for i = 1, #a do
|
||||
if a[i].sequence ~= b[i].sequence or a[i].command ~= b[i].command then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
end
|
||||
|
||||
local function render_keys(entries)
|
||||
if #entries == 0 then return "none" end
|
||||
local parts = {}
|
||||
for i, entry in ipairs(entries) do
|
||||
parts[i] = string.format("%s=%s", entry.sequence, entry.command)
|
||||
end
|
||||
return table.concat(parts, " ")
|
||||
bind("RET", "listview.visit")
|
||||
bind("SPC", "listview.visit")
|
||||
bind("n", "cursor.down")
|
||||
bind("<down>", "cursor.down")
|
||||
bind("p", "cursor.up")
|
||||
bind("<up>", "cursor.up")
|
||||
bind("TAB", "listview.toggle")
|
||||
bind("g", "listview.refresh")
|
||||
bind("q", "listview.quit")
|
||||
end
|
||||
|
||||
-- Build the persistent panel record for `name`. A user-killed panel
|
||||
|
|
@ -468,23 +293,9 @@ 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, key_entries)
|
||||
local function ensure_panel(name)
|
||||
local p = panel_for_requested_name(name)
|
||||
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
|
||||
if p then return p end
|
||||
|
||||
local actual = name
|
||||
if find_buffer_by_name(actual) then
|
||||
|
|
@ -504,64 +315,29 @@ local function ensure_panel(name, key_entries)
|
|||
|
||||
local buf = pmacs.buffer.create(actual)
|
||||
p = { requested_name = name, buffer = buf, line_to_item = {},
|
||||
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)
|
||||
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.
|
||||
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")
|
||||
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)
|
||||
return p
|
||||
end
|
||||
|
||||
function pmacs.listview.open(spec)
|
||||
assert(type(spec) == "table" and type(spec.name) == "string",
|
||||
"listview.open: spec.name (string) required")
|
||||
-- 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)
|
||||
local p = ensure_panel(spec.name)
|
||||
p.header = spec.header or spec.name
|
||||
p.on_visit = spec.on_visit
|
||||
p.on_refresh = spec.on_refresh
|
||||
|
|
|
|||
|
|
@ -1924,8 +1924,7 @@ end
|
|||
local FILE_WATCH_INTERVAL_MS = 250
|
||||
|
||||
-- file_watchers[tostring(sid)][registrationId] = list of watch records
|
||||
-- ({ cancelled = bool, form = "relative"|"absolute", _sleep = handle? }),
|
||||
-- one per glob watcher.
|
||||
-- ({ cancelled = bool, _sleep = handle? }), one per glob watcher.
|
||||
local file_watchers = {}
|
||||
|
||||
-- WatchKind is a bitmask (Create=1, Change=2, Delete=4); test it
|
||||
|
|
@ -2059,18 +2058,7 @@ end
|
|||
local FC_CREATED, FC_CHANGED, FC_DELETED = 1, 2, 3
|
||||
|
||||
local function start_file_watcher(sid, base, glob, kind_mask, record)
|
||||
-- Per LSP, a plain-string glob matches the file's ABSOLUTE path,
|
||||
-- while a RelativePattern's pattern is relative to its base — the
|
||||
-- record's `form` (from resolve_watcher) picks the match subject.
|
||||
-- scan_tree always walks in relative terms; only the string handed
|
||||
-- to the matcher changes.
|
||||
local match_glob = glob_matcher(glob)
|
||||
local matches = match_glob
|
||||
if record.form == "absolute" then
|
||||
matches = function(rel)
|
||||
return match_glob(base .. "/" .. rel)
|
||||
end
|
||||
end
|
||||
local matches = glob_matcher(glob)
|
||||
pmacs.async(function()
|
||||
local prev = scan_tree(base, matches)
|
||||
while not record.cancelled and server_is_live(sid) do
|
||||
|
|
@ -2081,29 +2069,6 @@ local function start_file_watcher(sid, base, glob, kind_mask, record)
|
|||
if record.cancelled or not server_is_live(sid) then break end
|
||||
|
||||
local cur = scan_tree(base, matches)
|
||||
-- The seam that makes the recheck below WITNESSABLE. `scan_tree`
|
||||
-- suspends on `read_dir` once per directory, and the race is a
|
||||
-- cancel arriving during one of those suspensions --- which no
|
||||
-- arrangement of real timing can be made to happen on demand.
|
||||
-- Same reason `git.lua` exposes `_deliver_status`: the contract is
|
||||
-- about an interleaving the caller does not choose. Unset in
|
||||
-- production, so this costs one nil test per tick.
|
||||
-- `cur` is handed over so a test can cancel on THE SCAN THAT
|
||||
-- OBSERVED a given change. Cancelling on any other scan is not a
|
||||
-- witness: the loop would break at the post-sleep check on the
|
||||
-- next iteration and emit nothing anyway, so the assertion would
|
||||
-- pass with the recheck below deleted.
|
||||
if pmacs.lsp._after_scan_for_tests then
|
||||
pcall(pmacs.lsp._after_scan_for_tests, record, cur)
|
||||
end
|
||||
-- RECHECKED AFTER THE SCAN, not only after the sleep (review P2).
|
||||
-- The coroutine is suspended for most of a tick with `_sleep`
|
||||
-- already cleared, so a cancel landing there sets `cancelled` and
|
||||
-- has no sleep to interrupt. Without this line the resumed scan
|
||||
-- runs on to `did_change_watched_files` below and a SUPERSEDED
|
||||
-- watcher emits one last batch under its OLD pattern. One batch is
|
||||
-- enough: it is a wrong-pattern notification the server acts on.
|
||||
if record.cancelled or not server_is_live(sid) then break end
|
||||
local changes = {}
|
||||
for rel, sig in pairs(cur) do
|
||||
local was = prev[rel]
|
||||
|
|
@ -2132,44 +2097,22 @@ local function start_file_watcher(sid, base, glob, kind_mask, record)
|
|||
end
|
||||
|
||||
-- Resolve a GlobPattern (string | { baseUri, pattern }) to
|
||||
-- (base_dir, pattern, form). The form must travel with the pair: a
|
||||
-- RelativePattern's pattern is relative to its baseUri, and dropping
|
||||
-- that distinction is what made absolute server globs unable to match
|
||||
-- anything. A bare string with no base falls back to the directory of
|
||||
-- an attached file on `sid` (best effort).
|
||||
--
|
||||
-- THE FORM COMES FROM THE PATTERN, NOT FROM THE UNION ARM (review P1).
|
||||
-- The first fix for #233 returned `"absolute"` for every string, which
|
||||
-- is a different bug wearing the same shape: LSP 3.17 defines `Pattern`
|
||||
-- relative to a base path, and VS Code treats a string watcher as
|
||||
-- applying across workspace folders, so a bare `*.txt` is a VALID
|
||||
-- relative pattern. Classifying it absolute matched it against
|
||||
-- `<base>/foo.txt`, which `^[^/]*%.txt$` can never match --- so that
|
||||
-- fix silently broke a case that worked before it. A leading `/` is
|
||||
-- what makes a pattern absolute; the arm it arrived in is not.
|
||||
-- (base_dir, pattern). A bare string with no base falls back to the
|
||||
-- directory of an attached file on `sid` (best effort).
|
||||
local function resolve_watcher(sid, gp)
|
||||
if type(gp) == "table" and gp.baseUri then
|
||||
return pmacs.lsp.path_for_uri(gp.baseUri), gp.pattern or "**", "relative"
|
||||
return pmacs.lsp.path_for_uri(gp.baseUri), gp.pattern or "**"
|
||||
end
|
||||
if type(gp) == "string" then
|
||||
for _, rec in pairs(attachments) do
|
||||
if rec.server == sid and rec.uri then
|
||||
local p = pmacs.lsp.path_for_uri(rec.uri)
|
||||
local dir = p and p:match("^(.*)/[^/]*$")
|
||||
if dir then
|
||||
return dir, gp, (gp:sub(1, 1) == "/") and "absolute" or "relative"
|
||||
end
|
||||
if dir then return dir, gp end
|
||||
end
|
||||
end
|
||||
end
|
||||
return nil, nil, nil
|
||||
end
|
||||
|
||||
local function cancel_watch_records(recs)
|
||||
for _, r in ipairs(recs or {}) do
|
||||
r.cancelled = true
|
||||
if r._sleep then pcall(function() r._sleep:cancel() end) end
|
||||
end
|
||||
return nil, nil
|
||||
end
|
||||
|
||||
local function register_file_watchers(sid, registrations)
|
||||
|
|
@ -2177,16 +2120,11 @@ local function register_file_watchers(sid, registrations)
|
|||
file_watchers[skey] = file_watchers[skey] or {}
|
||||
for _, reg in ipairs(registrations or {}) do
|
||||
if reg.method == "workspace/didChangeWatchedFiles" then
|
||||
-- Re-registering a live id supersedes it (rust-analyzer does
|
||||
-- this): cancel the outgoing records first, because the table
|
||||
-- write below drops the only reference to them and an
|
||||
-- uncancelled record polls until the server dies.
|
||||
cancel_watch_records(file_watchers[skey][reg.id])
|
||||
local recs = {}
|
||||
for _, w in ipairs((reg.registerOptions or {}).watchers or {}) do
|
||||
local base, pat, form = resolve_watcher(sid, w.globPattern)
|
||||
local base, pat = resolve_watcher(sid, w.globPattern)
|
||||
if base and pat then
|
||||
local r = { cancelled = false, form = form }
|
||||
local r = { cancelled = false }
|
||||
recs[#recs + 1] = r
|
||||
start_file_watcher(sid, base, pat, w.kind or 7, r)
|
||||
end
|
||||
|
|
@ -2201,7 +2139,10 @@ local function unregister_file_watchers(sid, unregs)
|
|||
if not byid then return end
|
||||
for _, u in ipairs(unregs or {}) do
|
||||
if u.method == "workspace/didChangeWatchedFiles" and byid[u.id] then
|
||||
cancel_watch_records(byid[u.id])
|
||||
for _, r in ipairs(byid[u.id]) do
|
||||
r.cancelled = true
|
||||
if r._sleep then pcall(function() r._sleep:cancel() end) end
|
||||
end
|
||||
byid[u.id] = nil
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -5,21 +5,6 @@ landed on `main`. Read it after `docs/agent-handoff.md`. Remove completed
|
|||
entries when their PR merges; do not let this become a second permanent
|
||||
backlog.
|
||||
|
||||
**Updated 2026-08-11 — two merges and a discharge.** The LSP file
|
||||
watcher D1+D2 landed as **#234** (`ae84d58`) after one review round
|
||||
(P1 form-from-the-pattern, P2 cancelled-scan emission — both
|
||||
bite-verified), and git integration Stage 1 landed as **#227**
|
||||
(`b867f64`) immediately after, refreshed onto the merged base and
|
||||
re-gated. The 2026-08-10 hold on #227 is **discharged in order**, not
|
||||
overridden. Both lanes below are rewritten to their remainders (D3;
|
||||
git Stage 2), their durable facts absorbed into
|
||||
`docs/agent-handoff.md` §1. **Next, by user ruling: D3.** The
|
||||
canonical-base line below and the handoff anchor both moved to
|
||||
`b867f64`. Several other lane headers still say OPEN for PRs that have
|
||||
since merged (#224–#232) — per this file's own rule, trust the
|
||||
canonical-base line over any lane header; those absorptions remain
|
||||
owed by their own lanes.
|
||||
|
||||
**Updated later the same day, on a new machine.** Development moved to
|
||||
the laptop; the recovery path in "Repository authority" below was
|
||||
exercised from this checkout and the `githubsucks` alias was absent and
|
||||
|
|
@ -126,14 +111,7 @@ lesson, §1 for the two framings).
|
|||
are identical on every machine. Remote names are otherwise
|
||||
machine-local: `origin` may name this canonical URL, a release mirror,
|
||||
or something else, and therefore has no authority by name alone.
|
||||
- Canonical base at this snapshot: **`githubsucks/main` @ `b867f64`** —
|
||||
git integration Stage 1 **#227**, atop `ae84d58` the LSP file-watcher
|
||||
fix **#234**, atop `0e4c58d` destination capture **#231**, `3cc1b85`
|
||||
worker identity Stage 1 **#232**, `0857bf4` discovery Stage 2
|
||||
**#228**, `0190102` LSP LaTeX coverage **#230**, `7cf4653` the gate
|
||||
`--protocol` build step **#229**, `4bc55e8` per-worktree gate target
|
||||
dirs **#225**, `dcb852e` the R8 fixture fix **#226** and `b833b13`
|
||||
the QoL docs retirement **#224**. Beneath those, `9a26ac8`:
|
||||
- Canonical base at this snapshot: **`githubsucks/main` @ `9a26ac8`** —
|
||||
GPU horizontal scroll **#223**, which **closes the QoL arc**, atop
|
||||
`2b56d16` TUI horizontal scroll **#222**, `02f3ec3` `ui.line-wrap`
|
||||
**#221** (protocol v22), `218d2e7` GUI zoom **#220** and `da56bec`
|
||||
|
|
@ -232,43 +210,6 @@ hazard in a shape that looks committed. **A documented error message
|
|||
that never appears is worse than no documentation**, because the reader
|
||||
waits for a signal that is not coming.
|
||||
|
||||
## LSP file watcher (issue #233) — D1+D2 MERGED as #234; D3 IS NEXT
|
||||
|
||||
**Issue #233** — https://github.com/levineuwirth/pmacs/issues/233,
|
||||
still OPEN: it closes when D3 does. **PR #234 MERGED 2026-08-11**
|
||||
(`main` @ `ae84d58`), one review round. The framing is
|
||||
`docs/lsp-file-watcher-framing.md`, revision 2 — it carries the full
|
||||
record: the approved design, the answered ruling, and the two review
|
||||
findings (P1 form-from-the-pattern, P2 cancelled-scan emission) with
|
||||
their bite results. Durable facts are absorbed in
|
||||
`docs/agent-handoff.md` §1.
|
||||
|
||||
**The #227 hold is DISCHARGED.** The 2026-08-10 ruling held #227
|
||||
unmerged until this was resolved; #234 merged first and #227 followed
|
||||
the same day (`b867f64`), refreshed and re-gated on the merged base.
|
||||
|
||||
**D3 — the polling cost — is the remainder, and the user has ruled it
|
||||
is next (2026-08-11).** No branch and no framing yet. What is known,
|
||||
verified while framing D1/D2:
|
||||
|
||||
- After #234 the watcher is *correct* but still walks: `walk` recurses
|
||||
unconditionally and `matches` gates only recording, so rust-analyzer
|
||||
walks the whole tree — `.git`, `target`, `node_modules` included —
|
||||
every 250 ms, six times per tick (was twelve before D2), one async
|
||||
job per directory. The modeline still shows the churn, at roughly
|
||||
half the pre-#234 rate.
|
||||
- **No `notify`/inotify dependency in the tree** — a real
|
||||
filesystem-notification primitive is a new crate plus a new Rust
|
||||
primitive plus its Lua binding.
|
||||
- **No ignore-list infrastructure to reuse** — `src/project.rs` knows
|
||||
`.git` as a *marker* name, not as something to skip.
|
||||
- Options named in the issue: coalesce a server's watchers into one
|
||||
scan; root the scan at the workspace root; an ignore list; back off
|
||||
when nothing changes; a real notification primitive.
|
||||
- It is a `COHERENCE.md` §9 concern — background work with no
|
||||
ownership model — and the activity indicator that surfaced it is
|
||||
§9's own Stage 1. The framing must state its §20 coherence impact.
|
||||
|
||||
## `scripts/gate` — PR #225 OPEN (build tooling)
|
||||
|
||||
**PR #225** — https://github.com/levineuwirth/pmacs/pull/225. Written
|
||||
|
|
@ -324,436 +265,7 @@ 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 MERGED as #227; Stage 2 must be scheduled alone
|
||||
|
||||
**PR #227 MERGED 2026-08-11** (`main` @ `b867f64`), after five review
|
||||
rounds, a macOS CI round, and a base refresh: it was held unmerged
|
||||
behind issue #233 by the 2026-08-10 ruling, refreshed onto the merged
|
||||
base (`e2394c7`, a clean merge whose only file shared with #234 was
|
||||
this ledger), re-gated 11/11 locally and 14/14 on CI. Durable facts
|
||||
are absorbed in `docs/agent-handoff.md` §1; the framing
|
||||
(`docs/git-integration-framing.md`, revision 5) and the PR carry the
|
||||
full five-round review history.
|
||||
|
||||
**What shipped:** `*git-status*` — a `listview` panel over
|
||||
`git --no-optional-locks -C <dir> status --porcelain=v2 --branch -z` —
|
||||
and `*git-diff*` (file-level, plain generated text), with the
|
||||
install-once `keys` extension on `listview`. `builtin/runtime/git.lua`,
|
||||
34 acceptance tests, **no wire change**.
|
||||
|
||||
**Stage 2 (gutter markers) is the remainder, and it is NOT freely
|
||||
schedulable: it needs new `DecorationKind` variants — a
|
||||
`PROTOCOL_VERSION` bump — so it must run alone**, per the strict
|
||||
serialization rule on wire changes. No branch, no framing yet.
|
||||
|
||||
**Residue that stays live here:**
|
||||
|
||||
- **§9 negative impact stands:** git runs as a spawned process, and
|
||||
spawned processes do not appear in `*workers*`. A fifth
|
||||
unattributable background thing, labelled honestly; a label is not
|
||||
attribution. The D3 lane (above) and §9 Stage 2 own the model.
|
||||
- **The latent macOS sibling:** `tests/gpu_invocation_acceptance.rs`
|
||||
writes a non-UTF-8 filename to disk inside
|
||||
`#[cfg(feature = "crdt")]`, and the crdt job is ubuntu-only — it
|
||||
fails the day that job gains a macOS leg, the same way `g6_2` did
|
||||
(handoff §1: macOS cannot hold a non-UTF-8 filename).
|
||||
|
||||
## Destination capture (Q#JR14 generalization) — PR #231 OPEN, revision 9, cleared to merge
|
||||
|
||||
**PR #231** — https://github.com/levineuwirth/pmacs/pull/231. #227
|
||||
blocks on this lane.
|
||||
|
||||
The mechanism landed at `0efc8c0`; review found a correctness blocker;
|
||||
`ca72461` implemented **revision 7**, which review then **also**
|
||||
rejected; `469d5c8` replaced it with **revision 8** and its §3
|
||||
enumeration is **performed and recorded in the framing**; review then
|
||||
found a hole in revision 8's guard **scope** and the commit below closes
|
||||
it as **revision 9**.
|
||||
|
||||
**The macOS red that blocked this lane, and how it was cleared.** Both
|
||||
CI attempts at `4654b94` failed `a_pty_resize_blanks_the_host_before_repainting`
|
||||
on `Test (macos-latest / luajit)`. A control experiment was run at the
|
||||
exact base commit `0190102`: **five valid observations, all green on
|
||||
both macOS flavours**, against the branch's 0/2 — 1/C(7,2) = 4.8% under
|
||||
an equal-rate model. That implicates the branch statistically. **The
|
||||
diff exonerates it mechanically**: grepping this lane's entire `src/`
|
||||
diff for `full_grid|resize|resync|Geometry|reconcile_panel_layout`
|
||||
matches an **import line and nothing else**, and
|
||||
`full_grid_resync_acceptance` (191 lines) has no panel, side-window,
|
||||
dedication, display or directory surface at all. Merged on that reading,
|
||||
with the equal-rate model itself in doubt — see the U4 row, and note a
|
||||
sixth base attempt reddened on a *third, unrelated* macOS selector
|
||||
(U8), which is what a background platform failure rate looks like.
|
||||
|
||||
**The original blocker:** the panel profile skipped checks 2–4 on the
|
||||
claim that a panel result never touches a document window. **Panel
|
||||
placement falls back to an ordinary document window** when the frontend
|
||||
is not panel-capable or its side slot is dedicated, so a `"panel"`
|
||||
commit could replace a **newer** document with every stale-intent guard
|
||||
skipped. Reproduced in review.
|
||||
|
||||
**Four designs, two rejected outright and one corrected — the sequence
|
||||
is the part worth not re-learning:**
|
||||
|
||||
1. **Revision 6 — predict at preflight.** Rejected: the `await` refusal
|
||||
stops concurrent interleaving, not the body, which is arbitrary
|
||||
synchronous Lua and can create the fallback itself.
|
||||
2. **Revision 7 — enforce at the placement boundary.** Implemented at
|
||||
`ca72461`, then rejected: `docs/agent-handoff.md:748` requires
|
||||
`commit_to` to preflight **before** the callback, because
|
||||
"validating at display time is four mutations too late". A body has
|
||||
already created buffers, handles and paint by then, so a
|
||||
placement-time refusal is a partial commit with an error return.
|
||||
3. **Revision 8 — keep the preflight, REFUSE the scope-invalidating
|
||||
mutation.** The shape the tree implements. Same as `Handle:await`
|
||||
being refused inside a commit scope: the fallback never comes into
|
||||
existence, and refusal stays mutation-free on `(false, reason)`.
|
||||
4. **Revision 9 — make the refusal hold for the WHOLE body.** Not a new
|
||||
shape; a correction to revision 8's scope. A nested `commit_to`
|
||||
**replaced** the enclosing contract and restored it afterwards, so
|
||||
an outer `"panel"` commit's restriction went out of force for the
|
||||
inner body's extent: nested `"document"` commit → callback dedicates
|
||||
the side slot, unrefused → outer commit resumes, falls back,
|
||||
overwrites a newer document. Reproduced in review. Contracts now
|
||||
**compose** — the core holds a stack, `commit_to` pushes and pops
|
||||
rather than swapping, and the guard consults every contract in force,
|
||||
so the strictest active restriction wins. Nesting itself is **not**
|
||||
forbidden: only the mutation is refused, so a nested commit that
|
||||
touches no dedication runs exactly as before. Detecting the
|
||||
dedication when the outer commit resumed was not available — that is
|
||||
a late refusal, which is what revision 7 was rejected for.
|
||||
|
||||
**WHAT REVISION 9 DID *NOT* INVALIDATE — read this before re-opening the
|
||||
enumeration.** The write-site enumeration below survived intact: every
|
||||
site is real, every one is still guarded, and review of the nesting
|
||||
defect found no missing route. What was wrong was the *surrounding*
|
||||
claim — that the guard was in force for the whole outer body. A complete
|
||||
list of write sites is not a complete argument until the guard's extent
|
||||
is stated too. The acceptance suite now drives the same rows at **two
|
||||
depths**, directly and through a nested `commit_to`.
|
||||
|
||||
**THE ENUMERATION IS THE LOAD-BEARING PART, AND IT IS CLOSED AS AN
|
||||
ENUMERATION OF WRITE SITES — for a structural reason, not because
|
||||
inspection ran out of ideas.** Full working in the framing §3; the short
|
||||
form:
|
||||
|
||||
- **Only two pieces of state can matter**, because `resolve_placement`
|
||||
reaches `Ordinary` from a side request through exactly two branches:
|
||||
`panel_capable`, and the one side window's `dedicated`.
|
||||
- **`panel_capable` is unreachable from a body.** It is written only
|
||||
where a `FrontendView` is constructed, and nothing in
|
||||
`src/lua_bindings/` constructs, registers or unregisters one —
|
||||
`register_frontend_view` has callers only in `daemon.rs` and core
|
||||
unit tests.
|
||||
- **Eight writes to `dedicated` exist** (`rg 'params\.dedicated\s*='
|
||||
src/`); **four are reachable and a fifth is guarded defensively** —
|
||||
`apply_placement`'s `Side` created / replacing / non-replacing arms
|
||||
and `set_params` are the reachable four, and `quit_window`'s
|
||||
`QuitAction::Restore` is the fifth, proved unreachable below and
|
||||
guarded anyway. **All five are guarded**, which is the count that
|
||||
matters; listing four under the word "five" is what an earlier version
|
||||
of this bullet did. Two `Ordinary` arms are harmless (their target is
|
||||
never a side window; one only ever clears the flag) and one is a unit
|
||||
test.
|
||||
- **The guards are sited where the property converges, not per caller.**
|
||||
All three `Side` arms are reached through `apply_placement`, which has
|
||||
**exactly one caller** — so one guard in `display_buffer` covers every
|
||||
request-driven dedication, including spellings that do not exist yet.
|
||||
`set_params` is a genuinely separate write and is guarded separately;
|
||||
dedication does **not** converge before the field itself, and that is
|
||||
stated rather than papered over.
|
||||
- **Closing the side window is NOT a route**, checked rather than
|
||||
assumed: with no side leaf `side_window_for` returns `None` and
|
||||
placement **creates** a fresh panel instead of falling back. Hiding is
|
||||
likewise irrelevant — `panel_hidden` is not consulted by placement.
|
||||
- **`quit_window`'s `QuitAction::Restore { dedicated: true }` is
|
||||
UNREACHABLE**, and this was the surprise. `Restore` is stored only on
|
||||
a *replacing* side placement, and a dedicated slot can never be the
|
||||
target of one. Guarded anyway, labelled defensive, because its
|
||||
unreachability is emergent from two rules in another function.
|
||||
- **What this does not rule out:** the enumeration is closed over the
|
||||
current tree, not future edits. `params.dedicated` is a public field,
|
||||
so nothing but the acceptance rows would catch a new direct writer.
|
||||
|
||||
**Also closed:** an invalid-UTF-8 profile (`string.char(255)`) reached
|
||||
`to_str()` and surfaced mlua's generic conversion error instead of the
|
||||
documented message naming the accepted values — the same reachability
|
||||
class as revision 5's `Option<String>` defect, one layer down. The
|
||||
comparison is on bytes now.
|
||||
|
||||
**Written with the lane's first commit**, per the standing correction
|
||||
from #171 and #215.
|
||||
|
||||
**Branch `destination-capture`**, base `githubsucks/main` @ `4bc55e8`
|
||||
(the #225 merge). **`githubsucks/destination-capture` is the
|
||||
authoritative tip** — the ref, not a SHA. Recover with
|
||||
`git fetch githubsucks && git checkout destination-capture`.
|
||||
|
||||
- **Framing `docs/destination-capture-framing.md`, revision 9.**
|
||||
Revisions 1–5 were approved over four review rounds; revisions 6–9 are
|
||||
corrections carrying the blocker above, and **revision 8's design as
|
||||
scoped by revision 9 is what the tree implements**. Revisions 6 and 7
|
||||
are described in that document as the record of why *not* those;
|
||||
neither is in the tree and neither should be restored from it.
|
||||
- **Implemented in four commits.** `779bb02` is the mechanism
|
||||
(`pmacs.window.capture_destination()`, the `ViewDestination` rename,
|
||||
the profile argument); `d5a6170` is
|
||||
`tests/destination_capture_acceptance.rs`; `469d5c8` is the
|
||||
revision-8 panel-profile correction plus the invalid-UTF-8 hole;
|
||||
`394fa43` is revision 9's contract stack and the commit below adds its
|
||||
cross-frontend pin. **15 pins**, and both preservation suites pass
|
||||
**unchanged** (journey 47, dired 31) — §7's stop signal not firing
|
||||
rather than being suppressed.
|
||||
- **HOW THE PANEL PROFILE IS ENFORCED, in one sentence so no earlier
|
||||
revision gets reinstated by someone reading only that document:** the
|
||||
preflight stays exactly where it was, and the mutations that would
|
||||
invalidate it are **refused at the attempt**.
|
||||
- `EditorCore::panel_commit_dedication_refusal` is the one rule. It
|
||||
fires while **any** `"panel"` `CommitContract` for this frontend is
|
||||
in force — every contract on the stack, not the innermost — and is
|
||||
consulted from `display_buffer` (before `apply_placement`, so a
|
||||
refused attempt mutates nothing), `pmacs.window.set_params` (before
|
||||
its borrow, so `fixed_rows` in the same table is not applied
|
||||
either), and `quit_window`.
|
||||
- **This is the same shape as `Handle:await` being refused inside a
|
||||
commit scope**, and for the identical reason: something that would
|
||||
invalidate the scope's guarantee is rejected outright rather than
|
||||
predicted around or caught late.
|
||||
- The contract (`CommitContract { destination, profile }`) rides on
|
||||
the core in a **stack**, pushed and popped by the **same**
|
||||
`ScopedFrontendGuard` that scopes the frontend, so a `"panel"`
|
||||
profile can never outlive the body that declared it. The field is
|
||||
private to the crate — Lua cannot claim a profile for a placement it
|
||||
did not commit to.
|
||||
- **A stack, not a slot, and the distinction is revision 9 (above).**
|
||||
The frontend override and the ambient frontend are *substitutions*,
|
||||
so a nested scope rightly replaces them; a contract is a
|
||||
*restriction*, and replacing one suspends it. The guard stores a
|
||||
depth and truncates back to it, so an inner exit removes exactly the
|
||||
contract it added and leaves every enclosing one in force.
|
||||
- **Matching is per FRONTEND as well as per profile, and that is a
|
||||
deliberate exception with its own positive pin.** A nested commit for
|
||||
a different frontend may dedicate *its* side slot: `resolve_placement`
|
||||
consults only the requesting frontend's `panel_capable` and its own
|
||||
one side window, so nothing done to B can change where A's side
|
||||
request lands. Pinned by
|
||||
`a_nested_commit_for_another_frontend_may_dedicate_its_own_slot`,
|
||||
which is the file's only row asserting that something is **allowed**
|
||||
— every other asserts a refusal, and an exception only the doc
|
||||
comment knows about is one review round from being simplified out.
|
||||
- **Prohibiting nested `commit_to` was the other candidate and was
|
||||
rejected.** It closes the hole by forbidding a construction no rule
|
||||
objects to — `commit_to` is public Lua API for saying where a
|
||||
continuation's result belongs, and a body committing to a second
|
||||
destination (a diff beside a status panel) is where #227's adoption
|
||||
is heading. Only the restriction needed preserving. **No Lua in the
|
||||
tree nests today** — `builtin/runtime/dired.lua` is the only
|
||||
`commit_to` consumer and it does not — so this is a decision about
|
||||
the API's future rather than about a live consumer, which is why it
|
||||
is recorded rather than left implicit.
|
||||
- **`panel_placement_can_fall_back` remains the preflight**, unchanged
|
||||
in role: it measures whether this frontend places side requests in
|
||||
the panel *right now*. With the invalidating mutations refused, that
|
||||
measurement stays true for the life of the body, which is what makes
|
||||
it a guarantee rather than a forecast.
|
||||
- The four document checks live once, in
|
||||
`EditorCore::document_destination_refusal`.
|
||||
- **Three deliberate limits**, each a different decision rather than a
|
||||
stricter version of this one: the **document profile is untouched**
|
||||
(constraining its body would newly refuse dired's own documented
|
||||
panel path — a preservation-suite stop signal); **dedicating a
|
||||
document window is still allowed** (it cannot change which of
|
||||
panel-or-document a side request resolves to); and **falling back is
|
||||
still allowed** — a frontend that cannot render a panel degrades
|
||||
gracefully exactly as today, because this refuses the mutation that
|
||||
*manufactures* a fallback, never the fallback itself.
|
||||
- **Mutation-checked per guard, and the pattern is the evidence the rows
|
||||
are independent rather than one assertion repeated.** Deleting the
|
||||
`display_buffer` guard fails the three `display{side, dedicated}` rows
|
||||
— verified **individually**, by rotating each to the front of the
|
||||
table, since the first failure otherwise masks the rest. Deleting the
|
||||
`set_params` guard fails only that row and leaves the display rows
|
||||
passing. Both leave every other test in the file green.
|
||||
- **Audit: nothing else relied on "a panel never touches a document".**
|
||||
Four doc sites repeated the claim (`ViewDestination`'s own doc twice,
|
||||
`capture_view_destination`, `ViewDestinationLua`) and were corrected;
|
||||
no other code depended on it. Dired — the only Lua `commit_to`
|
||||
consumer — takes the **two-argument document profile**, so all four
|
||||
checks already applied to it, and it separately documents and accepts
|
||||
the side-slot fallback (`builtin/runtime/dired.lua`).
|
||||
`compile.lua`'s `already_in_panel` queries live state rather than
|
||||
assuming, and the terminal adopter's rollback keys off
|
||||
`DisplayOutcome::created_side`, already false on a fallback.
|
||||
- **TWO FRAMING CLAIMS THE TREE DID NOT MATCH.** Neither changed a
|
||||
decision; both are recorded because the framing says "counted, not
|
||||
estimated" and a reader will check.
|
||||
1. **The rename was 11 references across 5 files, not 8 across 4.**
|
||||
`src/daemon.rs:1804` also calls the capture (the attaching
|
||||
frontend's directory open), and `editor.rs` holds six references
|
||||
rather than the counted total. Mechanical either way.
|
||||
2. **Q#DC-4's "a frontend with no document window" is a DEFENSIVE
|
||||
branch, not a routine one.** The obvious spelling — a frontend
|
||||
showing only a bottom panel — is asserted impossible: Q#BP6 says a
|
||||
layout always retains at least one non-side window, and
|
||||
`EditorCore::non_side_target` carries a `debug_assert!` that fires
|
||||
under `cargo test` when one does. So with Q#BP6 held a *registered*
|
||||
frontend always has a live document window. The decision still
|
||||
stands (capture stays total; an adopter with nowhere to land gets a
|
||||
refusal naming that rather than permission to fall back to ambient
|
||||
state), and the two Q#DC-4 pins drive the reachable spelling of the
|
||||
same condition — a layout whose document window has gone while the
|
||||
view remains. **#227 should not expect to hit this refusal**; it is
|
||||
insurance, not a path.
|
||||
- **Mutation-tested, since a matrix of deliberate omissions is exactly
|
||||
what passes vacuously.** Retyping the profile to `Option<String>`
|
||||
fails the table and boolean rows with mlua's conversion error (the
|
||||
number row survives — Lua coerces it — which is why the closed set is
|
||||
witnessed by more than one non-string). Applying all four checks in
|
||||
both profiles fails the panel column; applying only check 1 in both
|
||||
fails the document column. Defaulting an omitted profile to `"panel"`
|
||||
fails **`journey_acceptance`'s two preservation pins**, which is the
|
||||
contract claim being executable rather than asserted. Dropping the
|
||||
frontend scope for the panel profile fails the survives-a-switch pin's
|
||||
panel row; dropping the no-document-window arm fails the Q#DC-4 pair.
|
||||
|
||||
**Revision 8's four, each isolating a different way to get it wrong** —
|
||||
and the pattern of *which* rows survive each is the evidence the parts
|
||||
are independent rather than redundant:
|
||||
1. delete the `panel_commit_dedication_refusal` call from
|
||||
`display_buffer` → the three `display{side, dedicated}` rows fail,
|
||||
**verified individually** by rotating each to the front of the
|
||||
table so the first failure cannot mask the rest. Every other test
|
||||
passes — which is exactly the hole an implementation guarding only
|
||||
`set_params` would ship.
|
||||
2. delete it from `set_params` → **only** that row fails; the three
|
||||
display rows still pass.
|
||||
3. delete the `panel_placement_can_fall_back` arm from
|
||||
`commit_destination_refusal` → **only** the two pre-established
|
||||
fallback rows fail, which is the preflight half.
|
||||
4. make `panel_placement_can_fall_back` unconditionally `true` (the
|
||||
"widen the predicate" non-fix) → the really-lands-in-the-panel pin,
|
||||
the Q#DC-4 panel pin and the matrix's three panel rows all fail.
|
||||
That is the two profiles collapsing into one, made visible — the
|
||||
named fallback design, showing up as a test diff rather than
|
||||
silently.
|
||||
|
||||
And reverting the byte comparison to `to_str()?` fails the
|
||||
`invalid utf-8` row with mlua's conversion error, on content.
|
||||
|
||||
**Revision 9's two, each isolating a different half of the rule:**
|
||||
1. restore `panel_commit_dedication_refusal` to reading only the
|
||||
innermost contract (`.last()`, which is exactly revision 8's
|
||||
swapped slot) → **only**
|
||||
`a_nested_commit_cannot_mask_an_outer_panel_restriction` fails.
|
||||
Note the ordinary-nesting pin deliberately survives this — it
|
||||
exists to fail the *other* candidate fix (prohibit nesting), so the
|
||||
two are a pair rather than one test written twice.
|
||||
2. delete `&& contract.destination.frontend == fid` from the same
|
||||
scan, making any outer `"panel"` contract **globally** restrictive
|
||||
→ **only**
|
||||
`a_nested_commit_for_another_frontend_may_dedicate_its_own_slot`
|
||||
fails. Both single-frontend nesting tests pass under it, which is
|
||||
the evidence they are independent of the frontend match rather than
|
||||
merely looking so; the cross-frontend exception had no pin at all
|
||||
before this row, since every other test in the file drives one
|
||||
frontend.
|
||||
|
||||
Both were run across all three acceptance suites and the lib: in each
|
||||
case `journey_acceptance` (47), `dired_acceptance` (31) and
|
||||
`cargo test --lib` (1920) stay green, along with every other pin in
|
||||
this file.
|
||||
|
||||
**The counts above are journey 47 / dired 31**, matching the bullet
|
||||
further up. The mutation paragraph committed at `394fa43` had them
|
||||
**reversed** in both the ledger and that commit's message; the ledger
|
||||
is corrected here and the message is left as written, since rewriting
|
||||
a pushed commit is worse than a footnote. A reader following that SHA
|
||||
should take these numbers, not those.
|
||||
- **The public API #227 adopts against (Q#DC-5), pinned so it is a
|
||||
contract rather than an intention:**
|
||||
`pmacs.window.commit_to(dest, body [, profile])`. Profile is an
|
||||
optional trailing argument typed **`mlua::Value`, not
|
||||
`Option<String>`** — with `Option<String>` mlua rejects a number or
|
||||
table during argument *conversion*, before the closure runs, making
|
||||
the promised "accepted values are…" message unreachable. That is the
|
||||
same trap the existing binding documents for `dest`. Validated in the
|
||||
body against a **closed** set — `"document"` and
|
||||
`"panel"`. **Omitted means `"document"`**, so every existing
|
||||
two-argument caller keeps all four preflight checks *by definition of
|
||||
the signature*, which is what makes `journey_acceptance` passing
|
||||
untouched a consequence rather than a hope. An unrecognized or
|
||||
non-string profile **errors**, naming the accepted values — a silent
|
||||
fallback would hand a caller different checks than it asked for,
|
||||
which is the exact failure the parameterization exists to prevent.
|
||||
Git's mapping is settled here too: `*git-status*` → panel,
|
||||
`*git-diff*` → document. Revision 2 took three findings: Q#DC-2's parameterization was
|
||||
incomplete (a panel depends on **none** of checks 2–4, not just check
|
||||
3, so the question now carries a full preflight matrix with every
|
||||
omission testable); `tests/journey_acceptance.rs` joins dired as a
|
||||
**preservation suite and stop signal**, since it holds the
|
||||
`commit_to` scope, forged-userdata, preflight and restoration pins
|
||||
this lane generalizes; and the **coherence-impact section was missing
|
||||
entirely**, which `CLAUDE.md` and `COHERENCE.md` §25 both require.
|
||||
- **A PREREQUISITE LANE. PR #227 (git Stage 1) blocks on it.** #227's
|
||||
P1a review finding is why it exists: git's async completions mutate
|
||||
and display UI without capturing the initiating frontend
|
||||
(`builtin/runtime/git.lua:609`, `:854`), so a result surfaces in
|
||||
whichever frontend is active when git exits.
|
||||
- **The mechanism existed but was not Lua-reachable** until `779bb02`.
|
||||
`pmacs.window.commit_to` took a `DirectoryDestinationLua`, which is
|
||||
**nonconstructible from Lua** by design
|
||||
(`src/lua_bindings/mod.rs:4256`) and minted only inside the
|
||||
`path.open-directory` listener dispatch (`src/editor.rs:1311`) from a
|
||||
`pub(crate)` capture (`:1241`). So no async Lua continuation outside
|
||||
a directory open could say where its result belongs. Line numbers are
|
||||
the pre-lane ones, kept because they are what the finding was written
|
||||
against.
|
||||
- **Scope:** a Lua-reachable capture, a generic rename
|
||||
(`DirectoryDestination` → `ViewDestination`; the framing counted 8
|
||||
references across 4 files, the tree held **11 across 5** — see the
|
||||
finding above), and the preflight question below.
|
||||
**No adopter**: git's adoption is #227's work after this lands, since
|
||||
a prerequisite that converts its own first consumer cannot be
|
||||
reviewed separately from it.
|
||||
- **The substantive question (Q#DC-2)** is that git's two continuations
|
||||
differ in kind. `*git-status*` goes to the **bottom panel**
|
||||
(`listview.open` defaults `display` to `"panel"`,
|
||||
`builtin/runtime/listview.lua:550`); `*git-diff*` replaces a
|
||||
**document** window. `commit_to`'s stale-intent check (Q#JR14c) is
|
||||
right for the second and, *when the placement really is a panel*,
|
||||
irrelevant to the first. One shape over-refuses the panel or
|
||||
under-checks the document.
|
||||
|
||||
**DO NOT READ THE OLDER FORM OF THIS BULLET, WHICH SAID "the panel
|
||||
never touches the captured window's buffer".** That is the claim
|
||||
revisions 6–8 invalidate: panel placement **falls back** to an
|
||||
ordinary document window when the frontend is not panel-capable or
|
||||
its side slot is dedicated. The relaxation is conditional, and the
|
||||
mutations that could make it fall back are refused inside a
|
||||
panel-profile commit (revision 8) rather than predicted at preflight
|
||||
(revision 6) or caught at placement (revision 7, which would refuse
|
||||
after the callback had already mutated).
|
||||
- **Stop signal recorded in the framing:** if any existing dired test
|
||||
needs editing, the generalization changed Journey Stage 1a's
|
||||
semantics, and that is cause to stop rather than to adjust the test.
|
||||
- **Gates, as the executable line rather than a description:**
|
||||
|
||||
```
|
||||
scripts/gate --acceptance destination_capture_acceptance \
|
||||
--acceptance journey_acceptance \
|
||||
--acceptance dired_acceptance
|
||||
```
|
||||
|
||||
`--acceptance` is repeatable, so there is no reason for this ledger
|
||||
to say "plus dired's" and leave the reader to reconstruct it.
|
||||
**`journey_acceptance` and `dired_acceptance` are preservation suites
|
||||
and a STOP SIGNAL**: they carry the `commit_to` scope,
|
||||
forged-userdata, preflight and restoration pins this lane
|
||||
generalizes, and if either needs editing, the change altered Journey
|
||||
Stage 1a's semantics rather than closing a gap in them. No
|
||||
`--protocol` — core and Lua bindings only.
|
||||
|
||||
## Worker identity Stage 1 (§9) — MERGED as #232 (`3cc1b85`)
|
||||
## Worker identity Stage 1 (§9) — IMPLEMENTED, no PR yet
|
||||
|
||||
**Written with the lane's first commit**, per the standing correction
|
||||
from #171 and #215.
|
||||
|
|
@ -1489,7 +1001,9 @@ authoritative tip** — the ref, not a SHA. Recover with
|
|||
emission, an aborting runner, the build folded into `sweep-crdt`, and
|
||||
— added in the second round — a **rename of either** the build or the
|
||||
sweep step each fail the suite.
|
||||
|
||||
||||||| parent of 72bbb96 (docs: LSP LaTeX coverage framing revision 2, on a branch at last)
|
||||
||||||| parent of 312ec7a (docs: frame Discovery Stage 2 (revision 2) — M-x rows)
|
||||
||||||| parent of 8f86908 (docs: frame worker identity Stage 1 (revision 1))
|
||||
|
||||
## QoL arc retirement — PR #224 OPEN (docs only)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,20 +1,6 @@
|
|||
# Agent handoff — cross-machine continuity
|
||||
|
||||
**Last updated: 2026-08-11.** `main` is **`b867f64`** — git integration
|
||||
Stage 1 **#227** (`*git-status*` / `*git-diff*`, no wire change), atop
|
||||
`ae84d58` **#234**, the LSP file-watcher correctness fix (issue #233
|
||||
D1+D2, one review round; **D3 — the polling cost — is deliberately
|
||||
unfixed and is the ruled next lane**). Beneath them, in first-parent
|
||||
order: **#231** destination capture, **#232** worker identity Stage 1,
|
||||
**#228** discovery Stage 2, **#230** LSP LaTeX coverage, **#229** the
|
||||
gate `--protocol` build step, **#225** per-worktree gate target dirs,
|
||||
**#226** the R8 fixture fix, and **#224** the QoL docs retirement.
|
||||
**Only #227 and #234 are absorbed into §1 at this anchor**; the eight
|
||||
between carry their facts in their `docs/active-work.md` lanes, several
|
||||
of whose headers still say OPEN — trust this chain over any lane
|
||||
header, per the ledger's own rule.
|
||||
|
||||
Previously **2026-08-08**: `main` was `9a26ac8` — GPU horizontal
|
||||
**Last updated: 2026-08-08.** `main` is **`9a26ac8`** — GPU horizontal
|
||||
scroll **#223**, which **closes the QoL arc** (§1). Beneath it the arc's
|
||||
other four: **#222** TUI horizontal scroll, **#221** `ui.line-wrap` at
|
||||
protocol v22, **#220** GUI zoom, **#219** `full_grid` honored by the
|
||||
|
|
@ -100,94 +86,8 @@ reads it the way you just did.
|
|||
For volatile branches, checkpoints, verification, and recovery
|
||||
commands, read `docs/active-work.md` immediately after this file.
|
||||
|
||||
## 1. Where the project stands (2026-08-11)
|
||||
## 1. Where the project stands (2026-08-08)
|
||||
|
||||
- **Git integration Stage 1 — MERGED as #227 (2026-08-11).**
|
||||
`*git-status*` (a `listview` panel over `git --no-optional-locks -C
|
||||
<dir> status --porcelain=v2 --branch -z`) and `*git-diff*`
|
||||
(file-level, plain generated text), plus an install-once `keys`
|
||||
extension on `listview`. No wire change; **Stage 2 (gutter markers)
|
||||
needs new `DecorationKind` variants — a `PROTOCOL_VERSION` bump — and
|
||||
must be scheduled alone.** Held unmerged behind issue #233 by user
|
||||
ruling, then landed the same day as #234, refreshed and re-gated on
|
||||
the merged base. Durable facts:
|
||||
- **Capture at invocation, never read at continuation — enforced by
|
||||
ONE mechanism, not four counters.** Review found FOUR instances of
|
||||
the same shape (ordering by `rev-parse` completion; `state.root`
|
||||
read mid-plan; no generation on the diff path; one byte inside a
|
||||
fix). `new_channel()` tickets answer "is this still the request in
|
||||
force?" — **two channels, deliberately**: status and diff are
|
||||
independent things a user asks for, so each gets its own ordering
|
||||
and what is shared is the mechanism. The async-continuation census
|
||||
(three continuations, one dispatcher, one synchronous impostor)
|
||||
lives in the PR and framing.
|
||||
- **macOS cannot hold a non-UTF-8 filename, and this project will
|
||||
hit it again.** APFS/HFS+ reject invalid UTF-8 at the syscall
|
||||
(`EILSEQ`, errno 92); Linux's VFS treats names as opaque bytes. It
|
||||
cannot be reached around the filesystem either: an index-only
|
||||
entry still fails `git status`'s lstat with `EILSEQ`, which git
|
||||
skips — **there is no macOS arrangement in which real `git status`
|
||||
names a non-UTF-8 path.** The fix pattern: split coverage along
|
||||
the line the platform draws — behaviour runs everywhere (rows
|
||||
supplied through the `_deliver_status` seam), only *provenance* is
|
||||
Linux-gated. A latent sibling in `gpu_invocation_acceptance`'s
|
||||
crdt module is recorded in the git lane.
|
||||
- **Both root-parsing bugs lived in a pattern.** `rev-parse
|
||||
--show-toplevel` gets exactly ONE trailing `\n` removed, by an
|
||||
explicit last-byte test: `\r` is a legal POSIX name byte, and
|
||||
`rev-parse` has **no `-z`** — it echoes a literal `-z` onto stdout
|
||||
with exit 0, checked against the installed git rather than
|
||||
assumed. `first_line` was deliberately left alone both times: its
|
||||
three callers feed the single-line status band, where truncation
|
||||
is right.
|
||||
- **`{:?}` on a string containing NUL cannot build a `-z` fixture**
|
||||
— Lua's decimal escape swallows following digits, so payloads are
|
||||
assembled as raw bytes with three-digit escapes, joined in Lua
|
||||
with `string.char(0)`.
|
||||
- **A copy renders `copied from`, but `kind` stays `"rename"` for
|
||||
both, deliberately.** Every behaviour keyed on it is identical;
|
||||
splitting would force every present and future consumer to spell
|
||||
both arms, and a forgotten arm silently degrades copies. The score
|
||||
byte carries the distinction where presentation needs it.
|
||||
- **LSP file watcher — D1+D2 MERGED as #234 (2026-08-11); D3 is
|
||||
next.** Issue #233: with any server that dynamically registers
|
||||
`workspace/didChangeWatchedFiles`, plain-string globs never matched
|
||||
(matched **relative** where LSP says **absolute** — rust-analyzer
|
||||
saw no file change, ever; gopls saw `go.mod` but never `.go`), and
|
||||
re-registering a live id leaked the previous pollers uncancellably
|
||||
(rust-analyzer registers twice under one id: 12 pollers, 6
|
||||
unreachable). Invisible for three months until #232's activity
|
||||
indicator — §9's instrument doing exactly its job. Durable facts:
|
||||
- **The GlobPattern form travels with the pattern, and it is read
|
||||
FROM the pattern, not from the union arm.** `resolve_watcher`
|
||||
returns `(base, pattern, form)`; a leading `/` is what makes a
|
||||
string absolute. The first fix classified every string absolute —
|
||||
repairing rust-analyzer while silently breaking bare `*.txt`, a
|
||||
case that had worked since May. Review caught it (P1).
|
||||
- **A scan that completes after cancellation must not emit (P2).**
|
||||
The watcher coroutine spends most of a tick suspended in
|
||||
`read_dir` awaits with `_sleep` already cleared, so a cancel
|
||||
landing there had nothing to interrupt and the resumed scan
|
||||
emitted one stale batch under the superseded pattern. Cancellation
|
||||
and liveness are rechecked after the scan;
|
||||
`pmacs.lsp._after_scan_for_tests` (nil in production, handed the
|
||||
scan result) exists because no real timing produces that
|
||||
interleaving on demand — `git.lua`'s `_deliver_status` device
|
||||
again.
|
||||
- **F1's lesson fired twice in one lane.** The pre-existing test was
|
||||
insensitive (`**/` compiles to `.-`, which spans `/`, so it passes
|
||||
under either match subject) — and then the lane's own flat-pattern
|
||||
guard constrained the RelativePattern *object* arm while P1's
|
||||
regression lived in the *string* arm. A guard proves things about
|
||||
the arm it exercises, nothing more.
|
||||
- **All six watcher tests are mutation-verified, each bite failing
|
||||
only its own defect** — the two review fixes re-verified
|
||||
independently after review.
|
||||
- **D3 is deliberately unfixed and ruled next**: the walk still
|
||||
recurses into everything every 250 ms, six jobs per tick for
|
||||
rust-analyzer. The D3 lane in `docs/active-work.md` carries what
|
||||
was checked (no notify dependency, no ignore-list infrastructure)
|
||||
and the option space.
|
||||
- **QoL arc — CLOSED. All five stages merged (#219, #220, #221, #222,
|
||||
#223).** From one daily-driver report: terminal zoom broke TUI
|
||||
rendering and did nothing in the GUI, and a long line was unreadable
|
||||
|
|
|
|||
|
|
@ -693,40 +693,18 @@ not caused by the PRs they appeared on — that PR is **docs-only and its
|
|||
tree is byte-identical to a green `main`**. It is not evidence that any
|
||||
of them is harmless.
|
||||
|
||||
### U4 — `a_pty_resize_blanks_the_host_before_repainting`, macOS **both flavours**, three occurrences
|
||||
### U4 — `a_pty_resize_blanks_the_host_before_repainting`, macOS `lua54`, one occurrence
|
||||
|
||||
Surfaced on PR #229's CI; twice more on PR #231's.
|
||||
|
||||
**The `lua54` in this row's original title was wrong as a signature
|
||||
component, and matching on it would have missed two occurrences.** The
|
||||
row was filed from #229's single `lua54` red and recorded the flavour in
|
||||
the matching key. #231 then reddened the identical selector with the
|
||||
identical three fragments **twice on `luajit`** — so flavour is not part
|
||||
of this signature, and the row's own caution that "a deterministic
|
||||
defect *can* be Lua-flavour-specific" is now settled in the other
|
||||
direction: this one is not. Occurrence-keyed by suffix length, the three
|
||||
are `25 362` (#229, `lua54`), `25 222` (#231 attempt 1, `luajit`) and
|
||||
`25 054` (#231 attempt 2, `luajit`).
|
||||
|
||||
**A fourth sighting of these fragments was NOT an occurrence and must
|
||||
not be counted as one.** It came from a deliberate bite during this
|
||||
test's own development — the defect reintroduced on purpose (`consumer
|
||||
ignores full_grid`), 34 831 bytes, failing in 20.09 s. It earns its
|
||||
place here for what it proves instead: **the genuine defect and these
|
||||
CI reds are signature-indistinguishable**, same message class and same
|
||||
full-timeout duration, so the fragments alone can never tell a real
|
||||
resync failure from whatever this is.
|
||||
Surfaced on PR #229's CI.
|
||||
|
||||
| field | value |
|
||||
|---|---|
|
||||
| **selector** | `--test full_grid_resync_acceptance a_pty_resize_blanks_the_host_before_repainting` |
|
||||
| **job / flavor** | GitHub Actions, `Test (macos-latest / lua54)` **and** `Test (macos-latest / luajit)`, `macos-26-arm64`. **Flavour is not a matching key for this row** |
|
||||
| **job / flavor** | GitHub Actions, `Test (macos-latest / lua54)`, `macos-26-arm64` |
|
||||
| **required fragments** | `FG-INV: the post-resize resync must blank the host` · `no CSI 2 J appeared in the` · `bytes emitted after the first painted frame` |
|
||||
| **NOT fragments** | the byte count and the `:LINE` suffix are **occurrence-specific** and must not be matched on — the count is the collected suffix length, which varies per run, and the line moves with the file |
|
||||
| **status** | **three occurrences on two branches; INTERMITTENT on #229 (passed on rerun), NOT observed to pass on #231 (0/2)** |
|
||||
| **the #231 control experiment, and what it does and does not license** | Five valid observations at #231's exact base `0190102` — `run_attempt` 1, 2, 3, 4 and 6 — **all green on both macOS flavours**, against #231's 0/2. Under an equal-rate model the chance both failures land on the two branch runs is 1/C(7,2) = **4.8%**. Two things bound that number. First, **attempt 5 was discarded** because it reddened a *different* selector (U8) — so the base leg is 5/5 green *for this signature* and 5/6 overall, and "the base never fails" is not what was observed. Second, three unrelated macOS selectors reddening in one session is **a background platform failure rate**, and the equal-rate model the 4.8% assumes is exactly what such a rate violates. **The branch side was never resampled**: 5-vs-2 is an asymmetric experiment, and rerunning #231's failing job three more times at `4654b94` was the outstanding discriminator when it merged |
|
||||
| **why #231's diff is excluded** | grepping its **entire** `src/` diff for `full_grid\|resize\|resync\|Geometry\|reconcile_panel_layout` matches **one import line** and nothing else; all 721 changed lines are placement, dedication and commit-contract logic. From the other side, `full_grid_resync_acceptance` (191 lines) contains no panel, side-window, dedication, display or directory surface — grep for those matches only a comment about CSI 2 J. #231 merged on this reading **over** the statistical signal above, which is a judgement recorded here so that a fourth occurrence can revisit it rather than re-derive it |
|
||||
| **why #229's diff is excluded** | #229 changes only `scripts/gate`, `tests/gate_script_acceptance.rs` and documentation — **no `src/`, and the workflow never invokes `scripts/gate`**. Decisively, `full_grid_resync_acceptance` runs **before** the changed gate suite, so even a cross-suite leaked-state path is not available. The `luajit` leg passing on the same commit is **corroboration only** — a deterministic defect *can* be Lua-flavour-specific, so that observation must not be used as a structural exclusion |
|
||||
| **status** | **one occurrence; INTERMITTENT — passed on rerun** |
|
||||
| **why the diff is excluded** | #229 changes only `scripts/gate`, `tests/gate_script_acceptance.rs` and documentation — **no `src/`, and the workflow never invokes `scripts/gate`**. Decisively, `full_grid_resync_acceptance` runs **before** the changed gate suite, so even a cross-suite leaked-state path is not available. The `luajit` leg passing on the same commit is **corroboration only** — a deterministic defect *can* be Lua-flavour-specific, so that observation must not be used as a structural exclusion |
|
||||
| **what IS established** | **no blank was OBSERVED after the mark** within the test's fixed 20-second deadline. The collected suffix was the **entire** post-mark output (`suffix.len()`, 25 362 bytes on this occurrence — not a capped window; only the *displayed* head is truncated to 400 bytes), and that head shows ordinary repaint traffic (`ZQXMARKERQZ` rows with SGR + CUP), so the host was painting |
|
||||
| **what is NOT** | any mechanism. Whether the blank was never emitted, emitted after the deadline, or lost in transport is **open** — and "it never emitted the blank" is a claim this evidence does not support. **The failing run's ~20 s duration is the fixed `Duration::from_secs(20)` timeout**, so the spread against a fast passing run is mechanically determined and is **not** independent timing evidence |
|
||||
| **discriminating control — ASYMMETRIC, and only one direction concludes** | the suffix is already complete, so "capture more bytes" is not the gap — arrival time is. Extending the deadline and recording whether `CLEAR_ALL` arrives, and at what offset: **if it arrives, "emitted late" is established.** **If it does not, that establishes only "not observed by the longer deadline"** — *not* "never emitted", because transport loss produces the same absence. Separating non-emission from transport loss needs **producer-side emission evidence** (did pmacs write the clear?) cross-checked against the collected stream; no deadline, however long, can do it alone |
|
||||
|
|
@ -749,53 +727,3 @@ incident, not U4 occurring twice**.
|
|||
| **exclusion strength — WEAKER than U4's, deliberately** | the changed `gate_script_acceptance` ran **earlier in the same job**, and it creates worktrees and directories. No leaked child or persistent signal-state mutation was observed, but "the diff touches no `src/`" is **not** the argument here that it is for U4, because cross-suite leaked state is a path reachability reasoning does not close |
|
||||
| **control 1 — CROSS-SUITE ATTRIBUTION, and asymmetric** | run `m5_8_acceptance` alone on macOS `lua54`, without the gate suite ahead of it. **A matching isolated RED proves the gate suite is not necessary** for the failure. **An isolated GREEN proves nothing beyond that run** — the failure is intermittent, so absence under one run is not evidence of dependence. It also does **not** discriminate among the three mechanisms in either direction |
|
||||
| **control 2 — mechanism** | observe **readiness and raw-mode state at the moment of injection**. Another isolated pass, however many times repeated, cannot separate "injected before raw mode" from "raw mode lost" from a third cause |
|
||||
|
||||
### U8 — `acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel`, macOS `luajit`, one occurrence, **fragments destroyed**
|
||||
|
||||
**Numbered U8 deliberately: U6 and U7 are reserved** for the two
|
||||
wall-clock rows on `worker-identity-stage1` (PR #232), which renumbered
|
||||
into that range when #229 took U4/U5. Taking U6 here would recreate the
|
||||
duplicate-id collision that rebase already produced once.
|
||||
|
||||
**This row exists mostly as an admission.** It surfaced on attempt 5 of
|
||||
a merge-base control at `0190102`, and **I reran the job before reading
|
||||
its log**, which discarded it. GitHub keeps only the latest attempt's
|
||||
logs for a rerun job. So this is U2's original condition exactly — a
|
||||
selector with no fragments, unmatchable — and it was produced by the
|
||||
very mistake U3 is named for.
|
||||
|
||||
| field | value |
|
||||
|---|---|
|
||||
| **selector** | `--test bottom_panel_stage1_acceptance acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel` |
|
||||
| **job / flavor** | GitHub Actions, `Test (macos-latest / luajit)`, at base `0190102`, control attempt 5 |
|
||||
| **required fragments** | **NONE CAPTURED — destroyed by rerunning the job before reading its log.** Recovery attempted via the jobs API and the attempt-scoped jobs endpoint; the log is gone |
|
||||
| **what IS established** | it failed once (`46 passed; 1 failed`), panicking at `tests/bottom_panel_stage1_acceptance.rs:2454`, on the **exact merge base** — so it is not attributable to any open branch |
|
||||
| **what is NOT** | everything else. Without the assertion text this cannot be matched against a future occurrence, which is the whole purpose of a row here |
|
||||
| **why it matters anyway** | it is the **third distinct macOS selector** to red in one session, after U4 (`full_grid_resync`) and U5 (`ctrl_c_during_reconnect`). Three unrelated selectors failing on the macOS legs suggests a **background failure rate on that platform** rather than three independent test bugs — and that materially affects any equal-rate reasoning about which branch a failure "landed on" |
|
||||
| **next occurrence** | **read the log BEFORE rerunning anything.** That is U3's stated lesson and this row is its fourth violation |
|
||||
|
||||
### U9 — a PTY test and a budget test red **together** in one `11-sweep`, with an in-run control
|
||||
|
||||
Recorded on the `destination-capture` merge tree, 2026-08-10, in the
|
||||
gate run that was meant to clear PR #231.
|
||||
|
||||
**This row's value is its control, not its selectors.** U6 and U7 could
|
||||
only compare a red run against a *different* run. Here both selectors
|
||||
ran green **inside the same gate invocation**, minutes earlier, on the
|
||||
same tree and machine — `03-lib` (1928 passed, 0 failed) and
|
||||
`04-lib-crdt` (2113 passed, 0 failed) — and then failed in `11-sweep`.
|
||||
Whatever this is, it is not the tree.
|
||||
|
||||
| field | value |
|
||||
|---|---|
|
||||
| **selector** | `--lib process::tests::m6_1_pty_canonical_mode_keeps_kernel_echo` **and** `editor::tests::composition_overhead_under_ten_percent`, failing in the same `11-sweep` step |
|
||||
| **job / flavor** | local (Linux), `scripts/gate` step `11-sweep` (`cargo test --workspace --no-fail-fast -- --skip basedpyright`), fresh per-lane target dir, no sibling worktrees building |
|
||||
| **required fragments** | ``canonical mode should leave echo enabled (no `-echo` flag); stty -a output was: ""`` **and** `composition machinery added more than 10% overhead` |
|
||||
| **NOT fragments** | the measured numbers (`1.613`, `single=191935 ns`, `dispatch=309602 ns`) and every `:LINE` suffix — occurrence-specific |
|
||||
| **status** | **one occurrence; INTERMITTENT — the identical sweep command on the same tree was green (118 targets, 1928 passed, exit 0)** |
|
||||
| **what IS established** | intermittence, with the strongest available exclusion of the tree: green in two earlier steps of the **same run**, green isolated afterwards (`2 passed`, 1.70 s), green on a full sweep rerun. Both assertions are **timing-sensitive by construction** — one reads collected child output within a deadline, the other measures wall-clock composition overhead (observed 1.613× against a 1.10× budget; 61.3% dispatch and 124.6% realistic overhead) |
|
||||
| **what is NOT** | cause, and the load confound is **partially measured but NOT controlled**. The failing sweep ran inside a full gate; the green rerun started at load average 1.98 with the 5-minute figure still at 8.03 from that gate. Different conditions is not a measurement of the mechanism, and this row does not treat it as one |
|
||||
| **the structural difference worth testing next** | `cargo test --workspace` runs **many test binaries concurrently**; `--lib` runs **one**. That is a difference in kind between the passing steps and the failing one, not merely a difference in load average — and it is the first candidate this family has had that is checkable rather than atmospheric. **Discriminating control:** rerun the sweep with test-binary concurrency pinned to 1, and separately run the `--lib` binary alone under synthetic load. A red under synthetic load at low sweep concurrency implicates load; a red at high concurrency and low load implicates the concurrency itself |
|
||||
| **relation to U2 — a NEAR MISS, do not match it there** | the PTY fragment is U2's exact family (`stty -a output was: ""`), but U2's selector field names only `m6_1_pty_raw_mode_disables_kernel_echo`. U2's occurrence 2 saw raw **and** canonical fail together; here **canonical redded alone and raw passed**, which U2's evidence has never shown. It is recorded here rather than folded into U2 so that the "canonical alone" case stays visible |
|
||||
| **relation to U6 — its own instruction, honoured** | `composition_overhead_under_ten_percent` is one of U6's two selectors, and U6 says plainly: "If a future run reds **one** of these without the other, that is a different incident and should be judged as one." It redded without `criterion_1_end_of_line_typing…`, in a different step, at a far larger margin (1.613× here against U6's 1.297×). Judged as a different incident, as instructed |
|
||||
| **what this row does NOT assert** | that the two selectors share a mechanism. They failed together once; they belong to different subsystems; and U7 already refused this exact merge for U6. The **co-failure inside one step with an in-run green control** is the signature — not either name, and not a shared cause |
|
||||
|
|
|
|||
|
|
@ -1,816 +0,0 @@
|
|||
# A destination capture any async continuation can use
|
||||
|
||||
**Status: revision 9. The mechanism is implemented at `0efc8c0`; the
|
||||
correctness blocker revisions 6–9 carry is IMPLEMENTED, in revision 8's
|
||||
shape with revision 9's scope correction, and §3's enumeration is
|
||||
performed and recorded below.** Revisions 6 and 7 proposed fixes that
|
||||
review rejected; **neither is in the tree**, and the two paragraphs
|
||||
describing them are kept as the record of why this shape and not those.
|
||||
|
||||
*(Revisions 2–5 said "Pre-implementation. Awaiting approval" while the
|
||||
ledger recorded the lane as approved and implemented. Same
|
||||
contradiction class this document keeps correcting elsewhere, left
|
||||
standing in its own header.)*
|
||||
|
||||
**Revision 9 fixes a hole in revision 8's guard — one that is about the
|
||||
guard's SCOPE, not about which mutations it names.** Revision 8 refuses,
|
||||
inside a `"panel"` commit, the mutations that would make its relaxed
|
||||
preflight wrong. But a **nested `commit_to` REPLACED** the enclosing
|
||||
contract with its own and restored it afterwards (`src/editor.rs:129`,
|
||||
`src/lua_bindings/window_panel.rs`), so the outer restriction went out of
|
||||
force for the whole of the inner body. Review reproduced the sequence:
|
||||
an outer `"panel"` commit passes the relaxed preflight; a nested
|
||||
`"document"` commit masks its contract; the nested callback dedicates the
|
||||
side slot and **is not refused**; the outer commit resumes, its side
|
||||
request falls back, and it overwrites a newer document — the original
|
||||
P1a failure, reached through one extra call.
|
||||
|
||||
**What this invalidated, precisely.** *Not* §3's enumeration of
|
||||
dedication write sites. That enumeration was performed against the tree,
|
||||
it is still complete, and every site in it that can dedicate the slot is
|
||||
still guarded. What was wrong was the surrounding claim — that the guard
|
||||
was **in force for the whole outer body**. §3's "PREFLIGHT STAYS WHERE
|
||||
IT IS" paragraph and the enumeration that follows it are therefore kept
|
||||
and **qualified**, not withdrawn.
|
||||
|
||||
**The fix: contracts COMPOSE across nested scopes; the strictest active
|
||||
restriction wins.** The core holds a *stack* of contracts rather than one
|
||||
slot: `commit_to` pushes and pops rather than swapping, and the
|
||||
dedication guard consults **every** contract in force rather than the
|
||||
innermost. Matching stays per frontend, so a nested commit for a
|
||||
different frontend may still dedicate *its* side slot — that cannot
|
||||
change where this frontend's side request lands. The alternative shape,
|
||||
**prohibiting nested `commit_to` outright**, was rejected: it closes the
|
||||
hole by forbidding a construction no rule objects to. `commit_to` is
|
||||
public Lua API for saying where a continuation's result belongs, and a
|
||||
body that commits to a second destination (a diff beside a status panel)
|
||||
is where #227's adoption is heading. Only the *restriction* needed
|
||||
preserving. **Detecting the dedication when the outer commit resumed was
|
||||
not available**: by then the mutation has happened, which is a late
|
||||
refusal, which is what revision 7 was rejected for.
|
||||
|
||||
**Revision 8 rejects BOTH of the previous two fixes and takes a third
|
||||
shape.** Revision 6 predicted the fallback at preflight (the body can
|
||||
change it). Revision 7 moved enforcement to the placement boundary —
|
||||
which **breaks the invariant `commit_to` exists for**:
|
||||
`docs/agent-handoff.md:748` says it preflights *before* the callback
|
||||
because "validating at display time is four mutations too late", so a
|
||||
placement-time refusal arrives after arbitrary Lua has created buffers,
|
||||
handles and paint. Revision 8 keeps the preflight and **refuses the
|
||||
mutations that would invalidate it**, the same shape as the existing
|
||||
await refusal. Refusal stays mutation-free on the `(false, reason)`
|
||||
path.
|
||||
|
||||
**Revision 6 fixes an UNSOUND matrix, not a preference.** Q#DC-2 gave
|
||||
the panel profile only check 1, on the claim that a panel result never
|
||||
touches a document window. **Panel placement falls back to an ordinary
|
||||
document window** when the frontend is not panel-capable or its side
|
||||
slot is dedicated — so a `"panel"` commit could replace a *newer*
|
||||
document while skipping every stale-intent guard. Reproduced in review.
|
||||
The relaxation is now conditional on the placement really being a
|
||||
panel. Revision 6 also closes an invalid-UTF-8 hole in the profile
|
||||
diagnostic — the same reachability class as revision 5's, one layer
|
||||
down.
|
||||
|
||||
**Revision 5 fixes a binding-level contradiction in revision 4's own
|
||||
API spec.** It required `profile: Option<String>` *and* a pointed error
|
||||
naming the accepted values for a non-string — but mlua rejects a
|
||||
number or table during argument conversion, before the closure runs, so
|
||||
that message was unreachable. This is the exact trap the existing
|
||||
binding documents for `dest`, in a comment revision 4 quoted while
|
||||
repeating the mistake one argument to the right. The profile is now
|
||||
`mlua::Value`, validated in the body, with `nil` and absence both
|
||||
meaning `"document"`.
|
||||
|
||||
**Revision 4 specifies the call shape the last two revisions kept
|
||||
referring to without defining.** "The profile is declared at
|
||||
`commit_to`" named no signature, no value set, no invalid-profile
|
||||
behaviour, and nothing about the existing two-argument callers — so
|
||||
#227 had no stable API to adopt and the Journey preservation promise
|
||||
rested on care rather than contract. Q#DC-5 fixes that:
|
||||
`commit_to(dest, body [, profile])`, a **closed** two-value set,
|
||||
**omitted means `"document"`** so every existing call keeps all four
|
||||
preflight checks by definition, and an unrecognized profile **errors**
|
||||
rather than falling back.
|
||||
|
||||
**Revision 3 decides Q#DC-4, which revision 2 left contradicting
|
||||
Q#DC-2 — on the primary panel API.** Q#DC-2 concluded a panel needs
|
||||
only a live frontend; Q#DC-4 still returned `nil` without a document
|
||||
window and told git to fall back to ambient behaviour, which is the
|
||||
very bug this lane removes. Resolved: the destination's document pair
|
||||
is **optional**, `capture_destination()` is **profile-blind and
|
||||
argument-free**, the profile is declared at `commit_to`, and a
|
||||
document-profile commit without a document pair is refused. §4 and
|
||||
Q#DC-1 were updated to match rather than left to disagree.
|
||||
|
||||
**Revision 2 takes three review findings.** Q#DC-2's parameterization
|
||||
was **incomplete** — a panel result does not depend on the captured
|
||||
document window being live or non-dedicated either, not just on its
|
||||
buffer, so the question now carries a full **preflight matrix** with
|
||||
every omission testable. `tests/journey_acceptance.rs` joins dired as a
|
||||
named **preservation suite and stop signal**; it carries the
|
||||
`commit_to` scope, forged-userdata, preflight and restoration pins this
|
||||
lane generalizes, and Journey Stage 1a's framing treats it as a
|
||||
required gate. And **§5 (coherence impact) was missing entirely**,
|
||||
which `CLAUDE.md` and `COHERENCE.md` §25 both require of
|
||||
coherence-affecting work — this lane adds Lua API surface and
|
||||
generalizes a Journey substrate, so it qualifies twice over.
|
||||
|
||||
**A prerequisite lane. PR #227 (git Stage 1) blocks on it**, and its
|
||||
P1a review finding is the reason this exists.
|
||||
|
||||
---
|
||||
|
||||
## 1. Why, and why as its own lane
|
||||
|
||||
PR #227's review found that git's async completions mutate and display
|
||||
UI without capturing the initiating frontend
|
||||
(`builtin/runtime/git.lua:609`, `:854`), so a result can surface in
|
||||
whichever frontend happens to be active when git exits. Run
|
||||
`git.status` in frontend A, let frontend B become active, and A's panel
|
||||
opens in B.
|
||||
|
||||
**The finding named the right mechanism.** `pmacs.window.commit_to`
|
||||
exists for exactly this continuation boundary: Journey Stage 1a's
|
||||
Q#JR14 built it because "the listing settles a tick or more later, and
|
||||
by then the ambient frontend, selected window, and active buffer may
|
||||
all name something else" (`src/editor.rs:1238-1240`).
|
||||
|
||||
**But it is not reachable from Lua outside one path**, which is why
|
||||
this is a lane and not a line in #227:
|
||||
|
||||
- `commit_to` takes a `DirectoryDestinationLua`, **nonconstructible
|
||||
from Lua** by deliberate design (`src/lua_bindings/mod.rs:4256`) —
|
||||
userdata with no constructor and no setters, so a caller cannot
|
||||
fabricate a plausible triple.
|
||||
- The only site that mints one is inside the `path.open-directory`
|
||||
listener dispatch (`src/editor.rs:1311`), from
|
||||
`capture_directory_destination`, which is `pub(crate)`
|
||||
(`src/editor.rs:1241`).
|
||||
|
||||
So any async Lua continuation that is **not** a directory open has no
|
||||
way to say where its result belongs. Git is the first to need it; it
|
||||
will not be the last.
|
||||
|
||||
Landing this inside #227 would put new Lua API surface, over another
|
||||
lane's merged mechanism, inside a feature branch — the same folding
|
||||
that was declined for the `scripts/gate` repair, for the same reason.
|
||||
|
||||
## 2. Ground truth
|
||||
|
||||
- **The captured data is already generic.**
|
||||
`DirectoryDestination { frontend, window, buffer }`
|
||||
(`src/editor_core.rs:159-166`) contains nothing directory-specific.
|
||||
Only its **name** and its **capture site** are.
|
||||
- **The blast radius of a rename is small**: 8 references across 4
|
||||
files (`editor_core.rs`, `editor.rs`, `lua_bindings/mod.rs`,
|
||||
`lua_bindings/window_panel.rs`). Checked, not estimated.
|
||||
- **`commit_to`'s preflight is four checks**
|
||||
(`src/lua_bindings/window_panel.rs:488-525`), in order: the
|
||||
requesting frontend still has a layout; the destination window is
|
||||
still live in it; **the window still shows the captured buffer**
|
||||
(Q#JR14c stale intent); and the window is not dedicated (Q#JR14f).
|
||||
- **`Handle:await` refuses inside a commit scope**
|
||||
(`builtin/runtime/async.lua:87-90`) — yielding would restore the
|
||||
scope while the coroutine is still parked. Any adopter awaits
|
||||
*before* committing, as dired does.
|
||||
- **Git's two continuations do not have the same shape**, and this is
|
||||
the finding that shapes the design:
|
||||
- `*git-status*` goes through `listview.open`, which resolves
|
||||
`display` with a **`"panel"`** default
|
||||
(`builtin/runtime/listview.lua:550`). It **requests** the bottom
|
||||
panel rather than a document window — *requests*, because a side
|
||||
request FALLS BACK into a document window on a frontend that is not
|
||||
`panel_capable` or whose one slot is dedicated elsewhere. That
|
||||
fallback is this lane's blocker; §3 and Q#DC-2 carry it.
|
||||
- `*git-diff*` calls `pmacs.window.display(buf, { select = true })`
|
||||
— the **document** target, deliberately, "so the status panel it
|
||||
was invoked from stays visible beside it"
|
||||
(`builtin/runtime/git.lua:852-854`).
|
||||
|
||||
## 3. The tension this lane has to resolve
|
||||
|
||||
`DirectoryDestination.buffer` exists for one purpose, stated at its
|
||||
definition: *"what that window held at capture time, so **stale intent
|
||||
loses to the user**"* — a user who replaced the buffer while work was
|
||||
in flight is newer information than the request.
|
||||
|
||||
**That predicate is right for a document replacement and wrong for a
|
||||
panel — WHILE THE PANEL REALLY IS A PANEL, which is the qualification
|
||||
the rest of this document exists to add.** A git status panel that
|
||||
lands in the bottom panel does not replace the captured window's
|
||||
buffer; it opens beside it. Refusing to show it because the user
|
||||
switched files in the document window would be a refusal with no
|
||||
relationship to what the continuation actually does, and that case
|
||||
would inherit a check about a window it never touches.
|
||||
|
||||
**Read the previous paragraph with its condition attached, not as a
|
||||
standing fact.** Panel placement **falls back** into an ordinary
|
||||
document window when the frontend is not `panel_capable` or its one
|
||||
side slot is dedicated elsewhere — and then the panel case *does* touch
|
||||
the captured window, replacing whatever the user put there. That
|
||||
fallback is this lane's correctness blocker, and the unqualified
|
||||
version of this claim is precisely what made revision 5's matrix
|
||||
unsound. The resolution is below, at the end of Q#DC-2: the preflight
|
||||
measures whether this frontend places side requests in the panel, and
|
||||
the mutations that would falsify that measurement mid-commit are
|
||||
refused.
|
||||
|
||||
Meanwhile the diff case *is* a document replacement, and wants exactly
|
||||
the dired semantics.
|
||||
|
||||
So a single one-size destination either **over-refuses** the panel case
|
||||
or **under-checks** the document case. Q#DC-2 is where that gets
|
||||
decided, and it is the substance of this lane.
|
||||
|
||||
## 4. The change, in outline
|
||||
|
||||
- **A Lua-reachable capture**, returning the same nonconstructible
|
||||
userdata for the *current* frontend, **with** its document window and
|
||||
buffer when it has one and without them when it does not (Q#DC-4).
|
||||
The capture takes no arguments and is profile-blind; the profile is
|
||||
declared at `commit_to`.
|
||||
- **Generic naming.** `DirectoryDestination` becomes something that
|
||||
does not lie about a git panel; `capture_directory_destination` and
|
||||
the userdata type follow. 8 references (§2).
|
||||
- **The directory path keeps behaving exactly as it does today** — this
|
||||
lane generalizes the capture, it does not change Journey Stage 1a's
|
||||
semantics.
|
||||
- **No adopter in this lane.** Git's adoption is #227's, after this
|
||||
lands. A prerequisite that also converts its first consumer makes the
|
||||
two impossible to review separately.
|
||||
|
||||
## 5. Coherence impact (§20)
|
||||
|
||||
**Revision 1 omitted this section entirely, and it is required.**
|
||||
`CLAUDE.md` and `COHERENCE.md` §25 both say a framing for
|
||||
coherence-affecting work must cite the section it serves and state its
|
||||
impact — and this lane adds **new Lua API surface** and generalizes a
|
||||
Journey-substrate mechanism, which is coherence-affecting on both
|
||||
counts. Recording the impacts as neutral where they are neutral is part
|
||||
of the requirement, not a way around it.
|
||||
|
||||
- **§16 semantic frontend — the section this serves.** The defect it
|
||||
removes is a continuation resolving its target from *ambient* state a
|
||||
tick after the request, which is precisely the multi-frontend
|
||||
correctness §16 exists to protect. A capture makes "which frontend
|
||||
asked" a value rather than a guess.
|
||||
- **§14 workbench primitives — indirect, and the honest framing is
|
||||
*enabling*.** This does not add a primitive. It removes the reason an
|
||||
async adopter would hand-roll frontend tracking, which is the
|
||||
mechanism by which primitives acquire per-consumer idiosyncrasies.
|
||||
- **Journey steps touched: none directly, one PROTECTED.** The golden
|
||||
journey does not gain a step. But Journey Stage 1a's Q#JR14 substrate
|
||||
is what this generalizes, and §7 makes `tests/journey_acceptance.rs`
|
||||
a preservation suite precisely so a generalization cannot erode the
|
||||
step it came from.
|
||||
- **Interaction islands (§6): none added.** No key interception, no
|
||||
dispatch precedence rung. `dispatch_key` is untouched.
|
||||
- **Config registry: no setting.** Where a continuation lands is a
|
||||
correctness property, not a preference, and a toggle would offer to
|
||||
turn correctness off.
|
||||
- **Background-work attribution (§9): NEUTRAL, and worth stating
|
||||
precisely rather than skipping.** This lane adds no background work
|
||||
and no new unattributable surface. It also does **not** improve §9 —
|
||||
knowing which frontend a result belongs to is not knowing who asked
|
||||
for it or why. That is the worker-identity lane's arc, and the two
|
||||
should not be confused because both concern async continuations.
|
||||
- **§10 extension trust — a small positive.** The capture keeps the
|
||||
Q#JR14d property that a destination is **nonconstructible from Lua**,
|
||||
so generalizing the mechanism does not widen what extension code can
|
||||
fabricate. §7 re-asserts the forged-destination refusal after the
|
||||
rename for exactly this reason.
|
||||
|
||||
## 6. Open questions
|
||||
|
||||
### Q#DC-1 — what does the capture take as arguments?
|
||||
|
||||
*My vote: **no arguments** — capture the acting frontend and its
|
||||
document window from the ambient state at call time.* That is what the
|
||||
existing `capture_directory_destination(frontend, window)` is handed by
|
||||
its one caller, and a Lua-supplied frontend id would reintroduce the
|
||||
fabrication hole the userdata design closes.
|
||||
|
||||
### Q#DC-2 — one destination shape, or a panel/document distinction? **(the substantive one)**
|
||||
|
||||
§3 is the problem. Three candidates:
|
||||
|
||||
1. **One shape, all four checks.** Simplest; over-refuses the panel
|
||||
case, and the refusal reason would be about a window the panel does
|
||||
not touch.
|
||||
2. **One shape, preflight parameterized by the continuation** — the
|
||||
caller declares whether it is replacing the captured window's
|
||||
buffer, and the stale-intent check applies only then.
|
||||
3. **Two capture kinds**, document and panel, with different preflights.
|
||||
|
||||
*My vote: **(2)***, with the profiles spelled out below rather than
|
||||
left to implementation.
|
||||
|
||||
**Revision 1 said only "skip the stale-buffer check for a non-replacing
|
||||
continuation", and that was incomplete.** Review is right: a panel
|
||||
result does not depend on the captured **document window** at all. It
|
||||
does not replace that window's buffer, so check 3 is irrelevant; it
|
||||
does not occupy that window, so check 4 (dedicated) is irrelevant; and
|
||||
it does not need that specific window to exist, so check 2 is
|
||||
irrelevant. Retaining any of the three can reject `git.status` for a
|
||||
document-window change that has nothing to do with where the panel
|
||||
goes. But dropping them **without an explicit profile** is how document
|
||||
replacement quietly loses its guarantees.
|
||||
|
||||
**The matrix, stated so every omission is deliberate and testable:**
|
||||
|
||||
| # | Precondition (`window_panel.rs:488-525`) | Document replacement | Frontend/panel scope |
|
||||
|---|---|---|---|
|
||||
| 1 | Requesting frontend still has a layout | **required** | **required** |
|
||||
| 2 | Destination window still live in it | **required** | not applicable |
|
||||
| 3 | Window still shows the captured buffer (Q#JR14c stale intent) | **required** | not applicable |
|
||||
| 4 | Window is not dedicated (Q#JR14f) | **required** | not applicable |
|
||||
|
||||
**Check 1 is the entire panel profile ONLY WHEN THE PLACEMENT REALLY IS
|
||||
A PANEL — revision 5's matrix was unsound, and this is the correction.**
|
||||
|
||||
The matrix rested on "the panel never touches the captured window's
|
||||
buffer". **That is false when panel placement falls back.**
|
||||
`editor_core.rs:4138-4148` says so in its own comment: *"Reaching
|
||||
`Ordinary` while a side was REQUESTED means the request fell back (not
|
||||
panel-capable, or the one slot is dedicated elsewhere)"* — and the
|
||||
result is then installed into an ordinary **document** window. So a
|
||||
`"panel"` commit on a non-panel-capable frontend replaces a document
|
||||
view while skipping every check that exists to stop it replacing a
|
||||
*newer* one. That reintroduces exactly the stale-intent failure the
|
||||
API was built to prevent, which makes it a correctness defect and not
|
||||
a strictness preference.
|
||||
|
||||
**The rule, restated:** the panel profile's relaxation is conditional
|
||||
on the placement actually being a panel. Whenever placement **can**
|
||||
fall back to a document window, the panel profile runs the **full
|
||||
document preflight**.
|
||||
|
||||
**PREFLIGHT STAYS WHERE IT IS; THE MUTATION THAT WOULD INVALIDATE IT IS
|
||||
REFUSED. Revisions 6 and 7 were both wrong, in opposite directions.**
|
||||
|
||||
Revision 6 predicted the fallback at preflight and argued the body
|
||||
could not change it. **False**: the await refusal stops *concurrent
|
||||
interleaving*, not the body, which is arbitrary synchronous Lua and can
|
||||
dedicate the side slot itself.
|
||||
|
||||
Revision 7 then moved enforcement to the placement boundary. **That
|
||||
breaks the invariant `commit_to` exists for.** `docs/agent-handoff.md`
|
||||
`docs/agent-handoff.md:748` states it without qualification:
|
||||
|
||||
> [`commit_to`] preflights every precondition *before* invoking the
|
||||
> callback — dired mutates handle state, `prev`, and paint long before
|
||||
> it reaches anything that could refuse, so **validating at display
|
||||
> time is four mutations too late**.
|
||||
|
||||
Refusing at placement means refusing *after* arbitrary callback code has
|
||||
created buffers, handles and paint. A late refusal is not a refusal; it
|
||||
is a partial commit with an error return.
|
||||
|
||||
**So neither predict nor refuse late — forbid the mutation.** Inside a
|
||||
panel-profile commit, the operations that could change the placement
|
||||
outcome are **refused**, exactly as `Handle:await` is refused inside a
|
||||
commit scope and for the identical reason: something that would
|
||||
invalidate the scope's guarantee is rejected rather than predicted
|
||||
around. With them refused, the preflight measurement cannot go stale,
|
||||
and refusal stays mutation-free on the normal `(false, reason)` path.
|
||||
|
||||
**"Inside a panel-profile commit" MEANS THE WHOLE BODY, INCLUDING ANY
|
||||
NESTED `commit_to` (revision 9), and the unqualified version of that
|
||||
phrase is what revision 8 got wrong.** Contracts **compose**: the core
|
||||
holds a stack, `commit_to` pushes and pops rather than swapping, and the
|
||||
guard consults every contract in force rather than the innermost. Read
|
||||
every "inside a `\"panel\"` commit" below with that scope attached.
|
||||
Nesting itself is *not* refused — only the mutation is, so a nested
|
||||
commit that touches no dedication runs exactly as it did.
|
||||
|
||||
**The mutation surface is narrow, which is what makes this tight rather
|
||||
than aspirational:**
|
||||
|
||||
- `dedicated` **is** writable from Lua — and it is one of only two
|
||||
writable window fields (`window_panel.rs:888`, *"Only `fixed_rows`
|
||||
and `dedicated` are writable (Q#BP2c)"*).
|
||||
- `panel_capable` has **no Lua binding at all** — checked across
|
||||
`src/lua_bindings/`. A body cannot make a frontend panel-incapable.
|
||||
|
||||
**FOUR WRITES REACH DEDICATION, AND A FIFTH IS GUARDED DEFENSIVELY.**
|
||||
Review found the second *after* the first was specified, which is the
|
||||
evidence that guarding one named call site is not a design — and the
|
||||
enumeration below, performed against the tree rather than by recall,
|
||||
found three more: two further `apply_placement` arms, plus
|
||||
`quit_window`'s `QuitAction::Restore`, which step 6 proves *unreachable*
|
||||
and which is guarded anyway. So **four are reachable, a fifth is guarded
|
||||
defensively, and all five are guarded** — the last is the count the
|
||||
safety argument actually runs on. (Historical note, not the current
|
||||
count: earlier revisions of this section counted all five as
|
||||
*reachable*. The table below has always said four; the ledger was
|
||||
corrected in `fb3974b` and this section with it.) The two review named
|
||||
first are:
|
||||
|
||||
1. **`set_params`** — the writable-field path (`window_panel.rs:888`).
|
||||
2. **`display(buf, { side = …, dedicated = true })`** — writes
|
||||
`request.dedicated` straight into the side window
|
||||
(`editor_core.rs:4535`). A body can take this route, then request a
|
||||
second panel buffer and cause the fallback. **An implementation
|
||||
guarding only route 1 passes revision 8's test while keeping the
|
||||
original defect.**
|
||||
|
||||
**THE ENUMERATION, PERFORMED. It is CLOSED as an enumeration of WRITE
|
||||
SITES, and it is closed for a structural reason rather than by inspection
|
||||
stopping when it ran out of ideas.** Recorded here as the framing
|
||||
required, with what was looked for, what was found, and what cannot be
|
||||
ruled out.
|
||||
|
||||
**Read "closed" as scoped to the question it answers (revision 9).** It
|
||||
answers *which writes can dedicate the side slot*, and that answer
|
||||
survived review of the nesting defect intact — every site below is real
|
||||
and every one that can dedicate the slot is still guarded. It says
|
||||
nothing about *when the guard is in force*, and that is the axis
|
||||
revision 8 got wrong: a nested `commit_to` used to mask the enclosing
|
||||
contract, so all five guarded sites — the four reachable ones and the
|
||||
defensive fifth — were momentarily unguarded together. A complete list
|
||||
of write sites is not a complete argument until the guard's extent is
|
||||
stated too, which is what the composing-contracts paragraph above now
|
||||
does.
|
||||
|
||||
*Step 1 — how few pieces of state can matter.* `resolve_placement`
|
||||
reaches `Ordinary` from a side request through exactly two branches, so
|
||||
only two pieces of state are levers at all: `FrontendView::panel_capable`,
|
||||
and the one side window's `Window::params.dedicated`. Everything else a
|
||||
body can touch is irrelevant by construction, which is what makes the
|
||||
enumeration finite instead of "every mutation in the editor".
|
||||
|
||||
*Step 2 — `panel_capable` is unreachable, not merely unguarded.* It is
|
||||
written **only** where a `FrontendView` is constructed, and no
|
||||
`FrontendView` is constructed, registered or unregistered anywhere in
|
||||
`src/lua_bindings/` — `register_frontend_view` and
|
||||
`unregister_frontend_view` have callers only in `daemon.rs` (attach and
|
||||
detach) and in core unit tests. A body cannot reach it.
|
||||
|
||||
*Step 3 — every write to `dedicated`, from `rg 'params\.dedicated\s*='
|
||||
src/`, classified.* Eight sites, no exceptions:
|
||||
|
||||
| # | site | verdict |
|
||||
|---|---|---|
|
||||
| 1 | `apply_placement`, `Side` **created** | reachable — `display{side, dedicated}` with no panel yet |
|
||||
| 2 | `apply_placement`, `Side` **replacing** | reachable — `display{side, dedicated}`, different buffer |
|
||||
| 3 | `apply_placement`, `Side` **non-replacing** | reachable — `display{side, dedicated}`, same buffer |
|
||||
| 4 | `apply_placement`, `Ordinary` (`!fell_back`) | harmless — every `Ordinary` target is filtered `!is_side`, so it is never the slot |
|
||||
| 5 | `apply_placement`, `Ordinary` (clear) | harmless — only ever writes `false` |
|
||||
| 6 | `set_params` | reachable — the direct write (Q#BP2c) |
|
||||
| 7 | `quit_window`, `QuitAction::Restore` | **unreachable** — guarded anyway, defensively; see below |
|
||||
| 8 | an `EditorCore` unit test | not Lua-reachable |
|
||||
|
||||
*Step 4 — the guards, sited where the property converges rather than at
|
||||
each caller.* Sites 1, 2, 3 (and 4, 5) are all reached through
|
||||
`apply_placement`, which has **exactly one caller**, `display_buffer`.
|
||||
So one guard there covers every request-driven dedication, including
|
||||
routes that do not exist yet. `set_params` is a genuinely separate write
|
||||
and is guarded separately — dedication does *not* converge before the
|
||||
field itself, and that is stated rather than papered over. Two live
|
||||
guards over the four reachable sites; site 7 carries a third guard,
|
||||
defensive because the site is unreachable (step 6), so **all five are
|
||||
guarded**.
|
||||
|
||||
*Step 5 — what was looked for and found NOT to be a route.* Closing the
|
||||
side window is **not** one: with no side leaf `side_window_for` returns
|
||||
`None` and `resolve_placement` **creates** a fresh panel rather than
|
||||
falling back, so quitting or hiding the panel mid-commit is safe, and
|
||||
`panel_hidden` is not consulted by placement at all. `params.side` is
|
||||
likewise unreachable — `set_params` refuses it and only
|
||||
`apply_placement`'s created branch writes it, so a body cannot promote
|
||||
an already-dedicated document window into the slot.
|
||||
|
||||
*Step 6 — site 7 is unreachable, and this is the one finding that
|
||||
surprised.* `QuitAction::Restore` carries the outgoing `dedicated` flag,
|
||||
so quitting the panel looked like a route with no `dedicated` argument
|
||||
at the call site at all. It cannot be constructed: `Restore` is only
|
||||
ever *stored* on a **replacing** side placement, and a dedicated slot
|
||||
can never be the target of one — a side request with a different buffer
|
||||
falls through to `Ordinary`, and an exact-target request is refused by
|
||||
`window_accepts_buffer`. So `Restore { dedicated: true }` has no
|
||||
producer. It is guarded anyway, defensively and labelled as such,
|
||||
because its unreachability is an emergent property of two rules in a
|
||||
different function.
|
||||
|
||||
**What this does NOT rule out.** The enumeration is closed over the
|
||||
current tree, not over future edits: relaxing `resolve_placement`'s
|
||||
dedicated arm, or adding a binding that writes `params.dedicated`
|
||||
directly, reopens it. `Window::params.dedicated` is a public field, so
|
||||
the compiler does not enforce the funnel — the acceptance rows are what
|
||||
would catch a regression, one per reachable site.
|
||||
|
||||
**And it never ruled out a defect in the guard's EXTENT, which is what
|
||||
revision 9 found.** Nothing above is about *when*
|
||||
`panel_commit_dedication_refusal` answers; a list of write sites cannot
|
||||
notice that the contract it reads was masked by a nested scope. The
|
||||
acceptance suite now drives the same write-site rows at **two depths** —
|
||||
directly in a `"panel"` body, and through a nested `commit_to` — so a
|
||||
route guarded at one depth and not the other fails loudly rather than
|
||||
being covered by the enumeration's word "closed".
|
||||
|
||||
**If the enumeration had turned out open-ended**, the fallback was to
|
||||
**collapse the two profiles** — run all four checks always, losing the
|
||||
panel relaxation. That is safe, simple, and honest; it is not the
|
||||
preferred answer only because it makes the parameterization pointless.
|
||||
Choosing it is a design decision needing its own approval, not a
|
||||
silent retreat. **It was not needed.**
|
||||
|
||||
**What is NOT the fix: refusing a panel commit that would fall back.**
|
||||
Falling back to an ordinary window is existing, deliberate behaviour
|
||||
for a frontend without panel capability; refusing would turn a
|
||||
graceful degradation into an error and regress consumers that work
|
||||
today. The panel profile relaxes checks; it does not get to change
|
||||
where things land.
|
||||
|
||||
**Consequence for the capture, which follows and should not be
|
||||
discovered later:** if the panel profile needs only the frontend, then
|
||||
a frontend with **no document window** can still host a panel — so
|
||||
Q#DC-4's "return `nil`" is right for the document profile and possibly
|
||||
wrong for the panel one. That interaction is settled as part of
|
||||
answering this, not after it.
|
||||
|
||||
**I hold the *choice* loosely, not the matrix.** (1) has a real
|
||||
argument — a uniform rule is easier to reason about, and over-refusal
|
||||
is safe — but it would refuse the git panel for reasons unrelated to
|
||||
it, and "safe" refusals that users cannot explain are how a mechanism
|
||||
gets worked around. If review prefers (1) or (3), the matrix above is
|
||||
what changes, and **every cell marked "not applicable" must still be
|
||||
tested as deliberately omitted** (§7) so a future reader cannot mistake
|
||||
an omission for an oversight.
|
||||
|
||||
### Q#DC-3 — what is the type called?
|
||||
|
||||
*My vote: **`ViewDestination`***, with `pmacs.window.capture_destination()`
|
||||
as the Lua entry point. It names what it is — a place in a view where a
|
||||
continuation's result belongs — without claiming a directory or a
|
||||
buffer kind.
|
||||
|
||||
The Q#JR14 doc comments should keep their references intact; a rename
|
||||
that orphans the rationale is worse than a slightly stale name.
|
||||
|
||||
### Q#DC-5 — the exact Lua call shape for the profile **(new in rev 4)**
|
||||
|
||||
Revisions 2 and 3 said "the profile is declared at `commit_to`" and
|
||||
never said **how**. That is not a detail: today's binding accepts
|
||||
exactly `(dest, body)` (`window_panel.rs:453-456`), so without a
|
||||
specified form #227 has no stable API to adopt against, and the
|
||||
promise that existing callers keep their semantics is a hope rather
|
||||
than a contract.
|
||||
|
||||
**The signature:**
|
||||
|
||||
```lua
|
||||
pmacs.window.commit_to(dest, body) -- document profile
|
||||
pmacs.window.commit_to(dest, body, "panel") -- panel profile
|
||||
```
|
||||
|
||||
- **`profile` is an OPTIONAL THIRD argument, typed `mlua::Value` at
|
||||
the binding — NOT `Option<String>`.**
|
||||
|
||||
**Revision 4 said `Option<String>` and that contradicted its own
|
||||
error requirement.** mlua rejects a number or table *during argument
|
||||
conversion*, before the closure body runs, so the promised message
|
||||
naming `"document"` and `"panel"` would be **unreachable** — a caller
|
||||
passing `42` would get mlua's generic conversion error instead. This
|
||||
is the identical trap the existing binding already documented for
|
||||
`dest`, in a comment revision 4 cited while making the same mistake
|
||||
one argument to the right:
|
||||
|
||||
> Typed as `Value` rather than `AnyUserData` so this message is
|
||||
> REACHABLE: with the narrower type mlua rejects a table during
|
||||
> argument conversion, and a caller who fabricated one got "error
|
||||
> converting Lua table to userdata" — true, but it names neither the
|
||||
> rule nor how to get a real destination.
|
||||
|
||||
So: accept `Value`, and validate in the body.
|
||||
- **`Nil` or absent → `"document"`.** Both spellings, since
|
||||
`commit_to(dest, body, nil)` is what a Lua caller threading an
|
||||
optional variable produces, and it must not be a third behaviour.
|
||||
- **`String` → must be `"document"` or `"panel"`**, else refused,
|
||||
naming both accepted values.
|
||||
- **Anything else → refused by the SAME message**, which now names
|
||||
the accepted values *and* says a string was expected. That message
|
||||
only exists if the type is `Value`.
|
||||
- No arity sniffing and no table-or-function dispatch on argument 2 —
|
||||
a polymorphic second argument would put the *destination*'s error
|
||||
message back at risk, which is what that comment was protecting.
|
||||
- **Trailing, and readable in practice.** A profile after a long inline
|
||||
closure would read badly, but that is not the call shape in use:
|
||||
dired defines `local function commit() … end` and calls
|
||||
`commit_to(opts.dest, commit)` (`builtin/runtime/dired.lua:670,717`).
|
||||
Against a named body, `commit_to(dest, commit, "panel")` reads fine.
|
||||
- **The value set is CLOSED: `"document"` and `"panel"`.** Exactly the
|
||||
two profiles in Q#DC-2's matrix. Not an open string namespace — a
|
||||
third profile is a decision, not a spelling.
|
||||
- **Omitted means `"document"`.** This is the load-bearing part: every
|
||||
existing `commit_to(dest, fn)` call keeps **all four** preflight
|
||||
checks, unchanged, by definition of the signature. `journey_acceptance`
|
||||
passing untouched (§7) then follows from the API shape rather than
|
||||
from care.
|
||||
- **An unrecognized profile is an ERROR**, naming the accepted values —
|
||||
**not** a silent fall back to `"document"`. A fallback would hand a
|
||||
caller stricter or looser checks than it asked for, which is the
|
||||
failure mode the whole parameterization exists to prevent. A
|
||||
non-string profile errors the same way.
|
||||
|
||||
**Which profile each of git's continuations takes**, so #227's adoption
|
||||
is decided here rather than rediscovered: `*git-status*` → **panel**
|
||||
(it lands in the bottom panel, `listview.lua:550`); `*git-diff*` →
|
||||
**document** (it replaces a document window deliberately,
|
||||
`git.lua:852-854`).
|
||||
|
||||
### Q#DC-4 — what happens when there is no document window? **(DECIDED in rev 3)**
|
||||
|
||||
**Revision 2 left this contradicting Q#DC-2 and it is the primary panel
|
||||
API, so it is decided here rather than voted on.** Q#DC-2 concluded a
|
||||
panel profile depends only on a live frontend — so it can commit with
|
||||
no document window at all — while this question still said the capture
|
||||
returns `nil` in exactly that case, and told git to fall back to
|
||||
ambient behaviour. Those cannot both hold, and the fallback advice was
|
||||
independently wrong: falling back to ambient **is** the P1a bug this
|
||||
lane exists to remove.
|
||||
|
||||
**The decision:**
|
||||
|
||||
- **`ViewDestination { frontend, window: Option<WindowId>, buffer:
|
||||
Option<BufferId> }`.** The frontend is always present; the document
|
||||
pair is optional and absent exactly when the frontend has no document
|
||||
window.
|
||||
- **`capture_destination()` is NOT profile-aware and takes no
|
||||
arguments.** It records what is there. Making capture profile-aware
|
||||
would force the caller to know at *capture* time what it will do at
|
||||
*commit* time, which is the opposite of why capture exists — the
|
||||
whole point is to freeze the truth early and decide later.
|
||||
- **The profile is declared at `commit_to`**, which is where Q#DC-2's
|
||||
parameterization already lives. One place makes the decision, and it
|
||||
is the place that knows. **Its exact call shape is Q#DC-5**, which
|
||||
revisions 2 and 3 left unspecified.
|
||||
- **A document-profile commit on a destination with no document pair is
|
||||
REFUSED**, with a reason naming that, joining the four preflight
|
||||
refusals rather than being a separate failure mode.
|
||||
- **Capture therefore never returns `nil`** while a frontend exists,
|
||||
and the "adopter degrades to ambient" advice is **withdrawn**. An
|
||||
adopter with nowhere to land gets a refusal it can report; it does
|
||||
not get permission to guess.
|
||||
|
||||
**What this changes elsewhere, so the decision does not sit alone:**
|
||||
§4's outline says the capture returns userdata "for the *current*
|
||||
frontend and its document window" — it returns one for the current
|
||||
frontend, **with** its document window when there is one. Q#DC-1's "no
|
||||
arguments" answer is unchanged and now load-bearing rather than
|
||||
incidental: no arguments is what keeps capture profile-blind.
|
||||
|
||||
## 7. Verification
|
||||
|
||||
- **A captured destination survives a frontend switch**: capture in A,
|
||||
make B active, commit, and assert the result lands in **A**. This is
|
||||
P1a's actual failure and the reason the lane exists — asserting only
|
||||
that the API returns userdata would pass on a capture that does
|
||||
nothing.
|
||||
- **A fabricated destination is still refused** — the existing Q#JR14d
|
||||
guarantee, re-asserted after the rename so the generalization cannot
|
||||
quietly open the hole it was built to close.
|
||||
- **Every preflight refusal is witnessed by its own case, in BOTH
|
||||
profiles** (Q#DC-2's matrix): frontend gone, window gone, stale
|
||||
buffer, dedicated window — each asserted to **refuse** under the
|
||||
document profile, and each of the three marked "not applicable"
|
||||
asserted to **NOT refuse** under the panel profile. A deliberately
|
||||
omitted check that has no test is indistinguishable from a check
|
||||
someone forgot, and the next reader will restore it.
|
||||
- **A legacy two-argument `commit_to(dest, body)` gets the DOCUMENT
|
||||
profile** (Q#DC-5), witnessed by a check the panel profile omits —
|
||||
a stale-buffer refusal. Asserting merely that it does not error would
|
||||
pass on a call silently downgraded to the panel profile, which is the
|
||||
regression that would quietly void Journey Stage 1a's guarantees.
|
||||
- **A `"panel"` commit that FALLS BACK to a document window is checked
|
||||
against the document preconditions**, witnessed for **both** causes
|
||||
separately — a non-panel-capable frontend, and a dedicated side slot.
|
||||
Each asserts the stale-intent refusal fires: capture A, make B newer,
|
||||
commit `"panel"`, observe the refusal rather than B being replaced.
|
||||
- **A BODY THAT TRIES TO CREATE THE FALLBACK IS REFUSED AT THE ATTEMPT**,
|
||||
in its own test: the callback dedicates the side slot **mid-commit**.
|
||||
Three assertions, and the second and third are the ones that matter:
|
||||
the dedication call itself is **refused**; the side slot is **still
|
||||
undedicated afterwards**; and no partial result was installed.
|
||||
**One row per reachable WRITE SITE** (§3), which is four and not two:
|
||||
`set_params`, and `display{side, dedicated}` in each of
|
||||
`apply_placement`'s **created**, **replacing** and **non-replacing**
|
||||
arms. A single row against one route is what would let another keep
|
||||
the defect — and rows per *call spelling* would have missed that one
|
||||
spelling reaches three different writes. The
|
||||
two bullets above cannot catch this — both establish their fallback
|
||||
state *before* `commit_to` is entered, so a preflight-snapshot design
|
||||
passes them.
|
||||
|
||||
**Asserting only "document B was not replaced" is insufficient**, and
|
||||
revision 7's version of this test made exactly that mistake: it
|
||||
passes on a design that lets the body mutate freely and merely
|
||||
declines the final installation, leaving every other side effect
|
||||
behind. The refusal must land on the mutation, not on the outcome.
|
||||
- **THE SAME WRITE-SITE ROWS, DRIVEN THROUGH A NESTED `commit_to`**
|
||||
(revision 9), in their own test: an outer `"panel"` commit whose body
|
||||
opens a nested **`"document"`** commit — a perfectly valid one, whose
|
||||
destination is captured fresh inside the outer body so it passes all
|
||||
four of its own checks and its callback really runs — and *that*
|
||||
callback attempts the dedication. Asserted: the attempt is **refused**,
|
||||
the slot is **still undedicated** afterwards, and the outer commit's
|
||||
destination is **intact** (its result lands in the panel; the user's
|
||||
newer document buffer survives). The bullet above cannot catch this —
|
||||
its mutation runs at commit depth 1, where revision 8's single-slot
|
||||
contract was the right one to read. Rows per write site rather than one
|
||||
row, because a fix that reinstated the outer contract for only one site
|
||||
would pass a single-row version.
|
||||
- **ORDINARY NESTING STILL WORKS**, asserted rather than assumed: a
|
||||
nested `commit_to` that touches no dedication is accepted, its body
|
||||
runs, and its return value comes back through both frames. This is the
|
||||
pin against the other candidate fix — prohibiting nested `commit_to`
|
||||
outright — which would close the hole by forbidding a shape no rule
|
||||
objects to. Two further assertions, and the second is the one a
|
||||
`pop`-shaped fix gets wrong: the enclosing restriction is **back in
|
||||
force after the nested commit returns** (popped, not cleared), and
|
||||
**outside every commit dedication is ordinary again**, so the fix
|
||||
leaked no permanent restriction onto the editor.
|
||||
- **THE CROSS-FRONTEND EXCEPTION IS PINNED POSITIVELY**, over **two**
|
||||
frontends: while an outer `"panel"` commit for A is in force, a nested
|
||||
commit for **B** dedicates **B's** side slot and is **allowed** — and
|
||||
B's slot is asserted really dedicated afterwards, not merely
|
||||
unrefused. The far side runs in the same test: A's slot is still
|
||||
undedicated and A's result still lands in A's panel, so this cannot
|
||||
pass by having weakened the restriction generally. **This is the one
|
||||
row asserting that something is permitted**; every other in the suite
|
||||
asserts a refusal, and without it, deleting the `fid` comparison —
|
||||
making any outer panel contract *globally* restrictive — passes the
|
||||
whole file, because both nesting rows above drive a single frontend.
|
||||
The exception is real and not a convenience: `resolve_placement`
|
||||
consults only the requesting frontend's `panel_capable` and its own
|
||||
one side window, so nothing done to B can change where A's side
|
||||
request lands.
|
||||
- **A `"panel"` commit that really lands in the panel still skips
|
||||
checks 2–4** — otherwise the fix has quietly collapsed the two
|
||||
profiles into one and the parameterization buys nothing.
|
||||
- **An unrecognized profile string is REFUSED**, with a message naming
|
||||
the accepted values — not silently treated as `"document"`.
|
||||
- **An invalid-UTF-8 profile is refused by that SAME message.** Lua
|
||||
strings are byte strings, so a `string.char(255)` profile reaches
|
||||
`to_str()` and produces mlua's generic conversion error *before*
|
||||
the documented message is ever constructed — the same reachability
|
||||
class as the `Option<String>` defect, one layer deeper. Compare
|
||||
bytes, or map the conversion failure onto the message; asserted on
|
||||
content, in the bad-profile matrix beside the number and table rows.
|
||||
- **A non-string profile (a number, a table) is refused by that SAME
|
||||
message**, asserted **on its content**, not merely that an error
|
||||
occurred. This is the bullet that fails if the argument is ever
|
||||
retyped to `Option<String>`: mlua would reject the value during
|
||||
conversion and the assertion on the message would stop matching. The
|
||||
test is therefore the guard on the type choice, not just on the
|
||||
behaviour.
|
||||
- **An explicit `nil` profile takes the document profile**, identical
|
||||
to omitting it — witnessed separately, because a Lua caller threading
|
||||
an optional variable produces `nil` rather than absence, and a third
|
||||
behaviour there would be invisible until someone hit it.
|
||||
- **Capture SUCCEEDS with no document window** (Q#DC-4), returning a
|
||||
destination whose document pair is absent — asserted as a successful
|
||||
capture, not as `nil`.
|
||||
- **A panel-profile commit on that destination SUCCEEDS**, and a
|
||||
**document-profile commit on it is REFUSED** with a reason naming the
|
||||
missing document window. Both halves, because asserting only the
|
||||
refusal would pass on a capture that refuses everything.
|
||||
- **The directory path is unchanged** — dired's existing acceptance
|
||||
coverage passes untouched.
|
||||
- **`tests/journey_acceptance.rs` passes UNCHANGED**, as a named
|
||||
preservation suite. It carries the established contract this lane
|
||||
generalizes — 27 `commit_to` references across nine named pins
|
||||
including `commit_to_refuses_a_forged_destination`,
|
||||
`commit_to_scopes_and_restores_on_a_normal_return`,
|
||||
`commit_to_restores_when_the_callback_raises`,
|
||||
`commit_to_refuses_an_await_and_restores`,
|
||||
`commit_to_delivers_to_the_requesting_frontend_not_the_ambient_one`,
|
||||
`a_declining_listener_cannot_redirect_the_destination`, and two
|
||||
rows already named `preservation_*`. Journey Stage 1a's own framing
|
||||
treats this suite as a required gate; a lane that generalizes its
|
||||
substrate does not get to relax that.
|
||||
- **STOP SIGNAL, for both suites.** If any existing `dired` or
|
||||
`journey_acceptance` test needs editing, the generalization changed
|
||||
Journey Stage 1a's semantics. That is cause to stop and report, not
|
||||
to adjust the test — a suite edited to accommodate the change under
|
||||
test has stopped being evidence.
|
||||
- **`Handle:await` still refuses inside the scope**, including through
|
||||
`pmacs.async.yield_to_next_tick` if the worker-identity lane's Q#W-7
|
||||
has landed by then; if it has not, this lane does **not** add that
|
||||
guard — it belongs to that lane and duplicating it would produce a
|
||||
conflict for no benefit.
|
||||
|
||||
**What this will NOT prove:** that git surfaces in the right frontend —
|
||||
that is #227's adoption, after this lands. This lane ships the
|
||||
mechanism and one set of tests for the mechanism.
|
||||
|
||||
## 8. Not in scope
|
||||
|
||||
**Adopting the capture anywhere**, including git (#227 does that) and
|
||||
including migrating other async continuations that have the same latent
|
||||
bug — worth an audit, not this lane's work. Changing Journey Stage 1a's
|
||||
directory semantics. The `commit_to` scope guard for
|
||||
`yield_to_next_tick` (worker identity Q#W-7). Any protocol change —
|
||||
this is entirely core + Lua bindings. Panel geometry or placement
|
||||
policy, which is the bottom-panel arc's.
|
||||
|
|
@ -1,708 +0,0 @@
|
|||
# Git integration — Stage 1: seeing what changed
|
||||
|
||||
**Status: revision 5, APPROVED 2026-08-09. Implementation may
|
||||
proceed.**
|
||||
|
||||
**Revision 5 completes the unborn-repository policy, which revision 4
|
||||
wrote as three disjoint rows when a single file can be in two states at
|
||||
once.** `AM` — staged, then edited again — is not exotic; it is what a
|
||||
first commit looks like halfway through. The states below were
|
||||
**enumerated from a real unborn repository**, not reasoned about, and
|
||||
one of them settles a case by ruling it out entirely.
|
||||
|
||||
**Revision 4 fixes two contracts that would have failed in ordinary
|
||||
use, both verified against real behaviour rather than reasoned about:**
|
||||
re-binding `d` on every refresh (keymap binds refuse duplicates, so
|
||||
*every successful refresh* would have errored), and two git exit states
|
||||
the failure predicate got wrong. Measured, not assumed — the exit codes
|
||||
below were produced in a scratch repository.
|
||||
|
||||
**Revision 3 pins four Stage 1 contracts revision 2 left loose, and two
|
||||
of those were again claims I made without reading the code I was
|
||||
crediting.** I attributed selection preservation to `listview` and
|
||||
`d` to its key surface; neither is true, and both were checkable in the
|
||||
file I had already cited. The pattern is worth naming since it has now
|
||||
recurred across three revisions: **I cite a file, then describe what I
|
||||
expect it to contain.**
|
||||
|
||||
**Revision 2 answers four blockers, two of which were factual errors in
|
||||
revision 1 that scouting should have caught and did not.** I read
|
||||
`ProjectKind::Git`'s name instead of its doc comment, and I quoted
|
||||
`COHERENCE.md` §15's "no Git integration anywhere in the tree" without
|
||||
checking whether it was still true of the tree. It is not.
|
||||
|
||||
---
|
||||
|
||||
## 1. Why this, and why now
|
||||
|
||||
`COHERENCE.md` §15 is blunt about it:
|
||||
|
||||
> **There is no Git integration at all** — no status, stage, diff,
|
||||
> blame, or gutter markers anywhere in the tree (gutter git riders and
|
||||
> the `ResourceOffer` diff/blame family are named deferrals). The Git
|
||||
> affordance list above has nothing to attach to yet.
|
||||
|
||||
**That sentence is literally false about the tree, and revision 1
|
||||
repeated it without checking.** `tests/fixtures/pmacs-magit/` is a
|
||||
tracked, installable package — 1,914 lines across four modules, with
|
||||
`status.lua` spawning git through `pmacs.process.spawn` and parsing
|
||||
**`--porcelain=v2 --branch`** into structured sections, plus a 662-line
|
||||
acceptance suite (`tests/m8_6_acceptance.rs`, 32 tests) covering
|
||||
status, refresh, staging, commit, push and branch behaviour.
|
||||
|
||||
**The PRODUCT gap is real and unchanged** — none of that is bundled
|
||||
runtime, so a user who installs pmacs gets no git integration. But
|
||||
"nothing to attach to" understates what exists to *learn from*, and
|
||||
§15's wording should be corrected when this lands.
|
||||
|
||||
For a **daily driver**, this is the largest remaining gap. Not because
|
||||
git is the most architecturally interesting thing missing — §7
|
||||
workspaces and §9 worker identity are both deeper — but because it is
|
||||
the one a user touches *every working hour*, and pmacs currently makes
|
||||
them leave the editor to answer "what have I changed?".
|
||||
|
||||
That is the criterion this lane is chosen against: **frequency of use
|
||||
per day**, not depth of model.
|
||||
|
||||
## 2. Ground truth — what already exists
|
||||
|
||||
Scouted, not assumed:
|
||||
|
||||
- **`ProjectKind::Git` is NOT general repository detection**, and
|
||||
revision 1 said it was. Its doc comment is explicit: *"A bare git
|
||||
repository (no language marker found inside)"* (`src/project.rs:89`).
|
||||
Markers are ordered and a language marker beside `.git` **wins**
|
||||
(`src/project.rs:10`), so a normal Rust repository reports
|
||||
`kind = "rust"` and would have been invisible to a lane that gated on
|
||||
`kind == "git"`. That gate would have failed on this very repository.
|
||||
|
||||
**The rule this lane uses instead: never ask pmacs whether it is a
|
||||
git repo.** Run git in the **active file's directory** and let git
|
||||
resolve its own worktree — `git -C <dir> rev-parse --show-toplevel`
|
||||
establishes the root, and a non-zero exit *is* the "not a repository"
|
||||
answer. Git's own resolution handles submodules, worktrees, `GIT_DIR`
|
||||
and `.git` files; a marker walk reimplements a subset of that and
|
||||
gets it subtly wrong.
|
||||
- **`pmacs.process.spawn` / `events_take` / `terminate` / `forget`**
|
||||
is the working model for running an external tool asynchronously;
|
||||
`builtin/runtime/compile.lua` is a full worked example, including
|
||||
spawn-failure handling and exit markers.
|
||||
- **`pmacs.listview.open`** is a real primitive with existing adopters
|
||||
(`*references*`, `*lsp*`), carrying optional `depth`/`id`,
|
||||
primitive-owned collapse, and selection re-seated by id. `COHERENCE.md`
|
||||
P5 says the remaining work there is **adoption, not construction** —
|
||||
a `*git-status*` panel is exactly that.
|
||||
- **Gutter signs exist in both frontends** — the TUI's leading-column
|
||||
glyph (`src/diag.rs`) and the GPU's `GUTTER_SIGN_X` bars
|
||||
(`pmacs-gpu/src/main.rs:420`).
|
||||
- **A tested porcelain-v2 parser exists as a package fixture** (above).
|
||||
Its `status.lua` deliberately separates **pure `parse_*` functions
|
||||
that take a string and return structure** from the spawning around
|
||||
them — which is the shape that makes a parser testable without a
|
||||
repository, and it is already proven by 32 tests.
|
||||
|
||||
And the constraint that shapes the staging:
|
||||
|
||||
- **`DecorationKind` is a CLOSED enum on the wire**
|
||||
(`pmacs-protocol/src/message.rs:1472`): four diagnostic severities,
|
||||
`Selection`, `SearchMatch`, `SearchMatchActive`, `CurrentLine`.
|
||||
**Gutter markers for git hunks therefore require new variants, which
|
||||
is a protocol version bump.** The gutter signs that exist are keyed
|
||||
on `diagnostic_severity_rank` and have no notion of anything else.
|
||||
|
||||
## 3. The staging, and why the line falls where it does
|
||||
|
||||
**Stage 1 (this lane): read-only, panel-based, NO WIRE CHANGE.**
|
||||
|
||||
- `*git-status*` — a `listview` panel over
|
||||
`git status --porcelain=v2 --branch -z` (Q#G-6), rows visiting the
|
||||
file at RET, refreshed by `g` under the completion model in Q#G-1.
|
||||
- `*git-diff*` — the diff for the **file** under point (Q#G-7), in a
|
||||
generated buffer rendered as **plain text** (no `diff` grammar
|
||||
exists). **No hunk model** — hunks are Stage 2's concern.
|
||||
|
||||
**Stage 2 (separate lane): gutter markers.** Needs new
|
||||
`DecorationKind` variants and a `PROTOCOL_VERSION` bump, plus both
|
||||
frontends' gutter renderers learning a second rider family.
|
||||
|
||||
**Stage 3+ (unscheduled): staging, commit, blame.** Staging and commit
|
||||
are where an editor becomes a git *client*; blame is a lower-frequency
|
||||
read. Neither belongs in front of the two above.
|
||||
|
||||
**The line is drawn at the wire on purpose, and it is a scheduling
|
||||
decision as much as a design one.** Parallel lanes are about to start,
|
||||
and `PROTOCOL_VERSION` is a strict serialization point — two lanes
|
||||
bumping it collide, and this session already recorded what that costs
|
||||
(eight broken version assertions on CI from a single bump). Stage 1
|
||||
touching no wire is what lets it run **concurrently** with other work.
|
||||
Stage 2 must be scheduled alone.
|
||||
|
||||
## 4. Coherence impact (§20)
|
||||
|
||||
Required by `CLAUDE.md` for coherence-affecting work, and this is
|
||||
coherence-affecting — it is §15's named gap.
|
||||
|
||||
- **Journey steps touched:** none directly. Git is not currently a
|
||||
journey step; the golden journey runs open → edit → build → test →
|
||||
navigate. This lane does **not** add a step, and I would rather say
|
||||
so than inflate the claim.
|
||||
- **§15 contextual affordances — the direct target.** The audit's git
|
||||
affordance list ("a Git change stage/revert/diff") has *nothing to
|
||||
attach to*. Stage 1 creates the thing to attach to; the affordances
|
||||
themselves follow it, and the menu's context vocabulary
|
||||
(`src/menu.rs:44`) would need a `git` context to host them — **out of
|
||||
scope here**, named so it is not forgotten.
|
||||
- **§14 workbench primitives — adoption, which is the stated need.**
|
||||
`*git-status*` becomes the **fifth** `listview` call site and the
|
||||
first outside the LSP panels, which is the concrete evidence P5 asks
|
||||
for that the primitive generalizes past its first consumer.
|
||||
- **Interaction islands (§6): none added, and this is a real
|
||||
constraint.** The panel gets no hardcoded key interception; it uses
|
||||
`listview`'s existing key handling. §6 records six such shadows and
|
||||
calls them "weak, and growing" — this lane must not make it seven.
|
||||
- **Config registry adoption:** at least one setting
|
||||
(`git.enabled`, Q#G-4), defined through `pmacs.config.define` like
|
||||
`ui.line-wrap` and the zoom settings, not a bare Lua global.
|
||||
- **Background-work attribution (§9): NEGATIVE, and named as such.**
|
||||
Git runs as a spawned process, and spawned processes do **not** appear
|
||||
in `*workers*` — that view is `async.lua`'s job list; processes live
|
||||
under `pmacs.process.list` (Q#G-5). This lane therefore adds a fifth
|
||||
thing running in the background with no single place to see it. The
|
||||
process is labelled honestly, which is better than anonymous, but
|
||||
**a label is not attribution and this document does not pretend
|
||||
otherwise.** Accepted because these are short-lived reads; it would
|
||||
not be acceptable for Stage 3's push/pull.
|
||||
|
||||
## 5. Open questions
|
||||
|
||||
### Q#G-1 — is the status panel a snapshot or a live view?
|
||||
|
||||
A snapshot is a command that opens a panel; a live view refreshes on
|
||||
buffer save, on focus, or on a filesystem watch.
|
||||
|
||||
*My vote: **snapshot, refreshed explicitly***, with `g` re-running
|
||||
inside the panel. Live refresh needs a watch mechanism, an invalidation
|
||||
rule, and a §9 story for the recurring work — all real arcs. A snapshot
|
||||
is honest, useful the first day, and does not pretend to a currency it
|
||||
cannot maintain.
|
||||
|
||||
**But "explicit refresh" does not fit `listview` unmodified, and
|
||||
revision 1 missed that.** `listview.refresh` is synchronous:
|
||||
|
||||
```lua
|
||||
local rows = check_ids(p.on_refresh() or {}) -- listview.lua:402
|
||||
```
|
||||
|
||||
The result is consumed immediately. `pmacs.process.spawn` cannot return
|
||||
rows there — it returns a process id whose output is drained later. So
|
||||
revision 1's "adopt `listview`" would have produced exactly one of the
|
||||
two failures the reviewer named: a reimplemented list, or a `g` that
|
||||
silently does nothing. The primitive's own docs already call a dead `g`
|
||||
out as a defect it must not repeat (`listview.lua:416`).
|
||||
|
||||
**The completion model, specified.** `on_refresh` stays synchronous and
|
||||
honest:
|
||||
|
||||
1. **`on_refresh` returns the CURRENT rows immediately**, with a
|
||||
`refreshing…` marker row appended, and *kicks off* the spawn. `g` is
|
||||
therefore never a no-op — it always re-renders and always shows that
|
||||
work started.
|
||||
2. **On exit, the completion handler re-opens the panel** via
|
||||
`listview.open` with the same `name` — **and re-seats the selection
|
||||
itself.**
|
||||
|
||||
Revision 2 credited that to the primitive and was wrong.
|
||||
`listview.open` **resets collapse** (`p.collapsed = {}`) and
|
||||
**always seats line 1** (`seat_cursor(p, 1)`,
|
||||
`builtin/runtime/listview.lua:337-378`). The `listview.lua:82` note
|
||||
I cited is about **name** disambiguation to `<2>`, not selection.
|
||||
Only `listview.refresh` preserves a selection, and that is the
|
||||
synchronous path this model cannot use.
|
||||
|
||||
So the contract is explicit and owned here: **capture the selected
|
||||
row's git id (its current path) before re-opening, and after
|
||||
re-opening move to the line whose row carries that id**, computed
|
||||
from the handler's own rows array via `pmacs.editor.move_to_line`.
|
||||
If the id is gone from the new status — the commonest case, since a
|
||||
file that stopped being modified drops out — seat line 1 and say
|
||||
nothing; that is the correct answer, not a failure.
|
||||
|
||||
**Collapse state is moot in Stage 1** because the rows are flat: no
|
||||
`depth`, so nothing to collapse. Stage 2 or a sectioned view would
|
||||
have to revisit this, and would then face the same reset.
|
||||
3. **Concurrent refresh is suppressed by a generation counter.** A
|
||||
second `g` while one is in flight bumps the generation; the older
|
||||
completion sees a stale generation and **discards its rows** rather
|
||||
than racing. It does not terminate the first process — reaping is
|
||||
`pmacs.process.forget`'s job and killing git mid-read buys nothing.
|
||||
4. **Failure is a row, not a silence.** Non-zero exit or spawn failure
|
||||
renders a row carrying the exit code and the first stderr line, plus
|
||||
a status message. §1.2's silence asymmetry.
|
||||
5. **Panel lifetime.** If the panel's buffer is gone when the process
|
||||
exits, the handler drops the result. `compile.lua:252` already
|
||||
handles the buffer-killed case for its own slot; the same shape.
|
||||
|
||||
**The alternative — extending `listview` with an async contract — is
|
||||
the more correct long-term answer** and is deliberately not taken here:
|
||||
it changes a primitive with four existing adopters, and doing that from
|
||||
inside its fifth adopter's lane is how a primitive acquires a consumer's
|
||||
idiosyncrasies. **If review prefers it, it belongs in its own lane
|
||||
before this one.**
|
||||
|
||||
### Q#G-0 — what is the relationship to `pmacs-magit`? **(new in rev 2)**
|
||||
|
||||
The reviewer's framing of the choice is right: adopt, replace, or
|
||||
declare it out-of-product precedent. Doing none of those and quietly
|
||||
writing a second parser is the option that must not happen.
|
||||
|
||||
*My vote: **port its pure `parse_*` functions and its test corpus into
|
||||
the bundled runtime; leave the fixture itself untouched.***
|
||||
|
||||
- **The record TOKENIZER is deliberately rewritten, not ported.** The
|
||||
fixture parses **newline-delimited** v2; Stage 1 reads **`-z`**, and
|
||||
those are different grammars — under `-z` a record's fields are
|
||||
NUL-terminated and a rename carries its two paths as separate fields
|
||||
rather than tab-joined. Saying "port the parser" would have been
|
||||
wrong; what ports is the **separation** (pure `parse_*` functions
|
||||
over a string, testable with no repository) and the **case coverage**
|
||||
its 32 tests encode. The tokenizer underneath is new, and its
|
||||
correctness rests on this lane's own corpus.
|
||||
- **Port, not import.** The fixture's purpose is to prove the *package
|
||||
system* can host this. If bundled code became its dependency, it
|
||||
would stop demonstrating an independent package and `m8_6` would test
|
||||
less than it claims.
|
||||
- **The duplication is therefore deliberate**, and it is the one place
|
||||
this framing accepts two copies of a rule after a session spent
|
||||
removing them. The justification is that they answer different
|
||||
questions — one is product behaviour, one is package-system
|
||||
capability — and coupling them weakens the second. **If review
|
||||
prefers the coupling, that is a defensible call and I will take it**;
|
||||
what I will not do is leave the duplication unstated.
|
||||
- **It also settles Q#G-2's format**: the existing, tested parser is
|
||||
**porcelain v2**, so Stage 1 is v2. Revision 1 said v1 for no reason
|
||||
beyond familiarity.
|
||||
|
||||
### Q#G-2 — `git` the binary, or a library?
|
||||
|
||||
*My vote: **the binary**, via `pmacs.process.spawn`. `compile.lua` is
|
||||
the worked precedent, the daemon already spawns external tools, and a
|
||||
git library is a dependency with a much larger surface than "run one
|
||||
command and parse porcelain". `--porcelain=v2` is explicitly a stable
|
||||
machine format; that is what it is for.
|
||||
|
||||
**Named risk:** no `git` on `PATH`. §1.2's *silence asymmetry* says the
|
||||
failure must be **surfaced with guidance**, not swallowed — the same
|
||||
lesson #204 landed for a missing language server.
|
||||
|
||||
### Q#G-6 — the status data contract **(new in rev 2)**
|
||||
|
||||
Revision 1 said "`--porcelain=v1`" and proposed "a path with a space"
|
||||
as the parsing witness. **Both were inadequate.** Porcelain without
|
||||
`-z` emits paths in git's **C quoting** for anything non-ASCII or
|
||||
containing special characters, and rename/copy records carry *two*
|
||||
paths whose separation is positional. A single space-in-path fixture
|
||||
proves none of that.
|
||||
|
||||
*My vote: the exact invocation*
|
||||
|
||||
```
|
||||
git --no-optional-locks -C <dir> status --porcelain=v2 --branch -z
|
||||
```
|
||||
|
||||
**`--no-optional-locks` is part of the contract, not a nicety.**
|
||||
`git status` is **not strictly read-only**: it may refresh and write
|
||||
the index, and git's own documentation recommends this flag for
|
||||
background scripts precisely so a background reader does not contend
|
||||
for `index.lock` with the user's real git commands
|
||||
(<https://git-scm.com/docs/git-status>). This lane runs status
|
||||
*asynchronously, from an editor, while the user may be running git in a
|
||||
terminal* — the exact scenario the flag exists for. Revision 2 called
|
||||
the lane "read-only" and that was wrong about the mechanism.
|
||||
|
||||
It is **witnessed structurally** — the assembled argv is asserted to
|
||||
carry the flag — because observing a lock that was *not* taken is not
|
||||
something a test can do directly. Verified accepted by the git in use
|
||||
here.
|
||||
|
||||
The rest: `--porcelain=v2 --branch -z`, also verified accepted. NUL delimiting removes C quoting from
|
||||
the problem **entirely** rather than obliging a hand-written unquoter,
|
||||
and it makes the two-path rename record unambiguous: the paths are
|
||||
separate NUL-terminated fields rather than tab-joined inside one.
|
||||
|
||||
The rename/copy identity rule to pin: a `2` record carries the current
|
||||
path **and** its origin, and the panel must show which file it is now
|
||||
while remembering where it came from — a row whose id is the current
|
||||
path, since that is what RET visits.
|
||||
|
||||
**Witness corpus, not one case:** modified, added, deleted, untracked,
|
||||
**renamed (both paths)**, **copied**, a path with a space, a path with
|
||||
a newline, and a non-UTF-8 path. The last two are exactly what `-z`
|
||||
buys and what a quoted parser gets wrong.
|
||||
|
||||
### Q#G-7 — the diff gesture **(new in rev 2)**
|
||||
|
||||
Revision 1 wrote "the diff for the file or hunk under point" while also
|
||||
committing RET to visiting the file. **RET cannot do both, there is no
|
||||
second binding proposed, and no hunk model exists anywhere in the
|
||||
tree.**
|
||||
|
||||
*My vote:*
|
||||
|
||||
- **RET visits the file** — unchanged, and the behaviour a list of
|
||||
files should have.
|
||||
- **A named command, `git.diff-file`, bound to `d` inside the panel.**
|
||||
|
||||
**`d` is not on `listview`'s key surface**, and revision 2 said it
|
||||
was. The bound set is exactly `RET SPC n <down> p <up> TAB g q`
|
||||
(`builtin/runtime/listview.lua:266-279`), bound buffer-locally inside
|
||||
the primitive, which is the only place the panel's buffer handle is
|
||||
known. **Looking the buffer up by name from outside is unsafe** —
|
||||
`listview` deliberately disambiguates a collision to `<2>`, so the
|
||||
name a consumer passed is not necessarily the buffer it got.
|
||||
|
||||
*My vote: **a `keys` table on the open spec***, e.g.
|
||||
`keys = { d = "git.diff-file" }`, bound through the same
|
||||
`bind_local_keymap` that already binds the fixed set. It is additive,
|
||||
general to any adopter, keeps binding where the buffer is known, and
|
||||
adds **no** interception — the §6 constraint holds.
|
||||
|
||||
**The registration lifecycle, which revision 3 omitted and which
|
||||
would have broken the refresh path it depends on.** `Keymap::bind`
|
||||
**refuses duplicates** — `KeymapError::DuplicateBinding`, *"Refuse
|
||||
rather than silently overwrite"* (`src/keymap_tree.rs:75`) — and the
|
||||
completion model calls `listview.open` again on **every** refresh. A
|
||||
naive `keys` implementation therefore errors on the second open, so
|
||||
**every successful refresh would have failed while re-binding `d`.**
|
||||
|
||||
The contract:
|
||||
|
||||
1. **Keys are installed once, when the panel's buffer is created**,
|
||||
and stored on the panel.
|
||||
2. **A later `open` for a live panel does not re-bind.** It
|
||||
**compares** the supplied `keys` against the stored table and
|
||||
**errors on divergence** rather than ignoring it. Silently keeping
|
||||
the old binding would give the consumer a key that does something
|
||||
other than what it just asked for — a dead or lying key, which is
|
||||
the defect `listview` already condemns for `g`.
|
||||
3. **Collisions are rejected at install time**, against both the
|
||||
fixed set (`RET SPC n <down> p <up> TAB g q`) and any
|
||||
**prefix conflict** — `Keymap` has a separate error for turning a
|
||||
leaf into a submap, and a `keys` table must not be able to reach
|
||||
it.
|
||||
|
||||
(The alternative, idempotent re-registration, is tolerable but
|
||||
strictly weaker: it makes a consumer that changes its keys mid-session
|
||||
silently wrong instead of loudly wrong.)
|
||||
|
||||
**This IS a `listview` modification, and revision 2's "no listview
|
||||
modification" was false.** I distinguish it from the async-contract
|
||||
change I deferred: that one alters *when* an existing callback's
|
||||
result is consumed for four existing adopters; this adds an optional
|
||||
field that changes nothing for a spec that omits it. **If review
|
||||
judges any primitive change out of an adopter's lane, the alternative
|
||||
is `listview.open` returning the panel buffer** so the consumer binds
|
||||
its own key — smaller still, but it pushes binding to every adopter.
|
||||
- **No hunk model in Stage 1.** Hunks are precisely what gutter markers
|
||||
need, and that is Stage 2's protocol work. Introducing a half hunk
|
||||
model here to serve one gesture would prejudge Stage 2's design from
|
||||
the wrong side.
|
||||
|
||||
**And what `d` actually SHOWS, which revision 2 left unstated.** "File,
|
||||
not hunk" is a scope, not a contract. A porcelain-v2 row carries an
|
||||
**XY** pair — X staged (index vs HEAD), Y unstaged (worktree vs index)
|
||||
— and the three plausible diffs answer three different questions:
|
||||
`git diff` shows only Y, `--cached` only X, and neither shows an
|
||||
untracked file at all.
|
||||
|
||||
*My vote: **`d` answers the lane's own question — "what have I
|
||||
changed?" — against `HEAD`:***
|
||||
|
||||
| row | `d` runs | why |
|
||||
|---|---|---|
|
||||
| staged, unstaged, or both | `git diff HEAD -- <path>` | one view of the total change; splitting X from Y is a staging UI, which is Stage 3 |
|
||||
| deleted | `git diff HEAD -- <path>` | shows the deletion; no special case needed |
|
||||
| renamed / copied | `git diff HEAD -- <orig> <current>` | v2 gives both paths; passing both is what lets rename detection render it as a rename rather than an unrelated add+delete |
|
||||
| **untracked** | `git diff --no-index -- /dev/null <path>` | **a normal diff shows nothing at all** for an untracked file. Without this case `d` is silently dead on the rows a user is most likely to press it on |
|
||||
| non-UTF-8 path | *refuses, with a message* | see Q#G-8 |
|
||||
|
||||
The `HEAD` choice is deliberate and is the one thing here I would most
|
||||
expect review to push back on: it is the right default for *reading*
|
||||
what changed, and the wrong one for *staging*, which is why it is
|
||||
correct for Stage 1 and will need revisiting when Stage 3 arrives.
|
||||
|
||||
**The exit-state contract, which revision 3 got wrong in two ways.**
|
||||
"Non-zero exit renders a failure row" is not correct for `git diff`.
|
||||
Both cases below were measured in a scratch repository, not inferred:
|
||||
|
||||
**(a) `--no-index` implies `--exit-code`.** It exits **1 when it
|
||||
successfully finds differences** — measured: `exit=1` for an untracked
|
||||
file against `/dev/null`. Under revision 3's predicate, *every*
|
||||
untracked diff — the case `--no-index` exists to serve — would have
|
||||
rendered a failure row instead of the diff it just produced.
|
||||
|
||||
So for the untracked path the success predicate is **exit ∈ {0, 1}**,
|
||||
rendering whatever came out; **exit ≥ 2 is a real failure**. That
|
||||
asymmetry is confined to the `--no-index` invocation and does not leak
|
||||
to the others, where non-zero still means failure.
|
||||
|
||||
**(b) An unborn repository has no `HEAD`.** Measured:
|
||||
`git diff HEAD -- <path>` exits **128** with `fatal: bad revision
|
||||
'HEAD'`. This is not an edge case — it is a freshly `git init`-ed
|
||||
repository with the first files staged, which is exactly when someone
|
||||
opens a status panel to see what they are about to commit.
|
||||
|
||||
*Policy: **detect once, then split**.*
|
||||
|
||||
**Detection needs no extra subprocess.** `--branch` already reports
|
||||
`# branch.oid (initial)` when `HEAD` is unborn — observed in the
|
||||
output this lane already parses. Revision 4 proposed a separate
|
||||
`git rev-parse --verify --quiet HEAD`; that is a second process for a
|
||||
fact the first one hands over.
|
||||
|
||||
**The reachable states, enumerated from a real unborn repository** —
|
||||
`git init`, stage three files, then edit one, delete one, and `git mv`
|
||||
one:
|
||||
|
||||
```
|
||||
# branch.oid (initial)
|
||||
1 AD ... ad.txt
|
||||
1 AM ... am.txt
|
||||
1 A. ... r_new.txt <- the `git mv`
|
||||
? untracked.txt
|
||||
```
|
||||
|
||||
Two findings fall straight out:
|
||||
|
||||
- **`AM` and `AD` are ordinary and carry BOTH states**, which is
|
||||
exactly the gap: `--cached` alone loses the worktree delta, plain
|
||||
`git diff` alone loses the staged base.
|
||||
- **Rename and copy CANNOT occur under an unborn `HEAD`.** The
|
||||
`git mv` produced `1 A. … r_new.txt` — an ordinary add of the new
|
||||
path, **not** a `2` record. With no `HEAD` there is nothing to
|
||||
rename *from*, so the rename/copy row class is unreachable here and
|
||||
needs no unborn policy. That is a case closed by evidence rather than
|
||||
handled speculatively.
|
||||
|
||||
| unborn row | `d` renders |
|
||||
|---|---|
|
||||
| `A.` staged only | one patch: `git diff --cached -- <path>` |
|
||||
| **`AM` staged + edited** | **two labelled patches** — *staged* `git diff --cached -- <path>`, then *unstaged* `git diff -- <path>` |
|
||||
| **`AD` staged + deleted** | **two labelled patches**, same pair; the second renders the deletion |
|
||||
| `.M` / `.D` unstaged only | one patch: `git diff -- <path>` |
|
||||
| `?` untracked | `git diff --no-index -- /dev/null <path>` (exit ∈ {0,1}) |
|
||||
| rename / copy | **unreachable** — see above |
|
||||
|
||||
All four `--cached` / plain invocations above were run against that
|
||||
repository and render the expected patches.
|
||||
|
||||
**The split is unborn-only, and that asymmetry is deliberate.** Once
|
||||
`HEAD` exists, `git diff HEAD` gives one total — which is the lane's
|
||||
question — and splitting it would be a staging UI (Stage 3). The split
|
||||
appears here only because there is no `HEAD` to total *against*.
|
||||
|
||||
The generated buffer carries a **header naming what it is showing**:
|
||||
*"no commits yet — split view: staged (index) above, unstaged
|
||||
(worktree) below"*. Revision 4's wording ("showing staged changes")
|
||||
would have described a single total-against-`HEAD` diff, which is
|
||||
precisely what this is not. A diff that silently answers a different
|
||||
question than the one asked is worse than one that says so — and a
|
||||
header that misdescribes a split view is the same failure in smaller
|
||||
type.
|
||||
|
||||
### Q#G-8 — non-UTF-8 paths: an honest boundary **(new in rev 3)**
|
||||
|
||||
Revision 2 listed a non-UTF-8 path in the witness corpus as though it
|
||||
were an end-to-end case. **It cannot be**, and the boundary is in the
|
||||
bindings: `pmacs.process.spawn` takes `args: Vec<String>`
|
||||
(`src/lua_bindings/mod.rs:8683`) and `pmacs.buffer.find_or_open` takes
|
||||
`path: String` (`:3564`). Both are Rust `String`, i.e. UTF-8 by
|
||||
construction. A path that is valid bytes but not valid UTF-8 can be
|
||||
*read* from git's `-z` output and *displayed*, but it cannot be passed
|
||||
back to `spawn` for a diff, nor opened.
|
||||
|
||||
*My vote: **parse it, show it, and refuse the gesture with a
|
||||
message***:
|
||||
|
||||
- the row **appears** in the panel, so the user is not lied to about
|
||||
what is modified;
|
||||
- **RET and `d` on that row report** that the path is not representable
|
||||
and do nothing else — a witnessed refusal, not a stack trace or a
|
||||
silent no-op;
|
||||
- **it is removed from the end-to-end promise.** The witness is
|
||||
parser-and-display **plus the refusal**, and the framing does not
|
||||
claim visiting works.
|
||||
|
||||
Making it work end-to-end means `OsString`/bytes through two binding
|
||||
boundaries — a real change to the Lua API surface, and not this lane's.
|
||||
|
||||
### Q#G-3 — what does the diff view render into?
|
||||
|
||||
*My vote: **a generated buffer**, reusing the generated-buffer
|
||||
immutability work (Stage 1 merged; that lane's Stage 2 is queued).
|
||||
Diff output is read-only text and that machinery exists.
|
||||
|
||||
**RESOLVED in rev 2 — there is no bundled `diff` grammar.**
|
||||
`BUILTIN_LANGUAGES` (`src/syntax.rs`) has no `diff` entry; checked, not
|
||||
assumed. **Stage 1 renders plain generated text**, and diff
|
||||
highlighting is later work needing a grammar first.
|
||||
|
||||
### Q#G-4 — what is configurable?
|
||||
|
||||
*My vote: **one setting to start** — `git.enabled` (boolean, default
|
||||
`true`), through the config registry. Resist more until there is use
|
||||
evidence; §11's grade is "partial (foundation only)" and adding five
|
||||
speculative settings is how a registry becomes noise.
|
||||
|
||||
### Q#G-5 — §9 attribution — **RESOLVED, and the answer is negative**
|
||||
|
||||
Revision 1 deferred this to implementation. That was wrong: it is
|
||||
answerable by reading, and deferring it would have meant discovering a
|
||||
known coherence cost *after* committing to the design.
|
||||
|
||||
**A spawned git process does not appear in `*workers*` at all.** That
|
||||
buffer is `builtin/runtime/async.lua`'s (`:490`) and lists **async
|
||||
jobs**; spawned processes live separately under `pmacs.process.list`.
|
||||
They are two of the four disjoint activity views §9 grades as
|
||||
"mechanism without identity".
|
||||
|
||||
So, stated plainly rather than dressed up:
|
||||
|
||||
- **This lane adds a fifth thing that runs in the background and is not
|
||||
attributable from one place.** That is a **negative** coherence impact
|
||||
against §9, and it is the honest cost of shipping git status before
|
||||
worker identity exists.
|
||||
- **Labelling the process is still required** — a clear label under
|
||||
`pmacs.process.list` is strictly better than an anonymous `git`. But
|
||||
**a label does not solve attribution**, and this document does not
|
||||
claim it does. The claim is only: do not make it worse than it has to
|
||||
be.
|
||||
- **The mitigation is bounded in time, not in kind.** These are
|
||||
short-lived reads, not long-running jobs; a `git status` that has not
|
||||
finished is a bug, not a background task a user needs to supervise.
|
||||
That is why the cost is acceptable *now* and would not be for
|
||||
Stage 3's push/pull.
|
||||
|
||||
## 6. Verification
|
||||
|
||||
- **Parsing, against a corpus rather than a case (Q#G-6):** modified,
|
||||
added, deleted, untracked, **renamed with both paths**, **copied**, a
|
||||
path with a space, and **a path with a newline** — the last is what
|
||||
`-z` buys, and a parser that passes only the space case is the one
|
||||
that ships broken.
|
||||
- **A non-UTF-8 path is parsed and displayed, and its gestures refuse
|
||||
with a message** (Q#G-8) — a witnessed refusal at the binding
|
||||
boundary, **not** an end-to-end visit.
|
||||
- **The argv carries `--no-optional-locks`** (Q#G-6), asserted
|
||||
structurally. A lock not taken cannot be observed directly, so the
|
||||
invocation is what gets pinned.
|
||||
- **`d` is witnessed on every row class** (Q#G-7): staged, unstaged,
|
||||
both, deleted, renamed, and **untracked** — the last because a normal
|
||||
`git diff` shows nothing there, so a missing `--no-index` case makes
|
||||
`d` silently dead exactly where it is most used.
|
||||
- **A copy is reported as a COPY, not a rename** (Q#G-7). Porcelain v2
|
||||
folds both into the one `2` record, so `kind` stays `"rename"` for
|
||||
both — every *behaviour* keyed on it is the same — and the
|
||||
distinction is made where it is a distinction: the diff header reads
|
||||
the `<Xscore>` field's leading `R`/`C` and says which one happened.
|
||||
The status row is left alone, because its `XY` prefix already reads
|
||||
`R.` against `C.`. Both classes are asserted, and so is the **argv**:
|
||||
the two-path `git diff HEAD -- <orig> <current>` is right for a copy
|
||||
and a rename alike, so a fix to what the user is *told* must not
|
||||
reach what runs. **Parser-level, deliberately** — the copy ROW is
|
||||
supplied through `_deliver_status` while the repository, the panel,
|
||||
the `d` dispatch and the spawned diff around it are real.
|
||||
|
||||
**The reason, narrowed after review.** This bullet used to say real
|
||||
`git` emits no `2 C` record "even under `status.renames=copies`".
|
||||
**That is too strong, and git's own documentation contradicts it** —
|
||||
`git-status(1)` lists `C` as *"copied (if config option
|
||||
status.renames is set to `copies`)"*. What the test measures is
|
||||
narrower: **for its fixture, whose copy source is left unchanged**,
|
||||
git reports `1 A.`. That is a fact about the fixture, and it is
|
||||
sufficient reason to craft the row — a weaker and true justification
|
||||
in place of a stronger false one. No mechanism is claimed for why an
|
||||
unchanged source is not offered as a candidate; that was never
|
||||
established.
|
||||
- **The untracked diff renders on exit 1**, not a failure row (Q#G-7a)
|
||||
— the case `--exit-code` semantics would otherwise break, and the
|
||||
one most likely to be "fixed" later by someone who reads exit 1 as an
|
||||
error.
|
||||
- **An unborn repository is witnessed end to end**, and the fixture is
|
||||
**`AM`** specifically — staged then edited again, the shape a first
|
||||
commit actually has partway through. `git init`, stage, edit, open
|
||||
the panel, press `d`, and get **two labelled patches** with the
|
||||
split-view header — not `fatal: bad revision 'HEAD'`, and not a
|
||||
single `--cached` patch that silently drops the worktree edit.
|
||||
**`AD` rides the same fixture**, since one repository can hold both.
|
||||
- **Unborn detection reads `# branch.oid (initial)`** from the status
|
||||
output already being parsed — asserted, so nobody later reintroduces
|
||||
a second `rev-parse` process for a fact already in hand.
|
||||
- **Rename/copy under an unborn `HEAD` is asserted UNREACHABLE**: the
|
||||
fixture `git mv`s a staged-but-uncommitted file and the parser sees a
|
||||
`1 A.` record, never a `2`. Pinned so a future reader does not
|
||||
"fix" the missing unborn rename policy by inventing one.
|
||||
- **Re-binding across a refresh does not error** (Q#G-7): two
|
||||
successive refreshes on a live panel, asserting `d` still works and
|
||||
no `DuplicateBinding` surfaced. This is the one that would have
|
||||
broken on every refresh.
|
||||
- **A `keys` table colliding with the fixed set is rejected at install
|
||||
time**, as is a prefix conflict.
|
||||
- **Selection is re-seated by the completion handler** (Q#G-1), across
|
||||
a refresh that reorders rows, and **falls back to line 1 without
|
||||
complaint when the selected path drops out of status** — the common
|
||||
case, not an error.
|
||||
- **The pure `parse_*` functions are tested without a repository**,
|
||||
which is the shape `pmacs-magit/status.lua` already proves works and
|
||||
the reason to port that separation rather than invent one.
|
||||
- **A repository fixture built with real `git`**, in a tempdir, and
|
||||
**bounded with `set_search_boundary`** — R8 was retired two commits
|
||||
ago and is precisely what happens when a fixture lets project
|
||||
detection escape into the developer's environment.
|
||||
- **The root rule is witnessed on a repository whose `ProjectKind` is
|
||||
NOT `Git`** — i.e. an ordinary language project with a `.git` beside
|
||||
its manifest. That is the case revision 1's `kind == "git"` gate
|
||||
would have failed, and this repository is one.
|
||||
- **Missing `git` on `PATH` is witnessed**, not assumed (Q#G-2), and
|
||||
surfaces guidance rather than silence.
|
||||
- **`g` is never a no-op** (Q#G-1): it re-renders and marks that work
|
||||
started, even mid-flight. A dead `g` is a defect `listview` already
|
||||
names.
|
||||
- **Concurrent refresh discards the stale generation** rather than
|
||||
racing — asserted by driving two refreshes and completing them out of
|
||||
order.
|
||||
- **Failure renders a row**, carrying exit code and stderr.
|
||||
- **The panel is a `listview` adopter**, asserted structurally, so a
|
||||
future re-implementation of list behaviour inside git code fails the
|
||||
test rather than passing review.
|
||||
- **No new interaction island** — `d` is bound buffer-locally through
|
||||
`listview`'s own binding path (Q#G-7), not a hardcoded interception.
|
||||
§6 stays at six shadows.
|
||||
|
||||
Gates via `scripts/gate --acceptance <the new suite>`.
|
||||
|
||||
**What this will NOT prove:** that background git work is attributable
|
||||
(Q#G-5 — it is not, by construction), or that the parser handles
|
||||
porcelain versions other than v2.
|
||||
|
||||
## 7. Not in scope
|
||||
|
||||
Gutter markers and any `DecorationKind`/`PROTOCOL_VERSION` change
|
||||
(Stage 2 — must be scheduled alone). Staging, commit, push, pull,
|
||||
branch operations, merge-conflict resolution. Blame. A `git` context in
|
||||
the menu vocabulary. Any git *library* dependency. Live refresh
|
||||
(Q#G-1). Fixing §9's worker identity — this lane makes it marginally
|
||||
worse and says so (Q#G-5). Any hunk model (Q#G-7). Modifying the
|
||||
`listview` primitive to carry an async contract — the better long-term
|
||||
answer, but it belongs in its own lane before this one, not inside its
|
||||
fifth adopter (Q#G-1). Changing `tests/fixtures/pmacs-magit/` or
|
||||
`tests/m8_6_acceptance.rs` (Q#G-0).
|
||||
|
||||
**A `listview` change IS in scope after all** (Q#G-7): an optional
|
||||
`keys` table on the open spec. Revision 2 said no primitive
|
||||
modification; that was false, because `d` cannot be bound from outside
|
||||
the primitive safely. The async-contract change stays out.
|
||||
|
||||
**One correction this lane should carry when it lands:** `COHERENCE.md`
|
||||
§15's "no Git integration anywhere in the tree" is literally false —
|
||||
`tests/fixtures/pmacs-magit/` exists. The *product* gap it describes is
|
||||
real; the sentence needs narrowing to say so.
|
||||
|
|
@ -231,33 +231,12 @@ all share one keymap:
|
|||
| `RET` / `SPC` | `listview.visit` — act on the item under the cursor |
|
||||
| `n` / `<down>` | `cursor.down` |
|
||||
| `p` / `<up>` | `cursor.up` |
|
||||
| `TAB` | `listview.toggle` — collapse/expand the tree node under the cursor; a panel with no tree rows delegates to `buffer.tab` |
|
||||
| `g` | `listview.refresh` — re-run the data source and re-render |
|
||||
| `q` | `listview.quit` — restore the buffer that was active before the panel opened |
|
||||
|
||||
(`TAB` arrived with the tree primitive and this table had not recorded
|
||||
it. Noted rather than quietly added: the omission predates the git lane
|
||||
that found it.)
|
||||
|
||||
Panels currently built on this: `*references*`, `*outline*`,
|
||||
`*lsp-help*` (hover docs), `*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`.
|
||||
`*lsp-help*` (hover docs). Header text always spells out the same
|
||||
`RET`/`n`/`p`/`g`/`q` legend inline.
|
||||
|
||||
`*buffer-list*` (`editor.list-buffers`, `C-x C-b`) uses its own
|
||||
keymap, layered on the same idiom, in `builtin/commands/default.lua`:
|
||||
|
|
|
|||
|
|
@ -1,240 +0,0 @@
|
|||
# LSP file watcher — framing
|
||||
|
||||
**Status: revision 2 — approved design (2026-08-10), plus two
|
||||
correctness findings from review OF THE IMPLEMENTATION.** The user ruled
|
||||
that D1 and D2 proceed with the walking explicitly surviving this lane;
|
||||
D3 gets its own framing. The acceptance bar for this lane is correctness
|
||||
and the leak, not "the flipping stops".
|
||||
|
||||
**Revision 2 records a review round against the code, not the design.
|
||||
Both findings are cases where the first fix was itself wrong**, and both
|
||||
were confirmed against the tree before being acted on:
|
||||
|
||||
- **P1 — the form must be read from the PATTERN, not from the union
|
||||
arm.** `resolve_watcher` returned `"absolute"` for *every* string, so
|
||||
a bare `*.txt` — a valid relative pattern under LSP 3.17, and how VS
|
||||
Code treats string watchers across workspace folders — was matched
|
||||
against `<base>/foo.txt` and could never fire. **That case worked
|
||||
before this lane touched it**, so the repair for #233 silently broke a
|
||||
live path while fixing another. A leading `/` is what makes a pattern
|
||||
absolute; classification now reads the string.
|
||||
- **P2 — a scan completing after cancellation still emitted.**
|
||||
`scan_tree` awaits `read_dir` once per directory, so the coroutine
|
||||
sits suspended for most of a tick with `_sleep` already cleared. A
|
||||
cancel arriving there sets `cancelled` and has no sleep to interrupt,
|
||||
and the resumed scan ran on to `did_change_watched_files` — one stale
|
||||
batch under the superseded pattern, which is a wrong-pattern
|
||||
notification the server acts on. Cancellation and liveness are now
|
||||
rechecked after the scan.
|
||||
|
||||
**F1's lesson repeated itself inside this lane.** The flat-pattern test
|
||||
constrains the RelativePattern **object** arm, so it said nothing about
|
||||
the **string** arm P1's regression lived in — the same
|
||||
tested-path/exercised-path split this framing opened by naming. Both
|
||||
findings now have tests, and both tests were mutation-checked: each
|
||||
fails only its own defect.
|
||||
|
||||
**A test seam was added, and is recorded here rather than buried.**
|
||||
`pmacs.lsp._after_scan_for_tests` is a production hook, nil in normal
|
||||
operation, that P2's witness requires: the race is a cancel landing
|
||||
during one of the scan's suspensions, which no arrangement of real
|
||||
timing produces on demand. Same device and justification as `git.lua`'s
|
||||
`_deliver_status`. It is handed the scan result deliberately — a test
|
||||
that cancels on any *other* scan passes with the fix deleted, because
|
||||
the loop would break at the post-sleep check and emit nothing anyway.
|
||||
|
||||
Answers issue #233. **Scope is D1 and D2 only** — the two bug-shaped
|
||||
defects. D3 (the polling cost) is named here, deferred with reasons, and
|
||||
gets its own framing.
|
||||
|
||||
## What is and is not a regression
|
||||
|
||||
**#232 is not at fault and nothing about it should be reverted.** The
|
||||
statusline activity indicator it added is *correct*: it renders real
|
||||
in-flight jobs from `AsyncRuntime::activity_summary`, and the jobs it
|
||||
names (`sleep 250ms`, `read_dir <path>`) are real. What changed on
|
||||
2026-08-09 is **visibility**, not behaviour.
|
||||
|
||||
The behaviour has been there since `1c25730` (2026-05-19). So the user-
|
||||
facing report — "the modeline flips several times a second" — is a
|
||||
three-month-old defect that became observable last week, and the fix
|
||||
belongs to the watcher, not the indicator.
|
||||
|
||||
Recorded plainly because the tempting move is to quiet the indicator,
|
||||
and that would delete the only instrument that found this.
|
||||
|
||||
## Verified against the tree at `0e4c58d`
|
||||
|
||||
Every claim below was read or executed this session, not carried from
|
||||
the issue.
|
||||
|
||||
- `FILE_WATCH_INTERVAL_MS = 250` (`lsp.lua:1924`); each watcher is one
|
||||
`pmacs.async` coroutine looping sleep → `scan_tree`
|
||||
(`lsp.lua:2060-2097`).
|
||||
- `scan_tree` builds `rel` from an empty prefix and calls
|
||||
`matches(rel)` — **relative** paths (`lsp.lua:2035-2056`).
|
||||
- **`walk` recurses into every directory unconditionally.** `matches`
|
||||
gates only whether an entry is *recorded*. A watcher that can never
|
||||
match still walks the whole tree every tick.
|
||||
- `resolve_watcher`'s string branch returns the pattern **unchanged**
|
||||
with the base guessed from an attached file's directory
|
||||
(`lsp.lua:2102-2116`).
|
||||
- `register_file_watchers` ends `file_watchers[skey][reg.id] = recs`
|
||||
with no cancellation of the outgoing list (`lsp.lua:2132`).
|
||||
- Job purposes are `format!("sleep {}ms", …)` (`async_runtime.rs:1027`)
|
||||
and `format!("read_dir {}", …)` (`:1178`).
|
||||
- The fake LSP registers **one** watcher, a `RelativePattern`
|
||||
`{ baseUri, pattern: "**/*.txt" }`, id `watch-1`
|
||||
(`pmacs_fake_lsp.rs:312-331`).
|
||||
|
||||
### The glob table, reproduced
|
||||
|
||||
Ran the tree's own `expand_braces` / `glob_one_to_pattern` /
|
||||
`glob_matcher` under LuaJIT. Output matches the issue exactly, compiled
|
||||
patterns included:
|
||||
|
||||
| glob | compiled | `main.go` | `go.mod` | absolute |
|
||||
|---|---|---|---|---|
|
||||
| `**/*.{mod,work}` | `^.-[^/]*%.mod$` | false | **true** | true |
|
||||
| `<abs>/goproj/**/*.{go,…}` | `^/tmp/goproj/.-[^/]*%.go$` | false | false | true |
|
||||
| `<abs>/rsproj/**/*.rs` | `^/tmp/rsproj/.-[^/]*%.rs$` | false | false | true |
|
||||
|
||||
## Two findings the issue does not carry, both of which shape the fix
|
||||
|
||||
### F1 — the existing test cannot discriminate this fix, in either direction
|
||||
|
||||
`**/*.txt` compiles to `^.-[^/]*%.txt$`, and `.-` spans `/`. Measured:
|
||||
it matches `a.txt`, `sub/a.txt`, `/base/a.txt` **and**
|
||||
`/base/sub/a.txt`. So `m4_24_workspace_did_change_watched_files` passes
|
||||
whether the matching subject is relative or absolute.
|
||||
|
||||
The issue says the tested path and the exercised path are disjoint. The
|
||||
sharper statement is that the existing test is **insensitive**: it
|
||||
cannot fail for D1 and it cannot confirm D1's fix. New coverage must use
|
||||
a pattern whose two readings disagree, or it will inherit the same
|
||||
blindness.
|
||||
|
||||
### F2 — the fix cannot simply "match absolute"; the form must be carried
|
||||
|
||||
Per LSP, a plain-string glob matches the **absolute** path while a
|
||||
`RelativePattern`'s pattern is relative to **its base**. Matching
|
||||
everything absolutely breaks the second. Measured on `*.txt`:
|
||||
|
||||
| subject | matches |
|
||||
|---|---|
|
||||
| `a.txt` (relative, correct for RelativePattern) | **true** |
|
||||
| `/base/a.txt` (absolute) | **false** |
|
||||
|
||||
`resolve_watcher` returns `(base, pattern)` and **discards which form it
|
||||
came from**, so both callers below it are already unable to tell. The
|
||||
fix therefore changes that function's contract — a third return value or
|
||||
an explicit record field — rather than only changing the subject string
|
||||
at the match site. A fix that ignores this trades rust-analyzer's six
|
||||
broken globs for every `RelativePattern` whose pattern does not begin
|
||||
`**/`.
|
||||
|
||||
## D1 — plain-string globs never match
|
||||
|
||||
**Consequences, as measured in the issue and confirmed by the table
|
||||
above:** rust-analyzer is never told about any file change (all six
|
||||
globs absolute); gopls is told about `go.mod`/`go.work` but never `.go`
|
||||
sources (only its relative glob matches).
|
||||
|
||||
**Fix:** match a plain-string glob against `base .. "/" .. rel`; keep a
|
||||
`RelativePattern` matched against `rel`. `resolve_watcher` gains the
|
||||
form in its return, and the record carries it.
|
||||
|
||||
The leading `**/` in gopls' relative glob compiles to `.-`, which spans
|
||||
`/`, so that glob keeps matching under the absolute subject — which is
|
||||
why one server's working case does not regress.
|
||||
|
||||
## D2 — re-registration leaks the previous coroutines
|
||||
|
||||
`file_watchers[skey][reg.id] = recs` replaces the record list without
|
||||
setting `cancelled` or cancelling the in-flight `_sleep`. The old
|
||||
coroutines poll until the server dies and are unreachable by
|
||||
`unregister_file_watchers`, which can only see what the table now holds.
|
||||
|
||||
**Reachable today**: rust-analyzer registers
|
||||
`workspace/didChangeWatchedFiles` **twice under the same id**, six
|
||||
watchers each, with no intervening unregister — 12 concurrent
|
||||
coroutines, six permanently uncancellable. The issue's 44.1/s dir-open
|
||||
rate against a ~270 ms period implies 12 watchers, so the leak is
|
||||
measured from outside the process, not only read from the source.
|
||||
|
||||
**Fix:** cancel the outgoing list before replacing it, with the same
|
||||
treatment `unregister_file_watchers` already applies.
|
||||
|
||||
## What this lane does NOT fix, stated so the report is not mistaken for closed
|
||||
|
||||
**The poll cost survives both fixes.** D1 makes matching correct and D2
|
||||
halves rust-analyzer's watcher count; neither stops the walk. After this
|
||||
lane, rust-analyzer still walks the entire tree every 250 ms — six times
|
||||
per tick instead of twelve — including `.git`, `target` and
|
||||
`node_modules`, at one async job per directory.
|
||||
|
||||
So the modeline will still show activity, at roughly half the rate. **If
|
||||
the acceptance bar for this lane is "the flipping stops", this lane does
|
||||
not meet it** and should not be started until D3 is framed. That is a
|
||||
ruling for the user, not an assumption to make quietly.
|
||||
|
||||
**Answered 2026-08-10: the user accepted this scope.** D1 and D2
|
||||
proceed; the walking is D3's problem, framed separately.
|
||||
|
||||
## D3 — deferred, with what was checked
|
||||
|
||||
Options named in the issue: coalesce a server's watchers into one scan;
|
||||
root the scan at the workspace rather than an attached file's directory;
|
||||
an ignore list; back off when nothing changes; or a real
|
||||
filesystem-notification primitive.
|
||||
|
||||
Checked while framing: **there is no `notify`/inotify dependency in the
|
||||
tree**, so the last option is a new crate *and* a new Rust primitive
|
||||
plus its Lua binding — not a small change. There is also **no existing
|
||||
ignore-list infrastructure** to reuse; `src/project.rs` knows `.git` as
|
||||
a *marker* name, not as something to skip.
|
||||
|
||||
D3 is a `COHERENCE.md` §9 concern — background work with no ownership
|
||||
model — and §9's own Stage 1 is the indicator that surfaced it.
|
||||
|
||||
## Verification
|
||||
|
||||
The suite must fail without each fix, which the existing suite cannot
|
||||
(F1). Planned:
|
||||
|
||||
- **A fake-LSP mode registering a plain-string ABSOLUTE glob**, with a
|
||||
pattern whose relative and absolute readings **disagree** — so the
|
||||
test fails today and passes after D1.
|
||||
- **A fake-LSP mode registering a `RelativePattern` whose pattern does
|
||||
not begin `**/`** (e.g. `*.txt` at the base). This is F2's guard: it
|
||||
passes today, and fails against a fix that matches everything
|
||||
absolutely. Without it, the obvious wrong fix is green.
|
||||
- **A re-registration mode**: the same id twice, no unregister. The
|
||||
witness is that the superseded watchers **stop**, asserted on
|
||||
observable polling rather than on internal table shape, since the
|
||||
defect is precisely that the old records are unreachable.
|
||||
- Existing `m4_24` kept and expected **unchanged** — it covers the
|
||||
working branch and its insensitivity is now recorded rather than
|
||||
mistaken for coverage.
|
||||
|
||||
Each new test is mutation-tested against the fix it names.
|
||||
|
||||
## Coherence impact (§20)
|
||||
|
||||
- **Journey steps**: none added; step 5's editing surface is affected
|
||||
only in that a correct watcher makes servers see edits they currently
|
||||
miss.
|
||||
- **Interaction islands**: none.
|
||||
- **Config registry**: no new setting. The interval stays a module
|
||||
constant; making it configurable would offer the user a knob for a
|
||||
defect rather than a preference, and D3 may remove the poll entirely.
|
||||
- **Background-work attribution (§9)**: this lane *reduces* unattributed
|
||||
background work but does not model it. D3 owns that, and the honest
|
||||
statement is that the indicator worked — it made three months of
|
||||
invisible churn visible on its first week.
|
||||
|
||||
## Gates
|
||||
|
||||
`./scripts/gate --acceptance m4_acceptance` plus the touched LSP
|
||||
acceptance suites; no `--protocol` (no wire change, no
|
||||
`PROTOCOL_VERSION` bump).
|
||||
|
|
@ -332,106 +332,6 @@ fn main() {
|
|||
});
|
||||
write_frame(&mut stdout, &req);
|
||||
}
|
||||
// Issue #233 D1: `filewatchabs` registers the same watcher
|
||||
// as a PLAIN-STRING glob — `<base>/**/*.txt`, the form
|
||||
// rust-analyzer and gopls actually send. Per LSP it matches
|
||||
// the file's ABSOLUTE path; its relative reading matches
|
||||
// nothing, so the mode discriminates the match subject.
|
||||
("initialized", _) if mode == "filewatchabs" => {
|
||||
let base = std::env::var("PMACS_FAKE_LSP_WATCH_BASE").unwrap_or_default();
|
||||
let req = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": 9301,
|
||||
"method": "client/registerCapability",
|
||||
"params": { "registrations": [{
|
||||
"id": "watch-abs",
|
||||
"method": "workspace/didChangeWatchedFiles",
|
||||
"registerOptions": { "watchers": [{
|
||||
"globPattern": format!("{base}/**/*.txt"),
|
||||
"kind": 7
|
||||
}] }
|
||||
}] }
|
||||
});
|
||||
write_frame(&mut stdout, &req);
|
||||
}
|
||||
// Issue #233 review P1 guard: `filewatchbare` registers a
|
||||
// BARE STRING with no base and no leading `/` — `*.txt`.
|
||||
// The string arm and the `filewatchflat` arm below carry the
|
||||
// same pattern deliberately: `flat` proves a
|
||||
// RelativePattern stays relative, and this proves the
|
||||
// classification is read from THE PATTERN rather than from
|
||||
// the union arm it arrived in. The first fix for #233
|
||||
// called every string absolute, which matched this against
|
||||
// `<base>/foo.txt` and broke a case that had worked since
|
||||
// May. Without this mode that regression is invisible.
|
||||
("initialized", _) if mode == "filewatchbare" => {
|
||||
let req = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": 9304,
|
||||
"method": "client/registerCapability",
|
||||
"params": { "registrations": [{
|
||||
"id": "watch-bare",
|
||||
"method": "workspace/didChangeWatchedFiles",
|
||||
"registerOptions": { "watchers": [{
|
||||
"globPattern": "*.txt",
|
||||
"kind": 7
|
||||
}] }
|
||||
}] }
|
||||
});
|
||||
write_frame(&mut stdout, &req);
|
||||
}
|
||||
// Issue #233 F2 guard: `filewatchflat` registers a
|
||||
// RelativePattern whose pattern has no leading `**/`
|
||||
// (`*.txt` at the base). It matches base-level files
|
||||
// RELATIVELY and no absolute path at all, so a fix that
|
||||
// matches every form against the absolute path goes red.
|
||||
("initialized", _) if mode == "filewatchflat" => {
|
||||
let base = std::env::var("PMACS_FAKE_LSP_WATCH_BASE").unwrap_or_default();
|
||||
let req = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": 9302,
|
||||
"method": "client/registerCapability",
|
||||
"params": { "registrations": [{
|
||||
"id": "watch-flat",
|
||||
"method": "workspace/didChangeWatchedFiles",
|
||||
"registerOptions": { "watchers": [{
|
||||
"globPattern": {
|
||||
"baseUri": format!("file://{base}"),
|
||||
"pattern": "*.txt"
|
||||
},
|
||||
"kind": 7
|
||||
}] }
|
||||
}] }
|
||||
});
|
||||
write_frame(&mut stdout, &req);
|
||||
}
|
||||
// Issue #233 D2: `filewatchrereg` registers the SAME id
|
||||
// twice with no unregister between — `**/*.old` then
|
||||
// `**/*.new` — exactly rust-analyzer's shape. The second
|
||||
// registration must supersede the first: only `.new`
|
||||
// events may ever reach `.received`.
|
||||
("initialized", _) if mode == "filewatchrereg" => {
|
||||
let base = std::env::var("PMACS_FAKE_LSP_WATCH_BASE").unwrap_or_default();
|
||||
for (rid, pattern) in [(9303, "**/*.old"), (9304, "**/*.new")] {
|
||||
let req = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": rid,
|
||||
"method": "client/registerCapability",
|
||||
"params": { "registrations": [{
|
||||
"id": "watch-re",
|
||||
"method": "workspace/didChangeWatchedFiles",
|
||||
"registerOptions": { "watchers": [{
|
||||
"globPattern": {
|
||||
"baseUri": format!("file://{base}"),
|
||||
"pattern": pattern
|
||||
},
|
||||
"kind": 7
|
||||
}] }
|
||||
}] }
|
||||
});
|
||||
write_frame(&mut stdout, &req);
|
||||
}
|
||||
}
|
||||
("initialized", _) => {}
|
||||
// T M4.5: the client's file-watch notifications. Append
|
||||
// `type uri` lines to `<base>/.received` as a test
|
||||
|
|
|
|||
|
|
@ -1827,7 +1827,7 @@ fn open_initial_target(
|
|||
let (buffer_id, fire) = match resolved {
|
||||
crate::editor_core::ResolvedTarget::Directory { path } => {
|
||||
let dest = editor
|
||||
.capture_view_destination(frontend_id, origin_window)
|
||||
.capture_directory_destination(frontend_id, origin_window)
|
||||
.ok_or_else(|| format!("cannot open {}: no document window", path.display()))?;
|
||||
editor.dispatch_directory_open(&path, dest);
|
||||
editor.reconcile_panel_layout(frontend_id);
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@ use unicode_width::UnicodeWidthStr;
|
|||
|
||||
use crate::async_runtime::SharedAsyncRuntime;
|
||||
use crate::cell::{CellCoord, CellSize};
|
||||
use crate::editor_core::{CommitContract, EditorCore, GeometryUpdate};
|
||||
use crate::editor_core::{EditorCore, GeometryUpdate};
|
||||
use crate::frontend::{Event, Frontend, KeyEvent, KeyEventKind, MouseEvent, install_panic_hook};
|
||||
use crate::key::{Chord, display_sequence};
|
||||
use crate::keymap_stack::{Action, KeyDispatcher};
|
||||
|
|
@ -119,34 +119,20 @@ impl ScopedFrontend {
|
|||
}
|
||||
|
||||
/// Enter a background frontend scope, also swapping the core's
|
||||
/// ambient `active_frontend` and **pushing** `contract`. All three are
|
||||
/// restored on drop, on every exit path including a raising callback.
|
||||
///
|
||||
/// The frontend comes from `contract.destination` rather than being
|
||||
/// passed separately: a scope entered for one frontend while carrying
|
||||
/// another's destination would let the placement guard check the
|
||||
/// wrong window, and there is no caller that wants them to differ.
|
||||
///
|
||||
/// **The contract is pushed, not swapped (Q#DC-2, revision 9).** The
|
||||
/// frontend override and the ambient frontend are *substitutions* —
|
||||
/// an inner scope means what it says and the outer one resumes
|
||||
/// afterwards — but a contract is a *restriction*, and a nested scope
|
||||
/// masking one would suspend it for the extent of the inner body
|
||||
/// while the outer commit's relaxed preflight still depended on it.
|
||||
/// See [`crate::editor_core::EditorCore::push_commit_contract`].
|
||||
/// ambient `active_frontend`. Both are restored on drop, on every
|
||||
/// exit path including a raising callback.
|
||||
pub(crate) fn enter(
|
||||
&self,
|
||||
core: &SharedCore,
|
||||
commit_scope: &CommitScopeActive,
|
||||
contract: CommitContract,
|
||||
frontend_id: FrontendId,
|
||||
) -> ScopedFrontendGuard {
|
||||
let frontend_id = contract.destination.frontend;
|
||||
let previous = self.0.replace(Some(frontend_id));
|
||||
let (previous_active, contract_depth) = {
|
||||
let previous_active = {
|
||||
let mut core = core.borrow_mut();
|
||||
let was = core.active_frontend;
|
||||
core.active_frontend = frontend_id;
|
||||
(was, core.push_commit_contract(contract))
|
||||
was
|
||||
};
|
||||
let previous_commit = commit_scope.0.replace(true);
|
||||
ScopedFrontendGuard {
|
||||
|
|
@ -154,7 +140,6 @@ impl ScopedFrontend {
|
|||
core: core.clone(),
|
||||
previous,
|
||||
previous_active,
|
||||
contract_depth,
|
||||
commit_scope: commit_scope.clone(),
|
||||
previous_commit,
|
||||
}
|
||||
|
|
@ -166,15 +151,6 @@ pub(crate) struct ScopedFrontendGuard {
|
|||
core: SharedCore,
|
||||
previous: Option<FrontendId>,
|
||||
previous_active: FrontendId,
|
||||
/// Contract-stack depth to truncate back to (Q#DC-2). Held here
|
||||
/// rather than on a separate guard so a `"panel"` profile can never
|
||||
/// outlive the body that declared it and govern an unrelated later
|
||||
/// display.
|
||||
///
|
||||
/// A depth rather than a saved contract because nesting **composes**
|
||||
/// (revision 9): this scope adds one restriction and removes exactly
|
||||
/// that one, leaving every enclosing commit's still in force.
|
||||
contract_depth: usize,
|
||||
/// Cleared together with the scope, so an awaiting callback cannot
|
||||
/// leave `await` refused after the commit ends (Q#JR14b).
|
||||
commit_scope: CommitScopeActive,
|
||||
|
|
@ -184,11 +160,7 @@ pub(crate) struct ScopedFrontendGuard {
|
|||
impl Drop for ScopedFrontendGuard {
|
||||
fn drop(&mut self) {
|
||||
self.scope.0.set(self.previous);
|
||||
{
|
||||
let mut core = self.core.borrow_mut();
|
||||
core.active_frontend = self.previous_active;
|
||||
core.exit_commit_contract(self.contract_depth);
|
||||
}
|
||||
self.core.borrow_mut().active_frontend = self.previous_active;
|
||||
self.commit_scope.0.set(self.previous_commit);
|
||||
}
|
||||
}
|
||||
|
|
@ -797,20 +769,6 @@ 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
|
||||
|
|
@ -1261,32 +1219,22 @@ impl EditorState {
|
|||
}
|
||||
|
||||
/// Capture the destination a directory open must commit to
|
||||
/// (Q#JR14), or `None` when `window` is gone.
|
||||
/// (Q#JR14), or `None` when `frontend` has no document window.
|
||||
///
|
||||
/// Synchronous by necessity: the listing settles a tick or more
|
||||
/// later, and by then the ambient frontend, selected window, and
|
||||
/// active buffer may all name something else.
|
||||
///
|
||||
/// Takes the window **explicitly**, unlike
|
||||
/// [`crate::editor_core::EditorCore::capture_view_destination`],
|
||||
/// which reads the ambient one. Both directory callers already hold
|
||||
/// the exact window the open was resolved against — the daemon's is
|
||||
/// read before `resolve_target_buffer` runs (Q#BP11b) — and
|
||||
/// recapturing it from ambient state here would discard that.
|
||||
/// A directory open therefore always yields a full document pair,
|
||||
/// which is why this keeps returning `Option` rather than the total
|
||||
/// capture's `ViewDestination`.
|
||||
pub(crate) fn capture_view_destination(
|
||||
pub(crate) fn capture_directory_destination(
|
||||
&self,
|
||||
frontend: crate::protocol::FrontendId,
|
||||
window: crate::window::WindowId,
|
||||
) -> Option<crate::editor_core::ViewDestination> {
|
||||
) -> Option<crate::editor_core::DirectoryDestination> {
|
||||
let core = self.core.borrow();
|
||||
let buffer = core.windows.get(&window)?.buffer_id;
|
||||
Some(crate::editor_core::ViewDestination {
|
||||
Some(crate::editor_core::DirectoryDestination {
|
||||
frontend,
|
||||
window: Some(window),
|
||||
buffer: Some(buffer),
|
||||
window,
|
||||
buffer,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -1310,7 +1258,7 @@ impl EditorState {
|
|||
.borrow()
|
||||
.primary_document_window(crate::protocol::FrontendId::LOCAL);
|
||||
let dest = window.and_then(|window| {
|
||||
self.capture_view_destination(crate::protocol::FrontendId::LOCAL, window)
|
||||
self.capture_directory_destination(crate::protocol::FrontendId::LOCAL, window)
|
||||
});
|
||||
let Some(dest) = dest else {
|
||||
self.core.borrow_mut().status =
|
||||
|
|
@ -1340,13 +1288,13 @@ impl EditorState {
|
|||
pub(crate) fn dispatch_directory_open(
|
||||
&mut self,
|
||||
path: &std::path::Path,
|
||||
dest: crate::editor_core::ViewDestination,
|
||||
dest: crate::editor_core::DirectoryDestination,
|
||||
) {
|
||||
let display = path.display().to_string();
|
||||
let args = {
|
||||
let lua = self.lua_host.lua();
|
||||
let destination =
|
||||
match lua.create_userdata(crate::lua_bindings::ViewDestinationLua(dest)) {
|
||||
match lua.create_userdata(crate::lua_bindings::DirectoryDestinationLua(dest)) {
|
||||
Ok(userdata) => mlua::Value::UserData(userdata),
|
||||
Err(error) => {
|
||||
self.core.borrow_mut().status = format!("cannot open {display}: {error}");
|
||||
|
|
|
|||
|
|
@ -130,119 +130,39 @@ pub enum ResolvedTarget {
|
|||
},
|
||||
}
|
||||
|
||||
/// Where an asynchronous continuation's result belongs, captured
|
||||
/// **synchronously** at request time (Journey Stage 1a, Q#JR14;
|
||||
/// generalized by `docs/destination-capture-framing.md`).
|
||||
/// Where a directory open was requested, captured **synchronously** at
|
||||
/// resolve time (Journey Stage 1a, Q#JR14).
|
||||
///
|
||||
/// The work that satisfies such a request is asynchronous (a directory
|
||||
/// listing is worker-dispatched and must be awaited; so is a `git`
|
||||
/// invocation), so the code that finally builds and displays the result
|
||||
/// runs a tick or more later — outside interactive dispatch, where
|
||||
/// `pmacs.window.*` acts on the *ambient* frontend by documented design
|
||||
/// (`builtin/runtime/dired.lua`). Without a captured destination, a
|
||||
/// second frontend dispatching in the meantime silently redirects the
|
||||
/// result.
|
||||
/// The listing that satisfies a directory open is asynchronous
|
||||
/// (`pmacs.fs.read_dir` is worker-dispatched and must be awaited), so the
|
||||
/// code that finally builds and displays the listing runs a tick or more
|
||||
/// later — outside interactive dispatch, where `pmacs.window.*` acts on
|
||||
/// the *ambient* frontend by documented design (`builtin/runtime/dired.lua`).
|
||||
/// Without a captured destination, a second frontend dispatching in the
|
||||
/// meantime silently redirects the listing.
|
||||
///
|
||||
/// The fields are load-bearing, and the document pair is **optional**
|
||||
/// (Q#DC-4) because a panel result needs only a live frontend *when it
|
||||
/// really lands in a panel*, so a frontend whose document window has
|
||||
/// gone can still host one:
|
||||
/// All three fields are load-bearing:
|
||||
///
|
||||
/// * `frontend` — the scope the commit must run in. Always present.
|
||||
/// * `frontend` — the scope the commit must run in.
|
||||
/// * `window` — the exact destination; the ambient selected window is
|
||||
/// not it. Absent when the frontend had no document window at capture
|
||||
/// time.
|
||||
/// not it.
|
||||
/// * `buffer` — what that window held at capture time, so **stale
|
||||
/// intent loses to the user** (Q#JR14c). A user who replaced the
|
||||
/// buffer while the work was in flight is newer information than the
|
||||
/// launch argument, and must not be overwritten. Present exactly when
|
||||
/// `window` is.
|
||||
///
|
||||
/// The pair is set or cleared together — see
|
||||
/// [`EditorCore::capture_view_destination`], which is the only place
|
||||
/// that reads them off ambient state.
|
||||
///
|
||||
/// Which of those a commit actually requires is the **profile**, chosen
|
||||
/// at `pmacs.window.commit_to` rather than at capture (Q#DC-2/Q#DC-5):
|
||||
/// the document profile requires all of them, and the panel profile
|
||||
/// requires only a live `frontend` **while its result really lands in a
|
||||
/// panel**. A side request that falls back into a document window *is* a
|
||||
/// document replacement, so the panel profile's relaxed preflight is
|
||||
/// taken only when the fallback cannot happen, and the mutations that
|
||||
/// would manufacture one mid-commit are refused at the attempt
|
||||
/// (`EditorCore::panel_commit_dedication_refusal`). Capture stays
|
||||
/// profile-blind so a caller does not have to know at capture time what
|
||||
/// it will do at commit time.
|
||||
/// buffer while the listing was in flight is newer information than
|
||||
/// the launch argument, and must not be overwritten.
|
||||
///
|
||||
/// Exposed to Lua only as nonconstructible userdata (Q#JR14d): as a
|
||||
/// table, the *same* value is handed to every resolver listener in turn,
|
||||
/// so one could mutate it and then decline — redirecting later listeners
|
||||
/// — and any Lua could fabricate a plausible triple.
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub struct ViewDestination {
|
||||
/// Frontend that requested the work.
|
||||
pub struct DirectoryDestination {
|
||||
/// Frontend that requested the directory.
|
||||
pub frontend: FrontendId,
|
||||
/// Window the result must land in, when there is one.
|
||||
pub window: Option<WindowId>,
|
||||
/// Window the listing must land in.
|
||||
pub window: WindowId,
|
||||
/// Buffer that window held at capture time (stale-intent check).
|
||||
pub buffer: Option<BufferId>,
|
||||
}
|
||||
|
||||
/// Which of `commit_to`'s preconditions a body actually depends on
|
||||
/// (Q#DC-2).
|
||||
///
|
||||
/// A **closed** set of two, not an open string namespace: a third
|
||||
/// profile is a decision about what a continuation may depend on, not a
|
||||
/// spelling. Chosen at `commit_to` rather than at capture, because the
|
||||
/// caller knows what it is about to do only then.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum CommitProfile {
|
||||
/// The body replaces the captured window's buffer: **all four**
|
||||
/// preflight checks apply. This is what an omitted profile means, so
|
||||
/// every caller written before the profile existed keeps exactly the
|
||||
/// guarantees it was written against.
|
||||
Document,
|
||||
/// The body puts its result in a bottom panel rather than in the
|
||||
/// captured document window, and so does not depend on checks 2–4 —
|
||||
/// **for as long as its result really lands in a panel**. The
|
||||
/// preflight grants the relaxation only when a fallback into a
|
||||
/// document window is impossible ([`EditorCore::commit_destination_refusal`]),
|
||||
/// and what keeps that measurement true for the body's whole extent
|
||||
/// is that the mutations which would manufacture a fallback are
|
||||
/// refused at the attempt
|
||||
/// (`EditorCore::panel_commit_dedication_refusal`).
|
||||
Panel,
|
||||
}
|
||||
|
||||
/// The contract a `commit_to` body is running under, published on the
|
||||
/// core so the mutations that could invalidate it can consult it
|
||||
/// (Q#DC-2, revisions 8 and 9).
|
||||
///
|
||||
/// **Why this exists rather than a preflight prediction.** Revision 6
|
||||
/// tried to decide at preflight whether a `"panel"` commit's placement
|
||||
/// could fall back into a document window, on the argument that nothing
|
||||
/// could change in between because the body cannot `await`. Refusing
|
||||
/// `await` stops another coroutine interleaving; it says nothing about
|
||||
/// the body itself, which is arbitrary Lua running synchronously and can
|
||||
/// change the very state the snapshot measured — obtain the panel, set
|
||||
/// it `dedicated`, then request a side display. A snapshot cannot bind
|
||||
/// that. So the preflight stays where it is and the contract is what
|
||||
/// lets those mutations be **refused at the attempt**, which is the only
|
||||
/// point early enough to leave nothing behind
|
||||
/// (`EditorCore::panel_commit_dedication_refusal`).
|
||||
///
|
||||
/// Pushed and popped by the same guard that scopes the frontend, so the
|
||||
/// two can never disagree about whether a commit is on the stack.
|
||||
/// **Pushed** rather than swapped: a contract is a restriction, and a
|
||||
/// nested `commit_to` must add to the ones in force rather than mask
|
||||
/// them for the extent of its body
|
||||
/// (`EditorCore::push_commit_contract`, revision 9).
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
pub struct CommitContract {
|
||||
/// The destination the continuation captured.
|
||||
pub destination: ViewDestination,
|
||||
/// What that continuation declared it depends on.
|
||||
pub profile: CommitProfile,
|
||||
pub buffer: BufferId,
|
||||
}
|
||||
|
||||
/// A `display_buffer` request (Q#BP3).
|
||||
|
|
@ -700,34 +620,6 @@ pub struct EditorCore {
|
|||
/// slot; the producer clears any untaken record when the fan-out
|
||||
/// returns.
|
||||
typed_edit_armed: Option<(FrontendId, TypedEditRecord)>,
|
||||
/// Every `commit_to` contract currently on the stack, outermost
|
||||
/// first (Q#DC-2, revision 9).
|
||||
///
|
||||
/// **A STACK, NOT A SLOT, and that is the whole of revision 9's
|
||||
/// fix.** Revision 8 held one contract and had a nested `commit_to`
|
||||
/// replace it for the inner body's extent. That MASKED the enclosing
|
||||
/// contract: an outer `"panel"` commit took the relaxed preflight,
|
||||
/// its body opened a nested `"document"` commit, and inside that
|
||||
/// nested body the very mutation the outer commit's relaxation
|
||||
/// depends on — dedicating the one side slot — was no longer refused,
|
||||
/// because the guard consulted only the innermost contract. The outer
|
||||
/// commit then resumed and fell back into the document window,
|
||||
/// overwriting a newer buffer, which is exactly the defect the panel
|
||||
/// profile's relaxation was made safe against.
|
||||
///
|
||||
/// So restrictions **compose** rather than replace: a contract is
|
||||
/// pushed for its body and popped after, and every restriction
|
||||
/// pushed by an enclosing commit stays in force for the whole of it,
|
||||
/// nested scopes included. See
|
||||
/// [`Self::panel_commit_dedication_refusal`], the one reader.
|
||||
///
|
||||
/// Private and `pub(crate)`-free on purpose: entries are pushed only
|
||||
/// by [`crate::editor::ScopedFrontend::enter`]'s guard, which
|
||||
/// truncates back to its own depth on every exit path including a
|
||||
/// raising body. Nothing outside this crate can push one, so a
|
||||
/// `"panel"` profile is not something Lua can claim for a placement
|
||||
/// it did not commit to.
|
||||
commit_contracts: Vec<CommitContract>,
|
||||
}
|
||||
|
||||
impl EditorCore {
|
||||
|
|
@ -782,42 +674,9 @@ impl EditorCore {
|
|||
query_replace: None,
|
||||
typed_edit_pending: None,
|
||||
typed_edit_armed: None,
|
||||
commit_contracts: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Push `contract` for the duration of a `commit_to` body, returning
|
||||
/// the depth [`Self::exit_commit_contract`] must truncate back to.
|
||||
///
|
||||
/// **Pushes rather than replaces (revision 9).** A nested `commit_to`
|
||||
/// adds its contract to the ones already in force instead of masking
|
||||
/// them, so an enclosing `"panel"` commit's mutation refusal covers
|
||||
/// its *whole* body — including the part that runs inside a nested
|
||||
/// commit of a different profile. Replacing was revision 8's defect:
|
||||
/// the guard read only the innermost contract, so a nested
|
||||
/// `"document"` commit was a hole through which the body could
|
||||
/// dedicate the side slot the outer relaxation rests on.
|
||||
///
|
||||
/// Crate-private and paired with the frontend scope rather than a
|
||||
/// standalone setter: a contract that could be installed without
|
||||
/// being popped would outlive its body and silently govern the next
|
||||
/// unrelated display.
|
||||
pub(crate) fn push_commit_contract(&mut self, contract: CommitContract) -> usize {
|
||||
let depth = self.commit_contracts.len();
|
||||
self.commit_contracts.push(contract);
|
||||
depth
|
||||
}
|
||||
|
||||
/// Drop every contract pushed at or above `depth`.
|
||||
///
|
||||
/// Truncation rather than a bare `pop` so the guard restores exactly
|
||||
/// the set that was in force when it was entered, whatever happened
|
||||
/// in between — the same reason the frontend scope saves a value
|
||||
/// rather than assuming it can invert its own change.
|
||||
pub(crate) fn exit_commit_contract(&mut self, depth: usize) {
|
||||
self.commit_contracts.truncate(depth);
|
||||
}
|
||||
|
||||
/// Build a core from raw bytes under `name`. Used by tests.
|
||||
/// Replaces the scratch buffer's content; the active window is
|
||||
/// retained.
|
||||
|
|
@ -3183,143 +3042,6 @@ impl EditorCore {
|
|||
self.non_side_target(fid).ok()
|
||||
}
|
||||
|
||||
/// Capture where `fid`'s next asynchronous result belongs (Q#JR14,
|
||||
/// generalized by Q#DC-1/Q#DC-4).
|
||||
///
|
||||
/// **Profile-blind and total**: it records what is there rather than
|
||||
/// what a caller intends to do later, and it never fails while a
|
||||
/// frontend id exists. A frontend with no document window yields a
|
||||
/// destination carrying only `frontend` — enough for a panel commit
|
||||
/// that really places in the panel, and refused by a document commit
|
||||
/// (or by a panel commit on a frontend where a side request would
|
||||
/// fall back into a document window, see
|
||||
/// [`Self::commit_destination_refusal`]) with a reason naming the
|
||||
/// missing window. Returning `None` here instead would push the
|
||||
/// caller back onto ambient state, which is the misrouting the
|
||||
/// capture exists to remove.
|
||||
///
|
||||
/// The document pair is set or cleared **together**: a window whose
|
||||
/// entry has gone yields neither half, so no consumer has to handle
|
||||
/// a window without its captured buffer.
|
||||
///
|
||||
/// **How reachable the empty pair is, stated because the framing
|
||||
/// implies more than the tree does.** Q#BP6 says a frontend layout
|
||||
/// always retains at least one non-side window, and
|
||||
/// [`Self::non_side_target`] carries a `debug_assert!` that fires
|
||||
/// when one does not — so with that invariant held, a *registered*
|
||||
/// frontend always has a live document window and this branch is
|
||||
/// **defensive** rather than routine. It stays because the
|
||||
/// alternative is a capture that can fail, and a caller that can
|
||||
/// fail is a caller that falls back to ambient state.
|
||||
#[must_use]
|
||||
pub fn capture_view_destination(&self, fid: FrontendId) -> ViewDestination {
|
||||
let pair = self
|
||||
.primary_document_window(fid)
|
||||
.and_then(|window| Some((window, self.windows.get(&window)?.buffer_id)));
|
||||
ViewDestination {
|
||||
frontend: fid,
|
||||
window: pair.map(|(window, _)| window),
|
||||
buffer: pair.map(|(_, buffer)| buffer),
|
||||
}
|
||||
}
|
||||
|
||||
/// The document profile's preconditions on a captured destination —
|
||||
/// Q#DC-2's checks 2, 3 and 4, plus Q#DC-4's missing-pair case.
|
||||
///
|
||||
/// **One rule in one place**, because it is now evaluated from two
|
||||
/// sites and they must not drift: `commit_to`'s preflight runs it
|
||||
/// before the body, and [`Self::display_buffer`] runs it again when a
|
||||
/// `"panel"` commit's side request actually falls back into a
|
||||
/// document window. A second copy of these three checks is how the
|
||||
/// backstop ends up subtly weaker than the thing it backs.
|
||||
///
|
||||
/// Check 1 (the requesting frontend still has a layout) is
|
||||
/// deliberately *not* here: it is shared by both profiles rather than
|
||||
/// specific to the document one, and the placement path cannot fail
|
||||
/// it — it is placing into that very frontend.
|
||||
#[must_use]
|
||||
pub fn document_destination_refusal(&self, dest: &ViewDestination) -> Option<String> {
|
||||
let Some(window) = dest.window else {
|
||||
// The capture found no document window (Q#DC-4). A refusal
|
||||
// rather than a raise, so it joins the others as one more
|
||||
// thing the destination can fail to satisfy and an adopter
|
||||
// handles it the same way.
|
||||
return Some(
|
||||
"destination has no document window (capture it from a frontend that has \
|
||||
one, or commit with the \"panel\" profile)"
|
||||
.to_string(),
|
||||
);
|
||||
};
|
||||
// 2. The destination window is still live in the frontend.
|
||||
if !self
|
||||
.views
|
||||
.get(&dest.frontend)
|
||||
.is_some_and(|view| view.layout.iter_ids().contains(&window))
|
||||
{
|
||||
return Some(format!("window {} is gone", window.raw()));
|
||||
}
|
||||
// 3. Stale intent (Q#JR14c): the user replaced the buffer while
|
||||
// the work was in flight. Their action is newer information
|
||||
// than the request, so the request loses.
|
||||
if self
|
||||
.windows
|
||||
.get(&window)
|
||||
.is_some_and(|w| Some(w.buffer_id) != dest.buffer)
|
||||
{
|
||||
return Some(format!("window {} now shows another buffer", window.raw()));
|
||||
}
|
||||
// 4. Replaceability (Q#JR14f). `None` because the replacement
|
||||
// does not exist yet — passing the captured buffer would
|
||||
// approve a window dedicated to *it*, and the handler's
|
||||
// different buffer would be refused later, after mutating.
|
||||
if !self.window_accepts_buffer(window, None) {
|
||||
return Some(format!("window {} is dedicated", window.raw()));
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// `commit_to`'s **preflight**: what a commit under `profile` can be
|
||||
/// refused for before its body runs at all (Q#DC-2).
|
||||
///
|
||||
/// Ordering is the whole point of preflighting rather than validating
|
||||
/// at display time: an async body mutates real state (claims a
|
||||
/// buffer, registers a handle, paints) long before it reaches any
|
||||
/// call that could refuse, so a late refusal leaves debris behind.
|
||||
///
|
||||
/// **This measurement is only half the guarantee.** For the panel
|
||||
/// profile it can read only the state that holds *now*, and the body
|
||||
/// is arbitrary synchronous Lua that could change it — dedicate the
|
||||
/// side slot, then request a side display. What keeps the
|
||||
/// measurement true is that those mutations are **refused at the
|
||||
/// attempt**, for the body's whole extent including any nested
|
||||
/// `commit_to` (`Self::panel_commit_dedication_refusal`). Refusing
|
||||
/// at the placement boundary instead was revision 7, and it was
|
||||
/// rejected: by then the body has allocated buffers, handles and
|
||||
/// paint, which is the debris this preflight exists to avoid.
|
||||
#[must_use]
|
||||
pub fn commit_destination_refusal(
|
||||
&self,
|
||||
dest: &ViewDestination,
|
||||
profile: CommitProfile,
|
||||
) -> Option<String> {
|
||||
// 1. The requesting frontend still has a layout. Required by
|
||||
// BOTH profiles, because a frontend that is gone can host
|
||||
// nothing.
|
||||
if !self.views.contains_key(&dest.frontend) {
|
||||
return Some("requesting frontend is gone".to_string());
|
||||
}
|
||||
// 2, 3 and 4 are DELIBERATELY OMITTED for a panel result that
|
||||
// really lands in a panel, not overlooked (Q#DC-2): it does not
|
||||
// occupy the captured document window, does not replace its
|
||||
// buffer, and does not need it to exist, so each would refuse for
|
||||
// a reason unrelated to what the continuation does. Every one of
|
||||
// the three is pinned as NOT refusing under this profile.
|
||||
if profile == CommitProfile::Panel && !self.panel_placement_can_fall_back(dest.frontend) {
|
||||
return None;
|
||||
}
|
||||
self.document_destination_refusal(dest)
|
||||
}
|
||||
|
||||
/// [`Self::primary_document_window`]'s buffer, falling back to the
|
||||
/// focused window's when the layout is degenerate.
|
||||
#[must_use]
|
||||
|
|
@ -3538,23 +3260,6 @@ impl EditorCore {
|
|||
}
|
||||
other => other,
|
||||
};
|
||||
// Q#DC-2 (revision 8). A `Restore` carries the OUTGOING
|
||||
// presentation's `dedicated` flag (see `apply_placement`), so
|
||||
// quitting the panel can re-dedicate the one slot without any
|
||||
// `dedicated` argument appearing at the call site. Refused for
|
||||
// the same reason and at the same point as the other attempts —
|
||||
// before `quit_window` has touched anything.
|
||||
if let QuitAction::Restore {
|
||||
dedicated: true, ..
|
||||
} = action
|
||||
&& self
|
||||
.windows
|
||||
.get(&target)
|
||||
.is_some_and(crate::window::Window::is_side)
|
||||
&& let Some(reason) = self.panel_commit_dedication_refusal(fid)
|
||||
{
|
||||
return Err(format!("window.quit: {reason}"));
|
||||
}
|
||||
match action {
|
||||
QuitAction::Delete => {
|
||||
// Capture the remembered origin BEFORE the window dies:
|
||||
|
|
@ -4096,20 +3801,6 @@ impl EditorCore {
|
|||
.ok_or_else(|| format!("frontend {fid:?} has no window layout"))?
|
||||
.active;
|
||||
let placement = self.resolve_placement(fid, request)?;
|
||||
// Q#DC-2 (revision 8): dedicating the side slot inside a
|
||||
// `"panel"` commit is refused AT THE ATTEMPT, so the preflight's
|
||||
// measurement cannot go stale. `resolve_placement` is pure, so
|
||||
// this still refuses before anything is mutated.
|
||||
//
|
||||
// Note the guard is on the DEDICATION, not on the display: the
|
||||
// body's ordinary `display(buf, {side = "bottom"})` is exactly
|
||||
// what a panel continuation is for and always proceeds.
|
||||
if request.dedicated == Some(true)
|
||||
&& matches!(placement.kind, PlacementKind::Side { .. })
|
||||
&& let Some(reason) = self.panel_commit_dedication_refusal(fid)
|
||||
{
|
||||
return Err(format!("display: {reason}"));
|
||||
}
|
||||
self.apply_placement(fid, request, &placement)?;
|
||||
let select = request
|
||||
.select
|
||||
|
|
@ -4235,176 +3926,6 @@ impl EditorCore {
|
|||
.ok_or_else(|| "display_file: no eligible document window is available".into())
|
||||
}
|
||||
|
||||
/// Whether a `{side = ...}` request in `fid` would fall back into an
|
||||
/// ordinary document window **given the state right now** (Q#DC-2).
|
||||
///
|
||||
/// Adjacent to [`Self::resolve_placement`] because that is the rule
|
||||
/// it predicts, and a prediction that drifts from the rule is worse
|
||||
/// than none. The two fallback arms, in that function's own order:
|
||||
///
|
||||
/// 1. **step 2's capability guard** — `side` is honoured only on a
|
||||
/// `panel_capable` frontend; without the capability the request
|
||||
/// falls through to step 3's ordinary policy (Q#BP13).
|
||||
/// 2. **step 2's dedicated arm** — the one side slot exists but is
|
||||
/// dedicated, and a second one is never created, so a different
|
||||
/// buffer falls through instead (Q#BP3 2.iii).
|
||||
///
|
||||
/// **A MEASUREMENT, AND NOT SELF-SUPPORTING.** This is consulted by
|
||||
/// [`Self::commit_destination_refusal`] to refuse the statically
|
||||
/// knowable case *before* a body allocates anything — a frontend that
|
||||
/// cannot render a panel at all will not acquire the capability
|
||||
/// mid-body. On its own it would **not** make the panel profile safe:
|
||||
/// a `commit_to` body is arbitrary synchronous Lua and could dedicate
|
||||
/// the side slot itself between this answer and the placement it
|
||||
/// describes, and refusing `await` prevents another coroutine
|
||||
/// interleaving, not the body rewriting the state it was measured
|
||||
/// against. What holds the measurement true is
|
||||
/// `Self::panel_commit_dedication_refusal`, which refuses exactly
|
||||
/// those mutations for the body's whole extent.
|
||||
///
|
||||
/// Arm 2 is answered **conservatively**: `resolve_placement` falls
|
||||
/// back only when the arriving buffer differs from the dedicated one,
|
||||
/// and at preflight the body has not chosen a buffer yet.
|
||||
///
|
||||
/// A frontend with no view answers `false`: where placement would
|
||||
/// land is moot when there is nothing to place into, and
|
||||
/// `commit_destination_refusal` has already refused that case by its
|
||||
/// first check.
|
||||
#[must_use]
|
||||
pub fn panel_placement_can_fall_back(&self, fid: FrontendId) -> bool {
|
||||
let Some(view) = self.views.get(&fid) else {
|
||||
return false;
|
||||
};
|
||||
if !view.panel_capable {
|
||||
return true;
|
||||
}
|
||||
self.side_window_for(fid)
|
||||
.and_then(|side| self.windows.get(&side))
|
||||
.is_some_and(|side| side.params.dedicated)
|
||||
}
|
||||
|
||||
/// **The guarantee** behind the `"panel"` commit profile (Q#DC-2,
|
||||
/// revisions 8 and 9): anywhere inside such a commit — nested
|
||||
/// `commit_to` scopes included — the operations that would make this
|
||||
/// frontend's side request fall back are **refused at the attempt**.
|
||||
///
|
||||
/// # The defect this closes
|
||||
///
|
||||
/// The panel profile skips preflight checks 2–4 on the strength of "a
|
||||
/// panel result never touches a document window". Panel placement
|
||||
/// **falls back** into an ordinary document window when the frontend
|
||||
/// is not `panel_capable` or its one side slot is dedicated elsewhere
|
||||
/// ([`Self::apply_placement`] says so in its own comment), and then
|
||||
/// installs the result there. So a `"panel"` commit that reached a
|
||||
/// fallback would replace a document view with no stale-intent guard:
|
||||
/// capture A, the user opens B, the continuation lands, B is gone.
|
||||
///
|
||||
/// # Why this shape, and not the two that were tried first
|
||||
///
|
||||
/// * **Predicting the fallback at preflight is unsound.** The body is
|
||||
/// arbitrary *synchronous* Lua and can create the condition itself.
|
||||
/// Refusing `await` inside the commit scope stops a second
|
||||
/// coroutine interleaving; it places no restriction on the body's
|
||||
/// own statements.
|
||||
/// * **Refusing at the placement boundary is too late.** `commit_to`
|
||||
/// preflights *before* invoking the callback precisely because a
|
||||
/// body creates buffers, registers handles and paints long before
|
||||
/// it asks to display anything — "validating at display time is
|
||||
/// four mutations too late" (`docs/agent-handoff.md`). A refusal
|
||||
/// arriving after all of that is not a refusal; it is a partial
|
||||
/// commit with an error return.
|
||||
///
|
||||
/// So the preflight stays where it is and **the mutation that would
|
||||
/// invalidate it is rejected** — the same shape as `Handle:await`
|
||||
/// being refused inside a commit scope, for the identical reason.
|
||||
/// With these refused, the preflight measurement cannot go stale, the
|
||||
/// fallback never comes into existence, and nothing needs refusing
|
||||
/// late.
|
||||
///
|
||||
/// # Every enclosing contract, not just the innermost (revision 9)
|
||||
///
|
||||
/// This scans the whole contract stack. Revision 8 read a single
|
||||
/// slot, and a nested `commit_to` replaced it — so an outer
|
||||
/// `"panel"` commit whose body opened a nested `"document"` commit
|
||||
/// had its restriction **masked** for that body's extent, and the
|
||||
/// nested callback could dedicate the side slot the outer relaxation
|
||||
/// rests on. The outer commit then resumed and fell back into the
|
||||
/// document window, overwriting a newer buffer: the original defect,
|
||||
/// reachable through one extra call. Detecting it when the outer
|
||||
/// commit resumed would have been a late refusal, which revision 7
|
||||
/// was already rejected for. The restriction has to hold for the
|
||||
/// whole body, so **the strictest active restriction wins** and
|
||||
/// nesting is otherwise untouched.
|
||||
///
|
||||
/// Matching is per **frontend**, not per stack: a nested commit for a
|
||||
/// *different* frontend may dedicate *its* side slot, because that
|
||||
/// cannot change where this frontend's side request lands.
|
||||
///
|
||||
/// # The enumeration this rests on
|
||||
///
|
||||
/// [`Self::resolve_placement`] can only reach
|
||||
/// [`PlacementKind::Ordinary`] from a side request in two ways, so
|
||||
/// only two pieces of state matter:
|
||||
///
|
||||
/// 1. `FrontendView::panel_capable` is false. It is written **only**
|
||||
/// where a `FrontendView` is constructed, and no `FrontendView` is
|
||||
/// constructed, registered or unregistered anywhere in
|
||||
/// `src/lua_bindings/` — that is the daemon's attach path. **A
|
||||
/// body cannot reach it at all.**
|
||||
/// 2. The frontend's one side slot exists **and is dedicated** to a
|
||||
/// different buffer. `Window::params.dedicated` is the only
|
||||
/// remaining lever, and every write to it is guarded or harmless:
|
||||
/// the two in `apply_placement`'s `Ordinary` arm target a document
|
||||
/// window (never a side one — every `Ordinary` target is filtered
|
||||
/// `!is_side`) and one of them only ever clears the flag; the
|
||||
/// three in its `Side` arm and the one in `pmacs.window.set_params`
|
||||
/// are the attempts refused here; and `quit_window` restoring a
|
||||
/// saved `dedicated: true` presentation is refused too.
|
||||
///
|
||||
/// **Losing the side window is NOT a route** and was checked rather
|
||||
/// than assumed: with no side leaf, `side_window_for` returns `None`
|
||||
/// and `resolve_placement` **creates** a fresh panel instead of
|
||||
/// falling back. Closing or hiding the panel mid-commit is therefore
|
||||
/// safe, and `panel_hidden` is not consulted by placement at all.
|
||||
/// `params.side` is likewise unreachable — `set_params` refuses it,
|
||||
/// and only `apply_placement`'s created branch ever writes it, so a
|
||||
/// body cannot turn an already-dedicated document window into the
|
||||
/// side slot.
|
||||
///
|
||||
/// # What is deliberately NOT refused
|
||||
///
|
||||
/// * **The document profile is untouched.** Its preflight already
|
||||
/// checked the same destination, and constraining its body would
|
||||
/// newly refuse dired's own documented panel path.
|
||||
/// * **Dedicating a *document* window is fine.** It cannot change
|
||||
/// which of panel-or-document a side request resolves to.
|
||||
/// * **Falling back is still allowed.** A frontend that cannot render
|
||||
/// a panel degrades gracefully exactly as it does today; this
|
||||
/// refuses the *mutation that manufactures* a fallback, never the
|
||||
/// fallback itself.
|
||||
/// * **Nesting is untouched.** Only the mutation is refused, not the
|
||||
/// nested `commit_to` that reaches it, so a nested commit that does
|
||||
/// not dedicate this frontend's side slot runs exactly as before.
|
||||
/// Prohibiting nesting outright would have closed the hole by
|
||||
/// forbidding a shape no rule objects to (revision 9).
|
||||
pub(crate) fn panel_commit_dedication_refusal(&self, fid: FrontendId) -> Option<String> {
|
||||
// ANY enclosing contract, not the innermost one: a nested commit
|
||||
// composes with the restrictions already in force rather than
|
||||
// masking them (revision 9).
|
||||
if !self.commit_contracts.iter().any(|contract| {
|
||||
contract.profile == CommitProfile::Panel && contract.destination.frontend == fid
|
||||
}) {
|
||||
return None;
|
||||
}
|
||||
Some(
|
||||
"cannot dedicate the side window inside a \"panel\" commit_to --- the commit's \
|
||||
preflight was relaxed because this frontend places side requests in the panel, \
|
||||
and dedicating the one slot would silently redirect the result into a document \
|
||||
window instead (dedicate outside the commit, or use the \"document\" profile)"
|
||||
.to_string(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Q#BP3's precedence: exact target, then side affinity, then
|
||||
/// ordinary reuse. Placement affinity precedes generic reuse —
|
||||
/// otherwise a persistent `*compilation*` buffer already visible in a
|
||||
|
|
|
|||
|
|
@ -4239,35 +4239,25 @@ fn install_path_module(lua: &Lua) -> mlua::Result<Table> {
|
|||
Ok(path)
|
||||
}
|
||||
|
||||
/// Lua handle for a captured view destination (Q#JR14d).
|
||||
/// Lua handle for a captured directory destination (Q#JR14d).
|
||||
///
|
||||
/// Deliberately **nonconstructible from Lua** and read-only, which the
|
||||
/// generalization to `pmacs.window.capture_destination()` preserves:
|
||||
/// capture mints one from editor state, and there is still no
|
||||
/// constructor and no setter. The same value is passed to every
|
||||
/// `path.open-directory` listener in turn: as a table, an earlier
|
||||
/// listener could mutate it and then decline, redirecting later
|
||||
/// listeners or the fallback to a window the user never asked for — and
|
||||
/// any Lua could fabricate a plausible frontend/window/buffer triple and
|
||||
/// hand it to `commit_to`. Userdata with no constructor and no setters
|
||||
/// makes both unrepresentable rather than merely discouraged.
|
||||
/// Deliberately **nonconstructible from Lua** and read-only. The same
|
||||
/// value is passed to every `path.open-directory` listener in turn: as a
|
||||
/// table, an earlier listener could mutate it and then decline,
|
||||
/// redirecting later listeners or the fallback to a window the user
|
||||
/// never asked for — and any Lua could fabricate a plausible
|
||||
/// frontend/window/buffer triple and hand it to `commit_to`. Userdata
|
||||
/// with no constructor and no setters makes both unrepresentable rather
|
||||
/// than merely discouraged.
|
||||
///
|
||||
/// The single accessor exists because dired needs the exact window for
|
||||
/// its `display{window = …}` target; nothing needs the frontend or the
|
||||
/// captured buffer, which stay private to the preflight.
|
||||
///
|
||||
/// `window()` returns **nil** when the capturing frontend had no
|
||||
/// document window (Q#DC-4) — such a destination is still commitable
|
||||
/// under the panel profile wherever that profile's relaxation actually
|
||||
/// applies, so the accessor reports the absence rather than inventing an
|
||||
/// id.
|
||||
pub(crate) struct ViewDestinationLua(pub(crate) crate::editor_core::ViewDestination);
|
||||
pub(crate) struct DirectoryDestinationLua(pub(crate) crate::editor_core::DirectoryDestination);
|
||||
|
||||
impl mlua::UserData for ViewDestinationLua {
|
||||
impl mlua::UserData for DirectoryDestinationLua {
|
||||
fn add_methods<M: mlua::UserDataMethods<Self>>(methods: &mut M) {
|
||||
methods.add_method("window", |_, this, ()| {
|
||||
Ok(this.0.window.map(crate::window::WindowId::raw))
|
||||
});
|
||||
methods.add_method("window", |_, this, ()| Ok(this.0.window.raw()));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -34,9 +34,7 @@
|
|||
use mlua::{Lua, Table, Value};
|
||||
|
||||
use super::{BufferIdLua, SharedCore, config_u32, run_hook_if_defined};
|
||||
use crate::editor_core::{
|
||||
CommitContract, CommitProfile, DisplayOutcome, DisplayRequest, HookKind, QuitOutcome,
|
||||
};
|
||||
use crate::editor_core::{DisplayOutcome, DisplayRequest, HookKind, QuitOutcome};
|
||||
use crate::protocol::FrontendId;
|
||||
use crate::window::{DEFAULT_PANEL_ROWS, MIN_WINDOW_OUTER_ROWS, Side, WindowId};
|
||||
|
||||
|
|
@ -65,51 +63,6 @@ pub(crate) fn acting_frontend(lua: &Lua, core: &SharedCore) -> FrontendId {
|
|||
.unwrap_or_else(|| core.borrow().active_frontend_key())
|
||||
}
|
||||
|
||||
/// One message for every bad profile — an unrecognized string and a
|
||||
/// non-string alike (Q#DC-5).
|
||||
///
|
||||
/// Stated once so the parser and the message cannot drift, and phrased
|
||||
/// to name the accepted values *and* the default, because a caller who
|
||||
/// gets this wrong is guessing at the vocabulary.
|
||||
const BAD_COMMIT_PROFILE: &str = "pmacs.window.commit_to: profile must be the string \"document\" \
|
||||
or \"panel\" (omitting it, or passing nil, means \"document\")";
|
||||
|
||||
/// Resolve the optional third argument of `commit_to`.
|
||||
///
|
||||
/// Takes a [`Value`] rather than an `Option<String>` **so this refusal
|
||||
/// is reachable**: with the narrower type mlua rejects a number or a
|
||||
/// table during argument conversion, before the closure body runs, and
|
||||
/// the caller gets a generic conversion error that names neither the
|
||||
/// accepted values nor the default. That is the same trap the `dest`
|
||||
/// argument documents at its own borrow site.
|
||||
///
|
||||
/// `Nil` and absence are the **same** answer, not two: a Lua caller
|
||||
/// threading an optional variable produces `commit_to(dest, body, nil)`,
|
||||
/// and a third behaviour there would stay invisible until someone hit
|
||||
/// it.
|
||||
///
|
||||
/// The comparison is on **bytes**, for the same reachability reason one
|
||||
/// layer down. A Lua string is a byte string, not UTF-8, so
|
||||
/// `commit_to(dest, body, string.char(255))` fails a `to_str()`
|
||||
/// conversion and surfaces mlua's generic UTF-8 error *before* the
|
||||
/// message below is ever constructed. An invalid-UTF-8 profile is a bad
|
||||
/// profile like any other and gets the documented refusal.
|
||||
fn commit_profile(value: &Value) -> mlua::Result<CommitProfile> {
|
||||
match value {
|
||||
Value::Nil => Ok(CommitProfile::Document),
|
||||
Value::String(name) => match name.as_bytes().as_ref() {
|
||||
b"document" => Ok(CommitProfile::Document),
|
||||
b"panel" => Ok(CommitProfile::Panel),
|
||||
// An unrecognized profile ERRORS rather than falling back to
|
||||
// the document one: a fallback would silently hand a caller
|
||||
// stricter or looser checks than it asked for, which is the
|
||||
// failure the parameterization exists to prevent.
|
||||
_ => Err(mlua::Error::runtime(BAD_COMMIT_PROFILE)),
|
||||
},
|
||||
_ => Err(mlua::Error::runtime(BAD_COMMIT_PROFILE)),
|
||||
}
|
||||
}
|
||||
|
||||
/// Run the panel-reconciliation transaction from a Lua-owning context
|
||||
/// (Q#BP2b).
|
||||
///
|
||||
|
|
@ -499,7 +452,7 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result
|
|||
"commit_to",
|
||||
lua.create_function(
|
||||
move |lua,
|
||||
(dest, body, profile): (mlua::Value, mlua::Function, mlua::Value)|
|
||||
(dest, body): (mlua::Value, mlua::Function)|
|
||||
-> mlua::Result<mlua::MultiValue> {
|
||||
// Journey Stage 1a (Q#JR14). Preflight FIRST, then
|
||||
// scope, then run. The ordering is the whole point:
|
||||
|
|
@ -519,7 +472,7 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result
|
|||
// rule nor how to get a real destination.
|
||||
let dest = match &dest {
|
||||
mlua::Value::UserData(userdata) => {
|
||||
userdata.borrow::<super::ViewDestinationLua>().ok()
|
||||
userdata.borrow::<super::DirectoryDestinationLua>().ok()
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
|
|
@ -531,27 +484,45 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result
|
|||
)
|
||||
})?
|
||||
.0;
|
||||
// Q#DC-5. Resolved AFTER the destination so a caller
|
||||
// who got both wrong hears about the destination
|
||||
// first --- it is the argument that cannot be fixed
|
||||
// by reading this signature.
|
||||
let profile = commit_profile(&profile)?;
|
||||
|
||||
// The preflight itself lives on the core
|
||||
// (`commit_destination_refusal`) rather than being
|
||||
// hand-written here, so the panel profile's
|
||||
// relaxation is decided in one place: two copies of
|
||||
// the same three checks is how one of them ends up
|
||||
// weaker than the other.
|
||||
//
|
||||
// This call is only HALF the panel guarantee. It
|
||||
// measures whether this frontend places side requests
|
||||
// in the panel; what keeps that measurement true
|
||||
// while the body runs --- nested `commit_to` scopes
|
||||
// included --- is
|
||||
// `EditorCore::panel_commit_dedication_refusal`,
|
||||
// which refuses the mutations that would falsify it.
|
||||
let refusal = cc.borrow().commit_destination_refusal(&dest, profile);
|
||||
// 1. The requesting frontend still has a layout.
|
||||
let refusal = {
|
||||
let core = cc.borrow();
|
||||
if !core.views.contains_key(&dest.frontend) {
|
||||
Some("requesting frontend is gone".to_string())
|
||||
} else if !core
|
||||
.views
|
||||
.get(&dest.frontend)
|
||||
.is_some_and(|view| view.layout.iter_ids().contains(&dest.window))
|
||||
{
|
||||
// 2. The destination window is still live in it.
|
||||
Some(format!("window {} is gone", dest.window.raw()))
|
||||
} else if core
|
||||
.windows
|
||||
.get(&dest.window)
|
||||
.is_some_and(|w| w.buffer_id != dest.buffer)
|
||||
{
|
||||
// 3. Stale intent (Q#JR14c): the user
|
||||
// replaced the buffer while the work was
|
||||
// in flight. Their action is newer
|
||||
// information than the request, so the
|
||||
// request loses.
|
||||
Some(format!(
|
||||
"window {} now shows another buffer",
|
||||
dest.window.raw()
|
||||
))
|
||||
} else if !core.window_accepts_buffer(dest.window, None) {
|
||||
// 4. Replaceability (Q#JR14f). `None`
|
||||
// because the replacement does not exist
|
||||
// yet — passing the captured buffer would
|
||||
// approve a window dedicated to *it*, and
|
||||
// the handler's different buffer would be
|
||||
// refused later, after mutating.
|
||||
Some(format!("window {} is dedicated", dest.window.raw()))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
};
|
||||
if let Some(reason) = refusal {
|
||||
let mut out = mlua::MultiValue::new();
|
||||
out.push_back(mlua::Value::String(lua.create_string(reason.as_bytes())?));
|
||||
|
|
@ -575,32 +546,13 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result
|
|||
)
|
||||
})?
|
||||
.clone();
|
||||
// The override, the core's ambient `active_frontend`,
|
||||
// and the CONTRACT below are all restored when this
|
||||
// guard drops -- on the normal return AND on a
|
||||
// raising callback, which is why the result is
|
||||
// captured rather than `?`-propagated through the
|
||||
// drop. The contract rides with the scope because
|
||||
// every mutation this body reaches has to know which
|
||||
// destination and which profile it is running under.
|
||||
//
|
||||
// A NESTED `commit_to` PUSHES its contract onto the
|
||||
// ones already in force rather than replacing them
|
||||
// (Q#DC-2, revision 9). Replacing was a hole: an
|
||||
// outer `"panel"` commit's mutation refusal went out
|
||||
// of force for the extent of a nested body, which is
|
||||
// long enough to dedicate the side slot its relaxed
|
||||
// preflight depends on. Nesting itself is allowed --
|
||||
// only the mutation is refused.
|
||||
// Both the override and the core's ambient
|
||||
// `active_frontend` are restored when this guard
|
||||
// drops -- on the normal return AND on a raising
|
||||
// callback, which is why the result is captured
|
||||
// rather than `?`-propagated through the drop.
|
||||
let result = {
|
||||
let _guard = scope.enter(
|
||||
&cc,
|
||||
&commit,
|
||||
CommitContract {
|
||||
destination: dest,
|
||||
profile,
|
||||
},
|
||||
);
|
||||
let _guard = scope.enter(&cc, &commit, dest.frontend);
|
||||
body.call::<mlua::MultiValue>(())
|
||||
};
|
||||
let mut out = result?;
|
||||
|
|
@ -611,39 +563,6 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result
|
|||
)?;
|
||||
}
|
||||
|
||||
{
|
||||
// Q#DC-1 — the capture half, reachable from Lua at last.
|
||||
//
|
||||
// Journey Stage 1a built `commit_to` for the continuation
|
||||
// boundary, but the only thing that could mint a destination was
|
||||
// the `path.open-directory` dispatch, so every other async
|
||||
// continuation had to resolve its target from ambient state a
|
||||
// tick after the request --- which is a misrouting waiting for a
|
||||
// second frontend to become active.
|
||||
//
|
||||
// NO ARGUMENTS, and that is load-bearing rather than
|
||||
// minimalism. A Lua-supplied frontend id would reintroduce
|
||||
// exactly the fabrication hole the nonconstructible userdata
|
||||
// closes (Q#JR14d): the point of userdata is that Lua names a
|
||||
// destination it was *given*, never one it composed.
|
||||
//
|
||||
// PROFILE-BLIND, likewise (Q#DC-4). Capture records what is
|
||||
// there; what a commit depends on is declared at `commit_to`,
|
||||
// because a caller knows what it is about to do only then.
|
||||
// Making capture profile-aware would force it to know at capture
|
||||
// time what it will do at commit time, which is the opposite of
|
||||
// why capture exists --- freeze the truth early, decide later.
|
||||
let cc = core.clone();
|
||||
win.set(
|
||||
"capture_destination",
|
||||
lua.create_function(move |lua, ()| {
|
||||
let fid = acting_frontend(lua, &cc);
|
||||
let dest = cc.borrow().capture_view_destination(fid);
|
||||
lua.create_userdata(super::ViewDestinationLua(dest))
|
||||
})?,
|
||||
)?;
|
||||
}
|
||||
|
||||
{
|
||||
// Q#S3-1 — the shared adopter-display rule, reachable from Lua.
|
||||
//
|
||||
|
|
@ -915,24 +834,6 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result
|
|||
None => None,
|
||||
};
|
||||
let dedicated = opts.get::<Option<bool>>("dedicated")?;
|
||||
// Q#DC-2 (revision 8). The direct route to the one
|
||||
// mutation that could make a `"panel"` commit's
|
||||
// relaxed preflight wrong. Refused BEFORE the borrow
|
||||
// below, so the attempt changes nothing --- including
|
||||
// `fixed_rows`, which is in the same option table.
|
||||
if dedicated == Some(true) {
|
||||
let core = cc.borrow();
|
||||
if core
|
||||
.windows
|
||||
.get(&id)
|
||||
.is_some_and(crate::window::Window::is_side)
|
||||
&& let Some(reason) = core.panel_commit_dedication_refusal(fid)
|
||||
{
|
||||
return Err(mlua::Error::runtime(format!(
|
||||
"pmacs.window.set_params: {reason}"
|
||||
)));
|
||||
}
|
||||
}
|
||||
{
|
||||
let mut core = cc.borrow_mut();
|
||||
let window = core.windows.get_mut(&id).ok_or_else(|| {
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
|
|
@ -5351,416 +5351,6 @@ fn m4_24_workspace_did_change_watched_files() {
|
|||
);
|
||||
}
|
||||
|
||||
/// Issue #233 D1 — a PLAIN-STRING `GlobPattern` matches the file's
|
||||
/// ABSOLUTE path (LSP 3.17), not the walk's relative path. The
|
||||
/// `filewatchabs` fake registers `<base>/**/*.txt` as a bare string —
|
||||
/// the form rust-analyzer and gopls actually send. Its relative
|
||||
/// reading matches nothing (an anchored `^<base>/…` can never match
|
||||
/// `foo.txt`), so before the fix no event could ever be reported.
|
||||
/// The watcher's base is guessed from the attached file's directory —
|
||||
/// the tempdir here, and the production path for bare-string globs.
|
||||
#[test]
|
||||
fn m4_24_plain_string_glob_matches_absolute_path() {
|
||||
use pmacs::editor::EditorState;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let base = dir.path().to_path_buf();
|
||||
let base_disp = base.display().to_string();
|
||||
let a_path = base.join("a.rs");
|
||||
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
|
||||
let a_disp = a_path.display().to_string();
|
||||
let received = base.join(".received");
|
||||
let foo_uri = format!("file://{}", base.join("foo.txt").display());
|
||||
|
||||
let mut state = EditorState::new_with_roots(&crate::iso::roots());
|
||||
let fake = fake_lsp_path();
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!(
|
||||
"pmacs.lsp.config.rust = {{ command = '{fake}',
|
||||
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchabs',
|
||||
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
|
||||
))
|
||||
.exec()
|
||||
.expect("override rust config");
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
|
||||
.exec()
|
||||
.expect("open a.rs");
|
||||
assert!(
|
||||
pump_lua_flag(
|
||||
&mut state,
|
||||
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
|
||||
if r.state and r.state.kind=='initialized' then return true end \
|
||||
end return false end)()",
|
||||
5,
|
||||
),
|
||||
"fake never initialized"
|
||||
);
|
||||
|
||||
// Same warm-up as m4_24: let registerCapability land and the
|
||||
// watcher take its empty baseline before files appear.
|
||||
let warm = Instant::now() + Duration::from_millis(900);
|
||||
while Instant::now() < warm {
|
||||
state.tick_processes();
|
||||
state.tick_lsp();
|
||||
state.tick_async();
|
||||
std::thread::sleep(Duration::from_millis(15));
|
||||
}
|
||||
|
||||
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
|
||||
std::fs::write(base.join("bar.md"), b"md\n").expect("write bar.md");
|
||||
assert!(
|
||||
pump_until_file_contains(&mut state, &received, &format!("1 {foo_uri}"), 6),
|
||||
"CREATED for foo.txt never reported under a plain-string glob; \
|
||||
.received = {:?}",
|
||||
std::fs::read_to_string(&received).unwrap_or_default()
|
||||
);
|
||||
assert!(
|
||||
!std::fs::read_to_string(&received)
|
||||
.unwrap_or_default()
|
||||
.contains("bar.md"),
|
||||
"non-matching .md must be filtered out"
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #233 F2 guard — a `RelativePattern` stays relative to its
|
||||
/// base. The `filewatchflat` fake registers `{ baseUri, pattern =
|
||||
/// "*.txt" }`, whose pattern has no leading `**/`: it matches
|
||||
/// base-level files RELATIVELY and cannot match any absolute path
|
||||
/// (`[^/]*` spans no `/`). Green before and after D1's fix; red
|
||||
/// against the obvious wrong fix that matches every form absolutely.
|
||||
/// `sub/nested.txt` pins the other half of the same contract: a
|
||||
/// base-level pattern must not match into subdirectories.
|
||||
#[test]
|
||||
fn m4_24_relative_pattern_without_globstar_stays_relative() {
|
||||
use pmacs::editor::EditorState;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let base = dir.path().to_path_buf();
|
||||
let base_disp = base.display().to_string();
|
||||
let a_path = base.join("a.rs");
|
||||
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
|
||||
let a_disp = a_path.display().to_string();
|
||||
let received = base.join(".received");
|
||||
let foo_uri = format!("file://{}", base.join("foo.txt").display());
|
||||
std::fs::create_dir(base.join("sub")).expect("mkdir sub");
|
||||
|
||||
let mut state = EditorState::new_with_roots(&crate::iso::roots());
|
||||
let fake = fake_lsp_path();
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!(
|
||||
"pmacs.lsp.config.rust = {{ command = '{fake}',
|
||||
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchflat',
|
||||
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
|
||||
))
|
||||
.exec()
|
||||
.expect("override rust config");
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
|
||||
.exec()
|
||||
.expect("open a.rs");
|
||||
assert!(
|
||||
pump_lua_flag(
|
||||
&mut state,
|
||||
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
|
||||
if r.state and r.state.kind=='initialized' then return true end \
|
||||
end return false end)()",
|
||||
5,
|
||||
),
|
||||
"fake never initialized"
|
||||
);
|
||||
|
||||
let warm = Instant::now() + Duration::from_millis(900);
|
||||
while Instant::now() < warm {
|
||||
state.tick_processes();
|
||||
state.tick_lsp();
|
||||
state.tick_async();
|
||||
std::thread::sleep(Duration::from_millis(15));
|
||||
}
|
||||
|
||||
// nested.txt is written BEFORE foo.txt, so a watcher that wrongly
|
||||
// matched it would report it no later than foo.txt's event — the
|
||||
// negative assertion after the positive one is race-free.
|
||||
std::fs::write(base.join("sub").join("nested.txt"), b"deep\n").expect("write nested.txt");
|
||||
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
|
||||
assert!(
|
||||
pump_until_file_contains(&mut state, &received, &format!("1 {foo_uri}"), 6),
|
||||
"CREATED for base-level foo.txt never reported under a \
|
||||
RelativePattern without `**/`; .received = {:?}",
|
||||
std::fs::read_to_string(&received).unwrap_or_default()
|
||||
);
|
||||
assert!(
|
||||
!std::fs::read_to_string(&received)
|
||||
.unwrap_or_default()
|
||||
.contains("nested.txt"),
|
||||
"a base-level `*.txt` RelativePattern must not match into \
|
||||
subdirectories"
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #233 review P2 — a scan that completes AFTER cancellation
|
||||
/// must not emit.
|
||||
///
|
||||
/// `scan_tree` awaits `read_dir` once per directory, so the watcher
|
||||
/// coroutine spends most of a tick suspended with `_sleep` already
|
||||
/// cleared. A cancel arriving there — re-registration or unregistration
|
||||
/// — sets `cancelled` and has no sleep to interrupt, so before the fix
|
||||
/// the resumed scan ran on and emitted one last batch under the
|
||||
/// superseded pattern.
|
||||
///
|
||||
/// No arrangement of real timing produces that interleaving on demand,
|
||||
/// so it is driven through `pmacs.lsp._after_scan_for_tests`, the same
|
||||
/// device `git.lua` uses for out-of-order completions. The hook is
|
||||
/// handed the scan result and cancels **only on the scan that observed
|
||||
/// `foo.txt`** — cancelling on any other scan would pass with the fix
|
||||
/// deleted, because the loop would break at the post-sleep check and
|
||||
/// emit nothing regardless.
|
||||
#[test]
|
||||
fn m4_24_a_scan_finishing_after_cancellation_emits_nothing() {
|
||||
use pmacs::editor::EditorState;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let base = dir.path().to_path_buf();
|
||||
let base_disp = base.display().to_string();
|
||||
let a_path = base.join("a.rs");
|
||||
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
|
||||
let a_disp = a_path.display().to_string();
|
||||
let received = base.join(".received");
|
||||
|
||||
let mut state = EditorState::new_with_roots(&crate::iso::roots());
|
||||
let fake = fake_lsp_path();
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!(
|
||||
"pmacs.lsp.config.rust = {{ command = '{fake}',
|
||||
env = {{ PMACS_FAKE_LSP_MODE = 'filewatch',
|
||||
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
|
||||
))
|
||||
.exec()
|
||||
.expect("override rust config");
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
|
||||
.exec()
|
||||
.expect("open a.rs");
|
||||
assert!(
|
||||
pump_lua_flag(
|
||||
&mut state,
|
||||
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
|
||||
if r.state and r.state.kind=='initialized' then return true end \
|
||||
end return false end)()",
|
||||
5,
|
||||
),
|
||||
"fake never initialized"
|
||||
);
|
||||
|
||||
// Armed BEFORE the file exists, so the cancel cannot land early:
|
||||
// the hook fires on every scan and only cancels once the scan it is
|
||||
// inspecting actually contains foo.txt.
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(
|
||||
"pmacs.lsp._after_scan_for_tests = function(record, cur)
|
||||
if cur and cur['foo.txt'] then record.cancelled = true end
|
||||
end",
|
||||
)
|
||||
.exec()
|
||||
.expect("install scan hook");
|
||||
|
||||
let warm = Instant::now() + Duration::from_millis(900);
|
||||
while Instant::now() < warm {
|
||||
state.tick_processes();
|
||||
state.tick_lsp();
|
||||
state.tick_async();
|
||||
std::thread::sleep(Duration::from_millis(15));
|
||||
}
|
||||
|
||||
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(4);
|
||||
while Instant::now() < deadline {
|
||||
state.tick_processes();
|
||||
state.tick_lsp();
|
||||
state.tick_async();
|
||||
std::thread::sleep(Duration::from_millis(15));
|
||||
}
|
||||
|
||||
let got = std::fs::read_to_string(&received).unwrap_or_default();
|
||||
assert!(
|
||||
!got.contains("foo.txt"),
|
||||
"a watcher cancelled during its scan emitted a stale batch \
|
||||
anyway; .received = {got:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #233 review P1 — a BARE-STRING glob with no leading `/` is a
|
||||
/// relative pattern and must stay one.
|
||||
///
|
||||
/// The first fix for #233 classified every string-arm pattern as
|
||||
/// absolute, so `*.txt` was matched against `<base>/foo.txt` and could
|
||||
/// never fire — silently breaking a case that had worked since May
|
||||
/// while fixing the absolute one. `m4_24_relative_pattern_without_globstar_stays_relative`
|
||||
/// does not cover it: that mode sends the `RelativePattern` OBJECT form,
|
||||
/// so it constrains the object arm only. This sends the same pattern
|
||||
/// through the STRING arm, which is the arm the regression lived in.
|
||||
#[test]
|
||||
fn m4_24_bare_string_glob_stays_relative() {
|
||||
use pmacs::editor::EditorState;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let base = dir.path().to_path_buf();
|
||||
let base_disp = base.display().to_string();
|
||||
let a_path = base.join("a.rs");
|
||||
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
|
||||
let a_disp = a_path.display().to_string();
|
||||
let received = base.join(".received");
|
||||
let foo_uri = format!("file://{}", base.join("foo.txt").display());
|
||||
|
||||
let mut state = EditorState::new_with_roots(&crate::iso::roots());
|
||||
let fake = fake_lsp_path();
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!(
|
||||
"pmacs.lsp.config.rust = {{ command = '{fake}',
|
||||
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchbare',
|
||||
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
|
||||
))
|
||||
.exec()
|
||||
.expect("override rust config");
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
|
||||
.exec()
|
||||
.expect("open a.rs");
|
||||
assert!(
|
||||
pump_lua_flag(
|
||||
&mut state,
|
||||
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
|
||||
if r.state and r.state.kind=='initialized' then return true end \
|
||||
end return false end)()",
|
||||
5,
|
||||
),
|
||||
"fake never initialized"
|
||||
);
|
||||
|
||||
let warm = Instant::now() + Duration::from_millis(900);
|
||||
while Instant::now() < warm {
|
||||
state.tick_processes();
|
||||
state.tick_lsp();
|
||||
state.tick_async();
|
||||
std::thread::sleep(Duration::from_millis(15));
|
||||
}
|
||||
|
||||
std::fs::write(base.join("foo.txt"), b"one\n").expect("write foo.txt");
|
||||
assert!(
|
||||
pump_until_file_contains(&mut state, &received, &format!("1 {foo_uri}"), 6),
|
||||
"CREATED for foo.txt never reported under a bare-string `*.txt` \
|
||||
glob — the string arm is being classified absolute again; \
|
||||
.received = {:?}",
|
||||
std::fs::read_to_string(&received).unwrap_or_default()
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #233 D2 — re-registering a live id supersedes it. The
|
||||
/// `filewatchrereg` fake registers `watch-re` TWICE with no
|
||||
/// unregister between — `**/*.old`, then `**/*.new` — exactly the
|
||||
/// shape rust-analyzer sends. The superseded watchers must STOP,
|
||||
/// asserted on observable polling rather than on table shape (the
|
||||
/// defect is precisely that the replaced records become unreachable
|
||||
/// while still polling): `f.old` exists on disk before either `.new`
|
||||
/// event lands, so a leaked first-registration watcher, polling at
|
||||
/// the same 250 ms cadence, would have reported it by the time the
|
||||
/// second `.new` positive arrives.
|
||||
#[test]
|
||||
fn m4_24_reregistration_supersedes_previous_watchers() {
|
||||
use pmacs::editor::EditorState;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let base = dir.path().to_path_buf();
|
||||
let base_disp = base.display().to_string();
|
||||
let a_path = base.join("a.rs");
|
||||
std::fs::write(&a_path, b"fn a() {}\n").expect("write a");
|
||||
let a_disp = a_path.display().to_string();
|
||||
let received = base.join(".received");
|
||||
let f_old_uri = format!("file://{}", base.join("f.old").display());
|
||||
let f_new_uri = format!("file://{}", base.join("f.new").display());
|
||||
let g_new_uri = format!("file://{}", base.join("g.new").display());
|
||||
|
||||
let mut state = EditorState::new_with_roots(&crate::iso::roots());
|
||||
let fake = fake_lsp_path();
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!(
|
||||
"pmacs.lsp.config.rust = {{ command = '{fake}',
|
||||
env = {{ PMACS_FAKE_LSP_MODE = 'filewatchrereg',
|
||||
PMACS_FAKE_LSP_WATCH_BASE = '{base_disp}' }} }}"
|
||||
))
|
||||
.exec()
|
||||
.expect("override rust config");
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
|
||||
.exec()
|
||||
.expect("open a.rs");
|
||||
assert!(
|
||||
pump_lua_flag(
|
||||
&mut state,
|
||||
"(function() for _,r in ipairs(pmacs.lsp.list()) do \
|
||||
if r.state and r.state.kind=='initialized' then return true end \
|
||||
end return false end)()",
|
||||
5,
|
||||
),
|
||||
"fake never initialized"
|
||||
);
|
||||
|
||||
let warm = Instant::now() + Duration::from_millis(900);
|
||||
while Instant::now() < warm {
|
||||
state.tick_processes();
|
||||
state.tick_lsp();
|
||||
state.tick_async();
|
||||
std::thread::sleep(Duration::from_millis(15));
|
||||
}
|
||||
|
||||
std::fs::write(base.join("f.old"), b"old\n").expect("write f.old");
|
||||
std::fs::write(base.join("f.new"), b"new\n").expect("write f.new");
|
||||
assert!(
|
||||
pump_until_file_contains(&mut state, &received, &format!("1 {f_new_uri}"), 6),
|
||||
"CREATED for f.new never reported by the superseding watcher; \
|
||||
.received = {:?}",
|
||||
std::fs::read_to_string(&received).unwrap_or_default()
|
||||
);
|
||||
// A second positive puts at least one more full poll cycle between
|
||||
// f.old appearing on disk and the negative assertion below.
|
||||
std::fs::write(base.join("g.new"), b"new\n").expect("write g.new");
|
||||
assert!(
|
||||
pump_until_file_contains(&mut state, &received, &format!("1 {g_new_uri}"), 6),
|
||||
"CREATED for g.new never reported by the superseding watcher"
|
||||
);
|
||||
assert!(
|
||||
!std::fs::read_to_string(&received)
|
||||
.unwrap_or_default()
|
||||
.contains(&f_old_uri),
|
||||
"the superseded `**/*.old` watcher is still polling after \
|
||||
re-registration under the same id; .received = {:?}",
|
||||
std::fs::read_to_string(&received).unwrap_or_default()
|
||||
);
|
||||
}
|
||||
|
||||
/// Tier 1 single-binary language servers ship pre-configured in the
|
||||
/// default bundle. Binary-independent: we don't spawn anything, just
|
||||
/// assert the `pmacs.lsp.config` tables and the `pmacs.lsp.filetypes`
|
||||
|
|
|
|||
Loading…
Reference in New Issue