From 605e5fc4ed1a2180d9355006779f7113419039ff Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Mon, 13 Jul 2026 15:07:02 -0500 Subject: [PATCH 1/5] perf(terminal): optimize flat storage rebuilds Rework flat-storage indexing and replay around faster row paths. - Add arithmetic rebuild fast paths for uniform rows, carry-over rows, and wide-character spacer cases. - Preserve EntryBuilder capacity across flushes and add dense single-width row replay. - Harden truncate and cursor-offset edge cases used by resize. - Add differential/property tests, a fuzz target, and rebuild benchmark coverage. --- Cargo.lock | 1 + app/src/terminal/model/grid/resize.rs | 26 +- app/src/terminal/model/grid/tests.rs | 19 + crates/warp_terminal/Cargo.toml | 6 + .../benches/flat_storage_rebuild_bench.rs | 164 ++++ .../model/grid/flat_storage/attribute_map.rs | 71 +- .../src/model/grid/flat_storage/content.rs | 65 +- .../src/model/grid/flat_storage/index.rs | 736 ++++++++++++++--- .../model/grid/flat_storage/index_tests.rs | 774 ++++++++++++++++++ .../src/model/grid/flat_storage/mod.rs | 95 ++- .../src/model/grid/flat_storage/mod_tests.rs | 16 + .../model/grid/flat_storage/row_iterator.rs | 32 +- .../fuzz_targets/fuzz_flat_storage_rebuild.rs | 87 ++ 13 files changed, 1933 insertions(+), 159 deletions(-) create mode 100644 crates/warp_terminal/benches/flat_storage_rebuild_bench.rs create mode 100644 fuzz/fuzz_targets/fuzz_flat_storage_rebuild.rs diff --git a/Cargo.lock b/Cargo.lock index 7be189e7cd7..d83f657755e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15870,6 +15870,7 @@ dependencies = [ "cfg-if", "channel_versions", "command-corrections", + "criterion", "enum-iterator", "get-size", "itertools 0.14.0", diff --git a/app/src/terminal/model/grid/resize.rs b/app/src/terminal/model/grid/resize.rs index b45d39f92d0..3b7727080b0 100644 --- a/app/src/terminal/model/grid/resize.rs +++ b/app/src/terminal/model/grid/resize.rs @@ -282,21 +282,17 @@ impl InitialCursorState { } fn into_content_offset(self, grid: &GridHandler) -> CursorContentOffset { - match self { - Self::AtPoint(point) => { - let content_offset = grid - .flat_storage - .content_offset_at_point(point) - .expect("should have a content offset for point"); - CursorContentOffset::AtPoint(content_offset) - } - Self::AtCellAfterPoint(point) => { - let content_offset = grid - .flat_storage - .content_offset_at_point(point) - .expect("should have a content offset for point"); - CursorContentOffset::AtCellAfterPoint(content_offset) - } + let (point, cell_after_point) = match self { + Self::AtPoint(point) => (point, false), + Self::AtCellAfterPoint(point) => (point, true), + }; + + let content_offset = grid.flat_storage.content_offset_at_point_or_before(point); + + if cell_after_point { + CursorContentOffset::AtCellAfterPoint(content_offset) + } else { + CursorContentOffset::AtPoint(content_offset) } } } diff --git a/app/src/terminal/model/grid/tests.rs b/app/src/terminal/model/grid/tests.rs index dfe08210171..89f50d439ed 100644 --- a/app/src/terminal/model/grid/tests.rs +++ b/app/src/terminal/model/grid/tests.rs @@ -362,6 +362,25 @@ fn multiple_resizes_cursor_position_restoration() { assert_eq!(grid.grid_storage().cursor.point.col, 7); } +#[test] +fn resize_cursor_in_blank_tail_does_not_panic() { + let _flag = FeatureFlag::ResizeFix.override_enabled(true); + + let mut grid = GridHandler::new_for_test_with_scroll_limit(4, 200, 4); + grid.input_at_cursor("abc"); + grid.set_cursor_point(1, 0); + grid.input_at_cursor("def"); + + // Put the cursor far into the blank tail of the second row. This used to + // crash during resize when the cursor was reflowed back through flat + // storage. + grid.set_cursor_point(1, 148); + + grid.resize(SizeInfo::new_without_font_metrics(4, 80)); + + assert_eq!(grid.cursor_point(), Point::new(1, 3)); +} + #[test] fn non_sequential_resizes_cursor_restoration() { let _flag = FeatureFlag::ResizeFix.override_enabled(true); diff --git a/crates/warp_terminal/Cargo.toml b/crates/warp_terminal/Cargo.toml index bb91f9c86f9..19dd2a23c69 100644 --- a/crates/warp_terminal/Cargo.toml +++ b/crates/warp_terminal/Cargo.toml @@ -37,7 +37,13 @@ warp_util.workspace = true warpui_core.workspace = true [dev-dependencies] +criterion = { version = "0.5.1", features = ["html_reports"] } unicode-segmentation = "1.11.0" [features] test-util = [] + +[[bench]] +name = "flat_storage_rebuild_bench" +harness = false +required-features = ["test-util"] diff --git a/crates/warp_terminal/benches/flat_storage_rebuild_bench.rs b/crates/warp_terminal/benches/flat_storage_rebuild_bench.rs new file mode 100644 index 00000000000..47371518681 --- /dev/null +++ b/crates/warp_terminal/benches/flat_storage_rebuild_bench.rs @@ -0,0 +1,164 @@ +use std::num::NonZeroU16; + +use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; +use warp_terminal::model::grid::flat_storage::{GraphemeInfo, Index}; + +const ASCII_GRAPHEME_INFO: GraphemeInfo = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(1).unwrap(), +}; + +const MULTIBYTE_GRAPHEME_INFO: GraphemeInfo = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(3).unwrap(), +}; + +const EMOJI_GRAPHEME_INFO: GraphemeInfo = GraphemeInfo { + cell_width: 2, + utf8_bytes: NonZeroU16::new(4).unwrap(), +}; + +struct Fixture { + name: &'static str, + index: Index, + target_columns: &'static [usize], +} + +fn push_graphemes(index: &mut Index, count: usize, info: GraphemeInfo, trailing_newline: bool) { + let mut entry_builder = index.start_row(); + for _ in 0..count { + entry_builder.process_grapheme_info_unchecked(info); + } + if trailing_newline { + entry_builder.add_trailing_newline(); + } + entry_builder.append_to_index(index); +} + +fn make_uniform_index( + rows: usize, + columns: usize, + info: GraphemeInfo, + newline_every_row: bool, +) -> Index { + let mut index = Index::new(columns, Some(rows)); + let graphemes_per_row = columns / info.cell_width as usize; + for _ in 0..rows { + push_graphemes(&mut index, graphemes_per_row, info, newline_every_row); + } + index +} + +fn make_softwrapped_ascii_index(rows: usize, columns: usize, segment_rows: usize) -> Index { + let mut index = Index::new(columns, Some(rows)); + for row_idx in 0..rows { + let trailing_newline = (row_idx + 1) % segment_rows == 0; + push_graphemes(&mut index, columns, ASCII_GRAPHEME_INFO, trailing_newline); + } + index +} + +fn make_mixed_index(rows: usize, columns: usize) -> Index { + let mut index = Index::new(columns, Some(rows)); + + for row_idx in 0..rows { + let mut entry_builder = index.start_row(); + let mut cells = 0; + let mut pattern_idx = 0; + + while cells < columns { + let info = match (row_idx + pattern_idx) % 4 { + 0 => ASCII_GRAPHEME_INFO, + 1 => MULTIBYTE_GRAPHEME_INFO, + _ => EMOJI_GRAPHEME_INFO, + }; + + if cells + info.cell_width as usize > columns { + pattern_idx += 1; + continue; + } + + entry_builder.process_grapheme_info_unchecked(info); + cells += info.cell_width as usize; + pattern_idx += 1; + } + + if row_idx % 3 != 1 { + entry_builder.add_trailing_newline(); + } + entry_builder.append_to_index(&mut index); + } + + index +} + +fn total_graphemes(index: &Index) -> u64 { + (0..index.len()) + .filter_map(|row_idx| index.grapheme_infos_for_row(row_idx)) + .flatten() + .count() as u64 +} + +fn fixtures() -> Vec { + vec![ + Fixture { + name: "ascii_dense", + index: make_uniform_index(10_000, 80, ASCII_GRAPHEME_INFO, true), + target_columns: &[120, 81, 40], + }, + Fixture { + name: "wide_dense", + index: make_uniform_index(10_000, 80, EMOJI_GRAPHEME_INFO, true), + target_columns: &[120, 79, 7], + }, + Fixture { + name: "mixed_rows", + index: make_mixed_index(8_000, 80), + target_columns: &[120, 81, 40], + }, + Fixture { + name: "softwrapped_ascii", + index: make_softwrapped_ascii_index(10_000, 80, 32), + target_columns: &[120, 80, 40], + }, + ] +} + +fn criterion_benchmark(c: &mut Criterion) { + for fixture in fixtures() { + let graphemes = total_graphemes(&fixture.index); + let mut group = c.benchmark_group(format!("flat_storage/rebuild/{}", fixture.name)); + group.throughput(Throughput::Elements(graphemes)); + + for &target_columns in fixture.target_columns { + group.bench_with_input( + BenchmarkId::new("optimized", target_columns), + &target_columns, + |b, &target_columns| { + b.iter(|| Index::rebuild(black_box(&fixture.index), black_box(target_columns))) + }, + ); + group.bench_with_input( + BenchmarkId::new("baseline", target_columns), + &target_columns, + |b, &target_columns| { + b.iter(|| { + Index::rebuild_baseline( + black_box(&fixture.index), + black_box(target_columns), + ) + }) + }, + ); + } + + group.finish(); + } +} + +criterion_group!( + name = benches; + config = Criterion::default().sample_size(10); + targets = criterion_benchmark +); +criterion_main!(benches); diff --git a/crates/warp_terminal/src/model/grid/flat_storage/attribute_map.rs b/crates/warp_terminal/src/model/grid/flat_storage/attribute_map.rs index c07c6a95cd7..e2df247169d 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/attribute_map.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/attribute_map.rs @@ -1,5 +1,5 @@ -use std::collections::{btree_map, BTreeMap}; use std::ops::RangeFrom; +use std::slice; use get_size::GetSize; use string_offset::ByteOffset; @@ -10,13 +10,13 @@ use string_offset::ByteOffset; /// This internally coalesces ranges to store the data in a space-efficient /// manner. /// -/// A [`BTreeMap`] is used to achieve great performance both for looking up -/// a value in the map and scanning forward from that point. +/// A sorted vector is used because we only ever append changes in order, and +/// iteration heavily outweighs random inserts on this path. #[derive(Debug, Default, Clone)] pub struct AttributeMap { /// Stores a mapping between an _ending_ byte offset (inclusive) and the /// attribute value for the range ending at the given offset. - map: BTreeMap, + map: Vec<(ByteOffset, A)>, /// The attribute value for all offsets beyond the last end offset stored /// in the map. tail_value: A, @@ -32,22 +32,24 @@ impl AttributeMap { /// Truncates the attribute map to the given content offset. pub fn truncate(&mut self, new_len: ByteOffset) { - // Split off any ranges that end after our new length. - let mut truncated_ranges = self.map.split_off(&new_len); - // If we split off any ranges, the first range defines our new tail value. - if let Some((_, tail_value)) = truncated_ranges.pop_first() { + let split_idx = self.map.partition_point(|(offset, _)| *offset < new_len); + let truncated_ranges = self.map.split_off(split_idx); + if let Some((_, tail_value)) = truncated_ranges.into_iter().next() { self.tail_value = tail_value; } } /// Truncates the attribute map to start at the given content offset. pub fn truncate_front(&mut self, new_start_offset: ByteOffset) { - self.map = self.map.split_off(&new_start_offset); + let split_idx = self + .map + .partition_point(|(offset, _)| *offset < new_start_offset); + self.map.drain(..split_idx); } /// Returns the end offset of the last range in the map. fn last_end_offset(&self) -> ByteOffset { - if let Some((k, _v)) = self.map.last_key_value() { + if let Some((k, _v)) = self.map.last() { *k } else { ByteOffset::zero() @@ -69,7 +71,7 @@ impl AttributeMap { let prev_tail_value = std::mem::replace(&mut self.tail_value, value); if range.start == ByteOffset::zero() { - debug_assert!(self.map.last_key_value().is_none()); + debug_assert!(self.map.is_empty()); } else { debug_assert!( range.start > self.last_end_offset(), @@ -78,7 +80,7 @@ impl AttributeMap { self.last_end_offset(), self.map, ); - self.map.insert(range.start - 1, prev_tail_value); + self.map.push((range.start - 1, prev_tail_value)); } } } @@ -92,7 +94,7 @@ impl GetSize for AttributeMap { impl AttributeMap { /// Returns an iterator over per-byte attribute values starting at the /// given byte offset. - pub fn iter_from(&self, start_offset: ByteOffset) -> impl Iterator + '_ { + pub(super) fn iter_from(&self, start_offset: ByteOffset) -> Iter<'_, A> { Iter::new(self, start_offset) } @@ -103,16 +105,19 @@ impl AttributeMap { } /// An iterator over an attribute map. -struct Iter<'a, A> { +pub(super) struct Iter<'a, A> { cur_offset: ByteOffset, cur_range: (ByteOffset, A), - inner: btree_map::Range<'a, ByteOffset, A>, + inner: slice::Iter<'a, (ByteOffset, A)>, tail_value: A, } impl<'a, A: Copy> Iter<'a, A> { fn new(map: &'a AttributeMap, start_offset: ByteOffset) -> Self { - let mut inner = map.map.range(start_offset..); + let start_idx = map + .map + .partition_point(|(offset, _)| *offset < start_offset); + let mut inner = map.map[start_idx..].iter(); let cur_range = Self::next_range(&mut inner, map.tail_value); Self { @@ -124,7 +129,7 @@ impl<'a, A: Copy> Iter<'a, A> { } /// Returns the end point and value for the next range. - fn next_range(inner: &mut btree_map::Range, tail: A) -> (ByteOffset, A) { + fn next_range(inner: &mut slice::Iter<'a, (ByteOffset, A)>, tail: A) -> (ByteOffset, A) { inner .next() .map(|(k, v)| (*k, *v)) @@ -132,6 +137,17 @@ impl<'a, A: Copy> Iter<'a, A> { // with the tail attribute value. .unwrap_or((ByteOffset::from(usize::MAX), tail)) } + + fn advance_to_current_range(&mut self) { + while self.cur_offset > self.cur_range.0 { + self.cur_range = Self::next_range(&mut self.inner, self.tail_value); + } + } + + pub(super) fn skip_by(&mut self, n: usize) { + self.cur_offset += n; + self.advance_to_current_range(); + } } impl Iterator for Iter<'_, A> @@ -141,24 +157,17 @@ where type Item = A; fn next(&mut self) -> Option { - self.nth(0) - } - - fn nth(&mut self, n: usize) -> Option { - // Skip over the next n items. - self.cur_offset += n; - // While the offset of the next item is outside the current range, get - // the next range from the iterator over our BTreeMap. - while self.cur_offset > self.cur_range.0 { - self.cur_range = Self::next_range(&mut self.inner, self.tail_value); - } - - // Get the value and advance the iterator by one, in preparation for - // the next call. + self.advance_to_current_range(); let val = self.cur_range.1; self.cur_offset += 1; + self.advance_to_current_range(); Some(val) } + + fn nth(&mut self, n: usize) -> Option { + self.skip_by(n); + self.next() + } } #[cfg(test)] diff --git a/crates/warp_terminal/src/model/grid/flat_storage/content.rs b/crates/warp_terminal/src/model/grid/flat_storage/content.rs index 0e00bd46c0a..579e79d399b 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/content.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/content.rs @@ -1,6 +1,6 @@ //! Structures for storing the grid contents in a flat buffer. -use std::collections::BTreeMap; +use std::collections::{btree_map, BTreeMap}; use std::ops::{Index, Range}; use get_size::GetSize; @@ -109,6 +109,11 @@ impl Content { } } + /// Returns a cursor positioned at the given content offset. + pub fn cursor_at(&self, start_offset: ByteOffset) -> Cursor<'_> { + Cursor::new(self, start_offset) + } + /// Returns the offset of the end of the content (exclusive). /// /// If a new grapheme is pushed into the buffer, this is the offset at @@ -165,6 +170,64 @@ impl GetSize for Content { } } +/// A cursor for sequentially reading content slices. +pub struct Cursor<'a> { + current_chunk: &'a Chunk, + active_chunk: &'a Chunk, + inner: btree_map::Range<'a, ByteOffset, Chunk>, + current_offset: ByteOffset, +} + +impl<'a> Cursor<'a> { + fn new(content: &'a Content, start_offset: ByteOffset) -> Self { + let current_chunk = content + .filled_chunks + .range(start_offset..) + .next() + .map(|(_, chunk)| chunk) + .unwrap_or(&content.active_chunk); + + let inner = content + .filled_chunks + .range(current_chunk.content_range().end..); + + Self { + current_chunk, + active_chunk: &content.active_chunk, + inner, + current_offset: start_offset, + } + } + + fn advance_chunk(&mut self) { + self.current_chunk = self + .inner + .next() + .map(|(_, chunk)| chunk) + .unwrap_or(self.active_chunk); + } + + /// Returns the next `len` bytes from the cursor. + pub fn next_slice(&mut self, len: usize) -> &'a str { + debug_assert!(len > 0); + let chunk = self.current_chunk; + debug_assert!( + self.current_offset >= chunk.start_offset, + "cursor offset {:#?} must be within the current chunk starting at {:#?}", + self.current_offset, + chunk.start_offset + ); + let start = (self.current_offset - chunk.start_offset).as_usize(); + let end = start + len; + let content = &chunk[start..end]; + self.current_offset += len; + if self.current_offset >= chunk.content_range().end { + self.advance_chunk(); + } + content + } +} + /// A single chunk of content, backed by a byte array. #[derive(Debug, Clone, GetSize)] struct Chunk { diff --git a/crates/warp_terminal/src/model/grid/flat_storage/index.rs b/crates/warp_terminal/src/model/grid/flat_storage/index.rs index ed7fabd89b3..268ed66fe4a 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/index.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/index.rs @@ -27,9 +27,7 @@ use cfg_if::cfg_if; use get_size::GetSize; use string_offset::ByteOffset; use thiserror::Error; -use warp_errors::report_error; -use super::grapheme::Grapheme; use crate::model::grid::CellType; use crate::model::Point; @@ -98,53 +96,116 @@ impl Index { /// Rebuilds an [`Index`] to wrap lines at a different number of columns. pub fn rebuild(old_index: &Index, columns: usize) -> Self { - let mut index = Self::new(columns, Some(old_index.len())); - // Update the content length to be the start offset of the first row, - // or preserve the old content offset if no rows remain, to ensure we - // properly handle resizing after truncation. + let capacity = if columns > 0 && old_index.columns > 0 { + old_index.len().saturating_mul(old_index.columns) / columns + 1 + } else { + old_index.len() + }; + let mut index = Self::new(columns, Some(capacity)); index.content_len = old_index .rows .front() - .map(|entry| entry.content_offset.as_usize()) - .unwrap_or(old_index.content_len); + .map(|entry| entry.content_offset) + .unwrap_or_else(|| old_index.content_len.into()) + .as_usize(); let mut entry_builder = EntryBuilder::new(); - // Loop over rows in the old index, processing each grapheme in order - // and adding newlines where appropriate. - // - // TODO(vorporeal): This can be significantly optimized - processing - // each grapheme individually is a clearly poor choice in the (common) - // case of a grid that contains only ASCII text. We could take more - // advantage of the run-length encoded `GraphemeRun` structure here. - for row_idx in 0..old_index.len() { - if let Some(grapheme_infos) = old_index.grapheme_infos_for_row(row_idx) { - for info in grapheme_infos { - entry_builder.process_grapheme_info(info, &mut index); + // entries_with_runs() merges the row VecDeque and grapheme_sizing BTreeMap + // in a single O(n) scan, avoiding a per-row O(log n) BTreeMap lookup. + for (entry, row_runs) in old_index.entries_with_runs() { + if entry_builder.is_empty() { + if entry.has_trailing_newline { + // Fast path A: narrowing uniform — arithmetic split avoids + // per-grapheme work when the row must be split across many + // output rows. + if let GraphemeSizing::Uniform(run) = &entry.grapheme_sizing { + let cell_width = run.info.cell_width as usize; + if run.cols() > columns && cell_width > 0 && columns >= cell_width { + emit_narrowed_uniform(run, &mut index); + continue; + } + } + // Fast path B: row fits in the new width — memcpy the entry. + if try_emit_row_with_newline(entry, row_runs, &mut index) { + continue; + } + } else { + // Fast path C: soft-wrapped row that fits — absorb into the + // builder so it merges with subsequent soft-wrap rows. + if try_accumulate_softwrap(entry, row_runs, &mut entry_builder, columns) { + continue; + } } } - if old_index - .get_entry(row_idx) - .expect("row should have an entry") - .has_trailing_newline - { - entry_builder.process_grapheme(&Grapheme::NEWLINE, &mut index); + + // Fast path D: uniform run (cell_width 1 or 2) with carry-over + // from a previous soft-wrapped row — arithmetic split without + // per-grapheme processing. + if let GraphemeSizing::Uniform(run) = &entry.grapheme_sizing { + if try_emit_carryover_uniform( + run, + entry.has_trailing_newline, + &mut entry_builder, + &mut index, + ) { + continue; + } } + + // Medium path: bulk-accumulate runs that fit whole; delegate only + // the boundary-straddling run to process_graphemes_batch. + emit_runs( + row_runs, + entry.has_trailing_newline, + &mut entry_builder, + &mut index, + ); } - // Add the final entry to the index. - // - // If reflowing the content led to some trailing empty cells being - // pushed onto a new row, don't add that empty row to the index. entry_builder.append_to_index_if_nonempty(&mut index); if index.content_len > old_index.content_len { - report_error!("somehow ended up with too much flat storage content!"); + log::error!("somehow ended up with too much flat storage content!"); } index } + /// BASELINE: Simple, obviously-correct rebuild algorithm used as the + /// reference implementation in differential tests. + /// + /// Iterates over each source row's grapheme runs in order and feeds them + /// into [`EntryBuilder::process_graphemes_batch`] one run at a time. This + /// naturally handles soft-wrapping at the new column width without any of + /// the arithmetic fast-paths found in [`Self::rebuild`], making it easy to + /// audit for correctness. + #[cfg(any(test, feature = "test-util"))] + pub fn rebuild_baseline(old_index: &Index, columns: usize) -> Self { + let mut index = Self::new(columns, Some(old_index.len())); + index.content_len = old_index + .rows + .front() + .map(|entry| entry.content_offset) + .unwrap_or_else(|| old_index.content_len.into()) + .as_usize(); + + let mut entry_builder = EntryBuilder::new(); + + for (entry, runs) in old_index.entries_with_runs() { + for run in runs { + entry_builder.process_graphemes_batch(run.info, run.count.get(), &mut index); + } + if entry.has_trailing_newline { + entry_builder.add_trailing_newline(); + entry_builder.flush_to_index(&mut index); + } + } + + entry_builder.append_to_index_if_nonempty(&mut index); + index + } + /// Truncates the index to the given number of rows, returning the new /// content length. pub fn truncate(&mut self, new_len: usize) -> ByteOffset { @@ -167,19 +228,24 @@ impl Index { /// Removes the first `count` rows from the index, returning the new start /// offset for the remaining content. pub fn truncate_front(&mut self, count: usize) -> ByteOffset { + if self.is_empty() { + return self.content_len.into(); + } + + let bounded_count = count.min(self.rows.len()); let new_start_offset = self.content_offset_for_row(count).unwrap_or_else(|| { if count > self.rows.len() { - report_error!( - "Attempted to truncate more rows than exist in flat storage", - extra: { "rows" => %self.rows.len(), "truncate_count" => %count } + log::error!( + "should not attempt to truncate more rows than exist in flat storage; \ + have {} rows, trying to truncate {}", + self.rows.len(), + count ); } self.content_len.into() }); - for _ in 0..count { - self.rows.pop_front(); - } + self.rows.drain(..bounded_count); self.grapheme_sizing = self.grapheme_sizing.split_off(&new_start_offset); new_start_offset @@ -194,6 +260,11 @@ impl Index { self.rows.len() } + /// Returns whether the index contains no rows. + pub fn is_empty(&self) -> bool { + self.rows.is_empty() + } + /// Returns the content [`ByteOffset`] for the given point. /// /// Returns an error if: @@ -377,7 +448,7 @@ impl Index { GraphemeSizing::NonUniform => self.grapheme_sizing.get(&entry.content_offset), GraphemeSizing::EmptyRow => return Some(CellType::RegularChar), }) else { - report_error!( + log::error!( "Found entry with non-uniform grapheme sizing and no grapheme run information!" ); return None; @@ -414,6 +485,24 @@ impl Index { Some(runs) } + /// Returns an iterator over every row's `(Entry, runs)` pair in order. + /// + /// Unlike repeated `grapheme_runs_for_row` calls (O(log n) per row due to + /// BTreeMap lookup), this merges the row VecDeque and the grapheme_sizing + /// BTreeMap in a single O(n) scan, relying on both being ordered by + /// content_offset. + fn entries_with_runs(&self) -> impl Iterator + '_ { + let mut sizing_iter = self.grapheme_sizing.iter(); + self.rows.iter().map(move |entry| { + let runs: &[GraphemeRun] = match &entry.grapheme_sizing { + GraphemeSizing::Uniform(run) => std::slice::from_ref(run), + GraphemeSizing::NonUniform => sizing_iter.next().map_or(&[], |(_, v)| v.as_slice()), + GraphemeSizing::EmptyRow => &[], + }; + (entry, runs) + }) + } + /// Returns an iterator over the sizing information for each individual /// grapheme in the given row. /// @@ -486,54 +575,6 @@ impl EntryBuilder { Default::default() } - /// Processes the next [`Grapheme`] in the row. - pub fn process_grapheme(&mut self, grapheme: &Grapheme, index: &mut Index) { - if grapheme.starts_new_row() { - self.add_trailing_newline(); - std::mem::take(self).append_to_index(index); - return; - } - - self.process_grapheme_info(grapheme.sizing_info(), index); - } - - /// Processes the next grapheme in the row, based only on its sizing - /// information (and not its content). - fn process_grapheme_info(&mut self, info: GraphemeInfo, index: &mut Index) { - let grapheme_len = info.utf8_bytes.get() as usize; - debug_assert!( - grapheme_len > 0, - "should not process an empty string as a grapheme" - ); - - if info.cell_width == 0 { - #[cfg(debug_assertions)] - report_error!("encountered unexpected grapheme with a computed cell width of zero!"); - return; - } - debug_assert!( - info.cell_width <= 2, - "graphemes should not be more than two cells wide, but encountered one with width {}", - info.cell_width - ); - - // If there isn't enough room in the row for this grapheme, cut off - // the row here, starting the new row with the _current_ grapheme. - if self.num_cells + info.cell_width as usize > index.columns { - // If this is a non-full row and we've got a wide char, mark - // the fact that we have a leading wide char spacer. - if info.cell_width > 1 && self.num_cells != index.columns { - self.add_leading_wide_char_spacer(); - } - std::mem::take(self).append_to_index(index); - debug_assert_eq!(self.incr_content_offset, ByteOffset::zero()); - } - - self.num_cells += info.cell_width as usize; - - self.process_grapheme_info_unchecked(info); - } - /// Processes the next grapheme in the row, without performing any checks /// around whether or not the row is full. /// @@ -550,12 +591,7 @@ impl EntryBuilder { // Store information about this grapheme's cell width and UTF-8 length. match self.grapheme_runs.last_mut() { Some(last_run) if last_run.info == info => { - // TODO(vorporeal): might be able to eke out some extra performance - // if we remove the error checking here. - last_run.count = last_run - .count - .checked_add(1) - .expect("should not have more than 2^16 graphemes in a single row"); + checked_add_run_count(&mut last_run.count, 1); } _ => { self.grapheme_runs.push(GraphemeRun { @@ -566,6 +602,87 @@ impl EntryBuilder { } } + /// Batch processes multiple graphemes from the same run in one call. + /// + /// Uses arithmetic chunk processing (O(rows) not O(graphemes)) and + /// correctly accounts for bytes per flushed segment — the original + /// per-grapheme loop accumulated all bytes at the end, which caused + /// content-offset miscalculation whenever a flush occurred mid-batch. + fn process_graphemes_batch(&mut self, info: GraphemeInfo, count: u16, index: &mut Index) { + let grapheme_len = info.utf8_bytes.get() as usize; + let cell_width = info.cell_width as usize; + let mut remaining = count as usize; + + while remaining > 0 { + let space = index.columns.saturating_sub(self.num_cells); + let fits = if cell_width > 0 { + space / cell_width + } else { + 0 + }; + + if fits == 0 { + // No room for even one grapheme — flush and retry. + if cell_width > 1 && self.num_cells != index.columns { + self.add_leading_wide_char_spacer(); + } + self.flush_to_index(index); + + // If the grapheme is wider than the entire column width it + // can never fit via normal packing. Emit one grapheme per row + // (matching process_grapheme_info's behaviour) and move on. + if cell_width > index.columns { + // The initial flush above already emitted the preceding row + // (with ends_with_leading_wide_char_spacer set). Now emit + // each wide char on its own row; all but the last get the + // spacer flag (because the next wide char will "overflow" + // onto the following row, mirroring process_grapheme_info). + while remaining > 0 { + self.num_cells += cell_width; + self.incr_content_offset += grapheme_len; + self.grapheme_runs.push(GraphemeRun { + count: unsafe { NonZeroU16::new_unchecked(1) }, + info, + }); + if remaining > 1 { + self.add_leading_wide_char_spacer(); + } + self.flush_to_index(index); + remaining -= 1; + } + return; + } + continue; + } + + let take = remaining.min(fits); + // SAFETY: take > 0 because fits > 0 and remaining > 0. + let take_u16 = unsafe { NonZeroU16::new_unchecked(take as u16) }; + + self.num_cells += take * cell_width; + self.incr_content_offset += take * grapheme_len; + + match self.grapheme_runs.last_mut() { + Some(last) if last.info == info => { + checked_add_run_count(&mut last.count, take_u16.get()); + } + _ => { + self.grapheme_runs.push(GraphemeRun { + count: take_u16, + info, + }); + } + } + + remaining -= take; + + // If the row is now full and there are more graphemes, flush. + if self.num_cells >= index.columns && remaining > 0 { + self.flush_to_index(index); + } + } + } + /// Marks the [`Entry`]'s row as containing a trailing newline. pub fn add_trailing_newline(&mut self) { self.incr_content_offset += '\n'.len_utf8(); @@ -630,15 +747,59 @@ impl EntryBuilder { && !self.ends_with_leading_wide_char_spacer && self.grapheme_runs.is_empty() } + + /// Flushes the current entry to the index and resets in-place, + /// preserving Vec capacity to avoid reallocation. + pub(crate) fn flush_to_index(&mut self, index: &mut Index) { + #[cfg(debug_assertions)] + { + self.was_processed = true; + } + + let content_offset = index.content_len.into(); + + let grapheme_sizing = if self.grapheme_runs.len() == 1 { + // pop() leaves the Vec with capacity intact. + GraphemeSizing::Uniform(unsafe { self.grapheme_runs.pop().unwrap_unchecked() }) + } else if self.grapheme_runs.is_empty() { + GraphemeSizing::EmptyRow + } else { + // Clone the runs into a right-sized vec for storage, then clear + // self in-place so the next row reuses the existing allocation + // and avoids repeated Vec reallocations. + let runs = self.grapheme_runs.clone(); + self.grapheme_runs.clear(); + index.grapheme_sizing.insert(content_offset, runs); + GraphemeSizing::NonUniform + }; + + index.content_len += self.incr_content_offset.as_usize(); + index.rows.push_back(Entry { + content_offset, + grapheme_sizing, + has_trailing_newline: self.has_trailing_newline, + ends_with_leading_wide_char_spacer: self.ends_with_leading_wide_char_spacer, + }); + + // Reset the payload in-place, preserving `grapheme_runs` capacity. + // Keep the debug lifecycle marker set so dropping a successfully + // flushed builder does not trip the destructor assert. + self.num_cells = 0; + self.incr_content_offset = ByteOffset::zero(); + self.has_trailing_newline = false; + self.ends_with_leading_wide_char_spacer = false; + } } impl Drop for EntryBuilder { fn drop(&mut self) { #[cfg(debug_assertions)] - debug_assert!( - self.was_processed, - "EntryBuilder must be processed before it is dropped" - ); + if !std::thread::panicking() { + debug_assert!( + self.was_processed, + "EntryBuilder must be processed before it is dropped" + ); + } } } @@ -706,6 +867,391 @@ pub struct GraphemeInfo { pub utf8_bytes: NonZeroU16, } +// ── Rebuild helper functions ────────────────────────────────────────────────── +// +// Each function handles one fast path in `Index::rebuild`. They are free +// functions (not methods) so they can borrow `index` and `entry_builder` +// independently without fighting the borrow checker. + +/// Emits a narrowing uniform run as a sequence of full rows followed by one +/// partial row carrying the trailing newline. +/// +/// Preconditions (caller-enforced): +/// - `entry_builder` is empty +/// - the source entry has a trailing newline +/// - `run.cols() > index.columns` +/// - `run.info.cell_width > 0 && index.columns >= run.info.cell_width as usize` +fn emit_narrowed_uniform(run: &GraphemeRun, index: &mut Index) { + let cell_width = run.info.cell_width as usize; + let graphemes_per_row = index.columns / cell_width; + let byte_len = run.info.utf8_bytes.get() as usize; + let mut rem = run.count.get() as usize; + // A full row of wide chars leaves a 1-cell gap when columns is odd. + // The last cell becomes a leading-wide-char-spacer placeholder. + let full_row_spacer = cell_width == 2 && index.columns % 2 == 1; + + while rem > graphemes_per_row { + let content_offset: ByteOffset = index.content_len.into(); + index.content_len += graphemes_per_row * byte_len; + index.rows.push_back(Entry { + content_offset, + grapheme_sizing: GraphemeSizing::Uniform(GraphemeRun { + count: NonZeroU16::new(graphemes_per_row as u16).unwrap(), + info: run.info, + }), + has_trailing_newline: false, + ends_with_leading_wide_char_spacer: full_row_spacer, + }); + rem -= graphemes_per_row; + } + // The loop condition `rem > graphemes_per_row` guarantees rem >= 1. + debug_assert!(rem > 0); + // The final row ends with a trailing newline, not a wide-char overflow, + // so ends_with_leading_wide_char_spacer is always false here. + let content_offset: ByteOffset = index.content_len.into(); + index.content_len += rem * byte_len + 1; // +1 for newline + index.rows.push_back(Entry { + content_offset, + grapheme_sizing: GraphemeSizing::Uniform(GraphemeRun { + count: NonZeroU16::new(rem as u16).unwrap(), + info: run.info, + }), + has_trailing_newline: true, + ends_with_leading_wide_char_spacer: false, + }); +} + +/// Tries to emit a source row that fits entirely within `index.columns`, +/// including its trailing newline. +/// +/// Returns `true` if emitted; `false` if the row is wider than the new column +/// count (caller should fall through to the next path). +/// +/// Preconditions: `entry_builder` is empty; entry has a trailing newline. +fn try_emit_row_with_newline(entry: &Entry, row_runs: &[GraphemeRun], index: &mut Index) -> bool { + let (cells, byte_len) = match &entry.grapheme_sizing { + GraphemeSizing::Uniform(run) => ( + run.cols(), + run.count.get() as usize * run.info.utf8_bytes.get() as usize, + ), + GraphemeSizing::EmptyRow => (0, 0), + GraphemeSizing::NonUniform => { + let cells = row_runs.iter().map(GraphemeRun::cols).sum(); + let byte_len = row_runs + .iter() + .map(|r| r.count.get() as usize * r.info.utf8_bytes.get() as usize) + .sum(); + (cells, byte_len) + } + }; + if cells > index.columns { + return false; + } + let content_offset: ByteOffset = index.content_len.into(); + index.content_len += byte_len + 1; // +1 for newline + if matches!(entry.grapheme_sizing, GraphemeSizing::NonUniform) { + index + .grapheme_sizing + .insert(content_offset, row_runs.to_vec()); + } + index.rows.push_back(Entry { + content_offset, + grapheme_sizing: entry.grapheme_sizing, + has_trailing_newline: true, + ends_with_leading_wide_char_spacer: false, + }); + true +} + +/// Tries to absorb a soft-wrapped source row into `entry_builder` when the +/// row's cells fit entirely within `columns`. +/// +/// Returns `true` if absorbed (caller should `continue` to the next source +/// row); `false` if the row is too wide and needs splitting (fall through). +/// +/// Preconditions: `entry_builder` is empty; entry has no trailing newline. +fn try_accumulate_softwrap( + entry: &Entry, + row_runs: &[GraphemeRun], + entry_builder: &mut EntryBuilder, + columns: usize, +) -> bool { + match &entry.grapheme_sizing { + GraphemeSizing::Uniform(run) if run.cols() <= columns => { + entry_builder.num_cells += run.cols(); + entry_builder.incr_content_offset += + run.count.get() as usize * run.info.utf8_bytes.get() as usize; + entry_builder.grapheme_runs.push(*run); + true + } + GraphemeSizing::EmptyRow => true, // zero-width row — nothing to absorb + GraphemeSizing::NonUniform => { + let cells: usize = row_runs.iter().map(GraphemeRun::cols).sum(); + if cells > columns { + return false; + } + for run in row_runs { + entry_builder.num_cells += run.cols(); + entry_builder.incr_content_offset += + run.count.get() as usize * run.info.utf8_bytes.get() as usize; + match entry_builder.grapheme_runs.last_mut() { + Some(last) if last.info == run.info => { + checked_add_run_count(&mut last.count, run.count.get()); + } + _ => entry_builder.grapheme_runs.push(*run), + } + } + true + } + GraphemeSizing::Uniform(_) => false, // cols() > columns — needs splitting + } +} + +/// Tries to handle a uniform run using arithmetic carry-over, accounting for +/// any graphemes already accumulated in `entry_builder` from previous +/// soft-wrapped source rows. +/// +/// Handles `cell_width == 1` (ASCII/single-width) and `cell_width == 2` +/// (wide chars). Other cell widths fall through to the medium path. +/// +/// Returns `true` if handled; `false` to fall through to the medium path. +fn try_emit_carryover_uniform( + run: &GraphemeRun, + has_trailing_newline: bool, + entry_builder: &mut EntryBuilder, + index: &mut Index, +) -> bool { + let cell_width = run.info.cell_width as usize; + if cell_width == 0 || cell_width > 2 { + return false; + } + let count = run.count.get() as usize; + let byte_len = run.info.utf8_bytes.get() as usize; + let columns = index.columns; + // graphemes_per_row: how many graphemes of this width fit in a complete row. + // For wide chars (cell_width == 2) with an odd column count, there is a + // 1-cell remainder that cannot hold another wide char — rows filled to + // that boundary get ends_with_leading_wide_char_spacer set. + let graphemes_per_row = columns / cell_width; + let full_row_spacer = cell_width == 2 && columns % 2 == 1; + let remaining_cells = columns.saturating_sub(entry_builder.num_cells); + let remaining_graphemes = remaining_cells / cell_width; + + // Sub-case A: builder is exactly full — flush it, then process the run + // as if starting from an empty builder. + if entry_builder.num_cells >= columns && graphemes_per_row > 0 { + entry_builder.flush_to_index(index); + if count > graphemes_per_row { + // Fill and flush one full row via the builder, then use direct + // arithmetic for the remainder. + entry_builder.num_cells = graphemes_per_row * cell_width; + entry_builder.incr_content_offset += graphemes_per_row * byte_len; + entry_builder.grapheme_runs.push(GraphemeRun { + count: NonZeroU16::new(graphemes_per_row as u16).unwrap(), + info: run.info, + }); + if full_row_spacer { + entry_builder.add_leading_wide_char_spacer(); + } + entry_builder.flush_to_index(index); + let mut rem = count - graphemes_per_row; + // Strict `>` ensures rem stays in [1, graphemes_per_row] after the + // loop — with `>=` rem could reach 0, causing `has_trailing_newline` + // to emit an empty row instead of annotating the last content row. + while rem > graphemes_per_row { + let content_offset: ByteOffset = index.content_len.into(); + index.content_len += graphemes_per_row * byte_len; + index.rows.push_back(Entry { + content_offset, + grapheme_sizing: GraphemeSizing::Uniform(GraphemeRun { + count: NonZeroU16::new(graphemes_per_row as u16).unwrap(), + info: run.info, + }), + has_trailing_newline: false, + ends_with_leading_wide_char_spacer: full_row_spacer, + }); + rem -= graphemes_per_row; + } + // rem is in [1, graphemes_per_row] — never zero (see loop comment). + debug_assert!(rem > 0); + entry_builder.num_cells = rem * cell_width; + entry_builder.incr_content_offset += rem * byte_len; + entry_builder.grapheme_runs.push(GraphemeRun { + count: NonZeroU16::new(rem as u16).unwrap(), + info: run.info, + }); + } else { + // Entire run fits in a fresh row. + entry_builder.num_cells = count * cell_width; + entry_builder.incr_content_offset += count * byte_len; + match entry_builder.grapheme_runs.last_mut() { + Some(last) if last.info == run.info => { + checked_add_run_count(&mut last.count, count as u16); + } + _ => entry_builder.grapheme_runs.push(*run), + } + } + if has_trailing_newline { + entry_builder.add_trailing_newline(); + entry_builder.flush_to_index(index); + } + return true; + } + + // Sub-case B: run straddles the row boundary — fill the current partial + // row, flush, then use arithmetic for the remainder. + if remaining_graphemes > 0 && remaining_graphemes < count && graphemes_per_row > 0 { + // For wide chars with an odd column count the partial row may have a + // 1-cell gap that cannot fit another wide char. + let partial_row_spacer = cell_width == 2 && remaining_cells % 2 == 1; + entry_builder.num_cells += remaining_graphemes * cell_width; + entry_builder.incr_content_offset += remaining_graphemes * byte_len; + match entry_builder.grapheme_runs.last_mut() { + Some(last) if last.info == run.info => { + checked_add_run_count(&mut last.count, remaining_graphemes as u16); + } + _ => entry_builder.grapheme_runs.push(GraphemeRun { + count: NonZeroU16::new(remaining_graphemes as u16).unwrap(), + info: run.info, + }), + } + if partial_row_spacer { + entry_builder.add_leading_wide_char_spacer(); + } + entry_builder.flush_to_index(index); + + let mut rem = count - remaining_graphemes; + // Same strict `>` invariant as Sub-case A. + while rem > graphemes_per_row { + let content_offset: ByteOffset = index.content_len.into(); + index.content_len += graphemes_per_row * byte_len; + index.rows.push_back(Entry { + content_offset, + grapheme_sizing: GraphemeSizing::Uniform(GraphemeRun { + count: NonZeroU16::new(graphemes_per_row as u16).unwrap(), + info: run.info, + }), + has_trailing_newline: false, + ends_with_leading_wide_char_spacer: full_row_spacer, + }); + rem -= graphemes_per_row; + } + if rem > 0 { + entry_builder.num_cells = rem * cell_width; + entry_builder.incr_content_offset += rem * byte_len; + entry_builder.grapheme_runs.push(GraphemeRun { + count: NonZeroU16::new(rem as u16).unwrap(), + info: run.info, + }); + } + if has_trailing_newline { + entry_builder.add_trailing_newline(); + entry_builder.flush_to_index(index); + } + return true; + } + + // Sub-case C: entire run fits in the remaining space of the current row. + if count <= remaining_graphemes && entry_builder.num_cells + count * cell_width <= columns { + entry_builder.num_cells += count * cell_width; + entry_builder.incr_content_offset += count * byte_len; + match entry_builder.grapheme_runs.last_mut() { + Some(last) if last.info == run.info => { + checked_add_run_count(&mut last.count, count as u16); + } + _ => entry_builder.grapheme_runs.push(*run), + } + if has_trailing_newline { + entry_builder.add_trailing_newline(); + entry_builder.flush_to_index(index); + } + return true; + } + + false // fall through to the medium path +} + +/// Processes a slice of grapheme runs into output index rows, splitting at +/// column boundaries. +/// +/// Runs that fit entirely within the remaining row space are bulk-accumulated +/// via `extend_from_slice` — no per-run branches, cache-friendly, and +/// vectorizable by the compiler. Only the single run that straddles a row +/// boundary requires per-grapheme arithmetic via `process_graphemes_batch`. +/// +/// This is used by the medium path of `Index::rebuild` to replace the old +/// per-run fit-check loop, trading O(runs) branch-heavy operations for +/// O(output_rows) boundary events plus O(runs) branch-free arithmetic. +fn emit_runs( + runs: &[GraphemeRun], + has_trailing_newline: bool, + entry_builder: &mut EntryBuilder, + index: &mut Index, +) { + let mut run_idx = 0; + + while run_idx < runs.len() { + // Scan forward to find how many consecutive runs fit entirely in the + // remaining row space. This is pure arithmetic — no Vec mutations. + let scan_start = run_idx; + let mut col = entry_builder.num_cells; + while run_idx < runs.len() { + let run_cols = runs[run_idx].cols(); + if col + run_cols > index.columns { + break; + } + col += run_cols; + run_idx += 1; + } + + // Bulk-accumulate all fitting runs with a single extend_from_slice. + let fitting = &runs[scan_start..run_idx]; + if !fitting.is_empty() { + // Sum bytes in a branch-free pass (auto-vectorizable). + let bytes: usize = fitting + .iter() + .map(|r| r.count.get() as usize * r.info.utf8_bytes.get() as usize) + .sum(); + entry_builder.incr_content_offset += bytes; + entry_builder.num_cells = col; + + // Merge with the last existing run if the info matches, then + // extend the rest in one slice copy. + let skip_first = if let Some(last) = entry_builder.grapheme_runs.last_mut() { + if last.info == fitting[0].info { + checked_add_run_count(&mut last.count, fitting[0].count.get()); + true + } else { + false + } + } else { + false + }; + entry_builder + .grapheme_runs + .extend_from_slice(if skip_first { &fitting[1..] } else { fitting }); + } + + // Handle the one run that straddles the row boundary (if any). + if run_idx < runs.len() { + let br = &runs[run_idx]; + entry_builder.process_graphemes_batch(br.info, br.count.get(), index); + run_idx += 1; + } + } + + if has_trailing_newline { + entry_builder.add_trailing_newline(); + entry_builder.flush_to_index(index); + } +} + +fn checked_add_run_count(count: &mut NonZeroU16, add: u16) { + *count = count + .checked_add(add) + .expect("should not have more than 2^16 graphemes in a single row"); +} + #[cfg(test)] #[path = "index_tests.rs"] mod tests; diff --git a/crates/warp_terminal/src/model/grid/flat_storage/index_tests.rs b/crates/warp_terminal/src/model/grid/flat_storage/index_tests.rs index b7e6da2cf8e..7e0e8f31efc 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/index_tests.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/index_tests.rs @@ -262,6 +262,40 @@ fn test_push_extra_row_onto_index_with_softwrapped_first_line() { assert_eq!(storage.index.rows.len(), 2); } +#[test] +fn test_truncate_front_clamps_count() { + let mut index = Index::new(5, Some(2)); + + let mut eb = index.start_row(); + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let mut eb = index.start_row(); + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let new_start_offset = index.truncate_front(99); + + assert_eq!(new_start_offset, ByteOffset::from(4)); + assert_eq!(index.len(), 0); +} + +#[test] +#[should_panic(expected = "should not have more than 2^16 graphemes in a single row")] +fn test_rebuild_panics_on_run_count_overflow() { + let mut index = Index::new(1, Some(u16::MAX as usize + 1)); + + for _ in 0..=(u16::MAX as usize) { + let mut eb = index.start_row(); + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + eb.append_to_index(&mut index); + } + + let _ = Index::rebuild(&index, u16::MAX as usize + 1); +} + #[test] fn test_cell_type() { // 1: 😀😃 @@ -374,3 +408,743 @@ mod offset_point_conversion { assert_eq!(point, original_point); } } + +#[cfg(test)] +mod property_tests { + use std::num::NonZeroU16; + + use super::*; + + // Inline xorshift PRNG - no external dependencies + struct Rng(u64); + impl Rng { + fn new(seed: u64) -> Self { + Self(seed) + } + fn next(&mut self) -> u64 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 7; + self.0 ^= self.0 << 17; + self.0 + } + fn range(&mut self, lo: usize, hi: usize) -> usize { + lo + (self.next() as usize % (hi - lo)) + } + } + + fn random_index(rng: &mut Rng, rows: usize, cols: usize) -> Index { + let mut index = Index::new(cols, Some(rows)); + let ascii_info = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(1).unwrap(), + }; + let wide_info = GraphemeInfo { + cell_width: 2, + utf8_bytes: NonZeroU16::new(3).unwrap(), + }; + let multi_info = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(3).unwrap(), + }; + + for _ in 0..rows { + let mut eb = index.start_row(); + let mut cells_used = 0; + while cells_used < cols { + let kind = rng.next() % 10; + let info = if kind < 6 { + ascii_info + } else if kind < 8 { + wide_info + } else { + multi_info + }; + if cells_used + info.cell_width as usize > cols { + break; + } + eb.process_grapheme_info_unchecked(info); + cells_used += info.cell_width as usize; + } + if !rng.next().is_multiple_of(4) { + eb.add_trailing_newline(); + } + eb.append_to_index(&mut index); + } + index + } + + #[test] + fn prop_content_len_preserved() { + let mut rng = Rng::new(12345); + for _ in 0..100 { + let cols = rng.range(10, 200); + let rows = rng.range(10, 500); + let new_cols = rng.range(10, 200); + let index = random_index(&mut rng, rows, cols); + let original_content_len = index.content_len; + let rebuilt = Index::rebuild(&index, new_cols); + assert_eq!( + rebuilt.content_len, original_content_len, + "content_len mismatch: cols={cols}->{new_cols}, rows={rows}" + ); + } + } + + #[test] + fn prop_no_row_exceeds_columns() { + let mut rng = Rng::new(67890); + for _ in 0..100 { + let cols = rng.range(10, 200); + let rows = rng.range(10, 500); + let new_cols = rng.range(10, 200); + let index = random_index(&mut rng, rows, cols); + let rebuilt = Index::rebuild(&index, new_cols); + for row_idx in 0..rebuilt.len() { + if let Some(infos) = rebuilt.grapheme_infos_for_row(row_idx) { + let total_cells: usize = infos.map(|i| i.cell_width as usize).sum(); + assert!( + total_cells <= new_cols, + "row {row_idx} has {total_cells} cells but max is {new_cols}" + ); + } + } + } + } + + #[test] + fn prop_idempotent_rebuild() { + let mut rng = Rng::new(11111); + for _ in 0..50 { + let cols = rng.range(10, 200); + let rows = rng.range(10, 200); + let new_cols = rng.range(10, 200); + let index = random_index(&mut rng, rows, cols); + let rebuilt_once = Index::rebuild(&index, new_cols); + let rebuilt_twice = Index::rebuild(&rebuilt_once, new_cols); + assert_eq!(rebuilt_once.len(), rebuilt_twice.len()); + assert_eq!(rebuilt_once.content_len, rebuilt_twice.content_len); + } + } + + #[test] + fn prop_total_cells_preserved() { + let mut rng = Rng::new(99999); + for _ in 0..100 { + let cols = rng.range(10, 200); + let rows = rng.range(10, 500); + let new_cols = rng.range(10, 200); + let index = random_index(&mut rng, rows, cols); + + let original_cells: usize = (0..index.len()) + .filter_map(|i| index.grapheme_infos_for_row(i)) + .flatten() + .map(|info| info.cell_width as usize) + .sum(); + + let rebuilt = Index::rebuild(&index, new_cols); + let rebuilt_cells: usize = (0..rebuilt.len()) + .filter_map(|i| rebuilt.grapheme_infos_for_row(i)) + .flatten() + .map(|info| info.cell_width as usize) + .sum(); + + assert_eq!( + original_cells, rebuilt_cells, + "total cells mismatch: cols={cols}->{new_cols}" + ); + } + } +} + +#[cfg(test)] +mod differential_tests { + use std::num::NonZeroU16; + + use super::*; + + struct Rng(u64); + impl Rng { + fn new(seed: u64) -> Self { + Self(seed) + } + fn next(&mut self) -> u64 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 7; + self.0 ^= self.0 << 17; + self.0 + } + fn range(&mut self, lo: usize, hi: usize) -> usize { + lo + (self.next() as usize % (hi - lo)) + } + } + + fn random_index(rng: &mut Rng, rows: usize, cols: usize) -> Index { + let mut index = Index::new(cols, Some(rows)); + let ascii_info = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(1).unwrap(), + }; + let wide_info = GraphemeInfo { + cell_width: 2, + utf8_bytes: NonZeroU16::new(3).unwrap(), + }; + let multi_info = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(3).unwrap(), + }; + + for _ in 0..rows { + let mut eb = index.start_row(); + let mut cells_used = 0; + while cells_used < cols { + let kind = rng.next() % 10; + let info = if kind < 6 { + ascii_info + } else if kind < 8 { + wide_info + } else { + multi_info + }; + if cells_used + info.cell_width as usize > cols { + break; + } + eb.process_grapheme_info_unchecked(info); + cells_used += info.cell_width as usize; + } + if !rng.next().is_multiple_of(4) { + eb.add_trailing_newline(); + } + eb.append_to_index(&mut index); + } + index + } + + fn assert_indexes_equal(optimized: &Index, baseline: &Index, context: &str) { + assert_eq!( + optimized.len(), + baseline.len(), + "{context}: row count mismatch: optimized={}, baseline={}", + optimized.len(), + baseline.len() + ); + assert_eq!( + optimized.content_len, baseline.content_len, + "{context}: content_len mismatch: optimized={}, baseline={}", + optimized.content_len, baseline.content_len + ); + for row_idx in 0..optimized.len() { + let opt_entry = optimized.get_entry(row_idx).unwrap(); + let base_entry = baseline.get_entry(row_idx).unwrap(); + assert_eq!( + opt_entry.content_offset, base_entry.content_offset, + "{context}: row {row_idx} content_offset mismatch" + ); + assert_eq!( + opt_entry.has_trailing_newline, base_entry.has_trailing_newline, + "{context}: row {row_idx} has_trailing_newline mismatch" + ); + assert_eq!( + opt_entry.ends_with_leading_wide_char_spacer, + base_entry.ends_with_leading_wide_char_spacer, + "{context}: row {row_idx} ends_with_leading_wide_char_spacer mismatch" + ); + // Compare grapheme content + let opt_infos: Vec<_> = optimized + .grapheme_infos_for_row(row_idx) + .map(|it| it.collect()) + .unwrap_or_default(); + let base_infos: Vec<_> = baseline + .grapheme_infos_for_row(row_idx) + .map(|it| it.collect()) + .unwrap_or_default(); + assert_eq!( + opt_infos, base_infos, + "{context}: row {row_idx} grapheme infos differ" + ); + } + } + + #[test] + fn differential_random_inputs() { + let mut rng = Rng::new(0xDEADBEEF); + for trial in 0..200 { + let cols = rng.range(5, 300); + let rows = rng.range(1, 500); + let new_cols = rng.range(5, 300); + let index = random_index(&mut rng, rows, cols); + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + + assert_indexes_equal( + &optimized, + &baseline, + &format!("trial {trial}: {cols}cols x {rows}rows -> {new_cols}cols"), + ); + } + } + + #[test] + fn differential_widening_all_row_types() { + let mut rng = Rng::new(0xCAFE); + for trial in 0..100 { + let cols = rng.range(10, 80); + let rows = rng.range(10, 200); + let new_cols = rng.range(cols + 1, cols + 100); // always wider + let index = random_index(&mut rng, rows, cols); + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + + assert_indexes_equal( + &optimized, + &baseline, + &format!("widen trial {trial}: {cols}->{new_cols}, {rows} rows"), + ); + } + } + + #[test] + fn differential_narrowing_all_row_types() { + let mut rng = Rng::new(0xBEEF); + for trial in 0..100 { + let cols = rng.range(40, 200); + let rows = rng.range(10, 200); + let new_cols = rng.range(5, cols); // always narrower + let index = random_index(&mut rng, rows, cols); + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + + assert_indexes_equal( + &optimized, + &baseline, + &format!("narrow trial {trial}: {cols}->{new_cols}, {rows} rows"), + ); + } + } + + #[test] + fn differential_extreme_dimensions() { + let cases: &[(usize, usize, usize)] = &[ + (1, 100, 200), // single row widening + (1, 200, 1), // single row narrowing to 1 col + (1000, 1, 80), // many single-cell rows + (10, 1000, 50), // very wide rows narrowing + (100, 80, 80), // same width (should be identity) + (500, 80, 79), // off-by-one narrowing + (500, 80, 81), // off-by-one widening + ]; + + for &(rows, old_cols, new_cols) in cases { + let mut rng = Rng::new((rows * old_cols * new_cols) as u64); + let index = random_index(&mut rng, rows, old_cols); + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + + assert_indexes_equal( + &optimized, + &baseline, + &format!("{rows}rows x {old_cols}cols -> {new_cols}cols"), + ); + } + } + + #[test] + fn differential_all_softwrapped() { + // Worst case for our optimization: no newlines at all + let mut rng = Rng::new(0x5F5F); + let ascii_info = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(1).unwrap(), + }; + for trial in 0..50 { + let cols = rng.range(20, 120); + let rows = rng.range(50, 300); + let new_cols = rng.range(5, 200); + + let mut index = Index::new(cols, Some(rows)); + for _ in 0..rows { + let mut eb = index.start_row(); + for _ in 0..cols { + eb.process_grapheme_info_unchecked(ascii_info); + } + // No trailing newline — fully softwrapped + eb.append_to_index(&mut index); + } + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + + assert_indexes_equal( + &optimized, + &baseline, + &format!("softwrap trial {trial}: {cols}->{new_cols}, {rows} rows"), + ); + } + } + + #[test] + fn differential_mixed_newline_patterns() { + // Various patterns of newline placement + let mut rng = Rng::new(0xAAAA); + let ascii_info = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(1).unwrap(), + }; + let wide_info = GraphemeInfo { + cell_width: 2, + utf8_bytes: NonZeroU16::new(3).unwrap(), + }; + + for trial in 0..50 { + let cols = rng.range(10, 100); + let rows = rng.range(20, 200); + let new_cols = rng.range(5, 150); + + let mut index = Index::new(cols, Some(rows)); + for i in 0..rows { + let mut eb = index.start_row(); + let mut cells = 0; + // Alternate between full ASCII rows, partial rows, and wide char rows + match i % 5 { + 0 => { + // Full ASCII row + while cells < cols { + eb.process_grapheme_info_unchecked(ascii_info); + cells += 1; + } + } + 1 => { + // Partial row (half full) + let target = cols / 2; + while cells < target { + eb.process_grapheme_info_unchecked(ascii_info); + cells += 1; + } + } + 2 => { + // Wide char row + while cells + 2 <= cols { + eb.process_grapheme_info_unchecked(wide_info); + cells += 2; + } + } + 3 => { + // Mixed row + while cells < cols { + if rng.next().is_multiple_of(3) && cells + 2 <= cols { + eb.process_grapheme_info_unchecked(wide_info); + cells += 2; + } else { + eb.process_grapheme_info_unchecked(ascii_info); + cells += 1; + } + } + } + _ => { + // Empty row (just newline) + } + } + // Newline on odd rows only (creates groups of softwrapped lines) + if i % 2 == 1 { + eb.add_trailing_newline(); + } + eb.append_to_index(&mut index); + } + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + + assert_indexes_equal( + &optimized, + &baseline, + &format!("mixed trial {trial}: {cols}->{new_cols}, {rows} rows"), + ); + } + } + + /// Regression test: `try_emit_carryover_uniform` Sub-case A must use + /// strict `>` in its inner loop, not `>=`. + /// + /// When `count - graphemes_per_row` is an exact multiple of + /// `graphemes_per_row` the `>=` variant reduces `rem` to 0, causing the + /// trailing-newline path to emit an empty row instead of annotating the + /// last content row. + /// + /// Trigger: soft-wrapped row fills the builder to exactly `columns`, then + /// the next row is a uniform run of `2 * columns` graphemes with a + /// trailing newline. + #[test] + fn carryover_uniform_sub_a_exact_multiple_newline() { + // Source: 40-cell soft-wrap + 80-cell newline row, rebuild to 40 cols. + // entry_builder accumulates 40 cells from row 1 → Sub-case A triggers + // with count=80, graphemes_per_row=40, rem=40. + let mut index = Index::new(80, Some(2)); + + let mut eb = index.start_row(); + for _ in 0..40 { + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + } + eb.append_to_index(&mut index); // no newline — soft-wrap + + let mut eb = index.start_row(); + for _ in 0..80 { + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + } + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let optimized = Index::rebuild(&index, 40); + let baseline = Index::rebuild_baseline(&index, 40); + assert_indexes_equal(&optimized, &baseline, "carryover sub-A exact multiple"); + assert_eq!( + optimized.content_len, index.content_len, + "content_len not preserved" + ); + } + + /// Regression test: `try_emit_carryover_uniform` Sub-case B must also use + /// strict `>` in its inner loop. + /// + /// Trigger: partial soft-wrapped row leaves `remaining_graphemes` cells in + /// the builder such that `count - remaining_graphemes` is an exact multiple + /// of `graphemes_per_row`. + #[test] + fn carryover_uniform_sub_b_exact_multiple_newline() { + // Source: 20-cell soft-wrap + 60-cell newline row, rebuild to 40 cols. + // entry_builder has 20 cells → remaining_graphemes=20, rem=40=GPR. + let mut index = Index::new(80, Some(2)); + + let mut eb = index.start_row(); + for _ in 0..20 { + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + } + eb.append_to_index(&mut index); // no newline — soft-wrap + + let mut eb = index.start_row(); + for _ in 0..60 { + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + } + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let optimized = Index::rebuild(&index, 40); + let baseline = Index::rebuild_baseline(&index, 40); + assert_indexes_equal(&optimized, &baseline, "carryover sub-B exact multiple"); + assert_eq!( + optimized.content_len, index.content_len, + "content_len not preserved" + ); + } + + /// Regression test: `try_emit_carryover_uniform` now handles `cell_width == 2` + /// (wide chars) and correctly sets `ends_with_leading_wide_char_spacer` on + /// rows whose wide-char content doesn't reach an even column boundary. + /// + /// Uses an odd column count (7) so that every full wide-char row leaves a + /// 1-cell gap and must set the spacer flag — exercises both `full_row_spacer` + /// (inner direct-emit rows) and `partial_row_spacer` (the initial builder + /// flush in Sub-case B). + #[test] + fn carryover_uniform_wide_char_odd_columns() { + // Source: 4 ASCII (no nl) + 5 wide chars (with nl) in a 10-col index. + // Rebuild to 7 cols (odd). + // + // Expected output (verified against baseline): + // row 0: [4 ascii + 1 wide], ends_with_leading_wide_char_spacer=true + // row 1: [3 wide], ends_with_leading_wide_char_spacer=true + // row 2: [1 wide + nl], ends_with_leading_wide_char_spacer=false + let mut index = Index::new(10, Some(2)); + + let mut eb = index.start_row(); + for _ in 0..4 { + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + } + eb.append_to_index(&mut index); // no newline + + let mut eb = index.start_row(); + for _ in 0..5 { + eb.process_grapheme_info_unchecked(EMOJI_GRAPHEME_INFO); + } + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let optimized = Index::rebuild(&index, 7); + let baseline = Index::rebuild_baseline(&index, 7); + assert_indexes_equal(&optimized, &baseline, "carryover wide char odd cols"); + assert_eq!( + optimized.content_len, index.content_len, + "content_len not preserved" + ); + } + + /// Broader sweep: wide-char carry-over across various column widths including + /// odd counts, large runs, and multiple full-row direct-emits. + #[test] + fn carryover_uniform_wide_char_sweep() { + // (old_cols, new_cols) pairs that exercise wide-char carry-over paths + let cases: &[(usize, usize, usize, usize)] = &[ + // (old_cols, new_cols, ascii_prefix_len, wide_count) + (10, 7, 4, 5), // odd new_cols, Sub-case B + full_row_spacer + (10, 6, 4, 5), // even new_cols, Sub-case B no spacer + (20, 7, 3, 8), // longer run, multiple direct-emit rows + (20, 9, 5, 7), // odd new_cols, Sub-case B with partial_row_spacer + (20, 10, 6, 7), // even new_cols, no spacers needed + (30, 7, 2, 12), // many full rows through direct-emit loop + (14, 7, 0, 7), // no ASCII prefix — Sub-case A (builder full from softwrap) + ]; + + for &(old_cols, new_cols, ascii_len, wide_count) in cases { + let mut index = Index::new(old_cols, Some(2)); + + // Row 1: ASCII prefix, soft-wrapped. + if ascii_len > 0 { + let mut eb = index.start_row(); + for _ in 0..ascii_len { + eb.process_grapheme_info_unchecked(ASCII_GRAPHEME_INFO); + } + eb.append_to_index(&mut index); + } + + // Row 2 (or row 1 if no prefix): wide chars with newline. + // If no prefix we use a source row of wide chars that fills + // old_cols exactly so it appears as a soft-wrapped row followed + // by a second wide-char row with newline. + if ascii_len == 0 { + // First source row: enough wide chars to fill old_cols (soft-wrap) + let mut eb = index.start_row(); + let fill = old_cols / 2; + for _ in 0..fill { + eb.process_grapheme_info_unchecked(EMOJI_GRAPHEME_INFO); + } + eb.append_to_index(&mut index); + } + + let mut eb = index.start_row(); + for _ in 0..wide_count { + eb.process_grapheme_info_unchecked(EMOJI_GRAPHEME_INFO); + } + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + assert_indexes_equal( + &optimized, + &baseline, + &format!( + "wide-char sweep: old={old_cols}, new={new_cols}, ascii={ascii_len}, wide={wide_count}" + ), + ); + } + } + + /// Regression test: `emit_narrowed_uniform` (fast path A) must set + /// `ends_with_leading_wide_char_spacer` on inner loop rows when the column + /// count is odd and cell_width == 2. + /// + /// A full row of wide chars fills `(columns / 2) * 2 = columns - 1` cells, + /// leaving a 1-cell gap that acts as a leading-wide-char-spacer. + /// Previously the flag was hardcoded to `false`, causing a mismatch with + /// the baseline (`process_graphemes_batch`) which correctly sets it. + #[test] + fn narrowed_uniform_wide_char_spacer_odd_columns() { + // (old_cols, new_cols, wide_count): source has wide_count wide chars + // with a trailing newline, rebuild to an odd new_cols that forces + // multiple full-row emits through emit_narrowed_uniform. + let cases: &[(usize, usize, usize)] = &[ + (20, 7, 10), // 10 wide → 3+3+4 rows at new=7 (each full row needs spacer) + (20, 5, 10), // 10 wide → 5+5 rows at new=5 (even cols, no spacer) + (30, 7, 15), // 15 wide → 3+3+3+3+3 at new=7, each full row needs spacer + (30, 9, 15), // 15 wide → 4+4+4+3 at new=9 (odd cols, spacer on full rows) + (20, 3, 10), // extreme narrowing, odd cols + (10, 7, 5), // minimal: 5 wide → 3+2, first full row needs spacer + ]; + + for &(old_cols, new_cols, wide_count) in cases { + let mut index = Index::new(old_cols, Some(1)); + let mut eb = index.start_row(); + for _ in 0..wide_count { + eb.process_grapheme_info_unchecked(EMOJI_GRAPHEME_INFO); + } + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + assert_indexes_equal( + &optimized, + &baseline, + &format!( + "narrowed wide-char spacer: old={old_cols}, new={new_cols}, count={wide_count}" + ), + ); + } + } + + /// Regression test: narrowing fast path must preserve `content_len` when + /// `count` is an exact multiple of `graphemes_per_row`. + /// + /// Because the loop uses `while rem > graphemes_per_row` (strict), `rem` + /// exits in `[1, graphemes_per_row]` and the `else` branch is technically + /// unreachable today. The fix restructures the code so `content_len` is + /// only incremented inside the branch that actually pushes a row, making + /// the intent clear and preventing a real double-count if the loop + /// condition is ever relaxed to `>=`. + #[test] + fn narrowing_uniform_exactly_divisible_newline() { + // Build several source indexes whose rows divide evenly at the new + // column width, so the narrowing fast path always lands in the + // `rem == 0` branch. + let ascii_info = GraphemeInfo { + cell_width: 1, + utf8_bytes: NonZeroU16::new(1).unwrap(), + }; + + // (old_cols, new_cols) pairs where old_cols % new_cols == 0 + let cases: &[(usize, usize)] = &[ + (80, 40), + (80, 20), + (80, 10), + (80, 8), + (80, 5), + (80, 4), + (100, 50), + (100, 25), + (120, 60), + (120, 40), + (120, 30), + (120, 24), + ]; + + for &(old_cols, new_cols) in cases { + // Single row, fully packed, with a trailing newline. + let mut index = Index::new(old_cols, Some(1)); + let mut eb = index.start_row(); + for _ in 0..old_cols { + eb.process_grapheme_info_unchecked(ascii_info); + } + eb.add_trailing_newline(); + eb.append_to_index(&mut index); + + let optimized = Index::rebuild(&index, new_cols); + let baseline = Index::rebuild_baseline(&index, new_cols); + + assert_indexes_equal( + &optimized, + &baseline, + &format!("divisible narrowing: {old_cols}->{new_cols}"), + ); + + // Also verify the total byte count is preserved. + assert_eq!( + optimized.content_len, index.content_len, + "content_len not preserved for {old_cols}->{new_cols}: \ + optimized={}, original={}", + optimized.content_len, index.content_len + ); + } + } +} diff --git a/crates/warp_terminal/src/model/grid/flat_storage/mod.rs b/crates/warp_terminal/src/model/grid/flat_storage/mod.rs index 7fc1099662e..136a055796d 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/mod.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/mod.rs @@ -31,11 +31,15 @@ use attribute_map::AttributeMap; use content::Content; use get_size::GetSize; use grapheme::Grapheme; +#[cfg(not(feature = "test-util"))] use index::Index; +#[cfg(feature = "test-util")] +pub use index::{GraphemeInfo, Index}; use itertools::Itertools; use string_offset::ByteOffset; use style::BgAndStyle; +use super::cell::LineLength as _; use super::row::Row; use super::{cell, CellType}; use crate::model::{ansi, Point}; @@ -190,6 +194,16 @@ impl FlatStorage { let start_offset = ByteOffset::from(self.content().end_offset()); let mut entry_builder = self.index.start_row(); + if self.try_push_dense_single_width_row( + row, + start_offset, + &mut entry_builder, + &mut fg_color, + &mut bg_and_style, + ) { + continue; + } + let mut last_cell: isize = -1; // Use an empty but pre-allocated buffer to collect characters from @@ -216,7 +230,7 @@ impl FlatStorage { continue; } - let mut needs_processing = !cell.is_empty(); + let mut needs_processing = cell.c != cell::DEFAULT_CHAR; if cell.fg != fg_color { needs_processing = true; fg_color = cell.fg; @@ -268,6 +282,66 @@ impl FlatStorage { } } + fn try_push_dense_single_width_row( + &mut self, + row: &Row, + start_offset: ByteOffset, + entry_builder: &mut index::EntryBuilder, + fg_color: &mut ansi::Color, + bg_and_style: &mut BgAndStyle, + ) -> bool { + if row.occ != row.len() { + return false; + } + + if row.dirty_cells().iter().any(|cell| { + cell.flags().intersects( + cell::Flags::WIDE_CHAR + | cell::Flags::WIDE_CHAR_SPACER + | cell::Flags::LEADING_WIDE_CHAR_SPACER, + ) + }) { + return false; + } + + let mut offset = start_offset; + for cell in row.dirty_cells() { + if cell.fg != *fg_color { + *fg_color = cell.fg; + self.fg_color_map.push_attribute_change(offset.., *fg_color); + } + if *bg_and_style != cell { + *bg_and_style = cell.into(); + self.bg_and_style_map + .push_attribute_change(offset.., *bg_and_style); + } + if let Some(marker) = cell.end_of_prompt_marker() { + self.end_of_prompt_marker = Some(EndOfPromptMarker { + offset, + has_extra_trailing_newline: marker.has_extra_trailing_newline, + }); + } + + let grapheme = Grapheme::new_from_cell(cell); + entry_builder.process_grapheme_info_unchecked(grapheme.sizing_info()); + self.content.push_grapheme(&grapheme); + offset += grapheme.len().as_usize(); + } + + let row_soft_wraps = row.occ == self.columns + && row[self.columns - 1] + .flags() + .intersects(cell::Flags::WRAPLINE); + if !row_soft_wraps { + entry_builder.add_trailing_newline(); + self.content.push_grapheme(&Grapheme::NEWLINE); + } + + entry_builder.flush_to_index(&mut self.index); + + true + } + /// Clears out the contents of flat storage. pub fn clear(&mut self) { // Drop all rows from the index. @@ -333,6 +407,25 @@ impl FlatStorage { self.index.content_offset_at_point(point) } + /// Returns the content offset for a point, clamping points in the blank + /// tail of a row to the row's last contentful cell. + pub fn content_offset_at_point_or_before(&self, point: Point) -> ByteOffset { + match self.content_offset_at_point(point) { + Ok(content_offset) => content_offset, + Err(index::ContentOffsetToPointError::ColumnExceedsContent { row, .. }) => { + let fallback_col = self + .rows_from(row) + .next() + .map(|row| row.line_length()) + .unwrap_or_default() + .saturating_sub(1); + self.content_offset_at_point(Point::new(row, fallback_col)) + .expect("fallback point should have a content offset") + } + Err(err) => panic!("unexpected cursor resize conversion failure: {err}"), + } + } + /// Returns the grid [`Point`] where the content at a given offset is /// located. /// diff --git a/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs b/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs index 2ca4054f849..1e3c2bbfa77 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs @@ -118,6 +118,22 @@ fn test_push_rows_with_color() { assert_eq!(storage.rows_from(0).next().unwrap().as_ref(), &row); } +#[test] +fn test_push_rows_dense_single_width_row_does_not_panic() { + let num_cols = 12; + let rows = "hello world\n".to_rows(num_cols); + + let mut storage = FlatStorage::new(num_cols, None, Some(2)); + storage.push_rows(&rows); + + let flat_rows = storage + .rows_from(0) + .map(|row| row.as_ref().clone()) + .collect_vec(); + + assert_rows_equal(&flat_rows, &rows); +} + #[test] fn test_push_rows_with_color_and_multibyte_chars() { let mut storage = FlatStorage::new(5, None, Some(2)); diff --git a/crates/warp_terminal/src/model/grid/flat_storage/row_iterator.rs b/crates/warp_terminal/src/model/grid/flat_storage/row_iterator.rs index 4e5942e2765..312a572619b 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/row_iterator.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/row_iterator.rs @@ -47,16 +47,18 @@ impl Iterator for RowIterator<'_> { let mut fg_color_iter = self.storage.fg_color_map.iter_from(start_offset); let mut bg_and_style_iter = self.storage.bg_and_style_map.iter_from(start_offset); + let mut content_cursor = self.storage.content().cursor_at(start_offset); let row = Rc::make_mut(&mut self.row); row.reset(&self.template); let mut current_offset = start_offset; for grapheme_info in self.storage.index.grapheme_infos_for_row(self.row_index)? { - let content = { - let start = current_offset; - let end = start + grapheme_info.utf8_bytes.get() as usize; - &self.storage.content()[start..end] + let grapheme_len = grapheme_info.utf8_bytes.get() as usize; + let content = if grapheme_len == 1 { + content_cursor.next_slice(1) + } else { + content_cursor.next_slice(grapheme_len) }; let grapheme = Grapheme::new_from_str_and_info(content, grapheme_info); @@ -74,8 +76,16 @@ impl Iterator for RowIterator<'_> { // incorrect (but easy to get wrong). This works, but I wonder if the // iterator returned by `AttributeMap` shouldn't actually implement // `Iterator` and should provide its own `next(&Grapheme)` function. - let fg = next_attribute(&mut fg_color_iter, &grapheme); - let BgAndStyle { bg, flags } = next_attribute(&mut bg_and_style_iter, &grapheme); + let fg = fg_color_iter + .next() + .expect("should never fail to provide value"); + let BgAndStyle { bg, flags } = bg_and_style_iter + .next() + .expect("should never fail to provide value"); + if grapheme_len > 1 { + fg_color_iter.skip_by(grapheme_len - 1); + bg_and_style_iter.skip_by(grapheme_len - 1); + } let cell_width = grapheme.cell_width(); if cell_width == 0 { @@ -154,13 +164,3 @@ impl Iterator for RowIterator<'_> { Some(self.row.clone()) } } - -/// Returns the next value for the given attribute iterator, given the current -/// grapheme. -/// -/// This must be used instead of [`Iterator::next`] in order to handle -/// multi-byte graphemes properly. -fn next_attribute(iter: &mut impl Iterator, grapheme: &Grapheme) -> T { - iter.nth(grapheme.len().as_usize() - 1) - .expect("should never fail to provide value") -} diff --git a/fuzz/fuzz_targets/fuzz_flat_storage_rebuild.rs b/fuzz/fuzz_targets/fuzz_flat_storage_rebuild.rs new file mode 100644 index 00000000000..6329a54f7d6 --- /dev/null +++ b/fuzz/fuzz_targets/fuzz_flat_storage_rebuild.rs @@ -0,0 +1,87 @@ +#![no_main] +use libfuzzer_sys::fuzz_target; +use std::num::NonZeroU16; +use warp_terminal::model::grid::flat_storage::index::{GraphemeInfo, Index}; + +fuzz_target!(|data: &[u8]| { + if data.len() < 4 { + return; + } + + // Use first 2 bytes for dimensions, rest for row content decisions + let old_cols = (data[0] as usize % 200) + 1; + let new_cols = (data[1] as usize % 200) + 1; + let num_rows = ((data[2] as usize) % 100) + 1; + + let ascii_info = GraphemeInfo { cell_width: 1, utf8_bytes: NonZeroU16::new(1).unwrap() }; + let wide_info = GraphemeInfo { cell_width: 2, utf8_bytes: NonZeroU16::new(3).unwrap() }; + + let mut index = Index::new(old_cols, Some(num_rows)); + + for i in 0..num_rows { + let mut eb = index.start_row(); + let byte_idx = 3 + (i % (data.len() - 3)); + let row_byte = data[byte_idx]; + + let mut cells = 0; + match row_byte % 4 { + 0 => { + // Full ASCII row + while cells < old_cols { + eb.process_grapheme_info(ascii_info, &mut index); + cells += 1; + } + } + 1 => { + // Wide char row + while cells + 2 <= old_cols { + eb.process_grapheme_info(wide_info, &mut index); + cells += 2; + } + } + 2 => { + // Mixed row + while cells < old_cols { + if row_byte.wrapping_add(cells as u8) % 3 == 0 && cells + 2 <= old_cols { + eb.process_grapheme_info(wide_info, &mut index); + cells += 2; + } else { + eb.process_grapheme_info(ascii_info, &mut index); + cells += 1; + } + } + } + _ => { + // Partial row + let target = old_cols / 2; + while cells < target { + eb.process_grapheme_info(ascii_info, &mut index); + cells += 1; + } + } + } + + if row_byte % 3 != 0 { + eb.add_trailing_newline(); + } + eb.append_to_index(&mut index); + } + + let original_content_len = index.content_len; + + // Rebuild with new columns + let rebuilt = Index::rebuild(&index, new_cols); + + // Invariant: content_len must be preserved + assert_eq!(rebuilt.content_len, original_content_len, + "content_len diverged: old_cols={old_cols}, new_cols={new_cols}, rows={num_rows}"); + + // Invariant: no row exceeds new column width + for row_idx in 0..rebuilt.len() { + if let Some(infos) = rebuilt.grapheme_infos_for_row(row_idx) { + let total: usize = infos.map(|i| i.cell_width as usize).sum(); + assert!(total <= new_cols, + "row {row_idx} has {total} cells, max is {new_cols}"); + } + } +}); From a6b54e2c5dd78724ec1df9d3e41db58134429549 Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Mon, 13 Jul 2026 15:07:17 -0500 Subject: [PATCH 2/5] perf(search): reduce fuzzy search hot-path work Optimize repeated fuzzy/file-search work and input edit checks. - Add ASCII fast paths for wildcard matching, session/search columns, and remote result columns. - Reuse parsed fuzzy path query metadata instead of re-scanning wildcard state per candidate. - Avoid repeated editor buffer reads while handling input edits and AI context checks. - Fix multibyte @ context handling and navigation query heuristics with targeted coverage. --- app/src/search/ai_context_menu/search.rs | 6 +- .../search/ai_context_menu/search_tests.rs | 9 ++ .../command_palette/navigation/search.rs | 49 +++++++- .../navigation/search_tests.rs | 41 ++++++- app/src/search/files/model.rs | 116 +++++++++++------- app/src/terminal/input.rs | 105 ++++++++-------- app/src/terminal/input_tests.rs | 30 +++++ app/src/workspace/view/global_search/model.rs | 3 + .../view/global_search/model_tests.rs | 13 ++ crates/fuzzy_match/src/fuzzy_tests.rs | 11 ++ crates/fuzzy_match/src/lib.rs | 110 +++++++++++++---- 11 files changed, 372 insertions(+), 121 deletions(-) create mode 100644 app/src/search/ai_context_menu/search_tests.rs diff --git a/app/src/search/ai_context_menu/search.rs b/app/src/search/ai_context_menu/search.rs index 98324766d45..463a9f10975 100644 --- a/app/src/search/ai_context_menu/search.rs +++ b/app/src/search/ai_context_menu/search.rs @@ -10,8 +10,12 @@ pub fn is_valid_search_query(is_navigation: bool, prev_query: &str, query: &str) // We need a simple heuristic to handle when somebody jumps to the end // of the line. Since spaces are valid characters, we only count // how many spaces the users likely jumped over between queries - let new_chars = query.chars().skip(prev_query.len()); + let new_chars = query.chars().skip(prev_query.chars().count()); return new_chars.filter(|c| *c == ' ').count() < MAX_NEW_SPACES; } true } + +#[cfg(test)] +#[path = "search_tests.rs"] +mod tests; diff --git a/app/src/search/ai_context_menu/search_tests.rs b/app/src/search/ai_context_menu/search_tests.rs new file mode 100644 index 00000000000..d0e2c80da56 --- /dev/null +++ b/app/src/search/ai_context_menu/search_tests.rs @@ -0,0 +1,9 @@ +use super::is_valid_search_query; + +#[test] +fn navigation_heuristic_counts_chars_not_bytes() { + let prev_query = "→"; + let query = "→ ls"; + + assert!(!is_valid_search_query(true, prev_query, query)); +} diff --git a/app/src/search/command_palette/navigation/search.rs b/app/src/search/command_palette/navigation/search.rs index e06406e91f7..ef5b62c5707 100644 --- a/app/src/search/command_palette/navigation/search.rs +++ b/app/src/search/command_palette/navigation/search.rs @@ -130,7 +130,12 @@ fn searchable_session_string_and_ranges( session: &SessionNavigationData, ) -> (String, SearchableSessionStringRanges) { let mut searchable_string = session.prompt().to_string(); - let prompt_end = session.prompt().chars().count(); + let prompt = session.prompt(); + let prompt_end = if prompt.is_ascii() { + prompt.len() + } else { + prompt.chars().count() + }; let command_range = match session.command_context() { CommandContext::LastRunCommand { @@ -142,7 +147,12 @@ fn searchable_session_string_and_ranges( searchable_string.push_str(last_run_command.as_str()); let start = prompt_end + 1; - let end = start + last_run_command.chars().count(); + let end = start + + if last_run_command.is_ascii() { + last_run_command.len() + } else { + last_run_command.chars().count() + }; Some(start..end) } CommandContext::RunningCommand { running_command } => { @@ -151,7 +161,12 @@ fn searchable_session_string_and_ranges( searchable_string.push_str(running_command.as_str()); let start = prompt_end + 1; - let end = start + running_command.chars().count(); + let end = start + + if running_command.is_ascii() { + running_command.len() + } else { + running_command.chars().count() + }; Some(start..end) } CommandContext::LastRunAIBlock { prompt } | CommandContext::RunningAIBlock { prompt } => { @@ -160,7 +175,12 @@ fn searchable_session_string_and_ranges( searchable_string.push_str(prompt.as_str()); let start = prompt_end + 1; - let end = start + prompt.chars().count(); + let end = start + + if prompt.is_ascii() { + prompt.len() + } else { + prompt.chars().count() + }; Some(start..end) } CommandContext::None => None, @@ -172,12 +192,22 @@ fn searchable_session_string_and_ranges( let hint_text_range = match &command_range { Some(command_range) => { let start = command_range.end + 1; - let end = start + command_info.hint_text.chars().count(); + let end = start + + if command_info.hint_text.is_ascii() { + command_info.hint_text.len() + } else { + command_info.hint_text.chars().count() + }; start..end } None => { let start = prompt_end + 1; - let end = start + command_info.hint_text.chars().count(); + let end = start + + if command_info.hint_text.is_ascii() { + command_info.hint_text.len() + } else { + command_info.hint_text.chars().count() + }; start..end } }; @@ -363,6 +393,13 @@ mod full_text_searcher { /// char-based indices that align with the char-based ranges used by /// [`SessionHighlightIndices`]. pub(super) fn byte_indices_to_char_indices(text: &str, byte_indices: Vec) -> Vec { + if text.is_ascii() { + return byte_indices + .into_iter() + .filter(|&byte_idx| byte_idx < text.len()) + .collect(); + } + let byte_to_char: HashMap = text .char_indices() .enumerate() diff --git a/app/src/search/command_palette/navigation/search_tests.rs b/app/src/search/command_palette/navigation/search_tests.rs index 55bae5a716e..2cdf8062773 100644 --- a/app/src/search/command_palette/navigation/search_tests.rs +++ b/app/src/search/command_palette/navigation/search_tests.rs @@ -1,5 +1,15 @@ +use warpui::{EntityId, WindowId}; + use super::full_text_searcher::byte_indices_to_char_indices; -use super::{SearchableSessionStringRanges, SessionHighlightIndices}; +use super::{ + searchable_session_string_and_ranges, SearchableSessionStringRanges, SessionHighlightIndices, +}; +use crate::pane_group::PaneId; +use crate::session_management::{ + CommandContext, SessionNavigationData, SessionNavigationPromptElements, +}; +use crate::terminal::shared_session::SharedSessionStatus; +use crate::workspace::PaneViewLocator; // ── byte_indices_to_char_indices ───────────────────────────────────── @@ -68,6 +78,35 @@ fn out_of_bounds_byte_indices_are_dropped() { ); } +#[test] +fn ascii_searchable_session_ranges_use_byte_lengths() { + let session = SessionNavigationData::new( + "abc".to_string(), + SessionNavigationPromptElements { + ps1_prompt_grid: None, + prompt_chip_snapshot: None, + }, + CommandContext::LastRunCommand { + last_run_command: "ls".to_string(), + mins_since_completion: Some(3), + }, + PaneViewLocator { + pane_group_id: EntityId::new(), + pane_id: PaneId::dummy_pane_id(), + }, + None, + false, + WindowId::new(), + SharedSessionStatus::NotShared, + ); + + let (searchable, ranges) = searchable_session_string_and_ranges(&session); + + assert_eq!(searchable, "abc ls Completed 3 minutes ago"); + assert_eq!(ranges.command_range, Some(4..6)); + assert_eq!(ranges.hint_text_range, 7..30); +} + // ── End-to-end: highlight pipeline with multi-byte prompt ──────────── /// Simulates the same range construction that `searchable_session_string_and_ranges` diff --git a/app/src/search/files/model.rs b/app/src/search/files/model.rs index 99299803722..840e1926750 100644 --- a/app/src/search/files/model.rs +++ b/app/src/search/files/model.rs @@ -402,54 +402,25 @@ impl FileSearchModel { /// Prioritizes filename matches with 2x weight compared to path matches /// Supports space-separated queries - all parts must match for a result pub fn fuzzy_match_path(path: &str, query: &str) -> Option { - if query.is_empty() { - return Some(FuzzyMatchResult::no_match()); - } - - // Split query by spaces to support multi-term searches - let query_parts: Vec<&str> = query.split_whitespace().collect(); - - if query_parts.is_empty() { - return Some(FuzzyMatchResult::no_match()); - } - - // If there's only one query part, use the original logic - if query_parts.len() == 1 { - return Self::fuzzy_match_path_single(path, query_parts[0]); - } - - // For multiple query parts, each part must match somewhere in the path - let mut combined_score = 0i64; - let mut all_matched_indices = Vec::new(); - - for query_part in &query_parts { - if let Some(part_result) = Self::fuzzy_match_path_single(path, query_part) { - combined_score += part_result.score; - all_matched_indices.extend(part_result.matched_indices); - } else { - // If any part doesn't match, the entire query fails - return None; - } - } - - // Remove duplicates and sort indices - all_matched_indices.sort_unstable(); - all_matched_indices.dedup(); - - Some(FuzzyMatchResult { - score: combined_score, - matched_indices: all_matched_indices, - }) + let query = FuzzyPathQuery::new(query); + query.match_path(path) } /// Helper method for single-term fuzzy matching (supports wildcards) pub fn fuzzy_match_path_single(path: &str, query: &str) -> Option { + Self::fuzzy_match_path_single_with_wildcard_flag(path, query, contains_wildcards(query)) + } + + fn fuzzy_match_path_single_with_wildcard_flag( + path: &str, + query: &str, + has_wildcards: bool, + ) -> Option { if query.is_empty() { return Some(FuzzyMatchResult::no_match()); } - // Check if query contains wildcards - if contains_wildcards(query) { + if has_wildcards { // Use wildcard matching for the entire path return match_wildcard_pattern_case_insensitive(path, query); } @@ -608,8 +579,7 @@ impl FileSearchModel { return original_score; } - let path_chars: Vec = path.chars().collect(); - let total_chars = path_chars.len(); + let total_chars = path.chars().count(); // Calculate weighted proximity score based on distance from end of path let proximity_bonus: i64 = matched_indices @@ -674,6 +644,68 @@ impl FileSearchModel { } } +struct FuzzyPathQueryPart<'a> { + text: &'a str, + has_wildcards: bool, +} + +struct FuzzyPathQuery<'a> { + parts: Vec>, +} + +impl<'a> FuzzyPathQuery<'a> { + fn new(query: &'a str) -> Self { + let parts = query + .split_whitespace() + .map(|text| FuzzyPathQueryPart { + has_wildcards: contains_wildcards(text), + text, + }) + .collect(); + + Self { parts } + } + + fn match_path(&self, path: &str) -> Option { + if self.parts.is_empty() { + return Some(FuzzyMatchResult::no_match()); + } + + if self.parts.len() == 1 { + let part = &self.parts[0]; + return FileSearchModel::fuzzy_match_path_single_with_wildcard_flag( + path, + part.text, + part.has_wildcards, + ); + } + + let mut combined_score = 0i64; + let mut all_matched_indices = Vec::new(); + + for part in &self.parts { + if let Some(part_result) = FileSearchModel::fuzzy_match_path_single_with_wildcard_flag( + path, + part.text, + part.has_wildcards, + ) { + combined_score += part_result.score; + all_matched_indices.extend(part_result.matched_indices); + } else { + return None; + } + } + + all_matched_indices.sort_unstable(); + all_matched_indices.dedup(); + + Some(FuzzyMatchResult { + score: combined_score, + matched_indices: all_matched_indices, + }) + } +} + impl SingletonEntity for FileSearchModel {} impl Entity for FileSearchModel { diff --git a/app/src/terminal/input.rs b/app/src/terminal/input.rs index 456cfa6467e..a4c5a610551 100644 --- a/app/src/terminal/input.rs +++ b/app/src/terminal/input.rs @@ -10165,50 +10165,61 @@ impl Input { let mut check_alias_expansion = false; let mut should_open_ai_context_menu = false; - let cursor_position = self.editor.read(ctx, |editor, editor_ctx| { - editor.start_byte_index_of_last_selection(editor_ctx) - }); - let is_alias_expansion_enabled = self.should_expand_aliases(ctx); let session_context = self.completion_session_context(ctx); + let (cursor_position, last_action, shell_family) = + self.editor.read(ctx, |editor, editor_ctx| { + ( + editor.start_byte_index_of_last_selection(editor_ctx), + editor.get_last_action(editor_ctx), + editor.shell_family().unwrap_or(ShellFamily::Posix), + ) + }); - self.editor.read(ctx, |editor, editor_ctx| { - let last_action = editor.get_last_action(editor_ctx); - if Some(PlainTextEditorViewAction::Space) == last_action - && *edit_origin == EditOrigin::UserTyped - { - check_alias_expansion = true; - } + let should_check_ai_context = FeatureFlag::AIContextMenuEnabled.is_enabled() + && (is_ai_input_enabled || FeatureFlag::AtMenuOutsideOfAIMode.is_enabled()) + && Some(PlainTextEditorViewAction::InsertChar) == last_action + && *edit_origin == EditOrigin::UserTyped; + let should_check_attachment_patterns = + AISettings::as_ref(ctx).is_any_ai_enabled(ctx) && edit_origin.is_user(); + let needs_buffer_text = should_check_ai_context || should_check_attachment_patterns; + let buffer_text = + needs_buffer_text.then(|| self.editor.as_ref(ctx).buffer_text(ctx)); + + if Some(PlainTextEditorViewAction::Space) == last_action + && *edit_origin == EditOrigin::UserTyped + { + check_alias_expansion = true; + } - // Check if "@" was just typed in a valid context - if FeatureFlag::AIContextMenuEnabled.is_enabled() - && (is_ai_input_enabled || FeatureFlag::AtMenuOutsideOfAIMode.is_enabled()) - && Some(PlainTextEditorViewAction::InsertChar) == last_action - && *edit_origin == EditOrigin::UserTyped - { - let buffer_text = editor.buffer_text(ctx); - let should_enable = self.should_enable_ai_context( - &buffer_text, - cursor_position.as_usize(), - is_alias_expansion_enabled, - session_context.as_ref(), - editor.shell_family().unwrap_or(ShellFamily::Posix), - ctx, - ); - if should_enable { - should_open_ai_context_menu = true; - } + // Check if "@" was just typed in a valid context + if should_check_ai_context { + let Some(buffer_text) = buffer_text.as_deref() else { + unreachable!("buffer text should be available for AI context checks"); + }; + let should_enable = self.should_enable_ai_context( + buffer_text, + cursor_position.as_usize(), + is_alias_expansion_enabled, + session_context.as_ref(), + shell_family, + ctx, + ); + if should_enable { + should_open_ai_context_menu = true; } + } - if SHORT_CIRCUIT_HIGHLIGHTING_ACTIONS.contains(&last_action) { - short_circuit_highlighting = true; - } - }); + if SHORT_CIRCUIT_HIGHLIGHTING_ACTIONS.contains(&last_action) { + short_circuit_highlighting = true; + } // Force AI mode if buffer contains any attachment patterns (blocks, drive objects, diffs) - if AISettings::as_ref(ctx).is_any_ai_enabled(ctx) && edit_origin.is_user() { - let buffer_text = self.buffer_text(ctx); - if Self::buffer_contains_attachment_patterns(&buffer_text) { + if should_check_attachment_patterns { + let Some(buffer_text) = buffer_text.as_deref() else { + unreachable!("buffer text should be available for attachment checks"); + }; + if Self::buffer_contains_attachment_patterns(buffer_text) { self.ensure_agent_mode_for_ai_features( false, Some(InputTypeAutoDetectionSource::AttachmentForcedAi), @@ -10218,15 +10229,12 @@ impl Input { } if should_open_ai_context_menu { - let cursor_pos = self.editor.read(ctx, |editor, ctx| { - editor.start_byte_index_of_last_selection(ctx) - }); self.suggestions_mode_model.update(ctx, |m, ctx| { m.set_mode( InputSuggestionsMode::AIContextMenu { filter_text: "".to_string(), // -1 since cursor is after the @ symbol - at_symbol_position: cursor_pos.as_usize().saturating_sub(1), + at_symbol_position: cursor_position.as_usize().saturating_sub(1), }, ctx, ); @@ -12012,18 +12020,19 @@ impl Input { return false; } - if buffer_text.chars().nth(cursor_position.saturating_sub(1)) != Some('@') { + let Some(before_cursor) = buffer_text.get(..cursor_position) else { + return false; + }; + let mut chars = before_cursor.chars().rev(); + + if chars.next() != Some('@') { return false; } // Check if '@' is at beginning of line or after non-alphanumeric - let is_valid_context = if cursor_position == 1 { - true // '@' is the first character - } else { - buffer_text - .chars() - .nth(cursor_position.saturating_sub(2)) - .is_some_and(|c| !c.is_alphanumeric()) + let is_valid_context = match chars.next() { + None => true, + Some(c) => !c.is_alphanumeric(), }; if !is_valid_context { diff --git a/app/src/terminal/input_tests.rs b/app/src/terminal/input_tests.rs index 5eb3bcf4be5..9f622ef2357 100644 --- a/app/src/terminal/input_tests.rs +++ b/app/src/terminal/input_tests.rs @@ -7922,6 +7922,36 @@ fn test_ai_context_menu_closes_when_space_immediately_after_at_symbol() { }); } +#[test] +fn test_ai_context_menu_opens_after_multibyte_prefix_before_at_symbol() { + let _ai_context_menu_enabled = FeatureFlag::AIContextMenuEnabled.override_enabled(true); + let _at_menu_outside_ai_mode = FeatureFlag::AtMenuOutsideOfAIMode.override_enabled(true); + + App::test((), |mut app| async move { + initialize_app(&mut app); + + let terminal = add_window_with_bootstrapped_terminal( + &mut app, None, /* history_file_commands */ + None, + ) + .await; + let input = terminal.read(&app, |terminal, _| terminal.input().clone()); + + input.update(&mut app, |input, ctx| { + input.user_insert("→", ctx); + input.user_insert("@", ctx); + }); + + input.read(&app, |input, ctx| { + assert_eq!(input.buffer_text(ctx), "→@"); + assert!(matches!( + input.suggestions_mode_model().as_ref(ctx).mode(), + InputSuggestionsMode::AIContextMenu { .. } + )); + }); + }); +} + #[test] fn test_ai_context_menu_preserves_lock_state() { App::test((), |mut app| async move { diff --git a/app/src/workspace/view/global_search/model.rs b/app/src/workspace/view/global_search/model.rs index dcf929a2979..50c855f187c 100644 --- a/app/src/workspace/view/global_search/model.rs +++ b/app/src/workspace/view/global_search/model.rs @@ -513,6 +513,9 @@ impl GlobalSearch { if byte_start > line_text.len() || !line_text.is_char_boundary(byte_start) { return None; } + if line_text.is_ascii() { + return Some(byte_start + 1); + } Some(line_text[..byte_start].chars().count() + 1) } diff --git a/app/src/workspace/view/global_search/model_tests.rs b/app/src/workspace/view/global_search/model_tests.rs index beae9654af5..b2e8706e238 100644 --- a/app/src/workspace/view/global_search/model_tests.rs +++ b/app/src/workspace/view/global_search/model_tests.rs @@ -119,3 +119,16 @@ fn remote_match_column_counts_characters_not_bytes() { assert_eq!(results.len(), 1); assert_eq!(results[0].column_num, Some(2)); } + +#[test] +fn remote_match_column_fast_path_matches_ascii_byte_offset() { + let success = RipgrepSearchSuccess { + matches: vec![proto_match("/repo/a.rs", 1, "abcdef", vec![(3, 6)])], + capped: false, + }; + + let results = GlobalSearch::remote_matches_to_global(&host(), success); + + assert_eq!(results.len(), 1); + assert_eq!(results[0].column_num, Some(4)); +} diff --git a/crates/fuzzy_match/src/fuzzy_tests.rs b/crates/fuzzy_match/src/fuzzy_tests.rs index 0e0b0e65357..7a35a00cf1d 100644 --- a/crates/fuzzy_match/src/fuzzy_tests.rs +++ b/crates/fuzzy_match/src/fuzzy_tests.rs @@ -62,6 +62,12 @@ fn test_wildcard_pattern_case_insensitive() { assert!(!match_result.matched_indices.is_empty()); } +#[test] +fn test_wildcard_pattern_case_insensitive_unicode_fallback() { + let result = match_wildcard_pattern_case_insensitive("Straße.TXT", "*.txt"); + assert!(result.is_some()); +} + #[test] fn test_wildcard_pattern_complex() { let result = match_wildcard_pattern("/src/ui/button.rs", "*/ui/*.rs"); @@ -141,6 +147,11 @@ fn test_partial_suffix_patterns() { assert!(result4.is_none()); } +#[test] +fn test_partial_suffix_patterns_with_unicode_text() { + assert!(match_wildcard_pattern("naïve.rs", "*.r").is_some()); +} + #[test] fn test_ui_star_partial_patterns() { // Test the specific cases mentioned in the issue diff --git a/crates/fuzzy_match/src/lib.rs b/crates/fuzzy_match/src/lib.rs index e9bfa81122b..1bb6511f3ab 100644 --- a/crates/fuzzy_match/src/lib.rs +++ b/crates/fuzzy_match/src/lib.rs @@ -219,14 +219,25 @@ pub fn match_wildcard_pattern(text: &str, pattern: &str) -> Option = (start_char_idx..text_char_count).collect(); return Some(FuzzyMatchResult { @@ -236,7 +247,7 @@ pub fn match_wildcard_pattern(text: &str, pattern: &str) -> Option Option = (0..prefix_char_count).collect(); return Some(FuzzyMatchResult { score: 1000, @@ -265,7 +280,7 @@ pub fn match_wildcard_pattern(text: &str, pattern: &str) -> Option Option = (0..text.chars().count()).collect(); + let matched_indices: Vec = if text_is_ascii { + (0..text.len()).collect() + } else { + (0..text.chars().count()).collect() + }; let score = if pattern.contains('*') || pattern.contains('?') { 1000 // Good score for wildcard matches @@ -314,11 +333,21 @@ pub fn match_wildcard_pattern(text: &str, pattern: &str) -> Option Some([6, 7]) /// ``` -fn find_partial_suffix_match(text: &str, partial_suffix: &str) -> Option> { +fn find_partial_suffix_match( + text: &str, + partial_suffix: &str, + text_is_ascii: bool, +) -> Option> { if partial_suffix.is_empty() { return None; } + if text_is_ascii && partial_suffix.is_ascii() { + return text + .rfind(partial_suffix) + .map(|start_pos| (start_pos..start_pos + partial_suffix.len()).collect()); + } + // Look for any suffix in the text that starts with our partial suffix let text_chars: Vec = text.chars().collect(); let partial_chars: Vec = partial_suffix.chars().collect(); @@ -398,7 +427,7 @@ fn is_glob_match(text: &str, pattern: &str) -> bool { /// // Would return indices for "ui/" and ".rs" portions /// // Some([5, 6, 7, 14, 15, 16]) /// ``` -fn find_substring_glob_match(text: &str, pattern: &str) -> Option> { +fn find_substring_glob_match(text: &str, pattern: &str, text_is_ascii: bool) -> Option> { // Handle patterns like "ui/*.r" - need to find the prefix and match the suffix pattern if let Some(star_pos) = pattern.find('*') { let prefix = &pattern[..star_pos]; @@ -418,27 +447,39 @@ fn find_substring_glob_match(text: &str, pattern: &str) -> Option> { // Handle simple suffix patterns directly let suffix_part = &suffix_pattern[1..]; if remaining_text.ends_with(suffix_part) { - let remaining_char_count = remaining_text.chars().count(); - let suffix_char_count = suffix_part.chars().count(); + let remaining_char_count = if text_is_ascii { + remaining_text.len() + } else { + remaining_text.chars().count() + }; + let suffix_char_count = if text_is_ascii { + suffix_part.len() + } else { + suffix_part.chars().count() + }; let start_idx = remaining_char_count - suffix_char_count; Some(FuzzyMatchResult { score: 1000, matched_indices: (start_idx..remaining_char_count).collect(), }) } else { - find_partial_suffix_match(remaining_text, suffix_part).map(|partial_match| { - FuzzyMatchResult { + find_partial_suffix_match(remaining_text, suffix_part, text_is_ascii).map( + |partial_match| FuzzyMatchResult { score: 800, matched_indices: partial_match, - } - }) + }, + ) } } else { // For complex patterns, use the full matching but prevent recursion if is_glob_match(remaining_text, suffix_pattern) { Some(FuzzyMatchResult { score: 1000, - matched_indices: (0..remaining_text.chars().count()).collect(), + matched_indices: if text_is_ascii { + (0..remaining_text.len()).collect() + } else { + (0..remaining_text.chars().count()).collect() + }, }) } else { None @@ -447,9 +488,21 @@ fn find_substring_glob_match(text: &str, pattern: &str) -> Option> { if let Some(suffix_result) = suffix_result { // Combine the matched indices - let prefix_start_char = text[..prefix_start].chars().count(); - let prefix_char_count = prefix.chars().count(); - let remaining_start_char = text[..prefix_end].chars().count(); + let prefix_start_char = if text_is_ascii { + prefix_start + } else { + text[..prefix_start].chars().count() + }; + let prefix_char_count = if text_is_ascii { + prefix.len() + } else { + prefix.chars().count() + }; + let remaining_start_char = if text_is_ascii { + prefix_end + } else { + text[..prefix_end].chars().count() + }; let mut combined_indices: Vec = (prefix_start_char..prefix_start_char + prefix_char_count).collect(); @@ -472,8 +525,16 @@ fn find_substring_glob_match(text: &str, pattern: &str) -> Option> { let prefix = &pattern[..pattern.len() - 1]; if let Some(start_pos) = text.find(prefix) { // Found the prefix as a substring, create indices for the matched part - let start_char_idx = text[..start_pos].chars().count(); - let prefix_char_count = prefix.chars().count(); + let start_char_idx = if text_is_ascii { + start_pos + } else { + text[..start_pos].chars().count() + }; + let prefix_char_count = if text_is_ascii { + prefix.len() + } else { + prefix.chars().count() + }; let matched_indices: Vec = (start_char_idx..start_char_idx + prefix_char_count).collect(); return Some(matched_indices); @@ -652,9 +713,12 @@ pub fn match_wildcard_pattern_case_insensitive( return Some(FuzzyMatchResult::no_match()); } - // Convert both text and pattern to lowercase for case insensitive matching - let text_lower = text.to_lowercase(); - let pattern_lower = pattern.to_lowercase(); + // Use ASCII lowering on the dominant path to avoid Unicode case-folding work. + let (text_lower, pattern_lower) = if text.is_ascii() && pattern.is_ascii() { + (text.to_ascii_lowercase(), pattern.to_ascii_lowercase()) + } else { + (text.to_lowercase(), pattern.to_lowercase()) + }; match_wildcard_pattern(&text_lower, &pattern_lower).map(|mut result| { // Map the matched indices back to the original text From b0396ec17b141f71c8f4a9944176b41cee2027af Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Mon, 13 Jul 2026 15:07:27 -0500 Subject: [PATCH 3/5] perf(terminal): avoid redundant block list layout work Reduce repeated model and layout work in the terminal block list. - Skip live block-height SumTree rewrites when the computed height is unchanged. - Update rich-content heights by targeted SumTree item replacement instead of a full rebuild. - Track rich-content and CLI subagent children laid out in the current frame so after_layout only visits active children. - Add model coverage for mixed rich-content/banner positions and unchanged height updates. --- app/src/terminal/block_list_element.rs | 47 +++++++----- app/src/terminal/model/blocks.rs | 98 ++++++++++++++++++++++---- app/src/terminal/model/blocks_tests.rs | 69 ++++++++++++++++++ 3 files changed, 181 insertions(+), 33 deletions(-) diff --git a/app/src/terminal/block_list_element.rs b/app/src/terminal/block_list_element.rs index 6c92e72ca41..ebd51a807d9 100644 --- a/app/src/terminal/block_list_element.rs +++ b/app/src/terminal/block_list_element.rs @@ -719,6 +719,7 @@ pub struct BlockListElement { /// Child Elements to use for rich content inserted into the block list rich_content_elements: HashMap>, + laid_out_rich_content_items: HashSet, rich_content_metadata: HashMap, shared_session_banner_state: SharedSessionBanners, @@ -734,6 +735,7 @@ pub struct BlockListElement { input_size_at_last_frame: Vector2F, block_footer_elements: HashMap>, + laid_out_cli_subagent_views: HashSet, find_model: ModelHandle, @@ -961,6 +963,7 @@ impl BlockListElement { active_filter_editor_block_index: None, filtered_blocks: None, rich_content_elements: HashMap::new(), + laid_out_rich_content_items: HashSet::new(), rich_content_metadata: HashMap::new(), shared_session_banner_state: shared_session_banners, presence_manager: None, @@ -970,6 +973,7 @@ impl BlockListElement { ai_render_context: terminal_view_render_context.ai_render_context, input_size_at_last_frame, block_footer_elements: HashMap::new(), + laid_out_cli_subagent_views: HashSet::new(), cursor_hint_text_element, cli_subagent_views, inline_menu_positioner, @@ -3158,6 +3162,8 @@ impl Element for BlockListElement { let mut block_indices_with_label_elements = vec![]; let mut visible_block_indices = vec![]; let mut updated_rich_content_heights = HashMap::new(); + let mut laid_out_rich_content_items = HashSet::new(); + let mut laid_out_cli_subagent_views = HashSet::new(); // First, remeasure any rich content items whose heights may be out of date. // We track these in the BlockList via a dirty set keyed by view ID to @@ -3176,6 +3182,7 @@ impl Element for BlockListElement { let height_px = current_size.y(); updated_rich_content_heights.insert(*view_id, height_px as f64); + laid_out_rich_content_items.insert(*view_id); } } } @@ -3266,6 +3273,7 @@ impl Element for BlockListElement { app, ); let height_px = current_size.y() as f64; + laid_out_rich_content_items.insert(view_id); // If the new laid out height is different from the value currently in the // BlockList, then we flag the block for updating once we're done laying out. @@ -3363,6 +3371,7 @@ impl Element for BlockListElement { ctx, app, ); + laid_out_cli_subagent_views.insert(block.id().clone()); } } @@ -3481,12 +3490,16 @@ impl Element for BlockListElement { let height_px = if let Some(h) = updated_rich_content_heights.get(&view_id) { *h } else { - if let Some(rich_content) = self.rich_content_elements.get_mut(&view_id) { - let _ = rich_content.layout( - SizeConstraint::tight_on_cross_axis(Axis::Vertical, constraint), - ctx, - app, - ); + if !laid_out_rich_content_items.contains(&view_id) { + if let Some(rich_content) = self.rich_content_elements.get_mut(&view_id) + { + let _ = rich_content.layout( + SizeConstraint::tight_on_cross_axis(Axis::Vertical, constraint), + ctx, + app, + ); + laid_out_rich_content_items.insert(view_id); + } } height.as_f64() * cell_size.y() as f64 }; @@ -3651,6 +3664,8 @@ impl Element for BlockListElement { ), }; self.size = Some(size); + self.laid_out_rich_content_items = laid_out_rich_content_items; + self.laid_out_cli_subagent_views = laid_out_cli_subagent_views; size } @@ -3658,20 +3673,16 @@ impl Element for BlockListElement { fn after_layout(&mut self, ctx: &mut AfterLayoutContext, app: &AppContext) { // Rich Content is arbitrary, so we want to make sure to call after_layout on each of // those elements that were actually laid out - for rich_content in self - .rich_content_elements - .values_mut() - .filter(|e| e.size().is_some()) - { - rich_content.after_layout(ctx, app); + for view_id in &self.laid_out_rich_content_items { + if let Some(rich_content) = self.rich_content_elements.get_mut(view_id) { + rich_content.after_layout(ctx, app); + } } - for cli_subagent_view in self - .cli_subagent_views - .values_mut() - .filter(|e| e.size().is_some()) - { - cli_subagent_view.after_layout(ctx, app); + for block_id in &self.laid_out_cli_subagent_views { + if let Some(cli_subagent_view) = self.cli_subagent_views.get_mut(block_id) { + cli_subagent_view.after_layout(ctx, app); + } } let model = self.model.lock(); diff --git a/app/src/terminal/model/blocks.rs b/app/src/terminal/model/blocks.rs index 458e4de7bc0..c523e1e0c31 100644 --- a/app/src/terminal/model/blocks.rs +++ b/app/src/terminal/model/blocks.rs @@ -1857,6 +1857,30 @@ impl BlockList { // from the "clear" block and reduces the gap, thinking "clear" is a block executed after. This means the "clear" block, // above the gap, is not fully cleared from the visible screen. // => By calculating the delta in height only after the gap, the gap will only shrink from blocks executed _after_ the gap. + let block_height = if let Some(block) = self.block_at(block_index) { + block.height(&self.transcript_scope).into() + } else { + report_error!( + "Tried to update height of block, but no such block exists", + extra: { "block_index" => ?block_index } + ); + return; + }; + + let previous_block_height = { + let mut cursor = self.block_heights.cursor::(); + let next_index = block_index + BlockIndex(1); + cursor.seek(&next_index, SeekBias::Left); + match cursor.item() { + Some(BlockHeightItem::Block(height)) => Some(*height), + _ => None, + } + }; + if previous_block_height == Some(block_height) { + return; + } + let previous_block_height = previous_block_height.unwrap_or_else(BlockHeight::zero); + let previous_after_gap_height = match &self.active_gap { Some(gap) => { let mut cursor = self.block_heights.cursor::(); @@ -1873,24 +1897,10 @@ impl BlockList { } None => Lines::zero(), }; - let mut previous_block_height = BlockHeight::zero(); - let block_height = if let Some(block) = self.block_at(block_index) { - block.height(&self.transcript_scope).into() - } else { - report_error!( - "Tried to update height of block, but no such block exists", - extra: { "block_index" => ?block_index } - ); - return; - }; - self.block_heights = { let mut cursor = self.block_heights.cursor::(); let next_index = block_index + BlockIndex(1); let mut tree_before_last_block = cursor.slice(&next_index, SeekBias::Left); - if let Some(BlockHeightItem::Block(height)) = cursor.item() { - previous_block_height = *height; - } tree_before_last_block.push(BlockHeightItem::Block(block_height)); // Advance the cursor past the current block and take the suffix to get all the items @@ -2401,6 +2411,10 @@ impl BlockList { /// Updates rich-content heights from GUI pixel measurements, converting to /// the canonical line unit at the boundary. pub fn update_rich_content_heights(&mut self, updated_heights: &HashMap) { + if updated_heights.is_empty() { + return; + } + let cell_height_px = self.size().cell_height_px(); let updated_heights = updated_heights .iter() @@ -2420,7 +2434,61 @@ impl BlockList { &mut self, updated_heights: &HashMap, ) { - self.update_blocks_and_sumtree(None, Some(updated_heights), |_| {}, |_| {}); + if updated_heights.is_empty() { + return; + } + + let mut changed_rich_content_items = updated_heights + .iter() + .filter_map(|(view_id, updated_height)| { + let index = self + .removable_blocklist_item_positions + .get(&RemovableBlocklistItem::RichContent(*view_id))?; + + let mut cursor = self.block_heights.cursor::(); + cursor.seek(index, SeekBias::Right); + + let Some(BlockHeightItem::RichContent(item)) = cursor.item() else { + return None; + }; + if item.last_laid_out_height == *updated_height { + return None; + } + + Some(( + *index, + RichContentItem { + last_laid_out_height: *updated_height, + ..*item + }, + )) + }) + .collect::>(); + + if changed_rich_content_items.is_empty() { + return; + } + + changed_rich_content_items.sort_unstable_by_key(|(index, _)| *index); + + self.block_heights = { + let mut cursor = self.block_heights.cursor::(); + let mut new_tree = SumTree::new(); + + for (index, updated_item) in changed_rich_content_items { + new_tree.push_tree(cursor.slice(&index, SeekBias::Right)); + if matches!( + cursor.item(), + Some(BlockHeightItem::RichContent(item)) if item.view_id == updated_item.view_id + ) { + new_tree.push(BlockHeightItem::RichContent(updated_item)); + cursor.next(); + } + } + + new_tree.push_tree(cursor.suffix()); + new_tree + }; } pub fn set_show_in_band_command_blocks(&mut self, should_show_in_band_command_blocks: bool) { diff --git a/app/src/terminal/model/blocks_tests.rs b/app/src/terminal/model/blocks_tests.rs index 10156592426..9fd5bc62ee5 100644 --- a/app/src/terminal/model/blocks_tests.rs +++ b/app/src/terminal/model/blocks_tests.rs @@ -1640,6 +1640,75 @@ fn test_remove_rich_content_block() { .any(|item| matches!(&item, BlockHeightItem::RichContent { .. }))); } +#[test] +fn test_update_rich_content_heights_in_lines_updates_changed_items_by_position() { + let mut block_list = + new_bootstrapped_block_list(None, None, ChannelEventListener::new_for_test()); + + insert_block(&mut block_list, "cmd", "output"); + + let view_id_a = EntityId::new(); + block_list.append_rich_content(RichContentItem::new_for_test(None, view_id_a, None), false); + + let second_block_index = insert_block(&mut block_list, "cmd", "output"); + block_list.insert_inline_banner_before_block( + second_block_index, + InlineBannerItem::new(0, InlineBannerType::NotificationsDiscovery), + None, + ); + + let view_id_b = EntityId::new(); + block_list.append_rich_content(RichContentItem::new_for_test(None, view_id_b, None), false); + + let positions_before_update = block_list.removable_blocklist_item_positions.clone(); + let item_count_before_update = block_list.block_heights.items().len(); + + block_list.update_rich_content_heights_in_lines(&HashMap::from([ + (view_id_a, BlockHeight::from(3.0)), + (view_id_b, BlockHeight::from(4.0)), + (EntityId::new(), BlockHeight::from(8.0)), + ])); + + let rich_content_height = |block_list: &BlockList, view_id| { + block_list + .block_heights + .items() + .iter() + .find_map(|item| match item { + BlockHeightItem::RichContent(rich_content) if rich_content.view_id == view_id => { + Some(rich_content.last_laid_out_height) + } + _ => None, + }) + .expect("rich content item should exist") + }; + + assert_eq!( + rich_content_height(&block_list, view_id_a), + BlockHeight::from(3.0) + ); + assert_eq!( + rich_content_height(&block_list, view_id_b), + BlockHeight::from(4.0) + ); + assert_eq!( + block_list.removable_blocklist_item_positions, + positions_before_update + ); + assert_eq!( + block_list.block_heights.items().len(), + item_count_before_update + ); + + let items_after_update = block_list.block_heights.items(); + block_list.update_rich_content_heights_in_lines(&HashMap::from([ + (view_id_a, BlockHeight::from(3.0)), + (view_id_b, BlockHeight::from(4.0)), + (EntityId::new(), BlockHeight::from(8.0)), + ])); + assert_eq!(block_list.block_heights.items(), items_after_update); +} + #[test] fn test_conversation_scoped_rich_content_hidden_outside_fullscreen_agent_view() { FeatureFlag::AgentView.set_enabled(true); From 5dda7c7381af1aec59dc8595b68c01e5f0079725 Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Mon, 13 Jul 2026 16:00:55 -0500 Subject: [PATCH 4/5] fix(terminal): preserve cursor-only flat storage rows --- .../src/model/grid/flat_storage/mod.rs | 5 ++- .../src/model/grid/flat_storage/mod_tests.rs | 35 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/crates/warp_terminal/src/model/grid/flat_storage/mod.rs b/crates/warp_terminal/src/model/grid/flat_storage/mod.rs index 136a055796d..d78474bb170 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/mod.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/mod.rs @@ -230,7 +230,7 @@ impl FlatStorage { continue; } - let mut needs_processing = cell.c != cell::DEFAULT_CHAR; + let mut needs_processing = !cell.is_empty(); if cell.fg != fg_color { needs_processing = true; fg_color = cell.fg; @@ -412,6 +412,9 @@ impl FlatStorage { pub fn content_offset_at_point_or_before(&self, point: Point) -> ByteOffset { match self.content_offset_at_point(point) { Ok(content_offset) => content_offset, + Err(index::ContentOffsetToPointError::NonZeroColumnInEmptyRow { row, .. }) => self + .content_offset_at_point(Point::new(row, 0)) + .expect("empty row start should have a content offset"), Err(index::ContentOffsetToPointError::ColumnExceedsContent { row, .. }) => { let fallback_col = self .rows_from(row) diff --git a/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs b/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs index 1e3c2bbfa77..ea9d69c8638 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs @@ -291,3 +291,38 @@ fn test_clear_after_truncate_front_then_resize_and_push_does_not_panic() { .expect("should materialize a row after clearing and resizing storage"); assert_eq!(row[0].c, 'n'); } + +#[test] +fn content_offset_at_point_or_before_clamps_empty_row_columns() { + let storage = FlatStorage::from_content_using_rows("abc\n\ndef\n", 5, Some(3)); + + assert!(storage.content_offset_at_point(Point::new(1, 4)).is_err()); + assert_eq!( + storage.content_offset_at_point_or_before(Point::new(1, 4)), + storage + .content_offset_at_point(Point::new(1, 0)) + .expect("empty row start should map to a content offset") + ); +} + +#[test] +fn has_cursor_cell_materializes_empty_cells_before_it() { + let mut cursor_cell = Cell::default(); + cursor_cell.flags.insert(Flags::HAS_CURSOR); + + let row = Row::from_vec( + vec![ + Cell::default(), + Cell::default(), + Cell::default(), + Cell::default(), + cursor_cell, + ], + 5, + ); + + let mut storage = FlatStorage::new(5, None, Some(1)); + storage.push_rows([&row]); + + assert!(storage.content_offset_at_point(Point::new(0, 4)).is_ok()); +} From a2c2a9e11a6dbe767eabec008e22b731131ddc4e Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Mon, 13 Jul 2026 16:23:19 -0500 Subject: [PATCH 5/5] fix(terminal): clamp cursor-only blank tail resize --- .../src/model/grid/flat_storage/mod.rs | 15 +++++++++- .../src/model/grid/flat_storage/mod_tests.rs | 28 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/warp_terminal/src/model/grid/flat_storage/mod.rs b/crates/warp_terminal/src/model/grid/flat_storage/mod.rs index d78474bb170..abec1dfe27f 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/mod.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/mod.rs @@ -204,6 +204,7 @@ impl FlatStorage { continue; } + let row_has_content = row.line_length() > 0; let mut last_cell: isize = -1; // Use an empty but pre-allocated buffer to collect characters from @@ -231,6 +232,9 @@ impl FlatStorage { } let mut needs_processing = !cell.is_empty(); + if row_has_content && Self::is_only_cursor_marker(cell) { + needs_processing = false; + } if cell.fg != fg_color { needs_processing = true; fg_color = cell.fg; @@ -298,7 +302,8 @@ impl FlatStorage { cell.flags().intersects( cell::Flags::WIDE_CHAR | cell::Flags::WIDE_CHAR_SPACER - | cell::Flags::LEADING_WIDE_CHAR_SPACER, + | cell::Flags::LEADING_WIDE_CHAR_SPACER + | cell::Flags::HAS_CURSOR, ) }) { return false; @@ -342,6 +347,14 @@ impl FlatStorage { true } + fn is_only_cursor_marker(cell: &cell::Cell) -> bool { + cell.c == cell::DEFAULT_CHAR + && cell.fg == DEFAULT_FG_COLOR + && cell.bg == ansi::Color::Named(ansi::NamedColor::Background) + && *cell.flags() == cell::Flags::HAS_CURSOR + && cell.end_of_prompt_marker().is_none() + } + /// Clears out the contents of flat storage. pub fn clear(&mut self) { // Drop all rows from the index. diff --git a/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs b/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs index ea9d69c8638..451fb99f1ca 100644 --- a/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs +++ b/crates/warp_terminal/src/model/grid/flat_storage/mod_tests.rs @@ -326,3 +326,31 @@ fn has_cursor_cell_materializes_empty_cells_before_it() { assert!(storage.content_offset_at_point(Point::new(0, 4)).is_ok()); } + +#[test] +fn has_cursor_cell_does_not_extend_contentful_row_blank_tail() { + let mut cursor_cell = Cell::default(); + cursor_cell.flags.insert(Flags::HAS_CURSOR); + + let row = Row::from_vec( + vec![ + Cell::from('d'), + Cell::from('e'), + Cell::from('f'), + Cell::default(), + cursor_cell, + ], + 5, + ); + + let mut storage = FlatStorage::new(5, None, Some(1)); + storage.push_rows([&row]); + + assert!(storage.content_offset_at_point(Point::new(0, 4)).is_err()); + assert_eq!( + storage.content_offset_at_point_or_before(Point::new(0, 4)), + storage + .content_offset_at_point(Point::new(0, 2)) + .expect("last content cell should map to a content offset") + ); +}