From e9872926a1ef34c672b7b6c3c6e2caa8e561136b Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:30:00 +0700 Subject: [PATCH 1/2] fix(screen): stop rescanning the grid for every row a search visits searchable_row asked whether the whole screen was ASCII on every row it built, a pass over every cell, so find, find_all, locate, mask_matching and the regex matches and mask were quadratic in the size of the screen: one find on 1000x1000 took 3 s in a release build where contains took 6 ms. contains was quadratic too once the first non-ASCII cell came late. The question is now asked per cell, where it is cheap because an ASCII cell is NFC already, and a needle is folded on its own, since its folding never depended on the screen. contains keeps its one whole-screen check per query. find on 1000x1000 now takes 5 ms in release and 74 ms in debug. The test times every search against the work it cannot avoid on the same screen, one pass over the grid or building the masked screen, rather than against a clock: the old code ran at 27 to 464 times that, the new at under 2, and the bound is 10. Closes #584 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 8 +++ crates/termlens/src/screen.rs | 50 +++++++++------ crates/termlens/tests/search.rs | 110 +++++++++++++++++++++++++++++++- 3 files changed, 146 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 944b7bb..0b6cce2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,14 @@ reads that marker. also write a character XML 1.0 forbids, a C0 control other than tab, LF and CR or U+FFFE/U+FFFF, as U+FFFD, so a window title holding `\x01` or a cell holding U+FFFF no longer makes an SVG no parser opens. +- `Screen::find`, `find_all`, `locate`, `mask_matching` and, with `regex`, + `matches`, `find_match`, `find_all_matches` and `mask_matches` no longer + take time quadratic in the size of the screen (#584), and neither does + `contains` once the first non-ASCII cell comes late. Every row a search + visited re-read the whole grid to ask whether it was ASCII, so one `find` + on 1000x1000 took 3 s in a release build where `contains` took 6 ms; it + now takes 5 ms. A `wait_until` calling `find`, and `wait_until_matches`, + paid that on every wake-up. ## [0.11.6] - 2026-10-14 diff --git a/crates/termlens/src/screen.rs b/crates/termlens/src/screen.rs index b13bda8..ac4ed71 100644 --- a/crates/termlens/src/screen.rs +++ b/crates/termlens/src/screen.rs @@ -1620,7 +1620,7 @@ impl Screen { visit((0, 0)); return; } - let needle = &self.fold(needle); + let needle = &nfc(needle); if !needle.contains('\n') { for row in 0..self.rows { let (text, cols) = self.searchable_row(row); @@ -1776,7 +1776,7 @@ impl Screen { /// row's width, which is the one thing a mask exists not to do. #[must_use] pub fn mask_matching(&self, pattern: &str, fill: char) -> Screen { - let pattern = self.fold(pattern); + let pattern = nfc(pattern); if pattern.contains('\n') { // A needle that spans rows is matched by the engine `find_all` // uses. The per-row scan below is handed one row at a time and @@ -2027,22 +2027,17 @@ impl Screen { } /// True when every cell holds only ASCII, so normalization is the - /// identity and the search helpers can skip it. This is the shape of - /// virtually every screen, which is what keeps `contains` — evaluated - /// on every wait wake-up — as cheap as it was. + /// identity and `contains` can search [`text`](Self::text) as it is. + /// This is the shape of virtually every screen, which is what keeps + /// `contains` — evaluated on every wait wake-up — as cheap as it was. + /// + /// A pass over every cell, so it is asked once per query and never per + /// row: asked per row it made every search quadratic in the size of the + /// screen, three seconds for one `find` on 1000x1000 (#584). fn is_ascii(&self) -> bool { self.cells.iter().all(|c| c.contents().is_ascii()) } - /// `needle` in the form the search helpers compare against. - fn fold(&self, needle: &str) -> String { - if self.is_ascii() && needle.is_ascii() { - needle.to_owned() - } else { - nfc(needle) - } - } - /// The whole grid in searchable form: rows joined with `\n`, trailing /// whitespace stripped per row, each row folded the same way /// [`searchable_row`](Self::searchable_row) folds it — so `contains` and @@ -2067,8 +2062,12 @@ impl Screen { /// folding cell by cell composes exactly what the terminal draws in one /// cell and can never compose across a cell boundary. It also keeps the /// byte-to-column map exact, which is what [`find`](Self::find) reports. + /// + /// Whether to normalize at all is decided per cell for the same reason + /// it is cheap there: an ASCII cell is NFC already. Deciding it for the + /// whole screen, a pass over every cell, on each row visited is what + /// made every search quadratic (#584). fn searchable_row(&self, row: u16) -> (String, Vec) { - let ascii = self.is_ascii(); let mut text = String::with_capacity(usize::from(self.cols)); let mut cols = Vec::with_capacity(usize::from(self.cols)); for col in 0..self.cols { @@ -2081,10 +2080,8 @@ impl Screen { let before = text.len(); if cell.contents().is_empty() { text.push(' '); - } else if ascii { - text.push_str(cell.contents()); } else { - text.extend(cell.contents().nfc()); + push_nfc(&mut text, cell.contents()); } cols.resize(text.len(), col); debug_assert!(text.len() > before || cell.is_wide_continuation()); @@ -2208,9 +2205,22 @@ impl Screen { } } -/// `s` folded to NFC. +/// `s` folded to NFC, the form a needle and the screen are compared in. fn nfc(s: &str) -> String { - s.nfc().collect() + let mut out = String::with_capacity(s.len()); + push_nfc(&mut out, s); + out +} + +/// Append `s` folded to NFC. ASCII is NFC already — no ASCII character +/// decomposes, or composes with the one after it — so it is copied, and +/// that is the shape of virtually every cell and every needle. +fn push_nfc(out: &mut String, s: &str) { + if s.is_ascii() { + out.push_str(s); + } else { + out.extend(s.nfc()); + } } /// Clamp any range expression to `0..len`, as `(start, end)` exclusive. diff --git a/crates/termlens/tests/search.rs b/crates/termlens/tests/search.rs index 53bbc81..6f0acfc 100644 --- a/crates/termlens/tests/search.rs +++ b/crates/termlens/tests/search.rs @@ -1,9 +1,9 @@ //! `find_all`, the `regex` feature's matches and waits, and the grid-aware //! masks (#264, #245, #250). -use std::time::Duration; +use std::time::{Duration, Instant}; -use termlens::{Key, Terminal}; +use termlens::{Key, Screen, Terminal}; mod common; @@ -370,3 +370,109 @@ fn a_mask_covers_a_needle_that_spans_rows() -> termlens::Result<()> { t.wait_exit()?; Ok(()) } + +/// The fastest of `runs` timings of `search`: a transient stall on a loaded +/// machine has to hit every run to count. +fn fastest(runs: usize, search: impl Fn() -> T) -> Duration { + (0..runs) + .map(|_| { + let start = Instant::now(); + std::hint::black_box(search()); + start.elapsed() + }) + .min() + .expect("at least one run") +} + +/// Every search costs a small multiple of the work it cannot avoid. Each +/// used to re-read the whole grid for every row it visited, asking whether +/// the screen was ASCII, so `find` on 1000x1000 took three seconds where +/// `contains` took six milliseconds — and `contains` too, once the first +/// non-ASCII cell came late (#584). +/// +/// Timed against a reference on the same screen rather than against a +/// clock, so a slow or loaded machine slows both sides: one pass over the +/// grid (`contains` on an ASCII screen) for a search, and building the +/// masked screen (a mask that matches nothing) for a mask, which clones +/// every cell. The rows are what multiplied the cost, so the screen is as +/// tall as a terminal may be. +#[test] +fn every_search_costs_a_small_multiple_of_the_work_it_cannot_avoid() { + const COLS: usize = 200; + const ROWS: usize = 1000; + const BOUND: u32 = 10; + // Each row a run of the alphabet, so `zzz` is nowhere and every search + // reads the whole grid before it gives up. + let grid = |last: Option| { + let mut text = format!("size: {COLS}x{ROWS} cursor: 0,0\n"); + for row in 0..ROWS { + let mut line: String = (0..COLS) + .map(|col| char::from(b'a' + ((row + col) % 26) as u8)) + .collect(); + if let (true, Some(ch)) = (row + 1 == ROWS, last) { + line.pop(); + line.push(ch); + } + text.push_str(&line); + text.push('\n'); + } + Screen::parse(&text).expect("the grid is well formed") + }; + let ascii = grid(None); + let late = grid(Some('\u{e9}')); + assert!(ascii.find("zzz").is_none() && !late.contains("zzz")); + + let pass = fastest(3, || ascii.contains("zzz")); + let build = fastest(3, || ascii.mask_cells(|_| false)); + let within = |name: &str, took: Duration, reference: Duration| { + assert!( + took < reference * BOUND, + "{name} took {took:?}, {:.0} times its reference ({reference:?})", + took.as_secs_f64() / reference.as_secs_f64() + ); + }; + within("find", fastest(2, || ascii.find("zzz")), pass); + within("find_all", fastest(2, || ascii.find_all("zzz")), pass); + within( + "find across rows", + fastest(2, || ascii.find("a\nzzz")), + pass, + ); + within("locate", fastest(2, || ascii.locate("zzz")), pass); + within( + "contains, a non-ASCII needle", + fastest(2, || ascii.contains("z\u{e9}")), + pass, + ); + within( + "contains, the last cell non-ASCII", + fastest(2, || late.contains("zzz")), + pass, + ); + within( + "find, the last cell non-ASCII", + fastest(2, || late.find("zzz")), + pass, + ); + within( + "mask_matching", + fastest(2, || ascii.mask_matching("zzz", '#')), + build, + ); + #[cfg(feature = "regex")] + { + let re = regex::Regex::new("zzz").unwrap(); + within("matches", fastest(2, || ascii.matches(&re)), pass); + within("find_match", fastest(2, || ascii.find_match(&re)), pass); + within( + "find_all_matches", + fastest(2, || ascii.find_all_matches(&re)), + pass, + ); + within( + "mask_matches", + fastest(2, || ascii.mask_matches(&re, '#')), + build, + ); + } +} From c334041d96d41dd27d4a1a3e596ef305dc211591 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:47:41 +0700 Subject: [PATCH 2/2] fix(serde): refuse a cell no termlens could have written Deserialize for Screen checked the grid's shape and the cursor but took every cell as given, so a saved screen whose cell held an escape sequence loaded, and termlens render --text, --ansi and diff wrote it to the reader's terminal: an OSC set the window title, ESC [2J cleared the screen. A cell of three characters, a wide flag in the wrong place, an empty grid and mouse_modes 255 loaded too. Each cell, in the grid and in the styled history, is now held to the rules Screen::parse builds cells by, as far as they can be checked without a width table: no control character, by the text reader's own test; no printable ASCII after the first character; no empty cell marked wide, nor an ASCII one without a U+FE0F; no second column of a wide character that holds text or follows none. A screen of no columns or rows is refused. MouseModes drops the bits no mode occupies, through a hand-written Deserialize in the derive's shape. The JSON is a persisted format, so every check refuses only what no termlens wrote. vt100 0.16 and 0.18 drop control characters before the grid and append only zero-width characters to a cell, the tracker sets four mode bits, and spawn and resize refuse a grid under 2x2. What a writer did produce reads as before: a width the tables of another unicode-width release give differently (0.2.1 to 0.2.2 moved 197 code points), a keycap that U+FE0F makes two columns under vt100 0.18, and a wide character vt100 0.16's resize left in the last column. Closes #582 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 13 ++ crates/termlens-cli/tests/cli.rs | 31 +++ crates/termlens/src/screen.rs | 12 +- crates/termlens/src/screen/parse.rs | 23 ++- crates/termlens/src/screen/serde_impl.rs | 135 +++++++++++- crates/termlens/tests/export.rs | 250 +++++++++++++++++++++++ docs/DESIGN.md | 37 +++- 7 files changed, 484 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b6cce2..853e45e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,19 @@ reads that marker. on 1000x1000 took 3 s in a release build where `contains` took 6 ms; it now takes 5 ms. A `wait_until` calling `find`, and `wait_until_matches`, paid that on every wake-up. +- The JSON reader refuses a cell no termlens could have written (#582). A + saved screen whose cell held an escape sequence loaded, and `termlens + render --text`, `--ansi` and `diff` then wrote it to the terminal: an OSC + set the window title and `ESC [2J` cleared the screen. Refused now, in + the grid and the styled history, naming the cell: a control character + (C0, DEL or C1, as `Screen::parse` refuses it), printable ASCII after a + cell's first character, an empty cell marked wide or an ASCII one with + no U+FE0F after it, and a wide character's second column that holds text + or follows none; and a screen of no columns or rows. A tracking-mode set + keeps only its four mode bits, so `MouseModes::len` and `iter` agree. No + check refuses a file a termlens wrote: a keycap such as `1️⃣`, a cell + whose width only this build's width tables dispute, and a wide character + that vt100 0.16 left in the last column after a resize all still read. ## [0.11.6] - 2026-10-14 diff --git a/crates/termlens-cli/tests/cli.rs b/crates/termlens-cli/tests/cli.rs index b332da8..9ed788d 100644 --- a/crates/termlens-cli/tests/cli.rs +++ b/crates/termlens-cli/tests/cli.rs @@ -1214,6 +1214,37 @@ fn render_and_diff_read_a_screen_from_stdin() -> termlens::Result<()> { Ok(()) } +/// A saved screen whose cell carried an escape sequence drove the terminal +/// of whoever rendered it: `render --text` wrote the OSC that set the window +/// title and the `ESC [2J` that cleared the screen straight to stdout, and +/// then refused its own output as input (#582). The reader now refuses the +/// file, so nothing reaches stdout and the tool exits 2 naming the cell. +#[test] +fn a_saved_screen_cannot_drive_the_terminal_that_renders_it() { + let screen = Screen::parse("size: 8x1 cursor: 0,0\nhi").expect("a saved screen"); + let mut value = serde_json::to_value(&screen).expect("serializes"); + value["cells"][0][1]["contents"] = "\u{1b}]0;PWNED\u{7}\u{1b}[2Jh".into(); + let document = value.to_string(); + let before = data("before.snap"); + for args in [ + &["render", "--text", "-"][..], + &["render", "--ansi", "-"], + &["diff", "--color", "never", "-", &before], + ] { + let out = with_stdin(args, &document); + assert_eq!(out.status.code(), Some(2), "{args:?}: {out:?}"); + assert!( + !out.stdout.contains(&0x1b), + "{args:?} wrote an escape: {out:?}" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("cell 0,1") && stderr.contains("control character"), + "{args:?}: {stderr}" + ); + } +} + /// Standard input is read once, so it can be one of `diff`'s operands and /// not both — said plainly rather than left to look like an empty screen. #[test] diff --git a/crates/termlens/src/screen.rs b/crates/termlens/src/screen.rs index ac4ed71..f1e4048 100644 --- a/crates/termlens/src/screen.rs +++ b/crates/termlens/src/screen.rs @@ -191,12 +191,20 @@ impl MouseMode { /// # } /// ``` #[derive(Clone, Copy, PartialEq, Eq, Hash, Default)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +// `Deserialize` is written by hand in `serde_impl.rs`, to drop the bits no +// mode occupies (#582). +#[cfg_attr(feature = "serde", derive(serde::Serialize))] pub struct MouseModes(u8); impl MouseModes { + /// The set holding the modes whose bits are set in `bits`. A bit no mode + /// occupies is dropped: kept, it counted in `len()` and never appeared + /// in `iter()`, and a saved screen can carry one (#582). pub(crate) fn from_bits(bits: u8) -> Self { - Self(bits) + let known = MouseMode::TRACKING + .iter() + .fold(0, |known, mode| known | mode.bit()); + Self(bits & known) } /// Whether the application currently has `mode` enabled. diff --git a/crates/termlens/src/screen/parse.rs b/crates/termlens/src/screen/parse.rs index 05116aa..ffb9112 100644 --- a/crates/termlens/src/screen/parse.rs +++ b/crates/termlens/src/screen/parse.rs @@ -167,6 +167,19 @@ fn parse_header(line: &str) -> Result<(u16, u16, (u16, u16, bool))> { Ok((cols, rows, cursor)) } +/// The columns `ch` takes in a cell, or why no saved screen holds it. +/// +/// `unicode_width` answers `None` for a control character — C0, DEL and C1 +/// — and `Some(0)` for a zero-width mark. Reading them alike folded an +/// `ESC` into the cell before it, and the snapshot renderings then carried +/// the raw byte out (#376). The JSON reader asks this of every character in +/// every cell, so the two readers refuse the same ones (#582). +pub(super) fn cell_width(ch: char) -> std::result::Result { + ch.width().ok_or_else(|| { + format!("{ch:?} is a control character; the snapshot format never writes one") + }) +} + /// One grid line into `row`, a slice of exactly `cols` cells. A zero-width /// combining mark joins the cell before it; a wide character takes two; a /// control character is an error. @@ -194,15 +207,7 @@ fn lay_out_row(line: &str, number: usize, cols: u16, row: &mut [Cell], widen: bo }; let mut col = 0usize; for ch in line.chars() { - // `unicode_width` answers `None` for a control character and - // `Some(0)` for a zero-width mark. Reading them alike folded an - // `ESC` into the cell before it, and the snapshot renderings then - // carried the raw byte out (#376). - let Some(width) = ch.width() else { - return Err(Error::Parse(format!( - "line {number}: {ch:?} is a control character; the snapshot format never writes one" - ))); - }; + let width = cell_width(ch).map_err(|why| Error::Parse(format!("line {number}: {why}")))?; if width == 0 { // The cell that owns the glyph before this mark. A wide // character advanced the column by two, so one step back lands diff --git a/crates/termlens/src/screen/serde_impl.rs b/crates/termlens/src/screen/serde_impl.rs index cafec1f..de27a34 100644 --- a/crates/termlens/src/screen/serde_impl.rs +++ b/crates/termlens/src/screen/serde_impl.rs @@ -6,7 +6,10 @@ //! `from_parts` only debug-asserts — every row holds exactly `cols` cells //! and there are exactly `rows` of them — through a validating constructor //! rather than a bare derive, so a hand-edited or truncated file is an -//! error and never a screen that panics on its first `cell()`. +//! error and never a screen that panics on its first `cell()`. Each cell is +//! held to what a termlens could have written into one (#582): the CLI +//! prints what it reads, and a cell carrying `ESC` drove the terminal of +//! whoever ran `termlens render --text` on the file. //! //! The JSON is a persisted artifact, not a wire between two copies of one //! version: `termlens diff` and `termlens render` read it back as a saved @@ -20,7 +23,7 @@ use serde::de::Error as _; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use super::{Cell, Screen, TermState}; +use super::{parse, Cell, MouseModes, Screen, TermState}; use crate::terminal::MAX_DIMENSION; /// The cursor as three named fields rather than a tuple, so the JSON reads. @@ -100,6 +103,16 @@ impl<'de> Deserialize<'de> for Screen { wire.cols, wire.rows ))); } + // `spawn` and `resize` refuse a terminal under 2x2 and the text + // reader a screen under 1x1, so no termlens made an empty one. Read + // anyway, one of no columns re-serialised as a document of no rows, + // which this reader then refused. + if wire.cols == 0 || wire.rows == 0 { + return Err(D::Error::custom(format!( + "a screen has at least one column and one row, got {}x{}", + wire.cols, wire.rows + ))); + } if wire.cells.len() != usize::from(wire.rows) { return Err(D::Error::custom(format!( "a {}x{} screen needs {} rows of cells, not {}", @@ -121,6 +134,23 @@ impl<'de> Deserialize<'de> for Screen { cells.len() ))); } + for (row, cells) in wire.cells.iter().enumerate() { + check_row(cells) + .map_err(|(col, why)| D::Error::custom(format!("cell {row},{col}: {why}")))?; + } + // History rows keep the width they were captured at, so they are + // checked as rows of their own. + for (row, cells) in wire + .state + .scrollback_cells + .iter() + .flat_map(|h| h.iter()) + .enumerate() + { + check_row(cells).map_err(|(col, why)| { + D::Error::custom(format!("scrollback cell {row},{col}: {why}")) + })?; + } // `col == cols` is the pending-wrap position a terminal parks at // after a write fills the last row. Every release up to 0.11.1 wrote // it (#401 stops the emulator producing it), so refusing it here @@ -129,12 +159,12 @@ impl<'de> Deserialize<'de> for Screen { // instead, which is where #401 now reports it, so such a file also // round-trips through `render --text` (#375). Any other position off // the grid is a file nothing wrote, and is refused. - let cursor_col = if wire.cols > 0 && wire.cursor.col == wire.cols { + let cursor_col = if wire.cursor.col == wire.cols { wire.cols - 1 } else { wire.cursor.col }; - if wire.cursor.row >= wire.rows.max(1) || cursor_col >= wire.cols.max(1) { + if wire.cursor.row >= wire.rows || cursor_col >= wire.cols { return Err(D::Error::custom(format!( "cursor {},{} is outside a {}x{} screen", wire.cursor.row, wire.cursor.col, wire.cols, wire.rows @@ -151,3 +181,100 @@ impl<'de> Deserialize<'de> for Screen { )) } } + +/// Only the four tracking modes, whatever the file says (#582). The derive +/// took any byte, and a stray bit made `len()` count a mode `iter()` never +/// yields: 8 against 4 for `255`. No termlens wrote one — the tracker sets +/// exactly those four bits — so masking drops nothing a writer recorded, +/// and unlike a refusal it still reads a file whose writer knew a fifth. +impl<'de> Deserialize<'de> for MouseModes { + fn deserialize>(deserializer: D) -> Result { + // The derive's own shape, a newtype named `MouseModes` over the + // bits, so no serde format reads a different document than before. + #[derive(Deserialize)] + #[serde(rename = "MouseModes")] + struct Bits(u8); + let Bits(bits) = Bits::deserialize(deserializer)?; + Ok(MouseModes::from_bits(bits)) + } +} + +/// Whether `cells`, a row of the grid or of the history, could have come +/// from a termlens; `Err` names the first column that could not, and why. +/// +/// These are the rules `Screen::parse` builds its cells by, less every one +/// a width table decides (#582). Whether a character is a mark, one column +/// or two is an entry in `unicode_width`'s tables, and the tables move +/// between releases — 0.2.1 to 0.2.2, both of which vt100 0.16 accepted, +/// moved 197 code points — so a cell another release wrote correctly can +/// look malformed under this one's. Two facts are no table's: a control +/// character has no width, and printable ASCII is one column. Checks built +/// from those, and from the wide flags, refuse only what no termlens wrote. +/// +/// One rule of the text reader's is left out on purpose: a wide character +/// need not be followed by its second column. vt100 0.16, behind every +/// release up to 0.11.6, narrowed a row by truncating it, so a resize that +/// cut a wide character left it in the last column, and widening again +/// left it before a blank. +fn check_row(cells: &[Cell]) -> Result<(), (usize, String)> { + for (col, cell) in cells.iter().enumerate() { + check_cell(cell).map_err(|why| (col, why))?; + // vt100 clears or moves both halves of a wide character together, + // and a resize cuts a row from the right, so it can strand a first + // column but never a second. + let after_wide = col.checked_sub(1).is_some_and(|before| cells[before].wide); + if cell.wide_continuation && !after_wide { + return Err(( + col, + "a wide character's second column with no wide character before it".into(), + )); + } + } + Ok(()) +} + +/// One cell's contents and flags, by the rules [`check_row`] explains. +fn check_cell(cell: &Cell) -> Result<(), String> { + for ch in cell.contents.chars() { + // A control character, asked the way the text reader asks. No + // termlens wrote one into a cell, since vt100 drops them before the + // grid, and `render --text` and `diff` would print it to a terminal. + parse::cell_width(ch)?; + } + let mut chars = cell.contents.chars(); + let Some(first) = chars.next() else { + return if cell.wide { + Err("marked wide but holds no character".into()) + } else { + Ok(()) + }; + }; + if cell.wide_continuation { + return Err(format!( + "a wide character's second column holding {:?}", + cell.contents + )); + } + // vt100 marks a cell wide for its first character's own width, which + // printable ASCII never has, or, from 0.18, for the whole cell's once a + // U+FE0F asks for emoji presentation: a table makes the keycaps `1️⃣` + // and `#️⃣` two columns. + if cell.wide && one_column(first) && !cell.contents.contains('\u{fe0f}') { + return Err(format!("{first:?} is one column wide but marked wide")); + } + // vt100 appends to a cell only what its table calls zero-width, which + // printable ASCII never is. + if chars.any(one_column) { + return Err(format!( + "{:?} is more than one character; a cell holds one and the zero-width marks after it", + cell.contents + )); + } + Ok(()) +} + +/// Printable ASCII: the characters every `unicode_width` release makes one +/// column wide in code, ahead of its tables. +fn one_column(ch: char) -> bool { + matches!(ch, ' '..='~') +} diff --git a/crates/termlens/tests/export.rs b/crates/termlens/tests/export.rs index a159949..71ce9f2 100644 --- a/crates/termlens/tests/export.rs +++ b/crates/termlens/tests/export.rs @@ -520,6 +520,256 @@ mod json { last = at; } } + + /// A one-row screen to edit as JSON: `ab`, the wide `東` in columns 2 + /// and 3, then two blanks. + fn document() -> serde_json::Value { + let screen = Screen::parse("size: 6x1 cursor: 0,0\nab\u{6771}").expect("a 6x1 screen"); + serde_json::to_value(&screen).expect("serializes") + } + + /// [`document`] with `edit` applied, read back. + fn read_edited(edit: impl FnOnce(&mut serde_json::Value)) -> Result { + let mut value = document(); + edit(&mut value); + serde_json::from_value(value).map_err(|e| e.to_string()) + } + + /// A cell carrying a control character drove the terminal of whoever + /// rendered the file: `render --text` and `diff` printed it, so an OSC + /// set the window title and `ESC [2J` cleared the screen (#582). vt100 + /// drops every control character before the grid, so no termlens wrote + /// one, and the reader refuses it in the text reader's words (#376). + #[test] + fn a_control_character_in_a_cell_is_refused() { + for contents in [ + "\u{1b}]0;PWNED\u{7}\u{1b}[2Jh", + "\n", + "\0", + "a\u{7}", + "\u{7f}", + "\u{9b}2J", + ] { + let control = contents.chars().find(|ch| ch.is_control()).unwrap(); + let err = read_edited(|v| v["cells"][0][1]["contents"] = contents.into()) + .expect_err(contents); + assert!( + err.contains(&format!("cell 0,1: {control:?} is a control character")), + "{contents:?}: {err}" + ); + } + // The styled history holds cells too, in rows of their own width. + let err = read_edited(|v| { + let mut cell = v["cells"][0][0].clone(); + cell["contents"] = "\u{1b}[2J".into(); + v["state"]["scrollback"] = serde_json::json!(["x"]); + v["state"]["scrollback_cells"] = serde_json::json!([[cell]]); + }) + .expect_err("a control character in the history"); + assert!( + err.contains("scrollback cell 0,0: '\\u{1b}' is a control character"), + "{err}" + ); + } + + /// A cell is one character and the zero-width marks after it. vt100 + /// appends to a cell only what its width table calls zero-width, and + /// printable ASCII is one column in every table, so a cell holding a + /// second ASCII character is one no termlens wrote (#582). It rendered + /// wider than its column and pushed the rest of the row out of line. + #[test] + fn a_cell_of_more_than_one_character_is_refused() { + for contents in ["abc", "a ", "\u{e9}1", "\u{6771}x"] { + let err = read_edited(|v| v["cells"][0][0]["contents"] = contents.into()) + .expect_err(contents); + assert!( + err.contains(&format!( + "cell 0,0: {contents:?} is more than one character" + )), + "{err}" + ); + } + } + + /// The wide flags no grid holds (#582). vt100 marks a cell wide only + /// for a character its table makes two columns, which printable ASCII + /// never is, and never an empty one; it clears a cell before making it + /// a second column; and it clears or moves the two halves of a wide + /// character together, while a resize cuts a row from the right — so a + /// first column can be stranded, never a second. + #[test] + fn a_wide_flag_no_grid_holds_is_refused() { + let refused = |expected: &str, edit: fn(&mut serde_json::Value)| { + let err = read_edited(edit).expect_err(expected); + assert!(err.contains(expected), "{expected}: {err}"); + }; + refused("cell 0,0: 'a' is one column wide but marked wide", |v| { + v["cells"][0][0]["wide"] = true.into(); + v["cells"][0][1]["contents"] = "".into(); + v["cells"][0][1]["wide_continuation"] = true.into(); + }); + refused("cell 0,4: marked wide but holds no character", |v| { + v["cells"][0][4]["wide"] = true.into(); + }); + refused("cell 0,4: marked wide but holds no character", |v| { + v["cells"][0][4]["wide"] = true.into(); + v["cells"][0][4]["wide_continuation"] = true.into(); + }); + refused( + "cell 0,3: a wide character's second column holding \"x\"", + |v| v["cells"][0][3]["contents"] = "x".into(), + ); + let orphan = "a wide character's second column with no wide character before it"; + refused(&format!("cell 0,0: {orphan}"), |v| { + v["cells"][0][0]["contents"] = "".into(); + v["cells"][0][0]["wide_continuation"] = true.into(); + }); + refused(&format!("cell 0,5: {orphan}"), |v| { + v["cells"][0][5]["wide_continuation"] = true.into(); + }); + } + + /// A terminal is at least 2x2 and the text reader refuses a screen + /// under 1x1, so no termlens wrote an empty one. Read anyway, a screen + /// of no columns re-serialised as a document of no rows, which the + /// reader then refused (#582). + #[test] + fn a_screen_of_no_columns_or_no_rows_is_refused() { + for (cols, rows, cells) in [ + (0, 1, serde_json::json!([[]])), + (6, 0, serde_json::json!([])), + ] { + let err = read_edited(|v| { + v["cols"] = cols.into(); + v["rows"] = rows.into(); + v["cells"] = cells; + }) + .expect_err("an empty screen"); + assert!( + err.contains(&format!( + "at least one column and one row, got {cols}x{rows}" + )), + "{err}" + ); + } + } + + /// Four tracking modes have a bit each, and the derive read any byte: + /// `255` made `len()` say 8 while `iter()` yielded 4 (#582). The tracker + /// never set another bit, so the reader drops them rather than refuse + /// the file. + #[test] + fn a_mouse_mode_bit_no_mode_occupies_is_dropped() { + let screen = + read_edited(|v| v["state"]["mouse_modes"] = 255.into()).expect("the file reads"); + let modes = screen.mouse_modes(); + assert_eq!((modes.len(), modes.iter().count()), (4, 4), "{modes:?}"); + assert_eq!( + serde_json::to_value(&screen).unwrap()["state"]["mouse_modes"], + 15 + ); + let alone: termlens::MouseModes = + serde_json::from_str("240").expect("a set on its own reads"); + assert!(alone.is_empty(), "{alone:?}"); + } + + /// What the reader must go on reading (#582): cells a termlens wrote + /// that this build would not have. A width is a table entry, and the + /// tables move between unicode-width releases — U+11A3A was a mark in + /// 0.2.1 and is one column in 0.2.2, U+1ACF went the other way, U+16FF2 + /// from one column to two — so each is written here both ways. So is an + /// emoji-presentation sequence: `⚠️` one column, as vt100 0.16 wrote it, + /// and two, as 0.18 does, with a keycap whose first character is an + /// ASCII digit. U+FFFD, which termlens writes for a byte it could not + /// decode; the space vt100 puts under a mark that lands on an empty + /// cell; and a wide character a resize cut, which vt100 0.16 left in + /// the last column, and before a blank once the grid widened again. + #[test] + fn a_cell_an_earlier_release_wrote_still_reads() { + let reads = |edit: fn(&mut serde_json::Value)| { + let mut value = document(); + edit(&mut value); + let screen: Screen = serde_json::from_value(value.clone()) + .unwrap_or_else(|e| panic!("{e}: {}", value["cells"])); + assert_eq!( + serde_json::to_value(&screen).unwrap(), + value, + "and it writes back unchanged" + ); + }; + reads(|v| v["cells"][0][0]["contents"] = "a\u{11a3a}".into()); + reads(|v| v["cells"][0][4]["contents"] = "\u{11a3a}".into()); + reads(|v| v["cells"][0][0]["contents"] = "a\u{1acf}".into()); + reads(|v| v["cells"][0][4]["contents"] = "\u{1acf}".into()); + reads(|v| v["cells"][0][4]["contents"] = "\u{16ff2}".into()); + reads(|v| { + v["cells"][0][4]["contents"] = "\u{16ff2}".into(); + v["cells"][0][4]["wide"] = true.into(); + v["cells"][0][5]["wide_continuation"] = true.into(); + }); + reads(|v| { + v["cells"][0][4]["contents"] = "1\u{fe0f}\u{20e3}".into(); + v["cells"][0][4]["wide"] = true.into(); + v["cells"][0][5]["wide_continuation"] = true.into(); + }); + reads(|v| v["cells"][0][4]["contents"] = "\u{26a0}\u{fe0f}".into()); + reads(|v| { + v["cells"][0][4]["contents"] = "\u{26a0}\u{fe0f}".into(); + v["cells"][0][4]["wide"] = true.into(); + v["cells"][0][5]["wide_continuation"] = true.into(); + }); + reads(|v| v["cells"][0][4]["contents"] = "\u{fffd}".into()); + reads(|v| v["cells"][0][4]["contents"] = " \u{301}".into()); + reads(|v| { + v["cells"][0][5]["contents"] = "\u{6771}".into(); + v["cells"][0][5]["wide"] = true.into(); + }); + reads(|v| v["cells"][0][3]["wide_continuation"] = false.into()); + } + + /// The cells a live terminal writes read back as the same screen + /// (#582): combining marks, wide characters, a joiner, emoji that U+FE0F + /// makes two columns — among them a keycap, whose first character is an + /// ASCII digit — the space vt100 puts under a mark that lands on an + /// empty cell, and whatever a resize through a wide character leaves. + #[test] + #[cfg_attr( + windows, + ignore = "ConPTY re-renders wide characters into columns of its own choosing (#149)" + )] + fn every_cell_a_live_terminal_writes_reads_back() -> termlens::Result<()> { + // Eighteen columns: `e\u{301}`, `漢`, `☺️`, `👨\u{200d}` and `👩`, then + // the keycap `1️⃣` in 13 and 14 and `#️` in 16 and 17; then a mark on + // the empty cell (1,3). + let text = "e\u{301} \u{6f22} \u{263a}\u{fe0f} \u{1f468}\u{200d}\u{1f469} \ + 1\u{fe0f}\u{20e3} #\u{fe0f}\\e[2;5H\u{301}\\r\\nREADY"; + let mut t = emit(&["--raw", text, "--wait"])?; + t.wait_until(|s| s.contains("READY"))?; + let read_back = |screen: &Screen| -> Screen { + let json = serde_json::to_string(screen).expect("serializes"); + serde_json::from_str(&json).unwrap_or_else(|e| panic!("{e}:\n{screen}")) + }; + + let screen = t.screen(); + assert_eq!(screen.cell(0, 0).unwrap().contents(), "e\u{301}"); + let keycap = screen.cell(0, 13).unwrap(); + assert_eq!(keycap.contents(), "1\u{fe0f}\u{20e3}"); + assert!(keycap.is_wide(), "{screen:?}"); + assert_eq!(screen.cell(1, 3).unwrap().contents(), " \u{301}"); + assert_eq!(read_back(&screen), screen); + + // Through `#️`, and back. + t.resize(17, 4)?; + let cut = t.screen(); + assert_eq!(read_back(&cut), cut); + t.resize(30, 4)?; + let widened = t.screen(); + assert_eq!(read_back(&widened), widened); + + t.send(Key::Enter)?; + t.wait_exit()?; + Ok(()) + } } /// A grid can hold the words a styles block is made of. Reading them as diff --git a/docs/DESIGN.md b/docs/DESIGN.md index d42e737..5b5694f 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -868,7 +868,8 @@ is specified here next to the text format (#329). Format **1**: refuses the file and names both numbers rather than reading as far as the fields line up. A new number is a new stability candidate or a major version, never a silent change. -- `cols`, `rows` — the geometry, in the terminal's order. +- `cols`, `rows` — the geometry, in the terminal's order: at least 1 and + at most 1000 each, as a terminal's is (#582, #583). - `cursor` — three named fields. A hidden cursor keeps its position here (`"visible": false`), unlike the text format, where `cursor: hidden` drops it; `Screen::diff` treats a hidden cursor's position as not part @@ -889,7 +890,9 @@ is specified here next to the text format (#329). Format **1**: protocol and the tracking-mode set, the clipboard, the counters, the cursor style, the links, the graphics record, the scrollback (text, and cells when retained), the wrap flags, insert mode, the unsupported - sequences with their overflow count, the visual bells. + sequences with their overflow count, the visual bells. The tracking-mode + set keeps its four mode bits and drops any other, which no termlens + wrote and which `len()` counted while `iter()` never yielded it. Nothing is omitted that a `Screen` holds, which is why reading the JSON back gives a `Screen` that is `==` to the original where the text format @@ -897,6 +900,36 @@ gives one that only `diff`s empty. Every row must hold `cols` cells and there must be `rows` of them; a file that does not hold together is an error, never a screen that panics on its first `cell()`. +A cell must also be one a termlens could have written, because the CLI +prints what it reads: a cell carrying an escape sequence drove the +terminal of whoever ran `render --text` or `diff` on the file (#582). +Refused, naming the cell, in the grid and in the retained history alike: + +- a control character — C0, DEL or C1 — anywhere in a cell, by the test + `Screen::parse` applies to a line (#376). vt100 drops every one before + it reaches the grid; +- printable ASCII after a cell's first character. A cell is one character + and the zero-width marks after it, and no width table makes ASCII + zero-width; +- an empty cell marked `wide`, or one whose first character is ASCII + with no U+FE0F after it. vt100 sizes a cell by its first character, or, + from 0.18, by the whole cell once a U+FE0F asks for emoji presentation, + which makes a keycap such as `1️⃣` two columns; +- a `wide_continuation` cell that holds text, or that does not follow a + `wide` one. + +Two kinds of cell `Screen::parse` would never build are read as written, +because a termlens did write them. Whether a non-ASCII character is a +mark, one column or two is an entry in `unicode-width`'s tables, and the +tables move between releases — 0.2.1 to 0.2.2, both of which vt100 0.16 +accepted, moved 197 code points — so a cell another release wrote +correctly can look malformed under this one's; control characters and +printable ASCII are the only widths no release reads from a table. And a +`wide` cell need not be followed by its continuation: vt100 0.16, behind +every release up to 0.11.6, narrowed a row by truncating it, so a resize +that cut a wide character left it in the last column, and widening again +left it before a blank. + The compatibility corpus under `crates/termlens/tests/compat/` holds both formats as written by each release since 0.10.1, and `tests/compat.rs` holds every later release to reading them (#327).