diff --git a/builtin/runtime/desktop.lua b/builtin/runtime/desktop.lua index 78c3003..7a66adc 100644 --- a/builtin/runtime/desktop.lua +++ b/builtin/runtime/desktop.lua @@ -23,9 +23,9 @@ function pmacs.session.desktop_mode(on) return false end enabled = on - if on then - pmacs.session.arm_restore() - end + -- Arm (or, when disabling, unarm) restore-on-startup, so an + -- enable-then-disable in init.lua does not still restore. + pmacs.session.arm_restore(on) return enabled end diff --git a/docs/desktop-save-framing.md b/docs/desktop-save-framing.md index 3e52601..a85302e 100644 --- a/docs/desktop-save-framing.md +++ b/docs/desktop-save-framing.md @@ -135,21 +135,30 @@ switch focus, so restore sequences activation explicitly. to `core.views[LOCAL]` from `core.windows` (not just the startup scratch window — leftover windows would linger in the `pub` map and still take part in edit notifications and buffer-liveness checks, - finding). + finding). 4. Build a fresh `LayoutNode` from `SavedNode` with new `WindowId`s and a `Window` per surviving leaf (weights copied verbatim), install it as `core.views[LOCAL].layout.root`. -5. **Fire `after-load` with the right leaf active**: for each surviving - leaf in preorder, set its window active; the first time a given - buffer is seen, fire `buffer.after-load` (once per newly-loaded - buffer, so saveplace/LSP/syntax attach against the correct active - buffer); then set that window's exact `cursor`/`view_top` from the - leaf. Desktop's per-leaf write lands *after* the hook, so it wins - over saveplace for precision — and same-file-two-leaves keeps - distinct positions a single saveplace entry could not. +5. **Fire `after-load` with the right leaf active, once per leaf**: for + each surviving leaf in preorder, set its window active, fire + `buffer.after-load`, then set that window's exact `cursor`/`view_top`. + Firing **per leaf** (not per buffer) is deliberate — syntax attaches + its overlay to the *active window*, so each pane needs its own fire; + LSP's `attach_buffer` is idempotent, so the same file in two panes + attaches LSP once but syntax to both (finding, round 3). The per-leaf + `cursor`/`view_top` write lands *after* the hook, so desktop wins over + saveplace — and same-file-two-leaves keeps distinct positions a single + saveplace entry could not. 6. Set `active` to the `active_leaf` window (Q#DS10 fallback if that leaf didn't survive). +**Hidden buffers are registry-only in v1** (finding, round 3): a +restored `SavedBuffer` with no leaf (open but not shown) is loaded into +the registry — it is not lost, it is in the buffer list / recentf — but +it does not fire `after-load`, so it attaches syntax on first visit +(`after-switch`) and LSP when next shown. Full initial attach for hidden +buffers is deferred. + Structural construction against the `pub` fields — no new tree-builder API, matching how the window unit tests already assemble layouts. @@ -186,7 +195,9 @@ buffers had unsaved changes when the desktop was saved") via Restore is **armed, never inline in init** — `desktop_mode(true)` runs inside `new()` (before the file opens), so it only sets the flag + -before-quit hook. The Rust startup trigger fires restore: +before-quit hook. Arming is a **boolean** (`arm_restore(on)`), so +`desktop_mode(false)` *unarms* — an enable-then-disable in init does not +still restore (finding, round 3). The Rust startup trigger fires restore: - `editor::run`: capture `let had_file = file.is_some();` **before** the `match file` at `src/editor.rs:1522` consumes `file`. But fire restore @@ -217,13 +228,15 @@ to restore *into* until first attach. v1 targets **only** the local `editor::run` path (single `LOCAL` frontend view built at startup). **`desktop_mode(true)` auto-save and auto-restore are both no-ops in -daemon mode** — not half-enabled. Serializing "wherever a layout exists" -is ambiguous with multiple frontend layouts sharing one key, so the -before-quit save simply doesn't register (or early-returns) when the -process is a daemon; a diagnostic notes desktop-save is local-only in -v1. Daemon + GPU-attach save/restore is **deferred** to the first-attach -design. (Manual `desktop-save`/`desktop-restore` commands likewise -refuse in daemon mode in v1.) +daemon mode** — not half-enabled. The **enforcement is in Rust, not just +Lua** (finding, round 3): `save_session`/`restore_session` early-return +when the `DaemonMode` app-data marker is present. That marker is set +right after the daemon's `EditorState::new()`, so it holds for every +save/restore that can run after startup — the before-quit hook, manual +commands, and direct binding calls — even though `init.lua` (where +`desktop_mode` runs) executes before it is set, when `is_daemon()` in +Lua would still read false. Daemon + GPU-attach save/restore is +**deferred** to the first-attach design. ### Q#DS10 — Active-focus fallback diff --git a/src/desktop.rs b/src/desktop.rs index 4277854..b871d2b 100644 --- a/src/desktop.rs +++ b/src/desktop.rs @@ -15,7 +15,7 @@ //! //! Framing: docs/desktop-save-framing.md. -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::path::Path; use mlua::Lua; @@ -243,6 +243,18 @@ pub fn resolve_active_leaf( // SharedCore / StateDir / LocalInstanceInfo app-data). // --------------------------------------------------------------------------- +/// True when this process is a daemon (multi-frontend). Desktop +/// save/restore is local-only in v1 (Q#DS9); this is the **reliable** +/// enforcement — the `DaemonMode` marker is set right after the daemon's +/// `EditorState::new()`, so it is present for every save/restore that +/// can run after startup (the before-quit hook, manual commands, direct +/// binding calls), even though `init.lua` runs before it is set. Both +/// `save_session` and `restore_session` no-op when it holds. +fn is_daemon(lua: &Lua) -> bool { + lua.app_data_ref::() + .is_some() +} + /// The desktop session key for this process (`cwd.` in local mode). fn session_key_from_lua(lua: &Lua) -> String { match lua @@ -313,6 +325,9 @@ pub fn snapshot(core: &EditorCore, session_key: String) -> Option /// # Errors /// A state-write / serialization failure (surfaced for manual save). pub fn save_session(lua: &Lua) -> Result { + if is_daemon(lua) { + return Ok(false); // local-only in v1 (Q#DS9) + } let Some(base) = lua.app_data_ref::().map(|d| d.0.clone()) else { return Ok(false); }; @@ -340,6 +355,9 @@ pub fn save_session(lua: &Lua) -> Result { /// Parse / state-read failures; a missing individual file collapses its /// leaf rather than failing the whole restore. pub fn restore_session(lua: &Lua) -> Result<(), String> { + if is_daemon(lua) { + return Ok(()); // local-only in v1 (Q#DS9) + } let key = session_key_from_lua(lua); let Some(base) = lua.app_data_ref::().map(|d| d.0.clone()) else { return Ok(()); @@ -368,8 +386,6 @@ pub fn restore_session(lua: &Lua) -> Result<(), String> { /// activate-then-fire pass (Q#DS3). struct RestoreLeaf { window: WindowId, - buffer: BufferId, - newly_loaded: bool, cursor: u64, view_top: usize, } @@ -436,16 +452,22 @@ pub fn restore_into( active }; - // (5) activate-then-fire: after-load must observe the restored leaf - // as active (saveplace/recentf/syntax/LSP read active state). Fire - // once per newly-loaded buffer, then write the exact per-leaf - // cursor/view_top so desktop wins over saveplace. - let mut fired: HashSet = HashSet::new(); + // (5) activate-then-fire, once **per leaf** (per window). after-load + // must observe the restored leaf as active (saveplace/recentf/syntax/ + // LSP read active state). Firing per leaf — not per buffer — gives + // each pane its own per-window overlay (syntax attaches to the active + // window), while LSP's `attach_buffer` is idempotent, so the same + // file in two panes still attaches LSP once but syntax to both. + // Writing the exact per-leaf cursor/view_top *after* the hook lets + // desktop win over saveplace. + // + // NOTE (Q#DS3, hidden buffers): a restored buffer with NO leaf (open + // but hidden) is loaded into the registry but does not fire + // after-load here — it attaches syntax on first visit (after-switch) + // and LSP when it is next shown/opened. Registry-only in v1. for leaf in &leaves { - if leaf.newly_loaded && fired.insert(leaf.buffer) { - core.borrow_mut().set_active_window_id(leaf.window); - fire_after_load(); - } + core.borrow_mut().set_active_window_id(leaf.window); + fire_after_load(); let mut c = core.borrow_mut(); if let Some(win) = c.windows.get_mut(&leaf.window) { win.cursor = leaf.cursor; @@ -469,7 +491,7 @@ fn build_restore_node( ) -> Option { match node { SavedNode::Leaf(leaf) => { - let Some(&(buffer_id, newly_loaded)) = opened.get(&leaf.path) else { + let Some(&(buffer_id, _newly)) = opened.get(&leaf.path) else { save_slots.push(None); // file missing → leaf collapses return None; }; @@ -491,8 +513,6 @@ fn build_restore_node( core.windows.insert(wid, win); leaves.push(RestoreLeaf { window: wid, - buffer: buffer_id, - newly_loaded, cursor, view_top, }); diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 35f73ad..54ab0d3 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -1991,11 +1991,17 @@ pub(crate) fn fire_after_load_hook(lua: &Lua) { fn install_session_module(lua: &Lua) -> mlua::Result { let m = lua.create_table()?; - // arm_restore(): mark that a desktop restore should run at startup. + // arm_restore(on): arm (or, with `false`, unarm) restore-on-startup. + // A boolean app-data path so `desktop_mode(false)` can undo a prior + // `desktop_mode(true)` — the marker is not one-way. m.set( "arm_restore", - lua.create_function(|lua, ()| { - lua.set_app_data(DesktopRestoreArmed); + lua.create_function(|lua, on: Option| { + if on.unwrap_or(true) { + lua.set_app_data(DesktopRestoreArmed); + } else { + lua.remove_app_data::(); + } Ok(()) })?, )?; diff --git a/tests/desktop_acceptance.rs b/tests/desktop_acceptance.rs index 4f7db40..61cab86 100644 --- a/tests/desktop_acceptance.rs +++ b/tests/desktop_acceptance.rs @@ -220,7 +220,7 @@ fn after_load_fires_with_the_restored_leaf_active() { } #[test] -fn same_file_in_two_panes_keeps_distinct_positions() { +fn same_file_in_two_panes_keeps_distinct_positions_and_fires_per_pane() { let dir = fresh_state_dir(); let src = editor(&dir); let a = write_file(&dir, "a.txt", "aaaa\nbbbb\ncccc\ndddd\n"); @@ -236,7 +236,19 @@ fn same_file_in_two_panes_keeps_distinct_positions() { assert!(save(&src)); let mut dst = editor(&dir); + // after-load must fire once PER PANE (not once per buffer) so each + // window gets its own per-window overlay (syntax attaches to the + // active window; LSP attach is idempotent). + exec( + &dst, + "_G.fires = 0; pmacs.hook.add('buffer.after-load', function() _G.fires = _G.fires + 1 end)", + ); restore(&mut dst); + let fires: i64 = dst.lua_host.lua().load("return _G.fires").eval().unwrap(); + assert_eq!( + fires, 2, + "after-load fires once per pane for the same buffer" + ); let ls = leaves(&dst); assert_eq!(ls.len(), 2); assert_eq!(ls[0], (a.clone(), 2, 0)); @@ -244,6 +256,54 @@ fn same_file_in_two_panes_keeps_distinct_positions() { std::fs::remove_dir_all(&dir).ok(); } +#[test] +fn daemon_mode_disables_save_and_restore() { + let dir = fresh_state_dir(); + // Seed a desktop from a normal (non-daemon) editor. + let src = editor(&dir); + let a = write_file(&dir, "a.txt", "aaaa\n"); + exec(&src, &format!("pmacs.buffer.find_or_open({a:?})")); + assert!(save(&src)); + + // A daemon editor must not save or restore (local-only, Q#DS9) — + // the Rust gate holds regardless of what init did. + let mut daemon = editor(&dir); + daemon + .lua_host + .lua() + .set_app_data(pmacs::lua_bindings::DaemonMode); + assert!(!save(&daemon), "daemon save is a no-op"); + restore(&mut daemon); + assert!( + leaves(&daemon).iter().all(|(p, _, _)| p != &a), + "daemon restore is a no-op" + ); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn disabling_desktop_mode_unarms_restore() { + let dir = fresh_state_dir(); + let src = editor(&dir); + let a = write_file(&dir, "a.txt", "aaaa\n"); + exec(&src, &format!("pmacs.buffer.find_or_open({a:?})")); + assert!(save(&src)); + + // Enable then disable desktop_mode → the startup restore must NOT + // fire (arming is a boolean; disable unarms). + let mut dst = editor(&dir); + exec( + &dst, + "pmacs.session.desktop_mode(true); pmacs.session.desktop_mode(false)", + ); + dst.restore_desktop_if_armed(false); + assert!( + leaves(&dst).iter().all(|(p, _, _)| p != &a), + "disabled desktop_mode leaves restore unarmed" + ); + std::fs::remove_dir_all(&dir).ok(); +} + #[test] fn missing_file_collapses_and_focus_falls_back() { let dir = fresh_state_dir();