diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 2e894d2..544c1d7 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -836,7 +836,13 @@ local function start_run(slot, cmdline, opts) -- 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. - if display == "panel" or already_in_panel(slot.buf) then + -- + -- 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 pmacs.window.display(slot.buf, { side = "bottom", select = false }) else pmacs.window.switch_buffer(slot.buf) diff --git a/src/editor.rs b/src/editor.rs index 246a575..79f1225 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -161,11 +161,16 @@ pub struct EditorState { /// Last left-button down event, used to synthesize terminal double /// clicks from crossterm's plain Down/Up mouse event stream. mouse_click: Option, - /// In-progress split-boundary drag (bottom-panel arc, Q#BP5), armed + /// In-progress split-boundary drags (bottom-panel arc, Q#BP5), armed /// by a left press on a mode-line row that is an exposed segment of a - /// horizontal boundary. Lives beside `mouse_click`; selection is - /// untouched for the whole gesture. - window_drag: Option, + /// horizontal boundary. Selection is untouched for the whole gesture. + /// + /// Keyed by frontend, unlike the older global `mouse_click` slot: the + /// daemon routes every attached grid frontend through one + /// `dispatch_mouse`, so a single slot would let one frontend's press + /// steal or clear another's in-flight gesture, and concurrent drags + /// are perfectly legal. + window_drag: HashMap, } #[derive(Default)] @@ -220,7 +225,6 @@ struct MouseClickState { /// mutation mid-drag cannot move a boundary that no longer exists. #[derive(Copy, Clone)] struct WindowDragState { - frontend_id: FrontendId, owner: WindowId, last_row: u32, } @@ -619,7 +623,7 @@ impl EditorState { snippets, statusline_registry, mouse_click: None, - window_drag: None, + window_drag: HashMap::new(), } } @@ -875,6 +879,9 @@ impl EditorState { /// Drop one detached frontend's pending key and terminal escape state. pub fn detach_frontend_input(&mut self, frontend_id: FrontendId) { self.dispatchers.remove(&frontend_id); + // A detached frontend cannot finish a divider gesture, and its + // `owner` window is about to stop being live (Q#BP5). + self.window_drag.remove(&frontend_id); self.terminal_manager .borrow_mut() .detach_frontend(frontend_id); @@ -1930,17 +1937,16 @@ impl EditorState { // grid frontend through this same dispatcher, so an unscoped // check would let one frontend's in-flight drag cancel and // swallow another frontend's clicks. - if self - .window_drag - .is_some_and(|drag| drag.frontend_id == frontend_id) - { + if self.window_drag.contains_key(&frontend_id) { match ev.kind { MouseEventKind::Drag(MouseButton::Left) => { self.drag_window_boundary(frontend_id, cell_row, term_size); } // Any other event — release, a different button, a - // wheel notch — ends the gesture. - _ => self.window_drag = None, + // wheel notch — ends THIS frontend's gesture only. + _ => { + self.window_drag.remove(&frontend_id); + } } return; } @@ -2076,11 +2082,20 @@ impl EditorState { .views .get(&frontend_id) .is_some_and(|view| view.layout.boundary_below(owner).is_some()); - self.window_drag = is_divider.then_some(WindowDragState { - frontend_id, - owner, - last_row: cell_row, - }); + // Only this frontend's slot is written, and only its own press + // can clear it — a peer pressing some other window's mode line + // must not disarm an in-flight gesture here. + if is_divider { + self.window_drag.insert( + frontend_id, + WindowDragState { + owner, + last_row: cell_row, + }, + ); + } else { + self.window_drag.remove(&frontend_id); + } } /// Continue an armed divider drag (Q#BP5). @@ -2095,16 +2110,16 @@ impl EditorState { cell_row: u32, term_size: CellSize, ) { - let Some(drag) = self.window_drag else { + let Some(drag) = self.window_drag.get(&frontend_id).copied() else { return; }; - if drag.frontend_id != frontend_id { - return; - } - self.window_drag = Some(WindowDragState { - last_row: cell_row, - ..drag - }); + self.window_drag.insert( + frontend_id, + WindowDragState { + last_row: cell_row, + ..drag + }, + ); let delta = i64::from(cell_row) - i64::from(drag.last_row); let Ok(delta) = i32::try_from(delta) else { return; diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 959edbe..0d585ea 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -12365,30 +12365,23 @@ fn install_window_module(lua: &Lua, core: &SharedCore) -> mlua::Result { // panel" without first selecting the panel. win.set( "buffer", - lua.create_function(move |lua, target: Option| -> mlua::Result { - let Some(raw) = target else { - return Ok(BufferIdLua(cc.borrow().active_buffer_id())); - }; - let fid = window_panel::acting_frontend(lua, &cc); - let core = cc.borrow(); - let view = core.views.get(&fid).ok_or_else(|| { - mlua::Error::runtime("pmacs.window.buffer: acting frontend has no layout") - })?; - let id = view - .layout - .iter_ids() - .into_iter() - .find(|id| id.raw() == raw) - .ok_or_else(|| { - mlua::Error::runtime(format!( - "pmacs.window.buffer: window {raw} is not live in this frontend's layout" - )) - })?; - core.windows - .get(&id) - .map(|window| BufferIdLua(window.buffer_id)) - .ok_or_else(|| mlua::Error::runtime("pmacs.window.buffer: window not live")) - })?, + lua.create_function( + move |lua, target: Option| -> mlua::Result { + // Both arms resolve through the ACTING frontend, using + // the same validator the rest of the window surface + // does — no ambient `active_buffer_id()` asymmetry. + let fid = window_panel::acting_frontend(lua, &cc); + let id = match target { + Some(raw) => window_panel::lookup_window(&cc, fid, raw)?, + None => window_panel::selected_window(&cc, fid)?, + }; + cc.borrow() + .windows + .get(&id) + .map(|window| BufferIdLua(window.buffer_id)) + .ok_or_else(|| mlua::Error::runtime("pmacs.window.buffer: window not live")) + }, + )?, )?; } diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index 068f6c8..f4833ef 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -189,7 +189,7 @@ fn parse_request( /// Not `active_window_id()`, which resolves through the ambient active /// frontend: every other id in this module is `fid`-scoped, and the two /// only coincide because dispatch happens to set `active_frontend` first. -fn selected_window(core: &SharedCore, fid: FrontendId) -> mlua::Result { +pub(crate) fn selected_window(core: &SharedCore, fid: FrontendId) -> mlua::Result { core.borrow() .views .get(&fid) @@ -199,7 +199,11 @@ fn selected_window(core: &SharedCore, fid: FrontendId) -> mlua::Result /// Resolve a raw Lua window id, refusing one that is not live in the /// acting frontend's layout (Q#BP11). -fn lookup_window(core: &SharedCore, fid: FrontendId, raw: u64) -> mlua::Result { +pub(crate) fn lookup_window( + core: &SharedCore, + fid: FrontendId, + raw: u64, +) -> mlua::Result { let core = core.borrow(); let view = core .views diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 5d9d311..1c0957d 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -132,8 +132,7 @@ impl TerminalManager { last_bell_count: bell_count, ..TerminalViewState::default() }); - normalize_state(state, projection); - state.viewport_size = Some(viewport_size); + declare_view_size(state, projection, viewport_size); Some(project_snapshot( key.buffer_id, viewport_size, @@ -234,8 +233,7 @@ impl TerminalManager { last_bell_count: bell_count, ..TerminalViewState::default() }); - normalize_state(state, projection); - state.viewport_size = Some(viewport_size); + declare_view_size(state, projection, viewport_size); let rows = retained_rows(projection); let geometry = view_geometry(&rows, state, viewport_size.rows); Some(TerminalViewStatus { @@ -289,8 +287,7 @@ impl TerminalManager { last_bell_count: bell_count, ..TerminalViewState::default() }); - normalize_state(state, projection); - state.viewport_size = Some(viewport_size); + declare_view_size(state, projection, viewport_size); true } @@ -595,6 +592,56 @@ fn clamp_or_clear(rows: &RetainedRows<'_>, anchor: LogicalCellAnchor) -> Option< .then(|| row_lead(first)) } +/// The shared viewport-size declaration path (bottom-panel arc, Q#BP7). +/// +/// Normalize, then re-arm live-tail following when the newly declared +/// viewport reaches the tail, then record the size. Every path that +/// *declares* a size routes through here so grid and semantic +/// declarations cannot disagree; `scroll_view` and `begin_selection` +/// deliberately do not, because they write `top` themselves. +fn declare_view_size( + state: &mut TerminalViewState, + projection: BorrowedScreenProjection<'_>, + viewport_size: CellSize, +) { + normalize_state(state, projection); + rearm_follow_on_growth(state, projection, viewport_size.rows); + state.viewport_size = Some(viewport_size); +} + +/// Q#BP7 item 1: **growth reaching the live tail re-arms follow.** +/// +/// A height change is a viewport change, never a scroll change — `top` +/// is preserved verbatim — but once a taller viewport covers the tail, +/// staying anchored would leave the view frozen just short of the live +/// output while `at_bottom` reported `true`: `at_bottom` is the +/// instantaneous geometric readout `scroll_offset == 0`, so it cannot +/// distinguish "following" from "anchored, and currently tall enough to +/// reach". The next rows the child prints would then push the anchored +/// view back into history with nothing to explain it. +/// +/// **Only when no selection is active** (R1-8): a historical selection +/// froze this anchor on purpose, and growth must not yank the user's +/// region out from under them. `scroll_view` already handles the +/// scroll-driven arm (`next == tail_start`), so during ordinary +/// scrolling `scroll_offset == 0` implies follow is already armed — +/// which makes this rule fire on exactly the growth (and shrink-back) +/// case it names, and be idempotent everywhere else. +fn rearm_follow_on_growth( + state: &mut TerminalViewState, + projection: BorrowedScreenProjection<'_>, + viewport_rows: u32, +) { + if state.top.is_none() || state.selection.is_some() || viewport_rows == 0 { + return; + } + let rows = retained_rows(projection); + if view_geometry(&rows, state, viewport_rows).scroll_offset == 0 { + state.top = None; + state.selection_froze_top = false; + } +} + fn normalize_state(state: &mut TerminalViewState, projection: BorrowedScreenProjection<'_>) { if state .alternate_active diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index e413394..f9acea1 100644 --- a/tests/bottom_panel_stage1_acceptance.rs +++ b/tests/bottom_panel_stage1_acceptance.rs @@ -1336,6 +1336,26 @@ fn acc19b_recompile_reuses_the_panel_instead_of_duplicating_into_the_document() "…and did not duplicate itself into the document window" ); + // An EXPLICIT `display = "current"` still wins over the inference: + // it is the documented user-facing opt-out from the Stage 3 default + // flip, so it must reach the raw switch even while the panel holds + // this buffer. The resulting duplicate presentation is the escape + // hatch's documented cost (R3-rp2). + s.core + .borrow_mut() + .focus_window(FrontendId::LOCAL, document); + exec(&s, "pmacs.compile.run(\"true\", { display = \"current\" })"); + assert_eq!( + s.core.borrow().windows[&document].buffer_id, + compilation, + "explicit \"current\" reached the raw switch" + ); + assert_eq!( + s.core.borrow().windows[&panel].buffer_id, + compilation, + "…and the panel still holds it too — the escape hatch's cost" + ); + // A compilation that is NOT in a panel keeps the pre-arc raw switch. let s = editor(); exec(&s, "pmacs.compile.run(\"true\")"); @@ -1818,6 +1838,15 @@ fn acc30c_an_armed_drag_does_not_swallow_another_frontends_mouse_events() { "the peer's click reached its own window instead of being swallowed" ); + // …a peer press on a MODE-LINE row must not steal or clear the slot + // either — that press reaches the arming path, which a single global + // slot would let it overwrite. + s.dispatch_mouse( + other, + mouse(MouseEventKind::Down(MouseButton::Left), divider_row, 4), + CellSize::new(ROWS, COLS), + ); + // …and LOCAL's gesture must still be armed and still work. s.dispatch_mouse( FrontendId::LOCAL, @@ -1827,7 +1856,7 @@ fn acc30c_an_armed_drag_does_not_swallow_another_frontends_mouse_events() { assert_eq!( fixed_rows_of(&s, panel), Some(armed_rows.expect("armed rows") - 2), - "the peer's event did not cancel LOCAL's in-flight drag" + "the peer's events did not cancel or steal LOCAL's in-flight drag" ); } @@ -1981,8 +2010,14 @@ fn acc32_terminal_panel_height_change_is_a_viewport_change() { let mut s = editor(); exec( &s, + // `printf '...\\r\\n'`, not `echo`: a PTY in the default mode + // does not translate LF to CRLF for us, so LF-only output + // staircases rightward and every row past the viewport width + // clips to blanks — which would make the anchor assertions below + // compare "" with "" and pass for any regression. "TERM_BUF = pmacs.terminal.open { command = \"/bin/sh\", \ - args = { \"-c\", \"for i in $(seq 1 200); do echo line$i; done; sleep 30\" }, \ + args = { \"-c\", \"i=1; while [ $i -le 200 ]; do printf 'line%d\\\\r\\\\n' $i; \ + i=$((i+1)); done; sleep 30\" }, \ display = \"panel\" }", ); let panel = side_window(&s).expect("terminal panel"); @@ -2001,8 +2036,13 @@ fn acc32_terminal_panel_height_change_is_a_viewport_change() { let before_size = CellSize::new(11, COLS); s.terminal_manager .borrow_mut() - .scroll_view(key_before, before_size, 5); + .scroll_view(key_before, before_size, 30); let top_before = first_visible_row(&s, key_before, before_size); + assert!( + !top_before.is_empty(), + "the anchor row must carry real text, or the equality below \ + cannot fail for the regression it names" + ); exec( &s, &format!( @@ -2029,9 +2069,65 @@ fn acc32_terminal_panel_height_change_is_a_viewport_change() { .at_bottom, "…and a SHRINK cannot re-arm follow" ); + exec(&s, "pmacs.terminal.terminate(TERM_BUF)"); +} - // Growth that reaches the live tail re-arms follow (Q#BP7 case 1), - // and later output then scrolls in. +/// Q#BP7 item 1 proper: **growth reaching the live tail re-arms follow**, +/// so later output scrolls in. +/// +/// `at_bottom` alone cannot pin this — it is the instantaneous geometric +/// readout `scroll_offset == 0`, which a still-anchored view satisfies +/// whenever it happens to be tall enough to reach the tail. The pin has +/// to feed the child MORE output after the growth and assert the view +/// moved with it. +#[test] +fn acc32b_growth_reaching_the_tail_re_arms_follow_and_later_output_scrolls_in() { + let dir = tempfile::tempdir().expect("tempdir"); + let gate = dir.path().join("gate"); + // Inserted bare into the shell word: `tempfile` paths carry no + // spaces or quotes, and wrapping it would terminate the Lua string. + let gate_path = gate.display().to_string(); + let mut s = editor(); + // Two bursts with a filesystem gate between them, so "more output + // after the growth" is deterministic rather than a race. + exec( + &s, + &format!( + "TERM_BUF = pmacs.terminal.open {{ command = \"/bin/sh\", \ + args = {{ \"-c\", \"i=1; while [ $i -le 60 ]; do printf 'first%02d\\\\r\\\\n' $i; \ + i=$((i+1)); done; \ + while [ ! -f {gate_path} ]; do sleep 0.02; done; \ + i=1; while [ $i -le 40 ]; do printf 'second%02d\\\\r\\\\n' $i; \ + i=$((i+1)); done; sleep 30\" }}, \ + display = \"panel\" }}" + ), + ); + let panel = side_window(&s).expect("terminal panel"); + let buffer: pmacs::lua_bindings::BufferIdLua = eval(&s, "return TERM_BUF"); + let key_id = pmacs::terminal::TerminalViewKey::new(FrontendId::LOCAL, panel, buffer.0); + render(&s); + wait_for_terminal_text(&mut s, buffer.0, "first60", Duration::from_secs(10)); + + // Scroll back into history at a short viewport. + let short = CellSize::new(6, COLS); + assert!( + s.terminal_manager + .borrow_mut() + .scroll_view(key_id, short, 20) + ); + let anchored = first_visible_row(&s, key_id, short); + assert!(!anchored.is_empty(), "the anchor row carries real text"); + assert!( + s.terminal_manager + .borrow_mut() + .view_status(key_id) + .expect("status") + .scroll_offset + > 0, + "the view really is anchored in history" + ); + + // Grow the panel until the viewport covers the tail. exec( &s, &format!( @@ -2041,17 +2137,36 @@ fn acc32_terminal_panel_height_change_is_a_viewport_change() { ); render(&s); s.sync_terminal_layout(FrontendId::LOCAL, CellSize::new(ROWS, COLS)); + let grown = CellSize::new(40, COLS); s.terminal_manager .borrow_mut() - .snapshot_for_view(key_before, CellSize::new(22, COLS)) + .snapshot_for_view(key_id, grown) .expect("snapshot at the grown size"); - assert!( - s.terminal_manager - .borrow_mut() - .view_status(key_before) - .expect("view status") - .at_bottom, - "growth reaching the live tail re-arms follow when no selection is frozen" + + // Release the second burst. A view that merely LOOKS at-bottom while + // still anchored gets pushed back into history here; a re-armed one + // follows. + std::fs::write(&gate, b"go").expect("open the gate"); + wait_for_terminal_text(&mut s, buffer.0, "second40", Duration::from_secs(10)); + s.terminal_manager + .borrow_mut() + .snapshot_for_view(key_id, grown) + .expect("snapshot after the second burst"); + + let status = s + .terminal_manager + .borrow_mut() + .view_status(key_id) + .expect("status"); + assert_eq!( + status.scroll_offset, 0, + "the view followed the live tail through the new output" + ); + assert!(status.at_bottom); + assert_ne!( + anchored, + first_visible_row(&s, key_id, grown), + "…and its first visible row moved off the old anchor" ); exec(&s, "pmacs.terminal.terminate(TERM_BUF)"); } @@ -2197,7 +2312,7 @@ fn acc33_growth_with_a_historical_selection_keeps_the_anchor_frozen() { exec( &s, "TERM_BUF = pmacs.terminal.open { command = \"/bin/sh\", \ - args = { \"-c\", \"i=0; while [ $i -lt 60 ]; do printf 'row%02d\\\\n' $i; \ + args = { \"-c\", \"i=0; while [ $i -lt 60 ]; do printf 'row%02d\\\\r\\\\n' $i; \ i=$((i+1)); done; sleep 30\" }, \ display = \"panel\" }", ); @@ -2221,6 +2336,11 @@ fn acc33_growth_with_a_historical_selection_keeps_the_anchor_frozen() { assert!(manager.begin_selection(key_id, view_size, CellCoord::new(0, 0))); } let top_before = first_visible_row(&s, key_id, view_size); + assert!( + !top_before.is_empty(), + "the anchor row must carry real text, or the equality below \ + cannot fail for the regression it names" + ); // Grow the panel enough that following the tail WOULD reach it. exec( @@ -2255,13 +2375,29 @@ fn acc33_growth_with_a_historical_selection_keeps_the_anchor_frozen() { "follow is NOT re-armed while a selection is frozen" ); - // The contrast that makes this bite: clear the selection and the - // same geometry DOES re-arm follow. - s.terminal_manager.borrow_mut().clear_selection(key_id); + // The contrast that makes this bite: the freeze is owed to the + // SELECTION, so clearing it lets the next size declaration re-arm + // follow at the very same geometry. Without this, "no re-arm while + // selected" would also hold if the re-arm simply did not exist. + assert!(s.terminal_manager.borrow_mut().clear_selection(key_id)); s.terminal_manager .borrow_mut() .snapshot_for_view(key_id, grown) .expect("snapshot after clearing"); + let cleared = s + .terminal_manager + .borrow_mut() + .view_status(key_id) + .expect("view status after clearing"); + assert!( + cleared.at_bottom && cleared.scroll_offset == 0, + "clearing the selection re-arms follow at the same geometry" + ); + assert_ne!( + top_before, + first_visible_row(&s, key_id, grown), + "…and the view left the frozen anchor" + ); exec(&s, "pmacs.terminal.terminate(TERM_BUF)"); }