Merge pull request #196 from levineuwirth/dired-stage2-impl
dired Stage 2a: rename and delete reconciliation
This commit is contained in:
GitHub
commit
c14f2de428
|
|
@ -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.",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -655,6 +655,177 @@ 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`.
|
||||
|
||||
## 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 — PR #188 OPEN, PROPOSED
|
||||
|
||||
- Portable branch: `githubsucks/generated-buffer-immutability`; worktree
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
@ -4828,6 +4840,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 +5296,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(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
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) {}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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
Loading…
Reference in New Issue