diff --git a/crates/nyaterm-terminal-gpui/src/element/layout_cache_tests.rs b/crates/nyaterm-terminal-gpui/src/element/layout_cache_tests.rs index 338f5087..af540471 100644 --- a/crates/nyaterm-terminal-gpui/src/element/layout_cache_tests.rs +++ b/crates/nyaterm-terminal-gpui/src/element/layout_cache_tests.rs @@ -21,10 +21,13 @@ use super::{ terminal_visible_rows_for_bounds, terminal_visible_rows_for_clipped_bounds, }; use crate::keywords::{ - compile_terminal_keyword_highlighter, precompute_terminal_keyword_highlights, - terminal_keyword_row_reuse_keys, terminal_keyword_rules_key, + TerminalKeywordHighlightLookup, compile_terminal_keyword_highlighter, + precompute_terminal_keyword_highlights, terminal_keyword_row_reuse_keys, + terminal_keyword_rules_key, +}; +use crate::paint::{ + apply_action_link_ranges, flatten_highlight_spans, terminal_highlight_spans_with_keyword_ranges, }; -use crate::paint::{apply_action_link_ranges, flatten_highlight_spans}; use crate::types::{TerminalHighlightSpan, TerminalPaintGeometry}; fn edit_snapshot_row( @@ -1151,6 +1154,104 @@ fn precomputed_keyword_match_does_not_reuse_plain_pending_row() { assert!(pending_key.is_none()); } +#[test] +fn stale_keyword_prefix_is_painted_and_not_reused_as_the_final_parse() { + let mut screen = TerminalScreen::new(40, 3); + screen.advance(b"# ps -ef"); + let original = screen.snapshot(); + let rules = vec![ResolvedKeywordHighlightRule { + id: "options".into(), + name: "Options".into(), + patterns: vec!["-[a-z]+".into()], + color: "#ff2244".into(), + enabled: true, + }]; + let highlighter = compile_terminal_keyword_highlighter(&rules); + let palette = nyaterm_ui::theme_palette("github-dark"); + let old = Arc::new(precompute_terminal_keyword_highlights( + &original, + &highlighter, + palette, + None, + )); + screen.advance(b" -aux"); + let changed = Arc::new(screen.snapshot()); + let current = Arc::new(precompute_terminal_keyword_highlights( + &changed, + &highlighter, + palette, + Some(&old), + )); + let make_element = || { + NyaTerminalElement::new( + changed.clone(), + Arc::new(Vec::new()), + Vec::new(), + false, + "block", + 8.0, + 16.0, + palette, + "monospace".to_string(), + 14.0, + 400.0, + 700.0, + ) + }; + let plain = make_element(); + let stale = make_element().with_keyword_highlights(old.clone()); + let parsed = make_element().with_keyword_highlights(current.clone()); + let keyword_key = parsed.paint_style_key(current.rules_key()); + let empty_key = parsed.paint_style_key(0); + let reuse_key = terminal_keyword_row_reuse_keys(&changed)[0]; + let keys = |element: &NyaTerminalElement| { + element.row_layout_cache_keys(0, keyword_key, empty_key, reuse_key) + }; + assert_ne!(keys(&plain).0, keys(&stale).0); + assert_ne!(keys(&stale).0, keys(&parsed).0); + assert!(keys(&stale).1.is_none()); + assert!(keys(&parsed).1.is_none()); + + let row = changed.row(0).unwrap(); + let lookup = old.stale_lookup(0, &changed).unwrap(); + let spans = terminal_highlight_spans_with_keyword_ranges( + &row.text, + Some(&row.styled_spans), + lookup.ranges(), + &[], + palette, + ); + let cells = flatten_highlight_spans(spans); + assert!(cells[5..8].iter().all(|cell| cell.color == Some(0xff2244))); + assert!(cells[9..13].iter().all(|cell| !cell.keyword)); + assert_eq!( + current.lookup(0, &changed).unwrap().ranges().unwrap().len(), + 2 + ); +} + +#[test] +fn stale_keyword_cache_key_tracks_the_retained_ranges() { + let ranges = [ + crate::types::TerminalKeywordRange { + start_col: 5, + end_col: 8, + color: 0xff2244, + }, + crate::types::TerminalKeywordRange { + start_col: 9, + end_col: 13, + color: 0xff2244, + }, + ]; + let first = TerminalKeywordHighlightLookup::Stale(&ranges[..1]); + let second = TerminalKeywordHighlightLookup::Stale(&ranges); + assert_ne!( + super::terminal_keyword_row_paint_style_key(41, 0, Some(&first)), + super::terminal_keyword_row_paint_style_key(41, 0, Some(&second)), + ); +} + #[test] fn row_layout_key_ignores_keyword_rules_for_known_empty_keyword_rows() { let mut snapshot = TerminalScreen::default().snapshot(); diff --git a/crates/nyaterm-terminal-gpui/src/element/mod.rs b/crates/nyaterm-terminal-gpui/src/element/mod.rs index d4f34548..eaf7562c 100644 --- a/crates/nyaterm-terminal-gpui/src/element/mod.rs +++ b/crates/nyaterm-terminal-gpui/src/element/mod.rs @@ -15,9 +15,9 @@ use nyaterm_terminal::{ }; use crate::keywords::{ - CompiledKeywordRule, CompiledKeywordRules, TerminalKeywordHighlightSnapshot, - TerminalKeywordRowReuseKey, compile_keyword_rules, terminal_keyword_row_reuse_key, - terminal_keyword_row_reuse_keys, terminal_keyword_rules_key, + CompiledKeywordRule, CompiledKeywordRules, TerminalKeywordHighlightLookup, + TerminalKeywordHighlightSnapshot, TerminalKeywordRowReuseKey, compile_keyword_rules, + terminal_keyword_row_reuse_key, terminal_keyword_row_reuse_keys, terminal_keyword_rules_key, }; use crate::paint::{ apply_search_ranges, flush_bg, line_strike_color, push_col_range_bg, terminal_cell_text_at_col, @@ -675,11 +675,11 @@ impl NyaTerminalElement { .as_ref() .and_then(|lookup| lookup.ranges()) .is_some(); - let paint_style_key = if keyword_result_known_empty { - empty_keyword_paint_style_key - } else { - keyword_paint_style_key - }; + let paint_style_key = terminal_keyword_row_paint_style_key( + keyword_paint_style_key, + empty_keyword_paint_style_key, + keyword_lookup.as_ref(), + ); let default_decorations; let decorations = if let Some(decorations) = self.decorations.get(row) { decorations @@ -709,7 +709,9 @@ impl NyaTerminalElement { ) }; let row_key = row_layout_key(paint_style_key, keyword_spans_present); - let pending_keyword_row_is_equivalent = keyword_lookup.is_some() + let pending_keyword_row_is_equivalent = keyword_lookup + .as_ref() + .is_some_and(|lookup| !lookup.is_stale()) && (keyword_result_known_empty || !self.keyword_rules.is_empty()); let pending_keyword_row_key = pending_keyword_row_is_equivalent .then(|| row_layout_key(keyword_paint_style_key, false)) @@ -889,6 +891,30 @@ fn terminal_text_run_for_span( } } +fn terminal_keyword_row_paint_style_key( + keyword_key: u64, + empty_key: u64, + lookup: Option<&TerminalKeywordHighlightLookup<'_>>, +) -> u64 { + match lookup { + Some(lookup) if lookup.is_known_empty() => empty_key, + Some(TerminalKeywordHighlightLookup::Stale(ranges)) => { + // A provisional prefix is not equivalent to the final parse, and + // successive provisional snapshots may retain different ranges. + let mut hasher = DefaultHasher::new(); + "terminal-stale-keyword-prefix".hash(&mut hasher); + keyword_key.hash(&mut hasher); + for range in *ranges { + range.start_col.hash(&mut hasher); + range.end_col.hash(&mut hasher); + range.color.hash(&mut hasher); + } + hasher.finish() + } + _ => keyword_key, + } +} + #[cfg(test)] fn terminal_effective_keyword_rules_key(keyword_rules_key: u64, known_empty: bool) -> u64 { if known_empty { 0 } else { keyword_rules_key } @@ -1415,11 +1441,11 @@ impl Element for NyaTerminalElement { .is_some_and(|lookup| lookup.is_known_empty()); let keyword_ranges = keyword_lookup.as_ref().and_then(|lookup| lookup.ranges()); let keyword_spans_present = keyword_ranges.is_some(); - let row_paint_style_key = if keyword_result_known_empty { - empty_keyword_paint_style_key - } else { - keyword_paint_style_key - }; + let row_paint_style_key = terminal_keyword_row_paint_style_key( + keyword_paint_style_key, + empty_keyword_paint_style_key, + keyword_lookup.as_ref(), + ); let default_decorations; let decorations = if let Some(decorations) = self.decorations.get(row) { decorations @@ -1486,7 +1512,9 @@ impl Element for NyaTerminalElement { // Reuse a pending row only when its paint is equivalent to the parsed result. // TerminalSurface intentionally omits synchronous rules, so a matching result // there must rebuild instead of promoting the cached plain row as highlighted. - let pending_keyword_row_is_equivalent = keyword_lookup.is_some() + let pending_keyword_row_is_equivalent = keyword_lookup + .as_ref() + .is_some_and(|lookup| !lookup.is_stale()) && (keyword_result_known_empty || !self.keyword_rules.is_empty()); let pending_keyword_row_key = pending_keyword_row_is_equivalent .then(|| row_layout_key(keyword_paint_style_key, false)) @@ -1538,7 +1566,7 @@ impl Element for NyaTerminalElement { terminal_highlight_spans_with_keyword_ranges( display_line, ansi, - Some(ranges.as_ref()), + Some(ranges), &keyword_excluded_ranges, self.palette, ) diff --git a/crates/nyaterm-terminal-gpui/src/keywords.rs b/crates/nyaterm-terminal-gpui/src/keywords.rs index 262e3368..8c51554c 100644 --- a/crates/nyaterm-terminal-gpui/src/keywords.rs +++ b/crates/nyaterm-terminal-gpui/src/keywords.rs @@ -6,7 +6,7 @@ use std::time::{Duration, Instant}; use aho_corasick::{AhoCorasick, AhoCorasickBuilder, MatchKind}; use nyaterm_core::ResolvedKeywordHighlightRule; -use nyaterm_terminal::{TerminalSnapshot, terminal_cell_col_for_byte_index}; +use nyaterm_terminal::{TerminalSnapshot, TerminalSnapshotRow, terminal_cell_col_for_byte_index}; use crate::element::{TerminalBufferMatch, TerminalSearchFlags}; use crate::types::{TerminalHighlightSpan, TerminalKeywordRange}; @@ -59,27 +59,36 @@ pub(super) enum TerminalKeywordRowReuseKey { /// Immutable keyword data prepared away from GPUI's paint path. pub struct TerminalKeywordHighlightSnapshot { rules_key: u64, + cols: usize, display_offset: usize, + scrollback_len: usize, row_revisions: Vec, wrapped_flags: Vec, row_reuse_keys: Vec>, known_rows: Vec, rows: Vec>>>, + source_rows: Vec>>, rows_by_reuse_key: HashMap>>>, } pub(super) enum TerminalKeywordHighlightLookup<'a> { Current(Option<&'a Arc>>), Reused(Option<&'a Arc>>), + Stale(&'a [TerminalKeywordRange]), } impl<'a> TerminalKeywordHighlightLookup<'a> { - pub(super) fn ranges(&self) -> Option<&'a Arc>> { + pub(super) fn ranges(&self) -> Option<&'a [TerminalKeywordRange]> { match self { - Self::Current(ranges) | Self::Reused(ranges) => *ranges, + Self::Current(ranges) | Self::Reused(ranges) => ranges.map(|ranges| ranges.as_slice()), + Self::Stale(ranges) => Some(ranges), } } + pub(super) fn is_stale(&self) -> bool { + matches!(self, Self::Stale(_)) + } + pub(super) fn is_known_empty(&self) -> bool { self.ranges().is_none() } @@ -176,13 +185,37 @@ impl TerminalKeywordHighlightSnapshot { { return None; } - snapshot.row(row)?; - if !self.has_row_at_with_reuse_key(row, snapshot, reuse_key) { + let snapshot_row = snapshot.row(row)?; + if self.has_row_at_with_reuse_key(row, snapshot, reuse_key) { + return Some(TerminalKeywordHighlightLookup::Current( + self.rows.get(row)?.as_ref(), + )); + } + let source = self.source_rows.get(row)?.as_ref()?; + // Only retain a same-column prefix on the same unwrapped line. A changed + // logical line or viewport must not inherit another line's matches. + if self.cols != snapshot.cols + || self.scrollback_len != snapshot.scrollback_len + || source.line_id.is_none() + || source.line_id != snapshot_row.line_id + || source.wrapped + || snapshot_row.wrapped + || self.wrapped_flags.get(row + 1).copied().unwrap_or(false) + || snapshot.row(row + 1).is_some_and(|row| row.wrapped) + { return None; } - Some(TerminalKeywordHighlightLookup::Current( - self.rows.get(row)?.as_ref(), - )) + let ranges = self.rows.get(row)?.as_ref()?; + let end_col = ranges.last()?.end_col; + let unchanged_cols = source + .cells + .iter() + .zip(snapshot_row.cells.iter()) + .take(end_col) + .take_while(|(old, new)| old.text == new.text && old.width == new.width) + .count(); + let retained = ranges.partition_point(|range| range.end_col <= unchanged_cols); + (retained > 0).then(|| TerminalKeywordHighlightLookup::Stale(&ranges[..retained])) } fn has_row_at_with_reuse_key( @@ -360,11 +393,18 @@ pub fn precompute_terminal_keyword_highlights_for_rows_with_stats_and_cancel( .collect(); let snapshot = TerminalKeywordHighlightSnapshot { rules_key: highlighter.rules_key, + cols: snapshot.cols, display_offset: snapshot.display_offset, + scrollback_len: snapshot.scrollback_len, row_revisions: snapshot.rows().iter().map(|row| row.revision).collect(), wrapped_flags: snapshot.rows().iter().map(|row| row.wrapped).collect(), row_reuse_keys, known_rows, + source_rows: rows + .iter() + .zip(snapshot.rows()) + .map(|(ranges, row)| ranges.as_ref().map(|_| row.clone())) + .collect(), rows, rows_by_reuse_key, }; @@ -1161,8 +1201,9 @@ mod tests { use super::{ CompiledKeywordRule, MAX_KEYWORD_WRAPPED_GROUP_ROWS, TerminalKeywordHighlightLookup, - compile_keyword_rules, compile_terminal_keyword_highlighter, - keyword_highlight_spans_compiled, keyword_matches_compiled, keyword_matches_highlighter, + TerminalKeywordHighlightSnapshot, compile_keyword_rules, + compile_terminal_keyword_highlighter, keyword_highlight_spans_compiled, + keyword_matches_compiled, keyword_matches_highlighter, precompute_terminal_keyword_highlights, precompute_terminal_keyword_highlights_for_rows, precompute_terminal_keyword_highlights_for_rows_with_stats, terminal_buffer_matches, terminal_keyword_row_reuse_keys, @@ -1650,7 +1691,10 @@ mod tests { .and_then(|row| row.ranges()) .is_some() ); - assert!(highlights.stale_lookup(0, &changed_revision).is_none()); + assert!(matches!( + highlights.stale_lookup(0, &changed_revision), + Some(TerminalKeywordHighlightLookup::Stale(_)) + )); assert!(highlights.stale_lookup(usize::MAX, &snapshot).is_none()); assert!( highlights @@ -1665,6 +1709,171 @@ mod tests { assert!(highlights.rows.iter().skip(1).all(Option::is_none)); } + #[test] + fn stale_keyword_prefix_stays_highlighted_during_continuous_shell_echo() { + let mut screen = TerminalScreen::new(40, 3); + screen.advance(b"# ps -ef"); + let original = screen.snapshot(); + let highlights = command_highlights(&original); + let original_ranges = highlights.lookup(0, &original).unwrap().ranges().unwrap(); + + for echo in [" e", "f", "e", "\x08 \x08"] { + screen.advance(echo.as_bytes()); + let snapshot = screen.snapshot(); + assert!(highlights.lookup(0, &snapshot).is_none()); + let lookup = highlights.stale_lookup(0, &snapshot).unwrap(); + assert!(lookup.is_stale()); + assert_eq!(lookup.ranges().unwrap(), original_ranges); + // A provisional result must never satisfy the scheduler's freshness check. + assert!(!highlights.matches_snapshot_rows( + &snapshot, + nyaterm_ui::theme_palette("github-dark"), + 0..1, + )); + } + } + + fn command_highlights(snapshot: &TerminalSnapshot) -> TerminalKeywordHighlightSnapshot { + let rules = vec![ResolvedKeywordHighlightRule { + id: "options".into(), + name: "Options".into(), + patterns: vec!["-[a-z]+".into()], + color: "#ff2244".into(), + enabled: true, + }]; + precompute_terminal_keyword_highlights( + snapshot, + &compile_terminal_keyword_highlighter(&rules), + nyaterm_ui::theme_palette("github-dark"), + None, + ) + } + + #[test] + fn stale_keyword_prefix_rejects_edits_before_or_inside_the_match() { + let mut original = TerminalScreen::default().snapshot(); + set_snapshot_row(&mut original, 0, "# ps -ef tail", 41); + let highlights = command_highlights(&original); + for text in [ + "# ps x-ef tail", + "# ps-ef tail", + "# ps -ex tail", + "# ps -e tail", + ] { + let mut changed = original.clone(); + set_snapshot_row(&mut changed, 0, text, 42); + assert!(highlights.stale_lookup(0, &changed).is_none(), "{text}"); + } + } + + #[test] + fn stale_keyword_prefix_retains_only_complete_unchanged_matches() { + let mut original = TerminalScreen::default().snapshot(); + set_snapshot_row(&mut original, 0, "# ps -ef tail -aux", 41); + let highlights = command_highlights(&original); + let ranges = highlights.lookup(0, &original).unwrap().ranges().unwrap(); + assert_eq!(ranges.len(), 2); + let mut changed = original.clone(); + set_snapshot_row(&mut changed, 0, "# ps -ef tail -ax", 42); + assert_eq!( + highlights + .stale_lookup(0, &changed) + .unwrap() + .ranges() + .unwrap(), + &ranges[..1], + ); + } + + #[test] + fn stale_keyword_prefix_rejects_viewport_resize_scroll_and_line_replacement() { + let mut original = TerminalScreen::default().snapshot(); + set_snapshot_row(&mut original, 0, "# ps -ef", 41); + let highlights = command_highlights(&original); + let mut changed = original.clone(); + set_snapshot_row(&mut changed, 0, "# ps -ef e", 42); + + let mut resized = changed.clone(); + resized.cols += 1; + assert!(highlights.stale_lookup(0, &resized).is_none()); + let mut scrolled = changed.clone(); + scrolled.scrollback_len += 1; + assert!(highlights.stale_lookup(0, &scrolled).is_none()); + assert!( + highlights + .stale_lookup(0, &shifted_display_offset(&changed, 1)) + .is_none() + ); + let mut fewer_rows = changed.clone(); + fewer_rows.row_data = fewer_rows.rows()[..1].to_vec().into(); + assert!(highlights.stale_lookup(0, &fewer_rows).is_none()); + let rows = Arc::make_mut(&mut changed.row_data); + assert!(rows[0].line_id.is_some()); + Arc::make_mut(&mut rows[0]).line_id = None; + assert!(highlights.stale_lookup(0, &changed).is_none()); + + let rows = Arc::make_mut(&mut original.row_data); + Arc::make_mut(&mut rows[0]).line_id = None; + let without_line_id = command_highlights(&original); + assert!(without_line_id.stale_lookup(0, &changed).is_none()); + } + + #[test] + fn stale_keyword_prefix_rejects_new_and_existing_soft_wraps() { + for cols in [6, 9] { + let mut screen = TerminalScreen::new(cols, 4); + screen.advance(b"# ps -ef"); + let original = screen.snapshot(); + let highlights = command_highlights(&original); + screen.advance(b" e"); + let changed = screen.snapshot(); + assert!(changed.row(1).unwrap().wrapped); + for row in 0..2 { + assert!( + highlights.stale_lookup(row, &changed).is_none(), + "{cols}, {row}" + ); + } + } + } + + #[test] + fn stale_keyword_prefix_checks_wide_and_combining_cell_geometry() { + for text in ["\u{754c} -ef", "e\u{301} -ef"] { + let mut original = TerminalScreen::default().snapshot(); + set_snapshot_row(&mut original, 0, text, 41); + let highlights = command_highlights(&original); + let mut changed = original.clone(); + set_snapshot_row(&mut changed, 0, format!("{text} e"), 42); + assert!(highlights.stale_lookup(0, &changed).is_some()); + let rows = Arc::make_mut(&mut changed.row_data); + let width = rows[0].cells[0].width; + Arc::make_mut(&mut rows[0]).cells[0].width = if width == 1 { 2 } else { 1 }; + assert!(highlights.stale_lookup(0, &changed).is_none()); + set_snapshot_row(&mut changed, 0, format!("{text} e"), 42); + let rows = Arc::make_mut(&mut changed.row_data); + Arc::make_mut(&mut rows[0]).cells[0].text = Arc::from("e"); + assert!(highlights.stale_lookup(0, &changed).is_none()); + } + } + + #[test] + fn stale_keyword_prefix_does_not_treat_an_empty_old_result_as_current() { + let mut original = TerminalScreen::default().snapshot(); + set_snapshot_row(&mut original, 0, "# ps", 41); + let highlights = command_highlights(&original); + let mut changed = original.clone(); + set_snapshot_row(&mut changed, 0, "# ps -ef", 42); + assert!(highlights.stale_lookup(0, &changed).is_none()); + assert!( + command_highlights(&changed) + .lookup(0, &changed) + .unwrap() + .ranges() + .is_some() + ); + } + #[test] fn oversized_wrapped_group_degrades_to_individual_rows() { let row_count = MAX_KEYWORD_WRAPPED_GROUP_ROWS + 2; @@ -1723,7 +1932,7 @@ mod tests { .expect("first ERROR should be highlighted"); assert_eq!( - ranges.as_slice(), + ranges, &[ TerminalKeywordRange { start_col: 0, @@ -1763,7 +1972,7 @@ mod tests { .expect("version should still be highlighted"); assert_eq!( - ranges.as_slice(), + ranges, &[ TerminalKeywordRange { start_col: "Ubuntu ".len(), @@ -1939,11 +2148,7 @@ mod tests { let palette = nyaterm_ui::theme_palette("github-dark"); let first_highlights = precompute_terminal_keyword_highlights(&first_snapshot, &highlighter, palette, None); - let first_ranges = first_highlights - .lookup(0, &first_snapshot) - .and_then(|row| row.ranges()) - .expect("first ranges") - .clone(); + let first_ranges = first_highlights.rows[0].as_ref().expect("first ranges"); let second_highlights = precompute_terminal_keyword_highlights( &second_snapshot, @@ -1951,12 +2156,9 @@ mod tests { palette, Some(&first_highlights), ); - let second_ranges = second_highlights - .lookup(5, &second_snapshot) - .and_then(|row| row.ranges()) - .expect("second ranges"); + let second_ranges = second_highlights.rows[5].as_ref().expect("second ranges"); - assert!(Arc::ptr_eq(&first_ranges, second_ranges)); + assert!(Arc::ptr_eq(first_ranges, second_ranges)); } #[test] @@ -2064,7 +2266,7 @@ mod tests { .expect("second wrapped row ranges"); assert_eq!( - first.as_ref(), + first, &[TerminalKeywordRange { start_col: 0, end_col: 3, @@ -2072,7 +2274,7 @@ mod tests { }] ); assert_eq!( - second.as_ref(), + second, &[TerminalKeywordRange { start_col: 0, end_col: 2,