diff --git a/src/editor.rs b/src/editor.rs index 388ea01..ecf6f0b 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -375,6 +375,70 @@ const DOUBLE_CLICK_MAX_DELAY: Duration = Duration::from_millis(500); /// column and clobbers no information. const DIVIDER_HANDLE_GLYPH: char = '⇕'; +/// The panel's **inverse mapping**, captured exactly (§5b, +/// Q#BP-R3). +/// +/// A STRUCT compared structurally, not a hash. A hash would make +/// authoritative equality probabilistic: a collision silently +/// accepts a stale gesture, which is the precise failure this key +/// exists to prevent. The emitted `mapping_generation` is still a +/// `u64` on the wire — only the daemon's own comparison is exact. +/// +/// **Deliberately EXCLUDED**, each an explicit contract: focus, +/// styling and theme, the cursor, and the selection. None changes +/// which byte a cell denotes, and a drag repaints the selection on +/// every motion — a key that moved with them would cancel the +/// gesture it protects after one step. +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct PanelMappingSnapshot { + buffer_id: crate::buffer::BufferId, + /// Rows and columns held apart, never multiplied: a 2×6 panel + /// 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, + content: PanelMappingContent, +} + +impl PanelMappingSnapshot { + /// Which domain decided this mapping. Exposed for the row that pins + /// the branch is taken by target kind. + #[must_use] + pub fn content(&self) -> &PanelMappingContent { + &self.content + } +} + +/// What decides the mapping BELOW the geometry, which differs by +/// target kind. +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum PanelMappingContent { + /// A document panel: the buffer's content revision. + Document { + /// The buffer's content revision, or `None` if it is gone. + revision: Option, + }, + /// A terminal panel: the screen's **mapping revision** and the + /// view's scroll anchor. + /// + /// **Not the buffer's revision**, which tracks something else + /// entirely, and **not `Screen::generation`**, which advances + /// for style, title, bell, tab stops and cursor motion — none + /// of which changes what a coordinate denotes. + Terminal { + /// Content and topology identity, excluding style, title, bell, + /// tab stops and cursor motion. + mapping_revision: u64, + /// The view's scroll anchor; `None` follows the live tail. + anchor: Option, + }, +} + impl EditorState { /// Construct a fresh editor for an unnamed scratch buffer. /// @@ -2452,78 +2516,58 @@ impl EditorState { true } - /// Fingerprint of the panel's **inverse mapping** for one frontend - /// (§5b, Q#BP-R3). + /// Capture the panel's inverse mapping for one frontend. /// - /// This is the whole of "what decides which byte a cell means". A - /// generation is derived from it by advancing whenever it changes, - /// which makes the changing/stable split **structural** rather than - /// a list of bump sites someone must remember to touch: an input - /// that is hashed moves the key by construction, and one that is not - /// cannot. - /// - /// **Deliberately EXCLUDED**, and each exclusion is a contract: - /// focus, styling and theme, the cursor, and the selection. None of - /// them changes which byte a cell denotes, and a drag provokes - /// selection repaints on every motion — a key that moved with them - /// would cancel the gesture it is meant to protect after one step. - /// - /// Returns `None` when there is no presentable panel, which is not - /// the same as a zero key: absence of a mapping is not a mapping. - pub fn panel_mapping_fingerprint(&self, frontend_id: FrontendId) -> Option { - use std::hash::{Hash, Hasher}; - + /// `None` when no panel is presentable — the absence of a mapping, + /// which is not the same as a mapping of zero. + pub fn panel_mapping_snapshot(&self, frontend_id: FrontendId) -> Option { let core = self.core.borrow(); let size = core.panel_grid_size(frontend_id)?; let side = core.side_window_for(frontend_id)?; let window = core.windows.get(&side)?; + let buffer_id = window.buffer_id; - let mut hasher = std::collections::hash_map::DefaultHasher::new(); - // Identity first: a different buffer is a different mapping even - // if every geometry input coincides. - window.buffer_id.hash(&mut hasher); - // Grid ROWS and COLUMNS hashed separately, not as an area — a - // 2x6 and a 6x2 panel invert differently. - size.rows.hash(&mut hasher); - size.cols.hash(&mut hasher); - // Viewport, both axes. `view_left` matters from GUI arc 1b - // onward, and is hashed now so the key does not need revisiting - // when horizontal scrolling starts moving it. - window.view_top.hash(&mut hasher); - window.view_left.hash(&mut hasher); - // Wrap mode and the content width that the gutter reservation - // has already been subtracted from: together these decide how a - // source line is broken into display rows and where column zero - // sits. - window.last_wrap.hash(&mut hasher); - window.last_content_cols.hash(&mut hasher); - // Fold POLICY and fold CONTENT are separate inputs. The policy - // belongs to the owning frontend's view; the content belongs to - // the window. Either alone can change which source line a grid - // row shows. - core.views - .get(&frontend_id) - .is_some_and(|view| view.fold_projection) - .hash(&mut hasher); - // Hashed at their SOURCE — the registry's ranges — rather than - // through the derived `VisibleLineMap`, whose only public - // summary is `is_identity()`. That would be too coarse: a fold - // edit that leaves the map non-identity would not move the key - // while plainly changing which source line a row shows. - for range in core.fold_registry.folds(window.buffer_id) { - range.start.hash(&mut hasher); - range.end.hash(&mut hasher); - } - // Content. A foreign edit moves the mapping with every geometry - // input untouched, and is the case the epoch ladder cannot see. - let registry = core.registry.clone(); - let revision = registry - .borrow() - .get(window.buffer_id) - .ok() - .map(crate::buffer::Buffer::revision); - revision.hash(&mut hasher); - Some(hasher.finish()) + let content = if self.terminal_manager.borrow().is_terminal(buffer_id) { + let key = TerminalViewKey::new(frontend_id, side, buffer_id); + let (mapping_revision, anchor) = self + .terminal_manager + .borrow() + .view_mapping_identity(key) + .unwrap_or((0, None)); + PanelMappingContent::Terminal { + mapping_revision, + anchor, + } + } else { + let registry = core.registry.clone(); + let revision = registry + .borrow() + .get(buffer_id) + .ok() + .map(crate::buffer::Buffer::revision); + PanelMappingContent::Document { revision } + }; + + Some(PanelMappingSnapshot { + buffer_id, + rows: size.rows, + cols: size.cols, + view_top: window.view_top, + view_left: window.view_left, + wrap: window.last_wrap, + content_cols: window.last_content_cols, + fold_projection: core + .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. + folds: core.fold_registry.folds(buffer_id), + content, + }) } /// Paint one semantic frontend's side window into a panel-sized grid diff --git a/src/semantic_render.rs b/src/semantic_render.rs index b2c8d2b..ee1fe66 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -396,7 +396,7 @@ pub struct SemanticRenderState { /// backward, so this is a high-water mark for the session. /// `generation` starts at 0 meaning "never established"; the first /// real mapping takes 1, because zero is invalid on the wire. - panel_mapping: Option<(u64, u64)>, + panel_mapping: Option<(crate::editor::PanelMappingSnapshot, u64)>, /// Highest presentation epoch allocated for this session; `0` means /// none has been. Advanced only when a frame is actually shipped, so /// a frame that fails validation does not burn an identity the peer @@ -631,16 +631,22 @@ impl SemanticRenderState { /// **not** reset the key — the high-water mark survives `Absent`, /// so a frame delayed across a hide cannot come back with a lower /// generation and be believed. - pub fn panel_mapping_generation(&mut self, fingerprint: Option) -> Option { - let fingerprint = fingerprint?; - let next = match self.panel_mapping { - Some((seen, generation)) if seen == fingerprint => generation, + pub fn panel_mapping_generation( + &mut self, + snapshot: Option, + ) -> Option { + let snapshot = snapshot?; + let next = match &self.panel_mapping { + // Compared STRUCTURALLY. A hash would make this + // probabilistic, and a collision here silently accepts a + // stale gesture — the exact failure the key exists for. + Some((seen, generation)) if *seen == snapshot => *generation, Some((_, generation)) => generation.saturating_add(1), // First establishment takes 1, never 0: zero is the wire's // "uninitialised" value and is refused on sight. None => 1, }; - self.panel_mapping = Some((fingerprint, next)); + self.panel_mapping = Some((snapshot, next)); Some(next) } @@ -648,7 +654,9 @@ impl SemanticRenderState { /// callers that must not have a side effect. #[must_use] pub fn panel_mapping_generation_peek(&self) -> Option { - self.panel_mapping.map(|(_, generation)| generation) + self.panel_mapping + .as_ref() + .map(|(_, generation)| *generation) } /// Whether the last shipped declaration is a `Present` whose epochs diff --git a/src/terminal/screen.rs b/src/terminal/screen.rs index 11c92ef..87d4140 100644 --- a/src/terminal/screen.rs +++ b/src/terminal/screen.rs @@ -192,6 +192,9 @@ pub struct TerminalScreen { tab_stops: BTreeSet, title: Option, generation: u64, + /// §5b — see [`Screen::mapping_revision`]. Separate from + /// `generation`, which advances for style and title too. + mapping_revision: u64, published: ScreenProjection, sync_started: Option, next_line_id: u64, @@ -240,6 +243,7 @@ impl TerminalScreen { tab_stops: default_tab_stops(size.cols as usize), title: None, generation: 0, + mapping_revision: 0, published, sync_started: None, next_line_id, @@ -269,24 +273,24 @@ impl TerminalScreen { } AnsiEvent::SetStyle(style) => { self.style = style; - self.changed(); + self.display_only_changed(); None } AnsiEvent::CarriageReturn => { self.cursor.col = 0; self.cursor.pending_wrap = false; - self.changed(); + self.display_only_changed(); None } AnsiEvent::Backspace => { self.cursor.col = self.cursor.col.saturating_sub(1); self.cursor.pending_wrap = false; - self.changed(); + self.display_only_changed(); None } AnsiEvent::Bell => { self.bell_count = self.bell_count.saturating_add(1); - self.changed(); + self.display_only_changed(); None } AnsiEvent::LineFeed | AnsiEvent::Index => { @@ -308,17 +312,17 @@ impl TerminalScreen { } AnsiEvent::SetTabStop => { self.tab_stops.insert(self.cursor.col); - self.changed(); + self.display_only_changed(); None } AnsiEvent::ClearTabStop => { self.tab_stops.remove(&self.cursor.col); - self.changed(); + self.display_only_changed(); None } AnsiEvent::ClearAllTabStops => { self.tab_stops.clear(); - self.changed(); + self.display_only_changed(); None } AnsiEvent::CursorUp(n) => { @@ -432,23 +436,23 @@ impl TerminalScreen { CharacterSetSlot::G0 => self.g0 = charset, CharacterSetSlot::G1 => self.g1 = charset, } - self.changed(); + self.display_only_changed(); None } AnsiEvent::ShiftOut => { self.use_g1 = true; - self.changed(); + self.display_only_changed(); None } AnsiEvent::ShiftIn => { self.use_g1 = false; - self.changed(); + self.display_only_changed(); None } AnsiEvent::DeviceRequest(request) => Some(self.device_reply(request)), AnsiEvent::SetTitle(title) => { self.title = Some(sanitize_title(&title)); - self.changed(); + self.display_only_changed(); None } AnsiEvent::EraseToEol => { @@ -1466,6 +1470,32 @@ impl TerminalScreen { fn changed(&mut self) { self.generation = self.generation.saturating_add(1); + // §5b: by DEFAULT a change also moves the mapping. Anything not + // explicitly classified as display-only is treated as content, + // which fails in the safe direction — over-cancelling a gesture + // is a nuisance, under-cancelling one lets a stale coordinate + // reach a child. + self.mapping_revision = self.mapping_revision.saturating_add(1); + } + + /// A change that repaints but **cannot move what a coordinate + /// denotes** (§5b's stable controls). + /// + /// Style, title, bell, tab stops and pure cursor motion all land + /// here. The existing `generation` still advances — the screen does + /// look different — but `mapping_revision` does not, so a drag + /// survives them. Keying the panel's mapping on `generation` was + /// rejected for exactly this reason: it moves for all of these. + fn display_only_changed(&mut self) { + self.generation = self.generation.saturating_add(1); + } + + /// §5b — identity of what a terminal coordinate DENOTES. + /// + /// Advances with content and topology, and holds across the display + /// changes above. + pub fn mapping_revision(&self) -> u64 { + self.mapping_revision } fn current_snapshot(&self) -> ScreenSnapshot { ScreenSnapshot { @@ -1910,6 +1940,67 @@ mod tests { assert_eq!(s.snapshot().cursor, Some(CellCoord::new(2, 0))); } + /// §5b G3 — the terminal **stable controls**. + /// + /// These are exactly the events that make `generation` unusable as a + /// mapping key: each one advances it. `mapping_revision` must hold + /// across all of them, or a drag over a panel terminal dies the + /// moment the child recolours a character or rings the bell. + #[test] + fn display_only_events_advance_the_generation_but_not_the_mapping() { + for (name, event) in [ + ("style", AnsiEvent::SetStyle(Style::default())), + ("title", AnsiEvent::SetTitle("t".to_owned())), + ("bell", AnsiEvent::Bell), + ("tab stop", AnsiEvent::SetTabStop), + ("clear tab stops", AnsiEvent::ClearAllTabStops), + ("carriage return", AnsiEvent::CarriageReturn), + ] { + let mut s = screen(2, 16); + let before_generation = s.snapshot().generation; + let before_mapping = s.mapping_revision(); + + s.apply_event(event); + + assert!( + s.snapshot().generation > before_generation, + "{name} repaints, so the display generation must advance \ + — otherwise this row proves nothing about the split" + ); + assert_eq!( + s.mapping_revision(), + before_mapping, + "{name} cannot change what a coordinate denotes, so the \ + MAPPING revision must hold" + ); + } + } + + /// §5b G2 — content and topology **do** move the mapping revision. + /// + /// The positive half. Without it, a `mapping_revision` that never + /// advanced at all would pass every stable control above. + #[test] + fn content_events_advance_the_mapping_revision() { + for (name, event) in [ + ("text", AnsiEvent::Text("hi".to_owned())), + ("line feed", AnsiEvent::LineFeed), + ( + "erase display", + AnsiEvent::EraseDisplay(crate::ansi::EraseMode::ToEnd), + ), + ("scroll up", AnsiEvent::ScrollUp(1)), + ] { + let mut s = screen(2, 16); + let before = s.mapping_revision(); + s.apply_event(event); + assert!( + s.mapping_revision() > before, + "{name} changes what a coordinate denotes" + ); + } + } + #[test] fn resize_only_adds_default_tab_stops_in_new_columns() { let mut s = screen(2, 16); diff --git a/src/terminal/view.rs b/src/terminal/view.rs index b787dee..0719c6e 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -249,6 +249,25 @@ impl TerminalManager { self.screen_size(key.buffer_id) } + /// §5b — the terminal's **mapping revision** plus its per-view + /// scroll anchor: together, the identity of what a coordinate in + /// this view denotes. + /// + /// The anchor is part of it because the same coordinate names a + /// different retained row once the view scrolls, even with the + /// child's screen untouched. + #[must_use] + pub fn view_mapping_identity( + &self, + key: TerminalViewKey, + ) -> Option<(u64, Option)> { + let session = self.sessions.get(&key.buffer_id)?; + // `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 shared screen's current size, read from the borrowed /// projection. /// diff --git a/tests/bottom_panel_stage2b_daemon_acceptance.rs b/tests/bottom_panel_stage2b_daemon_acceptance.rs index a379167..a0cda31 100644 --- a/tests/bottom_panel_stage2b_daemon_acceptance.rs +++ b/tests/bottom_panel_stage2b_daemon_acceptance.rs @@ -1392,8 +1392,8 @@ fn foreign_edit(session: &Session, text: &str) { /// The key as the daemon would compute it for `FID`, advancing on change. fn mapping_generation(session: &mut Session) -> Option { - let fingerprint = session.state.panel_mapping_fingerprint(FID); - session.render.panel_mapping_generation(fingerprint) + let snapshot = session.state.panel_mapping_snapshot(FID); + session.render.panel_mapping_generation(snapshot) } /// §5b G1 — a **foreign** edit before the next render moves the key. @@ -1617,61 +1617,130 @@ fn g2_each_input_of_the_inverse_mapping_moves_the_key_on_its_own() { } } -/// §5b G2 — grid **rows** and **columns** each move the key. +/// §5b G2 — grid **rows** and **columns** are independent inputs, +/// proven by a **transposition**. /// -/// **Honest limit, recorded because mutation testing found it:** these -/// two legs do NOT discriminate `rows`/`cols` from their product. -/// Collapsing the key to `rows * cols` leaves both GREEN, because -/// `last_content_cols` co-varies with a column change and the panel's -/// row count co-varies with a resize — the key still moves, by another -/// input. Only a **transposition** (2×6 → 6×2, identical product) would -/// isolate it, and no production path reaches one: rows come from the -/// band's height and columns from the frame declaration, and nothing -/// swaps them. -/// -/// The key hashes them separately anyway. That is cheap and correct, -/// and the alternative — hashing a product because no test can currently -/// tell the difference — would be choosing the weaker construction for -/// the convenience of the test suite. What these legs *do* pin is that -/// each dimension moves the key at all, which is what the rest of the -/// slice depends on. +/// Revision-16's version changed one dimension at a time and could not +/// discriminate: `last_content_cols` co-varies with a column change, so +/// collapsing the key to `rows * cols` stayed green. I recorded that as +/// unwitnessable and claimed no production path reached a +/// same-area transition. **That was wrong** — resize plus redeclare +/// gets there: 4×80 → 8×40 holds the product at 320 while swapping the +/// dimensions, and `last_content_cols` is not refreshed until the next +/// render, so the two grid fields are isolated. #[test] -fn g2_grid_rows_and_columns_are_independent_inputs() { - // ROWS come from the band's own height, not the frame's total rows — - // declaring a shorter frame leaves a 4-row panel a 4-row panel. The - // resize path is the one that actually changes them. +fn g2_a_transposed_grid_moves_the_key_at_an_unchanged_area() { + let mut session = Session::new(); + open_panel(&session, "g2t", 4); + session.declare(1, 24, 80); + let _ = session.present(); + + let before = mapping_generation(&mut session).expect("a key"); + + // 4×80 → 8×40. Same area, different shape, and no render between. + assert!( + session.state.apply_panel_resize_rows(FID, 8), + "the resize must be accepted, or the transposition never happens" + ); + session.declare(2, 24, 40); + + let after = mapping_generation(&mut session).expect("a key"); + assert!( + after > before, + "a transposed grid inverts differently at the same area — a key \ + hashing rows*cols would not notice ({before} → {after})" + ); +} + +/// §5b G3 — **focus** is a stable input. +/// +/// Missing from the first version of G3, which covered only idle and +/// cursor. Focus in particular is the one a naive +/// implementation gets wrong, because the panel frame carries a +/// `focused` flag and it is tempting to fold the whole frame into the +/// key. +#[test] +fn g3_focus_is_a_stable_input() { + let mut session = Session::new(); + open_panel(&session, "g3b", 4); + session.declare(1, 24, 80); + let _ = session.present(); + + let baseline = mapping_generation(&mut session).expect("a key"); + + // Focus. The band's `focused` flag flips; no byte moves. + { + let mut core = session.state.core.borrow_mut(); + let side = core.side_window_for(FID).expect("side"); + core.focus_window(FID, side); + } + assert_eq!( + mapping_generation(&mut session), + Some(baseline), + "focus decides CHROME, never which byte a cell denotes — and a \ + click focuses the panel mid-gesture" + ); + + // Styling is pinned STRUCTURALLY rather than by driving a theme + // change here: `PanelMappingSnapshot` has no style field at all, so + // there is nothing a recolour could touch. The terminal side — where + // a convenient style-bumping counter DOES exist and had to be + // rejected — is pinned in `screen.rs`'s own tests, at the level the + // classification lives. +} + +/// §5b — the snapshot **selects the right domain by target kind**. +/// +/// Added because mutation testing found the branch unwitnessed: routing +/// terminal panels through the DOCUMENT arm — keying them on the +/// buffer's revision, which §5b explicitly rejects — left all thirty-five +/// other rows green. The daemon-level half of the terminal contract is +/// that the branch is taken at all; `screen.rs` owns the half that says +/// the revision it reads classifies events correctly. +#[test] +fn the_mapping_snapshot_picks_the_terminal_domain_for_a_terminal_panel() { + // Document panel → the document domain. { let mut session = Session::new(); - open_panel(&session, "g2rows", 4); + open_panel(&session, "doc", 4); session.declare(1, 24, 80); let _ = session.present(); - - let before = mapping_generation(&mut session).expect("a key"); + let snapshot = session + .state + .panel_mapping_snapshot(FID) + .expect("a presentable document panel"); assert!( - session.state.apply_panel_resize_rows(FID, 6), - "the resize must be accepted, or this leg proves nothing" - ); - let after = mapping_generation(&mut session).expect("a key"); - assert!( - after > before, - "changing grid ROWS alone must move the key — an area product \ - would miss a transposition" + matches!( + snapshot.content(), + pmacs::editor::PanelMappingContent::Document { .. } + ), + "a document panel is keyed on its buffer's content revision" ); } - // COLUMNS come from the declaration. + // Terminal panel → the terminal domain. { let mut session = Session::new(); - open_panel(&session, "g2cols", 4); session.declare(1, 24, 80); - let _ = session.present(); - - let before = mapping_generation(&mut session).expect("a key"); - session.declare(2, 24, 40); - let after = mapping_generation(&mut session).expect("a key"); + exec( + &session.state, + "TERM_BUF = pmacs.terminal.open { command = \"/bin/sh\", \ + args = { \"-c\", \"sleep 30\" }, display = \"panel\" }", + ); + let _ = session.frame(); + let snapshot = session + .state + .panel_mapping_snapshot(FID) + .expect("a presentable terminal panel"); assert!( - after > before, - "changing grid COLUMNS alone must move the key" + matches!( + snapshot.content(), + pmacs::editor::PanelMappingContent::Terminal { .. } + ), + "a terminal panel is keyed on the SCREEN's mapping revision \ + and scroll anchor — its buffer revision tracks something \ + else entirely and would both miss real changes and fire on \ + non-changes" ); } }