fix(journey): correct Q#JR3, report the post-dispatch buffer, unvacuate two pins
Four review findings, all confirmed against the tree. Q#JR3 was false. `replace_active_buffer` does not drop the startup scratch buffer -- its body is one `switch_active_buffer` call, which reassigns the window's buffer_id and removes nothing. The claim came from that function's own doc comment, wrong for as long as it has existed, and rev 5 propagated it into the framing and into new documentation this branch added. Both comments are corrected here, because this PR was adding further false references to a claim P4 depends on. Actually removing the stale scratch is buffer-lifetime work and stays out. The daemon bootstrap could report the wrong buffer. The directory arm captured the destination id, ran the resolver chain synchronously, then returned the captured id -- so a handler that opened something synchronously through commit_to had already replaced the window's buffer, and the reply paired one buffer's snapshot with another's identity. It also returned early, skipping the post-hook revalidation the framing said stayed active. The arm now re-reads the destination after dispatch and rehomes through `non_side_target` as the file arm does. Pinned by a test whose handler claims synchronously. N11 tested neither RET nor self-insert: it called display_file and buf:insert directly, so it stayed green with dired's RET binding, its entry dispatch, and the editor's self-insert path all broken. Both gestures now go through dispatch_key. P7 is removed rather than weakened. Q#JR12 has nothing to pin -- `had_file = file.is_some()` and a directory is Some like any other, so no directory-specific branch exists to break. The old test never armed restore and hard-coded had_file, so it could not fail against any implementation. Also adds the daemon bootstrap pins (N2, N5) and fixes an insertion that had orphaned a `#[cfg(feature = "crdt")]` from the test it guarded -- which would have made one new test dark and one existing test escape its gate. Framing: docs/journey-stage1a-framing.md rev 6.
This commit is contained in:
parent
f09f66ce37
commit
25b07be97b
|
|
@ -1,6 +1,7 @@
|
|||
# Journey Stage 1a — open a directory, on one path
|
||||
|
||||
**Status: framing, rev 5, awaiting approval.**
|
||||
**Status: framing, rev 6 — APPROVED at rev 5; rev 6 records
|
||||
corrections found during implementation.**
|
||||
**Serves `COHERENCE.md` §2 (the golden product journey), §19 (coherence
|
||||
acceptance tests), §20 Priority 1.**
|
||||
|
||||
|
|
@ -76,6 +77,43 @@ acceptance tests), §20 Priority 1.**
|
|||
- The dired accessor spelling, revision heading, and Stage 2 ledger
|
||||
claim are corrected.
|
||||
|
||||
- rev 6 (2026-07-26) — **corrections found while implementing**, not a
|
||||
new design round. Four, all confirmed against the tree:
|
||||
- **Q#JR3 was false.** `replace_active_buffer` does *not* drop the
|
||||
startup scratch buffer; its body is one `switch_active_buffer` call,
|
||||
which reassigns `aw.buffer_id` and removes nothing. The claim came
|
||||
from that function's own doc comment (`editor.rs:1071`), which has
|
||||
been wrong for as long as it has existed, and rev 5 propagated it
|
||||
into §2.2, §3, P4, and the decision list without checking the body.
|
||||
Corrected in all four places; the stale comment is corrected in this
|
||||
PR too, since this PR would otherwise add *more* false references to
|
||||
it. **Actually removing the stale scratch is separate work** —
|
||||
buffer-lifetime changes have their own consequences (what else holds
|
||||
the id, what `C-x b` lists) and are not smuggled into a directory-open
|
||||
stage.
|
||||
- **The daemon bootstrap could report the wrong buffer** (§4.5). The
|
||||
directory arm captured `dest.buffer`, ran the resolver chain
|
||||
*synchronously*, then returned the captured id — so a handler that
|
||||
opened something synchronously (through `commit_to`, the supported
|
||||
way) had already replaced the window's buffer, and the reply would
|
||||
pair one buffer's snapshot with another's identity. The early return
|
||||
also skipped the post-hook revalidation this framing claimed stayed
|
||||
active. Rev 6 decides: **report what the window actually holds after
|
||||
the dispatch**, and rehome through `non_side_target` exactly as the
|
||||
file arm does.
|
||||
- **N11 tested neither `RET` nor self-insert.** It called
|
||||
`display_file` and `buf:insert` directly, so it stayed green with
|
||||
dired's `RET` binding, its entry dispatch, and the editor's
|
||||
self-insert path all broken — most of what "the journey works" means.
|
||||
Both gestures are now dispatched as real keys.
|
||||
- **P7 was vacuous and is removed, not weakened.** Q#JR12 has nothing
|
||||
to pin: `run` computes `had_file = file.is_some()` and a directory
|
||||
path is `Some` like any other, so suppression is structural and the
|
||||
named mutation would require inventing the branch first. The rev 5
|
||||
test additionally never armed restore and hard-coded `had_file`, so
|
||||
it asserted nothing about `run`. Q#JR12 is downgraded to an
|
||||
observation.
|
||||
|
||||
---
|
||||
|
||||
## 0.5. Coherence impact (`COHERENCE.md` §20, required since #163)
|
||||
|
|
@ -160,7 +198,7 @@ daemon bootstrap (`daemon.rs:1641`). **Local startup is not one of them**
|
|||
| Displayed name | `path.display()` raw (`editor.rs:772`) | `path.display()` raw |
|
||||
| `NotFound` arm | empty path-backed buffer, `[new file]` | identical |
|
||||
| Dedup | none | `find_buffer_for_path` |
|
||||
| Window install | `replace_active_buffer` — drops the startup scratch (`editor.rs:797`) | none; caller installs |
|
||||
| Window install | `replace_active_buffer` — switches the ACTIVE window (`editor.rs:797`). **It does not drop the startup scratch** (rev 6): its body is one `switch_active_buffer` call, which reassigns `aw.buffer_id` and removes nothing. The doc comment claiming otherwise was wrong before this stage and is corrected in this PR | none; caller installs |
|
||||
| Error type | `io::Error`, bare | `String`, prefixed `cannot open {path}: ` |
|
||||
|
||||
**The two agree on every observable except the error prefix and the
|
||||
|
|
@ -319,7 +357,9 @@ invariant rather than working around it.
|
|||
## 3. The unification (Q#JR1)
|
||||
|
||||
`EditorState::open` becomes a thin caller of `resolve_target_buffer`,
|
||||
keeping `replace_active_buffer` (which drops the startup scratch, Q#JR3)
|
||||
keeping `replace_active_buffer` (which switches the **active** window,
|
||||
Q#JR3 as corrected in rev 6 — it does not destroy the old scratch, and
|
||||
never did)
|
||||
and keeping its "fire the hook after the core borrow ends" structure
|
||||
(`editor.rs:786-795`) — listeners re-enter `pmacs.editor.*` and re-borrow
|
||||
the core (Q#JR1a).
|
||||
|
|
@ -328,9 +368,15 @@ the core (Q#JR1a).
|
|||
`pmacs /root/secret` names the file, which today's bare message does not.
|
||||
This is the *only* user-visible change from the unification (§2.2).
|
||||
|
||||
**Q#JR12** — a directory argument counts as "had a file argument" and
|
||||
suppresses desktop restore, on Q#DS7's reasoning: a positional argument
|
||||
means "open this", not "restore my session".
|
||||
**Q#JR12 (downgraded to an observation, rev 6)** — a directory argument
|
||||
suppresses desktop restore, on Q#DS7's reasoning that a positional
|
||||
argument means "open this" rather than "restore my session". This needs
|
||||
no work and cannot be pinned: `run` computes `had_file = file.is_some()`
|
||||
(`editor.rs:3152`), and a directory path is `Some` like any other, so
|
||||
there is no directory-specific branch that could get it wrong. Rev 5
|
||||
carried an acceptance for it; that test never armed restore and
|
||||
hard-coded `had_file`, asserting nothing, and is removed rather than
|
||||
repaired.
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -538,9 +584,26 @@ problem is solved.
|
|||
### 4.5 Q#JR9 — what the bootstrap reply names, and what it shows
|
||||
|
||||
`open_initial_target` on a `Directory` installs nothing: it dispatches the
|
||||
resolver, then replies `Opened { buffer_id }` naming the buffer the fresh
|
||||
view's primary document window already holds. §2.8's reassert reasserts
|
||||
that same buffer — already correct, therefore harmless.
|
||||
resolver, then replies `Opened { buffer_id }` naming **whatever the
|
||||
destination window holds once that dispatch returns** — re-read, not the
|
||||
id captured beforehand (Q#JR9b, rev 6).
|
||||
|
||||
The distinction is not academic. The chain runs **synchronously**.
|
||||
dired's handler defers, because its listing must await; a user's resolver
|
||||
is under no such obligation, and one that opens something synchronously
|
||||
through `commit_to` — the supported way to do it — has already replaced
|
||||
the window's buffer by the time the reply is built. Reporting the
|
||||
captured id would pair one buffer's snapshot with another's identity, and
|
||||
the frontend would render a document nobody asked for.
|
||||
|
||||
Re-reading also subsumes the case where a hook closed the window, so this
|
||||
arm rehomes through `non_side_target` exactly as the file arm's reassert
|
||||
does, rather than returning early and skipping that check — which rev 5's
|
||||
implementation did while this section claimed the revalidation stayed
|
||||
active.
|
||||
|
||||
Absent a synchronous claimant the re-read yields the buffer the window
|
||||
already held, which is the ordinary case.
|
||||
|
||||
**That buffer is not necessarily `*scratch*`.** `build_fresh_frontend_view`
|
||||
clones **LOCAL's primary document buffer** (`daemon.rs:2997`) — M10.9 made
|
||||
|
|
@ -723,8 +786,13 @@ is **removed rather than recast**: it proved nothing N1 does not.
|
|||
window unchanged; `find_file_accepting_a_directory_reports_instead_of_raising`
|
||||
passes unmodified.
|
||||
*Mutation:* route `display_file` into the resolver chain.
|
||||
- **P7 — desktop restore stays suppressed (Q#JR12).**
|
||||
*Mutation:* pass `false` for `had_file` on the directory path.
|
||||
- **P7 — REMOVED in rev 6.** Q#JR12 is structural: `run` computes
|
||||
`had_file = file.is_some()` and a directory path is `Some` like any
|
||||
other, so there is no directory-specific branch to break and the named
|
||||
mutation would have to invent one first. Rev 5's test never armed
|
||||
restore and hard-coded `had_file`, so it could not fail against any
|
||||
implementation. Removed rather than repaired — a green test that cannot
|
||||
fail reads as coverage.
|
||||
- **P8 — startup errors name the file (Q#JR4).** A non-`NotFound`,
|
||||
non-directory failure produces a message containing `cannot open` and
|
||||
the path. *(Legitimately N-shaped for the prefix, P-shaped for the
|
||||
|
|
@ -851,7 +919,12 @@ discovered late.
|
|||
- **Q#JR1** `EditorState::open` adopts `resolve_target_buffer` wholesale.
|
||||
- **Q#JR1a** The hook fires outside the core borrow.
|
||||
- **Q#JR2** *Withdrawn (rev 2)* — its premise was false.
|
||||
- **Q#JR3** The scratch drop (`replace_active_buffer`) is preserved.
|
||||
- **Q#JR3 (corrected rev 6)** Startup keeps using
|
||||
`replace_active_buffer`, which switches the **active** window — not
|
||||
because it drops the old scratch (it does not, and never did) but
|
||||
because an `install_buffer_in_window` elsewhere would load the file
|
||||
while leaving the user looking at scratch. Removing the stale scratch
|
||||
buffer is separate work.
|
||||
- **Q#JR4** Startup errors gain the `cannot open {path}: ` prefix.
|
||||
- **Q#JR5** `resolve_target_buffer` returns a typed `ResolvedTarget`.
|
||||
- **Q#JR5b** Both `HookKind` types are written path-qualified.
|
||||
|
|
@ -860,12 +933,17 @@ discovered late.
|
|||
builtins do not subscribe; dired is a replaceable fallback slot.
|
||||
- **Q#JR8** `ResolvedTarget::Directory` carries an explicitly normalized
|
||||
path.
|
||||
- **Q#JR9** The bootstrap reply names the window's pre-existing buffer —
|
||||
LOCAL's primary document buffer, not necessarily scratch. Accepted and
|
||||
documented.
|
||||
- **Q#JR9** The bootstrap reply names the destination window's buffer —
|
||||
absent a synchronous claimant, LOCAL's primary document buffer, not
|
||||
necessarily scratch. Accepted and documented.
|
||||
- **Q#JR9b (rev 6)** That id is **re-read after the dispatch**, and the
|
||||
arm rehomes through `non_side_target` rather than returning early: a
|
||||
synchronous resolver may already have replaced the buffer.
|
||||
- **Q#JR10** An unclaimed directory with the handler cleared exits 0 with
|
||||
a status message.
|
||||
- **Q#JR12** A directory argument suppresses desktop restore.
|
||||
- **Q#JR12 (observation, rev 6)** A directory argument suppresses
|
||||
desktop restore structurally, via `had_file = file.is_some()`. No work,
|
||||
no pin.
|
||||
- **Q#JR13** `display_file` keeps its directory-is-an-error contract.
|
||||
- **Q#JR14** The destination `{frontend, window, buffer}` is captured at
|
||||
resolve time; `commit_to` preflights and scopes the **entire**
|
||||
|
|
|
|||
213
src/daemon.rs
213
src/daemon.rs
|
|
@ -1661,11 +1661,51 @@ fn open_initial_target(
|
|||
let dest = editor
|
||||
.capture_directory_destination(frontend_id, origin_window)
|
||||
.ok_or_else(|| format!("cannot open {}: no document window", path.display()))?;
|
||||
let buffer_id = dest.buffer;
|
||||
editor.dispatch_directory_open(&path, dest);
|
||||
editor.reconcile_panel_layout(frontend_id);
|
||||
|
||||
// The reply must name what the window ACTUALLY holds now, not
|
||||
// what it held before the dispatch.
|
||||
//
|
||||
// The chain runs synchronously. dired's handler defers (it
|
||||
// spawns a coroutine for the listing), but a user's resolver
|
||||
// is under no such obligation: a handler that opens something
|
||||
// synchronously -- through `commit_to`, which is exactly the
|
||||
// supported way to do it -- has already replaced this
|
||||
// window's buffer by the time we get here. Reporting the
|
||||
// captured id would then send the snapshot of one buffer and
|
||||
// the identity of another, and the frontend would render a
|
||||
// document nobody asked for.
|
||||
//
|
||||
// Re-reading also covers the case a hook closed the window,
|
||||
// which is why this rehomes through `non_side_target` exactly
|
||||
// as the file arm's reassert does rather than returning early
|
||||
// and skipping that check.
|
||||
let mut core = editor.core.borrow_mut();
|
||||
core.active_frontend = frontend_id;
|
||||
let destination = if core
|
||||
.views
|
||||
.get(&frontend_id)
|
||||
.is_some_and(|view| view.layout.iter_ids().contains(&origin_window))
|
||||
{
|
||||
origin_window
|
||||
} else {
|
||||
core.non_side_target(frontend_id)
|
||||
.map_err(|error| format!("cannot reselect {}: {error}", path.display()))?
|
||||
};
|
||||
core.focus_window(frontend_id, destination);
|
||||
let buffer_id = core
|
||||
.windows
|
||||
.get(&destination)
|
||||
.map(|window| window.buffer_id)
|
||||
.ok_or_else(|| format!("cannot reselect {}: window died", path.display()))?;
|
||||
return Ok(OpenedInitialTarget {
|
||||
buffer_id,
|
||||
// False whether or not the chain replaced the buffer: an
|
||||
// untouched destination is pre-existing and already
|
||||
// published, and a buffer a synchronous handler installed
|
||||
// went through the ordinary display path, which publishes
|
||||
// on its own terms.
|
||||
publish_to_replicas: false,
|
||||
});
|
||||
}
|
||||
|
|
@ -4924,6 +4964,177 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
/// **N2** (Journey Stage 1a) — a DIRECTORY initial target reaches
|
||||
/// readiness instead of failing.
|
||||
///
|
||||
/// This deliberately supersedes the directory half of the GPU
|
||||
/// initial-target framing's Q#GT6 and its acceptance 10, which
|
||||
/// required `IsADirectory` to fail before window creation.
|
||||
/// Permission-denied and every other pre-readiness failure keep that
|
||||
/// contract.
|
||||
#[test]
|
||||
fn initial_target_directory_reaches_ready() {
|
||||
use crate::editor::EditorState;
|
||||
use crate::protocol::FrontendId;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
std::fs::write(dir.path().join("alpha.txt"), b"alpha\n").expect("write");
|
||||
|
||||
let mut editor = EditorState::new();
|
||||
editor
|
||||
.lua_host
|
||||
.lua()
|
||||
.load("pmacs.lsp.config = {}")
|
||||
.exec()
|
||||
.expect("wipe lsp config");
|
||||
let fid = FrontendId(131);
|
||||
let view = build_fresh_frontend_view(&mut editor, false, false);
|
||||
editor.core.borrow_mut().register_frontend_view(fid, view);
|
||||
|
||||
let opened = open_initial_target(
|
||||
&mut editor,
|
||||
fid,
|
||||
InitialTarget {
|
||||
path: dir.path().as_os_str().as_bytes().to_vec(),
|
||||
cwd: dir.path().as_os_str().as_bytes().to_vec(),
|
||||
},
|
||||
)
|
||||
.expect("a directory target must reach readiness, not fail");
|
||||
|
||||
// The reply names a live buffer in a live document window: a
|
||||
// valid, ready session. The listing arrives later, asynchronously.
|
||||
let core = editor.core.borrow();
|
||||
assert!(
|
||||
core.registry.borrow().contains(opened.buffer_id),
|
||||
"the reported buffer must exist so its snapshot can be sent"
|
||||
);
|
||||
let active = core.views[&fid].active;
|
||||
assert_eq!(
|
||||
core.windows[&active].buffer_id, opened.buffer_id,
|
||||
"the reported buffer is the one the document window shows"
|
||||
);
|
||||
}
|
||||
|
||||
/// **N5** — the bootstrap buffer is not necessarily `*scratch*`.
|
||||
///
|
||||
/// `build_fresh_frontend_view` clones LOCAL's PRIMARY DOCUMENT
|
||||
/// buffer, so when LOCAL holds a real document the fresh session
|
||||
/// briefly displays and snapshots it. Q#JR9 accepts that rather than
|
||||
/// introducing a placeholder; this observes it instead of assuming.
|
||||
#[test]
|
||||
fn initial_target_directory_reports_a_non_scratch_primary() {
|
||||
use crate::editor::EditorState;
|
||||
use crate::protocol::FrontendId;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let doc = dir.path().join("already-open.txt");
|
||||
std::fs::write(&doc, b"local document\n").expect("write");
|
||||
|
||||
// LOCAL holds a real document, not scratch.
|
||||
let mut editor = EditorState::open(doc.clone()).expect("open");
|
||||
editor
|
||||
.lua_host
|
||||
.lua()
|
||||
.load("pmacs.lsp.config = {}")
|
||||
.exec()
|
||||
.expect("wipe lsp config");
|
||||
let local_primary = editor
|
||||
.core
|
||||
.borrow()
|
||||
.primary_document_buffer(FrontendId::LOCAL)
|
||||
.expect("LOCAL always has a document window");
|
||||
|
||||
let fid = FrontendId(132);
|
||||
let view = build_fresh_frontend_view(&mut editor, false, false);
|
||||
editor.core.borrow_mut().register_frontend_view(fid, view);
|
||||
|
||||
let opened = open_initial_target(
|
||||
&mut editor,
|
||||
fid,
|
||||
InitialTarget {
|
||||
path: dir.path().as_os_str().as_bytes().to_vec(),
|
||||
cwd: dir.path().as_os_str().as_bytes().to_vec(),
|
||||
},
|
||||
)
|
||||
.expect("a directory target must reach readiness");
|
||||
|
||||
assert_eq!(
|
||||
opened.buffer_id, local_primary,
|
||||
"the bootstrap reply names LOCAL's primary document buffer, \
|
||||
which is a real document here rather than *scratch*"
|
||||
);
|
||||
}
|
||||
|
||||
/// **N2b (rev 6)** — a resolver that claims SYNCHRONOUSLY is reported
|
||||
/// correctly.
|
||||
///
|
||||
/// The bug this pins: the arm captured the destination buffer id
|
||||
/// *before* dispatching the chain and reported that. The chain runs
|
||||
/// synchronously, so a handler that opens something immediately —
|
||||
/// through `commit_to`, the supported way — had already replaced the
|
||||
/// window's buffer, and the reply paired one buffer's snapshot with
|
||||
/// another's identity.
|
||||
///
|
||||
/// Falsified by reporting the captured id instead of re-reading.
|
||||
#[test]
|
||||
fn initial_target_directory_reports_what_a_synchronous_handler_installed() {
|
||||
use crate::editor::EditorState;
|
||||
use crate::protocol::FrontendId;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
let mut editor = EditorState::new();
|
||||
editor
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(
|
||||
"pmacs.lsp.config = {}
|
||||
claimed = pmacs.buffer.create('*claimed*')
|
||||
pmacs.path.set_directory_handler(function(path, dest)
|
||||
pmacs.window.commit_to(dest, function()
|
||||
pmacs.window.display(claimed, { select = true })
|
||||
end)
|
||||
end)",
|
||||
)
|
||||
.exec()
|
||||
.expect("install a synchronous handler");
|
||||
|
||||
let fid = FrontendId(133);
|
||||
let view = build_fresh_frontend_view(&mut editor, false, false);
|
||||
editor.core.borrow_mut().register_frontend_view(fid, view);
|
||||
|
||||
let opened = open_initial_target(
|
||||
&mut editor,
|
||||
fid,
|
||||
InitialTarget {
|
||||
path: dir.path().as_os_str().as_bytes().to_vec(),
|
||||
cwd: dir.path().as_os_str().as_bytes().to_vec(),
|
||||
},
|
||||
)
|
||||
.expect("a claimed directory target must reach readiness");
|
||||
|
||||
// Compare by NAME: the reported id must be the handler's buffer,
|
||||
// and naming it is what makes the failure legible when it is not.
|
||||
let core = editor.core.borrow();
|
||||
let reported_name = core
|
||||
.registry
|
||||
.borrow()
|
||||
.get(opened.buffer_id)
|
||||
.expect("the reported buffer exists")
|
||||
.name()
|
||||
.to_string();
|
||||
assert_eq!(
|
||||
reported_name, "*claimed*",
|
||||
"the reply must name what the handler installed, not the \
|
||||
buffer captured before the dispatch"
|
||||
);
|
||||
let active = core.views[&fid].active;
|
||||
assert_eq!(
|
||||
core.windows[&active].buffer_id, opened.buffer_id,
|
||||
"…and that buffer is what the window shows"
|
||||
);
|
||||
}
|
||||
|
||||
/// Bottom-panel §1.3 #1/#3/#21 — the three Projection producers whose
|
||||
/// only production caller is `dispatcher_loop`, pinned at the named
|
||||
/// seams that loop calls. Round 2 finding: reverting any of them to
|
||||
|
|
|
|||
|
|
@ -891,10 +891,19 @@ impl EditorState {
|
|||
///
|
||||
/// * **The window install.** `resolve_target_buffer` deliberately
|
||||
/// does not touch windows, so the caller places the buffer.
|
||||
/// Startup uses [`Self::replace_active_buffer`] specifically
|
||||
/// because it drops the just-created scratch buffer; an
|
||||
/// `install_buffer_in_window` here would leave a stray `*scratch*`
|
||||
/// behind every `pmacs FILE` (Q#JR3).
|
||||
/// Startup uses [`Self::replace_active_buffer`], which switches
|
||||
/// the ACTIVE window — an `install_buffer_in_window` into some
|
||||
/// other window would load the file and leave the user looking at
|
||||
/// scratch (Q#JR3).
|
||||
///
|
||||
/// It does **not** destroy the scratch buffer, despite what
|
||||
/// `replace_active_buffer`'s own doc comment has long claimed:
|
||||
/// that function only calls `switch_active_buffer`, which
|
||||
/// reassigns the window's `buffer_id` and removes nothing. The
|
||||
/// startup scratch survives in the registry, and did before this
|
||||
/// stage too. Changing that is buffer-lifetime work with its own
|
||||
/// consequences (what else may hold the id, what `C-x b` should
|
||||
/// list) and is deliberately not smuggled in here.
|
||||
/// * **Firing the hook outside the core borrow.** Listeners
|
||||
/// re-enter `pmacs.editor.*`, which re-borrows the core
|
||||
/// (Q#JR1a) — the same reason the daemon bootstrap and
|
||||
|
|
@ -1068,9 +1077,18 @@ impl EditorState {
|
|||
}
|
||||
}
|
||||
|
||||
/// Switch the active window to `buffer_id`, dropping any old
|
||||
/// scratch buffer if the active window's previous buffer has no
|
||||
/// other windows referencing it. Returns silently on a stale id.
|
||||
/// Switch the active window to `buffer_id`. Returns silently on a
|
||||
/// stale id.
|
||||
///
|
||||
/// **Corrected (Journey Stage 1a).** This comment previously claimed
|
||||
/// it dropped "any old scratch buffer if the active window's
|
||||
/// previous buffer has no other windows referencing it". It never
|
||||
/// did: the body is one `switch_active_buffer` call, which reassigns
|
||||
/// `aw.buffer_id` and removes nothing from the registry. The claim
|
||||
/// was load-bearing enough that a framing decision (Q#JR3) and an
|
||||
/// acceptance pin were written against it before anyone checked the
|
||||
/// body. Removing the stale scratch may well be worth doing; it is
|
||||
/// separate work, and this comment no longer promises it.
|
||||
fn replace_active_buffer(&self, buffer_id: crate::buffer::BufferId) {
|
||||
let mut core = self.core.borrow_mut();
|
||||
let _ = core.switch_active_buffer(buffer_id);
|
||||
|
|
|
|||
|
|
@ -31,8 +31,10 @@
|
|||
use std::path::Path;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers};
|
||||
use pmacs::editor::EditorState;
|
||||
use pmacs::editor_core::normalize_buffer_path;
|
||||
use pmacs::protocol::FrontendId;
|
||||
use tempfile::TempDir;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -65,6 +67,35 @@ fn pump(s: &mut EditorState) {
|
|||
}
|
||||
}
|
||||
|
||||
fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent {
|
||||
KeyEvent {
|
||||
code,
|
||||
modifiers: mods,
|
||||
kind: KeyEventKind::Press,
|
||||
state: KeyEventState::NONE,
|
||||
}
|
||||
}
|
||||
|
||||
fn press(s: &mut EditorState, code: KeyCode) {
|
||||
s.dispatch_key(FrontendId::LOCAL, key(code, KeyModifiers::NONE));
|
||||
}
|
||||
|
||||
fn type_char(s: &mut EditorState, c: char) {
|
||||
s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Char(c), KeyModifiers::NONE));
|
||||
}
|
||||
|
||||
/// The 0-based line an entry renders on, found by its trailing name
|
||||
/// column -- the same shape `dired_acceptance` uses.
|
||||
fn line_of(s: &EditorState, name: &str) -> usize {
|
||||
let text = active_text(s);
|
||||
for (index, line) in text.lines().enumerate() {
|
||||
if line.trim_end().ends_with(name) {
|
||||
return index;
|
||||
}
|
||||
}
|
||||
panic!("no listing line for {name:?} in:\n{text}");
|
||||
}
|
||||
|
||||
/// A project a journey can plausibly be run against.
|
||||
fn project() -> TempDir {
|
||||
let td = tempfile::tempdir().expect("tempdir");
|
||||
|
|
@ -333,11 +364,18 @@ fn journey_a_raising_resolver_suppresses_the_fallback_and_reports() {
|
|||
// Step 5 — edit immediately
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// **N11** — the journey's step-3-into-step-5 path: start on a
|
||||
/// directory, visit a listed file, and type into *that* file.
|
||||
/// **N11** — the journey's step-3-into-step-5 path, through the real
|
||||
/// input path at every step: start on a directory, press `RET` on a
|
||||
/// listed file, then type a character into it.
|
||||
///
|
||||
/// Rev 6 correction: this previously called `display_file` and
|
||||
/// `buf:insert` directly, so it stayed green with dired's `RET` binding,
|
||||
/// its entry dispatch, or the editor's self-insert path all broken —
|
||||
/// which is most of what "the journey works" is supposed to mean. Both
|
||||
/// gestures are now dispatched as keys.
|
||||
///
|
||||
/// Deliberately not a self-insert into the dired buffer, whose intercept
|
||||
/// rejects every edit — asserting an edit lands there would contradict
|
||||
/// rejects every edit: asserting an edit lands there would contradict
|
||||
/// the read-only contract rather than pin the journey.
|
||||
#[test]
|
||||
fn journey_step5_editing_a_file_reached_through_the_directory() {
|
||||
|
|
@ -345,21 +383,24 @@ fn journey_step5_editing_a_file_reached_through_the_directory() {
|
|||
let mut s = launch(td.path());
|
||||
assert!(active_name(&s).starts_with("*dired:"));
|
||||
|
||||
let target = td.path().join("alpha.txt");
|
||||
exec(
|
||||
&s,
|
||||
&format!(
|
||||
"pmacs.window.display_file({:?}, {{ select = true }})",
|
||||
target.display().to_string()
|
||||
),
|
||||
);
|
||||
// Seat on the entry, then VISIT it with the real key.
|
||||
let line = line_of(&s, "alpha.txt");
|
||||
exec(&s, &format!("pmacs.editor.move_to_line({line})"));
|
||||
press(&mut s, KeyCode::Enter);
|
||||
pump(&mut s);
|
||||
|
||||
exec(&s, "pmacs.window.buffer():insert(0, 'EDITED ')");
|
||||
assert_eq!(
|
||||
active_name(&s),
|
||||
td.path().join("alpha.txt").display().to_string(),
|
||||
"RET on a listed file must visit it"
|
||||
);
|
||||
|
||||
// And type into it with the real key.
|
||||
type_char(&mut s, 'X');
|
||||
let text = active_text(&s);
|
||||
assert!(
|
||||
text.starts_with("EDITED "),
|
||||
"the edit must land in the visited file's buffer; got {text:?}"
|
||||
text.starts_with('X'),
|
||||
"a self-insert must land in the visited file's buffer; got {text:?}"
|
||||
);
|
||||
assert!(
|
||||
buffer_count(&s) >= 2,
|
||||
|
|
@ -466,26 +507,21 @@ fn preservation_an_unreadable_file_reports_with_its_path() {
|
|||
);
|
||||
}
|
||||
|
||||
/// **P7** — a directory argument suppresses desktop restore, on the same
|
||||
/// reasoning a file argument does (Q#DS7): a positional argument means
|
||||
/// "open this", not "restore my session".
|
||||
///
|
||||
/// *Mutation:* pass `false` for `had_file` on the directory path.
|
||||
#[test]
|
||||
fn preservation_a_directory_argument_suppresses_desktop_restore() {
|
||||
let td = project();
|
||||
let mut s = launch(td.path());
|
||||
// Arm the restore AFTER startup, then confirm the startup path
|
||||
// treated its argument as a positional open: `had_file` is what
|
||||
// `run` passes, and a directory must set it.
|
||||
let had_file = true;
|
||||
s.restore_desktop_if_armed(had_file);
|
||||
assert!(
|
||||
!status(&s).contains("desktop-restore"),
|
||||
"a positional directory argument must not trigger a restore; got {:?}",
|
||||
status(&s)
|
||||
);
|
||||
}
|
||||
// **P7 — removed in rev 6, not weakened.**
|
||||
//
|
||||
// Q#JR12 said a directory argument must suppress desktop restore, and
|
||||
// rev 5 carried a pin for it. There is nothing to pin. `run` computes
|
||||
// `had_file = file.is_some()` (`editor.rs:3152`) and a directory path is
|
||||
// `Some` like any other, so the suppression is structural: no
|
||||
// directory-specific branch exists that could get it wrong, and the
|
||||
// named mutation ("pass false for `had_file` on the directory path")
|
||||
// would require inventing the branch first.
|
||||
//
|
||||
// The rev 5 test also never armed desktop restore and hard-coded
|
||||
// `had_file = true` after startup, so it asserted nothing about `run`'s
|
||||
// decision and would have passed against any implementation. Keeping a
|
||||
// green test that cannot fail is worse than having none: it reads as
|
||||
// coverage. Q#JR12 is downgraded to an observation in the framing.
|
||||
|
||||
/// **P6** — `display_file` keeps its directory-is-an-error contract and
|
||||
/// does not enter the resolver chain.
|
||||
|
|
|
|||
Loading…
Reference in New Issue