From c0eb16bd1266465ff608239f911354c1ce948e5e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Mon, 3 Aug 2026 11:10:14 -0400 Subject: [PATCH] =?UTF-8?q?feat(panel):=20flip=20the=20adopter=20default?= =?UTF-8?q?=20=E2=80=94=20Arc=207's=20last=20step?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 3 steps 3 and 4. Omitting `display` now resolves to the PANEL for listview, compile and terminal; dired keeps `"current"`, passed explicitly to the shared resolver. Per-adopter `select` per Q#BP12: listview true, compile false (passive output must not steal document focus), terminal true. The census predicted 37 failures across 5 suites and the flip produced exactly that — same suites, same per-suite counts. The measurement was a prediction, not an estimate, which is what the inverted step order was for. Final sweep: 3449 passed / 0 failed against a 3447 baseline, the +2 being new pins. THE CENSUS COUNTED FAILURES, NOT CAUSES. 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. The same applied to m4 and vterm_stage2. Geometry is authoritative state and a grid frontend's real frame size IS its declaration; the panel suites have always said so. THREE DEFECTS THE FLIP EXPOSED, each fixed rather than tested around: 1. The OUTLINE panel's `on_visit` 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. Once the panel became the default the active window WAS the outline panel, so RET clobbered the panel with the source and left nothing for `M-,` to return to. The references panel was migrated to `display_file` when the arc landed; the outline was missed because nothing exercised it from a panel until now. Q#BP11c names this exact corruption, and both the outline and compile tests now assert `M-,` FOCUSES the panel rather than cloning its buffer into the document — an assertion the previous one could not distinguish. 2. `pmacs.compile._last` stored only `{cmdline, cwd}`, so a recompile reached `start_run` with no `display` and took the new default. A user who ran `compile.run{display="current"}` would be moved into a panel the moment they pressed `g`. An opt-out that reverts on the next recompile is not an opt-out; `display` is stored and replayed, with nil kept as nil so an omitted value still resolves to the default rather than freezing at the first run's resolution. 3. `opts.display` on a nil `opts` — my own regression, introduced by fix 2 and caught by `journey_acceptance`, which is exactly what that ratchet is for. COMPILE'S CHORDS ARE NOW PANEL-LOCAL, and that is a contract rather than an accidental reachability loss. Every compile chord is bound `scope = "buffer"`, so with `select = false` none dispatch from the document — `C-c C-k` included. `acc34` pins it, and pins that `M-x compile.kill` still reaches the running slot from anywhere via its `or compile_slot()` fallback. A global chord is a command-surface decision and belongs in its own framing. TEST CLASSIFICATION WAS PER TEST, NOT PER SUITE. Two neighbouring compile tests land on opposite sides: acc15 (RET-visits-error, jump-back) asserts the NEW default, while acc16 (n/p within compile output) genuinely needs the buffer selected and says so. compile's suite-wide helper opts out because ITS subject is compile-BUFFER behaviour; the placement-subject tests use a second helper that takes the default. Every opt-out states why. Nothing was mass-added to make a suite green. s1_12's two concerns are split as directed: it keeps its Q#GB18 name-keyed-identity bite with explicit `display = "current"`, isolating the buffer-level `p.prev` skip rule, while a new `s3_1` pins the side-window presentation chain — C → B → A → delete, ending at the document with the wrapper collapsed. The mechanisms are complementary: presentation history chains in the side slot; `p.prev` prevents raw-switch and capability-fallback loops. Verified: fmt, diff-check, clippy with and without crdt, --lib 1896, --lib --features crdt 2081, pmacs-protocol 19, m4 149, required GPU 221, and the full serialized sweep at 3449/0. Co-Authored-By: Claude Opus 5 (1M context) --- builtin/runtime/compile.lua | 17 ++- builtin/runtime/listview.lua | 7 +- builtin/runtime/lsp.lua | 16 ++- docs/bottom-panel-stage3-framing.md | 46 ++++++- src/lua_bindings/mod.rs | 11 +- src/lua_bindings/window_panel.rs | 5 +- tests/bottom_panel_stage1_acceptance.rs | 42 +++++- tests/compile_mode_acceptance.rs | 174 +++++++++++++++++++++++- tests/listview_acceptance.rs | 164 +++++++++++++++++++--- tests/m4_acceptance.rs | 41 ++++++ tests/vterm_stage2_acceptance.rs | 29 ++++ 11 files changed, 507 insertions(+), 45 deletions(-) diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 1c2c674..20300cc 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -813,7 +813,10 @@ local function start_run(slot, cmdline, opts) -- panel-placed. Resolving first and testing `== "current"` afterwards -- would silently merge the two and break that opt-out. local display_omitted = opts.display == nil - local display = pmacs.window._resolve_display("compile.run", opts.display, "current") + -- 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. @@ -1150,7 +1153,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 @@ -1226,7 +1237,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/listview.lua b/builtin/runtime/listview.lua index 9c904a8..62b0cb3 100644 --- a/builtin/runtime/listview.lua +++ b/builtin/runtime/listview.lua @@ -229,7 +229,12 @@ function pmacs.listview.open(spec) -- 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. - local display = pmacs.window._resolve_display("listview.open", spec.display, "current") + -- 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/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md index e7ba982..2002fa0 100644 --- a/docs/bottom-panel-stage3-framing.md +++ b/docs/bottom-panel-stage3-framing.md @@ -236,6 +236,45 @@ 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 @@ -412,7 +451,12 @@ Inherits the parent framing's criteria 56–58, made concrete: 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 — omission still means the document for dired.** +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 diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 0dfbd2d..77bb4c7 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -9320,10 +9320,11 @@ 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 step 2: the shared resolver, still carrying the - // PRE-FLIP default. The unification must be provably - // behaviour-preserving before the default moves, so the - // flip to `AdopterDefault::Panel` is its own commit. + // 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, @@ -9331,7 +9332,7 @@ fn install_terminal( "pmacs.terminal.open", Some(&display_value), spec_table.get::>("window")?, - window_panel::AdopterDefault::Current, + 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 667cb68..fe8b758 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -262,8 +262,9 @@ pub(crate) enum AdopterPlacement { /// subtly wrong, and the next adopter is DAP. /// /// `default` is a **parameter, not a constant**, because the adopters do -/// not share one. listview / compile / terminal resolve omission to the -/// panel; **dired resolves it to `"current"`** and must keep doing so — +/// 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 diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index fb05e5e..40f2b93 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\" })"); @@ -1404,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\")"); 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/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)