diff --git a/docs/active-work.md b/docs/active-work.md index f462c16..fb835a6 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -137,13 +137,52 @@ If it does not, stop and repair the remote/fetch configuration. the viewport width, so any text equality over it is vacuously true. Emit `\r\n`, and guard text comparisons with a non-empty assertion the way the daemon pin guards on `!panel_hidden`. +- **Round-2 self-review caught a regression the round-2 commit + introduced**, in the change it labelled "minor": routing + `pmacs.window.buffer()`'s **no-argument** arm through the fid-scoped + `selected_window` validator made it **fallible**, and + `acting_frontend` can name a frontend with **no registered view** (a + bare `dispatch_key` from an unattached peer does exactly that). The + runtime calls that function on ordinary edits from `killring`, + `syntax`, `autosave`, `pair`, `indent` and `comment` **without + `pcall`**, so the raise never surfaced as an error — it silently + dropped the operation. `kill_ring_acceptance` went 30/30 → 25/5 + (`frontend_detached_drops_per_frontend_state`: "B has kill state"). + The no-arg arm is back on ambient `active_buffer_id()` and documented + as deliberately infallible; the explicit-window arm keeps its Q#BP11 + validation. New **acc19c** pins it through the real path (a + `buffer.after-edit` subscriber during a viewless peer's `dispatch_key`) + and bites against the regressing commit. + Generalizes: **a "uniformity" cleanup that changes a function's + fallibility is not minor** — check every caller's error discipline + first, and remember that an ambient resolver's fallback IS its + contract. - Verification on this branch: `cargo fmt --check` clean; strict workspace Clippy clean; 1,817 default + 1,994 CRDT library tests; - `bottom_panel_stage1_acceptance` 45/45; vterm Stage 1 9; M4 121; required GPU 152; - `gpu_initial_target_acceptance` 14 CRDT; compile 67; vterm Stage 2 4 / - Stage 3 5; folding Stage 2 48; statusline 7; listview 6; desktop 11; - **workspace sweep 3,128 passed, zero failures**; `git diff --check` - clean. + `bottom_panel_stage1_acceptance` 46/46; kill ring 30 default + 30 CRDT; + vterm Stage 1 9 default + 10 CRDT; M4 121; required GPU 152; + compile 67; vterm Stage 2 4 / Stage 3 5 (7 CRDT); folding Stage 2 48; + statusline 7; listview 6; + **isolated-config workspace sweep 3,130 passed across 89 suites, zero + failures**; `git diff --check` clean. + - **Run the sweep with an isolated `XDG_CONFIG_HOME`.** The real + `~/.config/pmacs/init.lua` on this desktop calls + `pmacs.packages.install_local(...)`, so every editor the sweep builds + races on one shared install root; a losing race sets a status message + that leaks into the mode line and breaks + `folding_stage2_acceptance::unfolded_frame_is_identical_to_the_pre_folding_baseline`, + which compares whole painted frames. Standalone it is 48/48. This + generalizes the known `compile_mode_acceptance` real-config trap: + any suite that paints the status area inherits it. + - **A latent pre-existing `main` bug surfaced while gating and is NOT + this branch's**: `buffer::tests::proptests::rope_matches_crdt_projection_after_arbitrary_edits` + fails on `main` @ `352bf0b` with `ops = [Insert(0,"a"), + Insert(0,"aaa"), Replace(0,1,"a"), Undo]` — undo of a textually-null + `Replace` returns a no-op edit result still carrying `crdt_op = + Some`, violating the suite's own shape invariant. `src/buffer.rs` is + byte-identical here, and the seed was deliberately **not** committed + (it would make an unrelated failure deterministically red on this + PR). Needs its own lane. - Durable test lesson from this round: `TerminalViewStatus.scroll_offset` is documented as the retained rows between **this viewport** and the live tail, so it necessarily tracks the viewport height. Asserting it diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 0d585ea..29e3b47 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -12357,9 +12357,9 @@ fn install_window_module(lua: &Lua, core: &SharedCore) -> mlua::Result { { let cc = core.clone(); - // With no argument: the selected window's buffer (unchanged). - // With an explicit window id: that window's buffer, validated - // against the acting frontend's layout like every other + // With no argument: the ambient active buffer, exactly as before + // this arc. With an explicit window id: that window's buffer, + // validated against the acting frontend's layout like every other // `WindowId`-taking operation (bottom-panel arc, Q#BP11) — an // adopter has to be able to ask "is my buffer the one in the // panel" without first selecting the panel. @@ -12367,14 +12367,24 @@ fn install_window_module(lua: &Lua, core: &SharedCore) -> mlua::Result
{ "buffer", lua.create_function( move |lua, target: Option| -> mlua::Result { - // Both arms resolve through the ACTING frontend, using - // the same validator the rest of the window surface - // does — no ambient `active_buffer_id()` asymmetry. - let fid = window_panel::acting_frontend(lua, &cc); - let id = match target { - Some(raw) => window_panel::lookup_window(&cc, fid, raw)?, - None => window_panel::selected_window(&cc, fid)?, + // The no-arg arm deliberately stays on ambient + // `active_buffer_id()`, and stays INFALLIBLE. This is not + // the asymmetry it looks like: dispatch sets + // `active_frontend` to the acting frontend before running a + // command, so the two agree on every real path — while + // `acting_frontend` can additionally name a frontend that + // has no registered view, where a `views`-keyed lookup + // raises instead of answering. `killring`, `syntax`, + // `autosave`, `pair`, `indent` and `comment` all call this + // on ordinary edits without `pcall`, so a raise here does + // not surface as an error — it silently drops the + // operation (it lost a whole kill in `kill_ring_acceptance` + // when this arm was routed through `selected_window`). + let Some(raw) = target else { + return Ok(BufferIdLua(cc.borrow().active_buffer_id())); }; + let fid = window_panel::acting_frontend(lua, &cc); + let id = window_panel::lookup_window(&cc, fid, raw)?; cc.borrow() .windows .get(&id) diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index e358351..fb3a4ad 100644 --- a/tests/bottom_panel_stage1_acceptance.rs +++ b/tests/bottom_panel_stage1_acceptance.rs @@ -1369,6 +1369,58 @@ fn acc19b_recompile_reuses_the_panel_instead_of_duplicating_into_the_document() ); } +/// `pmacs.window.buffer()` with NO argument must stay **infallible**. +/// +/// The optional window argument this arc added is validated against the +/// acting frontend's layout, and it is tempting to make the no-arg arm +/// symmetric by resolving it the same way. That silently breaks the +/// runtime: `acting_frontend` follows the interactive origin, which can +/// name a frontend with **no registered view** (as a bare +/// `dispatch_key` from a peer does), where a `views`-keyed lookup raises +/// instead of answering — and `killring`, `syntax`, `autosave`, `pair`, +/// `indent` and `comment` all call this on ordinary edits without +/// `pcall`, so the raise does not surface as an error, it just drops the +/// operation. Routing it through `selected_window` lost an entire kill in +/// `kill_ring_acceptance`. +#[test] +fn acc19c_window_buffer_stays_infallible_for_an_acting_frontend_without_a_view() { + let mut s = editor(); + let ambient = s.core.borrow().active_buffer_id(); + exec( + &s, + // A `buffer.after-edit` subscriber is the real shape: this is + // where syntax.lua, pair.lua and comment.lua each call + // `pmacs.window.buffer()` on every ordinary edit. + "SEEN = nil; ERR = nil \ + pmacs.hook.add(\"buffer.after-edit\", function() \ + local ok, got = pcall(pmacs.window.buffer) \ + if ok then SEEN = got else ERR = tostring(got) end \ + end)", + ); + + // A peer that never registered a view — the shape `dispatch_key` + // produces for an unattached frontend, and what the kill-ring suite + // drives with `ctrl_as`. + let viewless = FrontendId(9); + assert!( + !s.core.borrow().views.contains_key(&viewless), + "the premise: this frontend really has no view" + ); + s.dispatch_key(viewless, key(KeyCode::Char('z'), KeyModifiers::NONE)); + + let err: Option = eval(&s, "return ERR"); + assert_eq!( + err, None, + "pmacs.window.buffer() must not raise for a viewless acting frontend" + ); + let seen: Option = eval(&s, "return SEEN"); + assert_eq!( + seen.expect("the command observed a buffer").0, + ambient, + "…it answers with the ambient active buffer" + ); +} + // --------------------------------------------------------------------------- // 20 / 23 — quit: delete, restore chains, revalidation, and the cap // ---------------------------------------------------------------------------