diff --git a/docs/active-work.md b/docs/active-work.md index 830ce56..71b68e9 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -265,33 +265,36 @@ also removed: this branch's "R8 NEEDS A LANE" investigation block, and durable facts are in the retired registry row and the handoff §6 census. -## Destination capture (Q#JR14 generalization) — IMPLEMENTED at `0efc8c0`, then RE-OPENED by review +## Destination capture (Q#JR14 generalization) — revision 7 IMPLEMENTED, gate green, no PR yet -**DO NOT PREPARE A PR FROM THIS LANE'S CURRENT STATE.** The mechanism -landed at `0efc8c0` with 8 pins green — and review of that -implementation found a **correctness blocker** that is still open. -Framing revisions 6 and 7 carry it; neither is implemented yet. +**The blocker review re-opened this lane for is CLOSED.** The mechanism +landed at `0efc8c0` with 8 pins green; review of that implementation +found a correctness blocker, framing revisions 6 and 7 carried it, and +revision 7's design is implemented in the commit named below with 12 +pins green. No PR yet — the lane was told not to open one. -**The blocker:** the panel profile skips checks 2–4 on the claim that a -panel result never touches a document window. **Panel placement falls -back to an ordinary document window** when the frontend is not +**The blocker was:** the panel profile skipped checks 2–4 on the claim +that a panel result never touches a document window. **Panel placement +falls back to an ordinary document window** when the frontend is not panel-capable or its side slot is dedicated -(`src/editor_core.rs:4138-4148`), so a `"panel"` commit could replace a -**newer** document with every stale-intent guard skipped. Reproduced in -review. +(`src/editor_core.rs`, `apply_placement`), so a `"panel"` commit could +replace a **newer** document with every stale-intent guard skipped. +Reproduced in review. -**Revision 6's fix was itself unsound and revision 7 replaces it.** -Revision 6 predicted the fallback at preflight, arguing the body cannot -`await`. That stops concurrent interleaving, not the body: arbitrary -synchronous Lua can dedicate the side slot *inside the callback* and -cause the fallback the preflight just ruled out. **Enforcement belongs -at the placement boundary**, and §7 now requires an -inside-the-body test that no preflight-snapshot design can pass. +**Revision 6's fix was itself unsound and revision 7 replaced it, which +is the part most worth not re-learning.** Revision 6 predicted the +fallback at preflight, arguing the body cannot `await`. That stops +concurrent interleaving, not the body: arbitrary synchronous Lua can +dedicate the side slot *inside the callback* and cause the fallback the +preflight just ruled out. **No preflight snapshot can carry this +invariant.** Enforcement is therefore at the **placement boundary**, and +§7's inside-the-body test is what no preflight-snapshot design passes. -**Also open:** an invalid-UTF-8 profile (`string.char(255)`) reaches -`to_str()` and surfaces mlua's generic conversion error instead of the +**Also closed:** an invalid-UTF-8 profile (`string.char(255)`) reached +`to_str()` and surfaced mlua's generic conversion error instead of the documented message naming the accepted values — the same reachability -class as revision 5's `Option` defect, one layer down. +class as revision 5's `Option` defect, one layer down. The +comparison is on bytes now. **Written with the lane's first commit**, per the standing correction from #171 and #215. @@ -302,24 +305,62 @@ authoritative tip** — the ref, not a SHA. Recover with `git fetch githubsucks && git checkout destination-capture`. - **Framing `docs/destination-capture-framing.md`, revision 7.** - Revisions 1–5 were approved over four review rounds; **revisions 6 - and 7 are corrections carrying the open blocker above** and have not - been implemented. -- **Implemented in two commits, and superseded in part.** `779bb02` is - the mechanism (`pmacs.window.capture_destination()`, the - `ViewDestination` rename, the profile argument); `d5a6170` is - `tests/destination_capture_acceptance.rs`. The gate line below was - green at `0efc8c0` and both preservation suites passed **unchanged** - (journey 47, dired 31) — §7's stop signal not firing rather than - being suppressed. - - **But those eight pins do NOT cover §7 as it now reads.** They were - written against revision 5's matrix, which review disproved: none of - them exercises a fallback placement, and none could — the two - fallback tests revision 6 asked for did not exist yet, and revision - 7 adds a third (the inside-the-body transition) that no - preflight-snapshot design can pass. Reading "eight pins covering §7" - off this entry is exactly the mistake it now exists to prevent. + Revisions 1–5 were approved over four review rounds; revisions 6 and 7 + are corrections carrying the blocker above, and **revision 7's design + is what the tree implements** — revision 6's preflight prediction is + NOT the shipped mechanism and must not be restored from that document. +- **Implemented in three commits.** `779bb02` is the mechanism + (`pmacs.window.capture_destination()`, the `ViewDestination` rename, + the profile argument); `d5a6170` is + `tests/destination_capture_acceptance.rs`; the revision-7 commit is + the panel-profile correction plus the invalid-UTF-8 hole. **12 pins**, + and both preservation suites pass **unchanged** (journey 47, dired 31) + — §7's stop signal not firing rather than being suppressed. +- **HOW THE PANEL PROFILE IS ENFORCED, so revision 6's version does not + get reinstated by someone reading only that document.** + - `EditorCore::display_buffer` refuses **between** `resolve_placement` + and `apply_placement` when a side request resolved to + `PlacementKind::Ordinary` under an active `"panel"` contract whose + destination fails the document preconditions + (`fallback_commit_refusal`). Refusing there means a refused fallback + mutates nothing. + - The contract (`CommitContract { destination, profile }`) rides on + the core, installed and restored by the **same** `ScopedFrontendGuard` + that scopes the frontend, so a `"panel"` profile can never outlive + the body that declared it. The field is private to the crate — Lua + cannot claim a profile for a placement it did not commit to. + - **The preflight predicate survives as an EARLY REFUSAL, not as the + guarantee.** `panel_placement_can_fall_back` still gates the + relaxation in `commit_destination_refusal`, so the statically + knowable case — a frontend that cannot render a panel at all, and + will not acquire the capability mid-body — refuses *before* the body + allocates a buffer, registers a handle and paints. That is the same + reason `commit_to` preflights at all. Both layers are pinned + separately and neither test subsumes the other. + - The four document checks live once, in + `EditorCore::document_destination_refusal`, because they are now + evaluated from two sites and two hand-written copies is how a + backstop ends up weaker than the thing it backs. + - **Three deliberate limits**, each a different decision rather than a + stricter version of this one: the **document profile is untouched** + (re-running its checks at placement would newly refuse dired's own + documented panel path — a preservation-suite stop signal); only a + **fallback** is guarded, not every `Ordinary` placement (a `"panel"` + body calling `display_file` is pinned as succeeding by + `a_captured_destination_survives_a_frontend_switch`); and the + refusal is of the **placement**, not of falling back — a `"panel"` + commit with an intact destination still degrades gracefully into the + document window. +- **Audit: nothing else relied on "a panel never touches a document".** + Four doc sites repeated the claim (`ViewDestination`'s own doc twice, + `capture_view_destination`, `ViewDestinationLua`) and were corrected; + no other code depended on it. Dired — the only Lua `commit_to` + consumer — takes the **two-argument document profile**, so all four + checks already applied to it, and it separately documents and accepts + the side-slot fallback (`builtin/runtime/dired.lua`). + `compile.lua`'s `already_in_panel` queries live state rather than + assuming, and the terminal adopter's rollback keys off + `DisplayOutcome::created_side`, already false on a fallback. - **TWO FRAMING CLAIMS THE TREE DID NOT MATCH.** Neither changed a decision; both are recorded because the framing says "counted, not estimated" and a reader will check. @@ -351,6 +392,27 @@ authoritative tip** — the ref, not a SHA. Recover with contract claim being executable rather than asserted. Dropping the frontend scope for the panel profile fails the survives-a-switch pin's panel row; dropping the no-document-window arm fails the Q#DC-4 pair. + + **Revision 7's four, each isolating a different way to get it wrong** — + and the pattern of *which* rows survive each is the evidence the layers + are independent rather than redundant: + 1. delete the `fallback_commit_refusal` call from `display_buffer` → + **only** the inside-the-body pin fails. Every other test passes, + which is exactly the hole revision 6 would have shipped. + 2. delete the `panel_placement_can_fall_back` arm from + `commit_destination_refusal` → **only** the two pre-established + fallback rows fail, and they fail on shape (a raise from the + backstop, with the body having run) rather than on outcome. + 3. make `panel_placement_can_fall_back` unconditionally `true` (the + "widen the predicate" non-fix) → the really-lands-in-the-panel pin, + the Q#DC-4 panel pin and the matrix's three panel rows all fail. + That is the profiles collapsing into one, made visible. + 4. make `fallback_commit_refusal` refuse *every* panel fallback → only + the graceful-degradation pin fails, which is the guard + over-reaching. + + And reverting the byte comparison to `to_str()?` fails the + `invalid utf-8` row with mlua's conversion error, on content. - **The public API #227 adopts against (Q#DC-5), pinned so it is a contract rather than an intention:** `pmacs.window.commit_to(dest, body [, profile])`. Profile is an diff --git a/src/editor.rs b/src/editor.rs index eb58138..c3da5bb 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -25,7 +25,7 @@ use unicode_width::UnicodeWidthStr; use crate::async_runtime::SharedAsyncRuntime; use crate::cell::{CellCoord, CellSize}; -use crate::editor_core::{EditorCore, GeometryUpdate}; +use crate::editor_core::{CommitContract, EditorCore, GeometryUpdate}; use crate::frontend::{Event, Frontend, KeyEvent, KeyEventKind, MouseEvent, install_panic_hook}; use crate::key::{Chord, display_sequence}; use crate::keymap_stack::{Action, KeyDispatcher}; @@ -119,20 +119,26 @@ impl ScopedFrontend { } /// Enter a background frontend scope, also swapping the core's - /// ambient `active_frontend`. Both are restored on drop, on every - /// exit path including a raising callback. + /// ambient `active_frontend` and publishing `contract`. All three are + /// restored on drop, on every exit path including a raising callback. + /// + /// The frontend comes from `contract.destination` rather than being + /// passed separately: a scope entered for one frontend while carrying + /// another's destination would let the placement guard check the + /// wrong window, and there is no caller that wants them to differ. pub(crate) fn enter( &self, core: &SharedCore, commit_scope: &CommitScopeActive, - frontend_id: FrontendId, + contract: CommitContract, ) -> ScopedFrontendGuard { + let frontend_id = contract.destination.frontend; let previous = self.0.replace(Some(frontend_id)); - let previous_active = { + let (previous_active, previous_contract) = { let mut core = core.borrow_mut(); let was = core.active_frontend; core.active_frontend = frontend_id; - was + (was, core.enter_commit_contract(Some(contract))) }; let previous_commit = commit_scope.0.replace(true); ScopedFrontendGuard { @@ -140,6 +146,7 @@ impl ScopedFrontend { core: core.clone(), previous, previous_active, + previous_contract, commit_scope: commit_scope.clone(), previous_commit, } @@ -151,6 +158,11 @@ pub(crate) struct ScopedFrontendGuard { core: SharedCore, previous: Option, previous_active: FrontendId, + /// The contract in force before this commit, restored with the rest + /// (Q#DC-2). Held here rather than on a separate guard so a + /// `"panel"` profile can never outlive the body that declared it and + /// govern an unrelated later display. + previous_contract: Option, /// Cleared together with the scope, so an awaiting callback cannot /// leave `await` refused after the commit ends (Q#JR14b). commit_scope: CommitScopeActive, @@ -160,7 +172,11 @@ pub(crate) struct ScopedFrontendGuard { impl Drop for ScopedFrontendGuard { fn drop(&mut self) { self.scope.0.set(self.previous); - self.core.borrow_mut().active_frontend = self.previous_active; + { + let mut core = self.core.borrow_mut(); + core.active_frontend = self.previous_active; + core.enter_commit_contract(self.previous_contract); + } self.commit_scope.0.set(self.previous_commit); } } diff --git a/src/editor_core.rs b/src/editor_core.rs index bae3137..43c6656 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -144,8 +144,9 @@ pub enum ResolvedTarget { /// result. /// /// The fields are load-bearing, and the document pair is **optional** -/// (Q#DC-4) because a panel result needs only a live frontend, so a -/// frontend whose document window has gone can still host one: +/// (Q#DC-4) because a panel result needs only a live frontend *when it +/// really lands in a panel*, so a frontend whose document window has +/// gone can still host one: /// /// * `frontend` — the scope the commit must run in. Always present. /// * `window` — the exact destination; the ambient selected window is @@ -163,9 +164,13 @@ pub enum ResolvedTarget { /// /// Which of those a commit actually requires is the **profile**, chosen /// at `pmacs.window.commit_to` rather than at capture (Q#DC-2/Q#DC-5): -/// the document profile requires all of them, the panel profile requires -/// only a live `frontend`. Capture stays profile-blind so a caller does -/// not have to know at capture time what it will do at commit time. +/// the document profile requires all of them, and the panel profile +/// requires only a live `frontend` **while its result really lands in a +/// panel**. A side request that falls back into a document window *is* a +/// document replacement, and is held to all of them at the placement +/// boundary ([`EditorCore::fallback_commit_refusal`]). Capture stays +/// profile-blind so a caller does not have to know at capture time what +/// it will do at commit time. /// /// Exposed to Lua only as nonconstructible userdata (Q#JR14d): as a /// table, the *same* value is handed to every resolver listener in turn, @@ -181,6 +186,55 @@ pub struct ViewDestination { pub buffer: Option, } +/// Which of `commit_to`'s preconditions a body actually depends on +/// (Q#DC-2). +/// +/// A **closed** set of two, not an open string namespace: a third +/// profile is a decision about what a continuation may depend on, not a +/// spelling. Chosen at `commit_to` rather than at capture, because the +/// caller knows what it is about to do only then. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CommitProfile { + /// The body replaces the captured window's buffer: **all four** + /// preflight checks apply. This is what an omitted profile means, so + /// every caller written before the profile existed keeps exactly the + /// guarantees it was written against. + Document, + /// The body puts its result in a bottom panel rather than in the + /// captured document window, and so does not depend on checks 2–4 — + /// **for as long as its result really lands in a panel**. When a side + /// request falls back into a document window the relaxation is + /// withdrawn at the placement boundary, which is the only place the + /// fallback is a fact rather than a guess + /// ([`EditorCore::display_buffer`]). + Panel, +} + +/// The contract a `commit_to` body is running under, published on the +/// core for the placement path to consult (Q#DC-2, revision 7). +/// +/// **Why this exists rather than a preflight prediction.** Revision 6 +/// tried to decide at preflight whether a `"panel"` commit's placement +/// could fall back into a document window, on the argument that nothing +/// could change in between because the body cannot `await`. Refusing +/// `await` stops another coroutine interleaving; it says nothing about +/// the body itself, which is arbitrary Lua running synchronously and can +/// change the very state the snapshot measured — obtain the panel, set +/// it `dedicated`, then request a side display. A snapshot cannot bind +/// that. The fact "this asked for a side and landed in a document +/// window" is only ever known where placement resolves, so that is where +/// the document preconditions are enforced. +/// +/// Installed and restored by the same guard that scopes the frontend, so +/// the two can never disagree about whether a commit is on the stack. +#[derive(Clone, Copy, Debug)] +pub struct CommitContract { + /// The destination the continuation captured. + pub destination: ViewDestination, + /// What that continuation declared it depends on. + pub profile: CommitProfile, +} + /// A `display_buffer` request (Q#BP3). /// /// `height` and `dedicated` are deliberately option-valued at the policy @@ -636,6 +690,14 @@ pub struct EditorCore { /// slot; the producer clears any untaken record when the fan-out /// returns. typed_edit_armed: Option<(FrontendId, TypedEditRecord)>, + /// The `commit_to` contract currently on the stack, if any (Q#DC-2). + /// + /// Private and `pub(crate)`-free on purpose: it is installed only by + /// [`crate::editor::ScopedFrontend::enter`]'s guard, which restores + /// the previous value on every exit path including a raising body. + /// Nothing outside this crate can set it, so a `"panel"` profile is + /// not something Lua can claim for a placement it did not commit to. + commit_contract: Option, } impl EditorCore { @@ -690,9 +752,24 @@ impl EditorCore { query_replace: None, typed_edit_pending: None, typed_edit_armed: None, + commit_contract: None, } } + /// Install `contract` for the duration of a `commit_to` body, + /// returning the previous one for the guard to restore. + /// + /// Crate-private and paired with the frontend scope rather than a + /// standalone setter: a contract that could be installed without + /// being restored would outlive its body and silently govern the + /// next unrelated display. + pub(crate) fn enter_commit_contract( + &mut self, + contract: Option, + ) -> Option { + std::mem::replace(&mut self.commit_contract, contract) + } + /// Build a core from raw bytes under `name`. Used by tests. /// Replaces the scratch buffer's content; the active window is /// retained. @@ -3064,11 +3141,13 @@ impl EditorCore { /// **Profile-blind and total**: it records what is there rather than /// what a caller intends to do later, and it never fails while a /// frontend id exists. A frontend with no document window yields a - /// destination carrying only `frontend` — enough for a panel commit, - /// and refused by a document commit with a reason naming the missing - /// window. Returning `None` here instead would push the caller back - /// onto ambient state, which is the misrouting the capture exists to - /// remove. + /// destination carrying only `frontend` — enough for a panel commit + /// that really places in the panel, and refused by a document commit + /// (or by a panel commit that falls back into a document window, see + /// [`Self::fallback_commit_refusal`]) with a reason naming the + /// missing window. Returning `None` here instead would push the + /// caller back onto ambient state, which is the misrouting the + /// capture exists to remove. /// /// The document pair is set or cleared **together**: a window whose /// entry has gone yields neither half, so no consumer has to handle @@ -3095,6 +3174,102 @@ impl EditorCore { } } + /// The document profile's preconditions on a captured destination — + /// Q#DC-2's checks 2, 3 and 4, plus Q#DC-4's missing-pair case. + /// + /// **One rule in one place**, because it is now evaluated from two + /// sites and they must not drift: `commit_to`'s preflight runs it + /// before the body, and [`Self::display_buffer`] runs it again when a + /// `"panel"` commit's side request actually falls back into a + /// document window. A second copy of these three checks is how the + /// backstop ends up subtly weaker than the thing it backs. + /// + /// Check 1 (the requesting frontend still has a layout) is + /// deliberately *not* here: it is shared by both profiles rather than + /// specific to the document one, and the placement path cannot fail + /// it — it is placing into that very frontend. + #[must_use] + pub fn document_destination_refusal(&self, dest: &ViewDestination) -> Option { + let Some(window) = dest.window else { + // The capture found no document window (Q#DC-4). A refusal + // rather than a raise, so it joins the others as one more + // thing the destination can fail to satisfy and an adopter + // handles it the same way. + return Some( + "destination has no document window (capture it from a frontend that has \ + one, or commit with the \"panel\" profile)" + .to_string(), + ); + }; + // 2. The destination window is still live in the frontend. + if !self + .views + .get(&dest.frontend) + .is_some_and(|view| view.layout.iter_ids().contains(&window)) + { + return Some(format!("window {} is gone", window.raw())); + } + // 3. Stale intent (Q#JR14c): the user replaced the buffer while + // the work was in flight. Their action is newer information + // than the request, so the request loses. + if self + .windows + .get(&window) + .is_some_and(|w| Some(w.buffer_id) != dest.buffer) + { + return Some(format!("window {} now shows another buffer", window.raw())); + } + // 4. Replaceability (Q#JR14f). `None` because the replacement + // does not exist yet — passing the captured buffer would + // approve a window dedicated to *it*, and the handler's + // different buffer would be refused later, after mutating. + if !self.window_accepts_buffer(window, None) { + return Some(format!("window {} is dedicated", window.raw())); + } + None + } + + /// `commit_to`'s **preflight**: what a commit under `profile` can be + /// refused for before its body runs at all (Q#DC-2). + /// + /// Ordering is the whole point of preflighting rather than validating + /// at display time: an async body mutates real state (claims a + /// buffer, registers a handle, paints) long before it reaches any + /// call that could refuse, so a late refusal leaves debris behind. + /// + /// **This is an early refusal, NOT the guarantee.** For the panel + /// profile it can only read the state that holds *now*, and the body + /// is arbitrary synchronous Lua that may change it — dedicate the + /// side slot, then request a side display. The guarantee that a + /// `"panel"` commit never replaces a newer document therefore lives + /// at the placement boundary in [`Self::display_buffer`], where the + /// fallback is a fact. What this buys is that the common case — a + /// frontend that simply cannot render a panel — refuses **before** + /// the body allocates anything. + #[must_use] + pub fn commit_destination_refusal( + &self, + dest: &ViewDestination, + profile: CommitProfile, + ) -> Option { + // 1. The requesting frontend still has a layout. Required by + // BOTH profiles, because a frontend that is gone can host + // nothing. + if !self.views.contains_key(&dest.frontend) { + return Some("requesting frontend is gone".to_string()); + } + // 2, 3 and 4 are DELIBERATELY OMITTED for a panel result that + // really lands in a panel, not overlooked (Q#DC-2): it does not + // occupy the captured document window, does not replace its + // buffer, and does not need it to exist, so each would refuse for + // a reason unrelated to what the continuation does. Every one of + // the three is pinned as NOT refusing under this profile. + if profile == CommitProfile::Panel && !self.panel_placement_can_fall_back(dest.frontend) { + return None; + } + self.document_destination_refusal(dest) + } + /// [`Self::primary_document_window`]'s buffer, falling back to the /// focused window's when the layout is degenerate. #[must_use] @@ -3854,6 +4029,11 @@ impl EditorCore { .ok_or_else(|| format!("frontend {fid:?} has no window layout"))? .active; let placement = self.resolve_placement(fid, request)?; + // THE PLACEMENT BOUNDARY (Q#DC-2, revision 7). Refuse before + // `apply_placement` so a refused fallback mutates nothing. + if let Some(reason) = self.fallback_commit_refusal(request, &placement) { + return Err(reason); + } self.apply_placement(fid, request, &placement)?; let select = request .select @@ -3979,6 +4159,112 @@ impl EditorCore { .ok_or_else(|| "display_file: no eligible document window is available".into()) } + /// Whether a `{side = ...}` request in `fid` would fall back into an + /// ordinary document window **given the state right now** (Q#DC-2). + /// + /// Adjacent to [`Self::resolve_placement`] because that is the rule + /// it predicts, and a prediction that drifts from the rule is worse + /// than none. The two fallback arms, in that function's own order: + /// + /// 1. **step 2's capability guard** — `side` is honoured only on a + /// `panel_capable` frontend; without the capability the request + /// falls through to step 3's ordinary policy (Q#BP13). + /// 2. **step 2's dedicated arm** — the one side slot exists but is + /// dedicated, and a second one is never created, so a different + /// buffer falls through instead (Q#BP3 2.iii). + /// + /// **A PREDICTION, AND ONLY USED AS ONE.** This is consulted by + /// [`Self::commit_destination_refusal`] to refuse the statically + /// knowable case *before* a body allocates anything — a frontend that + /// cannot render a panel at all will not acquire the capability + /// mid-body. It is **not** what makes the panel profile safe. A + /// `commit_to` body is arbitrary synchronous Lua and can dedicate the + /// side slot itself between this answer and the placement it + /// describes; refusing `await` prevents another coroutine + /// interleaving, not the body rewriting the state it was measured + /// against. The guarantee is enforced where the fallback is a fact, + /// in [`Self::fallback_commit_refusal`]. + /// + /// Arm 2 is answered **conservatively**: `resolve_placement` falls + /// back only when the arriving buffer differs from the dedicated one, + /// and at preflight the body has not chosen a buffer yet. + /// + /// A frontend with no view answers `false`: where placement would + /// land is moot when there is nothing to place into, and + /// `commit_destination_refusal` has already refused that case by its + /// first check. + #[must_use] + pub fn panel_placement_can_fall_back(&self, fid: FrontendId) -> bool { + let Some(view) = self.views.get(&fid) else { + return false; + }; + if !view.panel_capable { + return true; + } + self.side_window_for(fid) + .and_then(|side| self.windows.get(&side)) + .is_some_and(|side| side.params.dedicated) + } + + /// **The guarantee** behind the `"panel"` commit profile (Q#DC-2, + /// revision 7): a side request that actually fell back into a + /// document window must satisfy the document preconditions. + /// + /// Reaching [`PlacementKind::Ordinary`] while a side was REQUESTED is + /// exactly the fallback [`Self::apply_placement`] documents — not + /// panel-capable, or the one slot is dedicated elsewhere — and the + /// result is then installed into a **document** window. A `"panel"` + /// commit that skipped checks 2–4 on the strength of "a panel never + /// touches a document window" would, right here, replace a document + /// view with no stale-intent guard at all: capture A, the user opens + /// B, the continuation lands, B is gone. That is the failure + /// `commit_to` exists to prevent, arrived at through the profile + /// meant to be the safe one. + /// + /// **Why here and not at preflight.** This is the first moment the + /// fallback is a *fact*. A preflight snapshot cannot bind it: the + /// body is arbitrary synchronous Lua and may create the very + /// condition — take the panel, set it `dedicated`, then ask for a + /// side — after the snapshot was taken. Refusing `await` inside the + /// commit scope stops a *second coroutine* interleaving; it places no + /// restriction on the body's own statements. + /// + /// Three deliberate limits, each of which would be a different + /// decision rather than a stricter version of this one: + /// + /// * **The document profile is untouched.** Its preflight already ran + /// these checks against the same destination, and re-running them + /// here would newly refuse dired's own panel path, which documents + /// and accepts the fallback (`builtin/runtime/dired.lua`). + /// * **Only a fallback, not every document placement.** A panel-profile + /// body that displays into a document window *without asking for a + /// side* has mislabelled its profile; it has not exercised this + /// relaxation. Widening to every [`PlacementKind::Ordinary`] would + /// also refuse a `"panel"` commit whose body calls `display_file`, + /// which is pinned as succeeding. + /// * **Refusing the placement, not the fallback.** Falling back is + /// deliberate graceful degradation for a frontend without panel + /// capability; a `"panel"` commit whose destination is still valid + /// falls back and lands exactly as it does today. The profile + /// relaxes checks; it does not get to move where a result goes. + fn fallback_commit_refusal( + &self, + request: &DisplayRequest, + placement: &Placement, + ) -> Option { + if request.side.is_none() || !matches!(placement.kind, PlacementKind::Ordinary) { + return None; + } + let contract = self.commit_contract.as_ref()?; + if contract.profile != CommitProfile::Panel { + return None; + } + let reason = self.document_destination_refusal(&contract.destination)?; + Some(format!( + "display: this \"panel\" commit fell back to a document window, and {reason}" + )) + } + /// Q#BP3's precedence: exact target, then side affinity, then /// ordinary reuse. Placement affinity precedes generic reuse — /// otherwise a persistent `*compilation*` buffer already visible in a diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index d4386d5..6679507 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -4258,8 +4258,9 @@ fn install_path_module(lua: &Lua) -> mlua::Result { /// /// `window()` returns **nil** when the capturing frontend had no /// document window (Q#DC-4) — such a destination is still commitable -/// under the panel profile, so the accessor reports the absence rather -/// than inventing an id. +/// under the panel profile wherever that profile's relaxation actually +/// applies, so the accessor reports the absence rather than inventing an +/// id. pub(crate) struct ViewDestinationLua(pub(crate) crate::editor_core::ViewDestination); impl mlua::UserData for ViewDestinationLua { diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index 11e92b7..d9051a0 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -34,7 +34,9 @@ use mlua::{Lua, Table, Value}; use super::{BufferIdLua, SharedCore, config_u32, run_hook_if_defined}; -use crate::editor_core::{DisplayOutcome, DisplayRequest, HookKind, QuitOutcome}; +use crate::editor_core::{ + CommitContract, CommitProfile, DisplayOutcome, DisplayRequest, HookKind, QuitOutcome, +}; use crate::protocol::FrontendId; use crate::window::{DEFAULT_PANEL_ROWS, MIN_WINDOW_OUTER_ROWS, Side, WindowId}; @@ -63,27 +65,6 @@ pub(crate) fn acting_frontend(lua: &Lua, core: &SharedCore) -> FrontendId { .unwrap_or_else(|| core.borrow().active_frontend_key()) } -/// Which of `commit_to`'s preconditions a body actually depends on -/// (Q#DC-2). -/// -/// A **closed** set of two, not an open string namespace: a third -/// profile is a decision about what a continuation may depend on, not a -/// spelling. Chosen at `commit_to` rather than at capture, because the -/// caller knows what it is about to do only then. -#[derive(Clone, Copy, PartialEq, Eq)] -enum CommitProfile { - /// The body replaces the captured window's buffer: **all four** - /// preflight checks apply. This is what an omitted profile means, - /// so every caller written before the profile existed keeps exactly - /// the guarantees it was written against. - Document, - /// The body puts its result somewhere that is not the captured - /// document window — a bottom panel, typically. Only the "requesting - /// frontend still has a layout" check applies; see the preflight for - /// why each of the other three is *deliberately* omitted. - Panel, -} - /// One message for every bad profile — an unrecognized string and a /// non-string alike (Q#DC-5). /// @@ -106,12 +87,19 @@ const BAD_COMMIT_PROFILE: &str = "pmacs.window.commit_to: profile must be the st /// threading an optional variable produces `commit_to(dest, body, nil)`, /// and a third behaviour there would stay invisible until someone hit /// it. +/// +/// The comparison is on **bytes**, for the same reachability reason one +/// layer down. A Lua string is a byte string, not UTF-8, so +/// `commit_to(dest, body, string.char(255))` fails a `to_str()` +/// conversion and surfaces mlua's generic UTF-8 error *before* the +/// message below is ever constructed. An invalid-UTF-8 profile is a bad +/// profile like any other and gets the documented refusal. fn commit_profile(value: &Value) -> mlua::Result { match value { Value::Nil => Ok(CommitProfile::Document), - Value::String(name) => match &*name.to_str()? { - "document" => Ok(CommitProfile::Document), - "panel" => Ok(CommitProfile::Panel), + Value::String(name) => match name.as_bytes().as_ref() { + b"document" => Ok(CommitProfile::Document), + b"panel" => Ok(CommitProfile::Panel), // An unrecognized profile ERRORS rather than falling back to // the document one: a fallback would silently hand a caller // stricter or looser checks than it asked for, which is the @@ -549,70 +537,22 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result // by reading this signature. let profile = commit_profile(&profile)?; - let refusal = { - let core = cc.borrow(); - // 1. The requesting frontend still has a layout. - // Required by BOTH profiles: it is the whole - // of the panel profile (Q#DC-2), because a - // frontend that is gone can host nothing. - if !core.views.contains_key(&dest.frontend) { - Some("requesting frontend is gone".to_string()) - } else if profile == CommitProfile::Panel { - // 2, 3 and 4 are DELIBERATELY OMITTED here, - // not overlooked (Q#DC-2). A panel result - // does not occupy the captured document - // window, does not replace its buffer, and - // does not need it to exist --- so each of - // those checks would refuse for a reason - // unrelated to what the continuation does, - // and a refusal a user cannot explain is how - // a mechanism gets worked around. Every one - // of the three is pinned as NOT refusing - // under this profile. - None - } else if let Some(window) = dest.window { - if !core - .views - .get(&dest.frontend) - .is_some_and(|view| view.layout.iter_ids().contains(&window)) - { - // 2. The destination window is still live in it. - Some(format!("window {} is gone", window.raw())) - } else if core - .windows - .get(&window) - .is_some_and(|w| Some(w.buffer_id) != dest.buffer) - { - // 3. Stale intent (Q#JR14c): the user - // replaced the buffer while the work was - // in flight. Their action is newer - // information than the request, so the - // request loses. - Some(format!("window {} now shows another buffer", window.raw())) - } else if !core.window_accepts_buffer(window, None) { - // 4. Replaceability (Q#JR14f). `None` - // because the replacement does not exist - // yet — passing the captured buffer would - // approve a window dedicated to *it*, and - // the handler's different buffer would be - // refused later, after mutating. - Some(format!("window {} is dedicated", window.raw())) - } else { - None - } - } else { - // The capture found no document window - // (Q#DC-4). A refusal rather than a raise, so - // it joins the four above as one more thing - // the destination can fail to satisfy and an - // adopter handles it the same way. - Some( - "destination has no document window (capture it from a frontend \ - that has one, or commit with the \"panel\" profile)" - .to_string(), - ) - } - }; + // The preflight itself lives on the core + // (`commit_destination_refusal`), because the panel + // profile's relaxation now has a SECOND evaluation + // site --- the placement boundary, where a fallback + // into a document window stops being a prediction and + // becomes a fact --- and two hand-written copies of + // the same three checks is how the backstop ends up + // weaker than the thing it backs. + // + // What survives here, and only here: an early refusal + // costs the body nothing, so the statically knowable + // case (a frontend that cannot render a panel at all) + // never reaches the body's buffer creation. The + // GUARANTEE is not this call; see + // `EditorCore::fallback_commit_refusal`. + let refusal = cc.borrow().commit_destination_refusal(&dest, profile); if let Some(reason) = refusal { let mut out = mlua::MultiValue::new(); out.push_back(mlua::Value::String(lua.create_string(reason.as_bytes())?)); @@ -636,13 +576,24 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result ) })? .clone(); - // Both the override and the core's ambient - // `active_frontend` are restored when this guard - // drops -- on the normal return AND on a raising - // callback, which is why the result is captured - // rather than `?`-propagated through the drop. + // The override, the core's ambient `active_frontend`, + // and the CONTRACT below are all restored when this + // guard drops -- on the normal return AND on a + // raising callback, which is why the result is + // captured rather than `?`-propagated through the + // drop. The contract rides with the scope because the + // placement boundary needs to know, for every display + // this body performs, which destination and which + // profile it is running under. let result = { - let _guard = scope.enter(&cc, &commit, dest.frontend); + let _guard = scope.enter( + &cc, + &commit, + CommitContract { + destination: dest, + profile, + }, + ); body.call::(()) }; let mut out = result?; diff --git a/tests/destination_capture_acceptance.rs b/tests/destination_capture_acceptance.rs index 534d90c..842b39e 100644 --- a/tests/destination_capture_acceptance.rs +++ b/tests/destination_capture_acceptance.rs @@ -11,7 +11,7 @@ //! it lands (§8). Every test here therefore drives the Lua surface //! directly rather than through a consumer. //! -//! Two disciplines it keeps: +//! Three disciplines it keeps: //! //! * **Every "not applicable" cell in Q#DC-2's preflight matrix is //! asserted as NOT refusing**, not merely left untested. A check @@ -20,6 +20,19 @@ //! * **A refusal is asserted on its reason**, never on the mere fact //! that something failed. `commit_to` has five distinct refusals and a //! raise; "it errored" would pass on any of the wrong ones. +//! * **The panel profile's relaxation is pinned at BOTH of its +//! evaluation sites** (revision 7). The preflight is an early refusal +//! that spares the body; the guarantee is enforced where placement +//! resolves, because the body is arbitrary synchronous Lua and can +//! create the fallback *after* any snapshot was taken — refusing +//! `await` stops a second coroutine interleaving, not the body's own +//! statements. Three tests carry that split and none subsumes another: +//! `a_panel_commit_that_falls_back_runs_the_document_preflight` (the +//! body must not run), +//! `a_panel_commit_whose_body_creates_the_fallback_is_refused_at_placement` +//! (the result must not land), and +//! `a_panel_commit_that_falls_back_with_a_valid_destination_still_lands` +//! (falling back is still graceful degradation, not an error). //! //! `tests/journey_acceptance.rs` and `tests/dired_acceptance.rs` are the //! preservation half of the same §7 and are run alongside this suite: @@ -68,6 +81,15 @@ fn buffer_in(s: &EditorState, window: WindowId) -> Option { s.core.borrow().windows.get(&window).map(|w| w.buffer_id) } +/// A window's buffer **by name**, so a placement assertion reads as +/// "`*result*` went to the panel" rather than as two opaque ids. +fn name_in(s: &EditorState, window: WindowId) -> String { + let buffer = buffer_in(s, window).expect("window is live"); + let core = s.core.borrow(); + let registry = core.registry.borrow(); + registry.get(buffer).expect("buffer").name().to_string() +} + fn local_window(s: &EditorState) -> WindowId { s.core .borrow() @@ -151,12 +173,15 @@ fn capture(s: &EditorState) { ); } -/// Run `body` under `profile` and report `(ok, reason)`. +/// Run a body that also executes `also` under `profile`, reporting +/// `(ok, reason)`. /// /// `profile` is spliced as a Lua expression, so a caller can pass /// `"nil"`, `"'panel'"`, `"42"` — the argument-shape distinctions -/// Q#DC-5 turns on are exactly what this suite has to vary. -fn commit(s: &EditorState, profile: Option<&str>) { +/// Q#DC-5 turns on are exactly what this suite has to vary. `also` is +/// spliced as Lua statements, for the rows that must observe *where* an +/// accepted commit put its result and not merely that it was accepted. +fn commit_body(s: &EditorState, profile: Option<&str>, also: &str) { let call = match profile { Some(profile) => format!("pmacs.window.commit_to(dest, body, {profile})"), None => "pmacs.window.commit_to(dest, body)".to_string(), @@ -165,7 +190,7 @@ fn commit(s: &EditorState, profile: Option<&str>) { s, &format!( "ran = false - local body = function() ran = true end + local body = function() ran = true; {also} end raised = nil local caught, a, b = pcall(function() return {call} end) if caught then ok, reason = a, b @@ -174,6 +199,11 @@ fn commit(s: &EditorState, profile: Option<&str>) { ); } +/// Run an inert body under `profile` and report `(ok, reason)`. +fn commit(s: &EditorState, profile: Option<&str>) { + commit_body(s, profile, ""); +} + fn ok(s: &EditorState) -> bool { eval(s, "return ok == true") } @@ -413,6 +443,342 @@ fn the_preflight_matrix_holds_in_both_profiles() { } } +// --------------------------------------------------------------------------- +// §7 — the panel profile's relaxation is CONDITIONAL (Q#DC-2, revision 7) +// --------------------------------------------------------------------------- + +/// The Lua a `"panel"` continuation runs: put a result buffer in the +/// bottom panel. It is the shape `listview.open` resolves to by default +/// (`builtin/runtime/listview.lua`), and the shape git's `*git-status*` +/// adoption will take. +const PANEL_BODY: &str = "pmacs.window.display(pmacs.buffer.create('*result*'), \ + { side = 'bottom' })"; + +/// Arrange one of the two reasons a side request falls back into a +/// document window, and assert the arrangement took. +/// +/// The two arms are independent branches of +/// `EditorCore::resolve_placement`, so a fix that handled only one would +/// leave the other live. Every fallback test below drives both. +fn arrange_fallback(s: &EditorState, cause: &str) { + if cause == "not panel-capable" { + // Q#BP13's capability gate: `side` is honoured only on a + // panel-capable frontend. + s.core + .borrow_mut() + .views + .get_mut(&FrontendId::LOCAL) + .expect("LOCAL view") + .panel_capable = false; + } else { + // Q#BP3 2.iii: the one side slot is dedicated to another buffer, + // and a second panel is never created. + exec( + s, + "pmacs.window.display(pmacs.buffer.create('*pinned*'), + { side = 'bottom', dedicated = true, select = false })", + ); + assert!( + s.core.borrow().side_window_for(FrontendId::LOCAL).is_some(), + "{cause}: the arrangement must actually create the side slot" + ); + } +} + +/// **N** — a `"panel"` commit whose placement *already* falls back is +/// refused **before its body runs**, on the stale-intent reason. +/// +/// The defect: the panel column dropped checks 2–4 on the claim that a +/// panel result never touches a document window — but panel placement +/// falls back to an ordinary document window and then *installs the +/// result there* (`EditorCore::apply_placement` says so in its own +/// comment). The relaxation therefore handed a `"panel"` commit +/// permission to overwrite a document view with no stale-intent guard: +/// capture A, the user opens B, the continuation lands and B is gone. +/// +/// **This is the EARLY half, not the guarantee.** It is served by +/// `EditorCore::commit_destination_refusal` consulting +/// `panel_placement_can_fall_back`, which can only read the state that +/// holds *now*. The reason that is worth having anyway is the same reason +/// `commit_to` preflights at all: a body allocates a buffer, registers a +/// handle and paints long before it reaches any call that could refuse, +/// so refusing here leaves no debris. A frontend that cannot render a +/// panel will not acquire the capability mid-body, which is exactly the +/// case this catches. +/// +/// The guarantee — for the case a snapshot **cannot** catch, where the +/// body creates the fallback itself — is +/// `a_panel_commit_whose_body_creates_the_fallback_is_refused_at_placement`. +/// Neither test subsumes the other: this one pins that nothing runs, that +/// one pins that nothing lands. +/// +/// Each row asserts four things: the commit **refuses**, it refuses for +/// the stale-intent reason (not incidentally), the body never ran, and +/// the newer buffer is still there. +/// +/// *Mutation:* delete the `panel_placement_can_fall_back` arm from +/// `commit_destination_refusal`. Both rows fail — the body runs, and the +/// placement backstop then refuses as a *raise*, so `ok`/`ran`/`reason` +/// all move. +#[test] +fn a_panel_commit_that_falls_back_runs_the_document_preflight() { + for cause in ["not panel-capable", "side slot dedicated elsewhere"] { + let s = editor(); + + // Arrange the fallback cause BEFORE capturing, so the preflight + // can see it — which is exactly what distinguishes this test from + // the body-induced one below. + arrange_fallback(&s, cause); + + capture(&s); + let doc = local_window(&s); + assert_eq!( + eval::>(&s, "return dest:window()"), + Some(doc.raw()), + "{cause}: the capture must name the document window, not the panel" + ); + + // The user replaces the captured buffer while the work is in + // flight: `*newer*` is newer information than the request. + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "{cause}: the arrangement must make the captured window stale" + ); + + commit_body(&s, Some("'panel'"), PANEL_BODY); + + assert_eq!( + raised(&s), + None, + "{cause}: a precondition is a refusal, not a raise" + ); + assert!( + !ok(&s), + "{cause}: a \"panel\" commit that lands in a DOCUMENT window must run the \ + document preflight -- the relaxation is conditional on the placement really \ + being a panel" + ); + assert!( + reason(&s).contains("now shows another buffer"), + "{cause}: and refuse on stale intent; got {:?}", + reason(&s) + ); + assert!(!ran(&s), "{cause}: the callback must not run"); + assert_eq!( + name_in(&s, doc), + "*newer*", + "{cause}: the user's newer buffer must survive -- this is the assertion that \ + fails loudest when the guard is removed" + ); + } +} + +/// **N** — the case no preflight snapshot can catch: the **body itself** +/// creates the fallback, and the refusal still fires. +/// +/// This is why the guarantee moved to the placement boundary. Revision 6 +/// argued that a prediction taken at preflight could not go stale, +/// because `commit_to`'s body cannot `await`. Refusing `await` prevents +/// another *coroutine* interleaving; it places no restriction on the body +/// itself, which is arbitrary Lua running synchronously: +/// +/// ```lua +/// pmacs.window.set_params(pmacs.window.panel(), { dedicated = true }) +/// pmacs.window.display(result, { side = "bottom" }) +/// ``` +/// +/// Two statements. The first invalidates the prediction, the second cashes +/// it in. The arrangement here is deliberately the **inverse** of the +/// preflight rows: an undedicated panel exists, so the prediction says +/// "this will land in the panel", the relaxation applies, and the body +/// runs. Only when placement resolves is the fallback a fact. +/// +/// What it asserts, and why each is load-bearing: +/// +/// * the body **did** run — otherwise the test would be re-proving the +/// preflight and this whole case would be untested; +/// * the refusal arrives as a **raise** from `display`, since the body was +/// already running and there is no `(false, reason)` left to return — +/// asserted on content, and it names both the fallback and the +/// stale-intent reason; +/// * `*newer*` is **still in the document window**. That is the actual +/// user-visible guarantee; everything above it is mechanism. +/// +/// *Mutation:* delete the `fallback_commit_refusal` call from +/// `display_buffer`. This test fails on all three; every other test in +/// this file still passes, which is precisely the hole revision 6 left. +#[test] +fn a_panel_commit_whose_body_creates_the_fallback_is_refused_at_placement() { + let s = editor(); + + // A REUSABLE panel: undedicated, so the preflight prediction says + // this frontend places side requests in the panel. + exec( + &s, + "pmacs.window.display(pmacs.buffer.create('*pinned*'), + { side = 'bottom', dedicated = false, select = false })", + ); + capture(&s); + let doc = local_window(&s); + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "the arrangement must make the captured window stale" + ); + + commit_body( + &s, + Some("'panel'"), + &format!( + "pmacs.window.set_params(pmacs.window.panel(), {{ dedicated = true }}) + {PANEL_BODY}" + ), + ); + + assert!( + ran(&s), + "the body must have run -- the preflight could not have known, and a test where \ + it did not run would be re-proving the preflight" + ); + let raised = raised(&s).expect( + "the refusal arrives as a raise: the body was already running, so there is no \ + (false, reason) return left to make", + ); + assert!( + raised.contains("fell back to a document window"), + "the message must name what happened; got {raised:?}" + ); + assert!( + raised.contains("now shows another buffer"), + "and which document precondition failed; got {raised:?}" + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "the user's newer buffer must survive -- this is the guarantee, and it is what a \ + preflight-only design cannot provide" + ); +} + +/// **P** — a `"panel"` commit that falls back with a **still-valid** +/// destination lands in the document window, exactly as it does today. +/// +/// The guard refuses on *staleness*, not on *falling back*. Falling back +/// is deliberate graceful degradation for a frontend that cannot render a +/// panel (`EditorCore::apply_placement`), and turning it into an error +/// would regress every consumer that works today on such a frontend — a +/// much bigger behaviour change than the defect being fixed. +/// +/// Both causes, and asserted on **where the result landed** rather than +/// on the commit merely being accepted: a design that accepted the commit +/// and then dropped the display on the floor would pass a weaker version +/// of this. +/// +/// *Mutation:* make `fallback_commit_refusal` refuse whenever a `"panel"` +/// commit falls back, instead of only when a document precondition fails. +/// Both rows fail here; every refusal test still passes, which is what +/// makes this the pin that stops the fix over-reaching. +#[test] +fn a_panel_commit_that_falls_back_with_a_valid_destination_still_lands() { + for cause in ["not panel-capable", "side slot dedicated elsewhere"] { + let s = editor(); + arrange_fallback(&s, cause); + capture(&s); + let doc = local_window(&s); + + // No staleness: the captured window still holds what it held. + commit_body(&s, Some("'panel'"), PANEL_BODY); + + assert_eq!(raised(&s), None, "{cause}: the commit must not raise"); + assert!( + ok(&s), + "{cause}: a fallback with an intact destination is graceful degradation, not \ + an error; got refusal {:?}", + reason(&s) + ); + assert!(ran(&s), "{cause}: the callback must run"); + assert_eq!( + name_in(&s, doc), + "*result*", + "{cause}: and the result really must land in the document window it fell \ + back to" + ); + } +} + +/// **P** — a `"panel"` commit that really lands in the panel still skips +/// checks 2–4. +/// +/// The other half of the correction, and it is not optional coverage. +/// The cheapest way to close the fallback hole is to make the panel +/// profile run the document preflight unconditionally — which passes +/// every fallback row above while quietly collapsing the two profiles +/// into one, leaving the whole parameterization buying nothing and +/// `git.status` refused for a document-window change unrelated to where +/// its panel goes. +/// +/// Deliberately arranged in the **same stale-intent state** the fallback +/// rows refuse on, so the only difference between this test and those is +/// whether the placement is really a panel. And it asserts *where* the +/// result went, not merely that the commit was accepted: an accepted +/// commit that still overwrote the document window would be the same +/// defect wearing a `true`. +/// +/// *Mutation:* widen the relaxation's condition back — i.e. make +/// `panel_placement_can_fall_back` return `true` unconditionally, or run +/// the document preflight for every `"panel"` commit. This fails on the +/// refusal; the fallback rows above still pass. **This is the pin that +/// makes "collapse the two profiles into one" a visible design change +/// rather than a quiet implementation choice.** +#[test] +fn a_panel_commit_that_really_lands_in_the_panel_keeps_its_relaxation() { + let s = editor(); + capture(&s); + let doc = local_window(&s); + + // Exactly the state the fallback rows refuse on. + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*newer*'))", + ); + + commit_body(&s, Some("'panel'"), PANEL_BODY); + + assert!( + ok(&s), + "a panel-capable frontend with no dedicated side slot really places in the \ + panel, so checks 2-4 stay omitted; got refusal {:?}", + reason(&s) + ); + assert!(ran(&s), "and the callback must run"); + + let panel = s + .core + .borrow() + .side_window_for(FrontendId::LOCAL) + .expect("the commit must have created the side window"); + assert_eq!( + name_in(&s, panel), + "*result*", + "the result must land in the PANEL -- an accepted commit that fell back would \ + be the same defect with a `true` in front of it" + ); + assert_eq!( + name_in(&s, doc), + "*newer*", + "and the captured document window must be untouched" + ); +} + // --------------------------------------------------------------------------- // §7 — the profile argument (Q#DC-5) // --------------------------------------------------------------------------- @@ -507,8 +873,16 @@ fn an_explicit_nil_profile_is_the_document_profile() { /// against the string case's message, not merely on "an error /// occurred". /// +/// **The `invalid utf-8` row is the same reachability class one layer +/// down.** A Lua string is a *byte* string, so `string.char(255)` is a +/// perfectly ordinary `Value::String` that a `to_str()` inside the body +/// still fails to convert — surfacing mlua's generic UTF-8 error before +/// the documented message is ever constructed. Accepting `Value` is not +/// enough on its own; the comparison has to be on bytes. +/// /// *Mutation:* retype the argument to `Option`. The number and -/// table rows fail. +/// table rows fail. *Second mutation:* compare via `name.to_str()?`. The +/// `invalid utf-8` row fails. #[test] fn a_bad_profile_is_refused_by_one_message_that_names_the_accepted_values() { let mut messages = Vec::new(); @@ -517,6 +891,7 @@ fn a_bad_profile_is_refused_by_one_message_that_names_the_accepted_values() { ("number", "42"), ("table", "{}"), ("boolean", "true"), + ("invalid utf-8", "string.char(255)"), ] { let s = editor(); capture(&s);