neuwirth
/
pmacs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(persistence): reliable daemon gate, unarm, per-pane after-load 

Addresses the PR #99 review:

- HIGH daemon local-only was not reliable: run_daemon sets DaemonMode
  only after EditorState::new() has run init.lua, so desktop_mode(true)
  in init saw is_daemon()==false and the raw bindings were ungated. Now
  save_session/restore_session early-return in Rust when the DaemonMode
  marker is present — set right after the daemon's new(), so it holds for
  every save/restore that can run after startup (before-quit hook, manual
  commands, direct binding calls).

- MEDIUM desktop_mode(false) could not unarm startup restore: arm_restore
  is now a boolean (arm_restore(on)) that sets/removes the marker, and
  desktop_mode(on) calls arm_restore(on). enable-then-disable no longer
  restores.

- MEDIUM/LOW same-file multi-pane missed per-window overlays: restore now
  fires buffer.after-load once PER LEAF (per window), not once per buffer.
  Syntax attaches its overlay to the active window, so each pane gets its
  own; LSP attach_buffer is idempotent, so the same file in two panes
  attaches LSP once but syntax to both.

- MEDIUM hidden restored buffers: documented as registry-only in v1 (they
  are live/openable/in recentf, but do not fire after-load, so they
  attach syntax on first visit via after-switch and LSP when next shown).
  Full initial attach for hidden buffers is deferred. Noted in the
  framing + a code comment.

- LOW trailing whitespace in docs/desktop-save-framing.md.

Tests (desktop_acceptance now 11): same_file_..._fires_per_pane asserts
after-load fires twice for two panes of one file; daemon_mode_disables_
save_and_restore; disabling_desktop_mode_unarms_restore.

Gates: fmt + workspace clippy clean; lib 1487; crdt 1658; desktop 11;
persistence 5; m4 90; GPU 58; git diff --check clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Levi Neuwirth 2026-07-08 22:27:24 -04:00
parent 3607df0afe
commit e56c3055f2
5 changed files with 138 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
builtin/runtime/desktop.lua
View File

@ -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

47
docs/desktop-save-framing.md
View File

@ -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

50
src/desktop.rs
View File

@ -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::<crate::lua_bindings::DaemonMode>()
.is_some()
}
/// The desktop session key for this process (`cwd.<hash>` 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<SavedDesktop>
/// # Errors
/// A state-write / serialization failure (surfaced for manual save).
pub fn save_session(lua: &Lua) -> Result<bool, String> {
if is_daemon(lua) {
return Ok(false); // local-only in v1 (Q#DS9)
}
let Some(base) = lua.app_data_ref::<StateDir>().map(|d| d.0.clone()) else {
return Ok(false);
};
@ -340,6 +355,9 @@ pub fn save_session(lua: &Lua) -> Result<bool, String> {
/// 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::<StateDir>().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<BufferId> = 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<LayoutNode> {
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,
});

12
src/lua_bindings/mod.rs
View File

@ -1991,11 +1991,17 @@ pub(crate) fn fire_after_load_hook(lua: &Lua) {
fn install_session_module(lua: &Lua) -> mlua::Result<Table> {
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<bool>| {
if on.unwrap_or(true) {
lua.set_app_data(DesktopRestoreArmed);
} else {
lua.remove_app_data::<DesktopRestoreArmed>();
}
Ok(())
})?,
)?;

62
tests/desktop_acceptance.rs
View File

@ -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();