fix(panel): the terminal mapping half --- classification, domain, publication
Three of the four terminal gaps. The fourth is recorded as owed rather than faked; see below. **THE STABLE CLASSIFICATION WAS INCOMPLETE.** Cursor motion still advanced the mapping revision: `restore_cursor`, `horizontal_tab`, `move_vertical`, `move_horizontal`, `set_col` and `set_row` all called `changed()`. Moving the caret denotes nothing new, and a child that merely repositions its cursor would have cancelled a drag. All six take the display-only path now. Worse, **rewriting the same glyph under another style advanced it**, which is precisely the control SS5b requires to hold. `write_character` now compares the glyph before writing --- sampled BEFORE `clear_wide_at`, which blanks a cell that is part of a wide pair and would otherwise make every rewrite look like a change. That ordering was found by instrumenting the failing row, not by reading the code. **THE SNAPSHOT CARRIED DOCUMENT-ONLY STATE FOR TERMINALS.** `view_top`, `view_left`, wrap, content columns, fold policy and folds describe a document projection and take no part in a terminal's, where the child's screen decides the mapping. They live inside the `Document` arm now; only common geometry --- buffer identity, rows, columns --- stays outside. **AND THE REVISION WAS NOT PUBLICATION-CONSISTENT.** `view_mapping_identity` read the LIVE screen revision while `projection_ref` returns the last PUBLISHED cells, so buffered output under synchronized-output would stamp displayed cells with authority they were never painted under --- a frontend echoing a generation matching nothing it can see. `ScreenProjection` carries `mapping_revision` now and the published value is what is read. **The witnesses were separated across the seam**, which review named exactly: `screen.rs` proved the counter, the daemon proved enum selection, and a `view_mapping_identity` returning a constant would have left both green. A daemon-level row now drives real events through a panel terminal and asserts the daemon's generation moves on a new glyph and holds across a style-only rewrite and across cursor motion. **OWED, NOT DONE: the scroll-anchor row.** The anchor is in the key, but three attempts failed to drive a scroll from this fixture --- `scroll_lines` wants a viewport the projection registers on its own schedule, and `scroll_view` with an explicit size reports no movement after forty line feeds. Recorded in the ledger rather than faked or quietly dropped: without it, a constant ANCHOR alongside a live revision still passes every terminal row that exists. The two test hooks are `#[doc(hidden)] pub`, not `#[cfg(test)]`, because the rows needing them are integration tests and those link the library without `cfg(test)`. Verified: focused suite 37/37, `cargo test --lib` 1945 green, clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
parent
42efc3618f
commit
0bdfc9b6dd
|
|
@ -437,6 +437,17 @@ from #171 and #215.
|
|||
three of them lost to these two signatures. U9's synthetic-load
|
||||
control remains unrun and is the cheapest thing that would either
|
||||
implicate load or clear it.
|
||||
- **OWED WITNESS — terminal scroll-anchor movement at the daemon
|
||||
level.** The anchor is in the key (`PanelMappingContent::Terminal`
|
||||
carries it) and the review asked for a row proving the daemon's
|
||||
generation moves when it does. **Three attempts failed to drive a
|
||||
scroll from the panel fixture**: `scroll_lines` needs a viewport the
|
||||
panel projection registers on its own schedule, and `scroll_view`
|
||||
with an explicit content size still reports no movement after forty
|
||||
line feeds, so the history it would move into is not accumulating as
|
||||
the fixture assumes. **Recorded rather than faked.** Without it, a
|
||||
`view_mapping_identity` returning a constant ANCHOR while reporting a
|
||||
live revision passes every terminal row that exists.
|
||||
- **Gates:** the four `bottom_panel_*` suites, the GUI 1a wire suite,
|
||||
`PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`, and **`--protocol`**.
|
||||
|
||||
|
|
|
|||
|
|
@ -396,12 +396,6 @@ pub struct PanelMappingSnapshot {
|
|||
/// inverts nothing like a 6×2 one.
|
||||
rows: u32,
|
||||
cols: u32,
|
||||
view_top: usize,
|
||||
view_left: u32,
|
||||
wrap: crate::view::WrapMode,
|
||||
content_cols: u32,
|
||||
fold_projection: bool,
|
||||
folds: Vec<pmacs_protocol::ByteRange>,
|
||||
content: PanelMappingContent,
|
||||
}
|
||||
|
||||
|
|
@ -422,6 +416,18 @@ pub enum PanelMappingContent {
|
|||
Document {
|
||||
/// The buffer's content revision, or `None` if it is gone.
|
||||
revision: Option<u64>,
|
||||
/// Vertical viewport origin.
|
||||
view_top: usize,
|
||||
/// Horizontal viewport origin. GUI arc 1b makes this move.
|
||||
view_left: u32,
|
||||
/// Wrap mode: it decides how a source line becomes display rows.
|
||||
wrap: crate::view::WrapMode,
|
||||
/// Text width with the gutter reservation already subtracted.
|
||||
content_cols: u32,
|
||||
/// The owning frontend's fold projection policy.
|
||||
fold_projection: bool,
|
||||
/// Fold content, read at its source.
|
||||
folds: Vec<pmacs_protocol::ByteRange>,
|
||||
},
|
||||
/// A terminal panel: the screen's **mapping revision** and the
|
||||
/// view's scroll anchor.
|
||||
|
|
@ -2545,13 +2551,8 @@ impl EditorState {
|
|||
.get(buffer_id)
|
||||
.ok()
|
||||
.map(crate::buffer::Buffer::revision);
|
||||
PanelMappingContent::Document { revision }
|
||||
};
|
||||
|
||||
Some(PanelMappingSnapshot {
|
||||
buffer_id,
|
||||
rows: size.rows,
|
||||
cols: size.cols,
|
||||
PanelMappingContent::Document {
|
||||
revision,
|
||||
view_top: window.view_top,
|
||||
view_left: window.view_left,
|
||||
wrap: window.last_wrap,
|
||||
|
|
@ -2560,12 +2561,23 @@ impl EditorState {
|
|||
.views
|
||||
.get(&frontend_id)
|
||||
.is_some_and(|view| view.fold_projection),
|
||||
// Read at their SOURCE — the registry's ranges — rather than
|
||||
// through the derived `VisibleLineMap`, whose only public
|
||||
// summary is `is_identity()`. That is too coarse: a fold edit
|
||||
// leaving the map non-identity still changes which source
|
||||
// line a row shows.
|
||||
// Read at their SOURCE — the registry's ranges — rather
|
||||
// than through the derived `VisibleLineMap`, whose only
|
||||
// public summary is `is_identity()`. Too coarse: a fold
|
||||
// edit leaving the map non-identity still changes which
|
||||
// source line a row shows.
|
||||
folds: core.fold_registry.folds(buffer_id),
|
||||
}
|
||||
};
|
||||
|
||||
// Only COMMON geometry lives out here. Everything below is
|
||||
// domain-specific: `view_top`, `view_left`, wrap, gutter width
|
||||
// and folds describe a DOCUMENT projection and take no part in a
|
||||
// terminal's, where the child's screen decides the mapping.
|
||||
Some(PanelMappingSnapshot {
|
||||
buffer_id,
|
||||
rows: size.rows,
|
||||
cols: size.cols,
|
||||
content,
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -114,6 +114,15 @@ pub struct ScreenProjection {
|
|||
pub title: Option<String>,
|
||||
/// Screen generation represented by this projection.
|
||||
pub generation: u64,
|
||||
/// §5b — the **mapping revision** this projection was published at.
|
||||
///
|
||||
/// Carried here, not read live, because the two diverge while
|
||||
/// synchronized output is held: `projection_ref` keeps returning the
|
||||
/// last PUBLISHED cells while the live screen races ahead, so
|
||||
/// stamping a frame with the live revision would give displayed
|
||||
/// cells authority they were never painted under — a frontend would
|
||||
/// then echo a generation that matches nothing it can see.
|
||||
pub mapping_revision: u64,
|
||||
}
|
||||
|
||||
/// Borrowed, publication-consistent row projection for in-process views.
|
||||
|
|
@ -128,6 +137,8 @@ pub(crate) struct BorrowedScreenProjection<'a> {
|
|||
pub cursor: Option<CellCoord>,
|
||||
pub title: Option<&'a str>,
|
||||
pub generation: u64,
|
||||
/// §5b — the mapping revision this projection was published at.
|
||||
pub mapping_revision: u64,
|
||||
}
|
||||
|
||||
impl BorrowedScreenProjection<'_> {
|
||||
|
|
@ -147,6 +158,7 @@ impl ScreenProjection {
|
|||
cursor: self.cursor,
|
||||
title: self.title.as_deref(),
|
||||
generation: self.generation,
|
||||
mapping_revision: self.mapping_revision,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -224,6 +236,7 @@ impl TerminalScreen {
|
|||
cursor: Some(CellCoord::new(0, 0)),
|
||||
title: None,
|
||||
generation: 0,
|
||||
mapping_revision: 0,
|
||||
};
|
||||
Ok(Self {
|
||||
size,
|
||||
|
|
@ -599,6 +612,7 @@ impl TerminalScreen {
|
|||
.then(|| CellCoord::new(self.cursor.row as u32, self.cursor.col as u32)),
|
||||
title: self.title.as_deref(),
|
||||
generation: self.generation,
|
||||
mapping_revision: self.mapping_revision,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -658,7 +672,7 @@ impl TerminalScreen {
|
|||
self.g0 = saved.g0;
|
||||
self.g1 = saved.g1;
|
||||
self.use_g1 = saved.use_g1;
|
||||
self.changed();
|
||||
self.display_only_changed();
|
||||
}
|
||||
|
||||
fn write_text(&mut self, text: &str) {
|
||||
|
|
@ -812,6 +826,24 @@ impl TerminalScreen {
|
|||
if self.modes.insert {
|
||||
self.insert_characters(width as u32);
|
||||
}
|
||||
// §5b — did the GLYPH change, or only the pen? Rewriting the
|
||||
// same character in a new colour repaints the cell without
|
||||
// changing what the coordinate denotes, and a drag must survive
|
||||
// it.
|
||||
//
|
||||
// Sampled BEFORE `clear_wide_at`, which blanks the cell when it
|
||||
// is part of a wide pair — sampling after would compare the new
|
||||
// glyph against a default and call every rewrite a change.
|
||||
let glyph_changed = {
|
||||
let row = self.cursor.row;
|
||||
let col = self.cursor.col;
|
||||
let cells = &self.active().rows[row].cells;
|
||||
cells[col].glyph != Glyph::Char(ch)
|
||||
|| (width == 2
|
||||
&& cells
|
||||
.get(col + 1)
|
||||
.is_none_or(|next| next.glyph != Glyph::Continuation))
|
||||
};
|
||||
self.clear_wide_at(self.cursor.row, self.cursor.col);
|
||||
if width == 2 {
|
||||
self.clear_wide_at(self.cursor.row, self.cursor.col + 1);
|
||||
|
|
@ -832,7 +864,11 @@ impl TerminalScreen {
|
|||
self.cursor.pending_wrap = false;
|
||||
}
|
||||
self.last_grapheme = Some((row, col));
|
||||
if glyph_changed {
|
||||
self.changed();
|
||||
} else {
|
||||
self.display_only_changed();
|
||||
}
|
||||
}
|
||||
|
||||
fn soft_wrap(&mut self) {
|
||||
|
|
@ -886,7 +922,7 @@ impl TerminalScreen {
|
|||
.copied()
|
||||
.unwrap_or(cols - 1);
|
||||
self.cursor.pending_wrap = false;
|
||||
self.changed();
|
||||
self.display_only_changed();
|
||||
}
|
||||
|
||||
fn move_vertical(&mut self, delta: i64) {
|
||||
|
|
@ -903,7 +939,7 @@ impl TerminalScreen {
|
|||
};
|
||||
self.cursor.row = moved.clamp(lo, hi);
|
||||
self.cursor.pending_wrap = false;
|
||||
self.changed();
|
||||
self.display_only_changed();
|
||||
}
|
||||
|
||||
fn move_horizontal(&mut self, delta: i64) {
|
||||
|
|
@ -915,13 +951,13 @@ impl TerminalScreen {
|
|||
};
|
||||
self.cursor.col = moved.min(self.size.cols as usize - 1);
|
||||
self.cursor.pending_wrap = false;
|
||||
self.changed();
|
||||
self.display_only_changed();
|
||||
}
|
||||
|
||||
fn set_col(&mut self, col: u32) {
|
||||
self.cursor.col = col.saturating_sub(1).min(self.size.cols - 1) as usize;
|
||||
self.cursor.pending_wrap = false;
|
||||
self.changed();
|
||||
self.display_only_changed();
|
||||
}
|
||||
fn set_row(&mut self, row: u32) {
|
||||
let base = if self.modes.origin {
|
||||
|
|
@ -936,7 +972,7 @@ impl TerminalScreen {
|
|||
};
|
||||
self.cursor.row = (base + row.saturating_sub(1) as usize).min(hi);
|
||||
self.cursor.pending_wrap = false;
|
||||
self.changed();
|
||||
self.display_only_changed();
|
||||
}
|
||||
fn set_position(&mut self, row: u32, col: u32) {
|
||||
self.set_row(row);
|
||||
|
|
@ -1525,8 +1561,20 @@ impl TerminalScreen {
|
|||
.then(|| CellCoord::new(self.cursor.row as u32, self.cursor.col as u32)),
|
||||
title: self.title.clone(),
|
||||
generation: self.generation,
|
||||
mapping_revision: self.mapping_revision,
|
||||
}
|
||||
}
|
||||
/// Publish the current projection, for tests that drive events
|
||||
/// directly instead of through the PTY reader.
|
||||
///
|
||||
/// `#[doc(hidden)]` rather than `#[cfg(test)]`: the rows that need
|
||||
/// it are integration tests, which link the library WITHOUT
|
||||
/// `cfg(test)` and so cannot see a gated item.
|
||||
#[doc(hidden)]
|
||||
pub fn publish_for_test(&mut self) {
|
||||
self.publish();
|
||||
}
|
||||
|
||||
fn publish(&mut self) {
|
||||
self.published = self.current_projection();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -249,6 +249,34 @@ impl TerminalManager {
|
|||
self.screen_size(key.buffer_id)
|
||||
}
|
||||
|
||||
/// Apply one parsed event to a session's screen, for tests.
|
||||
///
|
||||
/// Terminal output normally arrives on the PTY reader thread, which
|
||||
/// no daemon-level test can drive deterministically. §5b's terminal
|
||||
/// rows must nevertheless be witnessed **across the seam** — the
|
||||
/// screen counter and the daemon's key are separately provable, and
|
||||
/// a `view_mapping_identity` returning a constant would leave both
|
||||
/// green — so this exists to join them.
|
||||
///
|
||||
/// `#[doc(hidden)]` rather than `#[cfg(test)]`, because the rows
|
||||
/// that need it are integration tests and those link the library
|
||||
/// without `cfg(test)`.
|
||||
#[doc(hidden)]
|
||||
pub fn apply_event_for_test(
|
||||
&mut self,
|
||||
buffer_id: BufferId,
|
||||
event: crate::ansi::AnsiEvent,
|
||||
) -> bool {
|
||||
match self.sessions.get_mut(&buffer_id) {
|
||||
Some(session) => {
|
||||
session.screen.apply_event(event);
|
||||
session.screen.publish_for_test();
|
||||
true
|
||||
}
|
||||
None => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// §5b — the terminal's **mapping revision** plus its per-view
|
||||
/// scroll anchor: together, the identity of what a coordinate in
|
||||
/// this view denotes.
|
||||
|
|
@ -265,7 +293,14 @@ impl TerminalManager {
|
|||
// `top` IS the anchor: `None` means following the live tail,
|
||||
// which is itself a distinct state from any pinned row.
|
||||
let anchor = self.views.get(&key).and_then(|view| view.top);
|
||||
Some((session.screen.mapping_revision(), anchor))
|
||||
// The PUBLISHED revision, not the live one. While synchronized
|
||||
// output is held, `projection_ref` keeps returning the last
|
||||
// published cells while the screen races ahead — reading
|
||||
// `screen.mapping_revision()` there would stamp displayed cells
|
||||
// with authority they were never painted under, and the frontend
|
||||
// would echo a generation matching nothing it can see.
|
||||
let published = session.screen.projection_ref().mapping_revision;
|
||||
Some((published, anchor))
|
||||
}
|
||||
|
||||
/// The shared screen's current size, read from the borrowed
|
||||
|
|
@ -984,6 +1019,7 @@ mod tests {
|
|||
cursor: None,
|
||||
title: Some("shell".into()),
|
||||
generation: 7,
|
||||
mapping_revision: 0,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1264,6 +1300,7 @@ mod tests {
|
|||
cursor: None,
|
||||
title: None,
|
||||
generation: 2,
|
||||
mapping_revision: 0,
|
||||
};
|
||||
let mut state = TerminalViewState {
|
||||
top: Some(LogicalCellAnchor {
|
||||
|
|
|
|||
|
|
@ -1744,3 +1744,89 @@ fn the_mapping_snapshot_picks_the_terminal_domain_for_a_terminal_panel() {
|
|||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// §5b — the terminal key **across the seam**.
|
||||
///
|
||||
/// `screen.rs` proves the counter classifies events correctly, and the
|
||||
/// row above proves the snapshot picks the terminal domain. Neither
|
||||
/// notices a `view_mapping_identity` that returns a CONSTANT: the
|
||||
/// classification is right, the branch is taken, and the daemon's key
|
||||
/// still never moves. These rows join the two halves.
|
||||
#[test]
|
||||
fn g2_g3_a_terminal_panels_key_tracks_its_screen_and_anchor() {
|
||||
use pmacs::ansi::AnsiEvent;
|
||||
|
||||
let mut session = Session::new();
|
||||
session.declare(1, 24, 80);
|
||||
exec(
|
||||
&session.state,
|
||||
"TERM_BUF = pmacs.terminal.open { command = \"/bin/sh\", \
|
||||
args = { \"-c\", \"sleep 30\" }, display = \"panel\" }",
|
||||
);
|
||||
let _ = session.frame();
|
||||
|
||||
let buffer_id = {
|
||||
let core = session.state.core.borrow();
|
||||
let side = core.side_window_for(FID).expect("side");
|
||||
core.windows[&side].buffer_id
|
||||
};
|
||||
let feed = |session: &Session, event: AnsiEvent| {
|
||||
assert!(
|
||||
session
|
||||
.state
|
||||
.terminal_manager
|
||||
.borrow_mut()
|
||||
.apply_event_for_test(buffer_id, event),
|
||||
"the panel's terminal session must exist"
|
||||
);
|
||||
};
|
||||
|
||||
let start = mapping_generation(&mut session).expect("a terminal panel has a key");
|
||||
|
||||
// CHANGING: a glyph appears where none was.
|
||||
feed(&session, AnsiEvent::Text("A".to_owned()));
|
||||
let after_glyph = mapping_generation(&mut session).expect("a key");
|
||||
assert!(
|
||||
after_glyph > start,
|
||||
"new output changes what a coordinate denotes"
|
||||
);
|
||||
|
||||
// STABLE: the same glyph rewritten under a different pen. This is
|
||||
// the row that forced `write_character` to compare glyphs — a
|
||||
// blanket bump made a recolour cancel the drag.
|
||||
feed(&session, AnsiEvent::CursorPosition { row: 1, col: 1 });
|
||||
feed(
|
||||
&session,
|
||||
AnsiEvent::SetStyle(pmacs_protocol::Style::default()),
|
||||
);
|
||||
feed(&session, AnsiEvent::Text("A".to_owned()));
|
||||
assert_eq!(
|
||||
mapping_generation(&mut session),
|
||||
Some(after_glyph),
|
||||
"rewriting the SAME glyph in another style repaints the cell \
|
||||
without moving what it denotes"
|
||||
);
|
||||
|
||||
// STABLE: ordinary cursor motion.
|
||||
feed(&session, AnsiEvent::CursorPosition { row: 2, col: 3 });
|
||||
assert_eq!(
|
||||
mapping_generation(&mut session),
|
||||
Some(after_glyph),
|
||||
"moving the caret denotes nothing new — and these paths were \
|
||||
advancing the revision until §5b's terminal correction"
|
||||
);
|
||||
|
||||
// NOT YET WITNESSED: scroll-anchor movement at this level.
|
||||
//
|
||||
// The anchor IS in the key — `PanelMappingContent::Terminal` carries
|
||||
// it — but driving a scroll from here has defeated three attempts:
|
||||
// `scroll_lines` needs a viewport the panel projection registers on
|
||||
// its own schedule, and `scroll_view` with an explicit size still
|
||||
// reports no movement after forty line feeds, so the history the
|
||||
// scroll would move into is not accumulating the way this fixture
|
||||
// assumes.
|
||||
//
|
||||
// Recorded as OWED rather than faked. Without it, a
|
||||
// `view_mapping_identity` that returned a constant ANCHOR while
|
||||
// reporting a live revision would pass every row above.
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue