Close PR 187 review round 2

Retain panel statusline segments only for the exact side-window and
buffer presentation that published them. Clear that baseline whenever
the daemon publishes authoritative Absent.

Pin both transitions: same-window buffer replacement under NoMessage,
and Absent-to-Present under NoMessage.
This commit is contained in:
Levi Neuwirth 2026-07-28 21:30:50 -04:00
parent 0dba358987
commit bfaaf2bff6
2 changed files with 152 additions and 17 deletions

View File

@ -352,15 +352,16 @@ pub struct SemanticRenderState {
/// Whether an invalid panel frame was already reported since the last
/// valid one. Bounds the log exactly like `terminal_error_latched`.
panel_error_latched: bool,
/// The band's last PUBLISHED statusline segments and the side window
/// they belong to (review round 1, R2-4).
/// The band's last PUBLISHED statusline segments and the side-window
/// presentation they belong to (review rounds 1 and 2, R2-4).
///
/// The band repaints its whole mode line every frame, so
/// "publish nothing" has to be expressed as "paint what was published
/// last" — there is no wire-level suppression to fall back on the way
/// `StatuslineSegments` has. Keyed by window id so a replaced panel
/// never inherits its predecessor's provider text.
last_panel_statusline: Option<(WindowId, StatuslineWindowSegments)>,
/// `StatuslineSegments` has. Side affinity can replace a buffer in the
/// same window, so both identities are required to prevent one panel
/// presentation from inheriting its predecessor's provider text.
last_panel_statusline: Option<((WindowId, BufferId), StatuslineWindowSegments)>,
}
/// The presentation identity a shipped [`PanelFrame`] carries.
@ -956,8 +957,9 @@ impl SemanticRenderState {
// primary-document wire segments and the panel mode line, so the
// side half is taken from this same evaluation before it is
// consumed.
let side_window = state.core.borrow().side_window_for(self.frontend_id);
let panel_statusline = self.panel_statusline(statusline_evaluation.as_ref(), side_window);
let panel_presentation = self.panel_statusline_presentation(state);
let panel_statusline =
self.panel_statusline(statusline_evaluation.as_ref(), panel_presentation);
if let Some(evaluation) = statusline_evaluation {
self.emit_statusline_segments(evaluation, statusline_document_window, &mut out);
}
@ -1103,8 +1105,9 @@ impl SemanticRenderState {
// surface is a full-window terminal can still hold a side window,
// and suppressing the panel here would leave the peer's retained
// band on screen with no way to clear it.
let side_window = state.core.borrow().side_window_for(self.frontend_id);
let panel_statusline = self.panel_statusline(statusline_evaluation.as_ref(), side_window);
let panel_presentation = self.panel_statusline_presentation(state);
let panel_statusline =
self.panel_statusline(statusline_evaluation.as_ref(), panel_presentation);
if let Some(evaluation) = statusline_evaluation {
self.emit_statusline_segments(evaluation, statusline_document_window, &mut out);
}
@ -1198,14 +1201,23 @@ impl SemanticRenderState {
/// like `Invalidated` *removes* provider text on a transient
/// condition that said nothing about it.
///
/// The baseline is keyed by window id so it can never leak across a
/// panel replacement: a new side window starts with no retained text.
/// The baseline is keyed by both window and buffer identity. Side
/// affinity deliberately replaces the buffer in an existing side
/// window, and that replacement is a new presentation even though the
/// `WindowId` is stable.
fn panel_statusline_presentation(&self, state: &EditorState) -> Option<(WindowId, BufferId)> {
let core = state.core.borrow();
let window_id = core.side_window_for(self.frontend_id)?;
let buffer_id = core.windows.get(&window_id)?.buffer_id;
Some((window_id, buffer_id))
}
fn panel_statusline(
&mut self,
evaluation: Option<&StatuslineEvaluation>,
side_window: Option<WindowId>,
panel_presentation: Option<(WindowId, BufferId)>,
) -> Option<StatuslineWindowSegments> {
let Some(side_window) = side_window else {
let Some((side_window, side_buffer)) = panel_presentation else {
// No band to publish for; drop any baseline so a later panel
// cannot inherit a dead window's text.
self.last_panel_statusline = None;
@ -1215,7 +1227,7 @@ impl SemanticRenderState {
state
.last_panel_statusline
.as_ref()
.filter(|(window_id, _)| *window_id == side_window)
.filter(|(presentation, _)| *presentation == (side_window, side_buffer))
.map(|(_, segments)| segments.clone())
};
let Some(evaluation) = evaluation else {
@ -1231,9 +1243,12 @@ impl SemanticRenderState {
.find(|window| {
window.context.frontend_id == self.frontend_id
&& window.context.window_id == side_window
&& window.context.buffer_id == side_buffer
})
.cloned();
self.last_panel_statusline = found.clone().map(|segments| (side_window, segments));
self.last_panel_statusline = found
.clone()
.map(|segments| ((side_window, side_buffer), segments));
found
}
StatuslineEvaluationOutcome::Invalidated { .. } => {
@ -1372,9 +1387,13 @@ impl SemanticRenderState {
/// panel's presence.
fn publish_absent_panel(&mut self, out: &mut Vec<InstanceMessage>) {
self.panel_presentation = None;
// `Absent` also clears the peer's retained mode line. A later
// `Present` under `NoMessage` therefore has nothing it can
// legitimately retain, even if the same window and buffer reopen.
self.last_panel_statusline = None;
if self.last_panel_payload.as_ref() == Some(&PanelFramePayload::Absent) {
// A duplicate `Absent` does no work — but the clear above
// still runs, so the state stays idempotent rather than
// A duplicate `Absent` does no wire work — but both clears
// above still run, so the state stays idempotent rather than
// depending on which duplicate arrived first.
return;
}

View File

@ -1157,6 +1157,122 @@ fn r2_4_nomessage_retains_the_bands_published_segments() {
);
}
/// R2-4's retained baseline follows one window-and-buffer presentation,
/// not merely the side `WindowId`. Side affinity reuses the window when it
/// replaces the buffer, so `NoMessage` must not carry provider text from
/// the predecessor into the replacement.
#[test]
fn r2_4_nomessage_does_not_cross_a_same_window_buffer_replacement() {
let mut session = Session::new();
session.declare(1, ROWS, COLS);
open_panel(&session, "*first-panel*", 4);
let panel = session.side_window().expect("side window");
let document_buffer = session.state.core.borrow().windows[&session.document].buffer_id;
session.render.set_viewport(
document_buffer,
pmacs::protocol::ByteRange { start: 0, end: 0 },
0,
);
exec(
&session.state,
"pmacs.statusline.register {
name = 'buffer-probe', side = 'left',
fn = function(ctx) return 'CTX:' .. ctx.buffer:name() end,
}",
);
let first = rows_of(&session.present())
.last()
.expect("mode line")
.clone();
assert!(
first.contains("CTX:*first-panel*"),
"fixture precondition: the first panel publishes its buffer-scoped \
segment; got {first:?}"
);
// Replace the buffer in the existing side WindowId, then move the
// document off its declared viewport in the same transaction so phase
// 1 returns NoMessage before it can publish the replacement context.
exec(
&session.state,
"SECOND = pmacs.buffer.create('*second-panel*')
pmacs.window.display(SECOND, { side = 'bottom' })
pmacs.window.switch_buffer(pmacs.buffer.create('*elsewhere*'))",
);
assert_eq!(
session.side_window(),
Some(panel),
"fixture precondition: side-buffer replacement reuses the WindowId"
);
let replaced = rows_of(&session.present())
.last()
.expect("mode line")
.clone();
assert!(
replaced.contains("*second-panel*"),
"fixture precondition: the replacement panel was painted; got {replaced:?}"
);
assert!(
!replaced.contains("CTX:*first-panel*"),
"NoMessage must not carry buffer-scoped segments across a new panel \
presentation; got {replaced:?}"
);
}
/// An authoritative `Absent` clears the peer's retained band, including
/// its mode line. Reopening the same panel under `NoMessage` therefore
/// starts with no provider segments to retain.
#[test]
fn r2_4_nomessage_does_not_resurrect_segments_after_absent() {
let mut session = Session::new();
session.declare(1, ROWS, COLS);
open_panel(&session, "*panel*", 4);
let document_buffer = session.state.core.borrow().windows[&session.document].buffer_id;
session.render.set_viewport(
document_buffer,
pmacs::protocol::ByteRange { start: 0, end: 0 },
0,
);
exec(
&session.state,
"pmacs.statusline.register {
name = 'absence-probe', side = 'left',
fn = function() return 'BEFORE-ABSENT' end,
}",
);
assert!(
rows_of(&session.present())
.last()
.expect("mode line")
.contains("BEFORE-ABSENT"),
"fixture precondition: the segment was published"
);
exec(
&session.state,
"pmacs.window.switch_buffer(pmacs.buffer.create('*elsewhere*'))",
);
session.declare(2, 3, COLS);
assert_eq!(
session.frame(),
Some(PanelFramePayload::Absent),
"fixture precondition: the peer's panel state was cleared"
);
session.declare(3, ROWS, COLS);
let reappeared = rows_of(&session.present())
.last()
.expect("mode line")
.clone();
assert!(
!reappeared.contains("BEFORE-ABSENT"),
"NoMessage cannot retain across Absent because the peer has no panel \
statusline state left to retain; got {reappeared:?}"
);
}
// ---------------------------------------------------------------------------
// Review round 1 sweep — the same defect shape, found elsewhere
// ---------------------------------------------------------------------------