`docs/active-work.md` was the only conflicting file, in the same shape as #191's: `main` inserted the generated-buffer Stage 1 lane immediately above the bottom-panel header this branch had rewritten. The resolution keeps both. Each side's newer text wins where that side owns the fact: `main` carries the corrected #188 status (MERGED/APPROVED, replacing "OPEN, PROPOSED — do not implement"), and this branch carries the bottom-panel lane's 2B-3 state and the newer snapshot date, replacing main's "2B-2 MERGED; 2B-3 IS NEXT" and its 2B-2 boundary paragraphs. Verified: no conflict markers; every line absent from either parent is a deliberate supersession by the other, enumerated and checked one by one rather than counted; all three lane headers present exactly once.
This commit is contained in:
commit
7a271f13f5
|
|
@ -203,18 +203,27 @@ jobs:
|
|||
# render job. Set only where the install step ran.
|
||||
#
|
||||
# PMACS_REQUIRE_PYRIGHT is deliberately NOT set and basedpyright
|
||||
# is deliberately NOT installed: that test has no timeout and
|
||||
# hangs forever (root cause is the non-interruptible reader-thread
|
||||
# join in `RuntimeHandles::drop`, already a named deferral in
|
||||
# `src/process.rs`). This job has no `timeout-minutes`, so arming
|
||||
# it today would trade a vacuous green for a six-hour hang on four
|
||||
# legs. It gets armed after the hang fix and the CI timeouts land,
|
||||
# and its own variable exists so that flip is one line.
|
||||
# is deliberately NOT installed. Both original reasons are now
|
||||
# gone: the hang's root cause was the stdin-field drop ordering in
|
||||
# `RuntimeHandles::drop` and is fixed, and this job now carries
|
||||
# `timeout-minutes`, so a hang could no longer burn six hours.
|
||||
# The ONE remaining reason is the plain one --- basedpyright is not
|
||||
# installed here, so arming the variable would fail rather than
|
||||
# test anything. Installing it (a uv + bundled-node download on
|
||||
# every leg) is its own decision, not a rider on the hang fix.
|
||||
#
|
||||
# PMACS_REQUIRE_SETSID arms the teardown-deadlock unit test. Its
|
||||
# fixture orphans a grandchild with `setsid --fork`, which is
|
||||
# util-linux rather than coreutils, so the test skips when the
|
||||
# binary is absent (a minimal container must not fail `--lib`
|
||||
# without ever testing pmacs) and this variable is what makes the
|
||||
# skip fatal where the tool is guaranteed.
|
||||
- run: cargo test --all-targets --no-default-features --features ${{ matrix.lua }} -- --test-threads=1
|
||||
env:
|
||||
PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }}
|
||||
PMACS_REQUIRE_SHELLS: ${{ runner.os == 'Linux' && '1' || '' }}
|
||||
PMACS_REQUIRE_LUA: ${{ runner.os == 'Linux' && '1' || '' }}
|
||||
PMACS_REQUIRE_SETSID: ${{ runner.os == 'Linux' && '1' || '' }}
|
||||
- run: cargo test --doc --no-default-features --features ${{ matrix.lua }}
|
||||
# The workspace default member is only the root `pmacs` package, so
|
||||
# the runs above never execute pmacs-protocol's own tests — the
|
||||
|
|
|
|||
|
|
@ -220,6 +220,13 @@ translation) are routed through trampolines that exec these tools.
|
|||
shell-locator helper to find `bash` / `zsh` / `fish` for
|
||||
per-shell integration tests. The M7.2 fetcher's timeout test
|
||||
uses `sleep`.
|
||||
- **`setsid`** (util-linux, Linux only, **optional**). The process
|
||||
teardown-deadlock test uses `setsid --fork` to orphan a grandchild,
|
||||
which is the only way to reproduce that deadlock without depending on
|
||||
shell `&` semantics (they differ between `bash` and `dash`). The test
|
||||
**skips** when `setsid` is absent, so a minimal or BusyBox environment
|
||||
still runs `cargo test --lib`; set `PMACS_REQUIRE_SETSID=1` to make
|
||||
that skip a failure, as CI does on Linux.
|
||||
- **`git`** (added in M7.2). Required for any package operation:
|
||||
the package fetcher shells out to `git` to clone, fetch, and
|
||||
resolve refs, with a deterministic environment
|
||||
|
|
|
|||
|
|
@ -16,6 +16,12 @@
|
|||
-- format-on-save subscribe here.
|
||||
-- * editor.before-quit --- short-circuit. A callback may veto quit
|
||||
-- (e.g. "buffer modified --- save first?").
|
||||
-- * resource.renamed --- all-must-succeed (dired Stage 2a). Fired
|
||||
-- after a successful rename, with (old, new)
|
||||
-- canonical absolute paths.
|
||||
-- * resource.deleted --- all-must-succeed (dired Stage 2a). Fired
|
||||
-- after a successful delete, with the
|
||||
-- canonical absolute path.
|
||||
--
|
||||
-- These are *defined* here so user config can attach callbacks via
|
||||
-- pmacs.hook.add. Run sites are in Rust (after-load, after-edit) and in
|
||||
|
|
@ -76,6 +82,32 @@ define {
|
|||
kind = "short-circuit",
|
||||
}
|
||||
|
||||
define {
|
||||
name = "resource.renamed",
|
||||
description = "Fired once per SUCCESSFUL filesystem rename, with the old " ..
|
||||
"and new paths as canonical absolute strings. The core " ..
|
||||
"reconciles what it can reach -- buffer paths and names, the " ..
|
||||
"URI-keyed LSP stores, attached diagnostic overlays -- but a " ..
|
||||
"package that keys its own state by path or URI is invisible " ..
|
||||
"to that, so this hook is the mechanism that scales. It " ..
|
||||
"carries PATHS rather than a rebind list precisely because " ..
|
||||
"dired's listing buffers are pathless: a path-keyed consumer " ..
|
||||
"must be able to reconcile from (old, new) alone. Does not " ..
|
||||
"fire for a rename that failed or was cancelled.",
|
||||
kind = "all-must-succeed",
|
||||
}
|
||||
|
||||
define {
|
||||
name = "resource.deleted",
|
||||
description = "Fired once per SUCCESSFUL filesystem delete, with the " ..
|
||||
"canonical absolute path. Buffers on the path and beneath it " ..
|
||||
"have already been reconciled: unmodified ones killed " ..
|
||||
"through both removal phases, modified ones kept alive. " ..
|
||||
"Subscribers drop their own path-keyed state. Does not fire " ..
|
||||
"for a delete that failed or was cancelled.",
|
||||
kind = "all-must-succeed",
|
||||
}
|
||||
|
||||
define {
|
||||
name = "editor.before-quit",
|
||||
description = "Fired before the editor exits. Return false to veto.",
|
||||
|
|
|
|||
|
|
@ -364,11 +364,23 @@ local function render_text(handle)
|
|||
return table.concat(lines, "\n")
|
||||
end
|
||||
|
||||
-- Dired's own writes are the only ones that reach the buffer: the
|
||||
-- read-only intercept rejects everything else, and this bypasses it.
|
||||
-- Dired's own writes are the only ones that reach the buffer, and this
|
||||
-- is the one authorized door (Q#GB1,
|
||||
-- docs/generated-buffer-immutability-framing.md).
|
||||
--
|
||||
-- `set_generated_contents` lifts the rope's `read_only`, replaces the
|
||||
-- whole buffer skipping intercepts, discards the resulting history and
|
||||
-- re-asserts the lock --- all inside one registry borrow, so the buffer
|
||||
-- is never observably unlocked. The erroring intercept this replaces a
|
||||
-- bypass write beside is KEPT: it guards the edit path with a named
|
||||
-- error, but `Buffer::undo` reaches the rope through `ensure_writable`
|
||||
-- and never consults the intercept chain, so a listing protected by an
|
||||
-- intercept alone was emptied by a bare `C-/` --- dired rebinds no undo
|
||||
-- chord --- and by `M-x buffer.undo`, which no rebinding can remove.
|
||||
-- Only rope-level `read_only` closes that, and only the pairing keeps
|
||||
-- this repaint working after it.
|
||||
local function paint(handle)
|
||||
local text = render_text(handle)
|
||||
handle.buf:replace(0, handle.buf:len(), text, { bypass_intercept = true })
|
||||
pmacs.buffer.set_generated_contents(handle.buf, render_text(handle))
|
||||
end
|
||||
|
||||
-- ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -163,6 +163,30 @@ end
|
|||
-- If a package needs at-most-one-pending semantics for mutations,
|
||||
-- it should serialize on the package side (await each op before
|
||||
-- dispatching the next). The fs primitive can't enforce that.
|
||||
--
|
||||
-- **And that is a CORRECTNESS precondition, not only a cancellation
|
||||
-- one (dired Stage 2a, Q#DR29).** A successful `rename` or `remove`
|
||||
-- reconciles the editor's path owners in the main-thread drain — buffer
|
||||
-- paths and names, the URI-keyed LSP state, the `resource.renamed` /
|
||||
-- `resource.deleted` hooks. That reconciliation is deliberately
|
||||
-- order-INDEPENDENT: the runtime drains the reply bus with `try_recv`
|
||||
-- and establishes no execution token, so a worker can finish first and
|
||||
-- be descheduled before sending, and reply order therefore does not
|
||||
-- recover filesystem execution order.
|
||||
--
|
||||
-- Independent mutations commute, so nothing is owed for them. But
|
||||
-- **mutations whose source/target paths overlap must be serialized by
|
||||
-- dispatching the next only after the previous handle settles.** There
|
||||
-- is no static ordering rule that would substitute: rename `dir` ->
|
||||
-- `newdir` racing delete `dir/child.txt` needs delete-then-rename if
|
||||
-- the delete ran first on disk and rename-then-delete if the rename
|
||||
-- did, and a fixed "deletes before renames" rule gets one of the two
|
||||
-- wrong — the kill misses, the rename then rebinds the buffer onto a
|
||||
-- path whose file is gone, and it survives pointing at nothing.
|
||||
--
|
||||
-- A caller that ignores this owns the residue: a buffer left bound to a
|
||||
-- stale path, or killed when it should have been rebound. Recoverable
|
||||
-- and visible, not data loss — but real.
|
||||
|
||||
function fs.rename(from, to)
|
||||
if type(from) ~= "string" then
|
||||
|
|
|
|||
|
|
@ -5,11 +5,16 @@
|
|||
-- wholesale re-render, buffer-local RET/n/p/g/q keymap, a
|
||||
-- line->item map, previous-buffer capture + `q` restore, and the two
|
||||
-- disciplines the hand-rolled original lacks --- a read-only
|
||||
-- intercept (Q#P3; the panel's own renders write with
|
||||
-- bypass_intercept) and the Q#P6 round-trip-input mark, so a
|
||||
-- intercept (Q#P3) and the Q#P6 round-trip-input mark, so a
|
||||
-- semantic frontend's RET dispatches into the visit binding instead
|
||||
-- of optimistically inserting a newline.
|
||||
--
|
||||
-- Generated-buffer immutability (Q#GB1, docs/generated-buffer-immutability-framing.md):
|
||||
-- a panel's rope is genuinely read-only, and `render` is its owner's one
|
||||
-- authorized door through the lock. The intercept alone protected the
|
||||
-- edit path and left the history path open, so `C-/` emptied a panel.
|
||||
-- Ownership is the `panels` table, never a name match (Q#GB13/Q#GB18).
|
||||
--
|
||||
-- Panels are buffers, so both frontends render them with zero
|
||||
-- protocol change (Q#P2: switch-in-place; the GPU cannot show
|
||||
-- splits). Framing: docs/lsp-panels-framing.md.
|
||||
|
|
@ -24,9 +29,37 @@
|
|||
|
||||
pmacs.listview = pmacs.listview or {}
|
||||
|
||||
-- name -> { buffer, prev, header, line_to_item, on_visit, on_refresh }
|
||||
-- panels: array of
|
||||
-- { requested_name, buffer, prev, header, line_to_item, on_visit, on_refresh }
|
||||
--
|
||||
-- A LIST scanned by identity, not a name-keyed map (Q#GB18). `panels`
|
||||
-- used to be written under the name the CALLER asked for and read back
|
||||
-- under the buffer's ACTUAL name; those are the same string only while
|
||||
-- `ensure_panel` adopts whatever buffer already carries the name. Once
|
||||
-- ownership disambiguates a collision to `*references*<2>` (Q#GB13), a
|
||||
-- name-keyed lookup can never find its own record, and every consumer
|
||||
-- below fails: `RET`, `g` and `q` fail closed and silently, while
|
||||
-- `open`'s capture guard fails OPEN and captures a panel as its own `q`
|
||||
-- target --- the chained-panel loop its comment says it prevents.
|
||||
--
|
||||
-- Keyed by linear scan over `BufferIdLua.__eq` rather than by table key
|
||||
-- for the same reason dired's `handles` is (dired.lua:120-140): two
|
||||
-- BufferIdLua values for the same buffer are distinct userdata, so a
|
||||
-- `panels[buf]` lookup would miss. `compile.lua`'s `slot_for_buffer`
|
||||
-- is the third instance of this shape; listview adopts it rather than
|
||||
-- inventing a fourth.
|
||||
--
|
||||
-- Dead panels are compacted out on every scan. A map held at most one
|
||||
-- entry per name and self-limited; a list does not, so killing and
|
||||
-- reopening `*references*` ten times would otherwise leave nine dead
|
||||
-- records for every scan to walk.
|
||||
local panels = {}
|
||||
|
||||
-- How far the `<2>`, `<3>`, ... disambiguation walks before giving up.
|
||||
-- dired.lua:474's constant, same value, same give-up-rather-than-adopt
|
||||
-- rule.
|
||||
local NAME_VARIANT_LIMIT = 99
|
||||
|
||||
local function find_buffer_by_name(name)
|
||||
for _, id in ipairs(pmacs.buffer.list()) do
|
||||
local ok, d = pcall(pmacs.describe.buffer, id)
|
||||
|
|
@ -35,18 +68,52 @@ local function find_buffer_by_name(name)
|
|||
return nil
|
||||
end
|
||||
|
||||
local function live_panels()
|
||||
local live = {}
|
||||
for _, p in ipairs(panels) do
|
||||
local ok, valid = pcall(p.buffer.is_valid, p.buffer)
|
||||
if ok and valid then live[#live + 1] = p end
|
||||
end
|
||||
panels = live
|
||||
return live
|
||||
end
|
||||
|
||||
-- The record for the panel `spec.name` asked for. Stable across
|
||||
-- disambiguation: a repeated `listview.open{ name = "*references*" }`
|
||||
-- must reach the same panel even when its buffer is called
|
||||
-- `*references*<2>`.
|
||||
local function panel_for_requested_name(name)
|
||||
for _, p in ipairs(live_panels()) do
|
||||
if p.requested_name == name then return p end
|
||||
end
|
||||
return nil
|
||||
end
|
||||
|
||||
-- The record that owns `buf`, or nil. This is the identity question
|
||||
-- every command below actually asks.
|
||||
local function panel_for_buffer(buf)
|
||||
if buf == nil then return nil end
|
||||
for _, p in ipairs(live_panels()) do
|
||||
if p.buffer == buf then return p end
|
||||
end
|
||||
return nil
|
||||
end
|
||||
|
||||
-- The panel record whose buffer the active window shows, or nil.
|
||||
local function panel_for_current_buffer()
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return nil end
|
||||
local ok, d = pcall(pmacs.describe.buffer, buf)
|
||||
if not (ok and d) then return nil end
|
||||
return panels[d.name]
|
||||
local function active_panel()
|
||||
return panel_for_buffer(pmacs.window.buffer())
|
||||
end
|
||||
|
||||
-- Wholesale re-render: header + one line per row, rebuilding the
|
||||
-- line->item map (data lines are 1-based; the header is line 0).
|
||||
-- Panel writes bypass the read-only intercept.
|
||||
--
|
||||
-- One `set_generated_contents` (the owner-authorized write) rather than
|
||||
-- a delete-all + insert-all pair through `bypass_intercept`. The
|
||||
-- intercept guarded the edit path and left the HISTORY path open, so a
|
||||
-- bare `C-/` --- listview rebinds no undo chord --- emptied the panel;
|
||||
-- `M-x buffer.undo` did too, and no rebinding can remove that. The
|
||||
-- primitive lifts the rope lock, writes, discards the history and
|
||||
-- re-asserts the lock, all inside one registry borrow.
|
||||
local function render(p, rows)
|
||||
local lines = { p.header }
|
||||
p.line_to_item = {}
|
||||
|
|
@ -54,11 +121,7 @@ local function render(p, rows)
|
|||
lines[#lines + 1] = row.text
|
||||
p.line_to_item[#lines - 1] = row.item
|
||||
end
|
||||
local body = table.concat(lines, "\n")
|
||||
local buf = p.buffer
|
||||
local len = buf:len()
|
||||
if len > 0 then buf:delete(0, len, { bypass_intercept = true }) end
|
||||
if #body > 0 then buf:insert(0, body, { bypass_intercept = true }) end
|
||||
pmacs.buffer.set_generated_contents(p.buffer, table.concat(lines, "\n"))
|
||||
end
|
||||
|
||||
-- Re-seat the cursor on data line `line` (1-based, clamped).
|
||||
|
|
@ -87,19 +150,53 @@ local function bind_local_keymap(buf)
|
|||
bind("q", "listview.quit")
|
||||
end
|
||||
|
||||
-- Build (or adopt) the persistent panel record for `name`. Handles a
|
||||
-- user-killed panel buffer by recreating it.
|
||||
-- Build the persistent panel record for `name`. A user-killed panel
|
||||
-- buffer is compacted out by `live_panels`, so the next `open` builds a
|
||||
-- fresh record rather than resurrecting a dead one.
|
||||
--
|
||||
-- Q#GB13: found-by-name is NOT adoption. `pmacs.buffer.create` takes any
|
||||
-- caller-chosen name, so a foreign buffer may already be called
|
||||
-- `*references*`; this used to adopt it, clobber the user's bytes, and
|
||||
-- install an erroring intercept whose handle it discarded --- leaving
|
||||
-- the user's buffer permanently un-editable. Rendering through
|
||||
-- `set_generated_contents` would additionally lock its rope and clear
|
||||
-- the history, removing the `M-x buffer.undo` that is currently the only
|
||||
-- way back. So ownership is "this buffer is in `panels`", a name
|
||||
-- collision disambiguates `<2>`..`<99>`, and exhausting the limit raises
|
||||
-- rather than adopting --- the rule terminal.lua:300-305 states and
|
||||
-- dired.lua:476-504 already implements.
|
||||
local function ensure_panel(name)
|
||||
local p = panels[name]
|
||||
if p and p.buffer:is_valid() then return p end
|
||||
local buf = find_buffer_by_name(name) or pmacs.buffer.create(name)
|
||||
p = { buffer = buf, line_to_item = {} }
|
||||
panels[name] = p
|
||||
-- Read-only (Q#P3): every non-bypass edit is rejected. The
|
||||
-- intercept lives as long as the buffer; no teardown (the
|
||||
-- buffer-list precedent for its keymap).
|
||||
local p = panel_for_requested_name(name)
|
||||
if p then return p end
|
||||
|
||||
local actual = name
|
||||
if find_buffer_by_name(actual) then
|
||||
local unique = nil
|
||||
for i = 2, NAME_VARIANT_LIMIT do
|
||||
local candidate = string.format("%s<%d>", name, i)
|
||||
if find_buffer_by_name(candidate) == nil then
|
||||
unique = candidate
|
||||
break
|
||||
end
|
||||
end
|
||||
if unique == nil then
|
||||
error(string.format("listview: %s is taken and no free variant remains", name))
|
||||
end
|
||||
actual = unique
|
||||
end
|
||||
|
||||
local buf = pmacs.buffer.create(actual)
|
||||
p = { requested_name = name, buffer = buf, line_to_item = {} }
|
||||
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(name .. " is read-only")
|
||||
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).
|
||||
|
|
@ -119,7 +216,7 @@ function pmacs.listview.open(spec)
|
|||
-- (chained panels would trap `q` in a loop; restore targets the
|
||||
-- last real buffer).
|
||||
local active = pmacs.window.buffer()
|
||||
if active and not panel_for_current_buffer() then
|
||||
if active and not panel_for_buffer(active) then
|
||||
p.prev = active
|
||||
end
|
||||
render(p, spec.rows or {})
|
||||
|
|
@ -147,7 +244,7 @@ pmacs.command.define {
|
|||
name = "listview.visit",
|
||||
description = "Visit the list-panel item under the cursor.",
|
||||
fn = function()
|
||||
local p = panel_for_current_buffer()
|
||||
local p = active_panel()
|
||||
if not p then return end
|
||||
local item = p.line_to_item[pmacs.editor.cursor_line()]
|
||||
if item ~= nil and p.on_visit then p.on_visit(item) end
|
||||
|
|
@ -158,14 +255,18 @@ pmacs.command.define {
|
|||
name = "listview.refresh",
|
||||
description = "Re-run the list panel's data source and re-render.",
|
||||
fn = function()
|
||||
local p = panel_for_current_buffer()
|
||||
local p = active_panel()
|
||||
if not (p and p.on_refresh) then return end
|
||||
local saved = pmacs.editor.cursor_line()
|
||||
local rows = p.on_refresh() or {}
|
||||
render(p, rows)
|
||||
-- The wholesale rewrite leaves the window cursor at a stale byte
|
||||
-- offset; re-enter the buffer to reset, then re-seat.
|
||||
pmacs.window.switch_buffer(p.buffer)
|
||||
-- `set_generated_contents` has already refreshed this window's
|
||||
-- TextView. Re-seat through the editor primitives instead of
|
||||
-- switching to the buffer it already shows: that redundant switch
|
||||
-- rebuilt the TextView and hid a missing edit notification.
|
||||
pmacs.editor.clear_selection()
|
||||
pmacs.editor.set_view_top(0)
|
||||
pmacs.editor.move_to_line(0)
|
||||
seat_cursor(p, saved)
|
||||
end,
|
||||
}
|
||||
|
|
@ -174,7 +275,7 @@ pmacs.command.define {
|
|||
name = "listview.quit",
|
||||
description = "Leave the list panel, restoring the previous buffer.",
|
||||
fn = function()
|
||||
local p = panel_for_current_buffer()
|
||||
local p = active_panel()
|
||||
if not p then return end
|
||||
-- Bottom-panel arc (Q#BP11b): `q` keeps its name and its
|
||||
-- user-visible behavior, delegating to `window.quit` only when the
|
||||
|
|
|
|||
|
|
@ -1124,7 +1124,7 @@ pmacs.hook.add("buffer.after-edit", function()
|
|||
-- Stale suppression must stay keystroke-accurate even though the
|
||||
-- O(file) didChange send below is coalesced: render families
|
||||
-- anchored to pre-edit positions are hidden from this edit on.
|
||||
pcall(pmacs.lsp._mark_document_stale, rec.uri)
|
||||
pcall(pmacs.lsp._mark_document_stale, rec.server, rec.uri)
|
||||
-- Arc 1d: was this edit a typed character? The input-origin signal
|
||||
-- (see the trigger block below).
|
||||
local typed = pmacs.editor.this_command
|
||||
|
|
@ -1437,19 +1437,44 @@ local function apply_workspace_edit(ops)
|
|||
end
|
||||
end
|
||||
if #plan == 0 then return 0, 0, 0 end
|
||||
local origin = active_buffer_path()
|
||||
-- G1 — capture the origin BUFFER, not its path. A path captured here
|
||||
-- is a plain Lua local, and no amount of reconciliation can reach an
|
||||
-- already-captured local: once the batch renames or deletes the active
|
||||
-- file, that string names something that is no longer there. The
|
||||
-- handle follows a rename for free, because the buffer is what moved.
|
||||
--
|
||||
-- The framing's G1 described the failure as a "phantom buffer" created
|
||||
-- by `resolve_target_buffer`'s NotFound arm. **That is not what
|
||||
-- happens on this path, and the wrong explanation is recorded here
|
||||
-- rather than left to be rediscovered.** `pmacs.buffer.find_or_open`
|
||||
-- calls `crate::file_io::load_file` directly and maps the error, so a
|
||||
-- missing path RAISES; the NotFound arm belongs to
|
||||
-- `EditorCore::resolve_target_buffer`, which serves
|
||||
-- `pmacs.window.display_file` and the startup/daemon target, not this
|
||||
-- binding. The real defect is quieter: `restore_origin` runs under a
|
||||
-- `pcall`, so the raise is swallowed and the user is left in whatever
|
||||
-- buffer the last applied op made active. And when the old path DOES
|
||||
-- still resolve -- a batch that deletes and then recreates it -- the
|
||||
-- fallback silently opens a file the user asked to delete.
|
||||
local origin_buf = pmacs.window.buffer()
|
||||
local edit_total, files, res_ops = 0, 0, 0
|
||||
-- Plan items fully applied before a failure. Q#RD3 permits partial
|
||||
-- application, so this is what stops a caller claiming "nothing was
|
||||
-- mutated" when something was.
|
||||
local applied_ops = 0
|
||||
-- Return the user to where they invoked from — best-effort, since
|
||||
-- that path may have just been renamed or deleted. Runs on the
|
||||
-- FAILURE path too (Q#RD7): previously this ran only after a
|
||||
-- successful loop, so a mid-batch refusal stranded the user in
|
||||
-- whatever buffer the last applied op left active.
|
||||
-- Return the user to where they invoked from. Runs on the FAILURE
|
||||
-- path too (Q#RD7): previously this ran only after a successful loop,
|
||||
-- so a mid-batch refusal stranded the user in whatever buffer the last
|
||||
-- applied op left active.
|
||||
--
|
||||
-- **No path fallback (G1).** If the origin buffer is gone — the batch
|
||||
-- deleted its file and reconciliation killed it — restore NOTHING.
|
||||
-- "Return the user somewhere plausible" is not worth re-opening a path
|
||||
-- the batch just destroyed, and when that path has been recreated the
|
||||
-- fallback would drop the user into a file they asked to delete.
|
||||
local function restore_origin()
|
||||
if origin then pcall(pmacs.buffer.find_or_open, origin) end
|
||||
if not origin_buf then return end
|
||||
pcall(pmacs.window.switch_buffer, origin_buf)
|
||||
end
|
||||
for _, item in ipairs(plan) do
|
||||
local ok, err
|
||||
|
|
@ -2882,3 +2907,191 @@ pmacs.command.define {
|
|||
|
||||
pmacs.keymap.bind { scope = "global", sequence = "M-g n", command = "diag.next" }
|
||||
pmacs.keymap.bind { scope = "global", sequence = "M-g p", command = "diag.previous" }
|
||||
|
||||
-- Resource reconciliation ---------------------------------------------------
|
||||
--
|
||||
-- dired Stage 2a, §5. A rename or delete moves or destroys a path that
|
||||
-- FOURTEEN URI-keyed store families, the `documents` mirror, the pending
|
||||
-- response routes and the attached diagnostic overlays are all keyed by.
|
||||
-- `EditorCore` reconciles the buffer's own path and name; these two
|
||||
-- subscribers reconcile the LSP layer, which is buffer-keyed here
|
||||
-- (`rec.uri` is cached per buffer and read at dozens of sites, so ONE
|
||||
-- rebind reaches all of them) and URI-keyed in Rust.
|
||||
--
|
||||
-- These subscribers are independent of every other `resource.renamed`
|
||||
-- consumer by construction: this one touches URI-keyed state, dired's
|
||||
-- touches its own handle table, and neither reads what the other wrote.
|
||||
-- That matters because `all-must-succeed` does NOT abort the fan-out —
|
||||
-- `run_all_must_succeed` collects each callback's error and continues —
|
||||
-- so a subscriber may not rely on a raising peer to stop the sequence,
|
||||
-- and the ordered teardown below is ordered INTERNALLY rather than by
|
||||
-- registration.
|
||||
|
||||
-- Every attachment whose document is `path` or lies beneath it, as
|
||||
-- `{ key, rec, path }`. Resolved through `path_for_uri` and compared
|
||||
-- with `paths_related`, so the comparison is component-aware and runs on
|
||||
-- the same canonical form the buffer registry keys on.
|
||||
local function attachments_under(path)
|
||||
local out = {}
|
||||
for key, rec in pairs(attachments) do
|
||||
local rec_path = rec.uri and pmacs.lsp.path_for_uri(rec.uri)
|
||||
if rec_path and paths_related(rec_path, path) then
|
||||
out[#out + 1] = { key = key, rec = rec, path = rec_path }
|
||||
end
|
||||
end
|
||||
return out
|
||||
end
|
||||
|
||||
-- How many attributed failures one status line spells out before
|
||||
-- collapsing the rest into a count.
|
||||
local RESOURCE_REPORT_LIMIT = 2
|
||||
|
||||
-- A failure collector for a reconciliation fan-out.
|
||||
--
|
||||
-- **Why this exists rather than a bare `pcall` per step.** Every step
|
||||
-- below is fallible for reasons outside this file's control -- a stale
|
||||
-- server id makes `forget_uri` raise, a stopped server makes `did_close`
|
||||
-- raise -- and an IGNORED `pcall` makes the hook callback RETURN
|
||||
-- SUCCESSFULLY. `resource.renamed` and `resource.deleted` are
|
||||
-- `all-must-succeed`, so the registry's error logger is the mechanism
|
||||
-- that surfaces a failing subscriber; a callback that swallows its own
|
||||
-- failures gives that logger nothing to log, and the concrete outcome is
|
||||
-- silent: `forget_uri` fails, the callback carries on, and the old
|
||||
-- stores, routes and `documents` entry stay live under a URI the editor
|
||||
-- no longer holds.
|
||||
--
|
||||
-- It must NOT abort the loop. One unreachable server must not leave
|
||||
-- every other attachment unreconciled, so failures accumulate and are
|
||||
-- raised once, after every attachment has been processed.
|
||||
local function failure_sink(hook_name)
|
||||
local sink = { hook = hook_name, items = {} }
|
||||
|
||||
-- Run `fn(...)`, and on a raise record it attributed to `what`.
|
||||
-- Returns `ok, value` like `pcall`, so a caller can branch.
|
||||
function sink:step(what, fn, ...)
|
||||
local ok, value = pcall(fn, ...)
|
||||
if not ok then
|
||||
self.items[#self.items + 1] = string.format("%s: %s", what, tostring(value))
|
||||
end
|
||||
return ok, value
|
||||
end
|
||||
|
||||
-- Report everything collected, on BOTH channels, and raise.
|
||||
--
|
||||
-- The raise is what the `all-must-succeed` logger needs in order to
|
||||
-- write an attributed record to *errors*; the status line is what the
|
||||
-- user actually sees, because stale LSP state looks like the editor
|
||||
-- quietly breaking. `pmacs.error` is deliberately not used: it is
|
||||
-- defined only by a test stub, so writing there would reproduce the
|
||||
-- silence this replaces.
|
||||
function sink:finish()
|
||||
if #self.items == 0 then return end
|
||||
local shown, n = {}, #self.items
|
||||
for i = 1, math.min(n, RESOURCE_REPORT_LIMIT) do shown[i] = self.items[i] end
|
||||
local summary = table.concat(shown, "; ")
|
||||
if n > #shown then
|
||||
summary = summary .. string.format("; and %d more", n - #shown)
|
||||
end
|
||||
pcall(pmacs.editor.set_status,
|
||||
string.format("LSP %s: %d reconciliation failure%s -- %s",
|
||||
self.hook, n, (n == 1 and "" or "s"), summary))
|
||||
error(string.format("%s: %s", self.hook, table.concat(self.items, "; ")), 0)
|
||||
end
|
||||
|
||||
return sink
|
||||
end
|
||||
|
||||
pmacs.hook.add("resource.renamed", function(old_path, new_path)
|
||||
if type(old_path) ~= "string" or type(new_path) ~= "string" then return end
|
||||
local sink = failure_sink("resource.renamed")
|
||||
for _, hit in ipairs(attachments_under(old_path)) do
|
||||
local key, rec, old_uri = hit.key, hit.rec, hit.rec.uri
|
||||
-- The buffer's own path was rebound before this hook fired, so ask
|
||||
-- it rather than reconstructing the tail ourselves. A buffer that
|
||||
-- somehow lost its path (killed, unbound) cannot be re-opened, and
|
||||
-- falls through to the teardown-only path below. Not routed through
|
||||
-- the sink: a pathless buffer is a legitimate state here, not a
|
||||
-- reconciliation failure.
|
||||
local ok_path, new_buf_path = pcall(function() return rec.buffer:path() end)
|
||||
local new_uri = (ok_path and new_buf_path) and file_uri_for(new_buf_path) or nil
|
||||
|
||||
-- 1. Flush any pending didChange for the OLD uri, so the server is
|
||||
-- not left holding an edit it can no longer attribute.
|
||||
sink:step("flush didChange for " .. old_uri, flush_did_change_for, rec)
|
||||
pending_did_change[key] = nil
|
||||
|
||||
-- 2. didClose the old uri — this removes the open-document
|
||||
-- registration and nothing else.
|
||||
sink:step("didClose " .. old_uri, pmacs.lsp.did_close, rec.server, old_uri)
|
||||
|
||||
-- 3. Purge the routes, drain their awaiters, and clear all fourteen
|
||||
-- stores plus `documents` for the old key. Runs against the OLD
|
||||
-- server, which matters when step 4 picks a different one.
|
||||
-- A failure here is the one that most needs reporting: the
|
||||
-- callback would otherwise continue with the old stores, routes
|
||||
-- and `documents` entry all still live.
|
||||
sink:step("forget_uri " .. old_uri, pmacs.lsp.forget_uri, rec.server, old_uri)
|
||||
|
||||
if not new_uri then
|
||||
attachments[key] = nil
|
||||
styled_buffers[key] = nil
|
||||
diag_viewed_buffers[key] = nil
|
||||
else
|
||||
-- 4. Re-run ensure_server. Server affinity keys on the detected
|
||||
-- project root, so a rename ACROSS roots needs a different
|
||||
-- server; a same-root rename reuses the existing one.
|
||||
local ok_sid, sid = sink:step("ensure_server for " .. new_buf_path,
|
||||
ensure_server, rec.language, new_buf_path)
|
||||
if not (ok_sid and sid) then
|
||||
attachments[key] = nil
|
||||
styled_buffers[key] = nil
|
||||
diag_viewed_buffers[key] = nil
|
||||
else
|
||||
-- 5. didOpen the new uri with the buffer's current text and a
|
||||
-- fresh version. This also reclaims the tombstone for
|
||||
-- exactly (server, new uri).
|
||||
rec.server = sid
|
||||
rec.uri = new_uri
|
||||
rec.version = 1
|
||||
local ok_text, text = sink:step("read " .. new_uri, buffer_text, rec.buffer)
|
||||
sink:step("didOpen " .. new_uri, pmacs.lsp.did_open,
|
||||
sid, new_uri, rec.version, ok_text and text or "")
|
||||
-- 6. Re-root the diagnostic overlays. `DiagnosticView.uri` is
|
||||
-- set once at construction and is private, so this is the
|
||||
-- only way to move it — and the sweep reaches PASSIVE
|
||||
-- windows, which the attach path cannot, while preserving
|
||||
-- each overlay's position in the composition order.
|
||||
sink:step("re-root diagnostics to " .. new_uri,
|
||||
pmacs.diag._rename_resource, old_uri, new_uri)
|
||||
end
|
||||
end
|
||||
end
|
||||
-- Raised only after EVERY attachment has been processed: one
|
||||
-- unreachable server must not leave the rest unreconciled.
|
||||
sink:finish()
|
||||
end)
|
||||
|
||||
pmacs.hook.add("resource.deleted", function(path)
|
||||
if type(path) ~= "string" then return end
|
||||
local sink = failure_sink("resource.deleted")
|
||||
for _, hit in ipairs(attachments_under(path)) do
|
||||
local key, rec = hit.key, hit.rec
|
||||
-- No flush: the document is gone, and shipping a didChange for a
|
||||
-- file the server can no longer read buys nothing.
|
||||
pending_did_change[key] = nil
|
||||
sink:step("didClose " .. rec.uri, pmacs.lsp.did_close, rec.server, rec.uri)
|
||||
sink:step("forget_uri " .. rec.uri, pmacs.lsp.forget_uri, rec.server, rec.uri)
|
||||
-- Drop the record unconditionally, INCLUDING after a failure above.
|
||||
-- The buffer may be gone entirely (an unmodified visited file is
|
||||
-- killed), in which case a retained record is a dangling handle that
|
||||
-- `repull_for_attachments` would iterate; and a modified buffer kept
|
||||
-- alive has no file to analyze until it is saved, which re-attaches
|
||||
-- through the ordinary path. Keeping a record whose teardown failed
|
||||
-- would be strictly worse than dropping it: the failure is reported
|
||||
-- either way, and a retained one is re-swept every refresh.
|
||||
attachments[key] = nil
|
||||
styled_buffers[key] = nil
|
||||
diag_viewed_buffers[key] = nil
|
||||
end
|
||||
sink:finish()
|
||||
end)
|
||||
|
|
|
|||
|
|
@ -245,6 +245,139 @@ If it does not, stop and repair the remote/fetch configuration.
|
|||
never been enforced. Any CI job that compiles the `crdt` targets has to
|
||||
fix them first or it will be red on arrival.
|
||||
|
||||
## Generated-buffer immutability lane (Arc: workbench primitives) — STAGE 1 OPEN
|
||||
|
||||
**Framing: [PR #188](https://github.com/levineuwirth/pmacs/pull/188),
|
||||
revision 7, approved and merged to `main` as `27b1185`. #188 owns the
|
||||
acceptance contract; this lane adopts it.** On 2026-07-29 the user
|
||||
directed #191 to fold its review corrections into this branch and then
|
||||
merged #188, settling the implementation authority and merge ordering.
|
||||
The contract is now
|
||||
`docs/generated-buffer-immutability-framing.md` on canonical `main`.
|
||||
|
||||
- **Branch `generated-buffer-immutability-stage1`**, worktree
|
||||
`../pmacs-gbi-stage1`. `githubsucks/main` is integrated into it.
|
||||
Measured when this line was written:
|
||||
|
||||
```
|
||||
$ git rev-parse --short githubsucks/main
|
||||
27b1185
|
||||
$ git log --oneline -1 githubsucks/main
|
||||
27b1185 Merge pull request #188 from levineuwirth/generated-buffer-immutability
|
||||
$ git merge-base --is-ancestor githubsucks/main HEAD && echo "main IS integrated"
|
||||
main IS integrated
|
||||
```
|
||||
|
||||
**That is a reading, not a constant, and it went stale inside this
|
||||
lane's own review round.** `main` moved four times while the lane was
|
||||
open: #187 -> #192 -> #193 -> #188. An earlier revision of this bullet pasted
|
||||
the same three commands with `64883eb` and the same `main IS
|
||||
integrated` line, and #193 merged between writing it and pushing it ---
|
||||
so the pasted output was false in the tree that carried it. Pasting
|
||||
command output is necessary and **not sufficient**: re-measure at push
|
||||
time, and treat any base SHA in this file as expired on sight.
|
||||
- **What Stage 1 ships.** `dired.lua`'s `paint` and `listview.lua`'s
|
||||
`render` write through `pmacs.buffer.set_generated_contents` (zero
|
||||
`bypass_intercept` writes remain in either file); `listview` gains
|
||||
Q#GB13 ownership-by-handle with `<2>`..`<99>` disambiguation and
|
||||
Q#GB18's identity-routed `panels` list in the **same** commit;
|
||||
Q#GB6's cursor/view-top clamp plus selection clamp-or-clear in both
|
||||
`EditorCore::notify_buffer_edit` and `rebuild_views_for`;
|
||||
listview refresh reseating through the already-notified view rather
|
||||
than a redundant same-buffer switch; and Q#GB16(a)'s corrected fold
|
||||
status string. No protocol change, no new Lua surface, no new
|
||||
interaction island.
|
||||
- **Why these two families first, and it is not "the cheap half".**
|
||||
`compile.lua:219` and `builtin/commands/default.lua:855` rebind all
|
||||
seven undo chords to a no-op; `dired.lua` and `listview.lua` rebind
|
||||
**nothing**, so a bare `C-/` emptied a listing and a panel. Stage 1
|
||||
closes the only two families reachable without `M-x`.
|
||||
- **Review round 1 found the stale selection anchor and four acceptance
|
||||
contract mismatches.** Its provisional drop-on-stale fix stopped the
|
||||
crash but intentionally waited on #188 to decide the selection rule;
|
||||
criteria 5 and 7 likewise recorded evidence without claiming to
|
||||
replace the framing. That evidence produced #188 revision 7.
|
||||
- **Review round 2 closes both remaining P1 findings against revision
|
||||
7.**
|
||||
- **Q#GB6 now matches at both sites.** Cursor and anchor clamp to the
|
||||
new extent; a selection survives shortened unless an endpoint
|
||||
movement collapses it, in which case it clears. `acc16h` and
|
||||
`acc16i` each drive a real caller and assert both the surviving
|
||||
region and collapsed case. Unconditional drop and bare clamp are
|
||||
separately falsified.
|
||||
- **The Stage 1 criteria are adopted without local substitutes.**
|
||||
Criterion 5 has the exact rope-refusal + byte-identity half and the
|
||||
Rust-lifted named-intercept half for both adopters. Criterion 7 now
|
||||
bites the named fan-out mutation for both adopters: listview refresh
|
||||
no longer rebuilds the view with a redundant same-buffer switch.
|
||||
Criteria 11 and 12 carry the framing's `[main]` classification and
|
||||
also record where its narrower Q#GB13-without-Q#GB18 pre-image
|
||||
fails.
|
||||
- **Stage 2 still owes everything with new Rust in it**, per the
|
||||
framing's cut: `Buffer::apply_generated_edit` + `GeneratedOutcome` +
|
||||
the `{ generated = true }` option + its own `run_buffer_edit` arm;
|
||||
`set_generated_contents` reimplemented over it; Q#GB10's path-backed
|
||||
refusal and `mark_clean`; Q#GB15's `identity_protected`; Q#GB13/GB18
|
||||
for `compile.lua` and the search panel; Q#GB5's `ensure_slot` lock;
|
||||
conversion of the remaining 13 write sites; and the three
|
||||
`compile_mode_acceptance` intruder tests converted per Q#GB12.
|
||||
- **Verification at code checkpoint `5d92348`.** The ledger commit on
|
||||
top is docs-only; `cargo fmt --check` and `git diff --check` are
|
||||
re-run after it.
|
||||
`cargo fmt --check` clean; `cargo clippy --workspace --all-targets --
|
||||
-D warnings` clean; library **1,863 passed + 3 ignored** default and
|
||||
**2,048 passed + 4 ignored** CRDT; `listview_acceptance` **17**,
|
||||
`dired_acceptance` **31**, `folding_acceptance` **21**,
|
||||
`terminal_copy_mode_acceptance` **18** default and **19** with
|
||||
`--features crdt` — judge that step by the count, because `acc16e` is
|
||||
`#[cfg(feature = "crdt")]` and a default run never compiles it; M4
|
||||
**121 passed + 3 ignored + 1 filtered** with `--skip basedpyright`;
|
||||
required GPU **202/202**. The first GPU attempt inside the tool
|
||||
sandbox failed three managed-attach socket tests and left the
|
||||
closed-outbox reader blocked; the authoritative rerun outside that
|
||||
socket sandbox passed all 202. `git diff --check` clean.
|
||||
- **The dired 200 ms perf test is load-sensitive, and the conversion
|
||||
costs it nothing.** Review saw `dired_renders_10k_entries_within_200ms`
|
||||
take 241 ms in a combined run and pass alone. Measured here: 0.09 s
|
||||
isolated over five runs, and the whole 31-test suite finishes in
|
||||
0.12 s, so 241 ms was contention rather than a regression. Measured
|
||||
against the pre-image as well, by swapping in `main`'s `dired.lua`
|
||||
(the `bypass_intercept` paint): **0.09 s either way over five runs
|
||||
each**. A whole-buffer `set_generated_contents` costs the same as the
|
||||
bypass replace it replaces, which discharges Q#GB4's measurement
|
||||
obligation for the whole-buffer case only — the streaming case is
|
||||
Stage 2's and is not touched here.
|
||||
- **Bites, re-run under `scripts/bite`'s positive control (#192).**
|
||||
A bare `bite: OK` from the pre-#192 script is weaker than it looks, so
|
||||
every result below is from the current script or from a mutation
|
||||
harness carrying the same control (named tests must pass on the
|
||||
working tree and at least one must have run).
|
||||
- **Falsified by revert, all `OK (assertion)` — not `OK (COMPILE)`:**
|
||||
`builtin/runtime/listview.lua` for criteria 1, 2, 9 and 10;
|
||||
`builtin/runtime/dired.lua` for criteria 3 and 13a;
|
||||
`src/lua_bindings/fold.rs` for 13b; `src/editor_core.rs` for 8, 8b
|
||||
and both selection-normalization pins.
|
||||
- **Falsified by a named mutation, each observed to fail:** the
|
||||
fan-out drop in the `set_generated_contents` binding (criterion 7);
|
||||
deleting `self.read_only = false` (criterion 4, both adopters);
|
||||
deleting `add_intercept` and `set_round_trip_input` at each adopter
|
||||
(criteria 5 and 6); the name-keyed `panel_for_buffer` (criteria 11
|
||||
and 12); adopting at the variant limit (criterion 10); the old fold
|
||||
status string (13b); deleting each clamp (8, 8b); deleting the
|
||||
selection helper from either site; unconditionally dropping a stale
|
||||
anchor; and retaining a selection that an endpoint clamp collapsed.
|
||||
Criterion 7's fan-out drop now fails by assertion in **both**
|
||||
listview and dired.
|
||||
- **Recovery:**
|
||||
|
||||
```sh
|
||||
git fetch githubsucks
|
||||
git worktree add ../pmacs-gbi-stage1 generated-buffer-immutability-stage1
|
||||
cd ../pmacs-gbi-stage1
|
||||
cargo test --test listview_acceptance --test dired_acceptance
|
||||
cargo test --test terminal_copy_mode_acceptance --features crdt
|
||||
```
|
||||
|
||||
## Bottom-panel lane (Arc 7) — 2B-3 OPEN; Stage 2 is COMPLETE with it
|
||||
|
||||
Stage 1, the Stage 2 framing, Stage 2A, Stage 2B-1, and Stage 2B-2 are
|
||||
|
|
@ -764,21 +897,193 @@ has **no branch and no framing yet**.
|
|||
`git fetch githubsucks && git worktree add ../pmacs-rd-impl
|
||||
-b resource-op-delete-guard-impl githubsucks/resource-op-delete-guard-impl`.
|
||||
|
||||
## Generated-buffer immutability framing lane — PR #188 OPEN, PROPOSED
|
||||
## dired Stage 2a — rename/delete reconciliation — PR #196 OPEN, review round 1 closed
|
||||
|
||||
- Portable branch: `githubsucks/dired-stage2-impl`, worktree
|
||||
`../pmacs-dired-s2`. Implements **Stage 2a only** of the framing merged
|
||||
as #171 (`docs/dired-stage2-framing.md` rev 9, §5/§6/§10 — the
|
||||
substrate transaction, no dired surface). Position against `main`, as
|
||||
pasted command output rather than a remembered constant:
|
||||
|
||||
```
|
||||
$ git merge-base HEAD githubsucks/main
|
||||
e003b81cdd577140fc77330bd4578d3090696877
|
||||
```
|
||||
|
||||
That base is the #190 merge, and #190 matters here specifically:
|
||||
Stage 2a **adopts** its `delete_verdict` refusal rather than
|
||||
reinventing one, and lifts its walk query out into
|
||||
`editor_core::buffers_bound_under` so the guard and both
|
||||
reconciliation seams cannot disagree about which buffers an operation
|
||||
touches. **Re-measure the merge-base before relying on it** —
|
||||
`main` has branch protection, all 12 checks must pass on the merging
|
||||
head, and a conflicting PR builds no merge ref at all, so a green run
|
||||
from before a move reads as current when it is not. **Re-measured after
|
||||
round 1: `main` had not moved, so no integration was needed** — that is
|
||||
a reading of the tree, not a standing fact.
|
||||
- **What 2b and 2c still owe, stated so the split boundary is auditable.**
|
||||
2a ships **no user-visible surface at all** and no dired code: the
|
||||
`dired_acceptance` count is deliberately unchanged at **25**, and a
|
||||
moved count there would mean it touched something it should not have.
|
||||
2b owes the mark and operation layer (`m u U t d x D R w M`),
|
||||
`pmacs.minibuffer.confirm` plus its `src/editor.rs` load-sequence line,
|
||||
`pmacs.killring.push`, dired's own `resource.renamed` subscriber, and
|
||||
acceptance 1–22, 33, 39–41. 2c owes `mkdir`/`copy`/`remove_dir_all`,
|
||||
`JobKind` 12 → 15, `dired.recursive-deletes`, and acceptance 42–47.
|
||||
- **The split boundary has not moved since rev 9.** It was re-checked
|
||||
against this tree: #188 (generated-buffer immutability Stage 1) did not
|
||||
convert dired's `paint`, so §3.1's coordination note is still an
|
||||
obligation of that lane rather than a collision with this one, and
|
||||
nothing in this diff touches `builtin/runtime/dired.lua`.
|
||||
- **Two m4 rows were re-pinned, and that is a behaviour change to a
|
||||
landed lane's assertions.** `rd9` and `rd14` pinned #190's deliberate
|
||||
restraint on the `apply_resource_op` delete arm — descendants stay
|
||||
orphaned, only the first of two duplicate path-bound buffers is
|
||||
reconciled — and both doc comments gave the same reason: widening
|
||||
would have routed N buffers through `remove_buffer_and_fire`, phase 2
|
||||
without phase 1, leaving up to N windows on removed ids.
|
||||
`EditorCore::reconcile_delete` composes both phases, so the constraint
|
||||
is discharged and the old assertions became the defect. Each row now
|
||||
asserts BOTH directions — reconciled away **and** no window holding a
|
||||
removed id — and each direction is bite-verified.
|
||||
- **One framing claim is wrong and is corrected at the test, not
|
||||
silently worked around.** §5's G1 says a stale captured path
|
||||
"materializes a phantom" by reaching `resolve_target_buffer`'s
|
||||
`NotFound` arm. It does not: `pmacs.buffer.find_or_open` calls
|
||||
`crate::file_io::load_file` directly and maps the error, so a missing
|
||||
path **raises**, and the `NotFound` arm belongs to
|
||||
`resolve_target_buffer`, which serves `pmacs.window.display_file` and
|
||||
the startup/daemon target rather than that binding. The defect is real
|
||||
and smaller: the `pcall` swallows the raise, so the user is stranded
|
||||
wherever the last applied op left them. Acceptance 34 is restructured
|
||||
to bite on that (its plan edits another file first, which is what makes
|
||||
the restore observable at all) and the correction is recorded in the
|
||||
test's own doc comment.
|
||||
- **Two bites were vacuous as the framing specified them, and both
|
||||
reasons are worth keeping.** Item 28's *rename* row cannot pin the
|
||||
walk's containment rule: `reconcile_rename` calls
|
||||
`Path::strip_prefix` to rebuild a descendant's tail, and that is
|
||||
component-aware too, so a string-prefix walk is silently corrected a
|
||||
second time. The row moved to the **delete** side, where the walk's
|
||||
verdict IS the kill list. Item 30's composition-order assertion was a
|
||||
tautology: the LSP attach leaves `diagnostic` **last** in the stack, and
|
||||
moving the last element to the end is a no-op, so a remove-and-re-push
|
||||
was indistinguishable from an in-place mutation; the row now pushes one
|
||||
more overlay after it and asserts that precondition explicitly.
|
||||
- **23 acceptance criteria are bite-verified by executed mutation**, each
|
||||
labelled `OK (assertion)` — none merely `OK (COMPILE)`, and none
|
||||
vacuous. Items 25, 27, 28, 29 (both directions), 30 (both mutations),
|
||||
31, 31b (both gates), 31d (both halves), 34, 50 (both mutations), 51,
|
||||
52, 53b, 54, 55, plus the two re-pinned m4 rows in three
|
||||
configurations.
|
||||
- **Review round 1 found four defects; all four are fixed, and all four
|
||||
were the same shape — a failure that left state wrong and told nobody.**
|
||||
Worth keeping as one lesson rather than four bugs: every one of them
|
||||
was a `pcall` or a discarded return value, and each *looked* like
|
||||
defensive coding.
|
||||
- **P1 — delete refusals were silent.** `reconcile_delete_and_fire`
|
||||
returned `kept_modified` and `refused` and both production callers
|
||||
discarded them, so a last-buffer refusal or the asynchronous
|
||||
modified-buffer race left the file gone and the buffer still bound to
|
||||
it — and the next `C-x C-s` recreates the deleted file. Reporting
|
||||
moved **inside the shared seam**, for the same reason the
|
||||
reconciliation lives there: a caller that has to remember to report
|
||||
is a caller that will forget. Channel is `EditorCore::status`;
|
||||
**not `pmacs.error`**, which is defined only by a test stub, so a
|
||||
report there would have been the same silence.
|
||||
- **P2 — the LSP subscribers swallowed their own reconciliation
|
||||
failures.** Ignored `pcall`s made the callback return successfully,
|
||||
so the `all-must-succeed` logger had nothing to log. A shared
|
||||
failure sink now attributes each step and raises **after** the loop,
|
||||
because a fix that aborts on the first failure would leave every
|
||||
other attachment unreconciled — that wrong fix is itself a
|
||||
bite-verified mutation.
|
||||
- **P2 — `forget_uri` left purged requests live in the client.** It
|
||||
dropped `pending_routes` and `pending_external` but not the ids
|
||||
`send_request` puts in `LspClient.pending`, and recorded nothing in
|
||||
`cancelled_rids`, so a server that never replies leaked the entry and
|
||||
a late reply surfaced as a generic unrouted response. The per-rid
|
||||
work is now extracted from `drain_cancelled_externals` as
|
||||
`abandon_request` and **reused** rather than copied.
|
||||
- **P2 — acceptance 35 was unpinned even after the G1 correction.**
|
||||
With a plain delete the forbidden path fallback is unobservable:
|
||||
`find_or_open` raises out of `load_file` and the `pcall` swallows it,
|
||||
so both assertions passed with the fallback present. The plan now
|
||||
deletes the origin's file **and recreates it**, which gives the
|
||||
fallback something to open. The corrected G1 explanation also reached
|
||||
the production comments, which still repeated the false
|
||||
`resolve_target_buffer::NotFound` story — *a correction that stops at
|
||||
the test comment has only half landed.*
|
||||
- **One round-1 pin passed with its own bug restored, and the reason is
|
||||
reusable.** Acceptance 53 asserted `contains("only.txt")` for the
|
||||
buffer-name attribution — but the status line opens with
|
||||
`deleted only.txt:`, the deleted path's **basename**, so stripping the
|
||||
attribution changed nothing the assertion could see. Both halves now
|
||||
assert the buffer's *own* name, which for a path-backed buffer is the
|
||||
full path and which only the attribution can produce. **A pin written
|
||||
to close a review finding is exactly the kind that passes with the bug
|
||||
restored**, and the detector was running the bite rather than reading
|
||||
the assertion.
|
||||
- **31 bites now, all executed, every one labelled `OK (assertion)`** —
|
||||
the original 23 plus 8 for round 1 (report call removed; refusal reason
|
||||
unattributed; kept-modified name dropped; subscriber failures
|
||||
swallowed; the wrong fix that aborts the loop; `forget_uri` skipping
|
||||
`abandon_request`; and the forbidden path fallback restored, which must
|
||||
fail acceptance 34 **and** 35 independently).
|
||||
- Verification at this head, each gate run to its own file and its own
|
||||
exit code checked (never through a pipe): `cargo fmt --check` clean;
|
||||
`cargo clippy --workspace --all-targets -- -D warnings` clean;
|
||||
`cargo test --lib` **1,876** passed / 3 ignored; `--lib --features
|
||||
crdt` **2,061** / 4 ignored; the new
|
||||
`resource_reconciliation_acceptance` **25** default and **25** crdt;
|
||||
`dired_acceptance` **25** and **25** crdt, deliberately unmoved; the
|
||||
frozen additivity gate `m8_1` **10** / `m8_2` **15** / `m8_3` **32**,
|
||||
all unchanged; `m4_acceptance -- --skip basedpyright` **149** passed /
|
||||
3 ignored / 1 filtered; `lsp_multi_root_acceptance` **13**;
|
||||
`lsp_dispatch_seams_acceptance` **15**;
|
||||
`typed_edit_chain_acceptance` **13**; `journey_acceptance` **24**
|
||||
(the ratchet floor, asserted as a count rather than a colour);
|
||||
`gpu_invocation_acceptance` **15** crdt — **and that number is only
|
||||
real with `pmacs` and `pmacs-gpu` built first**, which is the `a37`
|
||||
trap in §5: the same command reported 12 failures before the build and
|
||||
15 passes after, so a red run there is not evidence of a regression
|
||||
until the binaries exist; `PMACS_REQUIRE_GPU=1 cargo test -p
|
||||
pmacs-gpu` **202**; isolated-`XDG_CONFIG_HOME` workspace sweep with
|
||||
`--no-fail-fast` **3,559** passed across **104** suites, 19 ignored, 0
|
||||
failed; `git diff --check` clean. Every one of those was run as its own
|
||||
step with its own exit status checked — never `cmd | tail` inside an
|
||||
`&&` chain, which returns *tail's* status and has masked a real failure
|
||||
in this repo before.
|
||||
- **Ownership, per the framing's own warning.** §16 says 2a must not run
|
||||
concurrently with **Journey Stage 1b**, because 1b's LSP
|
||||
spawn-failure reporting lands in `builtin/runtime/lsp.lua`'s
|
||||
attachment lifecycle and 1b's compile/binding half touches
|
||||
`src/editor_core.rs` — the same two files 2a rewrites, where the
|
||||
conflicts are semantic rather than textual so a clean `git merge`
|
||||
proves nothing. **1b must not be started while this PR is open.** No
|
||||
other lane in flight touches them: #188 is `dired.lua`/`buffer.rs`
|
||||
generated-buffer writes, and the bottom-panel and CI lanes are
|
||||
elsewhere.
|
||||
- Recovery from a clean checkout:
|
||||
`git fetch githubsucks && git worktree add ../pmacs-dired-s2
|
||||
-b dired-stage2-impl githubsucks/dired-stage2-impl`.
|
||||
|
||||
## Generated-buffer immutability framing lane — MERGED AS PR #188
|
||||
|
||||
- Portable branch: `githubsucks/generated-buffer-immutability`; worktree
|
||||
`../pmacs-generated-immutability`. **PR #188**, base `main`, forked from
|
||||
`githubsucks/main` @ `ad41cf1`, **integrated through `5e186c7`** —
|
||||
`../pmacs-generated-immutability`. **PR #188 landed on `main` as
|
||||
`27b1185` on 2026-07-29**, after forking from
|
||||
`githubsucks/main` @ `ad41cf1` and integrating through `5e186c7` —
|
||||
#189 (clean), then #186 and #171 (`docs/active-work.md` conflict),
|
||||
then #187 (the same file again, after it removed the two landed
|
||||
framing lanes), #192 at merge commit `76cfaac`, and #193
|
||||
(`docs/active-work.md` conflict again) after revision 7's first push.
|
||||
Revision 6 was reviewed at head `55c3061`; revision 7 closes that
|
||||
round. Framing only —
|
||||
round. The retained branch is provenance only. Framing only —
|
||||
`docs/generated-buffer-immutability-framing.md`, revision 7, plus this
|
||||
lane. **No runtime code, no protocol change.**
|
||||
- **PROPOSED — six review rounds closed (thirty-two findings,
|
||||
twenty-two P1, ten P2). Not approved. Do not implement, do not merge.**
|
||||
- **APPROVED and merged after six review rounds** (thirty-two findings,
|
||||
twenty-two P1, ten P2). Revision 7 is the governing contract.
|
||||
- **Stage 1 implementation is PR #191, open. The boundary is explicit
|
||||
and has already been needed twice:** #188 owns the **acceptance
|
||||
contract**; #191 **adopts** criteria and may not restate, narrow, or
|
||||
|
|
@ -855,7 +1160,7 @@ has **no branch and no framing yet**.
|
|||
panel's four, compile/search ownership + routing, the path-backed
|
||||
refusal plus `mark_clean`, and the terminal-only
|
||||
`identity_protected` guard. **No Lua unlock ships.**
|
||||
- **Nine facts from this lane that other lanes need before it merges:**
|
||||
- **Nine facts this lane landed for other lanes:**
|
||||
- **`bypass_intercept` is the wrong inventory key.** It misses
|
||||
`*buffer-list*`, `*help*` and `*workers*`, which are generated with
|
||||
plain writes and no intercept at all. `docs/agent-handoff.md` §4's
|
||||
|
|
@ -1062,6 +1367,7 @@ has **no branch and no framing yet**.
|
|||
at 42 insertions against 88 deletions — merging it would *revert*
|
||||
current documentation. The section said "whoever confirms the branch
|
||||
carries nothing unique removes the section"; this is that.
|
||||
|
||||
## Test-improvement arc, lane 3a — CI timeouts and concurrency
|
||||
|
||||
- Portable branch: `githubsucks/ci-timeouts-concurrency`, worktree
|
||||
|
|
@ -1148,6 +1454,96 @@ has **no branch and no framing yet**.
|
|||
`git fetch githubsucks && git worktree add ../pmacs-ci3
|
||||
-b ci-timeouts-concurrency githubsucks/ci-timeouts-concurrency`.
|
||||
|
||||
## Test-improvement arc, lane 4 — process teardown stdin deadlock
|
||||
|
||||
- Portable branch: `githubsucks/process-teardown-stdin-deadlock`,
|
||||
worktree `../pmacs-hang`. Implements
|
||||
`docs/process-teardown-stdin-deadlock-framing.md` (rev 3: one review
|
||||
round, then a CI round that falsified the reproduction).
|
||||
- **Base, measured rather than quoted:**
|
||||
|
||||
```
|
||||
$ git log --oneline -1 githubsucks/main
|
||||
e003b81 Merge pull request #190 from levineuwirth/resource-op-delete-guard-impl
|
||||
```
|
||||
|
||||
- Recovery from a clean checkout:
|
||||
`git fetch githubsucks && git worktree add ../pmacs-hang
|
||||
-b process-teardown-stdin-deadlock
|
||||
githubsucks/process-teardown-stdin-deadlock`.
|
||||
- **The defect:** `RuntimeHandles::drop` joined its reader threads in
|
||||
the `Drop` **body**, which runs before any field drops. The
|
||||
`ChildStdin` sink lives in the `stdin` **field**, so it could only be
|
||||
released after the join returned — and the join waited on readers
|
||||
blocked in `read()` on pipes whose write ends the child still held,
|
||||
because the child never got the stdin EOF that would have made it
|
||||
exit. A closed cycle inside one function; teardown hung forever.
|
||||
- **This is the root cause of the `m4_5_basedpyright` hang** that has
|
||||
parked `--workspace` sweeps (once for 2h26m) and forced
|
||||
`-- --skip basedpyright` into every gate recipe. The handoff's §3
|
||||
claim that the desktop's binary was broken is **retired by this PR**:
|
||||
the binary was fine. `basedpyright-langserver` is a uv console script
|
||||
that runs bundled `node` via `subprocess.run` and **waits**; at
|
||||
teardown `shutdown()` SIGTERMs the recorded pid (the wrapper), which
|
||||
dies without forwarding, and **that** orphans node to `PPid: 1`
|
||||
holding the pipes. A direct binary like `clangd` is a genuine child
|
||||
whose pipes close on reap. That is the whole of the "intermittent"
|
||||
story.
|
||||
- **Corrected in review round 2:** rev 1–3 said the wrapper "spawns node
|
||||
and exits". Wrong — and refutable from evidence already in hand, since
|
||||
the initialize handshake succeeds, which a wrapper that exited at spawn
|
||||
could not have done. The `PPid: 1` observation was taken *after*
|
||||
`shutdown()` had killed the wrapper. **We create the orphan.** The fix
|
||||
is unaffected; the parked follow-up changes from "tolerate
|
||||
self-orphaning servers" to "stop orphaning them" (signal the group).
|
||||
- **Diagnosis method, because reproduce-first was the instruction:**
|
||||
gdb thread stacks plus `/proc` fd forensics on a live wedged process,
|
||||
both pipe ends identified in both processes, reproduced 5/5. Three
|
||||
earlier reproductions were vacuous — see the handoff §5 lesson; the
|
||||
shipped test carries two positive controls because of it.
|
||||
- Verification (each gate its own step, real exit status, no
|
||||
`cmd | tail`): fmt 0; `git diff --check` 0; clippy 0; `--lib` 1864
|
||||
passed; `--lib --features crdt` 2049 passed; **`m4_acceptance`
|
||||
without the skip 150 passed in 2.66s with the basedpyright test
|
||||
`ok`**; the **eleven** PTY/REPL/worker/panel suites of the framing's
|
||||
Bet 2 all 0 (144 tests); `PMACS_REQUIRE_GPU=1 -p pmacs-gpu` 202
|
||||
passed. Bite verified by revert: `ok` in 2.03s with the fix, FAILED on
|
||||
timeout at 10.00s without it, both controls passing first.
|
||||
- **CI round 1 falsified the reproduction, and the control is what
|
||||
caught it.** Three Test legs failed on `9b1cf3d`'s predecessor: the
|
||||
synthetic child used `sh -c 'cat <&0 & exit 0'`, and `<&0` does not
|
||||
defeat the `/dev/null` rule it was chosen for — the rule applies
|
||||
*before explicit redirections*, so fd 0 is already `/dev/null` and the
|
||||
redirect duplicates it onto itself. `bash` skips the default when a
|
||||
stdin redirect is present; **`dash`, which is Ubuntu's and CI's
|
||||
`/bin/sh`, does not.** Local probing through `/bin/sh` could not see
|
||||
it. Now `setsid --fork cat`, with no shell at all. **Lesson recorded in
|
||||
the handoff §5: never probe shell behaviour through `/bin/sh` — name
|
||||
the implementation.**
|
||||
- **`acc28` on macos/lua54 was a flake, established not assumed.**
|
||||
`bottom_panel_stage1_acceptance::acc28` failed once on that leg;
|
||||
rerunning the same job on the *identical* head passed, and the suite is
|
||||
46/46 locally. It is now in Bet 2's falsifier list — its absence from
|
||||
rev 1 was a real gap, since it drives real child input through a PTY in
|
||||
a panel and this PR changes PTY-mode teardown ordering.
|
||||
- **Not fixed here, parked in the framing §5:** cancellable non-group
|
||||
`read` (covers a child that ignores EOF, and one that stops draining
|
||||
while `write_all` is blocked); the orphaned-server **leak** — post-fix
|
||||
the server exits by cooperation, not enforcement.
|
||||
- `CLAUDE.md`'s `--skip basedpyright` entry is deliberately untouched.
|
||||
Dropping it is a separate proposal owed evidence of repeated green.
|
||||
The timeout precondition is **already satisfied** — #195 (this PR's
|
||||
base) gave every job a `timeout-minutes` — so the only remaining reason
|
||||
`PMACS_REQUIRE_PYRIGHT` stays unarmed is that CI does not install
|
||||
basedpyright at all; arming it would fail rather than test anything.
|
||||
- Adds `PMACS_REQUIRE_SETSID`, armed on Linux. The teardown test's
|
||||
fixture needs `setsid --fork`, which is util-linux rather than
|
||||
coreutils, so it **skips** when absent (the standard `--lib` gate must
|
||||
not hard-fail a minimal container on an undeclared tool) and the
|
||||
variable makes that skip fatal where the tool is guaranteed. Both arms
|
||||
verified against a PATH with `setsid` genuinely removed: unarmed skips,
|
||||
armed FAILS. README's test-dependency list declares it.
|
||||
|
||||
## Parked lane: kill-ring browser + persistence
|
||||
|
||||
- Portable branch: `githubsucks/kill-ring-browser`
|
||||
|
|
|
|||
|
|
@ -1377,10 +1377,32 @@ git diff --check
|
|||
Machine-specific caveats — re-verify on a machine you haven't used
|
||||
before trusting them:
|
||||
|
||||
- **basedpyright**: the DESKTOP's local binary is broken and HANGS the
|
||||
`m4_5_basedpyright` tests — hence the `--skip` there. The LAPTOP has
|
||||
a working basedpyright 1.39.9 (verified 2026-07-10: the m4_5 test
|
||||
passes in 0.18s), so the skip is droppable on the laptop.
|
||||
- **basedpyright**: the desktop binary was **never broken** — this was a
|
||||
real code defect, diagnosed and fixed 2026-07-29 (see §5, "A `Drop`
|
||||
body runs before its fields"). `RuntimeHandles::drop` joined its reader
|
||||
threads before the `stdin` field dropped, so the server never got stdin
|
||||
EOF, never exited, and kept the output pipe the readers were blocked
|
||||
on. Deterministic on the desktop, invisible on the laptop and in CI,
|
||||
which is why it read as a broken local binary for weeks.
|
||||
**How the orphan is actually made — WE make it.** basedpyright's
|
||||
console script runs bundled `node` through `subprocess.run` and
|
||||
**waits** (`nodejs_wheel/executable.py:50`, verified in 1.39.6). At
|
||||
teardown `shutdown()` SIGTERMs the *recorded* pid — the Python wrapper
|
||||
— which dies without forwarding the signal, orphaning node to `PPid 1`
|
||||
holding the pipes. An earlier revision of this entry said the wrapper
|
||||
"spawns node and exits"; that was wrong, and the refutation was already
|
||||
in hand, since the initialize handshake succeeds, which a
|
||||
wrapper that exited at spawn could not have done. The consequence is
|
||||
for the follow-up, not the fix: the orphan-management work is **stop
|
||||
orphaning them** (signal the group), not tolerate self-orphaning.
|
||||
The `--skip` above stays for now: it is still correct on any tree
|
||||
predating the fix, and — the one live reason — **CI never installs
|
||||
basedpyright at all**, so arming `PMACS_REQUIRE_PYRIGHT` would fail
|
||||
rather than test anything. The two original reasons are both gone: the
|
||||
hang is fixed, and #195 gave every job a `timeout-minutes`, so a hang
|
||||
can no longer burn six hours. Installing basedpyright in CI (a uv plus
|
||||
bundled-node download per leg) and dropping the local skip are two
|
||||
separate proposals, each owed its own evidence.
|
||||
- **GPU on the laptop**: AMD Radeon 780M (RADV) — native Vulkan,
|
||||
`PMACS_REQUIRE_GPU=1` works without lavapipe.
|
||||
- **Flaky-under-load tests — rerun isolated before treating a sweep
|
||||
|
|
@ -1547,6 +1569,47 @@ round-trip cannot detect a discriminant shift.
|
|||
asks you to keep. Same family as the skip-reports-`ok` lesson below
|
||||
and the double-invocation traps: **the thing that summarizes a gate
|
||||
must not be able to lose the gate's verdict.**
|
||||
- **A reproduction is a measurement, and needs its own positive control.**
|
||||
The basedpyright-hang lane wrote **four** reproductions that passed
|
||||
against the *unfixed* tree, each vacuous for a different reason: the
|
||||
child exited before the join; the child never read stdin at all; the
|
||||
child's stdin was silently rebound to `/dev/null` (POSIX XCU §2.9.3
|
||||
assigns `/dev/null` to an asynchronous list's stdin when job control is
|
||||
off, so `sh -c 'cat & exit 0'` EOFs instantly); and then **the repair
|
||||
for that was also wrong** — the rule applies *before explicit
|
||||
redirections*, so `<&0` duplicates `/dev/null` onto itself. `bash`
|
||||
skips the default when a stdin redirect is present, `dash` does not, so
|
||||
`<&0` passed locally and failed in CI. The shipped test uses
|
||||
`setsid --fork`, removing the shell from the reproduction entirely.
|
||||
Every one of the four looked obviously right when written, and the
|
||||
fourth was verified locally before it failed. Note what a narrower rule
|
||||
would have missed: "check the child is still alive" catches only the
|
||||
first. Only the general form catches all four — **and the ones nobody
|
||||
has invented yet.** Note also which mechanism caught the fourth: not a
|
||||
reviewer, but the control itself, failing loudly in CI and naming its
|
||||
own cause. So: assert the precondition your reproduction
|
||||
depends on, in the test, before exercising the thing under test. In
|
||||
`teardown_closes_stdin_before_joining_readers` that is two controls
|
||||
(the recorded child has exited; both readers are still blocked in
|
||||
`read`), each with a failure message naming what its absence means —
|
||||
and a `/bin/sh` that is `bash` locally and `dash` in CI is exactly the
|
||||
sort of divergence no amount of local verification reaches.
|
||||
This is the same rule that produced #192's bite positive control and
|
||||
#194's re-read-the-artifact lesson, stated at full generality: **a
|
||||
measurement you have not controlled is a claim, not evidence.**
|
||||
- **A `Drop` body runs before its fields, whatever the declaration
|
||||
order.** Cost a multi-week misattribution: `RuntimeHandles::drop`
|
||||
joined its reader threads in the drop *body*, while the `stdin` sink it
|
||||
needed to close first sat in a *field* — reachable only after that body
|
||||
returned. The child never got EOF, never exited, and kept the output
|
||||
pipe the readers were blocked on, so teardown hung forever. Reordering
|
||||
the struct's fields cannot fix this shape; the operation has to move
|
||||
into the body. Generally: **if a `Drop` body waits on anything, check
|
||||
what the waited-on party needs that only a field drop will release.**
|
||||
Corollary from the same investigation — `cancel`-flag style wake-outs
|
||||
only work where the thread actually polls them; a thread blocked in a
|
||||
raw `read` never sees one, so a flag next to a blocking syscall is
|
||||
documentation, not a mechanism.
|
||||
- **A test that skips on a missing precondition reports `ok`, and a gate log
|
||||
cannot tell that apart from a pass.** `vterm_stage3_acceptance::a37` — the
|
||||
only acceptance driving a real daemon, a real PTY and a real wgpu render
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
# Generated-buffer immutability
|
||||
|
||||
**PROPOSED — needs explicit user approval before implementation. DO NOT
|
||||
implement, DO NOT merge.**
|
||||
**APPROVED and MERGED as PR #188** (`main` @ `27b1185`, 2026-07-29).
|
||||
Stage 1 implementation is PR #191.
|
||||
|
||||
**Revision 7 — answers review round 6 on `55c3061`, authored against
|
||||
canonical `githubsucks/main` @ `64883eb` and integrated through
|
||||
|
|
|
|||
|
|
@ -0,0 +1,561 @@
|
|||
# Framing — close child stdin before joining readers (process teardown deadlock)
|
||||
|
||||
A pipe-mode child that exits on stdin EOF can deadlock the supervisor's
|
||||
teardown forever. `RuntimeHandles::drop` joins its reader threads in the
|
||||
`Drop` body, which runs **before** the `stdin` field drops, so the child
|
||||
never receives the EOF that would make it close the very pipe write ends
|
||||
those readers are blocked on. The fix is a two-line reorder that reuses a
|
||||
mechanism already present in this file.
|
||||
|
||||
This is the diagnosed root cause of
|
||||
`m4_5_basedpyright_initializes_and_negotiates_encoding` hanging
|
||||
indefinitely — the hazard that has parked `cargo test --workspace` runs
|
||||
(once for 2h26m) and forced `-- --skip basedpyright` into every gate
|
||||
recipe.
|
||||
|
||||
**Scope: `src/process.rs` only. No protocol change. No Lua surface. No
|
||||
new primitive.**
|
||||
|
||||
---
|
||||
|
||||
## Revision history
|
||||
|
||||
- **rev 1** — initial framing. Root cause established by live diagnosis
|
||||
(gdb stacks + `/proc` fd forensics on a wedged process), reproduced
|
||||
5/5 deterministically at `e003b81`.
|
||||
- **rev 2** — review round 1. rev 1's synthetic child was **itself
|
||||
vacuous** (the third in this lane): POSIX assigns `/dev/null` to a
|
||||
background job's stdin when job control is off, so `sh -c 'cat &
|
||||
exit 0'` EOFs instantly and exits against the *unfixed* tree.
|
||||
Q#TD6 now uses the explicit-redirect form and criterion 2 gains a
|
||||
positive control. Also: Q#TD3's bound widened to cover a blocked
|
||||
stdin writer (a child that read stdin but stopped draining it),
|
||||
criterion 5 extended to `docs/agent-handoff.md` §3, Bet 4 marked as
|
||||
lane-stopping.
|
||||
- **rev 4** — review round 2. §1.7's causal account was **wrong**: the
|
||||
basedpyright wrapper uses `subprocess.run` and *waits*; the orphan is
|
||||
created by pmacs SIGTERMing the wrapper at shutdown, not by the wrapper
|
||||
exiting at spawn. Corrected here, in the handoff and in the ledger, and
|
||||
§5's P2 restated — the follow-up is "stop orphaning them", not "tolerate
|
||||
self-orphaning". Also: the `setsid` dependency is now skip-unless-armed
|
||||
rather than a hard assert, since it is util-linux and the standard
|
||||
`--lib` gate must not fail on an undeclared tool.
|
||||
- **rev 3** — CI falsified rev 2's repair. `<&0` is defeated on `dash`
|
||||
(the rule applies *before* explicit redirections, so `<&0` duplicates
|
||||
`/dev/null` onto itself); it passed locally only because `/bin/sh` here
|
||||
is `bash`. **Control 2 caught it in CI and named its own cause** — the
|
||||
fourth vacuity in this lane, and the first one a control found instead
|
||||
of a reviewer. The reproduction now uses `setsid --fork cat`, removing
|
||||
the shell entirely. Bet 2's falsifier list also gained
|
||||
`bottom_panel_stage1_acceptance`, which holds PTY-in-panel tests and
|
||||
was a genuine gap in rev 1's list.
|
||||
|
||||
---
|
||||
|
||||
## 0. Coherence impact (COHERENCE §20)
|
||||
|
||||
This is a defect fix, not coherence work, and it should not claim
|
||||
otherwise.
|
||||
|
||||
- **Journey steps touched:** none directly. It protects the steps that
|
||||
depend on a live language server (§2 step 6 onward) from an unbounded
|
||||
teardown, but it adds no journey surface.
|
||||
- **Interaction islands added:** none.
|
||||
- **Config registry:** no new options.
|
||||
- **Background-work attribution:** unchanged. The supervisor's process
|
||||
model is untouched; only the order of two teardown operations moves.
|
||||
- **Protocol:** unchanged.
|
||||
|
||||
The one genuine coherence connection is indirect and worth stating
|
||||
plainly: the hang parks `cargo test --workspace`, which is the ratchet
|
||||
every COHERENCE priority is verified against (§19, §25). A gate that can
|
||||
hang forever degrades every other lane's evidence. That is the argument
|
||||
for doing this now rather than parking it — not a claim that it advances
|
||||
a priority.
|
||||
|
||||
---
|
||||
|
||||
## 1. Ground truth (scouted @ `e003b81`)
|
||||
|
||||
Line numbers are hints; symbols are authoritative.
|
||||
|
||||
### 1.1 The reproduction is deterministic, not intermittent
|
||||
|
||||
`docs/agent-handoff.md` and the test-improvement audit both describe this
|
||||
hang as intermittent. On a machine where `basedpyright-langserver`
|
||||
resolves to a uv-installed shim it is **completely reliable**: 5 runs, 5
|
||||
hangs, via
|
||||
|
||||
```
|
||||
cargo test --test m4_acceptance -- --exact \
|
||||
m4_5_basedpyright_initializes_and_negotiates_encoding
|
||||
```
|
||||
|
||||
§1.7 explains why it looks intermittent across machines. The practical
|
||||
consequence: this defect is directly testable, and any fix has a
|
||||
revert-bite.
|
||||
|
||||
### 1.2 The cycle, in five links
|
||||
|
||||
Observed stack of the wedged test thread (gdb, `sudo` required —
|
||||
`ptrace_scope=1`):
|
||||
|
||||
```
|
||||
tests/m4_acceptance.rs:1374 Rc<RefCell<ProcessSupervisor>> dropped
|
||||
→ ProcessSupervisor::drop src/process.rs:1545
|
||||
→ ProcessSupervisor::shutdown src/process.rs:1463
|
||||
→ ProcessSupervisor::tick src/process.rs:1188
|
||||
→ ProcessSupervisor::poll_one src/process.rs:1268 (drop site: :1337)
|
||||
→ RuntimeHandles::drop src/process.rs:631
|
||||
→ JoinHandle::join ← blocked, indefinitely
|
||||
```
|
||||
|
||||
The links:
|
||||
|
||||
1. **`RuntimeHandles::drop` (`:631`)** sets `cancel`, then joins every
|
||||
handle in `self.readers`.
|
||||
2. **Rust runs a type's `Drop::drop` body before dropping its fields.**
|
||||
`stdin: Option<StdinWriter>` is a *field* (`:505`), so it cannot drop
|
||||
until the body returns. The body never returns.
|
||||
3. **`StdinWriter` (`:556`)** holds the `Sender`; `StdinWriter::spawn`
|
||||
(`:568`) moves the `ChildStdin` sink into its thread, which drops the
|
||||
sink only once `rx.recv()` errors. Sender alive ⇒ sink alive ⇒ **the
|
||||
child's stdin write end never closes.**
|
||||
4. **The child therefore never sees EOF**, stays alive, and keeps the
|
||||
stdout/stderr **write** ends it inherited.
|
||||
5. **The readers are blocked in `read()`** at `:1886` inside
|
||||
`spawn_reader` (`:1874`). `cancel` is consulted only at the loop top
|
||||
(`:1883`) and around `send_timeout` — **never while `read` is
|
||||
blocked.**
|
||||
|
||||
Verified on the live process: the test held fd 4 (child stdin, WRONLY)
|
||||
and fds 5 and 7 (stdout/stderr, RDONLY); the server process held the
|
||||
matching opposite ends on fds 0, 1, 2. Two reader threads sat in
|
||||
`anon_pipe_read`, and the stdin-writer thread sat parked in
|
||||
`Receiver::recv` at `:577` — alive, still owning the sink.
|
||||
|
||||
Confirmation from the other direction: when the wedged test process was
|
||||
killed, its fd 4 closed, the server immediately saw stdin EOF and
|
||||
exited. The cycle's load-bearing link is exactly the one the fix cuts.
|
||||
|
||||
### 1.3 The existing comment names the false premise
|
||||
|
||||
`RuntimeHandles::drop` documents its own reasoning:
|
||||
|
||||
> Wake any reader thread blocked in a bounded `send` — dropping the
|
||||
> master closes the kernel pipe and unblocks `read`, but does nothing
|
||||
> for a reader stuck on a full channel […]
|
||||
|
||||
The premise is true for a **PTY master** and false for **pipe mode**,
|
||||
where `read` unblocks only when *every* write end closes. `cancel` was
|
||||
introduced for the full-channel case and is correct for it; the comment
|
||||
mistakenly treats the `read` case as already handled.
|
||||
|
||||
### 1.4 `shutdown()`'s SIGKILL phase is unreachable on this path
|
||||
|
||||
`shutdown()` (`:1463`) sends SIGTERM to all ids, then runs a bounded
|
||||
grace loop (`deadline` at `:1476`) that calls `tick()`, and *then*
|
||||
escalates to SIGKILL. The stack shows the deadlock occurs **inside that
|
||||
grace loop's `tick()`**, because `poll_one` drops `RuntimeHandles` the
|
||||
moment it observes the recorded pid exited. The SIGKILL phase is never
|
||||
reached.
|
||||
|
||||
So "shutdown force-kills everything first" is not true of this path.
|
||||
(An earlier working assumption of mine said it did; the stack refutes
|
||||
it.) Even if reached, SIGKILL targets the *recorded* pid, which per
|
||||
§1.7 is not the surviving process.
|
||||
|
||||
### 1.5 Only pipe-mode, non-group spawns are affected
|
||||
|
||||
`spawn_pipes` (~`:1712`) chooses per stream:
|
||||
|
||||
| `spec.group` | reader | cancellable mid-`read`? |
|
||||
| --- | --- | --- |
|
||||
| `true` | `spawn_group_reader` (`:1941`) — `O_NONBLOCK` + `poll` | **yes** |
|
||||
| `false` | `spawn_reader` (`:1874`) — blocking `read` | **no** |
|
||||
|
||||
PTY mode (~`:1724`) also uses `spawn_reader`, but there §1.3's premise
|
||||
holds: dropping the master genuinely ends the read. The `spawn_ansi_parser`
|
||||
reader also lives in `readers`, and reads a channel rather than an fd, so
|
||||
it is unaffected.
|
||||
|
||||
Non-group pipe consumers are, per `spawn_reader`'s own doc comment, the
|
||||
**REPL and LSP** paths. This defect is therefore reachable by every LSP
|
||||
server and every REPL — not by terminals.
|
||||
|
||||
### 1.6 The fix mechanism already exists in this file
|
||||
|
||||
`close_stdin` (`:1611`) already does precisely what is needed, and
|
||||
already documents the semantics and the idempotence:
|
||||
|
||||
```rust
|
||||
// Dropping the writer closes the pipe at the kernel
|
||||
// level. `take()` is idempotent — second call sees None.
|
||||
let _ = runtime.stdin.take();
|
||||
```
|
||||
|
||||
The fix is applying an existing, already-reviewed mechanism at the one
|
||||
site that is missing it. It introduces no new concept.
|
||||
|
||||
### 1.7 Why basedpyright wedges and clangd/gopls do not
|
||||
|
||||
`basedpyright-langserver` is a uv-installed **Python console script**:
|
||||
|
||||
```python
|
||||
from basedpyright.langserver import main
|
||||
sys.exit(main())
|
||||
```
|
||||
|
||||
`main()` reaches `run_node.run`, which calls `nodejs_wheel`'s `node(...)`
|
||||
— and that is **`subprocess.run`** (`nodejs_wheel/executable.py:50`). It
|
||||
**waits**. Verified in the installed 1.39.6 source, not assumed.
|
||||
|
||||
So the wrapper does *not* exit at spawn time, and **pmacs creates the
|
||||
orphan itself**:
|
||||
|
||||
1. The wrapper runs `node …/langserver.index.js --stdio` and blocks. Node
|
||||
is a genuine grandchild; the initialize handshake completes normally.
|
||||
2. At teardown, `shutdown()` sends **SIGTERM to the recorded pid** — the
|
||||
Python wrapper — before entering its grace loop.
|
||||
3. The wrapper dies on the default disposition and **does not forward the
|
||||
signal**. Node is reparented to `PPid: 1`, still holding the inherited
|
||||
pipes, idle in `ep_poll`.
|
||||
4. `poll_one` then observes the recorded pid terminated, drops
|
||||
`RuntimeHandles`, and enters the deadlock.
|
||||
|
||||
**rev 1–3 of this doc said the wrapper "spawns node and exits".** That was
|
||||
wrong, and the evidence against it was already in hand: the test's
|
||||
assertions all pass *before* teardown, so the handshake succeeded — which
|
||||
is impossible if the wrapper had exited at spawn. The observation that
|
||||
generated the claim (`PPid: 1`, wrapper gone) was taken **after**
|
||||
`shutdown()` had already killed it.
|
||||
|
||||
This matters for the parked work, not for the fix. The follow-up is not
|
||||
"tolerate servers that self-orphan" — it is **stop orphaning them**:
|
||||
signal the process group rather than a wrapper pid that swallows the
|
||||
signal. P2 in §5 is restated accordingly.
|
||||
|
||||
`clangd` and `gopls` are real binaries: genuine children, reaped
|
||||
normally, write ends closed, blocking `read` returns `Ok(0)` cleanly. The
|
||||
"intermittency" in the handoff is not timing — it is *which server binary
|
||||
is installed how*.
|
||||
|
||||
### 1.8 Limits of the evidence
|
||||
|
||||
- The deterministic reproduction is **one machine, one server**. The
|
||||
causal chain is verified there link by link; its generality to other
|
||||
shim-launched servers is reasoned, not measured.
|
||||
- The gdb capture is a single sample of a state that was stable across a
|
||||
four-minute window and identical across two independent runs. That is
|
||||
strong for a deadlock and would be weak for a race.
|
||||
- Nothing here establishes how often the hang has fired in CI. CI never
|
||||
installs basedpyright (`PMACS_REQUIRE_PYRIGHT` is deliberately never
|
||||
set, #194), so in CI this test skips and the defect is **dark**. Every
|
||||
observation is local.
|
||||
|
||||
---
|
||||
|
||||
## 2. Decisions
|
||||
|
||||
### Q#TD1 — the fix is a reorder inside `Drop`, not a new primitive
|
||||
|
||||
```rust
|
||||
impl Drop for RuntimeHandles {
|
||||
fn drop(&mut self) {
|
||||
self.cancel.store(true, Ordering::Relaxed);
|
||||
// Close the child's stdin BEFORE joining. A stdio child exits
|
||||
// on EOF and closes its stdout/stderr write ends, and that —
|
||||
// not `cancel` — is what unblocks a reader parked in `read`
|
||||
// (`cancel` is only observed between reads and around `send`).
|
||||
// The sink lives in the `stdin` field, which cannot drop until
|
||||
// this body returns, so joining first deadlocks against it.
|
||||
let _ = self.stdin.take();
|
||||
for h in std::mem::take(&mut self.readers) {
|
||||
let _ = h.join();
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Rejected alternative: reordering the struct's *fields*. Field order does
|
||||
not help — the explicit `Drop::drop` body runs before **all** fields
|
||||
regardless of their declaration order. This is the trap that makes the
|
||||
bug non-obvious, and it belongs in the comment.
|
||||
|
||||
### Q#TD2 — the reorder is unconditional across modes
|
||||
|
||||
Applying it only to pipe+non-group would require `RuntimeHandles::drop`
|
||||
to learn which mode it is in, which it currently does not need to know.
|
||||
Closing stdin before teardown is correct in both modes, so the reorder is
|
||||
unconditional.
|
||||
|
||||
This is a uniformity change, and uniformity changes in this repo have
|
||||
made total functions partial before. It is therefore carried as a **bet
|
||||
with a named falsifier** (§3, Bet 2), not as an assumption: PTY-mode
|
||||
`stdin` is the pty *writer*, and dropping it while `pair.master` and the
|
||||
cloned reader still exist must not end the read early.
|
||||
|
||||
### Q#TD3 — the fix assumes the child drains stdin to EOF, and covers nothing outside that
|
||||
|
||||
Stated up front because it bounds the claim: the fix works by making the
|
||||
child exit. A child that never reads stdin — or reads it and ignores EOF
|
||||
— keeps its write ends open and still wedges the join.
|
||||
|
||||
There is a third member of that family, and it is not covered by the
|
||||
wording above because such a child *did* read stdin: **the EOF is only
|
||||
delivered if the writer thread reaches the end of its queue.** Its body
|
||||
is a blocking `sink.write_all(&bytes)` (`:578`), so a child that has
|
||||
stopped draining stdin while queued bytes remain blocks the writer
|
||||
indefinitely — the sink never drops, EOF never arrives, and the join
|
||||
re-wedges. This needs only a full stdin pipe buffer at teardown time, not
|
||||
a misbehaving child. For LSP teardown the queue is near-empty and the
|
||||
practical risk is nil, but the bound belongs in the claim: **the fix
|
||||
assumes the child keeps draining stdin until EOF.** A full stdin pipe
|
||||
with a non-draining child is P1's case as well.
|
||||
|
||||
Covering *that* case requires making the blocking `read` itself
|
||||
cancellable, i.e. moving non-group readers onto `spawn_group_reader`'s
|
||||
`O_NONBLOCK` + `poll` mechanism. `spawn_reader`'s doc comment already
|
||||
names this as a deferral from the compile-mode framing. It stays parked
|
||||
(§5, P1) rather than riding this PR, because it is a behavioural change
|
||||
to every REPL and LSP ingest path and deserves its own review.
|
||||
|
||||
The honest claim for this PR is therefore: **it fixes the observed
|
||||
deadlock for stdio children that honour EOF, which is what LSP servers
|
||||
are, and narrows — not eliminates — the class.**
|
||||
|
||||
### Q#TD4 — queued stdin writes are not lost, and the writer is not joined
|
||||
|
||||
`crossbeam`'s `Receiver::recv` drains buffered items before reporting
|
||||
disconnection, so dropping the `Sender` still lets the writer thread
|
||||
write everything already queued. The writer thread is **not** joined
|
||||
here, so there remains no guarantee the final flush completes before the
|
||||
process is signalled. That is pre-existing, unchanged by this PR, and
|
||||
noted rather than fixed (P3, §5).
|
||||
|
||||
Draining is also the mechanism by which the fix can fail to deliver EOF
|
||||
at all when the child has stopped reading — see Q#TD3's third case.
|
||||
|
||||
### Q#TD5 — the leaked orphan server is not fixed here
|
||||
|
||||
After the fix, the wedge is gone but a shim-launched server is still an
|
||||
orphaned grandchild that teardown's recorded pid cannot signal. It exits
|
||||
here only because it honours stdin EOF — by cooperation, not by
|
||||
enforcement. A server that ignores EOF leaks. Parked (§5, P2).
|
||||
|
||||
### Q#TD6 — the synthetic reproduction must model EOF-honouring, not sleeping, and needs an explicit stdin redirect
|
||||
|
||||
Two distinct traps here, and this lane has now walked into **three**
|
||||
vacuous reproductions, so the reasoning is recorded rather than the
|
||||
conclusion alone.
|
||||
|
||||
**Trap 1 — a sleeping child models the wrong defect.**
|
||||
`sh -c 'sleep 300 & exit 0'` orphans a grandchild that holds the write
|
||||
ends but **never reads stdin**, so closing stdin does not free it. That
|
||||
reproduces a hang this fix does *not* address; it belongs to P1 (§5), not
|
||||
here.
|
||||
|
||||
**Trap 2 — a background job does not inherit stdin.** POSIX XCU §2.9.3:
|
||||
|
||||
> If job control is disabled, the standard input of an asynchronous
|
||||
> list, before any explicit redirections, shall be assigned to
|
||||
> `/dev/null`.
|
||||
|
||||
Job control is off in every non-interactive `sh`, so in
|
||||
`sh -c 'cat & exit 0'` the background `cat` gets **`/dev/null`**, not the
|
||||
inherited pipe. It EOFs immediately and exits **against the unfixed
|
||||
tree** — the test would pass either way and Bet 3's revert-bite would
|
||||
report VACUOUS.
|
||||
|
||||
Measured on this machine (`/bin/sh` → `bash`), stdin attached to a
|
||||
held-open fifo, checking the orphan's `/proc/<pid>/fd/0`:
|
||||
|
||||
| form | grandchild | fd 0 |
|
||||
| --- | --- | --- |
|
||||
| `sh -c 'cat & exit 0'` | **gone** | — (EOF'd from `/dev/null`) |
|
||||
| `sh -c 'cat <&0 & exit 0'` | alive | the real pipe |
|
||||
|
||||
**Trap 3 — `<&0` does not repair it, and the obvious fix is wrong.** rev 2
|
||||
proposed `sh -c 'cat <&0 & exit 0'`, verified on this machine. **CI
|
||||
falsified it.** Re-read the rule: `/dev/null` is assigned *before any
|
||||
explicit redirections*, so by the time `<&0` runs, fd 0 already **is**
|
||||
`/dev/null`, and the redirect faithfully duplicates it onto itself.
|
||||
`bash` happens to skip the default when a stdin redirect is present;
|
||||
`dash` — Ubuntu's `/bin/sh`, and CI's — does not. Measured:
|
||||
|
||||
| shell | form | grandchild | fd 0 |
|
||||
| --- | --- | --- | --- |
|
||||
| bash | `cat & exit 0` | gone | — |
|
||||
| bash | `cat <&0 & exit 0` | alive | real pipe |
|
||||
| dash | `cat <&0 & exit 0` | **gone** | — (CI: control 2 failed) |
|
||||
|
||||
The local probe could not have caught this: `/bin/sh` here is `bash`.
|
||||
|
||||
**The model is therefore `setsid --fork cat`, with no shell at all.**
|
||||
`setsid --fork` forks, the parent exits, and the child inherits
|
||||
stdin/stdout/stderr untouched — no asynchronous list, no `/dev/null`
|
||||
rule, no implementation variance. The recorded pid (`setsid`) terminates
|
||||
promptly so `poll_one` reaches the teardown path, while `cat` survives
|
||||
holding the inherited pipes and exits on EOF exactly as a stdio language
|
||||
server does. Unfixed, this deadlocks; fixed, teardown completes.
|
||||
|
||||
`setsid(1)` is util-linux, which the Linux gate already assumes.
|
||||
Presence is **asserted, not skipped** — a skip would reintroduce the
|
||||
silent-green shape lane 2 removed.
|
||||
|
||||
The controls are what make this recoverable rather than a silent
|
||||
regression: control 2 failed loudly in CI and named its own cause. That
|
||||
is #192's lesson one level down — the bite needs a control, and so does
|
||||
the reproduction.
|
||||
|
||||
---
|
||||
|
||||
## 3. Bets (falsifiable)
|
||||
|
||||
1. **The reorder resolves the observed hang.** Falsified if
|
||||
`m4_5_basedpyright_initializes_and_negotiates_encoding` still fails to
|
||||
terminate after the change.
|
||||
2. **The reorder is safe for PTY mode.** Falsified by any regression in
|
||||
`vterm_stage1/2/3_acceptance`, `terminal_config_acceptance`,
|
||||
`terminal_copy_mode_acceptance`, `m6_4/m6_5_repl_acceptance`,
|
||||
`m6_7_scrollback_acceptance`, `m6_8_multi_repl_acceptance`,
|
||||
`worker_shutdown_acceptance`, or **`bottom_panel_stage1_acceptance`**
|
||||
— added in rev 3: it holds PTY-in-panel tests (`acc28` drives real
|
||||
child input and the `C-c` escape) and its absence from rev 1's list
|
||||
was a real gap, not a judgement call.
|
||||
3. **The synthetic test bites.** Falsified if the new test passes with
|
||||
`let _ = self.stdin.take();` removed. This must be checked by actual
|
||||
revert, per the standing rule that a new pin needs its own bite.
|
||||
4. **The basedpyright test passes rather than merely terminating.** The
|
||||
hang is at teardown (`m4_acceptance.rs:1374`), *after* the body's
|
||||
assertions, so it should now pass outright. Falsified if it terminates
|
||||
with a failure — which would mean a second, independent defect.
|
||||
**If falsified, stop the lane and frame that defect separately.** Do
|
||||
not paper over it: "terminates" was never the goal, and a failing
|
||||
assertion here is new information, not a loose end.
|
||||
|
||||
---
|
||||
|
||||
## 4. Acceptance
|
||||
|
||||
1. `RuntimeHandles::drop` takes `stdin` before joining readers, with a
|
||||
comment naming the drop-body-before-fields trap.
|
||||
2. New unit test in `src/process.rs` (so it runs under the standard
|
||||
`cargo test --lib` gate, not only an acceptance suite):
|
||||
`teardown_closes_stdin_before_joining_readers`.
|
||||
- Spawns `setsid --fork cat` as a **non-group pipe** process. The
|
||||
choice of `setsid` over a shell background job is load-bearing
|
||||
(Q#TD6) and gets a comment saying so. `setsid` presence is
|
||||
**asserted, not skipped.**
|
||||
- **Two positive controls, before teardown starts:** (1) the recorded
|
||||
child has actually exited — while it lives it holds the output pipe
|
||||
itself, so control 2 would pass for the wrong reason; (2) both
|
||||
readers are still blocked in `read`, which is only true while
|
||||
something still holds the write ends. Without these the test
|
||||
silently degrades into modelling the wrong thing and reports green
|
||||
while doing it — which is exactly what happened on `dash`, and
|
||||
control 2 is what caught it.
|
||||
- `#[cfg(target_os = "linux")]`: the controls read `/proc`, and
|
||||
`setsid(1)` is util-linux (absent on macOS). Gate it explicitly and
|
||||
say why, rather than letting it be incidentally Linux-only. (Same
|
||||
reasoning as the APFS gate — `cfg(unix)` would be wrong here.)
|
||||
- Performs the full reap-and-drop sequence on a helper thread and
|
||||
asserts completion via `recv_timeout`, so a regression **fails**
|
||||
within a bounded window instead of hanging. A test that hangs on
|
||||
regression would reproduce the exact hazard this PR removes.
|
||||
- Bound: 10s (default `grace_period` is 2s, `:927`).
|
||||
- On the failure path the helper thread stays wedged and the `cat`
|
||||
survives until the harness's fds close at process exit. That is
|
||||
bounded and acceptable — but the test comment must **say so**, or a
|
||||
future reviewer correctly flags a leaked thread as a defect.
|
||||
3. The bite is demonstrated by revert, and the result recorded in the PR
|
||||
body — pass/fail both ways, per Bet 3.
|
||||
4. `cargo test --test m4_acceptance` runs **without**
|
||||
`-- --skip basedpyright` and completes, locally, on the machine where
|
||||
it currently hangs 5/5.
|
||||
5. Docs, in **both** places the superseded cause lives — replacing it,
|
||||
not appending to it:
|
||||
- `docs/agent-handoff.md` §5 gains the drop-body-before-fields lesson
|
||||
and the corrected cause, replacing "no timeout on the initialize
|
||||
handshake".
|
||||
- `docs/agent-handoff.md` §3's machine caveat currently says the
|
||||
desktop's **local binary is broken and hangs**. §1.7 shows the
|
||||
binary was never broken: the shim architecture plus this defect
|
||||
was. Left alone, §3 keeps steering readers toward a false model —
|
||||
and toward keeping the skip forever.
|
||||
|
||||
**Deliberately not a criterion:** removing `-- --skip basedpyright` from
|
||||
`CLAUDE.md`'s standing gate list. It is a separate call that is the
|
||||
user's to make, and it changes only *local* behaviour — CI skips the test
|
||||
regardless (§1.8). I will propose it with evidence after the fix has been
|
||||
green repeatedly, rather than fold a process change into a defect fix.
|
||||
|
||||
When that proposal comes it owes two things beyond the green runs: the
|
||||
`docs/agent-handoff.md` §3 caveat updated (criterion 5 covers it here,
|
||||
but the *skip* rationale lives with it), and an explicit note that
|
||||
`PMACS_REQUIRE_PYRIGHT` stays **unarmed** in CI until the per-test
|
||||
timeout lane (3a) merges — the ordering #194 established, where presence
|
||||
of the variable decides execution and arming without a timeout would give
|
||||
CI the same unbounded hang this PR removes locally.
|
||||
|
||||
---
|
||||
|
||||
## 5. Parked (each needs its own evidence)
|
||||
|
||||
- **P1 — cancellable non-group `read`.** Move `spawn_reader` onto
|
||||
`spawn_group_reader`'s `O_NONBLOCK` + `poll` mechanism so `cancel` is
|
||||
observed within `READER_SEND_POLL_INTERVAL` (`:421`, 50ms) even
|
||||
mid-`read`. Bounds teardown unconditionally, including for children
|
||||
that ignore EOF (Q#TD3) — **and** the blocked-writer case, where EOF is
|
||||
never delivered because `write_all` is stuck on a full pipe. Already
|
||||
named as a deferral by `spawn_reader`'s own doc comment. Tests: the
|
||||
`sleep 300` shape from Q#TD6 (child never reads stdin), plus a
|
||||
fill-the-pipe-then-stop-reading shape for the writer case.
|
||||
- **P2 — stop orphaning wrapper-launched servers (Q#TD5).** Restated in
|
||||
rev 4, because the corrected §1.7 changes the target: the orphan is not
|
||||
self-inflicted by the server, it is created by **us** SIGTERMing a
|
||||
wrapper that does not forward the signal. Spawn stdio servers in their
|
||||
own process group and signal the group, reusing the machinery the group
|
||||
path and `reap_ledger` already have. Fixes a real leak: every
|
||||
basedpyright-backed session currently leaves a `node` process behind.
|
||||
Note the ordering consequence — a group-directed SIGTERM would reach
|
||||
node directly, so this also removes the condition the present fix works
|
||||
around, rather than merely tolerating it.
|
||||
- **P3 — join the stdin writer thread** so the final flush is ordered
|
||||
against child termination (Q#TD4).
|
||||
- **P4 — re-audit the "intermittent" label** in `docs/agent-handoff.md`
|
||||
and the audit now that §1.7 explains it. Rides this PR's doc update
|
||||
only insofar as criterion 5 requires; a broader sweep is separate.
|
||||
|
||||
---
|
||||
|
||||
## 6. Gates
|
||||
|
||||
Per `CLAUDE.md`, each as its own step with a real exit status checked
|
||||
(never `cmd | tail` — a pipe returns the tail's status and has masked a
|
||||
real failure here before):
|
||||
|
||||
- `cargo fmt --check`
|
||||
- `cargo clippy --workspace --all-targets -- -D warnings`
|
||||
- `cargo test --lib`
|
||||
- `cargo test --lib --features crdt`
|
||||
- `cargo test --test m4_acceptance` — **without** the basedpyright skip
|
||||
- The PTY/REPL suites named in Bet 2
|
||||
- `cargo test --test worker_shutdown_acceptance`
|
||||
- `PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`
|
||||
- `git diff --check`
|
||||
|
||||
Commit before gating, so the results describe the pushed tree.
|
||||
|
||||
---
|
||||
|
||||
## 7. Branch plan
|
||||
|
||||
One branch, one PR: `process-teardown-stdin-deadlock`, from `main` @
|
||||
`e003b81` or later. Worktree `pmacs-hang` (already clean at that SHA).
|
||||
|
||||
Small diff — the reorder, one unit test, one comment, the handoff
|
||||
update. P1–P4 do not ride it.
|
||||
|
||||
`docs/active-work.md` is integrated **late**, immediately before pushing,
|
||||
to avoid the ledger-contention treadmill with the other open lanes.
|
||||
|
|
@ -391,6 +391,70 @@ struct PendingJob {
|
|||
/// When the job was registered. Used to compute "age" in the
|
||||
/// `*workers*` buffer.
|
||||
dispatched_at: Instant,
|
||||
/// The filesystem mutation this job performs, retained so the
|
||||
/// main-thread drain can reconcile the editor's path owners once
|
||||
/// the syscall lands (dired Stage 2a, §5).
|
||||
///
|
||||
/// The paths have to live here because the dispatchers **move**
|
||||
/// them into the worker closure and nothing else retains them, and
|
||||
/// because the reply is undifferentiated — rename and remove both
|
||||
/// settle as `ReplyKind::FsUnit`, so a drain cannot key on the
|
||||
/// reply and must key on the pending job.
|
||||
///
|
||||
/// One enum field rather than a pair of `Option`s: two would admit
|
||||
/// a both-`Some` state that cannot occur, which every consumer
|
||||
/// would then have to rule out by hand. `COHERENCE.md` §9 is why
|
||||
/// this is a field on the job and not a side map — the parse
|
||||
/// job→buffer link already lives in a side map and §9 names that as
|
||||
/// the defect.
|
||||
resource: Option<ResourceOp>,
|
||||
}
|
||||
|
||||
/// A settled filesystem mutation, with the paths the worker consumed
|
||||
/// (dired Stage 2a, §5).
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub enum ResourceOp {
|
||||
/// A successful `rename(from, to)`.
|
||||
Rename {
|
||||
/// Source path, as the caller spelled it.
|
||||
from: PathBuf,
|
||||
/// Destination path, as the caller spelled it.
|
||||
to: PathBuf,
|
||||
},
|
||||
/// A successful `remove(path)`.
|
||||
Remove {
|
||||
/// The path that was removed.
|
||||
path: PathBuf,
|
||||
},
|
||||
}
|
||||
|
||||
/// What one [`AsyncRuntime::tick`] observed.
|
||||
///
|
||||
/// Settle identity and resource metadata come out of **one**
|
||||
/// transaction — the post-drain loop already borrows `pending` to
|
||||
/// record completions — so a consumer cannot see a settle without its
|
||||
/// resource, or the reverse.
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct TickOutcome {
|
||||
/// Ids that transitioned from `Running` to a terminal state during
|
||||
/// this tick. The Lua runtime resumes coroutines parked on these.
|
||||
pub settled: Vec<JobId>,
|
||||
/// Successful resource mutations, **in bus-arrival order. This is
|
||||
/// not filesystem execution order.**
|
||||
///
|
||||
/// [`AsyncRuntime::tick`] drains the reply bus with `try_recv` and
|
||||
/// the runtime establishes no execution token, so a worker can
|
||||
/// complete, be descheduled before sending, and have a later
|
||||
/// mutation's reply arrive first. A consumer that reads "in settle
|
||||
/// order" and infers causality is wrong; reconciliation is
|
||||
/// deliberately order-independent (Q#DR29), and the primitive's
|
||||
/// contract is that a caller with overlapping source/target paths
|
||||
/// serializes by awaiting each op before dispatching the next.
|
||||
///
|
||||
/// Carries **only** jobs that settled
|
||||
/// [`PendingState::Complete`] — a failed or cancelled mutation
|
||||
/// reconciles nothing and fires no hook.
|
||||
pub resources: Vec<ResourceOp>,
|
||||
}
|
||||
|
||||
/// Snapshot of a job's terminal state, returned by
|
||||
|
|
@ -684,6 +748,18 @@ impl AsyncRuntime {
|
|||
kind: JobKind,
|
||||
supersede_key: Option<&str>,
|
||||
stream: Option<usize>,
|
||||
) -> (JobId, CancellationToken) {
|
||||
self.allocate_with_resource(kind, supersede_key, stream, None)
|
||||
}
|
||||
|
||||
/// [`Self::allocate`], plus the filesystem mutation this job
|
||||
/// performs. Only the two mutating fs dispatchers pass `resource`.
|
||||
fn allocate_with_resource(
|
||||
&self,
|
||||
kind: JobKind,
|
||||
supersede_key: Option<&str>,
|
||||
stream: Option<usize>,
|
||||
resource: Option<ResourceOp>,
|
||||
) -> (JobId, CancellationToken) {
|
||||
let id = self.next_job_id.fetch_add(1, Ordering::Relaxed);
|
||||
let cancel = CancellationToken::new();
|
||||
|
|
@ -711,6 +787,7 @@ impl AsyncRuntime {
|
|||
max_batch: stream.unwrap_or(0),
|
||||
kind,
|
||||
dispatched_at: Instant::now(),
|
||||
resource,
|
||||
},
|
||||
);
|
||||
(id, cancel)
|
||||
|
|
@ -869,7 +946,17 @@ impl AsyncRuntime {
|
|||
/// Dispatch a `rename(from, to)` job. Settles to
|
||||
/// [`JobResult::Unit`] on success. T M8.1.
|
||||
pub fn dispatch_fs_rename(&self, from: PathBuf, to: PathBuf, supersede: Option<&str>) -> JobId {
|
||||
let (id, cancel) = self.allocate(JobKind::FsRename, supersede, None);
|
||||
// The closure below MOVES both paths; the pending entry is the
|
||||
// only thing that still knows them when the reply lands.
|
||||
let (id, cancel) = self.allocate_with_resource(
|
||||
JobKind::FsRename,
|
||||
supersede,
|
||||
None,
|
||||
Some(ResourceOp::Rename {
|
||||
from: from.clone(),
|
||||
to: to.clone(),
|
||||
}),
|
||||
);
|
||||
let bus = self.workers.clone();
|
||||
self.pool.dispatch(move |_pool| {
|
||||
let kind = run_fs_rename(&cancel, &from, &to);
|
||||
|
|
@ -891,7 +978,12 @@ impl AsyncRuntime {
|
|||
|
||||
/// Dispatch a `remove(path)` job. T M8.1.
|
||||
pub fn dispatch_fs_remove(&self, path: PathBuf, supersede: Option<&str>) -> JobId {
|
||||
let (id, cancel) = self.allocate(JobKind::FsRemove, supersede, None);
|
||||
let (id, cancel) = self.allocate_with_resource(
|
||||
JobKind::FsRemove,
|
||||
supersede,
|
||||
None,
|
||||
Some(ResourceOp::Remove { path: path.clone() }),
|
||||
);
|
||||
let bus = self.workers.clone();
|
||||
self.pool.dispatch(move |_pool| {
|
||||
let kind = run_fs_remove(&cancel, &path);
|
||||
|
|
@ -996,11 +1088,16 @@ impl AsyncRuntime {
|
|||
}
|
||||
}
|
||||
|
||||
/// Drain every queued reply on the main-thread bus, update
|
||||
/// pending entries, and return the list of ids that *transitioned
|
||||
/// from Running to a terminal state* during this tick. The Lua
|
||||
/// runtime resumes coroutines parked on these ids.
|
||||
pub fn tick(&self) -> Vec<JobId> {
|
||||
/// Drain every queued reply on the main-thread bus, update pending
|
||||
/// entries, and report what settled.
|
||||
///
|
||||
/// [`TickOutcome::settled`] is the ids that transitioned from
|
||||
/// `Running` to a terminal state during this tick — the Lua runtime
|
||||
/// resumes coroutines parked on these.
|
||||
/// [`TickOutcome::resources`] is the filesystem mutations among them
|
||||
/// that **succeeded**, in **bus-arrival order** (see the field's
|
||||
/// own documentation: that is not execution order).
|
||||
pub fn tick(&self) -> TickOutcome {
|
||||
let mut newly_settled = Vec::new();
|
||||
while let Ok(env) = self.main.try_recv() {
|
||||
let Ok(reply): Result<WorkerReply, _> = self.main.decode(&env) else {
|
||||
|
|
@ -1071,6 +1168,7 @@ impl AsyncRuntime {
|
|||
// a successor that came in mid-flight will have overwritten
|
||||
// the entry already, and that successor's pending lifetime
|
||||
// is what owns the slot now.
|
||||
let mut resources = Vec::new();
|
||||
if !newly_settled.is_empty() {
|
||||
let pending = self.pending.borrow();
|
||||
let mut sup = self.supersede.borrow_mut();
|
||||
|
|
@ -1078,6 +1176,16 @@ impl AsyncRuntime {
|
|||
let now = Instant::now();
|
||||
for id in &newly_settled {
|
||||
if let Some(job) = pending.get(id) {
|
||||
// The harvest (§5): one more read in a loop that
|
||||
// already borrows `pending` and reads `job.kind`,
|
||||
// so settle identity and resource metadata come out
|
||||
// of one transaction. Gated on `Complete` — a
|
||||
// failed or cancelled mutation reconciles nothing.
|
||||
if let Some(resource) = &job.resource
|
||||
&& matches!(job.state, PendingState::Complete(_))
|
||||
{
|
||||
resources.push(resource.clone());
|
||||
}
|
||||
if let Some(key) = &job.supersede_key
|
||||
&& sup.get(key) == Some(id)
|
||||
{
|
||||
|
|
@ -1106,7 +1214,10 @@ impl AsyncRuntime {
|
|||
completed.pop_back();
|
||||
}
|
||||
}
|
||||
newly_settled
|
||||
TickOutcome {
|
||||
settled: newly_settled,
|
||||
resources,
|
||||
}
|
||||
}
|
||||
|
||||
/// Snapshot the runtime's job tables for the `*workers*`
|
||||
|
|
@ -1749,6 +1860,126 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
/// dired Stage 2a, acceptance 54 (controlled-bus layer). Allocate
|
||||
/// two resource jobs **without dispatching workers**, inject their
|
||||
/// successful replies in a chosen order, and assert
|
||||
/// `TickOutcome.resources` reports exactly that order.
|
||||
///
|
||||
/// This is the honest statement of what the runtime guarantees:
|
||||
/// `tick` drains the reply bus with `try_recv` and establishes no
|
||||
/// execution token, so what a consumer sees is bus-arrival order.
|
||||
/// The test fails against sorting by job id or kind, and against any
|
||||
/// claim that the order recovers dispatch or filesystem-execution
|
||||
/// order — because the injection order here is *deliberately* the
|
||||
/// reverse of the allocation order in the first case.
|
||||
#[test]
|
||||
fn tick_reports_resources_in_bus_arrival_order_not_allocation_order() {
|
||||
fn run(reverse: bool) -> Vec<ResourceOp> {
|
||||
let rt = AsyncRuntime::with_pool_size(1);
|
||||
let (a, _) = rt.allocate_with_resource(
|
||||
JobKind::FsRename,
|
||||
None,
|
||||
None,
|
||||
Some(ResourceOp::Rename {
|
||||
from: PathBuf::from("/tmp/a-from"),
|
||||
to: PathBuf::from("/tmp/a-to"),
|
||||
}),
|
||||
);
|
||||
let (b, _) = rt.allocate_with_resource(
|
||||
JobKind::FsRemove,
|
||||
None,
|
||||
None,
|
||||
Some(ResourceOp::Remove {
|
||||
path: PathBuf::from("/tmp/b-gone"),
|
||||
}),
|
||||
);
|
||||
let order = if reverse { [b, a] } else { [a, b] };
|
||||
for id in order {
|
||||
rt.workers
|
||||
.send(
|
||||
ASYNC_REPLY_TOPIC,
|
||||
&WorkerReply {
|
||||
job_id: id,
|
||||
kind: ReplyKind::FsUnit,
|
||||
},
|
||||
)
|
||||
.expect("inject reply");
|
||||
}
|
||||
let outcome = rt.tick();
|
||||
assert_eq!(outcome.settled.len(), 2, "both jobs settled");
|
||||
outcome.resources
|
||||
}
|
||||
|
||||
let a_first = ResourceOp::Rename {
|
||||
from: PathBuf::from("/tmp/a-from"),
|
||||
to: PathBuf::from("/tmp/a-to"),
|
||||
};
|
||||
let b_first = ResourceOp::Remove {
|
||||
path: PathBuf::from("/tmp/b-gone"),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
run(true),
|
||||
vec![b_first.clone(), a_first.clone()],
|
||||
"B injected first must be reported first, even though A was \
|
||||
allocated first"
|
||||
);
|
||||
assert_eq!(
|
||||
run(false),
|
||||
vec![a_first, b_first],
|
||||
"and the reverse arrival order reverses the report"
|
||||
);
|
||||
}
|
||||
|
||||
/// A failed or cancelled mutation reconciles nothing, so it must not
|
||||
/// appear in `resources` at all (acceptance 37's runtime half).
|
||||
#[test]
|
||||
fn a_failed_or_cancelled_resource_job_is_not_harvested() {
|
||||
let rt = AsyncRuntime::with_pool_size(1);
|
||||
let (failed, _) = rt.allocate_with_resource(
|
||||
JobKind::FsRename,
|
||||
None,
|
||||
None,
|
||||
Some(ResourceOp::Rename {
|
||||
from: PathBuf::from("/tmp/nope"),
|
||||
to: PathBuf::from("/tmp/also-nope"),
|
||||
}),
|
||||
);
|
||||
let (cancelled, _) = rt.allocate_with_resource(
|
||||
JobKind::FsRemove,
|
||||
None,
|
||||
None,
|
||||
Some(ResourceOp::Remove {
|
||||
path: PathBuf::from("/tmp/never"),
|
||||
}),
|
||||
);
|
||||
rt.workers
|
||||
.send(
|
||||
ASYNC_REPLY_TOPIC,
|
||||
&WorkerReply {
|
||||
job_id: failed,
|
||||
kind: ReplyKind::Error("ENOENT".to_owned()),
|
||||
},
|
||||
)
|
||||
.expect("inject");
|
||||
rt.workers
|
||||
.send(
|
||||
ASYNC_REPLY_TOPIC,
|
||||
&WorkerReply {
|
||||
job_id: cancelled,
|
||||
kind: ReplyKind::Cancelled,
|
||||
},
|
||||
)
|
||||
.expect("inject");
|
||||
let outcome = rt.tick();
|
||||
assert_eq!(outcome.settled.len(), 2, "both settled");
|
||||
assert!(
|
||||
outcome.resources.is_empty(),
|
||||
"only Complete mutations are harvested; got {:?}",
|
||||
outcome.resources
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dispatch_sum_completes_with_correct_value() {
|
||||
let rt = AsyncRuntime::with_pool_size(2);
|
||||
|
|
|
|||
|
|
@ -148,6 +148,26 @@ struct EditDescription {
|
|||
inserted_len: u64,
|
||||
}
|
||||
|
||||
/// Provenance of a [`Buffer`]'s name (dired Stage 2a, Q#DR30).
|
||||
///
|
||||
/// A rename must move a name that merely *renders* the file's path and
|
||||
/// must leave a name the user chose alone. String inspection cannot
|
||||
/// tell those apart — a user may legitimately name a buffer with a
|
||||
/// string that normalizes to its own path — so the fact is recorded at
|
||||
/// the moment the name is written instead of being reconstructed
|
||||
/// later.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum BufferNameOrigin {
|
||||
/// A caller named this buffer: `Buffer::new`/`from_bytes`, an
|
||||
/// ordinary [`Buffer::set_name`], or the `pmacs.buffer.set_name`
|
||||
/// binding. A rename leaves the name alone.
|
||||
Explicit,
|
||||
/// The name was derived from the buffer's backing path by a
|
||||
/// path-backed creation site, through
|
||||
/// [`Buffer::set_path_derived_name`]. A rename rewrites it.
|
||||
PathDerived,
|
||||
}
|
||||
|
||||
/// The unit of editable content: rope + identity + views + undo.
|
||||
///
|
||||
/// # Threading
|
||||
|
|
@ -159,6 +179,14 @@ pub struct Buffer {
|
|||
id: BufferId,
|
||||
rope: Rope,
|
||||
name: String,
|
||||
/// Where [`Self::name`] came from. Recorded rather than inferred,
|
||||
/// because a path-backed buffer's name is **not** reliably its
|
||||
/// path: `get_or_load_buffer` takes the name from the path *as
|
||||
/// given* and normalizes only the stored `file_path`, so a
|
||||
/// relative open is named `foo.rs` while its path is absolute.
|
||||
/// Rename reconciliation asks this bit, never the string
|
||||
/// (dired Stage 2a, Q#DR30).
|
||||
name_origin: BufferNameOrigin,
|
||||
/// The buffer's single active major mode, if one has been selected.
|
||||
major_mode: Option<String>,
|
||||
is_modified: bool,
|
||||
|
|
@ -247,6 +275,10 @@ impl Buffer {
|
|||
id,
|
||||
rope,
|
||||
name: name.into(),
|
||||
// Construction names a buffer explicitly. A path-backed
|
||||
// creation site re-records provenance through
|
||||
// `set_path_derived_name` right after binding the path.
|
||||
name_origin: BufferNameOrigin::Explicit,
|
||||
major_mode: None,
|
||||
is_modified: false,
|
||||
read_only: false,
|
||||
|
|
@ -449,9 +481,35 @@ impl Buffer {
|
|||
&self.name
|
||||
}
|
||||
|
||||
/// Set the buffer's name. Used by save-as and rename operations.
|
||||
/// Set the buffer's name, recording it as **explicitly chosen**
|
||||
/// ([`BufferNameOrigin::Explicit`]).
|
||||
///
|
||||
/// This is the user-facing door — `pmacs.buffer.set_name` and
|
||||
/// save-as go through it — and it is deliberately explicit even
|
||||
/// when the string happens to denote the file: naming a buffer
|
||||
/// `notes` for `${cwd}/notes` is still a naming operation, and a
|
||||
/// later rename must not overwrite it. Path-backed creation sites
|
||||
/// use [`Self::set_path_derived_name`] instead.
|
||||
pub fn set_name(&mut self, name: impl Into<String>) {
|
||||
self.name = name.into();
|
||||
self.name_origin = BufferNameOrigin::Explicit;
|
||||
}
|
||||
|
||||
/// Set the buffer's name **and** record that it was derived from
|
||||
/// the buffer's backing path ([`BufferNameOrigin::PathDerived`]).
|
||||
///
|
||||
/// Every site that creates or re-binds a path-backed buffer uses
|
||||
/// this door, including rename reconciliation itself — so a second
|
||||
/// rename still follows the path.
|
||||
pub fn set_path_derived_name(&mut self, name: impl Into<String>) {
|
||||
self.name = name.into();
|
||||
self.name_origin = BufferNameOrigin::PathDerived;
|
||||
}
|
||||
|
||||
/// Where this buffer's name came from (dired Stage 2a, Q#DR30).
|
||||
#[must_use]
|
||||
pub fn name_origin(&self) -> BufferNameOrigin {
|
||||
self.name_origin
|
||||
}
|
||||
|
||||
/// This buffer's active major mode, if any.
|
||||
|
|
|
|||
70
src/diag.rs
70
src/diag.rs
|
|
@ -266,6 +266,22 @@ impl DiagnosticStore {
|
|||
*self.epochs.entry(uri.to_owned()).or_insert(0) += 1;
|
||||
}
|
||||
|
||||
/// Drop **every** trace of `uri`, epoch included (dired Stage 2a,
|
||||
/// §5 finding 4).
|
||||
///
|
||||
/// Distinct from [`Self::clear`] on purpose: `clear` *creates* an
|
||||
/// `epochs` entry (`or_insert(0) += 1`) because a consumer caching
|
||||
/// against the epoch must observe that the diagnostics went away.
|
||||
/// Forgetting is the opposite intent — the editor no longer holds
|
||||
/// this URI at all — so leaving the counter behind would be a
|
||||
/// URI-keyed leak in the one map nothing else prunes.
|
||||
pub fn forget(&mut self, uri: &str) {
|
||||
self.by_uri.remove(uri);
|
||||
self.severity_counts.remove(uri);
|
||||
self.stale_uris.remove(uri);
|
||||
self.epochs.remove(uri);
|
||||
}
|
||||
|
||||
/// Monotonic per-URI change counter: how many times `set` /
|
||||
/// `clear` ran for this URI. `0` for a URI never written.
|
||||
/// Consumers cache against this to detect republishes that no
|
||||
|
|
@ -489,6 +505,17 @@ impl DiagnosticView {
|
|||
}
|
||||
|
||||
impl View for DiagnosticView {
|
||||
/// Re-root this view when the buffer's file was renamed (dired
|
||||
/// Stage 2a, §5). The URI field is private and `View` has no
|
||||
/// downcast, so this hook is the only way an outside sweep can
|
||||
/// reach it — and mutating in place preserves this overlay's
|
||||
/// position in the window's composition order.
|
||||
fn rename_resource(&mut self, old_uri: &str, new_uri: &str) {
|
||||
if self.uri == old_uri {
|
||||
new_uri.clone_into(&mut self.uri);
|
||||
}
|
||||
}
|
||||
|
||||
fn kind(&self) -> &'static str {
|
||||
"diagnostic"
|
||||
}
|
||||
|
|
@ -745,6 +772,49 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
/// dired Stage 2a §5, finding 4. `clear` *creates* an `epochs`
|
||||
/// entry, because a consumer caching against the epoch has to see
|
||||
/// that the diagnostics went away; nothing ever removes one. So a
|
||||
/// `forget_uri` that called `clear` would leave a URI-keyed leak
|
||||
/// behind in the one map nothing prunes — which is why the forget
|
||||
/// path is its own store method.
|
||||
#[test]
|
||||
fn forget_drops_the_epoch_while_clear_deliberately_bumps_it() {
|
||||
let mut store = DiagnosticStore::new();
|
||||
store.set(
|
||||
"file:///a.rs",
|
||||
vec![diag(0, DiagnosticSeverity::Error, "boom")],
|
||||
);
|
||||
store.mark_stale("file:///a.rs");
|
||||
assert_eq!(store.epoch_for("file:///a.rs"), 1);
|
||||
|
||||
store.clear("file:///a.rs");
|
||||
assert_eq!(
|
||||
store.epoch_for("file:///a.rs"),
|
||||
2,
|
||||
"clear announces the removal to epoch-keyed caches"
|
||||
);
|
||||
|
||||
store.set(
|
||||
"file:///a.rs",
|
||||
vec![diag(0, DiagnosticSeverity::Error, "boom")],
|
||||
);
|
||||
store.mark_stale("file:///a.rs");
|
||||
store.forget("file:///a.rs");
|
||||
assert!(store.for_uri("file:///a.rs").is_empty(), "diagnostics");
|
||||
assert!(!store.is_stale("file:///a.rs"), "stale flag");
|
||||
assert_eq!(
|
||||
store.severity_counts_for("file:///a.rs"),
|
||||
(0, 0, 0, 0),
|
||||
"severity counts"
|
||||
);
|
||||
assert_eq!(
|
||||
store.epoch_for("file:///a.rs"),
|
||||
0,
|
||||
"forget leaves no trace at all, epoch included"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_lsp_value_parses_minimal_diagnostic() {
|
||||
let v = json!({
|
||||
|
|
|
|||
|
|
@ -938,11 +938,18 @@ impl EditorCore {
|
|||
}
|
||||
let normalized = normalize_buffer_path(path.to_path_buf());
|
||||
let (bytes, meta) = crate::file_io::load_file(path)?;
|
||||
// The name is the path **as given** — a relative open is named
|
||||
// `foo.rs` while `file_path` below is absolute. Recording the
|
||||
// provenance (Q#DR30) is what lets rename reconciliation move
|
||||
// this name without having to guess from the string.
|
||||
let display_name = path.display().to_string();
|
||||
let id = self
|
||||
.registry
|
||||
.borrow_mut()
|
||||
.create_from_bytes(display_name, &bytes);
|
||||
.create_from_bytes(display_name.clone(), &bytes);
|
||||
if let Ok(b) = self.registry.borrow_mut().get_mut(id) {
|
||||
b.set_path_derived_name(display_name);
|
||||
}
|
||||
self.set_buffer_path(id, Some(normalized));
|
||||
self.set_buffer_meta(id, Some(meta));
|
||||
Ok((id, true))
|
||||
|
|
@ -1001,7 +1008,12 @@ impl EditorCore {
|
|||
}),
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
|
||||
let display_path = path.display().to_string();
|
||||
let buffer_id = self.registry.borrow_mut().create(display_path);
|
||||
let buffer_id = self.registry.borrow_mut().create(display_path.clone());
|
||||
// Path-backed creation site (Q#DR30): the name is the
|
||||
// path, so a later rename may move it.
|
||||
if let Ok(b) = self.registry.borrow_mut().get_mut(buffer_id) {
|
||||
b.set_path_derived_name(display_path);
|
||||
}
|
||||
self.set_buffer_path(buffer_id, Some(path.to_path_buf()));
|
||||
"[new file]".clone_into(&mut self.status);
|
||||
Ok(ResolvedTarget::Buffer {
|
||||
|
|
@ -1833,22 +1845,66 @@ impl EditorCore {
|
|||
/// and translate the live origin — otherwise accepted-match
|
||||
/// highlights and the session origin survive at pre-edit offsets
|
||||
/// for every Lua mutator edit and applied CRDT op.
|
||||
///
|
||||
/// Q#GB6: each window coordinate is also clamped against **its own**
|
||||
/// post-edit bound, which [`Self::rebuild_views_for`] already does
|
||||
/// (`:1853-1857`) and this path did not. A generated refresh that
|
||||
/// shrinks its buffer otherwise leaves `cursor` past the end of the
|
||||
/// rope indefinitely — neither paint nor a motion command recovers
|
||||
/// it, because motion is computed from the stale value. The two
|
||||
/// coordinates fail on different axes and are therefore clamped
|
||||
/// separately and **unconditionally**: `cursor` is a byte position
|
||||
/// bounded by [`Buffer::len`], while `view_top` is a line index
|
||||
/// bounded by [`TextView::line_count`]. A replace can grow in bytes
|
||||
/// while collapsing many lines into one, so "the buffer shrank" is
|
||||
/// not a usable trigger for the second.
|
||||
///
|
||||
/// The selection anchor is a **third** coordinate. It is clamped to
|
||||
/// the buffer extent, and the selection is cleared only when moving
|
||||
/// an endpoint collapses it — see
|
||||
/// [`Self::clamp_cursor_and_selection`].
|
||||
pub fn notify_buffer_edit(&mut self, buffer_id: BufferId, edit: &Edit) {
|
||||
self.search_invalidate_for_edit(buffer_id, edit);
|
||||
let reg = self.registry.borrow();
|
||||
let Ok(buffer) = reg.get(buffer_id) else {
|
||||
return;
|
||||
};
|
||||
let len = buffer.len();
|
||||
for win in self.windows.values_mut() {
|
||||
if win.buffer_id == buffer_id {
|
||||
let _ = win.text_view.on_edit(buffer, edit);
|
||||
for overlay in &mut win.overlays {
|
||||
let _ = overlay.on_edit(buffer, edit);
|
||||
}
|
||||
Self::clamp_cursor_and_selection(win, len);
|
||||
let max_top = win.text_view.line_count().saturating_sub(1);
|
||||
if win.view_top > max_top {
|
||||
win.view_top = max_top;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Clamp `win`'s cursor and selection anchor to `len`, clearing the
|
||||
/// selection only when the clamp collapses it.
|
||||
///
|
||||
/// This mirrors terminal selection normalization: a surviving,
|
||||
/// shortened region remains selected, while a region whose content
|
||||
/// disappeared does not become an accidental active-but-empty
|
||||
/// selection. Looking at whether either endpoint moved distinguishes
|
||||
/// that case from a zero-width selection the user created.
|
||||
fn clamp_cursor_and_selection(win: &mut Window, len: Position) {
|
||||
let old_cursor = win.cursor;
|
||||
win.cursor = win.cursor.min(len);
|
||||
win.selection = win.selection.and_then(|mut selection| {
|
||||
let old_anchor = selection.anchor;
|
||||
selection.anchor = selection.anchor.min(len);
|
||||
let collapsed_by_clamp = selection.anchor == win.cursor
|
||||
&& (selection.anchor != old_anchor || win.cursor != old_cursor);
|
||||
(!collapsed_by_clamp).then_some(selection)
|
||||
});
|
||||
}
|
||||
|
||||
/// Force every window currently showing `buffer_id` to rebuild
|
||||
/// its [`TextView`] from scratch.
|
||||
///
|
||||
|
|
@ -1862,6 +1918,8 @@ impl EditorCore {
|
|||
///
|
||||
/// Cursor and `view_top` are clamped to the new buffer extent so
|
||||
/// they don't dangle past the end after a shrinking rewrite.
|
||||
/// Selection normalization uses the same clamp-or-clear rule as
|
||||
/// [`Self::notify_buffer_edit`].
|
||||
pub fn rebuild_views_for(&mut self, buffer_id: BufferId) {
|
||||
let reg = self.registry.borrow();
|
||||
let Ok(buffer) = reg.get(buffer_id) else {
|
||||
|
|
@ -1871,9 +1929,7 @@ impl EditorCore {
|
|||
for win in self.windows.values_mut() {
|
||||
if win.buffer_id == buffer_id {
|
||||
win.text_view = TextView::new(buffer);
|
||||
if win.cursor > len {
|
||||
win.cursor = len;
|
||||
}
|
||||
Self::clamp_cursor_and_selection(win, len);
|
||||
let max_top = win.text_view.line_count().saturating_sub(1);
|
||||
if win.view_top > max_top {
|
||||
win.view_top = max_top;
|
||||
|
|
@ -4828,6 +4884,205 @@ impl EditorCore {
|
|||
.map_err(|e| e.to_string())
|
||||
}
|
||||
|
||||
/// Rebind every buffer affected by a successful rename of `old` to
|
||||
/// `new` (dired Stage 2a, Q#DR14). Returns one
|
||||
/// [`RenameRebind`] per buffer moved.
|
||||
///
|
||||
/// A rename is a **transaction across path owners**, not a field
|
||||
/// update. This method owns the two owners that live in the buffer:
|
||||
/// the stored path and — subject to the provenance rule below — the
|
||||
/// name. Everything else keyed by the path (URI-keyed LSP stores,
|
||||
/// diagnostic overlays, dired's pathless handles, a package's own
|
||||
/// URI table) reconciles off the `resource.renamed` hook that the
|
||||
/// caller fires, because no buffer-keyed rebind can reach them.
|
||||
///
|
||||
/// Both rename paths call this — the drain harvest for
|
||||
/// `pmacs.fs.rename` and `apply_resource_op`'s rename arm — so the
|
||||
/// two cannot drift apart.
|
||||
///
|
||||
/// # The name
|
||||
///
|
||||
/// The name is rewritten only for a buffer whose name is
|
||||
/// [`crate::buffer::BufferNameOrigin::PathDerived`]. String
|
||||
/// inspection cannot substitute for that bit in either direction: a
|
||||
/// relative open is named `foo.rs` (so an equality test leaves it
|
||||
/// stale), and a user may name a buffer with a string that
|
||||
/// normalizes to its own path (so a path-equivalence test
|
||||
/// overwrites a chosen name). When it does fire, the new name is
|
||||
/// the **normalized** new path — a buffer opened relatively
|
||||
/// therefore acquires an absolute name, because no buffer records
|
||||
/// which base its name was relative to. Reconciliation re-records
|
||||
/// `PathDerived`, so a second rename still follows.
|
||||
pub fn reconcile_rename(&mut self, old: &Path, new: &Path) -> Vec<RenameRebind> {
|
||||
let old_n = normalize_buffer_path(old.to_path_buf());
|
||||
let new_n = normalize_buffer_path(new.to_path_buf());
|
||||
// A directory rename moves its whole subtree by construction,
|
||||
// so descendants are always in scope here.
|
||||
let affected = {
|
||||
let reg = self.registry.borrow();
|
||||
buffers_bound_under(®, &old_n, true)
|
||||
};
|
||||
let mut rebinds = Vec::with_capacity(affected.len());
|
||||
for (id, bound) in affected {
|
||||
// Rebuild the path under the new root. An exact match maps
|
||||
// to `new` itself; a descendant keeps its relative tail.
|
||||
let target = if bound == old_n {
|
||||
new_n.clone()
|
||||
} else {
|
||||
match bound.strip_prefix(&old_n) {
|
||||
Ok(tail) => new_n.join(tail),
|
||||
// Unreachable: `buffers_bound_under` matched on
|
||||
// exactly this prefix. Skip rather than guess.
|
||||
Err(_) => continue,
|
||||
}
|
||||
};
|
||||
let name_followed = {
|
||||
let mut reg = self.registry.borrow_mut();
|
||||
let Ok(buf) = reg.get_mut(id) else { continue };
|
||||
buf.set_file_path(Some(target.clone()));
|
||||
// The file behind this buffer moved, so metadata
|
||||
// captured against the old path no longer describes
|
||||
// it. Clearing is what `set_buffer_path`'s callers do
|
||||
// via `set_buffer_meta`; leaving it would make
|
||||
// external-change detection compare against a stat of
|
||||
// a path that is gone.
|
||||
buf.set_file_meta(None);
|
||||
if buf.name_origin() == crate::buffer::BufferNameOrigin::PathDerived {
|
||||
buf.set_path_derived_name(target.display().to_string());
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
};
|
||||
rebinds.push(RenameRebind {
|
||||
buffer_id: id,
|
||||
old_path: bound,
|
||||
new_path: target,
|
||||
name_followed,
|
||||
});
|
||||
}
|
||||
rebinds
|
||||
}
|
||||
|
||||
/// Reconcile the buffers a successful delete of `path` orphaned
|
||||
/// (dired Stage 2a, Q#DR18).
|
||||
///
|
||||
/// Walks the whole registry by normalized equality **or**
|
||||
/// component-aware prefix, so descendants of a deleted directory
|
||||
/// are included and a second buffer on one path is not missed.
|
||||
/// Descendants are unconditionally in scope here, unlike in
|
||||
/// `delete_verdict`: a recursive delete destroyed them, and a
|
||||
/// non-recursive one only succeeds on an *empty* directory, so a
|
||||
/// buffer still bound underneath it was already an orphan.
|
||||
///
|
||||
/// Policy, per buffer:
|
||||
///
|
||||
/// * **modified** — kept alive and reported. The buffer keeps its
|
||||
/// contents; only the file is gone. This is the half of the
|
||||
/// promise that is robust, because it runs at drain time against
|
||||
/// whatever state exists then.
|
||||
/// * **mid-edit** — skipped entirely and reported in `refused`,
|
||||
/// **preflighted** rather than discovered. A refusal from
|
||||
/// `BufferRegistry::remove` is *not* inert: by the time it
|
||||
/// returns `ConcurrentEdit`, [`Self::kill_buffer`] has already
|
||||
/// dropped the id from `round_trip_buffers`, closed any side
|
||||
/// window showing the buffer, and redirected every remaining
|
||||
/// window onto a fallback with cursor, selection, overlays and
|
||||
/// scroll position reset. The preflight is *sound*, not merely
|
||||
/// cheap: phase 1 is entirely `EditorCore`, which holds no Lua
|
||||
/// handle, so nothing between the check and the removal can
|
||||
/// re-enter Lua and begin an edit.
|
||||
/// * otherwise — killed through the full phase 1 above.
|
||||
///
|
||||
/// Neither refusal aborts the rest: a directory delete reaching
|
||||
/// twelve descendants must not stop at the one that is mid-edit.
|
||||
///
|
||||
/// # Phase 2 is the caller's
|
||||
///
|
||||
/// Buffer removal is two phases and the only place they are
|
||||
/// composed today is a Lua binding (`pmacs.buffer.kill`). Phase 2 —
|
||||
/// buffer-scoped keymaps, buffer-local config, folds, and the
|
||||
/// registered `on_removed` callbacks — lives in `lua_bindings` and
|
||||
/// needs `&Lua`, so this returns [`DeleteReconcile::killed`] and
|
||||
/// its caller runs phase 2 over those ids. `EditorCore` does not
|
||||
/// gain a Lua handle.
|
||||
pub fn reconcile_delete(&mut self, path: &Path) -> DeleteReconcile {
|
||||
let affected = {
|
||||
let reg = self.registry.borrow();
|
||||
buffers_bound_under(®, path, true)
|
||||
};
|
||||
let mut out = DeleteReconcile::default();
|
||||
for (id, _bound) in affected {
|
||||
let preflight = {
|
||||
let reg = self.registry.borrow();
|
||||
let Ok(buf) = reg.get(id) else { continue };
|
||||
let name = buf.name().to_owned();
|
||||
if buf.is_modified() {
|
||||
Some(Err((true, name)))
|
||||
} else if buf.editing_in_progress() {
|
||||
Some(Err((false, name)))
|
||||
} else {
|
||||
Some(Ok(()))
|
||||
}
|
||||
};
|
||||
match preflight {
|
||||
Some(Ok(())) => {}
|
||||
Some(Err((true, name))) => {
|
||||
out.kept_modified.push((id, name));
|
||||
continue;
|
||||
}
|
||||
Some(Err((false, name))) => {
|
||||
out.refused.push((
|
||||
id,
|
||||
format!("buffer {name:?} is mid-edit; finish the edit first"),
|
||||
));
|
||||
continue;
|
||||
}
|
||||
None => continue,
|
||||
}
|
||||
match self.kill_buffer(id) {
|
||||
Ok(()) => out.killed.push(id),
|
||||
// Named, because the reason alone is not actionable:
|
||||
// `kill_buffer`'s "cannot kill the last remaining
|
||||
// buffer" says nothing about *which* buffer is now
|
||||
// bound to a path whose file is gone, and that buffer's
|
||||
// name is what the user needs in order to save it
|
||||
// somewhere else.
|
||||
Err(message) => {
|
||||
let name = self
|
||||
.registry
|
||||
.borrow()
|
||||
.get(id)
|
||||
.map_or_else(|_| format!("{id:?}"), |b| b.name().to_owned());
|
||||
out.refused
|
||||
.push((id, format!("buffer {name:?}: {message}")));
|
||||
}
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// Re-root every URI-keyed overlay in **every** window from
|
||||
/// `old_uri` to `new_uri` (dired Stage 2a, §5).
|
||||
///
|
||||
/// The traversal mirrors overlay disposal's
|
||||
/// (`lua_bindings`'s `retain` over `overlay_identity`), with the
|
||||
/// `retain` replaced by [`View::rename_resource`]. That reaches
|
||||
/// passive windows as well as the active one — which the Lua attach
|
||||
/// path cannot, since `pmacs.diag._attach_view` takes the active
|
||||
/// window and errors otherwise — and preserves composition order,
|
||||
/// because nothing is removed or re-pushed.
|
||||
///
|
||||
/// A window that never received the overlay still has none;
|
||||
/// renaming cannot re-root an overlay that was never attached.
|
||||
pub fn rename_resource_in_views(&mut self, old_uri: &str, new_uri: &str) {
|
||||
for win in self.windows.values_mut() {
|
||||
for overlay in &mut win.overlays {
|
||||
overlay.rename_resource(old_uri, new_uri);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Switch one frontend's active window to a different buffer, allocating
|
||||
/// a fresh [`TextView`] for it without changing global active state.
|
||||
pub fn switch_active_buffer_for(
|
||||
|
|
@ -5085,6 +5340,85 @@ fn backward_word(buf: &Buffer, mut pos: Position) -> Position {
|
|||
pos
|
||||
}
|
||||
|
||||
/// Every path-bound buffer an operation on `target` affects, paired
|
||||
/// with its **normalized** stored path (dired Stage 2a; the shared walk
|
||||
/// query #190 introduced for `delete_verdict`, lifted so rename
|
||||
/// reconciliation and delete reconciliation cannot drift from it).
|
||||
///
|
||||
/// Three properties, each of which a naive lookup gets wrong:
|
||||
///
|
||||
/// * It scans **every** buffer.
|
||||
/// [`crate::buffer_registry::BufferRegistry::find_by_path`] is
|
||||
/// first-match-only, and duplicate path-bound buffers are reachable
|
||||
/// from public Lua via `pmacs.buffer.from_file` — so a first match
|
||||
/// can hide a second buffer on the same path, which then survives
|
||||
/// pointing at a path that no longer exists.
|
||||
/// * Both sides are normalized. Stored paths are normalized on write
|
||||
/// (`set_buffer_path`) while an op names its target however the
|
||||
/// caller spelled it, so a raw comparison misses the match entirely.
|
||||
/// * Containment is **component-aware** ([`Path::starts_with`]), never
|
||||
/// a string prefix: `/foo` is not an ancestor of `/foobar`.
|
||||
///
|
||||
/// `include_descendants` is the caller's decision because the two
|
||||
/// consumers legitimately differ. A delete *guard* scopes descendants
|
||||
/// to `recursive` (#190: a non-recursive delete destroys nothing
|
||||
/// beneath the target, so a buffer under it must not refuse the op),
|
||||
/// whereas a **rename** always moves its whole subtree and a
|
||||
/// post-delete reconciliation is looking at a directory that is
|
||||
/// already gone.
|
||||
pub fn buffers_bound_under(
|
||||
reg: &crate::buffer_registry::BufferRegistry,
|
||||
target: &Path,
|
||||
include_descendants: bool,
|
||||
) -> Vec<(BufferId, PathBuf)> {
|
||||
let target = normalize_buffer_path(target.to_path_buf());
|
||||
let mut out = Vec::new();
|
||||
for id in reg.ids() {
|
||||
let Ok(buf) = reg.get(*id) else { continue };
|
||||
let Some(bound) = buf.file_path() else {
|
||||
continue;
|
||||
};
|
||||
let bound = normalize_buffer_path(bound.to_path_buf());
|
||||
if bound == target || (include_descendants && bound.starts_with(&target)) {
|
||||
out.push((*id, bound));
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// One buffer moved by [`EditorCore::reconcile_rename`].
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct RenameRebind {
|
||||
/// The buffer that moved.
|
||||
pub buffer_id: BufferId,
|
||||
/// Its normalized path before the rename.
|
||||
pub old_path: PathBuf,
|
||||
/// Its normalized path after the rename.
|
||||
pub new_path: PathBuf,
|
||||
/// Whether the buffer's **name** followed the path, per
|
||||
/// [`crate::buffer::BufferNameOrigin`]. Reported rather than
|
||||
/// inferred so a consumer does not have to re-derive the
|
||||
/// provenance rule.
|
||||
pub name_followed: bool,
|
||||
}
|
||||
|
||||
/// Outcome of [`EditorCore::reconcile_delete`].
|
||||
///
|
||||
/// Three lists rather than two, because "kept on purpose" and "could
|
||||
/// not be removed" are different events: collapsing them makes a
|
||||
/// failure look like a policy decision.
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct DeleteReconcile {
|
||||
/// Buffers whose phase 1 (core-side removal) completed. The
|
||||
/// caller **must** run phase 2 (`after_buffer_removed`) over
|
||||
/// these — `EditorCore` holds no Lua handle.
|
||||
pub killed: Vec<BufferId>,
|
||||
/// Modified buffers kept alive deliberately, with their names.
|
||||
pub kept_modified: Vec<(BufferId, String)>,
|
||||
/// Buffers that could not be removed, with the reason.
|
||||
pub refused: Vec<(BufferId, String)>,
|
||||
}
|
||||
|
||||
/// Normalize a buffer path to an absolute, lexically-clean form:
|
||||
///
|
||||
/// 1. expand a leading `~` / `~/…` against `$HOME`,
|
||||
|
|
|
|||
1092
src/lsp.rs
1092
src/lsp.rs
File diff suppressed because it is too large
Load Diff
|
|
@ -232,6 +232,32 @@ pub fn install_diag(
|
|||
)?;
|
||||
}
|
||||
|
||||
// dired Stage 2a §5 step 6 — re-root every attached
|
||||
// `DiagnosticView` from `old_uri` to `new_uri` after a rename.
|
||||
//
|
||||
// `DiagnosticView.uri` is set once at construction and is private,
|
||||
// and `View` has no downcast, so nothing outside `diag.rs` can
|
||||
// reach it; the `View::rename_resource` hook is the seam. The sweep
|
||||
// walks EVERY window, which is what `_attach_view` above cannot do
|
||||
// — it takes the active window and errors otherwise — so a passive
|
||||
// split that already holds the overlay is re-rooted too. It mutates
|
||||
// in place, so each overlay keeps its position in the window's
|
||||
// composition order; a remove-and-re-push would move the underline
|
||||
// to the end of the stack and pass a one-window test anyway.
|
||||
{
|
||||
diag_mod.set(
|
||||
"_rename_resource",
|
||||
lua.create_function(move |lua, (old_uri, new_uri): (String, String)| {
|
||||
let Some(core) = lua.app_data_ref::<SharedCore>() else {
|
||||
return Ok(false);
|
||||
};
|
||||
core.borrow_mut()
|
||||
.rename_resource_in_views(&old_uri, &new_uri);
|
||||
Ok(true)
|
||||
})?,
|
||||
)?;
|
||||
}
|
||||
|
||||
pmacs.set("diag", diag_mod)?;
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -65,7 +65,13 @@ pub fn install_fold(lua: &Lua, fold_registry: &SharedFoldRegistry) -> mlua::Resu
|
|||
let id = buf.id();
|
||||
let requested = range_from_table(&range)?;
|
||||
let Some(bytes) = document_bytes(lua, id)? else {
|
||||
set_status(lua, "fold rejected: not a document buffer");
|
||||
// Q#GB16(a): the guard is spelled `is_read_only()`,
|
||||
// so this is the message it can actually justify.
|
||||
// Its author meant "terminal"; generated-buffer
|
||||
// immutability makes dired listings and listview
|
||||
// panels read-only too, and "not a document buffer"
|
||||
// would be a false explanation for those.
|
||||
set_status(lua, "fold rejected: buffer is read-only");
|
||||
return Ok(false);
|
||||
};
|
||||
if requested.start > bytes.len() as u64
|
||||
|
|
@ -307,6 +313,16 @@ fn range_to_table(lua: &Lua, r: ByteRange) -> mlua::Result<Table> {
|
|||
/// The buffer's bytes if it is a normal document buffer, or `None` if it is
|
||||
/// read-only (a terminal identity buffer or other non-document buffer —
|
||||
/// the Q#FD11 "normal document buffer" guard).
|
||||
///
|
||||
/// Q#GB16: the guard's author meant "terminal", and `read_only` is what
|
||||
/// they had. Generated-buffer immutability widens the flag's population
|
||||
/// — a dired listing and a listview panel are read-only from their first
|
||||
/// paint — so fold **creation** is now refused on those families too.
|
||||
/// That is accepted rather than worked around (option (a)): a generated
|
||||
/// buffer's contents are replaced wholesale on every refresh, which
|
||||
/// invalidates any stored range anyway. What is *not* accepted is
|
||||
/// explaining the refusal with a sentence that is no longer true, hence
|
||||
/// the status text at the `fold` call site.
|
||||
fn document_bytes(lua: &Lua, buf: BufferId) -> mlua::Result<Option<Vec<u8>>> {
|
||||
with_registry(lua, |r| {
|
||||
let buffer = resolve(r, buf)?;
|
||||
|
|
|
|||
|
|
@ -1671,16 +1671,11 @@ fn delete_verdict(
|
|||
}
|
||||
};
|
||||
|
||||
let target = crate::editor_core::normalize_buffer_path(path.to_path_buf());
|
||||
for id in reg.ids() {
|
||||
let Ok(buf) = reg.get(*id) else { continue };
|
||||
let Some(bound) = buf.file_path() else {
|
||||
continue;
|
||||
};
|
||||
let bound = crate::editor_core::normalize_buffer_path(bound.to_path_buf());
|
||||
if bound != target && !(scan_descendants && bound.starts_with(&target)) {
|
||||
continue;
|
||||
}
|
||||
// The shared walk (dired Stage 2a): one enumeration, so this guard
|
||||
// and the two reconciliation seams cannot disagree about which
|
||||
// buffers an operation on `path` touches.
|
||||
for (id, _bound) in crate::editor_core::buffers_bound_under(reg, path, scan_descendants) {
|
||||
let Ok(buf) = reg.get(id) else { continue };
|
||||
// "Modified" is `Buffer::is_modified()`. No new notion of
|
||||
// dirtiness, and a *clean* open buffer is deliberately not
|
||||
// guarded — refusing there would fail legitimate deletes for
|
||||
|
|
@ -1714,6 +1709,223 @@ fn delete_verdict(
|
|||
DeleteVerdict::Clear
|
||||
}
|
||||
|
||||
/// Reconcile a successful rename and fire `resource.renamed` (dired
|
||||
/// Stage 2a, §5).
|
||||
///
|
||||
/// Both rename paths land here — the drain harvest for
|
||||
/// `pmacs.fs.rename` and `apply_resource_op`'s rename arm — so the two
|
||||
/// can no longer drift, which is how the raw-lookup trap survived being
|
||||
/// "fixed" once already.
|
||||
///
|
||||
/// The hook carries the **paths**, normalized absolute, not the rebind
|
||||
/// list: dired's buffers are pathless, so a path-keyed consumer must be
|
||||
/// able to reconcile from `(old, new)` alone. And the Rust side is
|
||||
/// structurally incapable of being complete — any package may key state
|
||||
/// by URI in its own module table and the LSP manager will never know —
|
||||
/// so the hook is the mechanism that scales, not a convenience.
|
||||
///
|
||||
/// Returns the rebinds, for a caller that wants to report.
|
||||
fn reconcile_rename_and_fire(
|
||||
lua: &Lua,
|
||||
from: &std::path::Path,
|
||||
to: &std::path::Path,
|
||||
) -> Vec<crate::editor_core::RenameRebind> {
|
||||
let (rebinds, old_n, new_n) = {
|
||||
let Some(core) = lua.app_data_ref::<SharedCore>() else {
|
||||
return Vec::new();
|
||||
};
|
||||
let mut core = core.borrow_mut();
|
||||
let rebinds = core.reconcile_rename(from, to);
|
||||
(
|
||||
rebinds,
|
||||
crate::editor_core::normalize_buffer_path(from.to_path_buf()),
|
||||
crate::editor_core::normalize_buffer_path(to.to_path_buf()),
|
||||
)
|
||||
};
|
||||
// The borrow is released before re-entering Lua: subscribers call
|
||||
// back into the core (dired reverts a listing, the LSP subscriber
|
||||
// re-attaches), and a live borrow would panic.
|
||||
let mut args = mlua::MultiValue::new();
|
||||
args.push_back(mlua::Value::String(
|
||||
match lua.create_string(old_n.as_os_str().as_encoded_bytes()) {
|
||||
Ok(s) => s,
|
||||
Err(_) => return rebinds,
|
||||
},
|
||||
));
|
||||
args.push_back(mlua::Value::String(
|
||||
match lua.create_string(new_n.as_os_str().as_encoded_bytes()) {
|
||||
Ok(s) => s,
|
||||
Err(_) => return rebinds,
|
||||
},
|
||||
));
|
||||
run_hook_if_defined(lua, "resource.renamed", args);
|
||||
rebinds
|
||||
}
|
||||
|
||||
/// Reconcile a successful delete and fire `resource.deleted` (dired
|
||||
/// Stage 2a, §6).
|
||||
///
|
||||
/// Composes the **same two removal phases** `pmacs.buffer.kill`
|
||||
/// composes. Phase 1 (`EditorCore::reconcile_delete`) closes side
|
||||
/// windows showing a doomed buffer, redirects every other window to a
|
||||
/// fallback, and removes the id from the registry; phase 2 —
|
||||
/// buffer-scoped keymaps, buffer-local config, folds, and the
|
||||
/// registered `on_removed` callbacks — runs here, because it needs
|
||||
/// `&Lua` and `EditorCore` has no Lua handle.
|
||||
///
|
||||
/// `apply_resource_op`'s delete arm previously ran
|
||||
/// `remove_buffer_and_fire`, i.e. phase 2 **without** phase 1, leaving
|
||||
/// any window displaying that buffer pointing at a removed id. Routing
|
||||
/// both paths through here is what makes that go away as a property of
|
||||
/// the seam rather than as a separate patch.
|
||||
fn reconcile_delete_and_fire(
|
||||
lua: &Lua,
|
||||
path: &std::path::Path,
|
||||
) -> crate::editor_core::DeleteReconcile {
|
||||
let (outcome, normalized) = {
|
||||
let Some(core) = lua.app_data_ref::<SharedCore>() else {
|
||||
return crate::editor_core::DeleteReconcile::default();
|
||||
};
|
||||
let mut core = core.borrow_mut();
|
||||
let outcome = core.reconcile_delete(path);
|
||||
(
|
||||
outcome,
|
||||
crate::editor_core::normalize_buffer_path(path.to_path_buf()),
|
||||
)
|
||||
};
|
||||
// Phase 2, over exactly the ids phase 1 removed.
|
||||
for id in &outcome.killed {
|
||||
after_buffer_removed(lua, *id);
|
||||
}
|
||||
if let Ok(path_arg) = lua.create_string(normalized.as_os_str().as_encoded_bytes()) {
|
||||
let mut args = mlua::MultiValue::new();
|
||||
args.push_back(mlua::Value::String(path_arg));
|
||||
run_hook_if_defined(lua, "resource.deleted", args);
|
||||
}
|
||||
// Reported AFTER the fan-out, deliberately: a subscriber may set its
|
||||
// own status, and this message must be the last word because it is
|
||||
// the data-loss-adjacent one. Unconditional, so a path that cannot
|
||||
// cross into Lua still gets its refusal reported rather than losing
|
||||
// both the hook and the report.
|
||||
report_delete_reconcile(lua, &normalized, &outcome);
|
||||
outcome
|
||||
}
|
||||
|
||||
/// Cap on how many buffer names one status line spells out before
|
||||
/// collapsing the rest into a count. A directory delete can reach
|
||||
/// dozens; a status line that scrolls off is a message nobody reads.
|
||||
const DELETE_REPORT_NAMED_LIMIT: usize = 3;
|
||||
|
||||
/// Render the buffers a delete could not reconcile, and put it on the
|
||||
/// status channel.
|
||||
///
|
||||
/// **Silence here is the defect this exists to close.** Both outcomes
|
||||
/// leave a buffer alive and still bound to a path whose file is gone, so
|
||||
/// the next `C-x C-s` recreates the file the user just deleted. That is
|
||||
/// recoverable only if the user knows it happened:
|
||||
///
|
||||
/// * `kept_modified` — a modified buffer, kept on purpose. On the
|
||||
/// synchronous path #190 refuses before disk so this cannot arise, but
|
||||
/// `pmacs.fs.remove` dispatches a worker, and a buffer modified in the
|
||||
/// interval between the caller's check and the syscall reaches here.
|
||||
/// * `refused` — could not be removed at all: the last remaining buffer
|
||||
/// (`kill_buffer` refuses to empty the registry), or a buffer that was
|
||||
/// mid-edit when the reconciliation ran.
|
||||
///
|
||||
/// The channel is `EditorCore::status`, which is what
|
||||
/// `pmacs.editor.set_status` writes. **Not `pmacs.error`** — that
|
||||
/// channel is defined only by a test stub, so all fifteen of its guarded
|
||||
/// call sites are dead, and a report written there would be exactly the
|
||||
/// silence being fixed.
|
||||
///
|
||||
/// Lives inside the shared seam rather than at its two call sites, for
|
||||
/// the same reason the reconciliation does: a caller that has to
|
||||
/// remember to report is a caller that will forget. The first version of
|
||||
/// this function's callers both discarded the outcome.
|
||||
fn report_delete_reconcile(
|
||||
lua: &Lua,
|
||||
path: &std::path::Path,
|
||||
outcome: &crate::editor_core::DeleteReconcile,
|
||||
) {
|
||||
if outcome.kept_modified.is_empty() && outcome.refused.is_empty() {
|
||||
return;
|
||||
}
|
||||
let name_of = |p: &std::path::Path| {
|
||||
p.file_name()
|
||||
.map_or_else(|| p.display().to_string(), |n| n.to_string_lossy().into())
|
||||
};
|
||||
let mut parts: Vec<String> = Vec::new();
|
||||
if !outcome.kept_modified.is_empty() {
|
||||
let n = outcome.kept_modified.len();
|
||||
let named: Vec<&str> = outcome
|
||||
.kept_modified
|
||||
.iter()
|
||||
.take(DELETE_REPORT_NAMED_LIMIT)
|
||||
.map(|(_, name)| name.as_str())
|
||||
.collect();
|
||||
parts.push(format!(
|
||||
"{n} buffer{} with unsaved changes kept ({}{}) — saving {} will RECREATE the deleted file",
|
||||
if n == 1 { "" } else { "s" },
|
||||
named.join(", "),
|
||||
if n > named.len() {
|
||||
format!(", and {} more", n - named.len())
|
||||
} else {
|
||||
String::new()
|
||||
},
|
||||
if n == 1 { "it" } else { "them" },
|
||||
));
|
||||
}
|
||||
if !outcome.refused.is_empty() {
|
||||
let n = outcome.refused.len();
|
||||
let named: Vec<String> = outcome
|
||||
.refused
|
||||
.iter()
|
||||
.take(DELETE_REPORT_NAMED_LIMIT)
|
||||
.map(|(_, why)| why.clone())
|
||||
.collect();
|
||||
parts.push(format!(
|
||||
"{n} buffer{} could not be closed ({}{})",
|
||||
if n == 1 { "" } else { "s" },
|
||||
named.join("; "),
|
||||
if n > named.len() {
|
||||
format!("; and {} more", n - named.len())
|
||||
} else {
|
||||
String::new()
|
||||
},
|
||||
));
|
||||
}
|
||||
let message = format!("deleted {}: {}", name_of(path), parts.join("; "));
|
||||
if let Some(core) = lua.app_data_ref::<SharedCore>() {
|
||||
core.borrow_mut().status = message;
|
||||
}
|
||||
}
|
||||
|
||||
/// Drive [`crate::async_runtime::TickOutcome::resources`] through
|
||||
/// reconciliation, one settled mutation at a time (dired Stage 2a,
|
||||
/// Q#DR29).
|
||||
///
|
||||
/// **Each settled mutation reconciles on its own, and nothing here
|
||||
/// depends on the relative order of two mutations that were in flight
|
||||
/// simultaneously** — `resources` is bus-arrival order and the runtime
|
||||
/// establishes no execution token. That is safe rather than merely
|
||||
/// honest: independent mutations commute, and the primitive's contract
|
||||
/// (`builtin/runtime/fs.lua`) requires a caller with overlapping
|
||||
/// source/target paths to serialize by awaiting each op before
|
||||
/// dispatching the next.
|
||||
fn reconcile_settled_resources(lua: &Lua, resources: &[crate::async_runtime::ResourceOp]) {
|
||||
use crate::async_runtime::ResourceOp;
|
||||
for op in resources {
|
||||
match op {
|
||||
ResourceOp::Rename { from, to } => {
|
||||
reconcile_rename_and_fire(lua, from, to);
|
||||
}
|
||||
ResourceOp::Remove { path } => {
|
||||
reconcile_delete_and_fire(lua, path);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn remove_buffer_and_fire(lua: &Lua, registry: &SharedRegistry, id: BufferId) -> mlua::Result<()> {
|
||||
registry
|
||||
.borrow_mut()
|
||||
|
|
@ -3220,6 +3432,37 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
|
|||
)?;
|
||||
}
|
||||
|
||||
{
|
||||
// dired Stage 2a Q#DR21 — expose the existing Rust setter,
|
||||
// which already documents itself as for "save-as and rename
|
||||
// operations". Dired needs it because its listing buffers are
|
||||
// **pathless**: no buffer-keyed rebind can find them, so the
|
||||
// only way a `*dired:<path>*` buffer can follow a renamed
|
||||
// directory is for dired's own `resource.renamed` subscriber to
|
||||
// rename it. The alternative — kill and recreate under the new
|
||||
// name — loses window placement, the cursor, the read-only
|
||||
// intercept, round-trip input and the major mode, each of which
|
||||
// would have to be re-established in the right order.
|
||||
//
|
||||
// Uniqueness stays the CALLER's job, matching the Rust setter;
|
||||
// dired reuses its existing `<2>`-variant uniquifier.
|
||||
//
|
||||
// This records `BufferNameOrigin::Explicit` (Q#DR30): it is a
|
||||
// naming operation even when the string happens to denote the
|
||||
// file, so a later rename must not overwrite it.
|
||||
let reg = registry.clone();
|
||||
buffer.set(
|
||||
"set_name",
|
||||
lua.create_function(move |_, (id, name): (BufferIdLua, String)| {
|
||||
reg.borrow_mut()
|
||||
.get_mut(id.0)
|
||||
.map_err(mlua::Error::external)?
|
||||
.set_name(name);
|
||||
Ok(())
|
||||
})?,
|
||||
)?;
|
||||
}
|
||||
|
||||
{
|
||||
let reg = registry.clone();
|
||||
buffer.set(
|
||||
|
|
@ -3244,6 +3487,11 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
|
|||
))
|
||||
})?;
|
||||
let id = reg.borrow_mut().create_from_bytes(path.clone(), &bytes);
|
||||
// Path-backed creation site (Q#DR30): this name is the
|
||||
// path as given, so rename reconciliation may move it.
|
||||
if let Ok(b) = reg.borrow_mut().get_mut(id) {
|
||||
b.set_path_derived_name(path.clone());
|
||||
}
|
||||
if let Some(core) = lua.app_data_ref::<SharedCore>() {
|
||||
let mut core = core.borrow_mut();
|
||||
core.switch_active_buffer(id)
|
||||
|
|
@ -3307,6 +3555,10 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
|
|||
))
|
||||
})?;
|
||||
let id = reg.borrow_mut().create_from_bytes(path.clone(), &bytes);
|
||||
// Path-backed creation site (Q#DR30), as in `from_file`.
|
||||
if let Ok(b) = reg.borrow_mut().get_mut(id) {
|
||||
b.set_path_derived_name(path.clone());
|
||||
}
|
||||
if let Some(core) = lua.app_data_ref::<SharedCore>() {
|
||||
let mut core = core.borrow_mut();
|
||||
core.switch_active_buffer(id)
|
||||
|
|
@ -3428,12 +3680,18 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
|
|||
.map_err(|e| io_err("rename (parents)", e))?;
|
||||
}
|
||||
std::fs::rename(&from, &to).map_err(|e| io_err("rename", e))?;
|
||||
let bid = reg.borrow().find_by_path(&from);
|
||||
if let Some(id) = bid
|
||||
&& let Some(core) = lua.app_data_ref::<SharedCore>()
|
||||
{
|
||||
core.borrow_mut().set_buffer_path(id, Some(to.clone()));
|
||||
}
|
||||
// dired Stage 2a: the raw, first-match,
|
||||
// un-normalized `find_by_path` lookup this arm
|
||||
// used is replaced by the shared transaction.
|
||||
// Three defects went with it — stored paths are
|
||||
// normalized on write while the op names its
|
||||
// target raw, so the lookup could miss the
|
||||
// buffer entirely; a directory rename has many
|
||||
// affected buffers by construction and only the
|
||||
// first moved; and the buffer's *name* stayed
|
||||
// stale, so the statusline and buffer list kept
|
||||
// the old filename.
|
||||
reconcile_rename_and_fire(lua, &from, &to);
|
||||
}
|
||||
"delete" => {
|
||||
// Four ordered phases (Q#RD2): stat/no-op
|
||||
|
|
@ -3490,18 +3748,19 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
|
|||
};
|
||||
r.map_err(|e| io_err("delete", e))?;
|
||||
|
||||
// Phase 4 — reconcile exactly as before
|
||||
// (Q#RD10): the single first exact-path match is
|
||||
// removed and additional clean duplicates are
|
||||
// left in place. Removing them all would route N
|
||||
// Phase 4 — reconcile through the shared seam
|
||||
// (dired Stage 2a, Q#DR27). #190 deliberately
|
||||
// left this as the single first exact-path match
|
||||
// because removing them all would have routed N
|
||||
// buffers through `remove_buffer_and_fire`,
|
||||
// which is phase 2 without phase 1, creating up
|
||||
// to N dangling windows — the parked lifecycle
|
||||
// defect this lane must not enlarge.
|
||||
let bid = reg.borrow().find_by_path(&pb);
|
||||
if let Some(id) = bid {
|
||||
remove_buffer_and_fire(lua, ®, id)?;
|
||||
}
|
||||
// which is phase 2 *without* phase 1 and would
|
||||
// have created up to N dangling windows. That
|
||||
// constraint is now gone: `reconcile_delete`
|
||||
// composes both phases, so descendants and
|
||||
// duplicate path-bound buffers can all be
|
||||
// reconciled, and no window is left holding a
|
||||
// removed id.
|
||||
reconcile_delete_and_fire(lua, &pb);
|
||||
}
|
||||
other => {
|
||||
return Err(mlua::Error::external(format!(
|
||||
|
|
@ -7358,9 +7617,15 @@ pub fn install_async(
|
|||
async_mod.set(
|
||||
"_tick",
|
||||
lua.create_function(move |lua, ()| {
|
||||
let ids = rt.tick();
|
||||
let t = lua.create_table_with_capacity(ids.len(), 0)?;
|
||||
for (i, id) in ids.into_iter().enumerate() {
|
||||
let outcome = rt.tick();
|
||||
// Reconcile BEFORE the settled ids reach Lua. The Lua
|
||||
// runtime resumes parked coroutines from the table this
|
||||
// returns, so a coroutine that renamed and then
|
||||
// inspects a buffer would otherwise see pre-rename
|
||||
// state. Ordering here is by construction, not by luck.
|
||||
reconcile_settled_resources(lua, &outcome.resources);
|
||||
let t = lua.create_table_with_capacity(outcome.settled.len(), 0)?;
|
||||
for (i, id) in outcome.settled.into_iter().enumerate() {
|
||||
t.set(i + 1, id)?;
|
||||
}
|
||||
Ok(t)
|
||||
|
|
@ -9992,11 +10257,18 @@ pub fn install_lsp(
|
|||
// `builtin/runtime/lsp.lua` calls this per edit so stale
|
||||
// suppression stays keystroke-accurate while the O(file)
|
||||
// full-document notification is coalesced.
|
||||
//
|
||||
// **Takes the server id since dired Stage 2a.** It previously
|
||||
// took the URI alone while creating URI keys in three stores
|
||||
// for every server at once, which made it the second
|
||||
// uncorrelated writer able to resurrect a URI `forget_uri` had
|
||||
// just cleared. The sole production caller already holds
|
||||
// `rec.server`.
|
||||
let m = manager.clone();
|
||||
lsp_mod.set(
|
||||
"_mark_document_stale",
|
||||
lua.create_function(move |_, uri: String| {
|
||||
m.borrow().mark_document_stale(&uri);
|
||||
lua.create_function(move |_, (id, uri): (LspServerIdLua, String)| {
|
||||
m.borrow().mark_document_stale(id.0, &uri);
|
||||
Ok(())
|
||||
})?,
|
||||
)?;
|
||||
|
|
@ -10520,6 +10792,35 @@ pub fn install_lsp(
|
|||
)?;
|
||||
}
|
||||
|
||||
{
|
||||
// dired Stage 2a §5 — the per-document teardown the
|
||||
// `resource.renamed` subscriber needs. Modelled on `forget`
|
||||
// above: a closure over the shared manager that calls through
|
||||
// and maps the error with `mlua::Error::external`.
|
||||
//
|
||||
// Error contract: **raises** for an unknown server id, matching
|
||||
// `forget`'s behaviour for the same input, and **succeeds
|
||||
// silently** when the URI has no state under a known server.
|
||||
// The second arm is the one that matters — the subscriber runs
|
||||
// per attachment, an attachment need not have any pending route
|
||||
// or populated result store, and cleanup can be repeated after
|
||||
// an earlier partial teardown. An over-strict binding would turn
|
||||
// that ordinary idempotent case into an error inside a hook.
|
||||
//
|
||||
// Takes the **old** URI, so calling it after `did_open` of the
|
||||
// new one is safe and order-independent.
|
||||
let m = manager.clone();
|
||||
lsp_mod.set(
|
||||
"forget_uri",
|
||||
lua.create_function(move |_, (id, uri): (LspServerIdLua, String)| {
|
||||
m.borrow_mut()
|
||||
.forget_uri(id.0, &uri)
|
||||
.map_err(mlua::Error::external)?;
|
||||
Ok(())
|
||||
})?,
|
||||
)?;
|
||||
}
|
||||
|
||||
{
|
||||
let m = manager.clone();
|
||||
lsp_mod.set(
|
||||
|
|
|
|||
191
src/process.rs
191
src/process.rs
|
|
@ -636,6 +636,26 @@ impl Drop for RuntimeHandles {
|
|||
// channel because the consumer fell behind. Cancel flag
|
||||
// unwedges that case before we join. T M6.2.
|
||||
self.cancel.store(true, Ordering::Relaxed);
|
||||
// Close the child's stdin BEFORE joining. `cancel` covers a
|
||||
// reader stuck in `send`; it does NOT cover one stuck in
|
||||
// `read`, which is only consulted between reads. What actually
|
||||
// unblocks that reader is the child exiting and closing its
|
||||
// output pipe --- and a stdio child exits on stdin EOF.
|
||||
//
|
||||
// The premise in the comment above ("dropping the master
|
||||
// closes the kernel pipe") holds for a PTY master but NOT for
|
||||
// pipe mode, where `read` unblocks only once *every* write end
|
||||
// closes. An escaped descendant holding one (a shim-launched
|
||||
// language server that orphans its real process) keeps the
|
||||
// reader blocked indefinitely.
|
||||
//
|
||||
// The sink lives in the `stdin` FIELD, and a type's `Drop::drop`
|
||||
// body runs before *all* of its fields regardless of their
|
||||
// declaration order --- so reordering the struct cannot fix
|
||||
// this. Joining first deadlocks against the very EOF that would
|
||||
// have ended the join. `take()` is idempotent, matching
|
||||
// `close_stdin`.
|
||||
let _ = self.stdin.take();
|
||||
for h in std::mem::take(&mut self.readers) {
|
||||
let _ = h.join();
|
||||
}
|
||||
|
|
@ -3204,6 +3224,177 @@ mod tests {
|
|||
handle.join().expect("test thread should exit cleanly");
|
||||
}
|
||||
|
||||
/// The stdin sink lives in a *field* of [`RuntimeHandles`], so it
|
||||
/// cannot drop until `Drop::drop`'s body returns --- and a type's
|
||||
/// drop body runs before *all* of its fields, whatever their
|
||||
/// declaration order (so reordering the struct cannot fix this).
|
||||
/// Joining readers inside that body therefore deadlocks against any
|
||||
/// child that exits on stdin EOF while still holding the output
|
||||
/// pipe: no EOF, so no exit, so no pipe close, so a blocking
|
||||
/// `spawn_reader` never returns.
|
||||
///
|
||||
/// This is the root cause of
|
||||
/// `m4_5_basedpyright_initializes_and_negotiates_encoding` hanging
|
||||
/// forever. Modelled with an orphaned grandchild, which is exactly
|
||||
/// what a shim-launched language server is: the basedpyright
|
||||
/// console script spawns bundled `node` and exits, leaving the real
|
||||
/// server at `PPid 1` holding the inherited pipes.
|
||||
///
|
||||
/// `setsid --fork` is used rather than a shell background job, and
|
||||
/// that choice is LOAD-BEARING. POSIX XCU 2.9.3 assigns `/dev/null`
|
||||
/// to an asynchronous list's stdin when job control is off --- i.e.
|
||||
/// in every non-interactive `sh` --- so `sh -c 'cat & exit 0'` reads
|
||||
/// EOF immediately and exits *against the unfixed tree*, giving a
|
||||
/// test that passes either way and proves nothing. The obvious
|
||||
/// repair does not work either: the rule applies **before explicit
|
||||
/// redirections**, so by the time `<&0` runs, fd 0 already *is*
|
||||
/// `/dev/null` and the redirect faithfully duplicates it onto
|
||||
/// itself. `bash` happens to skip the default when a stdin redirect
|
||||
/// is present; `dash` --- Ubuntu's `/bin/sh`, and CI's --- does not,
|
||||
/// so `<&0` passed locally and failed in CI.
|
||||
///
|
||||
/// `setsid --fork` sidesteps all of it: it forks, the parent exits,
|
||||
/// and the child inherits stdin/stdout/stderr untouched by any shell.
|
||||
/// No async list, no `/dev/null` rule, no implementation variance.
|
||||
///
|
||||
/// Linux-gated deliberately rather than incidentally: the controls
|
||||
/// read `/proc`, and `setsid(1)` is util-linux (absent on macOS).
|
||||
///
|
||||
/// On the failure path this leaks a wedged worker thread, and `cat`
|
||||
/// survives until the harness's fds close at process exit. Bounded
|
||||
/// and intentional --- a test that *hung* on regression would
|
||||
/// reproduce the very hazard it exists to catch.
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn teardown_closes_stdin_before_joining_readers() {
|
||||
use std::sync::mpsc;
|
||||
|
||||
/// `sh` becomes a zombie when it exits, because this test
|
||||
/// deliberately never ticks (a tick runs `poll_one`, which is
|
||||
/// the teardown path under test). `kill(pid, None)` succeeds on
|
||||
/// a zombie, so liveness has to come from the process state
|
||||
/// rather than from signal 0.
|
||||
fn reaped_or_zombie(pid: u32) -> bool {
|
||||
match std::fs::read_to_string(format!("/proc/{pid}/stat")) {
|
||||
Err(_) => true,
|
||||
Ok(s) => s
|
||||
.rsplit_once(')')
|
||||
.and_then(|(_, rest)| rest.split_whitespace().next())
|
||||
.is_some_and(|state| state == "Z"),
|
||||
}
|
||||
}
|
||||
|
||||
// setsid(1) is util-linux, not coreutils, and the standard
|
||||
// `cargo test --lib` gate must not hard-fail on a tool the
|
||||
// README does not require --- a minimal or BusyBox container
|
||||
// would fail without ever testing pmacs. So: skip when absent,
|
||||
// but FAIL when `PMACS_REQUIRE_SETSID` is set, which CI sets on
|
||||
// Linux. That is the arming pattern from the silent-skip lane,
|
||||
// and it is what keeps this from becoming a test that reports
|
||||
// `ok` having never run. Presence decides, so an empty value
|
||||
// counts as unset (a `${{ cond && '1' || '' }}` expression sets
|
||||
// the empty string, not nothing).
|
||||
let armed = std::env::var_os("PMACS_REQUIRE_SETSID").is_some_and(|v| !v.is_empty());
|
||||
if !binary_available("setsid") {
|
||||
assert!(
|
||||
!armed,
|
||||
"PMACS_REQUIRE_SETSID is set but setsid(1) is not on PATH: \
|
||||
install util-linux, or unset the variable to allow the skip"
|
||||
);
|
||||
eprintln!(
|
||||
"setsid(1) not on PATH; skipping \
|
||||
teardown_closes_stdin_before_joining_readers"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let (done_tx, done_rx) = mpsc::channel();
|
||||
let handle = std::thread::spawn(move || {
|
||||
let mut sup = ProcessSupervisor::new();
|
||||
sup.set_grace_period(Duration::from_millis(300));
|
||||
let mut spec = ProcessSpec::new("orphan-holds-pipe", "setsid");
|
||||
// `setsid --fork` forks and the parent exits, so the
|
||||
// *recorded* pid terminates promptly (letting `poll_one`
|
||||
// reach the teardown path) while `cat` survives holding the
|
||||
// inherited pipes. `cat` reads stdin and exits on EOF,
|
||||
// exactly as a stdio language server does.
|
||||
spec.args = vec!["--fork".into(), "cat".into()];
|
||||
// The default, restated because it is the whole point: with
|
||||
// `StdinMode::Null` there is no sink to drop and no EOF to
|
||||
// deliver.
|
||||
spec.stdin = StdinMode::Piped;
|
||||
let id = sup.spawn(spec).expect("spawn");
|
||||
|
||||
let sh_pid = sup
|
||||
.processes
|
||||
.get(&id)
|
||||
.and_then(|p| p.runtime.as_ref())
|
||||
.map(|rt| rt.pid)
|
||||
.expect("runtime records the spawned pid");
|
||||
|
||||
// CONTROL 1: the recorded child must actually exit. Until it
|
||||
// does, *it* holds the output pipe, and control 2 would pass
|
||||
// for the wrong reason. (`setsid` without `--fork` may exec
|
||||
// directly instead of forking, in which case there is no
|
||||
// grandchild and this is the control that notices.)
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
while Instant::now() < deadline && !reaped_or_zombie(sh_pid) {
|
||||
std::thread::sleep(Duration::from_millis(10));
|
||||
}
|
||||
assert!(
|
||||
reaped_or_zombie(sh_pid),
|
||||
"control 1 failed: the recorded child (`sh`) should exit \
|
||||
promptly, leaving the grandchild orphaned. While `sh` is \
|
||||
alive it holds the output pipe itself, so control 2 would \
|
||||
pass without the grandchild modelling anything"
|
||||
);
|
||||
|
||||
// CONTROL 2: both readers must still be blocked in `read`,
|
||||
// which is only true while something still holds the output
|
||||
// pipe's write ends. If the grandchild never inherited the
|
||||
// real stdin, it has already read EOF and exited, the write
|
||||
// ends are closed, the readers have finished --- and the
|
||||
// deadlock is not being modelled at all. This control is
|
||||
// what caught the shell form failing on dash after it
|
||||
// passed on bash.
|
||||
let readers = sup
|
||||
.processes
|
||||
.get(&id)
|
||||
.and_then(|p| p.runtime.as_ref())
|
||||
.map(|rt| {
|
||||
(
|
||||
rt.readers.len(),
|
||||
rt.readers.iter().filter(|h| !h.is_finished()).count(),
|
||||
)
|
||||
})
|
||||
.expect("runtime still present before teardown");
|
||||
assert_eq!(
|
||||
readers,
|
||||
(2, 2),
|
||||
"control 2 failed: both readers must still be blocked in \
|
||||
`read`, i.e. an escaped grandchild still holds the output \
|
||||
pipe. Finished readers mean `cat` read EOF and exited \
|
||||
already, so it never inherited the real stdin --- check \
|
||||
that `setsid --fork` still forks and passes fds 0/1/2 \
|
||||
through untouched on this runner"
|
||||
);
|
||||
|
||||
// The deadlock, if present, is here:
|
||||
// shutdown -> tick -> poll_one -> RuntimeHandles::drop -> join.
|
||||
drop(sup);
|
||||
let _ = done_tx.send(());
|
||||
});
|
||||
|
||||
done_rx.recv_timeout(Duration::from_secs(10)).expect(
|
||||
"supervisor drop should complete within 10s --- if hung, \
|
||||
`RuntimeHandles::drop` is joining its readers before dropping \
|
||||
the `stdin` field, so the child never receives EOF, never \
|
||||
exits, and never closes the output pipe the readers are \
|
||||
blocked on",
|
||||
);
|
||||
handle.join().expect("test thread should exit cleanly");
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------
|
||||
// Compile-mode group lifecycle (Q#CM3; framing acceptance 34)
|
||||
// -----------------------------------------------------------------
|
||||
|
|
|
|||
14
src/view.rs
14
src/view.rs
|
|
@ -310,6 +310,20 @@ pub trait View {
|
|||
fn clone_for_split(&self) -> Option<Box<dyn View>> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Retarget this overlay from `old_uri` to `new_uri` after a
|
||||
/// resource rename (dired Stage 2a, §5). Default: no-op — a view
|
||||
/// that renders nothing URI-keyed is unaffected.
|
||||
///
|
||||
/// Mutates **in place**, so the overlay keeps its position in the
|
||||
/// window's composition order. That is the reason this is a trait
|
||||
/// hook rather than a remove-and-re-push at the call site: overlays
|
||||
/// are an ordered `Vec` merged in sequence, and re-pushing would
|
||||
/// move a diagnostic underline to the end of the stack. It is also
|
||||
/// how *passive* windows are reached at all — the Lua attach path
|
||||
/// (`pmacs.diag._attach_view`) can only touch the active window,
|
||||
/// while the sweep that drives this walks every window.
|
||||
fn rename_resource(&mut self, _old_uri: &str, _new_uri: &str) {}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ use std::path::{Path, PathBuf};
|
|||
use std::time::{Duration, Instant, SystemTime};
|
||||
|
||||
use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers};
|
||||
use pmacs::buffer::BufferId;
|
||||
use pmacs::cell::{CellGrid, CellSize, Glyph};
|
||||
use pmacs::editor::EditorState;
|
||||
use pmacs::editor_core::normalize_buffer_path;
|
||||
|
|
@ -70,6 +71,39 @@ fn type_str(s: &mut EditorState, text: &str) {
|
|||
}
|
||||
}
|
||||
|
||||
fn alt(s: &mut EditorState, c: char) {
|
||||
s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Char(c), KeyModifiers::ALT));
|
||||
}
|
||||
|
||||
/// `M-x <name> RET` through the real minibuffer. `buffer.undo` is
|
||||
/// reachable this way on every buffer in the tree and no buffer-local
|
||||
/// rebinding can remove it (generated-buffer immutability, §0).
|
||||
fn m_x(s: &mut EditorState, name: &str) {
|
||||
alt(s, 'x');
|
||||
type_str(s, name);
|
||||
press(s, KeyCode::Enter);
|
||||
}
|
||||
|
||||
fn active_buffer_id(s: &EditorState) -> BufferId {
|
||||
s.core.borrow().active_buffer_id()
|
||||
}
|
||||
|
||||
/// Q#GB14: the rope lock has no Lua surface, so every "is it locked"
|
||||
/// assertion goes through Rust.
|
||||
fn is_read_only(s: &EditorState, id: BufferId) -> bool {
|
||||
let core = s.core.borrow();
|
||||
let reg = core.registry.borrow();
|
||||
reg.get(id).expect("buffer in registry").is_read_only()
|
||||
}
|
||||
|
||||
fn set_read_only(s: &EditorState, id: BufferId, value: bool) {
|
||||
let core = s.core.borrow();
|
||||
let mut reg = core.registry.borrow_mut();
|
||||
reg.get_mut(id)
|
||||
.expect("buffer in registry")
|
||||
.set_read_only(value);
|
||||
}
|
||||
|
||||
fn exec(s: &EditorState, src: &str) {
|
||||
s.lua_host.lua().load(src.to_string()).exec().unwrap();
|
||||
}
|
||||
|
|
@ -958,13 +992,21 @@ fn dired_does_not_adopt_a_foreign_buffer_with_its_name() {
|
|||
// 5 --- read-only discipline
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// An ordinary self-insert is rejected by the intercept and leaves the
|
||||
/// text byte-identical, while dired's own repaint succeeds through
|
||||
/// `bypass_intercept`. `set_round_trip_input` is pinned through the
|
||||
/// An ordinary self-insert is rejected and leaves the text
|
||||
/// byte-identical, while dired's own repaint still succeeds through the
|
||||
/// owner-authorized write. `set_round_trip_input` is pinned through the
|
||||
/// **production** seam a semantic frontend reads (`dispatch_idle_for`,
|
||||
/// published as `DispatchIdle`) rather than by a direct-call assertion:
|
||||
/// without it, a GPU session would optimistically apply `g` as an
|
||||
/// insert instead of letting it reach the revert binding.
|
||||
///
|
||||
/// **This test is NOT coverage of the generated-buffer adoption**, and
|
||||
/// the `contains("read-only")` assertion below is the reason to say so:
|
||||
/// `BufferError::ReadOnly` renders as ``buffer `{name}` (id {id:?}) is
|
||||
/// read-only`` and the intercept's own message ends in `is read-only`
|
||||
/// too, so that substring passes on both sides of the change. What the
|
||||
/// adoption adds here is the explicit `is_read_only` assertion and
|
||||
/// criterion 6(c); the undo criteria are separate tests below.
|
||||
#[test]
|
||||
fn dired_buffer_is_read_only_and_round_trips_input() {
|
||||
let td = fixture_dir();
|
||||
|
|
@ -988,6 +1030,33 @@ fn dired_buffer_is_read_only_and_round_trips_input() {
|
|||
"a round-trip buffer must turn optimistic apply OFF"
|
||||
);
|
||||
|
||||
// Generated-buffer immutability Stage 1 criterion 6(c), the positive
|
||||
// control: `dispatch_idle_for` has SIX ways to return false, so the
|
||||
// assertion above is satisfied by any of them. Switching the same
|
||||
// window to a plain buffer must flip the gate back ON --- a stuck
|
||||
// minibuffer, a pending chord, an open menu or a live search would
|
||||
// keep it off across the switch, so this failing is the signal that
|
||||
// the assertion above passed for the wrong reason.
|
||||
let listing = active_buffer_id(&s);
|
||||
exec(
|
||||
&s,
|
||||
"DIRED_LISTING = pmacs.window.buffer()\n\
|
||||
pmacs.window.switch_buffer(pmacs.buffer.create('*plain*'))",
|
||||
);
|
||||
assert!(
|
||||
s.dispatch_idle_for(FrontendId::LOCAL),
|
||||
"and back ON for a plain buffer"
|
||||
);
|
||||
exec(&s, "pmacs.window.switch_buffer(DIRED_LISTING)");
|
||||
|
||||
// The listing's rope is genuinely locked, not merely intercepted
|
||||
// (generated-buffer immutability Stage 1). Asserted Rust-side
|
||||
// because `describe.buffer` carries no `read_only` field.
|
||||
assert!(
|
||||
is_read_only(&s, listing),
|
||||
"the first paint must leave the listing's rope read-only"
|
||||
);
|
||||
|
||||
// `z` is bound nowhere in dired mode, so it reaches self-insert.
|
||||
type_char(&mut s, 'z');
|
||||
assert_eq!(
|
||||
|
|
@ -1654,3 +1723,277 @@ fn dired_renders_10k_entries_within_200ms() {
|
|||
"10K entries must render within 200ms; took {elapsed:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Generated-buffer immutability, Stage 1
|
||||
// (docs/generated-buffer-immutability-framing.md §6, Stage 1)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// Criterion 3 [`main`] --- neither `C-/` nor `M-x buffer.undo` can empty
|
||||
/// a dired listing.
|
||||
///
|
||||
/// Both halves in one test because they are one claim about one buffer,
|
||||
/// and both are needed: dired rebinds **no** undo chord, so `C-/` is the
|
||||
/// whole distance from a keystroke to an empty listing, while
|
||||
/// `M-x buffer.undo` is the half that no rebinding could ever close.
|
||||
/// The assertion is on the listing's own content --- the header line and
|
||||
/// a real entry --- not on `!is_empty()`.
|
||||
///
|
||||
/// *Bite:* measured on the pre-image --- one undo takes the listing to
|
||||
/// `""`. `scripts/bite githubsucks/main builtin/runtime/dired.lua`
|
||||
/// falsifies it: `paint`'s `bypass_intercept` replace pushed a poppable
|
||||
/// undo entry over a writable rope, and `Buffer::undo` reaches that rope
|
||||
/// through `ensure_writable` without ever consulting the intercept chain.
|
||||
#[test]
|
||||
fn dired_undo_cannot_empty_the_listing() {
|
||||
let td = fixture_dir();
|
||||
let mut s = editor();
|
||||
open_ok(&mut s, td.path(), "nil");
|
||||
let before = active_text(&s);
|
||||
let name = active_name(&s);
|
||||
assert!(
|
||||
before.contains("a.txt") && before.contains(&canon(td.path())),
|
||||
"precondition: a real listing, got {before:?}"
|
||||
);
|
||||
|
||||
ctrl(&mut s, '/');
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
before,
|
||||
"C-/ must leave the listing's content intact"
|
||||
);
|
||||
|
||||
m_x(&mut s, "buffer.undo");
|
||||
assert_eq!(
|
||||
active_name(&s),
|
||||
name,
|
||||
"the minibuffer round trip lands back in the listing"
|
||||
);
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
before,
|
||||
"M-x buffer.undo must leave the listing's content intact"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 4 [fix-shape] --- the owner's own repaint still works after
|
||||
/// the lock, and the listing is still locked afterwards.
|
||||
///
|
||||
/// *Bite:* a bare `set_read_only(true)` would pass criterion 3 and fail
|
||||
/// here, because it refuses the refresh the buffer exists for; the
|
||||
/// falsifying one-line mutation on the shipped primitive is deleting
|
||||
/// `self.read_only = false` from `Buffer::set_generated_contents`
|
||||
/// (`src/buffer.rs:546`), after which `g` raises. Asserted on the content
|
||||
/// the repaint produced, never on the absence of an error.
|
||||
#[test]
|
||||
fn dired_revert_still_repaints_after_the_lock() {
|
||||
let td = fixture_dir();
|
||||
let mut s = editor();
|
||||
open_ok(&mut s, td.path(), "nil");
|
||||
let listing = active_buffer_id(&s);
|
||||
assert!(
|
||||
is_read_only(&s, listing),
|
||||
"precondition: the first paint locked the rope"
|
||||
);
|
||||
assert!(!active_text(&s).contains("c.txt"));
|
||||
|
||||
std::fs::write(td.path().join("c.txt"), b"new\n").expect("write c");
|
||||
type_char(&mut s, 'g');
|
||||
pump(&mut s);
|
||||
|
||||
assert!(
|
||||
active_text(&s).contains("c.txt"),
|
||||
"the owner's repaint must land through the lock: {:?}",
|
||||
active_text(&s)
|
||||
);
|
||||
assert!(
|
||||
is_read_only(&s, listing),
|
||||
"and leave the listing locked afterwards"
|
||||
);
|
||||
}
|
||||
|
||||
/// Stage 1 criterion 5 [`main`, and also fix-shape] — the rope lock
|
||||
/// refuses an ordinary edit first, and the named dired intercept
|
||||
/// survives behind it.
|
||||
///
|
||||
/// The rope half asserts the exact `BufferError::ReadOnly` rendering and
|
||||
/// byte identity. The lifted half distinguishes the intercept by its
|
||||
/// `intercept rejected the edit` message, so deleting `add_intercept`
|
||||
/// still fails with the rope guard intact.
|
||||
#[test]
|
||||
fn dired_rope_lock_and_named_intercept_refuse_in_order() {
|
||||
let td = fixture_dir();
|
||||
let mut s = editor();
|
||||
open_ok(&mut s, td.path(), "nil");
|
||||
let listing = active_buffer_id(&s);
|
||||
let name = active_name(&s);
|
||||
let before = active_text(&s);
|
||||
|
||||
type_char(&mut s, 'z');
|
||||
assert_eq!(
|
||||
status(&s),
|
||||
format!("insert failed: buffer `{name}` (id {listing:?}) is read-only"),
|
||||
"with the lock on, the rope must provide the exact refusal"
|
||||
);
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
before,
|
||||
"the rope refusal leaves every byte unchanged"
|
||||
);
|
||||
|
||||
set_read_only(&s, listing, false);
|
||||
type_char(&mut s, 'z');
|
||||
set_read_only(&s, listing, true);
|
||||
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
before,
|
||||
"the intercept must refuse the edit even with the rope writable"
|
||||
);
|
||||
let st = status(&s);
|
||||
assert!(
|
||||
st.starts_with("insert failed: intercept rejected the edit:")
|
||||
&& st.contains("dired.lua")
|
||||
&& st.contains("is read-only"),
|
||||
"the lifted path must carry the named intercept refusal; got {st:?}"
|
||||
);
|
||||
assert!(
|
||||
!st.contains(&format!("buffer `{name}` (id {listing:?}) is read-only")),
|
||||
"the lifted path must not masquerade as the rope refusal: {st:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// **Stage 1 criterion 7 [mutation]**, the dired half: *a refresh
|
||||
/// reaches the window, not just the rope --- pinned by painting a
|
||||
/// shrinking render (many rows -> one) and asserting row 1 is empty, for
|
||||
/// each adopter.* Bite: *delete the `notify_buffer_edit_to_windows` call
|
||||
/// in the `set_generated_contents` binding
|
||||
/// (`src/lua_bindings/mod.rs:3092`).*
|
||||
///
|
||||
/// The criterion says **for each adopter**, so the listview half is
|
||||
/// `listview_acceptance::s1_7_a_shrinking_refresh_reaches_the_window`.
|
||||
/// This half is the one that carries the framing's bite; the note on the
|
||||
/// listview half records why, and that observation is with PR #188 as a
|
||||
/// revision request rather than being settled here.
|
||||
///
|
||||
/// A rope write is only half of an edit: the window holds a `TextView`
|
||||
/// line index that only `on_edit` maintains, so a write that reaches the
|
||||
/// rope without the fan-out leaves the two disagreeing, and the next
|
||||
/// paint indexes the new rope with the old offsets. `dired.revert`
|
||||
/// paints and does **not** follow the paint with a
|
||||
/// `window.switch_buffer`.
|
||||
#[test]
|
||||
fn dired_a_shrinking_repaint_reaches_the_window() {
|
||||
let td = tempfile::tempdir().expect("tempdir");
|
||||
for name in ["a.txt", "b.txt", "c.txt", "d.txt", "e.txt"] {
|
||||
std::fs::write(td.path().join(name), b"x").expect("write");
|
||||
}
|
||||
let mut s = editor();
|
||||
open_ok(&mut s, td.path(), "nil");
|
||||
let rows = painted_rows(&s);
|
||||
assert!(
|
||||
rows[5].contains("e.txt"),
|
||||
"precondition: five entries paint on rows 1-5, got {:?}",
|
||||
&rows[..7]
|
||||
);
|
||||
|
||||
for name in ["b.txt", "c.txt", "d.txt", "e.txt"] {
|
||||
std::fs::remove_file(td.path().join(name)).expect("remove");
|
||||
}
|
||||
type_char(&mut s, 'g');
|
||||
pump(&mut s);
|
||||
|
||||
let rows = painted_rows(&s);
|
||||
assert!(
|
||||
rows[1].contains("a.txt"),
|
||||
"the one surviving entry paints: {:?}",
|
||||
&rows[..7]
|
||||
);
|
||||
assert_eq!(
|
||||
rows[2],
|
||||
"",
|
||||
"and nothing of the four rows it replaced: {:?}",
|
||||
&rows[..7]
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 13a [`main`] --- a locked generated buffer is not foldable
|
||||
/// (Q#GB16, option (a)).
|
||||
///
|
||||
/// This is a regression pin **for the intended change**: sweep C found
|
||||
/// that `document_bytes` (`src/lua_bindings/fold.rs`) is spelled
|
||||
/// `if buffer.is_read_only() { return Ok(None) }`, so locking dired
|
||||
/// listings silently disables fold *creation* on them. The decision is to
|
||||
/// accept that --- a generated buffer's contents are replaced wholesale
|
||||
/// on every refresh, which invalidates any stored range --- and to state
|
||||
/// it rather than let it ship silently.
|
||||
///
|
||||
/// *Bite:* this **fails on `main`**, where the same call returns `true`.
|
||||
/// `scripts/bite githubsucks/main builtin/runtime/dired.lua` falsifies
|
||||
/// it.
|
||||
#[test]
|
||||
fn dired_a_locked_listing_is_not_foldable() {
|
||||
let td = fixture_dir();
|
||||
let mut s = editor();
|
||||
open_ok(&mut s, td.path(), "nil");
|
||||
let text = active_text(&s);
|
||||
let first_nl = text.find('\n').expect("header line");
|
||||
let second_nl = text[first_nl + 1..]
|
||||
.find('\n')
|
||||
.map(|i| first_nl + 1 + i)
|
||||
.expect("at least two entry lines");
|
||||
|
||||
let folded: bool = eval(
|
||||
&s,
|
||||
&format!(
|
||||
"return pmacs.fold.fold(pmacs.window.buffer(), \
|
||||
{{ start = {first_nl}, ['end'] = {second_nl} }})"
|
||||
),
|
||||
);
|
||||
|
||||
assert!(!folded, "a locked generated buffer is not foldable");
|
||||
let n: i64 = eval(&s, "return #pmacs.fold.folds(pmacs.window.buffer())");
|
||||
assert_eq!(n, 0, "and no fold is stored");
|
||||
}
|
||||
|
||||
/// Criterion 13b [mutation] --- ...and the refusal says why.
|
||||
///
|
||||
/// Separate from 13a because the two halves have different pre-images:
|
||||
/// on `main` the call *succeeds* and sets no status at all, so this
|
||||
/// cannot share 13a's `main` pre-image. The shape that ships if 13a is
|
||||
/// written alone is correct behaviour with a false explanation --- the
|
||||
/// guard's author meant "terminal", and "not a document buffer" is not
|
||||
/// true of a dired listing.
|
||||
///
|
||||
/// *Bite:* revert `src/lua_bindings/fold.rs`'s status string to
|
||||
/// `"fold rejected: not a document buffer"`.
|
||||
#[test]
|
||||
fn dired_the_fold_refusal_names_the_read_only_lock() {
|
||||
let td = fixture_dir();
|
||||
let mut s = editor();
|
||||
open_ok(&mut s, td.path(), "nil");
|
||||
let text = active_text(&s);
|
||||
let first_nl = text.find('\n').expect("header line");
|
||||
let second_nl = text[first_nl + 1..]
|
||||
.find('\n')
|
||||
.map(|i| first_nl + 1 + i)
|
||||
.expect("at least two entry lines");
|
||||
|
||||
let _: bool = eval(
|
||||
&s,
|
||||
&format!(
|
||||
"return pmacs.fold.fold(pmacs.window.buffer(), \
|
||||
{{ start = {first_nl}, ['end'] = {second_nl} }})"
|
||||
),
|
||||
);
|
||||
|
||||
let st = status(&s);
|
||||
assert!(
|
||||
st.contains("read-only"),
|
||||
"the refusal must name the lock; got {st:?}"
|
||||
);
|
||||
assert!(
|
||||
!st.contains("not a document buffer"),
|
||||
"and not the sentence that is no longer true; got {st:?}"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,8 +7,15 @@
|
|||
//! substrate hermetically.
|
||||
//!
|
||||
//! Framing: docs/lsp-panels-framing.md.
|
||||
//!
|
||||
//! Generated-buffer immutability Stage 1
|
||||
//! (docs/generated-buffer-immutability-framing.md §6) adds the
|
||||
//! criteria below `refresh_reruns_the_source_and_reseats`: the undo
|
||||
//! paths the Q#P3 intercept never guarded, the Q#GB13 ownership rule,
|
||||
//! and the Q#GB18 identity routing that ownership makes load-bearing.
|
||||
|
||||
use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers};
|
||||
use pmacs::buffer::BufferId;
|
||||
use pmacs::editor::EditorState;
|
||||
use pmacs::protocol::FrontendId;
|
||||
|
||||
|
|
@ -25,6 +32,142 @@ fn press(s: &mut EditorState, code: KeyCode) {
|
|||
s.dispatch_key(FrontendId::LOCAL, key(code, KeyModifiers::NONE));
|
||||
}
|
||||
|
||||
fn ctrl(s: &mut EditorState, c: char) {
|
||||
s.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
key(KeyCode::Char(c), KeyModifiers::CONTROL),
|
||||
);
|
||||
}
|
||||
|
||||
fn alt(s: &mut EditorState, c: char) {
|
||||
s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Char(c), KeyModifiers::ALT));
|
||||
}
|
||||
|
||||
fn type_str(s: &mut EditorState, text: &str) {
|
||||
for ch in text.chars() {
|
||||
s.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
key(KeyCode::Char(ch), KeyModifiers::NONE),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// `M-x <name> RET` through the **real** minibuffer, not
|
||||
/// `pmacs.command.invoke`: `buffer.undo` is reachable that way on every
|
||||
/// buffer in the tree and no buffer-local rebinding can remove it, which
|
||||
/// is the whole reason the intercept idiom did not close this hole.
|
||||
fn m_x(s: &mut EditorState, name: &str) {
|
||||
alt(s, 'x');
|
||||
type_str(s, name);
|
||||
press(s, KeyCode::Enter);
|
||||
}
|
||||
|
||||
fn exec(s: &EditorState, src: &str) {
|
||||
s.lua_host.lua().load(src.to_string()).exec().unwrap();
|
||||
}
|
||||
|
||||
fn eval<T: mlua::FromLuaMulti>(s: &EditorState, src: &str) -> T {
|
||||
s.lua_host.lua().load(src.to_string()).eval().unwrap()
|
||||
}
|
||||
|
||||
fn status(s: &EditorState) -> String {
|
||||
s.core.borrow().status.clone()
|
||||
}
|
||||
|
||||
fn buffer_names(s: &EditorState) -> Vec<String> {
|
||||
eval(
|
||||
s,
|
||||
"local out = {}\n\
|
||||
for _, id in ipairs(pmacs.buffer.list()) do\n\
|
||||
out[#out + 1] = pmacs.describe.buffer(id).name\n\
|
||||
end\n\
|
||||
return out",
|
||||
)
|
||||
}
|
||||
|
||||
fn active_name(s: &EditorState) -> String {
|
||||
eval(
|
||||
s,
|
||||
"return pmacs.describe.buffer(pmacs.window.buffer()).name",
|
||||
)
|
||||
}
|
||||
|
||||
fn active_text(s: &EditorState) -> String {
|
||||
eval(
|
||||
s,
|
||||
"local b = pmacs.window.buffer()\nreturn b:slice(0, b:len())",
|
||||
)
|
||||
}
|
||||
|
||||
fn id_of(s: &EditorState, name: &str) -> BufferId {
|
||||
let core = s.core.borrow();
|
||||
let reg = core.registry.borrow();
|
||||
reg.find_by_name(name)
|
||||
.unwrap_or_else(|| panic!("no buffer named {name:?} in {:?}", buffer_names(s)))
|
||||
}
|
||||
|
||||
/// Q#GB14: the rope lock is not observable from Lua --- `describe.buffer`
|
||||
/// carries `name`, `length`, `modified`, `view_count` and nothing else ---
|
||||
/// so every "is it locked" assertion goes through Rust.
|
||||
fn is_read_only(s: &EditorState, id: BufferId) -> bool {
|
||||
let core = s.core.borrow();
|
||||
let reg = core.registry.borrow();
|
||||
reg.get(id).expect("buffer in registry").is_read_only()
|
||||
}
|
||||
|
||||
fn set_read_only(s: &EditorState, id: BufferId, value: bool) {
|
||||
let core = s.core.borrow();
|
||||
let mut reg = core.registry.borrow_mut();
|
||||
reg.get_mut(id)
|
||||
.expect("buffer in registry")
|
||||
.set_read_only(value);
|
||||
}
|
||||
|
||||
/// Render the active window's text view into a cell grid. Criterion 7 is
|
||||
/// pinned by PAINTING, because that is where a rope/window disagreement
|
||||
/// bites: the rope is right and the screen is not.
|
||||
fn paint_active_window(s: &EditorState, rows: u32, cols: u32) -> Vec<pmacs::cell::Cell> {
|
||||
use pmacs::cell::{Cell, CellGrid, CellSize};
|
||||
use pmacs::view::{View, Viewport};
|
||||
use pmacs::window::Rect;
|
||||
|
||||
let mut core = s.core.borrow_mut();
|
||||
let active = core.active_window_id();
|
||||
let registry = core.registry.clone();
|
||||
let win = core.windows.get_mut(&active).expect("active window");
|
||||
let rect = Rect::new(0, 0, rows, cols);
|
||||
let mut backing = vec![Cell::default(); (rows * cols) as usize];
|
||||
let reg = registry.borrow();
|
||||
let buf = reg.get(win.buffer_id).expect("buffer in registry");
|
||||
let viewport = Viewport {
|
||||
buffer_start: 0,
|
||||
buffer_end: buf.len(),
|
||||
cell_origin: rect.origin,
|
||||
cell_size: CellSize::new(rows, cols),
|
||||
gutter_w: 0,
|
||||
folds: None,
|
||||
};
|
||||
let mut grid = CellGrid {
|
||||
cells: &mut backing,
|
||||
stride: cols,
|
||||
size: CellSize::new(rows, cols),
|
||||
};
|
||||
win.text_view.render(buf, viewport, &mut grid);
|
||||
backing
|
||||
}
|
||||
|
||||
fn grid_row(cells: &[pmacs::cell::Cell], row: u32, cols: u32) -> String {
|
||||
use pmacs::cell::Glyph;
|
||||
(0..cols)
|
||||
.map(|c| match cells[(row * cols + c) as usize].glyph {
|
||||
Glyph::Char(ch) => ch,
|
||||
_ => ' ',
|
||||
})
|
||||
.collect::<String>()
|
||||
.trim_end()
|
||||
.to_owned()
|
||||
}
|
||||
|
||||
/// Open a three-row test panel whose visits record into `_G.VISITED`.
|
||||
fn open_test_panel(s: &mut EditorState) {
|
||||
s.lua_host
|
||||
|
|
@ -136,3 +279,479 @@ fn refresh_reruns_the_source_and_reseats() {
|
|||
let (_, _, _, visited) = probe(&s);
|
||||
assert_eq!(visited.as_deref(), Some("D"), "the refreshed row visits");
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Generated-buffer immutability, Stage 1
|
||||
// (docs/generated-buffer-immutability-framing.md §6, Stage 1)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// The exact bytes `open_test_panel` renders. Asserted by value, not by
|
||||
/// `is_empty()`: "the panel is not empty" is the assertion shape the
|
||||
/// framing's §0.1 shows passing with the bug live on other families.
|
||||
const PANEL_TEXT: &str = "3 items RET visit q quit\nalpha\nbeta\ngamma";
|
||||
|
||||
/// Criterion 1 [`main`] --- `C-/` cannot empty a listview panel.
|
||||
///
|
||||
/// Driven by a real chord through `dispatch_key`, because listview
|
||||
/// rebinds **no** undo chord (`grep -n 'C-/\|C-_\|C-x u\|undo'
|
||||
/// builtin/runtime/listview.lua` is empty), so this is the whole
|
||||
/// distance from a keystroke to an empty panel.
|
||||
///
|
||||
/// *Bite:* measured on the pre-image --- `"H\nrow-one\nrow-two"` -> `""`.
|
||||
/// `scripts/bite githubsucks/main builtin/runtime/listview.lua` falsifies
|
||||
/// it: the panel's own render pushes a poppable undo entry, and
|
||||
/// `Buffer::undo` reaches the rope through `ensure_writable` without ever
|
||||
/// consulting the intercept chain.
|
||||
#[test]
|
||||
fn s1_1_the_undo_chord_cannot_empty_a_listview_panel() {
|
||||
let mut s = EditorState::new();
|
||||
open_test_panel(&mut s);
|
||||
assert_eq!(active_text(&s), PANEL_TEXT, "precondition: rendered");
|
||||
|
||||
ctrl(&mut s, '/');
|
||||
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
PANEL_TEXT,
|
||||
"C-/ must leave the panel's content intact"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 2 [`main`] --- `M-x buffer.undo` cannot empty a listview
|
||||
/// panel, driven through the **real** minibuffer.
|
||||
///
|
||||
/// Separate from criterion 1 on purpose: a fix that only rebound the
|
||||
/// chords would pass 1 and fail this. `compile.lua`'s own comment already
|
||||
/// concedes the point ("command/menu undo stays dispatchable").
|
||||
///
|
||||
/// *Bite:* same empty result on the pre-image.
|
||||
#[test]
|
||||
fn s1_2_m_x_buffer_undo_cannot_empty_a_listview_panel() {
|
||||
let mut s = EditorState::new();
|
||||
open_test_panel(&mut s);
|
||||
|
||||
m_x(&mut s, "buffer.undo");
|
||||
|
||||
assert_eq!(
|
||||
active_name(&s),
|
||||
"*test-panel*",
|
||||
"the minibuffer round trip must land back in the panel"
|
||||
);
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
PANEL_TEXT,
|
||||
"M-x buffer.undo must leave the panel's content intact"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 4 [fix-shape] --- the owner's own refresh still works after
|
||||
/// the lock, and the buffer is still locked afterwards.
|
||||
///
|
||||
/// *Bite:* a naive `set_read_only(true)` at panel creation passes
|
||||
/// criteria 1-3 and fails here, because it refuses the refresh the panel
|
||||
/// exists for. That is the failure mode `Buffer::set_generated_contents`
|
||||
/// exists to prevent, and it is why the **pairing** is the primitive.
|
||||
/// The assertion is on the content `g` produced, not on the call not
|
||||
/// raising.
|
||||
#[test]
|
||||
fn s1_4_the_owners_refresh_still_works_after_the_lock() {
|
||||
let mut s = EditorState::new();
|
||||
open_test_panel(&mut s);
|
||||
let panel = id_of(&s, "*test-panel*");
|
||||
assert!(
|
||||
is_read_only(&s, panel),
|
||||
"precondition: the first render locked the rope"
|
||||
);
|
||||
|
||||
press(&mut s, KeyCode::Char('g'));
|
||||
|
||||
let text = active_text(&s);
|
||||
assert!(text.contains("delta"), "g must re-render: {text:?}");
|
||||
assert!(
|
||||
!text.contains("alpha"),
|
||||
"and replace the old rows: {text:?}"
|
||||
);
|
||||
assert!(
|
||||
is_read_only(&s, panel),
|
||||
"and the panel must still be locked afterwards"
|
||||
);
|
||||
}
|
||||
|
||||
/// Stage 1 criterion 5 [`main`, and also fix-shape] — the rope lock
|
||||
/// refuses an ordinary edit first, and the named intercept survives
|
||||
/// behind it.
|
||||
///
|
||||
/// Both halves are required. The first asserts the exact
|
||||
/// `BufferError::ReadOnly` rendering and byte identity. The second lifts
|
||||
/// the lock Rust-side and distinguishes the intercept by its
|
||||
/// `intercept rejected the edit` message. Deleting `add_intercept`
|
||||
/// therefore passes the rope half and fails the lifted half.
|
||||
#[test]
|
||||
fn s1_5_the_rope_lock_and_named_intercept_refuse_in_order() {
|
||||
let mut s = EditorState::new();
|
||||
open_test_panel(&mut s);
|
||||
let panel = id_of(&s, "*test-panel*");
|
||||
let before = active_text(&s);
|
||||
|
||||
press(&mut s, KeyCode::Char('z'));
|
||||
assert_eq!(
|
||||
status(&s),
|
||||
format!("insert failed: buffer `*test-panel*` (id {panel:?}) is read-only"),
|
||||
"with the lock on, the rope must provide the exact refusal"
|
||||
);
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
before,
|
||||
"the rope refusal leaves every byte unchanged"
|
||||
);
|
||||
|
||||
set_read_only(&s, panel, false);
|
||||
press(&mut s, KeyCode::Char('z'));
|
||||
set_read_only(&s, panel, true);
|
||||
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
PANEL_TEXT,
|
||||
"the intercept must refuse the edit even with the rope writable"
|
||||
);
|
||||
let st = status(&s);
|
||||
assert!(
|
||||
st.starts_with("insert failed: intercept rejected the edit:")
|
||||
&& st.contains("listview.lua")
|
||||
&& st.contains("*test-panel* is read-only"),
|
||||
"the lifted path must carry the named intercept refusal; got {st:?}"
|
||||
);
|
||||
assert!(
|
||||
!st.contains(&format!(
|
||||
"buffer `*test-panel*` (id {panel:?}) is read-only"
|
||||
)),
|
||||
"the lifted path must not masquerade as the rope refusal: {st:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 6 [fix-shape] --- `set_round_trip_input` survives adoption,
|
||||
/// asserted so that only the round-trip mark can make it pass.
|
||||
///
|
||||
/// `dispatch_idle_for` (`src/editor.rs:1126-1155`) returns `false` for
|
||||
/// **six** independent reasons, so `!dispatch_idle_for(..)` alone is
|
||||
/// satisfied by any of them. All three halves are required:
|
||||
///
|
||||
/// * **(a)** the document-window premise (`!window.is_side()`), so a
|
||||
/// fixture that later displays the panel in a side window fails loudly
|
||||
/// rather than passing vacuously --- `tests/dired_acceptance.rs:975`'s
|
||||
/// shape;
|
||||
/// * **(b)** the gate itself while the panel is focused;
|
||||
/// * **(c)** the positive control --- switching the same window to a
|
||||
/// plain buffer must flip the gate back to `true`. A stuck minibuffer,
|
||||
/// a pending chord, an open menu or a live search would keep it `false`
|
||||
/// across the switch, so (c) failing is the signal that (b) passed for
|
||||
/// the wrong reason.
|
||||
///
|
||||
/// *Bite:* delete the `set_round_trip_input` call in `listview.lua` and
|
||||
/// criteria 1-5 all still pass; only (b) fails. A daemon-side rope
|
||||
/// refusal does nothing for a replica's own mirror, which is why this is
|
||||
/// pinned through `dispatch_idle_for` rather than through `read_only`.
|
||||
#[test]
|
||||
fn s1_6_round_trip_input_survives_the_adoption() {
|
||||
let mut s = EditorState::new();
|
||||
open_test_panel(&mut s);
|
||||
|
||||
// (a) the premise.
|
||||
{
|
||||
let core = s.core.borrow();
|
||||
let active = core.active_window_id();
|
||||
assert!(
|
||||
!core.windows.get(&active).expect("live window").is_side(),
|
||||
"fixture premise: the panel is in a document window here"
|
||||
);
|
||||
}
|
||||
// (b) the gate.
|
||||
assert!(
|
||||
!s.dispatch_idle_for(FrontendId::LOCAL),
|
||||
"a round-trip buffer must turn optimistic apply OFF"
|
||||
);
|
||||
// (c) the positive control.
|
||||
exec(
|
||||
&s,
|
||||
"pmacs.window.switch_buffer(pmacs.buffer.create('*plain*'))",
|
||||
);
|
||||
assert!(
|
||||
s.dispatch_idle_for(FrontendId::LOCAL),
|
||||
"and back ON for a plain buffer --- otherwise (b) passed for one \
|
||||
of the other five reasons"
|
||||
);
|
||||
}
|
||||
|
||||
/// **Stage 1 criterion 7 [mutation]**, the listview half: *a refresh
|
||||
/// reaches the window, not just the rope --- pinned by painting a
|
||||
/// shrinking render (many rows -> one) and asserting row 1 is empty, for
|
||||
/// each adopter.* Bite: *delete the `notify_buffer_edit_to_windows` call
|
||||
/// in the `set_generated_contents` binding
|
||||
/// (`src/lua_bindings/mod.rs:3092`).*
|
||||
///
|
||||
/// The criterion says **for each adopter**, so both halves exist; the
|
||||
/// dired half is `dired_acceptance::dired_a_shrinking_repaint_reaches_the_window`.
|
||||
///
|
||||
/// `listview.refresh` re-seats through the already-notified `TextView`;
|
||||
/// it deliberately does not rebuild the view by switching to the buffer
|
||||
/// it already shows. Deleting the notification fan-out therefore leaves
|
||||
/// the old line index live and this paint assertion bites.
|
||||
#[test]
|
||||
fn s1_7_a_shrinking_refresh_reaches_the_window() {
|
||||
let mut s = EditorState::new();
|
||||
open_test_panel(&mut s);
|
||||
let painted = paint_active_window(&s, 6, 24);
|
||||
assert_eq!(
|
||||
grid_row(&painted, 1, 24),
|
||||
"alpha",
|
||||
"precondition: three data rows paint"
|
||||
);
|
||||
assert_eq!(grid_row(&painted, 3, 24), "gamma");
|
||||
|
||||
// `g` re-renders from three rows to one.
|
||||
press(&mut s, KeyCode::Char('g'));
|
||||
|
||||
let painted = paint_active_window(&s, 6, 24);
|
||||
assert_eq!(
|
||||
grid_row(&painted, 1, 24),
|
||||
"delta",
|
||||
"the refreshed row must paint"
|
||||
);
|
||||
assert_eq!(
|
||||
grid_row(&painted, 2, 24),
|
||||
"",
|
||||
"and nothing of the rows it replaced"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 9 [`main`] --- a foreign buffer that happens to share the
|
||||
/// panel's name is never adopted (Q#GB13).
|
||||
///
|
||||
/// Both halves, because the second is what fails if adoption is merely
|
||||
/// made "safe" by skipping the render: the user's bytes survive **and**
|
||||
/// an ordinary edit to the user's buffer still lands. Adoption installed
|
||||
/// an erroring intercept whose handle it discarded, so the pre-image left
|
||||
/// the clobbered buffer permanently un-editable --- and this arc removes
|
||||
/// the `M-x buffer.undo` that was the only way back.
|
||||
///
|
||||
/// *Bite:* measured on the pre-image --- `"my precious notes"` ->
|
||||
/// `"H\nr1"`, one buffer where there should be two, and the user's buffer
|
||||
/// left un-editable. `scripts/bite githubsucks/main
|
||||
/// builtin/runtime/listview.lua` falsifies it.
|
||||
#[test]
|
||||
fn s1_9_a_foreign_buffer_with_the_panels_name_is_never_adopted() {
|
||||
let mut s = EditorState::new();
|
||||
exec(
|
||||
&s,
|
||||
"FOREIGN = pmacs.buffer.create('*test-panel*')\n\
|
||||
FOREIGN:insert(0, 'my precious notes')",
|
||||
);
|
||||
|
||||
open_test_panel(&mut s);
|
||||
|
||||
let foreign: String = eval(&s, "return FOREIGN:slice(0, FOREIGN:len())");
|
||||
assert_eq!(
|
||||
foreign, "my precious notes",
|
||||
"the user's bytes must survive the panel opening"
|
||||
);
|
||||
let landed: String = eval(
|
||||
&s,
|
||||
"FOREIGN:insert(0, 'still mine: ')\n\
|
||||
return FOREIGN:slice(0, FOREIGN:len())",
|
||||
);
|
||||
assert_eq!(
|
||||
landed, "still mine: my precious notes",
|
||||
"and an ordinary edit to it must still land"
|
||||
);
|
||||
assert_eq!(
|
||||
active_name(&s),
|
||||
"*test-panel*<2>",
|
||||
"the panel opens under a disambiguated name"
|
||||
);
|
||||
let names = buffer_names(&s);
|
||||
assert!(
|
||||
names.iter().any(|n| n == "*test-panel*") && names.iter().any(|n| n == "*test-panel*<2>"),
|
||||
"two buffers, not one: {names:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 10 [fix-shape] --- exhausting the disambiguation limit
|
||||
/// raises rather than falling back to adoption, matching
|
||||
/// `dired.lua:493-503` and `terminal.lua:309-315`.
|
||||
///
|
||||
/// *Bite:* an implementation that adopts once `<99>` is taken passes
|
||||
/// criterion 9 and fails here --- and it fails in the worst direction,
|
||||
/// because the buffer it would adopt is by construction one a user
|
||||
/// created.
|
||||
#[test]
|
||||
fn s1_10_the_disambiguation_limit_raises_rather_than_adopting() {
|
||||
let s = EditorState::new();
|
||||
exec(
|
||||
&s,
|
||||
"MINE = pmacs.buffer.create('*test-panel*')\n\
|
||||
MINE:insert(0, 'mine')\n\
|
||||
for i = 2, 99 do pmacs.buffer.create(string.format('*test-panel*<%d>', i)) end",
|
||||
);
|
||||
|
||||
let (ok, err): (bool, String) = eval(
|
||||
&s,
|
||||
"local ok, err = pcall(pmacs.listview.open, { name = '*test-panel*', rows = {} })\n\
|
||||
return ok, tostring(err)",
|
||||
);
|
||||
|
||||
assert!(!ok, "the open must raise, not adopt");
|
||||
assert!(
|
||||
err.contains("no free variant remains"),
|
||||
"and say why; got {err:?}"
|
||||
);
|
||||
let mine: String = eval(&s, "return MINE:slice(0, MINE:len())");
|
||||
assert_eq!(mine, "mine", "and touch nothing");
|
||||
}
|
||||
|
||||
/// Stage 1 criterion 11 [`main`] — a **disambiguated** panel still answers
|
||||
/// `RET`, `g` and `q` (Q#GB18). The framing labels it `[main]` and names
|
||||
/// its bite as *Q#GB13 landed without Q#GB18*.
|
||||
///
|
||||
/// On `main` the test first fails at the disambiguation premise because
|
||||
/// ownership is absent. The framing's narrower pre-image is also pinned:
|
||||
/// keep disambiguation but restore a name-keyed `panel_for_buffer`, and
|
||||
/// the test reaches the consumer checks and fails at `g`.
|
||||
///
|
||||
/// Disambiguation alone leaves the old lookup reading
|
||||
/// `panels["*test-panel*<2>"]` for a record stored under
|
||||
/// `"*test-panel*"`, so all three commands return early. Every one of
|
||||
/// them fails **silently**, so the assertion is on the content each
|
||||
/// command produced, never on "it did not raise".
|
||||
#[test]
|
||||
fn s1_11_a_disambiguated_panel_still_answers_ret_g_and_q() {
|
||||
let mut s = EditorState::new();
|
||||
exec(
|
||||
&s,
|
||||
"FOREIGN = pmacs.buffer.create('*test-panel*')\n\
|
||||
ORIGIN = pmacs.buffer.create('*origin*')\n\
|
||||
pmacs.window.switch_buffer(ORIGIN)",
|
||||
);
|
||||
open_test_panel(&mut s);
|
||||
assert_eq!(active_name(&s), "*test-panel*<2>", "premise: disambiguated");
|
||||
|
||||
// g --- re-render from the data source.
|
||||
press(&mut s, KeyCode::Char('g'));
|
||||
let text = active_text(&s);
|
||||
assert!(text.contains("delta"), "g must re-render: {text:?}");
|
||||
|
||||
// RET --- fire on_visit for the row under the cursor.
|
||||
press(&mut s, KeyCode::Enter);
|
||||
let visited: Option<String> = eval(&s, "return _G.VISITED");
|
||||
assert_eq!(
|
||||
visited.as_deref(),
|
||||
Some("D"),
|
||||
"RET must visit the refreshed row"
|
||||
);
|
||||
|
||||
// q --- restore the buffer the panel was opened from.
|
||||
press(&mut s, KeyCode::Char('q'));
|
||||
assert_eq!(
|
||||
active_name(&s),
|
||||
"*origin*",
|
||||
"q must restore the previous buffer"
|
||||
);
|
||||
}
|
||||
|
||||
/// Stage 1 criterion 12 [`main`] — the `q`-target capture is not inverted
|
||||
/// (Q#GB18), which needs its own criterion because it fails **open**
|
||||
/// rather than closed. The framing labels it `[main]`.
|
||||
///
|
||||
/// On `main` the ownership premise fails first. With disambiguation kept
|
||||
/// and only `panel_for_buffer` restored to name-keyed lookup, the test
|
||||
/// reaches and fails the `q`-target assertion the criterion exists for.
|
||||
///
|
||||
/// `listview.open`'s guard reads "capture the current buffer as the `q`
|
||||
/// target, but never another panel (chained panels would trap `q` in a
|
||||
/// loop)". When the lookup cannot recognise a disambiguated panel it
|
||||
/// returns nil, the guard reads "not a panel", and the panel is captured
|
||||
/// as the next panel's `q` target --- producing exactly the loop the
|
||||
/// guard exists to prevent. Criterion 11 passes with that bug live,
|
||||
/// because each command works in isolation; only the two-panel sequence
|
||||
/// shows it.
|
||||
///
|
||||
/// *Bite:* restore the name-keyed `panels[d.name]` lookup while keeping
|
||||
/// the disambiguation and `q` lands back in `*test-panel*<2>`.
|
||||
#[test]
|
||||
fn s1_12_the_q_target_capture_is_not_inverted_across_two_panels() {
|
||||
let mut s = EditorState::new();
|
||||
exec(
|
||||
&s,
|
||||
"FOREIGN = pmacs.buffer.create('*test-panel*')\n\
|
||||
ORIGIN = pmacs.buffer.create('*origin*')\n\
|
||||
pmacs.window.switch_buffer(ORIGIN)",
|
||||
);
|
||||
open_test_panel(&mut s);
|
||||
assert_eq!(active_name(&s), "*test-panel*<2>", "premise: disambiguated");
|
||||
|
||||
exec(
|
||||
&s,
|
||||
"pmacs.listview.open { name = '*other-panel*', header = 'O', \
|
||||
rows = { { text = 'x', item = 'X' } } }",
|
||||
);
|
||||
assert_eq!(active_name(&s), "*other-panel*", "premise: second panel");
|
||||
|
||||
press(&mut s, KeyCode::Char('q'));
|
||||
|
||||
assert_ne!(
|
||||
active_name(&s),
|
||||
"*test-panel*<2>",
|
||||
"q must never return into another panel --- the chained-panel loop"
|
||||
);
|
||||
assert_eq!(
|
||||
active_name(&s),
|
||||
"*scratch*",
|
||||
"with no capturable previous buffer, q falls back to *scratch*"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 14 [structural] --- rides **alongside** 1-13, never instead:
|
||||
/// a structural comparison of two authorities does not catch a misrouted
|
||||
/// consumer, which is why 11 and 12 assert through `dispatch_key`.
|
||||
///
|
||||
/// Three claims, each keyed to a decision: no `bypass_intercept` write
|
||||
/// survives in either Stage 1 adopter (§1.1's arithmetic is the
|
||||
/// reference, so the check is per non-comment line rather than a
|
||||
/// substring sweep that the explanatory comments would trip);
|
||||
/// `ensure_panel` contains no find-by-name adoption (Q#GB13); and every
|
||||
/// `panels[` subscript is an append, so none can be keyed by a name
|
||||
/// derived from `describe.buffer` (Q#GB18).
|
||||
#[test]
|
||||
fn s1_14_no_bypass_write_or_name_keyed_identity_remains() {
|
||||
const LISTVIEW: &str = include_str!("../builtin/runtime/listview.lua");
|
||||
const DIRED: &str = include_str!("../builtin/runtime/dired.lua");
|
||||
|
||||
for (file, src) in [("listview.lua", LISTVIEW), ("dired.lua", DIRED)] {
|
||||
let writes: Vec<&str> = src
|
||||
.lines()
|
||||
.filter(|l| !l.trim_start().starts_with("--") && l.contains("bypass_intercept"))
|
||||
.collect();
|
||||
assert!(
|
||||
writes.is_empty(),
|
||||
"{file} must contain no bypass_intercept write; found {writes:?}"
|
||||
);
|
||||
assert!(
|
||||
src.contains("set_generated_contents"),
|
||||
"{file} must write through the authorized primitive"
|
||||
);
|
||||
}
|
||||
|
||||
assert!(
|
||||
!LISTVIEW.contains("find_buffer_by_name(name) or pmacs.buffer.create"),
|
||||
"ensure_panel must not adopt a same-named foreign buffer"
|
||||
);
|
||||
let subscripts: Vec<&str> = LISTVIEW
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|line| !line.starts_with("--") && line.contains("panels["))
|
||||
.filter(|line| !line.starts_with("panels[#panels + 1]"))
|
||||
.collect();
|
||||
assert!(
|
||||
subscripts.is_empty(),
|
||||
"every `panels[` subscript must be an append; found {subscripts:?}"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8549,17 +8549,26 @@ fn rd8_recursive_delete_refuses_for_a_modified_descendant() {
|
|||
assert!(tree.exists(), "including the directory itself");
|
||||
}
|
||||
|
||||
/// Criterion 9 — a *clean* recursive delete leaves descendant buffers
|
||||
/// orphaned, not removed.
|
||||
/// Criterion 9 — a *clean* recursive delete reconciles descendant
|
||||
/// buffers, through both removal phases.
|
||||
///
|
||||
/// This pin deliberately asserts today's imperfect behaviour. Widening
|
||||
/// reconciliation to the tree would route N buffers through
|
||||
/// `remove_buffer_and_fire` — phase 2 without phase 1 — promoting the
|
||||
/// parked dangling-window defect from exact-path to tree-wide.
|
||||
/// **Rewritten by dired Stage 2a** (`docs/dired-stage2-framing.md` §6,
|
||||
/// Q#RD27 / acceptance 23). This row previously pinned the opposite —
|
||||
/// that the descendant buffer stayed orphaned — and gave the reason:
|
||||
/// widening reconciliation would have routed N buffers through
|
||||
/// `remove_buffer_and_fire`, which is phase 2 *without* phase 1, so a
|
||||
/// tree delete would have left up to N windows pointing at removed ids.
|
||||
/// That constraint is discharged: `EditorCore::reconcile_delete`
|
||||
/// composes the same two phases `pmacs.buffer.kill` composes, and the
|
||||
/// delete arm routes through it. The old assertion is not merely
|
||||
/// obsolete, it is now the defect — an orphaned buffer whose next
|
||||
/// `C-x C-s` recreates a file the user deleted.
|
||||
///
|
||||
/// Bite: fails against an implementation that widens reconciliation.
|
||||
/// Bite, both directions: fails against an exact-path reconciliation
|
||||
/// (the descendant survives) **and** against a widening that skips
|
||||
/// phase 1 (a window keeps a removed id).
|
||||
#[test]
|
||||
fn rd9_clean_recursive_delete_leaves_descendants_orphaned() {
|
||||
fn rd9_clean_recursive_delete_reconciles_descendants_through_both_phases() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let tree = dir.path().join("tree");
|
||||
std::fs::create_dir(&tree).expect("mkdir");
|
||||
|
|
@ -8568,6 +8577,13 @@ fn rd9_clean_recursive_delete_leaves_descendants_orphaned() {
|
|||
|
||||
let mut state = pmacs::editor::EditorState::new();
|
||||
rd_open(&mut state, "B", &inner);
|
||||
// Display it, so the phase-1 window redirect has something to do.
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load("pmacs.window.switch_buffer(B)")
|
||||
.exec()
|
||||
.expect("show the descendant");
|
||||
|
||||
let (ok, err) = rd_delete(&mut state, &tree, ", recursive = true");
|
||||
assert!(ok, "a clean tree deletes: {err}");
|
||||
|
|
@ -8580,9 +8596,23 @@ fn rd9_clean_recursive_delete_leaves_descendants_orphaned() {
|
|||
.eval()
|
||||
.expect("validity probe");
|
||||
assert!(
|
||||
still,
|
||||
"THE BITE: reconciliation stays exact-path, so the descendant \
|
||||
buffer is orphaned rather than removed"
|
||||
!still,
|
||||
"THE BITE: a buffer under a recursively deleted directory must be \
|
||||
reconciled away, not left bound to a path whose file is gone"
|
||||
);
|
||||
|
||||
let core = state.core.borrow();
|
||||
let dangling: Vec<_> = core
|
||||
.windows
|
||||
.iter()
|
||||
.filter(|(_, w)| !core.registry.borrow().contains(w.buffer_id))
|
||||
.map(|(id, w)| (*id, w.buffer_id))
|
||||
.collect();
|
||||
assert!(
|
||||
dangling.is_empty(),
|
||||
"THE OTHER HALF: widening the reconciliation must not promote the \
|
||||
dangling-window defect from exact-path to tree-wide; dangling: \
|
||||
{dangling:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -8633,15 +8663,22 @@ fn rd10_absent_plus_ignore_does_not_destroy_a_modified_buffer() {
|
|||
assert_eq!(text, "unsavedcontent\n", "the unsaved edit survives");
|
||||
}
|
||||
|
||||
/// Criterion 14 — clean duplicates: exactly one match reconciled.
|
||||
/// Criterion 14 — clean duplicates: **every** match reconciled.
|
||||
///
|
||||
/// Bite: fails against an implementation that removes **all** matches.
|
||||
/// It pins the reconciliation half of Q#RD10 and *only* that: with both
|
||||
/// buffers clean there is no verdict difference between consulting one
|
||||
/// match and consulting all, so this setup cannot see validation
|
||||
/// breadth. Criterion 6 is what detects incomplete validation.
|
||||
/// **Rewritten by dired Stage 2a** (§6, acceptance 23). This row
|
||||
/// previously pinned "exactly one", which was Q#RD10's deliberate
|
||||
/// restraint: removing them all would have routed N buffers through
|
||||
/// `remove_buffer_and_fire` — phase 2 without phase 1 — so the second
|
||||
/// duplicate was left alive rather than have its window dangle.
|
||||
/// `reconcile_delete` composes both phases, so the restraint is gone and
|
||||
/// the surviving duplicate is now the defect: it is bound to a path
|
||||
/// whose file no longer exists, and `find_by_path` cannot even see it.
|
||||
///
|
||||
/// Bite: fails against a first-match implementation (one duplicate
|
||||
/// survives) and against a widening that skips phase 1 (a window keeps
|
||||
/// a removed id).
|
||||
#[test]
|
||||
fn rd14_clean_duplicates_reconcile_exactly_one() {
|
||||
fn rd14_clean_duplicates_all_reconcile() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let f = dir.path().join("twin.rs");
|
||||
std::fs::write(&f, b"twin\n").expect("write");
|
||||
|
|
@ -8658,6 +8695,13 @@ fn rd14_clean_duplicates_reconcile_exactly_one() {
|
|||
.exec()
|
||||
.expect("two clean buffers on one path");
|
||||
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load("pmacs.window.switch_buffer(SECOND)")
|
||||
.exec()
|
||||
.expect("show the second duplicate");
|
||||
|
||||
let (ok, err) = rd_delete(&mut state, &f, "");
|
||||
assert!(ok, "two clean duplicates must not block: {err}");
|
||||
|
||||
|
|
@ -8668,9 +8712,23 @@ fn rd14_clean_duplicates_reconcile_exactly_one() {
|
|||
.eval()
|
||||
.expect("validity probe");
|
||||
assert!(
|
||||
first != second,
|
||||
"THE BITE: exactly one duplicate is reconciled away, not both \
|
||||
and not neither (first={first}, second={second})"
|
||||
!first && !second,
|
||||
"THE BITE: both buffers bound to the deleted path must be \
|
||||
reconciled away; a survivor points at a file that is gone and is \
|
||||
invisible to `find_by_path` (first={first}, second={second})"
|
||||
);
|
||||
|
||||
let core = state.core.borrow();
|
||||
let dangling: Vec<_> = core
|
||||
.windows
|
||||
.iter()
|
||||
.filter(|(_, w)| !core.registry.borrow().contains(w.buffer_id))
|
||||
.map(|(id, w)| (*id, w.buffer_id))
|
||||
.collect();
|
||||
assert!(
|
||||
dangling.is_empty(),
|
||||
"THE OTHER HALF: removing every match must not leave a window on \
|
||||
a removed id; dangling: {dangling:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -992,3 +992,311 @@ fn copy_mode_refuses_a_non_terminal_buffer() {
|
|||
"the refusal must say why: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Generated-buffer immutability Stage 1, criterion 8 [`main`] — Q#GB6's
|
||||
/// cursor clamp.
|
||||
///
|
||||
/// `EditorCore::notify_buffer_edit` — the fan-out every generated write
|
||||
/// goes through — updated each window's `TextView` and overlays but
|
||||
/// clamped neither window coordinate; only `rebuild_views_for` did, and
|
||||
/// its doc comment said so. So a shrinking generated refresh left
|
||||
/// `win.cursor` past the end of the rope **indefinitely**: paint does not
|
||||
/// crash, and a motion command does not recover it, because motion is
|
||||
/// computed from the stale value.
|
||||
///
|
||||
/// *Bite:* measured on the pre-image — cursor 29, len 2, and `C-p` leaves
|
||||
/// it at 29. This ships today for terminal copy mode: refresh a snapshot
|
||||
/// to a shorter one with the point low in the buffer and this is the
|
||||
/// state. Falsify by deleting the `win.cursor > len` clamp.
|
||||
#[test]
|
||||
fn acc16f_a_shrinking_generated_write_clamps_the_window_cursor() {
|
||||
let state = EditorState::new();
|
||||
exec(
|
||||
&state,
|
||||
r"
|
||||
GEN = pmacs.buffer.create('*generated-probe*')
|
||||
pmacs.buffer.set_generated_contents(GEN, 'alpha\nbeta\ngamma\ndelta\nepsilon\n')
|
||||
pmacs.window.switch_buffer(GEN)
|
||||
pmacs.editor.goto_byte(30)
|
||||
",
|
||||
);
|
||||
let (cursor, len): (i64, i64) = eval(
|
||||
&state,
|
||||
"return pmacs.editor.cursor(), pmacs.window.buffer():len()",
|
||||
);
|
||||
assert_eq!(
|
||||
(cursor, len),
|
||||
(30, 31),
|
||||
"precondition: point low in a 31-byte buffer"
|
||||
);
|
||||
|
||||
exec(&state, r"pmacs.buffer.set_generated_contents(GEN, 'x\n')");
|
||||
|
||||
let (cursor, len): (i64, i64) = eval(
|
||||
&state,
|
||||
"return pmacs.editor.cursor(), pmacs.window.buffer():len()",
|
||||
);
|
||||
assert_eq!(len, 2, "precondition: the buffer shrank");
|
||||
assert!(
|
||||
cursor <= len,
|
||||
"the cursor must be clamped into the new rope; got {cursor} for len {len}"
|
||||
);
|
||||
|
||||
// And motion works from there: `C-p` reaches line 0, which it cannot
|
||||
// do from a dangling offset.
|
||||
exec(&state, "pmacs.editor.move_up()");
|
||||
let cursor: i64 = eval(&state, "return pmacs.editor.cursor()");
|
||||
assert_eq!(cursor, 0, "C-p must move to the first line");
|
||||
}
|
||||
|
||||
/// Generated-buffer immutability Stage 1, criterion 8b [`main`] — Q#GB6's
|
||||
/// `view_top` clamp, on a buffer that GREW.
|
||||
///
|
||||
/// The two coordinates fail on different axes: `cursor` is a byte
|
||||
/// position bounded by `Buffer::len`, while `view_top` is a **line
|
||||
/// index** bounded by `TextView::line_count`. A replace can grow in bytes
|
||||
/// while collapsing many lines into one, so a clamp gated on "the buffer
|
||||
/// shrank" passes criterion 8 and fails here — which is why the clamp
|
||||
/// runs unconditionally, each coordinate against its own bound, exactly
|
||||
/// as `rebuild_views_for` already does.
|
||||
///
|
||||
/// *Bite:* falsify by gating the clamp on a byte-length comparison, or by
|
||||
/// deleting the `view_top` half. Unlike criterion 8 this case is argued
|
||||
/// from the types rather than measured on `main`; the assertion below is
|
||||
/// the measurement.
|
||||
#[test]
|
||||
fn acc16g_a_line_collapsing_generated_write_clamps_view_top() {
|
||||
let state = EditorState::new();
|
||||
exec(
|
||||
&state,
|
||||
r"
|
||||
GEN = pmacs.buffer.create('*viewtop-probe*')
|
||||
pmacs.buffer.set_generated_contents(GEN, 'a\nb\nc\nd\ne\nf\n')
|
||||
pmacs.window.switch_buffer(GEN)
|
||||
pmacs.editor.set_view_top(5)
|
||||
",
|
||||
);
|
||||
let (top, len): (i64, i64) = eval(
|
||||
&state,
|
||||
"return pmacs.editor.view_top(), pmacs.window.buffer():len()",
|
||||
);
|
||||
assert_eq!(
|
||||
(top, len),
|
||||
(5, 12),
|
||||
"precondition: scrolled to line 5 of a 12-byte, 7-line buffer"
|
||||
);
|
||||
|
||||
// 20 bytes on ONE line: longer than what it replaces, so any
|
||||
// "the buffer shrank" trigger is false here.
|
||||
exec(
|
||||
&state,
|
||||
r"pmacs.buffer.set_generated_contents(GEN, '0123456789abcdefghij')",
|
||||
);
|
||||
|
||||
let len: i64 = eval(&state, "return pmacs.window.buffer():len()");
|
||||
assert_eq!(len, 20, "precondition: the buffer GREW in bytes");
|
||||
|
||||
let (top, lines) = {
|
||||
let core = state.core.borrow();
|
||||
let active = core.active_window_id();
|
||||
let win = core.windows.get(&active).expect("active window");
|
||||
(win.view_top, win.text_view.line_count())
|
||||
};
|
||||
assert!(
|
||||
top < lines,
|
||||
"view_top must be clamped into the new line count; got {top} of {lines}"
|
||||
);
|
||||
assert_eq!(top, 0, "the collapsed buffer has exactly one line");
|
||||
}
|
||||
|
||||
/// Generated-buffer immutability Stage 1 criterion 8c [`main`] — the
|
||||
/// selection anchor is a third window coordinate normalized by Q#GB6's
|
||||
/// clamp-or-clear rule.
|
||||
///
|
||||
/// `Window::region` orders `(anchor, cursor)`, so a stale anchor above a
|
||||
/// clamped cursor is still the high end of the region, and
|
||||
/// `region_bytes` slices the rope with it. Reproduced on this branch
|
||||
/// before the fix: `assertion failed: end <= self.len()` at
|
||||
/// `src/rope.rs:145`, reached from `EditorCore::clipboard_copy`.
|
||||
///
|
||||
/// Both outcomes matter: clamping a backward selection from 0..30 into
|
||||
/// 0..2 preserves the shortened region, while clamping a forward
|
||||
/// selection from 2..30 collapses both endpoints at 2 and clears it.
|
||||
/// Clearing every stale anchor passes the crash check but fails the
|
||||
/// first half; clamping without the collapsed check fails the second.
|
||||
///
|
||||
/// *Bite:* delete `clamp_cursor_and_selection` from
|
||||
/// `notify_buffer_edit`; the first copy reaches the stale-anchor panic.
|
||||
#[test]
|
||||
fn acc16h_a_shrinking_generated_write_clamps_or_clears_the_selection() {
|
||||
let state = EditorState::new();
|
||||
exec(
|
||||
&state,
|
||||
r"
|
||||
GEN = pmacs.buffer.create('*anchor-probe*')
|
||||
pmacs.buffer.set_generated_contents(GEN, 'alpha\nbeta\ngamma\ndelta\nepsilon\n')
|
||||
pmacs.window.switch_buffer(GEN)
|
||||
",
|
||||
);
|
||||
{
|
||||
// A BACKWARD selection: anchor at the far end, point at the
|
||||
// start. The forward one is not a discriminator.
|
||||
let mut core = state.core.borrow_mut();
|
||||
core.begin_selection(30);
|
||||
core.set_cursor_byte(0);
|
||||
assert_eq!(
|
||||
core.active_region(),
|
||||
Some((0, 30)),
|
||||
"precondition: a live 30-byte region"
|
||||
);
|
||||
}
|
||||
|
||||
exec(&state, r"pmacs.buffer.set_generated_contents(GEN, 'xy')");
|
||||
|
||||
let mut core = state.core.borrow_mut();
|
||||
assert_eq!(
|
||||
core.active_buffer_len(),
|
||||
2,
|
||||
"precondition: the buffer shrank"
|
||||
);
|
||||
assert_eq!(
|
||||
core.active_window()
|
||||
.selection
|
||||
.map(|selection| selection.anchor),
|
||||
Some(2),
|
||||
"the stale anchor is clamped into the new extent"
|
||||
);
|
||||
assert_eq!(
|
||||
core.active_region(),
|
||||
Some((0, 2)),
|
||||
"a non-collapsed selection survives as the shortened region"
|
||||
);
|
||||
assert!(
|
||||
core.clipboard_copy(),
|
||||
"the production consumer copies the valid shortened region"
|
||||
);
|
||||
drop(core);
|
||||
|
||||
// The other result: cursor clamping moves 30 to the anchor at 2, so
|
||||
// the selected content is gone and no empty active selection remains.
|
||||
exec(
|
||||
&state,
|
||||
r"pmacs.buffer.set_generated_contents(GEN, 'alpha\nbeta\ngamma\ndelta\nepsilon\n')",
|
||||
);
|
||||
{
|
||||
let mut core = state.core.borrow_mut();
|
||||
core.begin_selection(2);
|
||||
core.set_cursor_byte(30);
|
||||
assert_eq!(
|
||||
core.active_region(),
|
||||
Some((2, 30)),
|
||||
"precondition: a forward 28-byte region"
|
||||
);
|
||||
}
|
||||
exec(&state, r"pmacs.buffer.set_generated_contents(GEN, 'xy')");
|
||||
let mut core = state.core.borrow_mut();
|
||||
assert_eq!(
|
||||
core.active_window().selection,
|
||||
None,
|
||||
"a cursor clamp that collapses the region clears the selection"
|
||||
);
|
||||
assert!(
|
||||
!core.clipboard_copy(),
|
||||
"there is no collapsed region to copy"
|
||||
);
|
||||
}
|
||||
|
||||
/// Criterion 8c's second clamp site, driven through its own real Lua
|
||||
/// path.
|
||||
///
|
||||
/// `EditorCore::rebuild_views_for` had the identical defect and is a
|
||||
/// separate exit: the `*help*` renderer rewrites end to end and calls it
|
||||
/// rather than `notify_buffer_edit` (`src/lua_bindings/mod.rs:1650`, via
|
||||
/// `pmacs.help.show_command`). Fixing one function and not the other
|
||||
/// would leave a live panic reachable from `M-x` help, so this pins the
|
||||
/// second exit rather than trusting that one call site implies the
|
||||
/// other.
|
||||
///
|
||||
/// This site also asserts both halves: a clamp can preserve the
|
||||
/// shortened region, and an anchor that clamps exactly onto the cursor
|
||||
/// clears it. *Bite:* delete `clamp_cursor_and_selection` from
|
||||
/// `rebuild_views_for`; the first copy panics at `src/rope.rs:145` while
|
||||
/// `acc16h` stays green.
|
||||
#[test]
|
||||
fn acc16i_a_shrinking_view_rebuild_clamps_or_clears_the_selection() {
|
||||
let state = EditorState::new();
|
||||
// 286 bytes, then 154: a real shrink through the help renderer.
|
||||
exec(
|
||||
&state,
|
||||
r"
|
||||
HELP = pmacs.help.show_command('cursor.down')
|
||||
pmacs.window.switch_buffer(HELP)
|
||||
",
|
||||
);
|
||||
let long_len: i64 = eval(&state, "return HELP:len()");
|
||||
{
|
||||
let mut core = state.core.borrow_mut();
|
||||
let anchor = u64::try_from(long_len).expect("non-negative");
|
||||
core.begin_selection(anchor);
|
||||
core.set_cursor_byte(0);
|
||||
assert_eq!(
|
||||
core.active_region(),
|
||||
Some((0, anchor)),
|
||||
"precondition: a live region anchored at the end"
|
||||
);
|
||||
}
|
||||
|
||||
exec(&state, "pmacs.help.show_command('editor.quit')");
|
||||
|
||||
let short_len: i64 = eval(&state, "return HELP:len()");
|
||||
assert!(
|
||||
short_len < long_len,
|
||||
"precondition: the help buffer shrank ({long_len} -> {short_len})"
|
||||
);
|
||||
let short = u64::try_from(short_len).expect("non-negative");
|
||||
let mut core = state.core.borrow_mut();
|
||||
assert_eq!(
|
||||
core.active_window()
|
||||
.selection
|
||||
.map(|selection| selection.anchor),
|
||||
Some(short),
|
||||
"the anchor is clamped to the shorter help buffer"
|
||||
);
|
||||
assert_eq!(
|
||||
core.active_region(),
|
||||
Some((0, short)),
|
||||
"the non-collapsed region survives the rebuild"
|
||||
);
|
||||
assert!(
|
||||
core.clipboard_copy(),
|
||||
"copy consumes the clamped region without slicing past the rope"
|
||||
);
|
||||
drop(core);
|
||||
|
||||
// Grow the same help buffer, then choose an anchor that the next
|
||||
// short render will clamp exactly onto the cursor.
|
||||
exec(&state, "pmacs.help.show_command('cursor.down')");
|
||||
{
|
||||
let mut core = state.core.borrow_mut();
|
||||
let long = u64::try_from(long_len).expect("non-negative");
|
||||
core.begin_selection(long);
|
||||
core.set_cursor_byte(short);
|
||||
assert_eq!(
|
||||
core.active_region(),
|
||||
Some((short, long)),
|
||||
"precondition: a region whose anchor exceeds the next extent"
|
||||
);
|
||||
}
|
||||
exec(&state, "pmacs.help.show_command('editor.quit')");
|
||||
|
||||
let mut core = state.core.borrow_mut();
|
||||
assert_eq!(
|
||||
core.active_window().selection,
|
||||
None,
|
||||
"an anchor clamp that collapses the region clears the selection"
|
||||
);
|
||||
assert!(
|
||||
!core.clipboard_copy(),
|
||||
"the collapsed region is not retained as active-but-empty"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue