diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 2c00731..2e894d2 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -732,6 +732,17 @@ end) -- Start a run in `slot`. Shared by compile and shell-command; grep -- has its own worker path. +-- Whether `buf` is currently the acting frontend's side-window buffer +-- (bottom-panel arc). Used so a recompile re-displays into the panel it +-- is already in rather than duplicating itself into the document window. +local function already_in_panel(buf) + if not buf then return false end + local panel = pmacs.window.panel() + if not panel then return false end + local ok, shown = pcall(pmacs.window.buffer, panel) + return ok and shown == buf +end + local function start_run(slot, cmdline, opts) opts = opts or {} -- Bottom-panel arc (Q#BP11b): validate placement BEFORE the run @@ -818,9 +829,14 @@ local function start_run(slot, cmdline, opts) -- The FIRST display of this run is the side-affine one (Q#BP3): a -- persistent *compilation* already visible in a document window must -- not preempt the requested panel. Compile output is passive, so it - -- takes `select = false` explicitly; a recompile simply reuses the - -- panel it is already in. - if display == "panel" then + -- 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. + if display == "panel" or 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/builtin/runtime/listview.lua b/builtin/runtime/listview.lua index 6081587..6a6d717 100644 --- a/builtin/runtime/listview.lua +++ b/builtin/runtime/listview.lua @@ -136,10 +136,8 @@ function pmacs.listview.open(spec) tostring(display))) end if display == "panel" then - p.side = true pmacs.window.display(p.buffer, { side = "bottom", select = true }) else - p.side = false pmacs.window.switch_buffer(p.buffer) end seat_cursor(p, 1) diff --git a/src/daemon.rs b/src/daemon.rs index 44d4b61..5af71d0 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -4190,4 +4190,133 @@ mod tests { "key must self-insert into the displayed buffer, not the attach-time scratch" ); } + + /// Bottom-panel arc, §1.3 #22 (framing acceptance 51's Stage-1 half). + /// + /// A fresh no-target attach clones `LOCAL`'s **primary document** + /// buffer, not `local_view.active`. Stage 1 makes a TUI panel a real + /// focus target, so `LOCAL` can legitimately own focus in a panel at + /// attach time — and panel content must never become a newly attached + /// frontend's full-window document. + #[test] + fn fresh_attach_inherits_locals_document_buffer_not_its_focused_panel() { + let mut editor = EditorState::new(); + let document_buffer = editor.core.borrow().active_buffer_id(); + let panel_buffer = editor.core.borrow().registry.borrow_mut().create("*panel*"); + // Open a bottom panel on LOCAL and focus it. + let panel = { + let mut core = editor.core.borrow_mut(); + let mut request = crate::editor_core::DisplayRequest::new(panel_buffer); + request.side = Some(crate::window::Side::Bottom); + request.height = Some(5); + request.select = Some(true); + let outcome = core + .display_buffer(FrontendId::LOCAL, &request) + .expect("panel placement"); + core.focus_window(FrontendId::LOCAL, outcome.target); + outcome.target + }; + assert_eq!( + editor.core.borrow().views[&FrontendId::LOCAL].active, + panel, + "LOCAL really is focused in the panel" + ); + + let fid = FrontendId(123); + let view = build_fresh_frontend_view(&mut editor, false, false); + editor.core.borrow_mut().register_frontend_view(fid, view); + + assert_eq!( + editor + .core + .borrow() + .active_window_for(fid) + .expect("fresh view window") + .buffer_id, + document_buffer, + "the new frontend inherited LOCAL's DOCUMENT buffer; inheriting \ + `local_view.active` would have made the panel its document" + ); + assert_ne!(document_buffer, panel_buffer); + } + + /// Bottom-panel arc, Q#BP11b / R4-B4 (framing acceptance 55's + /// Stage-1 half). + /// + /// Stage 1 lets a startup hook create and select a side window. The + /// initial-target bootstrap must still reassert the requested buffer + /// in — and activate — a **non-side** document window, rather than + /// overwriting the panel merely because it became `view.active`. + #[test] + fn initial_target_reasserts_a_document_window_when_a_hook_selects_a_panel() { + use std::os::unix::ffi::OsStrExt as _; + + let dir = tempfile::tempdir().expect("tempdir"); + let target = dir.path().join("target.txt"); + std::fs::write(&target, b"target contents\n").expect("write target"); + + let mut editor = EditorState::new(); + editor + .lua_host + .lua() + .load( + r#" + pmacs.lsp.config = {} + pmacs.hook.add("buffer.after-load", function() + if HOOK_RAN then return end + HOOK_RAN = true + HOOK_PANEL = pmacs.window.display( + pmacs.buffer.create("*hook-panel*"), + { side = "bottom", height = 4, select = true }) + end) + "#, + ) + .exec() + .expect("install hook"); + + // A GRID session (panel-capable), which is the realistic shape + // for a hook-created panel in Stage 1 — and real geometry, so + // the panel is genuinely VISIBLE and focused when the reassert + // runs. Without the declaration, reconciliation would hide the + // panel and move focus out on its own, and the assertions below + // would pass without exercising the reassert at all. + let fid = FrontendId(124); + let view = build_fresh_frontend_view(&mut editor, true, true); + editor.core.borrow_mut().register_frontend_view(fid, view); + editor.sync_frame_geometry(fid, CellSize::new(24, 80)); + + let opened = open_initial_target( + &mut editor, + fid, + InitialTarget { + path: target.as_os_str().as_bytes().to_vec(), + cwd: dir.path().as_os_str().as_bytes().to_vec(), + }, + ) + .expect("bootstrap succeeds despite the panel-creating hook"); + + let core = editor.core.borrow(); + assert!( + !core.views[&fid].panel_hidden, + "the hook's panel is visible, so focus really was on it when \ + the reassert ran" + ); + let active = core.views[&fid].active; + let active_window = core.windows.get(&active).expect("active window live"); + assert!( + !active_window.is_side(), + "bootstrap activated a DOCUMENT window, not the hook's panel" + ); + assert_eq!( + active_window.buffer_id, opened.buffer_id, + "…showing the requested target" + ); + let panel = core + .side_window_for(fid) + .expect("the hook's panel survived"); + assert_ne!( + core.windows[&panel].buffer_id, opened.buffer_id, + "the panel was not overwritten with the target" + ); + } } diff --git a/src/editor.rs b/src/editor.rs index 2d2a5e8..246a575 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1925,7 +1925,15 @@ impl EditorState { // pointer for the whole gesture, INCLUDING rows outside any // window — otherwise tracking would stop the moment the pointer // crossed the frame's status row. - if self.window_drag.is_some() { + // + // Scoped to the ARMING frontend. The daemon routes every attached + // 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) + { match ev.kind { MouseEventKind::Drag(MouseButton::Left) => { self.drag_window_boundary(frontend_id, cell_row, term_size); @@ -3860,9 +3868,6 @@ fn mode_line_grapheme_width(graphemes: &[ModeLineGrapheme]) -> u32 { graphemes.iter().map(|grapheme| grapheme.width).sum() } -/// Paint complete graphemes at a logical signed origin. A grapheme that -/// straddles either clip edge is omitted wholesale, so a wide glyph can never -/// leave a dangling half-cell at a window or left/right collision boundary. /// Restyle one exposed segment of a horizontal split boundary and stamp /// its grip (Q#BP5a). /// @@ -3890,6 +3895,9 @@ fn paint_divider_segment( cell.glyph = crate::cell::Glyph::Char(DIVIDER_HANDLE_GLYPH); } +/// Paint complete graphemes at a logical signed origin. A grapheme that +/// straddles either clip edge is omitted wholesale, so a wide glyph can never +/// leave a dangling half-cell at a window or left/right collision boundary. fn paint_mode_line_graphemes( grid: &mut crate::cell::CellGrid<'_>, rect: &crate::window::Rect, diff --git a/src/editor_core.rs b/src/editor_core.rs index 039ee43..cfb2bcb 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -1073,7 +1073,12 @@ impl EditorCore { .is_some_and(|window| window.buffer_id == entry.buffer_id) && !self.side_window_is_hidden(fid, entry.window_id); if origin_valid { - self.set_active_window_id(entry.window_id); + // Through `focus_window`, not `set_active_window_id`: + // returning INTO a panel from a document window is a + // focus transition like any other, so it refreshes + // `origin_document` and a later `window.quit` returns to + // the window the jump came from. + self.focus_window(fid, entry.window_id); } else { // A stale SIDE origin is skipped outright: switching a // panel's buffer into the document window is exactly the @@ -2667,6 +2672,12 @@ impl EditorCore { /// Focus an explicit window in the acting frontend, refreshing the /// panel's remembered document origin on the way (Q#BP2c). + /// + /// **The caller must have validated `target`** — that it is live and + /// belongs to `fid`'s layout. Every Lua path does so through + /// `lookup_window` or the display transaction's own revalidation; + /// this function only `debug_assert!`s it, so a release-mode caller + /// passing a foreign or dead id would leave `view.active` dangling. pub fn focus_window(&mut self, fid: FrontendId, target: WindowId) { let Some(view) = self.views.get_mut(&fid) else { return; diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 3d8a4e9..959edbe 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -12218,14 +12218,20 @@ fn install_window_module(lua: &Lua, core: &SharedCore) -> mlua::Result { window_panel::install(lua, core, &win)?; { + // Bottom-panel arc (Q#BP6): `try_split_active` refuses a side + // window. This binding is what `C-x 2` reaches, so the refusal + // has to live on THIS path — splitting the panel leaf would make + // the root wrapper's final child a split rather than + // `Leaf(side)`, and both `Layout::compute`'s fixed pass and + // `document_subtree` key on exactly that shape. let cc = core.clone(); win.set( "split_horizontal", lua.create_function(move |_, ()| { - let new_id = cc - .borrow_mut() - .split_active(crate::window::Orientation::Horizontal, true); - Ok(new_id.raw()) + cc.borrow_mut() + .try_split_active(crate::window::Orientation::Horizontal, true) + .map(crate::window::WindowId::raw) + .map_err(mlua::Error::runtime) })?, )?; } @@ -12235,10 +12241,10 @@ fn install_window_module(lua: &Lua, core: &SharedCore) -> mlua::Result
{ win.set( "split_vertical", lua.create_function(move |_, ()| { - let new_id = cc - .borrow_mut() - .split_active(crate::window::Orientation::Vertical, true); - Ok(new_id.raw()) + cc.borrow_mut() + .try_split_active(crate::window::Orientation::Vertical, true) + .map(crate::window::WindowId::raw) + .map_err(mlua::Error::runtime) })?, )?; } @@ -12351,9 +12357,38 @@ fn install_window_module(lua: &Lua, core: &SharedCore) -> mlua::Result
{ { let cc = core.clone(); + // With no argument: the selected window's buffer (unchanged). + // With an explicit window id: that window's buffer, validated + // against the acting frontend's layout like every other + // `WindowId`-taking operation (bottom-panel arc, Q#BP11) — an + // adopter has to be able to ask "is my buffer the one in the + // panel" without first selecting the panel. win.set( "buffer", - lua.create_function(move |_, ()| Ok(BufferIdLua(cc.borrow().active_buffer_id())))?, + 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")) + })?, )?; } diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index d0ee6c0..068f6c8 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -184,6 +184,19 @@ fn parse_request( Ok(request) } +/// The ACTING frontend's selected window. +/// +/// 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 { + core.borrow() + .views + .get(&fid) + .map(|view| view.active) + .ok_or_else(|| mlua::Error::runtime("pmacs.window: acting frontend has no layout")) +} + /// 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 { @@ -484,7 +497,7 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result let fid = acting_frontend(lua, &cc); let id = match target { Some(raw) => lookup_window(&cc, fid, raw)?, - None => cc.borrow().active_window_id(), + None => selected_window(&cc, fid)?, }; let core = cc.borrow(); let window = core @@ -577,7 +590,7 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result let fid = acting_frontend(lua, &cc); let id = match target { Some(raw) => lookup_window(&cc, fid, raw)?, - None => cc.borrow().active_window_id(), + None => selected_window(&cc, fid)?, }; let area_rows = cc.borrow().frontend_area_rows(fid).ok_or_else(|| { mlua::Error::runtime( diff --git a/src/window.rs b/src/window.rs index f38cf53..b66499e 100644 --- a/src/window.rs +++ b/src/window.rs @@ -979,14 +979,18 @@ fn compute_node( rows } else { let w = weights.get(i).copied().unwrap_or(1).max(1); + // u64 intermediates: `remainder * w` is the only + // place this arithmetic could overflow a u32, and a + // saturating fallback there would hand a non-last + // child the whole remainder and underflow the last + // one. Widening deletes the case outright. let e = if Some(i) == last_flexible { remainder - flexible_used + } else if total == 0 { + 0 } else { - remainder - .checked_mul(w) + u32::try_from(u64::from(remainder) * u64::from(w) / u64::from(total)) .unwrap_or(remainder) - .checked_div(total) - .unwrap_or(0) }; flexible_used += e; e diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index b89ca15..73b3986 100644 --- a/tests/bottom_panel_stage1_acceptance.rs +++ b/tests/bottom_panel_stage1_acceptance.rs @@ -1300,6 +1300,55 @@ fn acc19_adopters_place_side_affinely_through_real_entry_points() { assert_ne!(panel, document); } +/// A recompile carries no `display` (only cmdline/cwd are stored), so +/// the raw switch would put `*compilation*` in the selected DOCUMENT +/// window while the panel still shows it — the duplicate presentation +/// this arc removes elsewhere. +#[test] +fn acc19b_recompile_reuses_the_panel_instead_of_duplicating_into_the_document() { + let s = editor(); + exec(&s, "pmacs.window.split_horizontal()"); + exec(&s, "pmacs.compile.run(\"true\", { display = \"panel\" })"); + let panel = side_window(&s).expect("compile opened a panel"); + let compilation = s.core.borrow().windows[&panel].buffer_id; + + // Focus a document window, then recompile — which reaches + // `start_run` with no `display` at all. + let document = s + .core + .borrow() + .non_side_target(FrontendId::LOCAL) + .expect("document"); + s.core + .borrow_mut() + .focus_window(FrontendId::LOCAL, document); + let document_buffer = s.core.borrow().windows[&document].buffer_id; + exec(&s, "pmacs.command.invoke(\"compile.recompile\")"); + + assert_eq!( + s.core.borrow().windows[&panel].buffer_id, + compilation, + "the recompile stayed in the panel" + ); + assert_eq!( + s.core.borrow().windows[&document].buffer_id, + document_buffer, + "…and did not duplicate itself into the document window" + ); + + // A compilation that is NOT in a panel keeps the pre-arc raw switch. + let s = editor(); + exec(&s, "pmacs.compile.run(\"true\")"); + assert!(side_window(&s).is_none()); + let target = active_window(&s); + exec(&s, "pmacs.command.invoke(\"compile.recompile\")"); + assert_eq!(active_window(&s), target); + assert!( + side_window(&s).is_none(), + "no panel is created out of nowhere" + ); +} + // --------------------------------------------------------------------------- // 20 / 23 — quit: delete, restore chains, revalidation, and the cap // --------------------------------------------------------------------------- @@ -1549,27 +1598,30 @@ fn acc26_close_others_and_split_respect_the_side_window() { let s = editor(); exec(&s, "pmacs.window.split_horizontal()"); let panel = open_panel(&s, "*panel*", 5); - // From a side window both are pointed errors. + // From a side window both are pointed errors — asserted through the + // REAL Lua bindings, which is what `C-x 1` / `C-x 2` / `C-x 3` + // reach. A direct `core.try_split_active(..)` call would pass even + // with the guard unwired, which is exactly how an unwired guard + // survives review. exec(&s, "pmacs.window.focus_next()"); while active_window(&s) != panel { exec(&s, "pmacs.window.focus_next()"); } - assert!(s.core.borrow_mut().close_others().is_err()); - assert!( - s.core - .borrow_mut() - .try_split_active(Orientation::Horizontal, true) - .is_err() - ); + let before = structure(&layout_root(&s)); + assert!(try_exec(&s, "pmacs.window.close_others()").is_err()); + assert!(try_exec(&s, "pmacs.window.split_horizontal()").is_err()); + assert!(try_exec(&s, "pmacs.window.split_vertical()").is_err()); assert!(side_window(&s).is_some(), "nothing was mutated"); + assert_eq!( + before, + structure(&layout_root(&s)), + "the wrapper's final child is still Leaf(side)" + ); // From a document window, close_others deletes the panel too. exec(&s, "pmacs.window.focus_next()"); assert_ne!(active_window(&s), panel); - s.core - .borrow_mut() - .close_others() - .expect("document may close others"); + exec(&s, "pmacs.window.close_others()"); assert!(side_window(&s).is_none()); assert_eq!( s.core.borrow().views[&FrontendId::LOCAL] @@ -1728,6 +1780,57 @@ fn acc30_divider_drag_writes_fixed_rows_and_weights_and_creates_no_selection() { assert!(doubled > after, "the ratio scales with the frame"); } +/// An armed drag owns the pointer for its OWN frontend only. The daemon +/// routes every attached grid frontend through one `dispatch_mouse`, so +/// an unscoped guard would let one frontend's in-flight gesture cancel +/// and swallow another frontend's clicks. +#[test] +fn acc30c_an_armed_drag_does_not_swallow_another_frontends_mouse_events() { + let mut s = editor(); + let panel = open_panel(&s, "*panel*", 6); + let document = s + .core + .borrow() + .non_side_target(FrontendId::LOCAL) + .expect("document"); + let other = FrontendId(30); + let other_window = attach_frontend(&s, other, true); + + let rects = render(&s); + let divider_row = u16::try_from(rects[&document].origin.row + rects[&document].size.rows - 1) + .expect("row fits"); + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Down(MouseButton::Left), divider_row, 3), + CellSize::new(ROWS, COLS), + ); + let armed_rows = fixed_rows_of(&s, panel); + + // A click from the OTHER frontend must be dispatched normally… + s.dispatch_mouse( + other, + mouse(MouseEventKind::Down(MouseButton::Left), 1, 2), + CellSize::new(ROWS, COLS), + ); + assert_eq!( + s.core.borrow().views[&other].active, + other_window, + "the peer's click reached its own window instead of being swallowed" + ); + + // …and LOCAL's gesture must still be armed and still work. + s.dispatch_mouse( + FrontendId::LOCAL, + mouse(MouseEventKind::Drag(MouseButton::Left), divider_row + 2, 3), + CellSize::new(ROWS, COLS), + ); + 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" + ); +} + #[test] fn acc30b_ui_divider_face_resolves_and_paints_every_exposed_segment() { let s = editor();