fix(panels): follow active buffer on semantic frontends; re-attach overlays on switch
Two PR #94 validation findings. 1. (High, blocking) GPU stuck after leaving a panel: the GPU only swaps its displayed buffer on BufferSnapshot, and the daemon only sent one on the first CRDT upgrade (F29's ensure returns None for an already-backed buffer). A panel's q / RET switched the daemon's active buffer back to the already-known source and sent nothing -- the GPU kept rendering the panel while input targeted the source: a typing-into-a-buffer-you-can't-see hazard. Fix: the per-tick loop now FOLLOWS each replica frontend's own active buffer -- when it differs from the last snapshot sent to that frontend, ship that buffer's snapshot to that frontend only (the F29 broadcast records itself so the upgrade tick doesn't double-send). First-tick send also repairs the attach-time last-snapshot-wins ambiguity. Snapshot export extracted and shared with the F29 broadcast; per-fid state cleaned on both detach paths. 2. (High, wider than reported) 'LSP doesn't activate on navigate': switch_active_buffer clears the window's overlays, and the runtime dedup tables (highlighted_buffers, styled_buffers, diag_viewed_buffers) blocked re-attachment -- so EVERY buffer switch (plain C-x b included, long-latent) permanently stripped syntax color, LSP semantic style, and diagnostic underlines; verified: overlay kinds [syntax-highlight, lsp-style, diagnostic] -> [] after one away-and-back. Fix: a new additive buffer.after-switch hook, fired by the window.switch_buffer binding and find_or_open's existing-buffer branch; syntax.lua and lsp.lua subscribe and re-push their views (the just-cleared window makes that exactly-once per switch; fresh loads keep firing after-load). Regression: tests/overlay_reattach_acceptance.rs (double round-trip counts exactly one highlight overlay; panel q restores styling). The daemon follow path is validated live (daemon + GPU) -- its unit seam is the shared export helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
b25b47334d
commit
5c5caa9482
|
|
@ -44,6 +44,16 @@ define {
|
|||
kind = "all-must-succeed",
|
||||
}
|
||||
|
||||
define {
|
||||
name = "buffer.after-switch",
|
||||
description = "Fired after the active window switches to a different, " ..
|
||||
"already-open buffer (C-x b, panel visits, find_or_open of " ..
|
||||
"an open file). Switching clears the window's overlays; " ..
|
||||
"syntax/LSP subscribers re-attach theirs here. Fresh loads " ..
|
||||
"fire buffer.after-load instead.",
|
||||
kind = "all-must-succeed",
|
||||
}
|
||||
|
||||
define {
|
||||
name = "buffer.after-save",
|
||||
description = "Fired after a successful save. LSP did_save and " ..
|
||||
|
|
|
|||
|
|
@ -569,6 +569,26 @@ pmacs.hook.add("buffer.after-load", function()
|
|||
pcall(attach_buffer, pmacs.window.buffer())
|
||||
end)
|
||||
|
||||
pmacs.hook.add("buffer.after-switch", function()
|
||||
-- Arc 1b: switching buffers clears the window's overlays, and
|
||||
-- `attach_buffer` early-returns for a live attachment without
|
||||
-- touching views — so a switch back to an attached buffer must
|
||||
-- re-push the LSP style + diagnostic views itself. The just-
|
||||
-- cleared window makes this exactly-once per switch; the dedup
|
||||
-- tables keep gating the after-load path only. Without this,
|
||||
-- navigating between attached buffers looked like "the LSP
|
||||
-- deactivated" (no semantic color, no underlines).
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return end
|
||||
local key = tostring(buf)
|
||||
local rec = attachments[key]
|
||||
if not rec then return end
|
||||
local ok_s, attached_s = pcall(pmacs.lsp._attach_style, buf)
|
||||
if ok_s and attached_s then styled_buffers[key] = true end
|
||||
local ok_d, attached_d = pcall(pmacs.diag._attach_view, buf, rec.uri)
|
||||
if ok_d and attached_d then diag_viewed_buffers[key] = true end
|
||||
end)
|
||||
|
||||
pmacs.hook.add("buffer.after-edit", function()
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return end
|
||||
|
|
|
|||
|
|
@ -83,6 +83,25 @@ pmacs.hook.add("buffer.after-load", function()
|
|||
end
|
||||
end)
|
||||
|
||||
pmacs.hook.add("buffer.after-switch", function()
|
||||
-- Arc 1b: switching buffers clears the window's overlays
|
||||
-- (`switch_active_buffer` resets window view state), so the
|
||||
-- highlight view must be re-pushed for the now-active buffer.
|
||||
-- Dropping the `highlighted_buffers` entry first lets
|
||||
-- `attach_for_active_buffer` re-attach; the just-cleared window
|
||||
-- makes that exactly-once per switch. Without this, C-x b /
|
||||
-- panel navigation permanently stripped syntax color.
|
||||
local ok, err = pcall(function()
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return end
|
||||
highlighted_buffers[tostring(buf)] = nil
|
||||
attach_for_active_buffer()
|
||||
end)
|
||||
if not ok and pmacs.error then
|
||||
pmacs.error("syntax.after-switch: " .. tostring(err))
|
||||
end
|
||||
end)
|
||||
|
||||
local function reparse_active_buffer_after_edit()
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return end
|
||||
|
|
|
|||
107
src/daemon.rs
107
src/daemon.rs
|
|
@ -842,6 +842,16 @@ fn dispatcher_loop(
|
|||
// attach emits an initial `DispatchIdle` so the frontend starts
|
||||
// from a known idle state (its default is pessimistic-`false`).
|
||||
let mut last_dispatch_idle_sent: HashMap<FrontendId, bool> = HashMap::new();
|
||||
// Arc 1b — the buffer each replica frontend last received a
|
||||
// `BufferSnapshot` for via the active-buffer-follow path. Absence
|
||||
// means "never sent": the first tick after attach ships the
|
||||
// frontend its own active buffer, which also repairs the
|
||||
// attach-time last-snapshot-wins ambiguity (the initial
|
||||
// `send_buffer_snapshots` sweep sends every buffer; the display
|
||||
// follows whichever arrived last, not necessarily the active one).
|
||||
// Declared for both flavors (the follow path is crdt-gated; the
|
||||
// detach cleanup isn't).
|
||||
let mut last_active_buffer_sent: HashMap<FrontendId, crate::buffer::BufferId> = HashMap::new();
|
||||
let mut session_registry = SessionRegistry::new();
|
||||
// T M10.11 Q8 — jitter PRNG, seeded once so the
|
||||
// convergence-under-jitter scenario is deterministically
|
||||
|
|
@ -966,6 +976,31 @@ fn dispatcher_loop(
|
|||
&session_registry,
|
||||
&mut streams,
|
||||
);
|
||||
// The broadcast just delivered this buffer to this
|
||||
// frontend too; record it so the follow check below
|
||||
// doesn't send a duplicate on the same tick.
|
||||
last_active_buffer_sent.insert(*fid, upgraded);
|
||||
}
|
||||
|
||||
// Arc 1b — follow this frontend's active buffer. The
|
||||
// F29 push above only fires on the *upgrade* tick;
|
||||
// switching to an already-CRDT-backed buffer (a
|
||||
// panel's `q`, `find_or_open` of an open file, plain
|
||||
// `C-x b`) previously sent nothing, so a semantic
|
||||
// frontend kept rendering the old buffer while
|
||||
// daemon-side input targeted the new one — a
|
||||
// typing-into-a-buffer-you-can't-see hazard. Ship the
|
||||
// now-active buffer's snapshot to THIS frontend only
|
||||
// (its own view changed; nobody else's did).
|
||||
let active_now = {
|
||||
let core = editor.core.borrow();
|
||||
core.active_window_for(*fid).map(|w| w.buffer_id)
|
||||
};
|
||||
if let Some(active_now) = active_now
|
||||
&& last_active_buffer_sent.get(fid) != Some(&active_now)
|
||||
{
|
||||
send_buffer_snapshot_to_frontend(editor, active_now, *fid, &mut streams);
|
||||
last_active_buffer_sent.insert(*fid, active_now);
|
||||
}
|
||||
}
|
||||
#[cfg(not(feature = "crdt"))]
|
||||
|
|
@ -1181,6 +1216,7 @@ fn dispatcher_loop(
|
|||
semantic_states.remove(fid);
|
||||
term_sizes.remove(fid);
|
||||
last_dispatch_idle_sent.remove(fid);
|
||||
last_active_buffer_sent.remove(fid);
|
||||
session_registry.unregister_session(*fid);
|
||||
editor.core.borrow_mut().unregister_frontend_view(*fid);
|
||||
}
|
||||
|
|
@ -1221,6 +1257,7 @@ fn dispatcher_loop(
|
|||
&mut streams,
|
||||
&mut term_sizes,
|
||||
&mut last_dispatch_idle_sent,
|
||||
&mut last_active_buffer_sent,
|
||||
&mut session_registry,
|
||||
);
|
||||
// Drain a burst of immediately-available events to
|
||||
|
|
@ -1236,6 +1273,7 @@ fn dispatcher_loop(
|
|||
&mut streams,
|
||||
&mut term_sizes,
|
||||
&mut last_dispatch_idle_sent,
|
||||
&mut last_active_buffer_sent,
|
||||
&mut session_registry,
|
||||
);
|
||||
}
|
||||
|
|
@ -1341,6 +1379,7 @@ fn handle_dispatcher_event(
|
|||
streams: &mut HashMap<FrontendId, UnixStream>,
|
||||
term_sizes: &mut HashMap<FrontendId, CellSize>,
|
||||
last_dispatch_idle_sent: &mut HashMap<FrontendId, bool>,
|
||||
last_active_buffer_sent: &mut HashMap<FrontendId, crate::buffer::BufferId>,
|
||||
session_registry: &mut SessionRegistry,
|
||||
) {
|
||||
match event {
|
||||
|
|
@ -1515,6 +1554,7 @@ fn handle_dispatcher_event(
|
|||
streams.remove(&frontend_id);
|
||||
term_sizes.remove(&frontend_id);
|
||||
last_dispatch_idle_sent.remove(&frontend_id);
|
||||
last_active_buffer_sent.remove(&frontend_id);
|
||||
session_registry.unregister_session(frontend_id);
|
||||
editor
|
||||
.core
|
||||
|
|
@ -1691,6 +1731,55 @@ fn ensure_active_buffer_crdt_backed(
|
|||
/// send is small (snapshot bytes for the upgrade-instant state,
|
||||
/// which is the empty / freshly-loaded buffer content the replica
|
||||
/// already has) and only fires on the actual upgrade tick.
|
||||
/// Export `buffer_id`'s CRDT snapshot bytes, or `None` (logged) when
|
||||
/// the buffer is missing, not CRDT-backed, or the export fails.
|
||||
#[cfg(feature = "crdt")]
|
||||
fn export_buffer_snapshot(
|
||||
editor: &EditorState,
|
||||
buffer_id: crate::buffer::BufferId,
|
||||
) -> Option<Vec<u8>> {
|
||||
let core = editor.core.borrow();
|
||||
let registry = core.registry.borrow();
|
||||
let buf = registry.get(buffer_id).ok()?;
|
||||
let crdt = buf.crdt_state()?;
|
||||
match crdt.export_snapshot() {
|
||||
Ok(bytes) => Some(bytes),
|
||||
Err(e) => {
|
||||
eprintln!("pmacs: export_snapshot for {buffer_id:?} failed: {e:?}");
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Arc 1b — send `buffer_id`'s snapshot to ONE frontend. The
|
||||
/// active-buffer-follow path (see the per-tick loop) uses this when a
|
||||
/// semantic frontend's own active buffer changes to an
|
||||
/// already-CRDT-backed buffer: the F29 broadcast only fires on the
|
||||
/// upgrade tick, so without this a frontend that switched *back* to a
|
||||
/// known buffer (a panel's `q`, `find_or_open` of an open file) kept
|
||||
/// displaying the old buffer while daemon-side input targeted the new
|
||||
/// one.
|
||||
#[cfg(feature = "crdt")]
|
||||
fn send_buffer_snapshot_to_frontend(
|
||||
editor: &EditorState,
|
||||
buffer_id: crate::buffer::BufferId,
|
||||
fid: FrontendId,
|
||||
streams: &mut HashMap<FrontendId, UnixStream>,
|
||||
) {
|
||||
let Some(snapshot_bytes) = export_buffer_snapshot(editor, buffer_id) else {
|
||||
return;
|
||||
};
|
||||
let msg = InstanceMessage::BufferSnapshot {
|
||||
buffer_id,
|
||||
crdt_snapshot: snapshot_bytes,
|
||||
};
|
||||
if let Some(stream) = streams.get_mut(&fid)
|
||||
&& let Err(e) = write_message(stream, &msg)
|
||||
{
|
||||
eprintln!("pmacs: send BufferSnapshot for {buffer_id:?} to {fid:?} failed: {e}");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "crdt")]
|
||||
fn broadcast_buffer_snapshot_to_replicas(
|
||||
editor: &EditorState,
|
||||
|
|
@ -1698,22 +1787,8 @@ fn broadcast_buffer_snapshot_to_replicas(
|
|||
session_registry: &SessionRegistry,
|
||||
streams: &mut HashMap<FrontendId, UnixStream>,
|
||||
) {
|
||||
let snapshot_bytes = {
|
||||
let core = editor.core.borrow();
|
||||
let registry = core.registry.borrow();
|
||||
let Ok(buf) = registry.get(buffer_id) else {
|
||||
return;
|
||||
};
|
||||
let Some(crdt) = buf.crdt_state() else {
|
||||
return;
|
||||
};
|
||||
match crdt.export_snapshot() {
|
||||
Ok(bytes) => bytes,
|
||||
Err(e) => {
|
||||
eprintln!("pmacs: F29 export_snapshot for {buffer_id:?} failed: {e:?}");
|
||||
return;
|
||||
}
|
||||
}
|
||||
let Some(snapshot_bytes) = export_buffer_snapshot(editor, buffer_id) else {
|
||||
return;
|
||||
};
|
||||
let msg = InstanceMessage::BufferSnapshot {
|
||||
buffer_id,
|
||||
|
|
|
|||
|
|
@ -2402,6 +2402,11 @@ fn install_buffer_module(lua: &Lua, registry: &SharedRegistry) -> mlua::Result<T
|
|||
.switch_active_buffer(existing)
|
||||
.map_err(mlua::Error::external)?;
|
||||
}
|
||||
// Arc 1b: switching clears the window's overlays;
|
||||
// subscribers (syntax highlight, LSP style/diag
|
||||
// views) re-attach theirs. The fresh-load branch
|
||||
// below fires `buffer.after-load` instead.
|
||||
run_hook_if_defined(lua, "buffer.after-switch", mlua::MultiValue::new());
|
||||
return Ok(BufferIdLua(existing));
|
||||
}
|
||||
let (bytes, meta) = crate::file_io::load_file(&path_buf).map_err(|source| {
|
||||
|
|
@ -10348,10 +10353,17 @@ fn install_window_module(lua: &Lua, core: &SharedCore) -> mlua::Result<Table> {
|
|||
let cc = core.clone();
|
||||
win.set(
|
||||
"switch_buffer",
|
||||
lua.create_function(move |_, id: BufferIdLua| -> mlua::Result<()> {
|
||||
lua.create_function(move |lua, id: BufferIdLua| -> mlua::Result<()> {
|
||||
cc.borrow_mut()
|
||||
.switch_active_buffer(id.0)
|
||||
.map_err(mlua::Error::external)
|
||||
.map_err(mlua::Error::external)?;
|
||||
// Arc 1b: switching clears the window's overlays;
|
||||
// subscribers (syntax highlight, LSP style/diag views)
|
||||
// re-attach theirs here — without this, C-x b / panel
|
||||
// navigation permanently stripped styling from the
|
||||
// session.
|
||||
run_hook_if_defined(lua, "buffer.after-switch", mlua::MultiValue::new());
|
||||
Ok(())
|
||||
})?,
|
||||
)?;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,123 @@
|
|||
//! Overlay re-attach on buffer switch (Arc 1b, PR #94 validation
|
||||
//! finding 2): `switch_active_buffer` clears the window's overlays,
|
||||
//! and the runtime's dedup tables blocked re-attachment — so plain
|
||||
//! `C-x b`, buffer-list visits, and panel navigation permanently
|
||||
//! stripped syntax/LSP styling ("the LSP doesn't activate if I
|
||||
//! navigate to a reference"). The `buffer.after-switch` hook now
|
||||
//! re-pushes the views.
|
||||
//!
|
||||
//! Hermetic: only the tree-sitter `syntax-highlight` overlay is
|
||||
//! asserted (always available for `.rs`); the LSP style/diag views
|
||||
//! ride the same hook but need a live server.
|
||||
|
||||
use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers};
|
||||
use pmacs::editor::EditorState;
|
||||
|
||||
fn open_probe_file(_s: &EditorState) -> String {
|
||||
let dir = std::env::temp_dir().join(format!("pmacs-ovl-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).expect("mk tempdir");
|
||||
let file = dir.join("probe.rs");
|
||||
std::fs::write(&file, "fn main() {}\n").expect("write probe file");
|
||||
file.display().to_string()
|
||||
}
|
||||
|
||||
/// `(overlay kinds on the active window, active buffer name)`.
|
||||
fn kinds(s: &EditorState) -> (Vec<String>, String) {
|
||||
s.lua_host
|
||||
.lua()
|
||||
.load(
|
||||
r"
|
||||
local d = pmacs.describe.buffer(pmacs.window.buffer())
|
||||
return pmacs.window._overlay_kinds(), d.name
|
||||
",
|
||||
)
|
||||
.eval()
|
||||
.expect("probe overlay kinds")
|
||||
}
|
||||
|
||||
fn count_of(kinds: &[String], kind: &str) -> usize {
|
||||
kinds.iter().filter(|k| *k == kind).count()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn switch_away_and_back_reattaches_syntax_overlay_exactly_once() {
|
||||
let s = EditorState::new();
|
||||
let path = open_probe_file(&s);
|
||||
s.lua_host
|
||||
.lua()
|
||||
.load(format!(
|
||||
r#"
|
||||
_G.TARGET = pmacs.buffer.find_or_open("{path}")
|
||||
for _, id in ipairs(pmacs.buffer.list()) do
|
||||
if pmacs.describe.buffer(id).name == "*scratch*" then _G.SCRATCH = id end
|
||||
end
|
||||
"#
|
||||
))
|
||||
.exec()
|
||||
.expect("open probe + find scratch");
|
||||
let (before, _) = kinds(&s);
|
||||
assert_eq!(
|
||||
count_of(&before, "syntax-highlight"),
|
||||
1,
|
||||
"fresh open attaches the highlight overlay once (got {before:?})"
|
||||
);
|
||||
|
||||
// Away and back — twice, so stacking would show as a count > 1.
|
||||
s.lua_host
|
||||
.lua()
|
||||
.load(
|
||||
r"
|
||||
pmacs.window.switch_buffer(_G.SCRATCH)
|
||||
pmacs.window.switch_buffer(_G.TARGET)
|
||||
pmacs.window.switch_buffer(_G.SCRATCH)
|
||||
pmacs.window.switch_buffer(_G.TARGET)
|
||||
",
|
||||
)
|
||||
.exec()
|
||||
.expect("switch away and back twice");
|
||||
let (after, name) = kinds(&s);
|
||||
assert!(name.ends_with("probe.rs"), "back on the probe file");
|
||||
assert_eq!(
|
||||
count_of(&after, "syntax-highlight"),
|
||||
1,
|
||||
"the switch re-attaches exactly one highlight overlay (got {after:?})"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn panel_quit_restores_overlays_on_the_source_buffer() {
|
||||
let s = EditorState::new();
|
||||
let path = open_probe_file(&s);
|
||||
s.lua_host
|
||||
.lua()
|
||||
.load(format!(
|
||||
r#"
|
||||
pmacs.buffer.find_or_open("{path}")
|
||||
pmacs.listview.open {{
|
||||
name = "*ovl-panel*",
|
||||
header = "h",
|
||||
rows = {{ {{ text = "row", item = 1 }} }},
|
||||
}}
|
||||
"#
|
||||
))
|
||||
.exec()
|
||||
.expect("open probe + panel");
|
||||
// q leaves the panel back to the source file.
|
||||
let mut s = s;
|
||||
s.dispatch_key(
|
||||
pmacs::protocol::FrontendId::LOCAL,
|
||||
KeyEvent {
|
||||
code: KeyCode::Char('q'),
|
||||
modifiers: KeyModifiers::NONE,
|
||||
kind: KeyEventKind::Press,
|
||||
state: KeyEventState::empty(),
|
||||
},
|
||||
);
|
||||
let (after, name) = kinds(&s);
|
||||
assert!(name.ends_with("probe.rs"), "q restored the source buffer");
|
||||
assert_eq!(
|
||||
count_of(&after, "syntax-highlight"),
|
||||
1,
|
||||
"leaving a panel restores the source buffer's styling (got {after:?})"
|
||||
);
|
||||
}
|
||||
Loading…
Reference in New Issue