diff --git a/COHERENCE.md b/COHERENCE.md index c447a76..0efa19f 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -107,7 +107,7 @@ remain open to them. | 11 | Config layering + provenance | **Partial (foundation only)** | Typed registry is right; 5 settings live in it; no value provenance | | 12 | Profiles | **Missing** | One hardcoded default keymap; not a named concept | | 13 | Package lifecycle UX | **Resolution without lifecycle** | Mature resolver/lockfile; init-only install; no uninstall/disable/search | -| 14 | Workbench primitives | **Partial (best trajectory)** | Listview is a real primitive but only 3 call sites, all LSP panels; buffer-list and search re-implement it; bottom panel complete on BOTH frontends (#155 + Stage 2) | +| 14 | Workbench primitives | **Partial (best trajectory)** | Listview is a real primitive but only 3 call sites, all LSP panels; buffer-list and search re-implement it; **the bottom panel is COMPLETE — both frontends, and Stage 3 flipped the adopter default so omission means the panel**. **Tree is still ✗ and is now the arc's successor** | | 15 | Contextual affordances | **Weak** | Right-click menu only; code actions apply first-blindly; no git integration at all | | 16 | Semantic frontend | **Strong** | v6..=v21 schema support; production attach remains v20 during the dark panel slice; degradation practiced | | 17 | Distribution | **Partial** | **v1.1.0 ships prebuilt Linux/macOS binaries on tag** (#211) with checksums and a stated glibc floor. No channels, in-place update, rollback, signing, or package-manager distribution | @@ -1667,10 +1667,9 @@ and §18's floor ride on this. ### Priority 5: Finish the workbench convergence -**State: partial and moving (§14) — the bottom panel is now complete on -both frontends (Stage 1 #155 through Stage 2B-3), and listview is -proven.** Only the adopter default flip (Stage 3) remains on the panel -itself. Remaining elsewhere: the tree primitive (build it before dired +**State: the bottom panel is DONE (§14) — both frontends, Stage 1 #155 +through Stage 2B-3, and Stage 3 flipped the adopter default so omitting +`display` means the panel. Arc 7 is complete.** Remaining elsewhere: the tree primitive (build it before dired and the worker tree invent two), table/inspector/diff, help unification. Wiring plus one modest model piece (the tree model). diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index e6e85d2..22fc643 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -803,12 +803,20 @@ local function start_run(slot, cmdline, opts) -- supersedes anything, rewrites the buffer, or spawns a process, so -- an unknown value leaves no half-started run behind. In Stages 1-2 -- omission means "current"; Stage 3 flips the default. - local display = opts.display - if display ~= nil and display ~= "current" and display ~= "panel" then - error(string.format( - "compile.run: unknown display %q (expected \"current\" or \"panel\")", - tostring(display))) - end + -- Q#S3-1: one shared rule for vocabulary, error text and default. + -- + -- `display_omitted` is captured SEPARATELY and deliberately. The + -- resolver collapses omission into its default, but the recompile gate + -- below distinguishes them: it fires on OMISSION only, never on an + -- explicit `display = "current"`, which is the documented opt-out and + -- must reach the raw switch even when the previous run was + -- panel-placed. Resolving first and testing `== "current"` afterwards + -- would silently merge the two and break that opt-out. + local display_omitted = opts.display == nil + -- Stage 3 (Q#BP12): omission resolves to the PANEL, with + -- `select = false` at the display call below — compile output is + -- passive and must not steal focus from the buffer being compiled. + local display = pmacs.window._resolve_display("compile.run", opts.display, "panel") -- q-target discipline (Q#CM11): capture only when coming from a -- non-generated buffer, so `g` reruns don't re-capture and -- compile → g → q restores the original buffer. @@ -885,18 +893,23 @@ local function start_run(slot, cmdline, opts) -- not preempt the requested panel. Compile output is passive, so it -- takes `select = false` explicitly. -- - -- A recompile reaches here with NO `display` (only cmdline/cwd are - -- stored in `_last`), so the raw switch below would put this buffer in - -- the selected DOCUMENT window while the panel still shows it — the - -- duplicate presentation this arc removes elsewhere. Detect that the - -- buffer already owns the panel slot and keep it there. + -- A recompile REPLAYS `_last`, which since Stage 3 carries `display` + -- alongside cmdline/cwd — an opt-out that did not survive replay + -- would silently revert to the panel on the next `g`. So an explicit + -- `display = "current"` reaches here again on a recompile, and must + -- still take the raw switch below. + -- + -- The `display_omitted` arm remains for the genuinely omitted case: + -- it keeps a buffer that already owns the panel slot in the panel + -- rather than duplicating it into the selected DOCUMENT window while + -- the panel still shows it. -- -- Gated on OMISSION, never on an explicit value: `display = "current"` -- is the documented user-facing opt-out from the Stage 3 default flip, -- so it must reach the raw switch even when the previous run was -- panel-placed. The duplicate presentation that produces is the -- escape hatch's documented cost (R3-rp2). - if display == "panel" or (display == nil and already_in_panel(slot.buf)) then + if display == "panel" or (display_omitted and already_in_panel(slot.buf)) then pmacs.window.display(slot.buf, { side = "bottom", select = false }) else pmacs.window.switch_buffer(slot.buf) @@ -1145,7 +1158,15 @@ function pmacs.compile.run(cmdline, opts) slot.parse = true local proc = start_run(slot, cmdline, opts) if proc then - pmacs.compile._last = { cmdline = cmdline, cwd = slot.cwd } + -- Stage 3: `display` is stored too. It is the documented opt-out + -- from the panel default, and `g` (recompile) reaches `start_run` + -- with whatever `_last` holds — so without this, a user who ran + -- `compile.run{display="current"}` would be moved into a panel the + -- moment they recompiled. An opt-out that reverts on the next `g` + -- is not an opt-out. `nil` is stored as `nil`, so an omitted + -- `display` keeps resolving to the default rather than being + -- frozen at the first run's resolution. + pmacs.compile._last = { cmdline = cmdline, cwd = slot.cwd, display = opts and opts.display } claim_compile_source(slot) end return proc @@ -1221,7 +1242,7 @@ pmacs.command.define { pmacs.editor.set_status("compile: nothing to recompile yet (run compile.run first)") return end - pmacs.compile.run(last.cmdline, { cwd = last.cwd }) + pmacs.compile.run(last.cmdline, { cwd = last.cwd, display = last.display }) end, } diff --git a/builtin/runtime/dired.lua b/builtin/runtime/dired.lua index 11d78ce..f747f60 100644 --- a/builtin/runtime/dired.lua +++ b/builtin/runtime/dired.lua @@ -642,11 +642,13 @@ local function open_directory(path, opts, departed) error(string.format("pmacs.dired.open: unknown opts key %q", tostring(key))) end end - local wanted = opts.display - if wanted ~= nil and wanted ~= "current" and wanted ~= "panel" then - error(string.format('pmacs.dired.open: unknown display %q (expected "current" or "panel")', - tostring(wanted))) - end + -- Q#S3-1/§1.1a: the shared rule, with dired's default passed + -- EXPLICITLY as "current" and kept there through Stage 3. The + -- `pmacs.path.directory_handler` slot calls this with no `display` at + -- all, so flipping dired's default would open `pmacs .` in a bottom + -- panel. Dired produces a document the user works in, not output they + -- consult; the panel default is right for the latter only. + local wanted = pmacs.window._resolve_display("pmacs.dired.open", opts.display, "current") local canonical = canonicalize(path) -- Read first: a failure must leave no buffer, no window change, and diff --git a/builtin/runtime/listview.lua b/builtin/runtime/listview.lua index 3c83119..62b0cb3 100644 --- a/builtin/runtime/listview.lua +++ b/builtin/runtime/listview.lua @@ -226,12 +226,15 @@ function pmacs.listview.open(spec) -- window. In Stages 1-2 omitting `display` keeps today's raw switch; -- Stage 3 flips the default. An unknown value errors before anything -- is displayed. - local display = spec.display - if display ~= nil and display ~= "current" and display ~= "panel" then - error(string.format( - "listview.open: unknown display %q (expected \"current\" or \"panel\")", - tostring(display))) - end + -- Q#S3-1: the vocabulary, the error and the default policy are one + -- rule (`window._resolve_display`), not a copy per adopter. The + -- default is passed in because the adopters do not share one. + -- Stage 3 (Q#BP12): omission resolves to the PANEL. `select = true` + -- below is a correctness requirement, not a preference — `seat_cursor` + -- and `listview.refresh` drive `pmacs.editor.move_down()`, which acts + -- on the ACTIVE window, so an unselected panel would seat the cursor + -- in the user's document. + local display = pmacs.window._resolve_display("listview.open", spec.display, "panel") if display == "panel" then pmacs.window.display(p.buffer, { side = "bottom", select = true }) else diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 7c636cb..6107b38 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -2493,7 +2493,21 @@ function pmacs.lsp.document_symbols() rows = rows, on_visit = function(sym) pmacs.editor.push_jump() - local okv = pcall(pmacs.window.switch_buffer, source_buf) + -- Bottom-panel arc (Q#BP11b), completed in Stage 3: a visit FROM + -- a panel must land in the DOCUMENT target and leave the panel + -- intact — the same rule `visit_location` above already follows + -- via `display_file`. + -- + -- This path used `pmacs.window.switch_buffer`, the RAW switch, + -- which replaces the buffer in the ACTIVE window. That was + -- harmless while the outline opened into a document window: the + -- switch simply reused it. Once Stage 3 made the panel the + -- default, the active window IS the outline panel, so RET + -- clobbered the panel with the source and left nothing for `M-,` + -- to return to. The references panel was migrated when the arc + -- landed; the outline was missed because nothing exercised it + -- from a panel until the default flipped. + local okv = pcall(pmacs.window.display, source_buf, { select = true }) if not okv then pmacs.editor.jump_back() pmacs.editor.set_status("LSP: outline source buffer is gone") diff --git a/docs/active-work.md b/docs/active-work.md index 6374a1b..f11e10e 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -311,19 +311,81 @@ them) and why `dired`/`listview` were the correct first two families. Whichever starts second integrates first. -## Bottom-panel lane (Arc 7) — STAGE 2 COMPLETE; STAGE 3 IS THE LAST STEP +## Bottom-panel lane (Arc 7) — STAGE 3 COMPLETE; ARC DONE, pending PR -**Stage 1, the Stage 2 framing, and Stages 2A, 2B-1, 2B-2 and 2B-3 are -all on `main` @ `4cd4a7b`** (#155, #175, #177, #184, #187, #198). Stage 2 -is complete. Durable facts are in `docs/agent-handoff.md` §1, including -the v20-baseline / v21-negotiated handshake that Stage 2B-3 made -compatible. +**Arc 7 is finished.** Stage 1 (#155), the Stage 2 framing (#175), +Stages 2A (#177), 2B-1 (#184), 2B-2 (#187) and 2B-3 (#198) are on +`main`; **Stage 3 — the adopter default flip — is implemented on +`bottom-panel-stage3`** and is the arc's last step. Framing +`docs/bottom-panel-stage3-framing.md` revision 3, approved with +amendments after three review rounds. -- **Stage 3 — the adopter default flip — is the arc's last step and is - not started.** This lane stays until it lands; it is not removed at - 2B-3's merge. -- **DAP waits for Stage 2, not Stage 1** — that dependency is now - satisfied. +**This lane is removed once Stage 3 merges and its facts are in +`docs/agent-handoff.md` §1** (rule 4). It is retained now only because +the PR has not landed. + +- **Branch `bottom-panel-stage3`** off `githubsucks/main` @ `21de0b2`, + pushed, upstream tracking set. Six commits: `fa12095` framing, + `0224c68` census, `41d37fc` shared resolver, `8d14e6a` lane/portability, + `c0eb16b` the flip + test revisions, `5f01ede` the fallback pin. +- **Verification:** full serialized sweep **3449 passed / 0 failed** + against a 3447 baseline (+2 new pins); fmt, diff-check, clippy with and + without `crdt`, `--lib` 1896, `--lib --features crdt` 2081, + pmacs-protocol 19, m4 149, required GPU 221, and + `bottom_panel_stage1_acceptance` 47/47 under both Lua flavors. + +### What Stage 3 changed + +Omitting `display` resolves to the **panel** for listview, compile and +terminal. **Dired keeps `"current"`** — `pmacs.path.directory_handler` +calls it with no `display`, so a flipped default would open `pmacs .` in +a bottom panel. Per-adopter `select`: listview `true`, compile `false`, +terminal `true`. + +Four hand-written copies of the `display` validator collapsed into one +shared `resolve_adopter_display(operation, raw, default)`; the default is +a **parameter**, which is what makes dired's exemption visible at its +call site rather than hidden in a divergent copy. + +### Three defects the flip exposed, each fixed rather than tested around + +- **The outline panel's `on_visit` used the RAW switch** + (`pmacs.window.switch_buffer`), which replaces the buffer in the ACTIVE + window. Harmless while the outline opened into a document window; + once the panel became the default, RET **clobbered the panel with the + source**. The references panel was migrated to `display_file` when the + arc landed — the outline was missed because nothing exercised it from a + panel until the flip. Q#BP11c names this exact corruption. +- **`compile._last` stored only `{cmdline, cwd}`**, so `g` reached + `start_run` with no `display` and took the new default: an explicit + `display = "current"` silently reverted on the next recompile. **An + opt-out that reverts is not an opt-out.** +- **`opts.display` on a nil `opts`** — a regression introduced by the + fix above and caught by `journey_acceptance`, which is what that + ratchet is for. + +### Two contracts now pinned, not merely observed + +- **Compile's chords are PANEL-LOCAL.** All are bound + `scope = "buffer"`, so with `select = false` none dispatch from the + document — `C-c C-k` included. `M-x compile.kill` still reaches the + running slot anywhere via its `or compile_slot()` fallback. A global + chord is a command-surface decision and belongs in its own framing. +- **The two `q` mechanisms are complementary, not competing.** + Presentation history chains in the side slot (`C → B → A → delete`, + Q#BP2c); `p.prev` prevents raw-switch and capability-fallback listview + loops. `s1_12` pins the second with explicit `display = "current"`; + the new `s3_1` pins the first. + +### The census lesson worth carrying past this arc + +**It counted failures, not causes.** Thirteen listview failures had ONE +root cause — a panel is derived-hidden while frame geometry is unknown, +and that suite never declared any because it never needed to. The same +applied to `m4_acceptance` and `vterm_stage2_acceptance`. And +**`--no-fail-fast` is mandatory**: the first sweep reported 2 failures in +1 suite because `cargo test` halts after a failing binary; the real +figure was 37 across 5. ## Reap-ledger silent failures — MERGED (#202); kept for its parked follow-ons diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 77c7642..6b633aa 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -77,6 +77,41 @@ commands, read `docs/active-work.md` immediately after this file. #204 and **1b-3** #205, the ambient-root isolation **implementation** #206, and discovery Stage 1 #207. Each has its own bullet below; this line is the head-of-`main` anchor and nothing else. +- **Bottom panel Arc 7 COMPLETE — Stage 3, the adopter default flip.** + Omitting `display` now means the panel for listview, compile and + terminal; **dired keeps `"current"`** because + `pmacs.path.directory_handler` calls it with no `display` and a flipped + default would open `pmacs .` in a bottom panel. Four hand-written + copies of the validator collapsed into one + `resolve_adopter_display(operation, raw, default)` — the default is a + **parameter**, which is what makes dired's exemption visible at its + call site instead of hidden in a divergent copy. Durable facts: + - **A visit FROM a panel must never use the raw switch.** + `pmacs.window.switch_buffer` replaces the buffer in the ACTIVE + window, so from a panel it clobbers the panel itself. The outline's + `on_visit` still did this; the references panel had been migrated to + `display_file` when the arc landed and the outline was missed, + because nothing exercised it from a panel until the default flipped. + **Q#BP11c is the contract**: after RET, `M-,` must FOCUS the + still-present panel, not clone its buffer into the document — and an + assertion on the active buffer name alone cannot tell those apart. + - **An opt-out that does not survive replay is not an opt-out.** + `compile._last` stored `{cmdline, cwd}` only, so `g` re-resolved + `display` and silently reverted an explicit `"current"` to the new + default. Anything that replays a stored invocation must store the + escape hatch with it. + - **Compile's chords are PANEL-LOCAL, deliberately.** All are bound + `scope = "buffer"`, so with `select = false` none dispatch from the + document — `C-c C-k` included. `M-x compile.kill` still works + anywhere via its `or compile_slot()` fallback. A global chord is a + command-surface decision, framed separately. + - **Two `q` mechanisms coexist by design**: presentation history + chains in the side slot (`C → B → A → delete`, Q#BP2c), while + `p.prev` prevents raw-switch and capability-fallback listview loops. + Neither supersedes the other. + - **A capability fallback must strip the QUIT ACTION too**, not just + the side parameters — a quit action stranded on a document window + makes a later `q` try to restore a presentation that never happened. - **pmacs is installable without cloning — Distribution Stage 1, #211, released as v1.1.0.** A `v*` tag builds `pmacs` and `pmacs-gpu` on pinned `ubuntu-22.04` / `macos-15` and publishes a GitHub Release with @@ -181,7 +216,7 @@ anchor, so every item is startable. | 2 | Workspace + location | Missing; model gap | The long-lead arc. Start before a fifth subsystem grows its own root convention — four have already diverged (§7) | | 3 | Extension ownership | Missing; prerequisite-shaped | **`pmacs.hook.remove` does not exist.** That one bug-sized gap blocks §13's disable/uninstall, §10's trust classes, and package-scoped cancellation | | 4 | **Discovery** | **Stage 1 MERGED (#207)** | Stage 2 candidates, in rough dependency order: richer M-x rows (**protocol change** — `MinibufferPrompt.candidates` is `Vec`; `CompletionPopupRow` already proves the pattern), `Command` gaining title/category/aliases/flags/arg-schema (~147 definition sites), predicate evaluation, help-layer unification, and the help-prefix decision | -| 5 | Workbench convergence | Partial, best trajectory | Bottom panel done both frontends; **Stage 3 = flip the adopter default**. Then the tree primitive — build it *before* dired and the worker tree invent two | +| 5 | Workbench convergence | Partial; **Arc 7 COMPLETE** (Stage 3 implemented) | The bottom panel is finished on both frontends and the adopter default is flipped. **The tree primitive is now the arc's successor** — `COHERENCE.md` §14 grades Tree ✗, and DAP's variables view is its next would-be inventor. Build it *before* dired's `i` and the worker tree invent two | | 6 | Config productization | Foundation only | Value provenance, then layering, then adoption migration (**table-valued settings are the hard prerequisite** — `ConfigValue` is four scalars) | | 7 | Package lifecycle | Not started | Correctly sequenced after P3 | | 8 | **Distribution** | **Stage 1 SHIPPED (v1.1.0, #211)** | Binaries on tag, checksums, machine-checked glibc floor. **Journey step 1 now works and the "invisible until this exists" blocker is lifted.** Next is a *decision* about channels / update / signing, not a queued plan | diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md new file mode 100644 index 0000000..18ba3b3 --- /dev/null +++ b/docs/bottom-panel-stage3-framing.md @@ -0,0 +1,591 @@ +# Framing — Bottom panel Stage 3: the adopter default flip + +**Revision 4.** Status: **IMPLEMENTED — all six branch-plan steps done; +Arc 7 complete.** Branch `bottom-panel-stage3`, based on +`githubsucks/main` @ `21de0b2`. Full sweep 3449 passed / 0 failed +against a 3447 baseline. + +**Revision 3 → 4** records what implementation found. The plan held; the +surprises were all defects the flip exposed rather than design changes: + +- **Three defects, each fixed rather than tested around** — the outline's + raw-switch visit (§1.7a), compile's non-replayed opt-out (§1.7b), and + a nil-`opts` regression of my own that `journey_acceptance` caught. +- **The census counted failures, not causes** (§1.6d): 13 listview + failures had ONE root cause, a missing frame-geometry declaration. +- **Compile's chords are panel-local** (§1.5a), pinned by `acc34`. +- **The two `q` mechanisms are complementary** (§1.7c), pinned by + `s1_12` and the new `s3_1`. + +**Revision 2 → 3** adds the measurement and its classification. Two +findings changed the plan rather than confirming it: the census needs +`--no-fail-fast` or it under-reports by an order of magnitude, and +**`m4_acceptance` is a transitive adopter via listview that no table +named**. + +**Revision 1 → 2**, all from review and all verified rather than +accepted: + +- **Dired is a FOURTH copy of the validator, not merely a fourth + adopter** — and it must keep `"current"` as its default, because the + `pmacs .` path reaches it with no `display` at all (§1.1a). Revision 1 + had this as an open question with a leaning; it is now a decision with + a mechanism. +- **Unify narrowly, not wholesale** (§2 Q#S3-1). A shared + `resolve_adopter_display(operation, raw, default)` covering vocabulary, + error text and default policy — with terminal's `window` + mutual-exclusion staying in its Rust wrapper, because the parsers are + *not* identical and pretending otherwise is its own defect. +- **A negative criterion is added** (acceptance 9): omitted `display` + must still mean document placement for direct dired **and** for + `pmacs .`. +- **The suite fallout is named, not just counted** (§1.6a): two Stage 1 + tests assert the old default deliberately and must be revised + knowingly. + +Stage 3 is **Arc 7's last step**. Stages 1, 2A, 2B-1, 2B-2 and 2B-3 are +all on `main` (#155, #175, #177, #184, #187, #198); the panel is complete +on both frontends and every mechanism this stage needs exists. What +remains is the decision the arc deferred on purpose: **omitting `display` +should mean the panel, not the selected window.** + +The parent framing already decided the policy (Q#BP12) and wrote the +acceptance sketch (criteria 56–58). This document re-scouts that plan +against current `main` and records where it has drifted. + +--- + +## 0. Coherence impact (COHERENCE §20) + +- **Concern: §14 Coherent Workbench Primitives**, graded *"partial, with + the best trajectory of any concern."* This closes the panel half of + P5. §14's bottom-panel bullet names Stage 3 explicitly as what is + left. +- **§6 Interaction islands — this REMOVES one.** Today every adopter + that wants a panel must say so at each call site, and three separate + code paths decide what silence means. After Stage 3 the policy is the + default and the call sites stop carrying it. +- **Journey steps touched:** none directly, though steps 6 and 9 (LSP + panels, compile output) change where their output lands. +- **Config registry adoption:** none — Q#BP12 is explicit that this is + **not a hidden global setting**. It is a resolved default, not a + preference. +- **Background-work attribution:** none. +- **Enables:** DAP. Q#BP12's adopter table **already contains a + `DAP stack/variables` row**, so this stage settles the debugger's panel + policy before the debugger exists. That is the intended order. + +--- + +## 1. Ground truth (measured at `000b6cd`) + +### 1.1 The flip is three sites in two languages + +The parent framing says *"Stage 3 is not one line per consumer."* True — +but the sharper reason is that **the option was never parsed in one +place.** Each adopter validates and dispatches `display` itself: + +| site | adopter | omission today | +|---|---|---| +| `src/lua_bindings/window_panel.rs:269` | `pmacs.terminal.open` | `None \| Some("current") => AdopterPlacement::Current` | +| `builtin/runtime/listview.lua:235` | listview | `if display == "panel" … else switch_buffer` | +| `builtin/runtime/compile.lua:899` | compile | same, plus an `already_in_panel` special case | + +A **fourth** copy validates the same vocabulary without being a +default-flip site: `builtin/runtime/dired.lua:645`. It is the one that +must NOT flip (§1.1a). + +`parse_adopter_placement` looks like the shared parser its doc comment +implies, but **it has exactly one caller** — the terminal. listview and +compile each re-implement the same three-value validation in Lua, +including their own copy of the error message. + +**The three default-resolution branches are the flip; the fourth +validator is not.** `window_panel.rs:269`, `listview.lua:235` and +`compile.lua:899` all move. Changing only the Rust parser would leave +**both** Lua adopters resolving omission to the current window — a +half-flip that would look done and behave inconsistently per adopter. + +Four copies of one rule is also how the next adopter gets it subtly +wrong — and the next adopter is **DAP**, already named in Q#BP12's +table. Hence Q#S3-1's narrow unification. + +### 1.1a Dired must NOT flip, and the reason is the golden journey + +`dired.lua:645` validates the same `"current" | "panel"` vocabulary with +the same error shape, so it is a fourth copy of the rule. **Its default +must stay `"current"`**, and the mechanism is specific rather than +stylistic: + +```lua +pmacs.path.set_directory_handler(function(path, dest) + open_async(path, { dest = dest }, nil, "dired") +end) +``` + +The `pmacs .` path reaches dired through that slot with **`{ dest = +dest }` and no `display` key at all** — so it resolves by omission. If +dired's default flipped with the others, **`pmacs .` would open the +directory listing in a bottom panel**, which is wrong for journey step +2 and for every subsequent step that navigates from it. Journey Stage +1a made `pmacs .` open a directory at all; putting the result in a +panel would undo the point of it. + +This is the difference between an adopter and a surface. listview, +compile and terminal produce *output the user consults*; dired produces +*a document the user works in*, like a buffer. The panel default is +right for the first kind and wrong for the second. + +### 1.2 `select` is not cosmetic for listview, and the citation drifted + +Q#BP12 requires `select = true` for interactive listview, citing +`listview.lua:64` for `seat_cursor`. **That line is now +`NAME_VARIANT_LIMIT`; `seat_cursor` is at line 130.** The constraint +itself is intact and verified: + +```lua +local function seat_cursor(p, line) + ... + for _ = 1, target do + pmacs.editor.move_down() + end +``` + +`pmacs.editor.move_down()` acts on the **active window**. A listview +panel displayed without `select` would seat the cursor in whatever +window is selected — the user's document. `listview.refresh` has the +same property. **This is a data-corruption-shaped bug, not a focus +annoyance**, and it is why the table's `select` column differs per +adopter rather than being uniform. + +Compile takes `select = false` deliberately (passive output); terminal +takes `select = true`. + +### 1.3 Compile has already been prepared for this stage + +`compile.lua:894` carries a comment written for Stage 3: + +> Gated on OMISSION, never on an explicit value: `display = "current"` +> is the documented user-facing opt-out from the Stage 3 default flip, +> so it must reach the raw switch even when the previous run was +> panel-placed. + +and the condition is already +`display == "panel" or (display == nil and already_in_panel(slot.buf))`. +So compile's *recompile* path already keeps a panel-placed buffer in the +panel on omission. **Stage 3 makes the first run behave like the +recompile.** Re-read this comment before editing: it encodes a +distinction (omission vs explicit `"current"`) that the flip must +preserve, and the `already_in_panel` branch may become redundant. + +### 1.4 What Stage 3 owes beyond the flip + +Q#BP12's table is a per-adopter contract, not a single switch. For each +of listview / compile / terminal: + +- **panel placement** with that adopter's `select` value; +- **`dedicated = false`** — a dedicated panel refuses to host anything + else, and dired Stage 1 already established that `display_buffer` will + not replace a buffer in a slot dedicated to another (Q#BP3 2.iii); +- **quit action**: delete the panel if this adopter created it, restore + the panel it replaced otherwise; +- **visit path onto `display_file` / `display_target`** with `select` + per the table. + +`display_file` already exists and is used by dired (`dired.lua:831`, +`:840`) and `default.lua:729`, so the visit half has a proven caller +shape to copy. + +### 1.5 Capability fallback still applies, and must be re-proved + +The Stage 3 default resolves as a *panel request*, so it passes through +Q#BP13 capability fallback exactly as an explicit `"panel"` does. On a +pre-panel semantic frontend the request degrades, and criterion 57 +requires that the degraded path leave **no side parameters and no quit +action on the document window**. + +This is the criterion most likely to be quietly wrong, because the +fallback is invisible from the adopter's side. + +### 1.6 What is NOT established + +- **The blast radius is MEASURED — §1.6b: 37 failures across 5 suites**, + classified per test in §1.6c. Stage 1 shipped the mechanism opt-in + precisely so existing suites kept their meaning; flipping the default + changes where output lands for every suite that exercises listview, + compile or terminal **without** passing `display`. +- **Steps 1 and 2 of §7 are done** (`0224c68`, `a2f4411`). The flip + itself, the test revisions, and the capability-fallback criterion are + not. +- ~~Interaction with dired.~~ **DECIDED — §1.1a and Q#S3-2: dired keeps + `"current"`**, passed explicitly to the shared resolver so the + exemption is visible at its call site. + +--- + +### 1.6a Two Stage 1 tests assert the OLD default deliberately + +These are not collateral damage; they encode intent and must be revised +knowingly. + +- **`tests/bottom_panel_stage1_acceptance.rs:1223`** — + `acc19_adopters_place_side_affinely_through_real_entry_points` opens a + listview with **no `display`** specifically to seed the panel buffer + into a DOCUMENT window first, *"so side-affine placement cannot be + vacuous."* After the flip that setup no longer produces a document + window, and the test's own anti-vacuity guarantee is what breaks. It + needs a new way to seed, not a `display = "current"` bolted on. +- **`tests/bottom_panel_stage1_acceptance.rs:1308`** — + `acc19b_recompile_reuses_the_panel_instead_of_duplicating_into_the_document` + is built entirely around a recompile reaching `start_run` with no + `display`. Its subject survives the flip, but its mechanism (`§1.3`'s + `already_in_panel` gate) may not — see Q#S3-3. + +**The rule for the fallout sweep:** a test whose *subject* is placement +must assert the **new** default. A test whose subject is compile or +terminal behaviour opts out with `display = "current"` **only when its +setup genuinely requires the document window**. Mass-adding the opt-out +to make a suite green converts a behavioural change into an invisible +one, which is the failure this stage's inverted ordering exists to +avoid. + +### 1.5a Compile's chords become PANEL-LOCAL — a deliberate contract + +**Discovered while revising the fallout, and decided rather than +absorbed.** Every compile-mode chord is bound +`scope = "buffer", buffer = slot.buf` (`compile.lua:221`): `RET`, `n`, +`p`, `q`, **`C-c C-k`**, `g`, and the seven undo no-ops. They dispatch +only when `*compilation*` is the focused buffer. + +Before Stage 3 compile switched in place, so the user was *in* that +buffer and the chords worked. **Stage 3 keeps `select = false`** — Q#BP12 +is explicit, and passive build output stealing focus mid-edit would be +worse than the alternative — so the user stays in their document and +none of those chords reach compile-mode without focusing the panel +first. + +**The contract, stated so it is deliberate rather than an accidental +reachability loss:** + +1. A default `compile.run` opens **passively**; document focus remains. +2. **Buffer-local compile chords require focusing the panel** (`C-x o`, + or a click). +3. **The capability is not lost.** `compile.kill` is reachable through + `M-x` from anywhere, because its body is + `slot_for_buffer(pmacs.window.buffer()) or compile_slot()` — the + `or` arm falls back to the current compilation slot precisely when + the caller is not in a compile buffer (`compile.lua:1124`). Verified, + not assumed; it is what makes panel-local chords acceptable rather + than a lost feature. + +**A global `C-c C-k` is deliberately NOT part of this stage.** It is a +command-surface decision — which chords earn global scope — and it would +ride in on a placement flip without its own reasoning. Framed +separately or not at all. + +Pinned by a default-placement test asserting `C-c C-k` from the document +window does **not** dispatch to compile (acceptance 10), so a future +change that quietly makes it global has to change a test that says why +it was not. + +### 1.6b The fallout census — MEASURED, and it found an adopter nobody named + +Taken as branch-plan step 1: record a baseline, apply the three-site +flip as a throwaway edit, sweep, revert. The flip is **not** in the +tree; only this table survives it. + +**A census that stops at the first failing binary is not a census.** +The first sweep reported **2 failures in 1 suite** and looked +comfortingly small — `cargo test` halts after a failing test binary, so +everything alphabetically past `bottom_panel_stage1_acceptance` never +ran. With `--no-fail-fast` the real figure is **37 failures across 5 +suites**. Any future re-measurement must pass that flag or it will +under-report by an order of magnitude, in the same shape as this arc's +other silent-success traps. + +| suite | base → flipped | failures | share of suite | +|---|---|---:|---:| +| `listview_acceptance` | 17 → 4 | **13** | **76%** | +| `compile_mode_acceptance` | 72 → 55 | **17** | 24% | +| `vterm_stage2_acceptance` | 6 → 3 | **3** | 50% | +| `bottom_panel_stage1_acceptance` | 46 → 44 | **2** | 4% | +| `m4_acceptance` | 149 → 147 | **2** | 1% | +| | | **37** | | + +**`m4_acceptance` was predicted by nobody** — not the parent framing, +not Q#BP12's adopter table, not revision 1 of this document. Its two +failures are `hover_doc_panel_shows_full_contents_via_binding` and +`outline_panel_opens_visits_and_restores`: **the LSP panels are +listview consumers**, so flipping listview's default reaches the LSP +suite transitively. + +This materially improves the adopter map. Q#BP12 lists four rows +(listview, compile, terminal, DAP) as though they were the population. +They are the *direct* population; **the real one includes everything +built on listview**, and the LSP hover/outline panels are the proof. +Anything added on listview later inherits the panel default without +appearing in any table — which is the intended behaviour, but only if +the map says so. + +**The proportions invert the obvious reading.** `compile_mode` has the +most failures and the least placement content: `acc01_spawn_streams…`, +`acc05_kill_reaps_backgrounded_descendant`, +`acc21_kill_produces_signaled_marker` and the `r1f*`/`r5f*`/`r6f*` group +are process-lifecycle and styling tests that merely *use* compile and +now find its output elsewhere. `listview_acceptance` has fewer failures +but loses **three quarters of its suite**, and its failures +(`open_seats_cursor_and_ret_visits_the_row`, `panel_rejects_typing`, +`dispatch_idle_is_false_while_a_panel_is_focused`) are placement in +substance. + +### 1.6c Classification, decided + +Applying §1.6a's rule to the measured set. **Placement-subject tests +assert the NEW default; only genuine document-window setups opt out.** + +| test | classification | +|---|---| +| `bottom_panel_stage1::acc19` | **placement** — needs a new way to seed a document window (§1.6a) | +| `bottom_panel_stage1::acc19b` | **placement** — subject survives; mechanism may not (Q#S3-3) | +| `m4::outline_panel_opens_visits_and_restores` | **placement** — *almost a direct realization of criterion 58*: open → visit → jump-back → quit. Assert the panel/document split | +| `m4::hover_doc_panel_shows_full_contents_via_binding` | **placement** — a listview panel lifecycle test. Keep the omitted default; assert the new placement **plus** its existing content and quit guarantees | +| `compile::acc15_ret_visits_error_and_jump_back_returns` | **placement** — Q#BP12 explicitly requires compilation panel → RET source → `M-,` back to the still-present panel with the document window intact | +| `compile::acc16_n_p_walk_error_lines_without_wrap` | **opt out**, explicit `display = "current"` — its subject is cursor navigation *within* compile output, which genuinely needs the compilation buffer selected | +| `listview_acceptance` ×13 | **placement**, predominantly — assert the new default | +| remaining `compile_mode` ×15 | **incidental** — process lifecycle and styling; opt out only where the setup needs the document window | +| `vterm_stage2` ×3 | **to classify at implementation** — terminal's `select = true` makes these likelier placement than incidental | + +The `acc15` / `acc16` split is the one worth remembering: **two +neighbouring tests in the same suite land on opposite sides**, because +one is about where output goes and the other is about moving within it. +A sweep that classified per *suite* rather than per *test* would have +got both wrong. + + +### 1.6d The census counted failures, not causes + +The measured 37 was accurate as a count and misleading as a work +estimate. **Thirteen listview failures had one root cause:** a panel is +derived-hidden while frame geometry is unknown, and +`listview_acceptance` never declared any — it never needed to while +listview defaulted to the current window. One helper took it from 13 to +2. `m4_acceptance` and `vterm_stage2_acceptance` were the same. + +Read a census as "how many assertions move", never "how many decisions +are required". The two differed here by an order of magnitude. + +### 1.7a Defect: the outline visited through the RAW switch + +`lsp.lua`'s outline `on_visit` called `pmacs.window.switch_buffer`, +which replaces the buffer in the **active** window. Harmless while the +outline opened into a document window — the switch simply reused it. +Once the panel became the default, the active window WAS the outline +panel, so **RET clobbered the panel with the source file** and left +nothing for `M-,` to return to. + +The references panel (`visit_location`) was migrated to `display_file` +when the arc landed; the outline was missed because **nothing exercised +it from a panel until the default flipped**. Its own neighbouring +comment states the rule it violated: *"a visit FROM a panel must land in +the document target and leave the panel intact."* + +Q#BP11c names the corruption precisely, and it is why both the outline +and compile tests now assert `M-,` **focuses** the still-present panel +rather than cloning its buffer into a document window — the previous +assertion, on the active buffer name alone, could not tell those apart. + +### 1.7b Defect: an opt-out that did not survive replay + +`pmacs.compile._last` stored `{cmdline, cwd}` and no `display`, so `g` +reached `start_run` with the value omitted and took the new default. A +user who ran `compile.run{display="current"}` was moved into a panel the +moment they recompiled. + +**An opt-out that reverts on the next replay is not an opt-out.** +`display` is now stored and replayed, with `nil` kept as `nil` so an +omitted value still resolves to the current default rather than freezing +at the first run's resolution. The general form: *anything that replays +a stored invocation must store the escape hatch alongside it.* + +### 1.7c The two `q` mechanisms are complementary + +Stage 3 made a latent conflict live. `listview.lua`'s `p.prev` captures +the previous buffer only if it is not a panel; `QuitAction::Restore` +(Q#BP2c) deliberately chains `C → B → A → delete`. With two listviews +sharing one bottom slot, the second replaces the first and `q` walks the +restore chain. + +**The restore chain wins**, per criterion 20 — listview `q` routes +through `window.quit` and must not exempt itself from the panel +contract. The mechanisms are then complementary rather than competing: +**presentation history chains in the side slot; `p.prev` prevents +raw-switch and capability-fallback loops.** `s1_12` keeps its Q#GB18 +name-keyed-identity bite by pinning its panels in document windows, +isolating `p.prev`; the new `s3_1` pins the chain. + +## 2. Questions + +- **Q#S3-1 — DECIDED: unify NARROWLY.** A shared + `resolve_adopter_display(operation, raw, default)` owns exactly three + things: the **vocabulary**, the **error text**, and the **default + policy**. Its four callers are listview, compile, terminal and dired — + the last passing `default = "current"`, which is what makes dired's + exemption a parameter rather than a divergent copy. + + **Terminal's `window` mutual-exclusion stays in its Rust wrapper.** + The parsers are *not* identical, and a helper that pretended otherwise + would be its own defect: only terminal accepts a `window` id, and only + it must reject `window` combined with `display = "panel"`. + + **One normalization must be named rather than absorbed.** Terminal + reads `spec_table.get::>("display")?`, so a non-string + value raises **mlua's type error before** the custom "unknown display" + message is ever reached. The Lua callers instead `tostring()` whatever + they got and report it inside their own error. These are different + observable behaviours for the same bad input, and unifying the error + text without deciding this would silently change one of them. + + **RESOLVED and PINNED at step 2.** The custom error wins, because it + names the legal vocabulary where mlua's type error does not. + Non-strings render by **type alone** (`unknown display (integer)`), + never quoted, so the message cannot imply a string was passed. Pinned + in `bottom_panel_stage1_acceptance::acc19` at the **terminal** entry + point — the one adopter whose behaviour actually changed — asserting + the shared error *and* that nothing is created, exactly as the + unknown-string case already does. + + **The type SPELLING is deliberately not pinned.** Lua 5.4 reports + `integer`; LuaJIT has no integer subtype. Asserting either literal + would pass on one CI flavor and fail on the other, so the test pins + the shape (`unknown display (`, the vocabulary, and the *absence* of a + quoted value). Verified 46/46 under both flavors. + + Consequently step 2 is **default-preserving with one intentional + normalization**, not "behaviour-preserving": every adopter kept its + default, but invalid-input behaviour moved on purpose. +- **Q#S3-2 — DECIDED: dired does not flip.** It keeps `"current"` as + its default, expressed as the `default` argument to the shared + resolver so the exemption is visible at the call site rather than + implied by a fourth copy of the parser. §1.1a records the mechanism — + the `pmacs .` handler passes no `display` — and acceptance 9 pins it + from both entry points. +- **Q#S3-3 — does `already_in_panel` survive?** Once omission means + panel, compile's special case may be dead code. **Leaning: measure, + then delete if dead** — but check the explicit-`"current"` path first, + because that arm is what the comment says the gate protects. +- **Q#S3-4 — how many existing suites move?** Unknown (§1.6). This must + be measured **before** the flip, so the diff to acceptance files can be + read as intended-vs-collateral rather than discovered afterwards. +- **Q#S3-5 — is there a user-facing escape beyond per-call `display`?** + Q#BP12 says this is deliberately not a setting. Someone who dislikes + panels has no global opt-out, only per-call `display = "current"`, + which they do not control for builtin commands. **Leaning: accept for + this stage and record it**, since a setting is §11 work and panel + persistence is already blocked on settings persistence. + +--- + +## 3. Bets + +- **Bet 1 — the flip itself is small; the suite churn is the work.** + Three dispatch sites, each a few lines. The cost is criterion 58's + per-adopter open→visit→return→quit suites plus whatever §1.6's + measurement turns up. +- **Bet 2 — the capability-fallback criterion (57) is where a defect + hides.** It is invisible from the adopter side and only observable on a + pre-panel semantic frontend. +- **Bet 3 — `select` gets one adopter wrong.** The values differ per + adopter for real reasons (§1.2), and a uniform `select = true` would + look correct and break compile's passive-output behaviour. + +--- + +## 4. Acceptance + +Inherits the parent framing's criteria 56–58, made concrete: + +1. Omitting `display` from listview, compile and terminal entry points + resolves to the Q#BP12 panel/select policy on a panel-capable grid + **and** semantic frontend. +2. Explicit `display = "current"` preserves each adopter's pre-arc + selected-window behaviour, including compile's raw-switch path. +3. **Per-adopter `select` matches the table** — listview `true`, + compile `false`, terminal `true` — asserted individually, not by a + shared helper that would pass with a uniform value. +4. An interactive listview panel seats its cursor **in the panel**, not + in the previously selected window (§1.2's real failure mode). +5. On a pre-panel semantic frontend the omitted default takes capability + fallback with **no side parameters and no quit action** left on the + document window; visit and `q` remain the existing non-side paths. +6. Per-adopter open→visit→return→quit suites, not a generic helper + (criterion 58), preserving Stage 1's unknown-value rollback + assertions. +7. **The unknown-`display` error still fires before anything is + created** — buffer, session, process or wrapper — for all three + adopters, whichever parser survives Q#S3-1. +8. The count of existing suites whose behaviour changes is **stated**, + and each change is classified intended or collateral — **not + silenced by mass-adding `display = "current"`** (§1.6a). +9. **NEGATIVE criterion — compile's chords are panel-local.** With the + default placement, `C-c C-k` pressed in the **document** window does + not dispatch to compile (§1.5a). Tests whose subject is + compile-BUFFER behaviour opt out with `display = "current"` and say + why. `M-x compile.kill` still works from anywhere. +10. **NEGATIVE criterion — omission still means the document for dired.** + Both entry points are pinned: a direct `pmacs.dired.open(path)` with + no `display`, **and** the `pmacs .` launch path through + `pmacs.path.directory_handler`. Neither may place into a panel. This + is the criterion that would catch a well-intentioned "make all four + consistent" change, and it guards journey step 2. + +--- + +## 5. Parked + +- Everything in the parent framing's §6 "Deferred (named)" — left/right/ + top side windows, multiple slots, `no_other_window`, manual + hide/show, `display-buffer-alist`-style user rules, panel persistence + (blocked on settings persistence), GPU document splits. +- **A global panel preference** (Q#S3-5) — §11 work. +- **The tree primitive.** Not part of this stage, and the next thing to + scope: §14 grades Tree ✗, and DAP's variables view is its next + would-be inventor. + +--- + +## 6. Gates + +The standing `CLAUDE.md` suite, with the touched acceptance suites being +at minimum `bottom_panel_stage1_acceptance`, `bottom_panel_stage2a`, +both `stage2b` daemon/GPU suites, plus the listview, compile-mode and +terminal suites — the last three are where §1.6's unmeasured churn will +land. + +--- + +## 7. Branch plan + +One branch, `bottom-panel-stage3`: + +1. **Measure first** (Q#S3-4): run the full suite with the flip applied + as a throwaway edit, record which suites move, revert. *The + measurement is the first commit's evidence, not the flip.* Classify + each mover per §1.6a's rule before writing a line of the fix. +2. **Land `resolve_adopter_display`** (Q#S3-1) with all four callers + still passing their CURRENT defaults, so nothing flips yet. It is + **default-preserving with one intentional normalization**, not + "behaviour-preserving" — every adopter keeps its default and the + suite is byte-identical to baseline, but terminal's invalid-input + behaviour moves deliberately (§1.6b). Decide and pin that + normalization here, since no existing assertion can catch it. +3. **Flip the three sites** by changing only the `default` argument at + listview, compile and terminal — dired keeps `"current"` — with + per-adopter `select`. +4. **Per-adopter open→visit→return→quit suites** (criterion 6). +5. **Capability fallback** (criterion 5), the one needing a semantic + frontend. +6. **Update the lane and handoff**; Arc 7 closes. + +Step 1 before step 3 is the point: flipping first and reading the +fallout as it appears makes intended and collateral changes +indistinguishable. diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 2d4f359..77bb4c7 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -9320,12 +9320,19 @@ fn install_terminal( // Bottom-panel arc (Q#BP11b): parse placement BEFORE the // session, process, buffer, or wrapper exists, so an // unknown `display` value creates nothing to roll back. + // Stage 3 (Q#BP12): omitting `display` resolves to the + // PANEL. `select = true` is passed to + // `place_adopter_buffer` below — a terminal is + // interactive, so it takes focus, unlike compile's + // passive output. + let display_value = spec_table.get::("display")?; let placement = window_panel::parse_adopter_placement( &core, frontend_id, "pmacs.terminal.open", - spec_table.get::>("display")?.as_deref(), + Some(&display_value), spec_table.get::>("window")?, + window_panel::AdopterDefault::Panel, )?; let buffer_id = { let mut manager = manager.borrow_mut(); diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index 1c700a5..fe8b758 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -251,9 +251,98 @@ pub(crate) enum AdopterPlacement { Window(WindowId), } +/// The one rule for the adopter `display` vocabulary: which values are +/// legal, what the error says, and what omission means. +/// +/// **Bottom-panel Stage 3 (Q#S3-1).** Before this, four adopters +/// validated the same three-value vocabulary in three places — Rust for +/// the terminal, and hand-written Lua copies in `listview.lua`, +/// `compile.lua` and `dired.lua`, each with its own copy of the error +/// string. Four copies of one rule is how the next adopter gets it +/// subtly wrong, and the next adopter is DAP. +/// +/// `default` is a **parameter, not a constant**, because the adopters do +/// not share one. Since Stage 3, listview / compile / terminal resolve +/// omission to the panel; **dired resolves it to `"current"`** and must +/// keep doing so — +/// `pmacs.path.directory_handler` calls it with no `display` at all, so +/// a flipped default would open `pmacs .` in a bottom panel (§1.1a). +/// Passing the default in is what makes dired's exemption visible at its +/// call site instead of hidden in a divergent copy. +/// +/// **Non-string values are reported as unknown, not as type errors**, +/// and this is a deliberate normalization (Q#S3-1). Terminal previously +/// read `get::>` and so raised mlua's type error *before* +/// reaching any custom message, while the Lua copies stringified and +/// reported their own. Nothing pinned either behaviour. The custom error +/// wins because it names the legal vocabulary and mlua's does not. +/// +/// # Errors +/// Any non-nil value that is not `"current"` or `"panel"`. +pub(crate) fn resolve_adopter_display( + operation: &str, + raw: Option<&mlua::Value>, + default: AdopterDefault, +) -> mlua::Result { + let raw = match raw { + None | Some(mlua::Value::Nil) => return Ok(default.placement()), + Some(value) => value, + }; + match raw.as_str().as_deref() { + Some("current") => Ok(AdopterPlacement::Current), + Some("panel") => Ok(AdopterPlacement::Panel), + _ => Err(mlua::Error::runtime(format!( + "{operation}: unknown display {} (expected \"current\" or \"panel\")", + display_for_error(raw) + ))), + } +} + +/// What omission means for one adopter. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum AdopterDefault { + /// listview, compile, terminal — Stage 3's flip. + Panel, + /// dired, and anything else whose output is a document the user + /// works in rather than output they consult. + Current, +} + +impl AdopterDefault { + fn placement(self) -> AdopterPlacement { + match self { + Self::Panel => AdopterPlacement::Panel, + Self::Current => AdopterPlacement::Current, + } + } +} + +/// Render a rejected `display` value for the error message. +/// +/// Strings are quoted so `display = "sideways"` reads as `"sideways"`; +/// anything else is shown **by type alone** — `unknown display +/// (number)` — because quoting a non-string as `"42"` would imply the +/// caller passed a string when they passed a number. +/// +/// The value itself is deliberately not interpolated: it is already +/// wrong, `mlua::Value`'s `Display` is not guaranteed useful for tables +/// or userdata, and the type is what tells the caller what to fix. +fn display_for_error(value: &mlua::Value) -> String { + value.as_str().map_or_else( + || format!("({})", value.type_name()), + |s| format!("{:?}", &*s), + ) +} + /// Parse an adopter's placement **before** it creates a buffer, session, /// process, or wrapper — so an unknown value leaves nothing to roll back. /// +/// Terminal-specific wrapper around [`resolve_adopter_display`]: only +/// the terminal accepts a `window` id, and only it must reject `window` +/// combined with `display = "panel"`. That asymmetry stays here rather +/// than in the shared resolver, because a helper that pretended the four +/// parsers were identical would be its own defect. +/// /// # Errors /// An unknown `display` value, a `window` combined with /// `display = "panel"`, or a window id that is not live in the acting @@ -262,18 +351,11 @@ pub(crate) fn parse_adopter_placement( core: &SharedCore, fid: FrontendId, operation: &str, - display: Option<&str>, + display: Option<&mlua::Value>, window: Option, + default: AdopterDefault, ) -> mlua::Result { - let display = match display { - None | Some("current") => AdopterPlacement::Current, - Some("panel") => AdopterPlacement::Panel, - Some(other) => { - return Err(mlua::Error::runtime(format!( - "{operation}: unknown display {other:?} (expected \"current\" or \"panel\")" - ))); - } - }; + let display = resolve_adopter_display(operation, display, default)?; match (window, &display) { (Some(_), AdopterPlacement::Panel) => Err(mlua::Error::runtime(format!( "{operation}: `window` and `display = \"panel\"` are mutually exclusive" @@ -481,6 +563,43 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result )?; } + { + // Q#S3-1 — the shared adopter-display rule, reachable from Lua. + // + // Underscore-prefixed because it is an internal seam between the + // builtin runtime modules and this one, not user-facing API: + // `listview.lua`, `compile.lua` and `dired.lua` call it instead + // of each keeping a hand-written copy of the same three-value + // check and error string. + // + // Returns the resolved `"current"` / `"panel"` rather than an + // opaque handle, so the Lua callers keep their existing + // `if display == "panel"` dispatch and this change stays a + // validation unification rather than a control-flow rewrite. + win.set( + "_resolve_display", + lua.create_function( + |_, + (operation, raw, default): (String, mlua::Value, String)| + -> mlua::Result<&'static str> { + let default = match default.as_str() { + "panel" => AdopterDefault::Panel, + "current" => AdopterDefault::Current, + other => { + return Err(mlua::Error::runtime(format!( + "window._resolve_display: bad default {other:?}" + ))); + } + }; + match resolve_adopter_display(&operation, Some(&raw), default)? { + AdopterPlacement::Panel => Ok("panel"), + AdopterPlacement::Current | AdopterPlacement::Window(_) => Ok("current"), + } + }, + )?, + )?; + } + { let cc = core.clone(); win.set( diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index 400754a..36efbe2 100644 --- a/tests/bottom_panel_stage1_acceptance.rs +++ b/tests/bottom_panel_stage1_acceptance.rs @@ -1220,18 +1220,42 @@ fn acc18_display_file_targets_the_document_from_a_focused_panel() { // --------------------------------------------------------------------------- #[test] +#[allow( + clippy::too_many_lines, + reason = "one placement scenario per adopter; splitting it would hide that they share a contract" +)] fn acc19_adopters_place_side_affinely_through_real_entry_points() { - // listview: pre-seed the persistent panel buffer in a DOCUMENT window - // first, so side-affine placement cannot be vacuous. + // Stage 3: the DEFAULT is now the panel, so assert that first — + // this test's subject is placement through the real entry points. let s = editor(); exec( &s, - "pmacs.listview.open { name = \"*outline*\", rows = { { text = \"row\" } } }", + "pmacs.listview.open { name = \"*default*\", rows = { { text = \"row\" } } }", + ); + assert!( + side_window(&s).is_some(), + "omitting display places a listview in the panel" + ); + + // listview: pre-seed the persistent panel buffer in a DOCUMENT window + // first, so side-affine placement cannot be vacuous. + // + // The seed now says `display = "current"` EXPLICITLY. That is not a + // bolt-on to keep the test green: the seed's whole purpose is "this + // buffer starts in a document window", and after the flip that + // requires saying so. Leaving it omitted would seed a panel and the + // side-affine assertion below would pass without having moved + // anything — exactly the vacuity this fixture was built to prevent. + let s = editor(); + exec( + &s, + "pmacs.listview.open { name = \"*outline*\", rows = { { text = \"row\" } }, \ + display = \"current\" }", ); let seeded = active_window(&s); assert!( side_window(&s).is_none(), - "the default placement is unchanged" + "the explicit opt-out still places in the document window" ); exec( &s, @@ -1254,9 +1278,10 @@ fn acc19_adopters_place_side_affinely_through_real_entry_points() { "an unknown display value is a pointed error" ); - // compile: same shape, but passive (`select = false`). + // compile: same shape, but passive (`select = false`). Seeded with + // the explicit opt-out for the same reason as the listview above. let s = editor(); - exec(&s, "pmacs.compile.run(\"true\")"); + exec(&s, "pmacs.compile.run(\"true\", { display = \"current\" })"); assert!(side_window(&s).is_none()); let document = active_window(&s); exec(&s, "pmacs.compile.run(\"true\", { display = \"panel\" })"); @@ -1291,6 +1316,53 @@ fn acc19_adopters_place_side_affinely_through_real_entry_points() { "unknown display fails before session/process/buffer creation" ); assert_eq!(s.core.borrow().registry.borrow().ids().len(), before); + + // Bottom-panel Stage 3, Q#S3-1 — the NON-STRING normalization. + // + // Pinned because step 2 CHANGED this deliberately and nothing else + // covers it. Before the shared resolver, terminal read + // `get::>("display")?`, so a number raised mlua's + // TYPE error before any custom message existed; the Lua adopters + // stringified instead and reported their own. Unifying the error + // text without pinning this would have let the two drift back apart + // unnoticed, and the surrounding assertions could not have caught it + // — they all pass unknown STRINGS, which take the same path in both + // designs. + // + // The custom error wins because it names the legal vocabulary. The + // type is reported WITHOUT the value, so the message cannot imply a + // string was passed. + let before = s.core.borrow().registry.borrow().ids().len(); + let err = try_exec( + &s, + "pmacs.terminal.open { command = \"/bin/sh\", display = 42 }", + ) + .expect_err("a non-string display is rejected"); + // The TYPE SPELLING is deliberately not pinned: Lua 5.4 reports + // `integer` where LuaJIT has no integer subtype, so asserting either + // literal would pass on one CI flavor and fail on the other. What is + // pinned is the shape — our operation name, a parenthesised type + // rather than a quoted value, and the vocabulary. + assert!( + err.contains("pmacs.terminal.open: unknown display ("), + "a non-string display takes the shared unknown-display error naming the \ + operation and a type, not mlua's type error; got: {err}" + ); + assert!( + err.contains("expected \"current\" or \"panel\""), + "the error still names the legal values; got: {err}" + ); + assert!( + !err.contains("\"42\""), + "the rejected value is reported by TYPE, not quoted as though it were a \ + string; got: {err}" + ); + assert_eq!( + s.core.borrow().registry.borrow().ids().len(), + before, + "…and still creates nothing, exactly as an unknown string does" + ); + exec( &s, "TERM_BUF = pmacs.terminal.open { command = \"/bin/sh\", display = \"panel\" }", @@ -1357,8 +1429,11 @@ fn acc19b_recompile_reuses_the_panel_instead_of_duplicating_into_the_document() ); // A compilation that is NOT in a panel keeps the pre-arc raw switch. + // Reaching that state now takes an explicit opt-out, since the + // default would panel it — and "not in a panel" is the precondition + // this half exists to exercise. let s = editor(); - exec(&s, "pmacs.compile.run(\"true\")"); + exec(&s, "pmacs.compile.run(\"true\", { display = \"current\" })"); assert!(side_window(&s).is_none()); let target = active_window(&s); exec(&s, "pmacs.command.invoke(\"compile.recompile\")"); @@ -1585,6 +1660,70 @@ fn acc21_panel_visit_and_jump_back_returns_to_the_panel() { ); } +/// Bottom-panel Stage 3, criterion 5 — the OMITTED default degrades on a +/// frontend that cannot host a panel. +/// +/// `acc14` already proves capability fallback for an EXPLICIT +/// `request.side` at the core level. This is the Stage 3 case and it is +/// not the same one: the default is now resolved into a panel request +/// inside the adopter, so a pre-panel semantic frontend must degrade a +/// request the caller never wrote. The framing calls this the criterion +/// most likely to be quietly wrong, because the fallback is invisible +/// from the adopter's side — nothing in `listview.open` says "panel", +/// yet the request that reaches the core does. +/// +/// What must survive the degradation: no side window, no side +/// parameters, and NO QUIT ACTION left on the document window. A quit +/// action stranded on a document window would make a later `q` try to +/// restore a presentation that never existed. +#[test] +fn s3_2_the_omitted_default_degrades_on_a_pre_panel_frontend() { + let s = editor(); + let fid = FrontendId(31); + let document = attach_frontend(&s, fid, false); + s.core.borrow_mut().active_frontend = fid; + + // No `display` at all — the Stage 3 default resolves to a panel + // request, which this frontend cannot honour. + exec( + &s, + "pmacs.listview.open { name = \"*degraded*\", rows = { { text = \"row\" } } }", + ); + + assert!( + s.core.borrow().side_window_for(fid).is_none(), + "a pre-panel frontend gets no side window from the omitted default" + ); + { + let core = s.core.borrow(); + let window = &core.windows[&document]; + assert!( + window.params.side.is_none(), + "no side parameter is left on the document window" + ); + assert!( + !window.params.dedicated, + "the document window is not dedicated by a degraded request" + ); + assert!( + window.params.quit_action().is_none(), + "NO quit action is left behind — `q` must not try to restore a \ + presentation that never happened" + ); + assert_eq!( + core.registry + .borrow() + .get(window.buffer_id) + .expect("live buffer") + .name(), + "*degraded*", + "the buffer still reached the document target" + ); + } + + s.core.borrow_mut().active_frontend = FrontendId::LOCAL; +} + #[test] fn acc22_jump_histories_are_per_frontend_and_skip_stale_side_origins() { let s = editor(); diff --git a/tests/compile_mode_acceptance.rs b/tests/compile_mode_acceptance.rs index a223406..1b38abe 100644 --- a/tests/compile_mode_acceptance.rs +++ b/tests/compile_mode_acceptance.rs @@ -84,6 +84,13 @@ fn errors_buffer(s: &EditorState) -> String { fn editor() -> EditorState { let s = EditorState::new_with_roots(&crate::iso::roots()); exec(&s, "pmacs.lsp.config = {}"); + // Bottom-panel Stage 3: a panel is derived-hidden while the + // frontend's frame geometry is unknown, so the default-placement + // tests here could neither see nor focus one. Geometry is + // authoritative state and a grid frontend's real frame size IS its + // declaration; every test that does not render declares it, exactly + // as `bottom_panel_stage1_acceptance` always has. + s.sync_frame_geometry(FrontendId::LOCAL, pmacs::protocol::CellSize::new(24, 80)); s } @@ -148,7 +155,38 @@ fn pump_until( } /// Start a compile run programmatically with an explicit cwd. +/// Run a compilation **in the document window**, via an explicit +/// `display = "current"`. +/// +/// Bottom-panel Stage 3 flipped `compile.run`'s default to the panel +/// with `select = false`, so a default run leaves focus in the user's +/// document — and every compile-mode chord is bound +/// `scope = "buffer", buffer = slot.buf` (`compile.lua:221`): `RET`, +/// `n`, `p`, `q`, `C-c C-k`, `g`, and the seven undo no-ops. None of +/// them dispatch unless `*compilation*` is focused. +/// +/// **This suite's subject is compile-BUFFER behaviour** — streaming, +/// styling, error navigation, kill/reap, undo suppression — all of which +/// genuinely require that buffer selected. Opting out here is a +/// statement about what these tests are for, not a way to make them +/// green: the placement-subject tests use [`compile_run_default`] +/// instead, and the panel-local contract itself is pinned by +/// `acc34_default_placement_leaves_compile_chords_panel_local`. fn compile_run(s: &EditorState, cmdline: &str, cwd: &Path) { + exec( + s, + &format!( + "pmacs.compile.run({cmdline:?}, {{ cwd = {:?}, display = \"current\" }})", + cwd.display().to_string() + ), + ); +} + +/// Run a compilation through the **Stage 3 default** — no `display` at +/// all, so it resolves to the panel with `select = false`. +/// +/// For tests whose subject IS placement. +fn compile_run_default(s: &EditorState, cmdline: &str, cwd: &Path) { exec( s, &format!( @@ -715,17 +753,113 @@ fn acc14_malformed_rule_containers_fail_closed() { /// Fixture: a target file plus a compile run reporting one error at /// target.c:3:2. Returns the editor, finished, in *compilation*. -fn error_fixture(dir: &Path) -> EditorState { +/// The same fixture through the **Stage 3 default** — the compilation +/// lands in the panel and focus stays in the document. +fn error_fixture_default(dir: &Path) -> EditorState { std::fs::write(dir.join("target.c"), "l1\nl2\nl3 body\nl4\n").unwrap(); let mut s = editor(); - compile_and_finish(&mut s, "printf 'target.c:3:2: error: boom\\n'", dir); + compile_run_default(&s, "printf 'target.c:3:2: error: boom\\n'", dir); + assert!( + pump_until(&mut s, 10_000, |s| compilation_text(s) + .contains("[compile ")), + "compile run must reach its exit marker" + ); s } +/// Is `*compilation*` currently shown in a side window? +fn compilation_is_panelled(s: &EditorState) -> bool { + let core = s.core.borrow(); + core.windows.values().any(|w| { + w.is_side() + && core + .registry + .borrow() + .get(w.buffer_id) + .is_ok_and(|b| b.name() == "*compilation*") + }) +} + +/// Bottom-panel Stage 3 §1.5a — compile's chords are PANEL-LOCAL, and +/// that is a contract rather than an accidental reachability loss. +/// +/// The default flip put compile output in the panel with +/// `select = false`, so document focus survives a build. Every +/// compile-mode chord is bound `scope = "buffer", buffer = slot.buf` +/// (`compile.lua:221`), so none of them dispatch from the document — +/// `C-c C-k` included. +/// +/// This is pinned so a future change that quietly promotes one of them +/// to a global binding has to edit a test that says why it was not. The +/// capability is NOT lost: `M-x compile.kill` reaches the running slot +/// from anywhere, because its body falls back to `compile_slot()` when +/// the caller is not in a compile buffer. +/// +/// A global `C-c C-k` is a command-surface decision and belongs in its +/// own framing, not in a placement flip. +#[test] +fn acc34_default_placement_leaves_compile_chords_panel_local() { + let dir = tempfile::tempdir().unwrap(); + let mut s = editor(); + compile_run_default(&s, "echo ready; sleep 30", dir.path()); + assert!( + pump_until(&mut s, 5_000, |s| compilation_text(s).contains("\nready\n")), + "the run is live before we test the chord; buffer:\n{}", + compilation_text(&s) + ); + assert_ne!( + active_buffer_name(&s), + "*compilation*", + "premise: the default left focus in the document" + ); + + // C-c C-k from the DOCUMENT does not reach compile-mode. + ctrl(&mut s, 'c'); + ctrl(&mut s, 'k'); + assert!( + !status(&s).contains("killed"), + "C-c C-k must NOT dispatch to compile from the document window; \ + status was: {}", + status(&s) + ); + + // The capability is reachable, just not by that chord from here. + exec(&s, "pmacs.command.invoke('compile.kill')"); + assert!( + status(&s).contains("killed"), + "M-x compile.kill still reaches the running slot from anywhere; \ + status was: {}", + status(&s) + ); +} + #[test] fn acc15_ret_visits_error_and_jump_back_returns() { let dir = tempfile::tempdir().unwrap(); - let mut s = error_fixture(dir.path()); + // Stage 3: this test's SUBJECT is placement, so it takes the default + // — Q#BP12 requires compilation panel -> RET source -> `M-,` back to + // the still-present panel, with the document window intact. + let mut s = error_fixture_default(dir.path()); + assert!( + compilation_is_panelled(&s), + "premise: the default put compilation in the panel" + ); + assert_ne!( + active_buffer_name(&s), + "*compilation*", + "premise: select = false left focus in the document" + ); + + // The chords are PANEL-LOCAL (§1.5a), so reaching them is an + // explicit focus change — that is the contract, not a workaround. + ctrl(&mut s, 'x'); + press(&mut s, KeyCode::Char('o')); + assert_eq!( + active_buffer_name(&s), + "*compilation*", + "C-x o reaches the panel" + ); + // Cursor starts on the header (row 0): RET there reports and // stays. press(&mut s, KeyCode::Enter); @@ -742,9 +876,33 @@ fn acc15_ret_visits_error_and_jump_back_returns() { let line: i64 = eval(&s, "return pmacs.editor.cursor_line()"); let col: i64 = eval(&s, "return pmacs.editor.cursor_col()"); assert_eq!((line, col), (2, 1), "0-based landing from 1-based 3:2"); - // M-, returns to the compilation buffer (jump ring). + // M-, returns to the compilation buffer (jump ring) — and the panel + // is still present rather than having been consumed by the visit. alt(&mut s, ','); assert_eq!(active_buffer_name(&s), "*compilation*"); + assert!( + compilation_is_panelled(&s), + "the panel survives the visit and the jump back" + ); + // …and the jump back FOCUSED the panel rather than duplicating + // `*compilation*` into the document window. Without this, the + // assertion above would pass for a tree holding the buffer twice. + let doc_shows_compilation = { + let core = s.core.borrow(); + core.windows.values().any(|w| { + !w.is_side() + && core + .registry + .borrow() + .get(w.buffer_id) + .is_ok_and(|b| b.name() == "*compilation*") + }) + }; + assert!( + !doc_shows_compilation, + "M-, must focus the existing panel, not clone the compilation \ + buffer into a document window" + ); } #[test] @@ -984,7 +1142,13 @@ fn acc24_command_path_undo_after_completed_run_recovers_immediately() { exec( &s, &format!( - "pmacs.shell.command('echo shell-out', {{ cwd = {:?} }})", + // Stage 3: explicit `display = "current"`. `shell.command` + // shares compile's `start_run` and so flipped with it, but + // this test's subject is that `M-x buffer.undo` recovers + // SYNCHRONOUSLY in the generated buffer — which requires + // that buffer focused, or the undo targets the document + // instead and the test would pass for the wrong reason. + "pmacs.shell.command('echo shell-out', {{ cwd = {:?}, display = 'current' }})", dir.path().display().to_string() ), ); diff --git a/tests/compile_mode_crdt_acceptance.rs b/tests/compile_mode_crdt_acceptance.rs index ba79d49..a6970d9 100644 --- a/tests/compile_mode_crdt_acceptance.rs +++ b/tests/compile_mode_crdt_acceptance.rs @@ -105,6 +105,13 @@ where /// the daemon broadcasts a snapshot for the newly-CRDT-backed buffer /// and via the active-buffer-follow path). Re-seats the replica's /// mirror on that buffer. +/// +/// **Bottom-panel Stage 3:** this is why the runs below pass an explicit +/// `display = "current"`. The default now places compile output in the +/// panel with `select = false`, so the ACTIVE buffer never becomes +/// `*compilation*` — and the active-buffer-follow path named above is +/// what publishes the snapshot this function waits for. The subject +/// here is CRDT convergence of a generated buffer, not placement. fn adopt_next_buffer(replica: &mut Replica, what: &str) { let deadline = std::time::Instant::now() + Duration::from_secs(10); loop { @@ -188,7 +195,7 @@ fn compile_run_converges_and_replica_edit_triggers_recovery() { name = "test.compile", description = "compile-mode CRDT fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 9", command = "test.compile" }} @@ -275,7 +282,7 @@ fn r3f1_unicode_cr_backspace_survive_crdt_replication() { name = "test.compile-unicode", description = "round-3 unicode fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 8", command = "test.compile-unicode" }} @@ -327,7 +334,7 @@ fn r4f1_column_rewrites_replicate_and_converge() { name = "test.compile-columns", description = "round-4 column-rewrite fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 7", command = "test.compile-columns" }} diff --git a/tests/listview_acceptance.rs b/tests/listview_acceptance.rs index d546f00..e316e41 100644 --- a/tests/listview_acceptance.rs +++ b/tests/listview_acceptance.rs @@ -17,7 +17,23 @@ use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; use pmacs::buffer::BufferId; use pmacs::editor::EditorState; -use pmacs::protocol::FrontendId; +use pmacs::protocol::{CellSize, FrontendId}; + +/// Bottom-panel Stage 3: a listview now opens into the PANEL by default, +/// and a panel is derived-hidden while the frontend's frame geometry is +/// unknown — so focus would fall back to the document window and every +/// panel assertion here would read the wrong buffer. +/// +/// `bottom_panel_stage1_acceptance` has always declared geometry for the +/// same reason: a grid frontend's real frame size IS its declaration, +/// and every test that does not render must state it before any input. +/// This suite never needed to while listview defaulted to the current +/// window. It does now. +fn editor() -> EditorState { + let s = EditorState::new_with_roots(&crate::iso::roots()); + s.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(24, 80)); + s +} fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { KeyEvent { @@ -194,6 +210,31 @@ fn open_test_panel(s: &mut EditorState) { .expect("open test panel"); } +/// Open the fixture panel with an explicit `display = "current"`, for +/// the tests whose subject requires it to sit in a document window. +fn open_test_panel_in_document(s: &mut EditorState) { + s.lua_host + .lua() + .load( + r#" + _G.VISITED = nil + pmacs.listview.open { + name = "*test-panel*", + header = "3 items RET visit q quit", + display = "current", + rows = { + { text = "alpha", item = "A" }, + { text = "beta", item = "B" }, + { text = "gamma", item = "C" }, + }, + on_visit = function(item) _G.VISITED = item end, + } + "#, + ) + .exec() + .expect("open test panel in a document window"); +} + /// `(active buffer name, buffer text, cursor line, visited)` probed /// through the Lua surface. fn probe(s: &EditorState) -> (String, String, i64, Option) { @@ -212,7 +253,7 @@ fn probe(s: &EditorState) -> (String, String, i64, Option) { #[test] fn open_seats_cursor_and_ret_visits_the_row() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let (name, text, line, _) = probe(&s); assert_eq!(name, "*test-panel*"); @@ -227,7 +268,7 @@ fn open_seats_cursor_and_ret_visits_the_row() { #[test] fn header_row_is_not_visitable() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); press(&mut s, KeyCode::Char('p')); // up onto the header press(&mut s, KeyCode::Enter); @@ -237,7 +278,7 @@ fn header_row_is_not_visitable() { #[test] fn q_restores_the_previous_buffer() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); press(&mut s, KeyCode::Char('q')); let (name, _, _, _) = probe(&s); @@ -246,7 +287,7 @@ fn q_restores_the_previous_buffer() { #[test] fn panel_rejects_typing() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let (_, before, _, _) = probe(&s); press(&mut s, KeyCode::Char('z')); // unbound printable → self-insert → intercept rejects @@ -258,7 +299,7 @@ fn panel_rejects_typing() { fn dispatch_idle_is_false_while_a_panel_is_focused() { // Q#P6: while the panel is the active buffer, semantic frontends // must round-trip every key (RET = visit, not an optimistic \n). - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); assert!(s.dispatch_idle(), "scratch buffer: idle"); open_test_panel(&mut s); assert!(!s.dispatch_idle(), "panel focused: keys must round-trip"); @@ -268,7 +309,7 @@ fn dispatch_idle_is_false_while_a_panel_is_focused() { #[test] fn refresh_reruns_the_source_and_reseats() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); press(&mut s, KeyCode::Char('g')); let (_, text, line, _) = probe(&s); @@ -304,7 +345,7 @@ const PANEL_TEXT: &str = "3 items RET visit q quit\nalpha\nbeta\ngamma"; /// consulting the intercept chain. #[test] fn s1_1_the_undo_chord_cannot_empty_a_listview_panel() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); assert_eq!(active_text(&s), PANEL_TEXT, "precondition: rendered"); @@ -327,7 +368,7 @@ fn s1_1_the_undo_chord_cannot_empty_a_listview_panel() { /// *Bite:* same empty result on the pre-image. #[test] fn s1_2_m_x_buffer_undo_cannot_empty_a_listview_panel() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); m_x(&mut s, "buffer.undo"); @@ -355,7 +396,7 @@ fn s1_2_m_x_buffer_undo_cannot_empty_a_listview_panel() { /// raising. #[test] fn s1_4_the_owners_refresh_still_works_after_the_lock() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let panel = id_of(&s, "*test-panel*"); assert!( @@ -388,7 +429,7 @@ fn s1_4_the_owners_refresh_still_works_after_the_lock() { /// therefore passes the rope half and fails the lifted half. #[test] fn s1_5_the_rope_lock_and_named_intercept_refuse_in_order() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let panel = id_of(&s, "*test-panel*"); let before = active_text(&s); @@ -453,8 +494,15 @@ fn s1_5_the_rope_lock_and_named_intercept_refuse_in_order() { /// pinned through `dispatch_idle_for` rather than through `read_only`. #[test] fn s1_6_round_trip_input_survives_the_adoption() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); - open_test_panel(&mut s); + let mut s = editor(); + // Stage 3: an EXPLICIT opt-out, because this fixture genuinely needs + // the document window. `dispatch_idle` goes false for TWO reasons — + // a round-trip buffer (this test's subject) and a focused panel + // (`dispatch_idle_is_false_while_a_panel_is_focused`, a different + // test). Letting the panel default apply here would satisfy the gate + // for the wrong reason and the test would pass while proving + // nothing. The premise assertion below is what keeps that honest. + open_test_panel_in_document(&mut s); // (a) the premise. { @@ -498,7 +546,7 @@ fn s1_6_round_trip_input_survives_the_adoption() { /// the old line index live and this paint assertion bites. #[test] fn s1_7_a_shrinking_refresh_reaches_the_window() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let painted = paint_active_window(&s, 6, 24); assert_eq!( @@ -540,7 +588,7 @@ fn s1_7_a_shrinking_refresh_reaches_the_window() { /// builtin/runtime/listview.lua` falsifies it. #[test] fn s1_9_a_foreign_buffer_with_the_panels_name_is_never_adopted() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); exec( &s, "FOREIGN = pmacs.buffer.create('*test-panel*')\n\ @@ -585,7 +633,7 @@ fn s1_9_a_foreign_buffer_with_the_panels_name_is_never_adopted() { /// created. #[test] fn s1_10_the_disambiguation_limit_raises_rather_than_adopting() { - let s = EditorState::new_with_roots(&crate::iso::roots()); + let s = editor(); exec( &s, "MINE = pmacs.buffer.create('*test-panel*')\n\ @@ -624,7 +672,7 @@ fn s1_10_the_disambiguation_limit_raises_rather_than_adopting() { /// command produced, never on "it did not raise". #[test] fn s1_11_a_disambiguated_panel_still_answers_ret_g_and_q() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); exec( &s, "FOREIGN = pmacs.buffer.create('*test-panel*')\n\ @@ -676,21 +724,96 @@ fn s1_11_a_disambiguated_panel_still_answers_ret_g_and_q() { /// /// *Bite:* restore the name-keyed `panels[d.name]` lookup while keeping /// the disambiguation and `q` lands back in `*test-panel*<2>`. +/// Bottom-panel Stage 3 — the SIDE-WINDOW half of `q`, complementary to +/// `s1_12`'s buffer-level `p.prev` rule. +/// +/// The parent framing's criterion 20 requires listview `q` to route +/// through `window.quit`, with `C → B → A` restoring each prior +/// presentation and the first panel deleting its wrapper. Before Stage 3 +/// this was unreachable from listview's own entry point without an +/// explicit `display = "panel"` on every open; the default flip makes it +/// the ordinary path, so it gets an ordinary-path test. +/// +/// The two mechanisms are complementary, not competing: **presentation +/// history chains in the side slot**, while **`p.prev` prevents +/// raw-switch and capability-fallback listview loops**. `s1_12` pins the +/// second by keeping its panels in document windows; this pins the +/// first. +#[test] +fn s3_1_q_walks_the_side_presentation_chain_back_to_the_document() { + let mut s = editor(); + exec( + &s, + "ORIGIN = pmacs.buffer.create('*origin*')\n\ + pmacs.window.switch_buffer(ORIGIN)", + ); + assert_eq!(active_name(&s), "*origin*", "premise: a document window"); + + for name in ["*panel-a*", "*panel-b*", "*panel-c*"] { + exec( + &s, + &format!( + "pmacs.listview.open {{ name = '{name}', header = 'H', \ + rows = {{ {{ text = 'x', item = 'X' }} }} }}" + ), + ); + assert_eq!(active_name(&s), name, "each open takes the panel slot"); + } + + // C → B → A: each `q` restores the presentation the next one + // replaced, rather than forgetting them or jumping straight out. + press(&mut s, KeyCode::Char('q')); + assert_eq!( + active_name(&s), + "*panel-b*", + "q restores the replaced panel" + ); + press(&mut s, KeyCode::Char('q')); + assert_eq!(active_name(&s), "*panel-a*", "…and again, in order"); + + // A → delete: the FIRST panel deletes its wrapper and focus lands + // back in the document. This is what bounds the chain — a loop + // between panels would never reach here. + press(&mut s, KeyCode::Char('q')); + assert_eq!( + active_name(&s), + "*origin*", + "the last q deletes the wrapper and returns to the document" + ); + assert!( + s.core.borrow().windows.values().all(|w| !w.is_side()), + "the side wrapper is collapsed, not left empty" + ); +} + #[test] fn s1_12_the_q_target_capture_is_not_inverted_across_two_panels() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); exec( &s, "FOREIGN = pmacs.buffer.create('*test-panel*')\n\ ORIGIN = pmacs.buffer.create('*origin*')\n\ pmacs.window.switch_buffer(ORIGIN)", ); - open_test_panel(&mut s); + // Stage 3: BOTH opens are explicitly `display = "current"`, and that + // is what keeps this test meaningful rather than what makes it pass. + // + // Its subject is the BUFFER-level `p.prev` skip rule and the Q#GB18 + // name-keyed identity guard — the `FOREIGN` buffer above shares the + // panel's name, so the disambiguation to `*test-panel*<2>` is the + // regression this pins. Under the panel default those two listviews + // would share the one bottom slot and `q` would exercise the + // SIDE-WINDOW restore chain instead (Q#BP2c criterion 20), which is + // a different mechanism with its own test below. Keeping them in + // document windows isolates the two, so a `p.prev` inversion stays + // detectable rather than being masked by presentation history. + open_test_panel_in_document(&mut s); assert_eq!(active_name(&s), "*test-panel*<2>", "premise: disambiguated"); exec( &s, "pmacs.listview.open { name = '*other-panel*', header = 'O', \ + display = 'current', \ rows = { { text = 'x', item = 'X' } } }", ); assert_eq!(active_name(&s), "*other-panel*", "premise: second panel"); @@ -700,7 +823,8 @@ fn s1_12_the_q_target_capture_is_not_inverted_across_two_panels() { assert_ne!( active_name(&s), "*test-panel*<2>", - "q must never return into another panel --- the chained-panel loop" + "q must never return into another panel via p.prev --- the \ + raw-switch/capability-fallback loop this rule exists to prevent" ); assert_eq!( active_name(&s), diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index 32511ff..68b1825 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -7982,6 +7982,19 @@ fn m4_5_symbols_and_highlight_round_trip() { /// (shared bootstrap for the panel tests). fn open_against_fake(path: &std::path::Path) -> pmacs::editor::EditorState { let mut state = pmacs::editor::EditorState::new_with_roots(&crate::iso::roots()); + // Bottom-panel Stage 3: the LSP panels are listview consumers, so + // they inherited the panel default — and a panel is derived-hidden + // while frame geometry is unknown. Without this declaration the + // outline and hover panels open into a hidden window and the probes + // below read the document buffer instead. + // + // This suite is the TRANSITIVE adopter the arc's own Q#BP12 table + // never named: nothing here calls `listview.open` directly, but + // `lsp.lua` does. + state.sync_frame_geometry( + pmacs::protocol::FrontendId::LOCAL, + pmacs::protocol::CellSize::new(40, 100), + ); let fake = fake_lsp_path(); state .lua_host @@ -8013,6 +8026,10 @@ fn open_against_fake(path: &std::path::Path) -> pmacs::editor::EditorState { /// method): open, depth-indented rows, RET jump-ring visit to the /// symbol's selectionRange, M-, back to the outline row, q restore. #[test] +#[allow( + clippy::too_many_lines, + reason = "criterion 58's whole flow: open -> visit -> jump-back -> quit, in one scenario" +)] fn outline_panel_opens_visits_and_restores() { let dir = tempfile::tempdir().expect("tempdir"); let a_path = dir.path().join("a.rs"); @@ -8097,6 +8114,30 @@ fn outline_panel_opens_visits_and_restores() { .eval() .expect("post-jump-back probe"); assert_eq!(name, "*outline*", "M-, returns to the outline panel"); + // Q#BP11c — and it FOCUSES the panel rather than cloning `*outline*` + // into the document window. The jump ring stores only + // `(BufferId, Position)`, so a naive `jump_back` would switch the + // active (document) window to the panel's buffer and leave the panel + // open too: the duplicate-buffer/window corruption that question + // names. The assertion above cannot tell those apart on its own. + let (panelled, doc_clone): (bool, bool) = { + let core = state.core.borrow(); + let named = |w: &pmacs::window::Window| { + core.registry + .borrow() + .get(w.buffer_id) + .is_ok_and(|b| b.name() == "*outline*") + }; + ( + core.windows.values().any(|w| w.is_side() && named(w)), + core.windows.values().any(|w| !w.is_side() && named(w)), + ) + }; + assert!(panelled, "the outline is still in its panel after M-,"); + assert!( + !doc_clone, + "M-, must not clone *outline* into a document window (Q#BP11c)" + ); // q restores the source buffer. state.dispatch_key( diff --git a/tests/vterm_stage2_acceptance.rs b/tests/vterm_stage2_acceptance.rs index d5b86e3..b537d7d 100644 --- a/tests/vterm_stage2_acceptance.rs +++ b/tests/vterm_stage2_acceptance.rs @@ -87,11 +87,20 @@ fn lua_surface_is_strict_fresh_transactional_and_context_safe() { let kind: String = lua .load(format!( r#" + -- Bottom-panel Stage 3: an EXPLICIT opt-out. This test + -- drives an 8x30 frame (see `sync_terminal_layout` + -- below), and DEFAULT_PANEL_ROWS is 12 — a panel plus a + -- MIN_WINDOW_OUTER_ROWS document window cannot fit in + -- eight rows, so the default placement would be hidden + -- by construction and there would be no layout to sync. + -- The subject here is the Lua surface being strict, + -- fresh and transactional, not placement. TERM_BUFFER = pmacs.terminal.open {{ command = {command_lua}, args = {{ "-c", "printf 'copy-me\\n'; sleep 30" }}, rows = 4, cols = 30, + display = "current", }} local first = pmacs.terminal.state(TERM_BUFFER) first.process.kind = "poisoned" @@ -365,6 +374,11 @@ fn lua_surface_is_strict_fresh_transactional_and_context_safe() { #[allow(clippy::too_many_lines, reason = "shared view and controller scenario")] fn shared_screen_keeps_view_scroll_selection_and_controller_independent() { let mut state = EditorState::new_with_roots(&crate::iso::roots()); + // Bottom-panel Stage 3: `terminal.open` now defaults to the panel + // (with `select = true`, so focus follows). A panel is derived-hidden + // while frame geometry is unknown, and a hidden terminal has no + // layout to sync — declare it, as the panel suites always have. + state.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(24, 80)); let mut spec = TerminalSpec::new("/bin/sh"); spec.args = vec![ "-c".into(), @@ -509,6 +523,11 @@ fn terminal_escape_gates_local_bindings_and_double_escape_sends_interrupt() { input_path.to_str().expect("UTF-8 input path") ); let mut state = EditorState::new_with_roots(&crate::iso::roots()); + // Bottom-panel Stage 3: `terminal.open` now defaults to the panel + // (with `select = true`, so focus follows). A panel is derived-hidden + // while frame geometry is unknown, and a hidden terminal has no + // layout to sync — declare it, as the panel suites always have. + state.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(24, 80)); state .lua_host .lua() @@ -802,12 +821,22 @@ fn real_tui_terminal_smoke_restores_host_after_output_input_resize_scroll_copy_a if f then f:write(text) f:close() end end breadcrumb({:?}, "1") + -- Bottom-panel Stage 3: an EXPLICIT opt-out. This is the real + -- TUI smoke -- a genuine pmacs in a genuine PTY -- and its + -- subject is that the HOST terminal is restored after output, + -- input, resize, scroll, copy (OSC 52) and bell. All of that is + -- measured against the rendered host stream over a full-frame + -- terminal; placing the child in a 12-row bottom panel changes + -- the geometry those measurements are taken over and the copy + -- path stops reaching the host. Placement is covered by the + -- panel suites, not here. local ok, terminal_buffer = pcall(pmacs.terminal.open, {{ command = "/bin/sh", args = {{ "-c", "exec /usr/bin/python3 -c \"$1\"", "pmacs-vterm-probe", {} }}, rows = 10, cols = 40, scrollback_rows = 200, + display = "current", }}) breadcrumb({:?}, ok and "ok" or ("ERROR: " .. tostring(terminal_buffer))) assert(ok, terminal_buffer) diff --git a/tests/vterm_stage3_acceptance.rs b/tests/vterm_stage3_acceptance.rs index afa4d83..0305146 100644 --- a/tests/vterm_stage3_acceptance.rs +++ b/tests/vterm_stage3_acceptance.rs @@ -608,8 +608,14 @@ pmacs.command.define { name = "vterm-probe.open", description = "Open the Stage 3 acceptance terminal child.", fn = function() + -- Bottom-panel Stage 3: explicit opt-out. This suite measures + -- RENDERED FRAMES and child PTY geometry against a full document + -- window; the panel default would put the child in a 12-row side + -- window and change the very geometry under test. Placement is + -- covered by the panel suites. return pmacs.terminal.open { command = "/bin/sh", + display = "current", args = { "-c", "i=0; while [ $i -lt 400 ]; do printf 'VTERMROW%02d\n' \"$i\"; i=$((i+1)); sleep 0.05; done" }, } @@ -1140,8 +1146,11 @@ pmacs.command.define { name = "vterm-probe.open", description = "Open a quiet terminal that counts SIGWINCH.", fn = function() + -- Stage 3 opt-out: this test asserts the child's PTY geometry + -- settles and stops signalling. A panel changes that geometry. return pmacs.terminal.open { command = "/bin/sh", + display = "current", args = { "-c", "n=0; trap 'n=$((n+1)); printf \"WINCH %d\r\n\" \"$n\"' WINCH; " .. "printf 'READY\r\n'; while :; do sleep 0.2; done" }, @@ -1163,8 +1172,11 @@ pmacs.command.define { name = "vterm-probe.open", description = "Open a terminal child that copies stdin to stdout.", fn = function() + -- Stage 3 opt-out: input must round-trip through a frame rendered + -- over the document window this test measures. return pmacs.terminal.open { command = "/bin/sh", + display = "current", args = { "-c", "printf 'READY\r\n'; exec cat" }, } end,