From 0f81c7d0e02e2731568f30cd5e67498c5f6589da Mon Sep 17 00:00:00 2001 From: Arijit Dey Date: Sun, 3 May 2026 02:20:36 +0530 Subject: [PATCH 1/4] overhaul the incremental search system * Massively faster incremental search by highlighting only the visible lines * Full buffer is only highlighted once the search query is confirmed * All searches (including incremental previews) now wrap around to the opposite direction when no match found --- src/core/ev_handler.rs | 5 +- src/core/utils/mod.rs | 15 ++- src/search.rs | 271 +++++++++++++++++++++++------------------ src/state.rs | 4 +- 4 files changed, 168 insertions(+), 127 deletions(-) diff --git a/src/core/ev_handler.rs b/src/core/ev_handler.rs index 7e5a8ee..0c4bfe5 100644 --- a/src/core/ev_handler.rs +++ b/src/core/ev_handler.rs @@ -425,9 +425,10 @@ pub fn handle_io_command( if let Some(incremental_search_result) = search_result.incremental_search_result { p.search_state.search_term = search_result.compiled_regex; p.upper_mark = incremental_search_result.upper_mark; - p.search_state.search_mark = incremental_search_result.search_mark; + p.search_state.search_mark = 0; p.search_state.search_idx = incremental_search_result.search_idx; - p.screen.formatted_lines = incremental_search_result.formatted_lines; + p.format_lines(); + command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); return Ok(()); } diff --git a/src/core/utils/mod.rs b/src/core/utils/mod.rs index 691c388..f7ba3c9 100644 --- a/src/core/utils/mod.rs +++ b/src/core/utils/mod.rs @@ -37,12 +37,15 @@ impl LinesRowMap { self.0.get(ln) } - #[allow(dead_code)] - pub fn row_to_line(&self, row: usize) -> Option { - match self.0.binary_search(&row) { - Ok(line) => Some(line), - Err(0) => None, - Err(next_line) => Some(next_line - 1), + pub(crate) fn row_to_line(&self, row: usize) -> Option { + if self.0.is_empty() { + return None; } + + Some(match self.0.binary_search(&row) { + Ok(idx) => idx, + Err(0) => 0, + Err(idx) => idx.saturating_sub(1), + }) } } diff --git a/src/search.rs b/src/search.rs index 553dd95..d708ab7 100644 --- a/src/search.rs +++ b/src/search.rs @@ -51,7 +51,7 @@ //! ``` #![allow(unused_imports)] -use crate::minus_core::utils::{display, term}; +use crate::minus_core::utils::{LinesRowMap, display, term}; use crate::screen::Screen; use crate::{LineNumbers, PagerState}; use crate::{error::MinusError, input::HashedEventRegister, screen}; @@ -139,9 +139,6 @@ pub struct SearchOpts<'a> { } /// Options to control incremental search -/// -/// NOTE: `text` and `initial_formatted_lines` are experimental in this context and are subject to -/// change. Use them at your own risk. pub struct IncrementalSearchOpts<'a> { /// Current status of line numbering pub line_numbers: LineNumbers, @@ -149,7 +146,9 @@ pub struct IncrementalSearchOpts<'a> { pub initial_upper_mark: usize, /// Reference to [`PagerState::screen`] pub screen: &'a Screen, - /// Value of [`PagerState::upper_mark`] before starting of search prompt + /// Cached map from logical lines to formatted rows. + pub lines_to_row_map: &'a LinesRowMap, + /// Value of [PagerState::upper_mark] before starting of search prompt pub initial_left_mark: usize, } @@ -159,6 +158,7 @@ impl<'a> From<&'a PagerState> for IncrementalSearchOpts<'a> { line_numbers: ps.line_numbers, initial_upper_mark: ps.upper_mark, screen: &ps.screen, + lines_to_row_map: &ps.lines_to_row_map, initial_left_mark: ps.left_mark, } } @@ -238,12 +238,7 @@ impl FetchInputResult { /// A cache for storing all the new data obtained by running incremental search pub(crate) struct IncrementalSearchCache { - /// Lines to be displayed with highlighted search matches - pub(crate) formatted_lines: Vec, - /// Index from `search_idx` where a search match after current upper mark may be found - /// NOTE: There is no guarantee that this will stay within the bounds of `search_idx` - pub(crate) search_mark: usize, - /// Indices of `formatted_lines` where search matches have been found + /// Indices of formatted_lines where search matches have been found pub(crate) search_idx: BTreeSet, /// Index of the line from which to display the text. /// This will be set to the index of line which is after the current upper mark and will @@ -251,6 +246,129 @@ pub(crate) struct IncrementalSearchCache { pub(crate) upper_mark: usize, } +fn line_matches_query(line: &str, query: &Regex) -> bool { + let stripped = ANSI_REGEX.replace_all(line, ""); + query.is_match(stripped.as_ref()) +} + +fn incremental_preview( + iso: &IncrementalSearchOpts<'_>, + query: &Regex, + cols: usize, + rows: usize, +) -> Option<(Vec, usize)> { + fn preview_line( + iso: &IncrementalSearchOpts<'_>, + query: &Regex, + cols: usize, + line_number_digits: usize, + line_idx: usize, + line: &str, + visible_lines: &mut Vec, + upper_mark: &mut Option, + writable_rows: usize, + wrapped: bool, + ) -> Option<()> { + // Skip all lines that don't have any match + if upper_mark.is_none() && !line_matches_query(line, query) { + return Some(()); + } + + let row_start = *iso.lines_to_row_map.get(line_idx).unwrap_or(&0); + let mut search_idx = BTreeSet::new(); + let mut formatted_rows = screen::formatted_line( + line, + line_number_digits, + line_idx, + iso.line_numbers, + cols, + iso.screen.line_wrapping, + row_start, + &mut search_idx, + Some(query), + ); + + if upper_mark.is_none() { + let match_row = *search_idx + .iter() + .find(|idx| wrapped || **idx >= iso.initial_upper_mark)?; + let skip_rows = match_row.saturating_sub(row_start); + *upper_mark = Some(match_row); + visible_lines.extend(formatted_rows.drain(skip_rows..)); + } else { + visible_lines.append(&mut formatted_rows); + } + + if visible_lines.len() >= writable_rows { + visible_lines.truncate(writable_rows); + } + + Some(()) + } + + let writable_rows = rows.saturating_sub(1); + if writable_rows == 0 { + return None; + } + + let start_line_idx = iso.lines_to_row_map.row_to_line(iso.initial_upper_mark)?; + let line_number_digits = crate::minus_core::utils::digits(iso.screen.line_count()); + let mut visible_lines = Vec::with_capacity(writable_rows); + let mut upper_mark = None; + + for (line_idx, line) in iso + .screen + .orig_text + .lines() + .enumerate() + .skip(start_line_idx) + { + preview_line( + iso, + query, + cols, + line_number_digits, + line_idx, + line, + &mut visible_lines, + &mut upper_mark, + writable_rows, + false, + )?; + if visible_lines.len() >= writable_rows { + break; + } + } + + if upper_mark.is_none() { + for (line_idx, line) in iso + .screen + .orig_text + .lines() + .enumerate() + .take(start_line_idx) + { + preview_line( + iso, + query, + cols, + line_number_digits, + line_idx, + line, + &mut visible_lines, + &mut upper_mark, + writable_rows, + true, + )?; + if visible_lines.len() >= writable_rows { + break; + } + } + } + + upper_mark.map(|upper_mark| (visible_lines, upper_mark)) +} + /// Runs the incremental search /// /// It will return if `Ok(SomeIncrementalSearchCache)` if there was a successful run of incremental @@ -263,7 +381,6 @@ fn run_incremental_search<'a, F, O>( out: &mut O, so: &'a SearchOpts<'a>, incremental_search_condition: F, - mut buffer: Vec, ) -> crate::Result> where O: Write, @@ -315,47 +432,35 @@ where return Ok(None); } - // Format the text with search highlights and get the index of the element in - // format_result.append_search_idx which is after the current upper mark - // - // PERF: Check if this can be futhur optimized - let format_result = screen::format_lines_into( - &mut buffer, - &screen.orig_text, - line_numbers, - so.cols.into(), - screen.line_wrapping, - so.compiled_regex.as_ref(), - ); - let position_of_next_match = - next_nth_match(&format_result.append_search_idx, initial_upper_mark, 0); - // Get the upper mark. If we can't find one, reset the display - let upper_mark; - if let Some(pnm) = position_of_next_match { - upper_mark = *format_result.append_search_idx.iter().nth(pnm).unwrap(); - // Draw the incrementally searched lines from upper mark - display::write_text_checked( - out, - &buffer, - upper_mark, - so.rows.into(), - so.cols.into(), - screen.line_wrapping, - initial_left_mark, - line_numbers, - screen.line_count(), - )?; - } else { + let query = so.compiled_regex.as_ref().unwrap(); + + let Some((visible_lines, upper_mark)) = + incremental_preview(iso, query, so.cols.into(), so.rows.into()) + else { reset_screen(out, so)?; return Ok(None); - } + }; + + // Draw the incrementally searched lines from upper mark + display::write_text_checked( + out, + &visible_lines, + 0, + so.rows.into(), + so.cols.into(), + iso.screen.line_wrapping, + iso.initial_left_mark, + iso.line_numbers, + iso.screen.line_count(), + )?; + // Return the results obtained by running incremental search so that they can be stored as a // cache. + let mut search_idx = BTreeSet::new(); + search_idx.insert(upper_mark); Ok(Some(IncrementalSearchCache { - formatted_lines: buffer, - search_mark: position_of_next_match.unwrap(), upper_mark, - search_idx: format_result.append_search_idx, + search_idx, })) } @@ -392,14 +497,8 @@ where // Cache the compiled regex if the regex is valid so.compiled_regex = Regex::new(&so.string).ok(); - // Run incremental search and update the upper mark if incremental search had a successful - // run otherwise set it to the initial upper mark - let buffer = so - .incremental_search_cache - .take() - .map_or_else(|| Vec::with_capacity(256), |cache| cache.formatted_lines); so.incremental_search_cache = - run_incremental_search(out, so, incremental_search_condition, buffer)?; + run_incremental_search(out, so, incremental_search_condition)?; // Update prompt term::move_cursor(out, 0, so.rows, false)?; @@ -1120,70 +1219,6 @@ mod tests { } } - #[test] - fn incremental_search_reuses_cached_formatted_lines_buffer() { - use regex::Regex; - - let mut ps = crate::PagerState::new().unwrap(); - ps.search_state.search_mode = crate::search::SearchMode::Forward; - ps.append_str("alpha\nbeta\nalpha\n"); - - let mut so = crate::search::SearchOpts::from(&ps); - #[allow(clippy::trivial_regex)] - { - so.compiled_regex = Some(Regex::new("alpha").unwrap()); - } - so.incremental_search_cache = Some(crate::search::IncrementalSearchCache { - formatted_lines: vec!["x".repeat(64), "y".repeat(64), "z".repeat(64)], - search_mark: 0, - search_idx: std::collections::BTreeSet::new(), - upper_mark: 0, - }); - - let previous_vec_ptr = so - .incremental_search_cache - .as_ref() - .unwrap() - .formatted_lines - .as_ptr(); - let previous_row0_ptr = so - .incremental_search_cache - .as_ref() - .unwrap() - .formatted_lines[0] - .as_ptr(); - let previous_row1_ptr = so - .incremental_search_cache - .as_ref() - .unwrap() - .formatted_lines[1] - .as_ptr(); - - let mut out = Vec::new(); - let buffer = so - .incremental_search_cache - .take() - .map_or_else(|| Vec::with_capacity(256), |cache| cache.formatted_lines); - let result = super::run_incremental_search(&mut out, &so, |_| true, buffer) - .unwrap() - .unwrap(); - - assert_eq!( - result.formatted_lines, - vec![ - format!("{i}alpha{n}", i = *super::INVERT, n = *super::NORMAL), - "beta".to_string(), - format!("{i}alpha{n}", i = *super::INVERT, n = *super::NORMAL) - ] - ); - assert_eq!(result.formatted_lines.as_ptr(), previous_vec_ptr); - assert_eq!(result.formatted_lines[0].as_ptr(), previous_row0_ptr); - assert_eq!(result.formatted_lines[1].as_ptr(), previous_row1_ptr); - assert_eq!(result.search_mark, 0); - assert_eq!(result.upper_mark, 0); - assert_eq!(result.search_idx, std::collections::BTreeSet::from([0, 2])); - } - #[allow(clippy::trivial_regex)] mod highlighting { use std::collections::BTreeSet; diff --git a/src/state.rs b/src/state.rs index f09c262..60a4426 100644 --- a/src/state.rs +++ b/src/state.rs @@ -2,7 +2,7 @@ #![allow(dead_code)] #[cfg(feature = "search")] -use crate::search::{SearchMode, SearchOpts}; +use crate::search::{SearchMode, SearchOpts, next_nth_match}; use crate::{ LineNumbers, @@ -261,6 +261,8 @@ impl PagerState { #[cfg(feature = "search")] { self.search_state.search_idx = format_result.append_search_idx; + self.search_state.search_mark = + next_nth_match(&self.search_state.search_idx, self.upper_mark, 0).unwrap_or(0); } self.lines_to_row_map = format_result.lines_to_row_map; self.screen.max_line_length = format_result.max_line_length; From 37809eb0981b526030b65c779ddc0a9cc0e724c7 Mon Sep 17 00:00:00 2001 From: Arijit Dey Date: Tue, 12 May 2026 23:31:28 +0530 Subject: [PATCH 2/4] fix: empty rows below search match if match near eof --- src/search.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/search.rs b/src/search.rs index d708ab7..5a4e3cb 100644 --- a/src/search.rs +++ b/src/search.rs @@ -340,6 +340,27 @@ fn incremental_preview( } } + // visible_lines places the first search march as its first element. However if the match is + // near the EOF, it might not fill up completely and show blank lines on the display. + // We fix this by filling visible_lines by as many rows such that a pageful of data can be + // displayed. + if let Some(um) = upper_mark + && visible_lines.len() < writable_rows + && iso.screen.formatted_lines_count() > writable_rows + { + let start = iso + .screen + .formatted_lines_count() + .saturating_sub(writable_rows); + let to_insert = um.saturating_sub(start); + let shift = visible_lines.len(); + visible_lines.resize(writable_rows, String::new()); + visible_lines.rotate_left(shift); + for i in 0..to_insert { + visible_lines[i] = iso.screen.formatted_lines[start + i].clone(); + } + } + if upper_mark.is_none() { for (line_idx, line) in iso .screen From 59cd56d5cfc5aa3c9e364e74fb58304211a533cd Mon Sep 17 00:00:00 2001 From: Arijit Dey Date: Wed, 13 May 2026 15:41:43 +0530 Subject: [PATCH 3/4] refactor: drop all usage of screen::formatted_line --- src/core/ev_handler.rs | 10 --- src/screen/mod.rs | 52 +---------- src/search.rs | 199 ++++++++++++++++++----------------------- 3 files changed, 88 insertions(+), 173 deletions(-) diff --git a/src/core/ev_handler.rs b/src/core/ev_handler.rs index 0c4bfe5..9242f8b 100644 --- a/src/core/ev_handler.rs +++ b/src/core/ev_handler.rs @@ -421,16 +421,6 @@ pub fn handle_io_command( cvar.notify_one(); command_queue.push_back(Command::Io(IoCommand::RedrawPrompt)); - // If we have incremental search cache directly use it and return - if let Some(incremental_search_result) = search_result.incremental_search_result { - p.search_state.search_term = search_result.compiled_regex; - p.upper_mark = incremental_search_result.upper_mark; - p.search_state.search_mark = 0; - p.search_state.search_idx = incremental_search_result.search_idx; - p.format_lines(); - command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); - return Ok(()); - } // If we only have compiled regex cached, use that otherwise compile the original // string query if its not empty diff --git a/src/screen/mod.rs b/src/screen/mod.rs index 6884a8f..4386227 100644 --- a/src/screen/mod.rs +++ b/src/screen/mod.rs @@ -23,7 +23,7 @@ pub type TextBlock<'a> = &'a str; pub type OwnedTextBlock = String; pub(crate) struct FormattedRow<'a> { - row: Cow<'a, str>, + pub(crate) row: Cow<'a, str>, show_line_numbers: bool, line_number: Option, padding: usize, @@ -658,56 +658,6 @@ where row_count } -/// Formats the given `line` -/// -/// - `line`: The line to format -/// - `line_numbers`: tells whether to format the line with line numbers. -/// - `len_line_number`: is the number of digits that number of lines in [`Screen::orig_text`] -/// occupy. For example, this will be 2 if number of lines in [`Screen::line_count`] is 50 and 3 -/// if the number of lines in [`Screen::line_count`] is 500. This is used for calculating the -/// padding of each displayed line. -/// - `idx`: is the position index where the line is placed in [`Screen::orig_text`]. -/// - `formatted_idx`: is the position index where the line will be placed in the resulting -/// [`Screen::formatted_lines`] -/// - `cols`: Number of columns in the terminal -/// - `search_term`: Contains the regex if a search is active -#[allow(clippy::too_many_arguments)] -#[allow(dead_code)] -pub(crate) fn formatted_line( - line: Line<'_>, - len_line_number: usize, - idx: usize, - line_numbers: LineNumbers, - cols: usize, - line_wrapping: bool, - #[cfg(feature = "search")] formatted_idx: usize, - #[cfg(feature = "search")] search_idx: &mut BTreeSet, - #[cfg(feature = "search")] search_term: Option<®ex::Regex>, -) -> Rows { - let rows = format_line( - line, - len_line_number, - idx, - line_numbers, - cols, - line_wrapping, - ); - let mut formatted_rows = Vec::with_capacity(256); - - #[cfg(feature = "search")] - { - let rows = format_search_rows(rows, search_term); - collect_rows(&mut formatted_rows, rows, formatted_idx, search_idx); - } - - #[cfg(not(feature = "search"))] - { - collect_rows(&mut formatted_rows, rows); - } - - formatted_rows -} - pub(crate) fn format_lines_into( buffer: &mut Rows, text: &String, diff --git a/src/search.rs b/src/search.rs index 5a4e3cb..6a91e83 100644 --- a/src/search.rs +++ b/src/search.rs @@ -54,7 +54,7 @@ use crate::minus_core::utils::{LinesRowMap, display, term}; use crate::screen::Screen; use crate::{LineNumbers, PagerState}; -use crate::{error::MinusError, input::HashedEventRegister, screen}; +use crate::{error::MinusError, input::HashedEventRegister, minus_core::utils, screen}; use crossterm::{ cursor::{self, MoveTo}, event::{self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers}, @@ -134,7 +134,6 @@ pub struct SearchOpts<'a> { pub cols: u16, /// Options specifically controlling incremental search pub incremental_search_options: Option>, - incremental_search_cache: Option, compiled_regex: Option, } @@ -148,8 +147,12 @@ pub struct IncrementalSearchOpts<'a> { pub screen: &'a Screen, /// Cached map from logical lines to formatted rows. pub lines_to_row_map: &'a LinesRowMap, - /// Value of [PagerState::upper_mark] before starting of search prompt + /// Value of [`PagerState::upper_mark`] before starting of search prompt pub initial_left_mark: usize, + /// Value of [`PagerState::cols`] + cols: usize, + /// Value of [`PagerState::rows`] - 1 and 0 if rows is 0. + writable_rows: usize, } impl<'a> From<&'a PagerState> for IncrementalSearchOpts<'a> { @@ -160,10 +163,18 @@ impl<'a> From<&'a PagerState> for IncrementalSearchOpts<'a> { screen: &ps.screen, lines_to_row_map: &ps.lines_to_row_map, initial_left_mark: ps.left_mark, + cols: ps.cols, + writable_rows: ps.rows.saturating_sub(1), } } } +impl IncrementalSearchOpts<'_> { + const fn line_number_digits(&self) -> usize { + utils::digits(self.screen.line_count()) + } +} + #[allow(clippy::fallible_impl_from)] impl<'a> From<&'a PagerState> for SearchOpts<'a> { fn from(ps: &'a PagerState) -> Self { @@ -187,7 +198,6 @@ impl<'a> From<&'a PagerState> for SearchOpts<'a> { rows: ps.rows.try_into().unwrap(), cols: ps.cols.try_into().unwrap(), incremental_search_options: Some(incremental_search_options), - incremental_search_cache: None, compiled_regex: None, search_mode: ps.search_state.search_mode, } @@ -218,8 +228,6 @@ impl InputStatus { pub(crate) struct FetchInputResult { /// Original search query pub(crate) string: String, - /// Incremental search cache if available - pub(crate) incremental_search_result: Option, /// Cached pre-compiled [`Regex`] if available pub(crate) compiled_regex: Option, } @@ -230,90 +238,75 @@ impl FetchInputResult { const fn new_empty() -> Self { Self { string: String::new(), - incremental_search_result: None, compiled_regex: None, } } } -/// A cache for storing all the new data obtained by running incremental search -pub(crate) struct IncrementalSearchCache { - /// Indices of formatted_lines where search matches have been found - pub(crate) search_idx: BTreeSet, - /// Index of the line from which to display the text. - /// This will be set to the index of line which is after the current upper mark and will - /// have a search match for sure - pub(crate) upper_mark: usize, -} - fn line_matches_query(line: &str, query: &Regex) -> bool { let stripped = ANSI_REGEX.replace_all(line, ""); query.is_match(stripped.as_ref()) } -fn incremental_preview( +fn preview_line( iso: &IncrementalSearchOpts<'_>, query: &Regex, - cols: usize, - rows: usize, -) -> Option<(Vec, usize)> { - fn preview_line( - iso: &IncrementalSearchOpts<'_>, - query: &Regex, - cols: usize, - line_number_digits: usize, - line_idx: usize, - line: &str, - visible_lines: &mut Vec, - upper_mark: &mut Option, - writable_rows: usize, - wrapped: bool, - ) -> Option<()> { - // Skip all lines that don't have any match - if upper_mark.is_none() && !line_matches_query(line, query) { - return Some(()); - } - - let row_start = *iso.lines_to_row_map.get(line_idx).unwrap_or(&0); - let mut search_idx = BTreeSet::new(); - let mut formatted_rows = screen::formatted_line( - line, - line_number_digits, - line_idx, - iso.line_numbers, - cols, - iso.screen.line_wrapping, - row_start, - &mut search_idx, - Some(query), - ); + line_idx: usize, + line: &str, + visible_lines: &mut Vec, + upper_mark: &mut Option, + wrapped: bool, +) { + // Skip all lines that don't have any match + if upper_mark.is_none() && !line_matches_query(line, query) { + return; + } - if upper_mark.is_none() { - let match_row = *search_idx - .iter() - .find(|idx| wrapped || **idx >= iso.initial_upper_mark)?; - let skip_rows = match_row.saturating_sub(row_start); - *upper_mark = Some(match_row); - visible_lines.extend(formatted_rows.drain(skip_rows..)); - } else { - visible_lines.append(&mut formatted_rows); + let row_start = *iso.lines_to_row_map.get(line_idx).unwrap_or(&0); + let mut match_row_idx = None; + let mut formatted_rows = screen::format_line( + line, + iso.line_number_digits(), + line_idx, + iso.line_numbers, + iso.cols, + iso.screen.line_wrapping, + ) + .enumerate() + .map(|(i, fr)| { + let h = highlight_matches_args(&fr.row, query, false); + let match_row = format!("{h}"); + if h.is_match && (wrapped || line_idx + i > iso.initial_upper_mark) { + match_row_idx = Some(line_idx + i); } + match_row + }) + .collect::>(); - if visible_lines.len() >= writable_rows { - visible_lines.truncate(writable_rows); + if upper_mark.is_none() { + if match_row_idx.is_none() { + return; } + let match_row_idx = match_row_idx.unwrap(); + let skip_rows = match_row_idx.saturating_sub(row_start); + *upper_mark = Some(match_row_idx); + visible_lines.extend(formatted_rows.drain(skip_rows..)); + } else { + visible_lines.append(&mut formatted_rows); + } - Some(()) + if visible_lines.len() >= iso.writable_rows { + visible_lines.truncate(iso.writable_rows); } +} - let writable_rows = rows.saturating_sub(1); - if writable_rows == 0 { +fn incremental_preview(iso: &IncrementalSearchOpts<'_>, query: &Regex) -> Option> { + if iso.writable_rows == 0 { return None; } let start_line_idx = iso.lines_to_row_map.row_to_line(iso.initial_upper_mark)?; - let line_number_digits = crate::minus_core::utils::digits(iso.screen.line_count()); - let mut visible_lines = Vec::with_capacity(writable_rows); + let mut visible_lines = Vec::with_capacity(iso.writable_rows); let mut upper_mark = None; for (line_idx, line) in iso @@ -326,16 +319,13 @@ fn incremental_preview( preview_line( iso, query, - cols, - line_number_digits, line_idx, line, &mut visible_lines, &mut upper_mark, - writable_rows, false, - )?; - if visible_lines.len() >= writable_rows { + ); + if visible_lines.len() >= iso.writable_rows { break; } } @@ -345,19 +335,23 @@ fn incremental_preview( // We fix this by filling visible_lines by as many rows such that a pageful of data can be // displayed. if let Some(um) = upper_mark - && visible_lines.len() < writable_rows - && iso.screen.formatted_lines_count() > writable_rows + && visible_lines.len() < iso.writable_rows + && iso.screen.formatted_lines_count() > iso.writable_rows { let start = iso .screen .formatted_lines_count() - .saturating_sub(writable_rows); + .saturating_sub(iso.writable_rows); let to_insert = um.saturating_sub(start); let shift = visible_lines.len(); - visible_lines.resize(writable_rows, String::new()); + visible_lines.resize(iso.writable_rows, String::new()); visible_lines.rotate_left(shift); - for i in 0..to_insert { - visible_lines[i] = iso.screen.formatted_lines[start + i].clone(); + for (d, s) in visible_lines + .iter_mut() + .zip(iso.screen.formatted_lines.iter().skip(start)) + .take(to_insert) + { + d.clone_from(s); } } @@ -372,22 +366,23 @@ fn incremental_preview( preview_line( iso, query, - cols, - line_number_digits, line_idx, line, &mut visible_lines, &mut upper_mark, - writable_rows, true, - )?; - if visible_lines.len() >= writable_rows { + ); + if visible_lines.len() >= iso.writable_rows { break; } } } - upper_mark.map(|upper_mark| (visible_lines, upper_mark)) + if upper_mark.is_some() { + Some(visible_lines) + } else { + None + } } /// Runs the incremental search @@ -402,13 +397,13 @@ fn run_incremental_search<'a, F, O>( out: &mut O, so: &'a SearchOpts<'a>, incremental_search_condition: F, -) -> crate::Result> +) -> crate::Result<()> where O: Write, F: Fn(&'a SearchOpts) -> bool, { let Some(iso) = so.incremental_search_options.as_ref() else { - return Ok(None); + return Ok(()); }; let screen = iso.screen; let line_numbers = iso.line_numbers; @@ -440,26 +435,16 @@ where // If the query prior to the current one had a successful incremental search run and now the // current query isn't a valid regex or the incremental search condition has returned false // then - if so.incremental_search_cache.is_some() && !should_proceed { - reset_screen(out, so)?; - return Ok(None); - } - - // Return immediately if search query isn't valid or incremental search condition is false - // NOTE: This must come after the reset screen display code in above statement, otherwise this - // will cover all the cases of the above statement's condition and hence the terminal will ever - // get reset if !should_proceed { - return Ok(None); + reset_screen(out, so)?; + return Ok(()); } let query = so.compiled_regex.as_ref().unwrap(); - let Some((visible_lines, upper_mark)) = - incremental_preview(iso, query, so.cols.into(), so.rows.into()) - else { + let Some(visible_lines) = incremental_preview(iso, query) else { reset_screen(out, so)?; - return Ok(None); + return Ok(()); }; // Draw the incrementally searched lines from upper mark @@ -475,14 +460,7 @@ where iso.screen.line_count(), )?; - // Return the results obtained by running incremental search so that they can be stored as a - // cache. - let mut search_idx = BTreeSet::new(); - search_idx.insert(upper_mark); - Ok(Some(IncrementalSearchCache { - upper_mark, - search_idx, - })) + Ok(()) } /// Respond to keyboard events @@ -518,8 +496,7 @@ where // Cache the compiled regex if the regex is valid so.compiled_regex = Regex::new(&so.string).ok(); - so.incremental_search_cache = - run_incremental_search(out, so, incremental_search_condition)?; + run_incremental_search(out, so, incremental_search_condition)?; // Update prompt term::move_cursor(out, 0, so.rows, false)?; @@ -742,7 +719,6 @@ pub(crate) fn fetch_input( // in the cache InputStatus::Confirmed => FetchInputResult { string: search_opts.string, - incremental_search_result: search_opts.incremental_search_cache, compiled_regex: search_opts.compiled_regex, }, }; @@ -968,7 +944,6 @@ mod tests { rows: 25, cols: 100, incremental_search_options: None, - incremental_search_cache: None, compiled_regex: None, search_mode: sm, } From fb30318dd966f69f38aa4264cbb94f511d11ce76 Mon Sep 17 00:00:00 2001 From: Arijit Dey Date: Wed, 13 May 2026 21:16:33 +0530 Subject: [PATCH 4/4] perf: use Cow for visible_lines * fix incremental previe not being cleared when search is cancelled * fix upper mark is reset if search is confirmed --- src/core/ev_handler.rs | 30 +++++++++++++---------- src/core/utils/display/mod.rs | 32 ++++++++++++------------- src/core/utils/display/tests.rs | 28 +++++++++++----------- src/search.rs | 42 ++++++++++++++++++++------------- src/state.rs | 8 +++---- src/tests.rs | 6 ++--- 6 files changed, 80 insertions(+), 66 deletions(-) diff --git a/src/core/ev_handler.rs b/src/core/ev_handler.rs index 9242f8b..da149e3 100644 --- a/src/core/ev_handler.rs +++ b/src/core/ev_handler.rs @@ -35,7 +35,7 @@ pub fn handle_event( Command::SetData(text) => { p.screen.orig_text = text; p.screen.line_count = p.screen.orig_text.lines().count(); - p.format_lines(); + p.reformat_display(); command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); } Command::UserInput(InputEvent::Exit) => { @@ -149,13 +149,13 @@ pub fn handle_event( Command::UserInput(InputEvent::UpdateTermArea(c, r)) => { p.rows = r; p.cols = c; - p.format_lines(); + p.reformat_display(); // Readjust the text wrapping for the new number of columns command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); } Command::UserInput(InputEvent::UpdateLineNumber(l)) => { p.line_numbers = l; - p.format_lines(); + p.reformat_display(); command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); } Command::UserInput(InputEvent::Number(n)) => { @@ -278,7 +278,7 @@ pub fn handle_event( Command::UserInput(InputEvent::HorizontalScroll(val)) => { p.screen.line_wrapping = val; - p.format_lines(); + p.reformat_display(); command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); } @@ -316,7 +316,7 @@ pub fn handle_event( } Command::SetLineNumbers(ln) => { p.line_numbers = ln; - p.format_lines(); + p.reformat_display(); command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); } Command::SetExitStrategy(es) => { @@ -336,7 +336,7 @@ pub fn handle_event( } Command::LineWrapping(lw) => { p.screen.line_wrapping = lw; - p.format_lines(); + p.reformat_display(); } #[cfg(feature = "static_output")] Command::SetRunNoOverflow(val) => p.run_no_overflow = val, @@ -420,8 +420,6 @@ pub fn handle_io_command( drop(active); cvar.notify_one(); - command_queue.push_back(Command::Io(IoCommand::RedrawPrompt)); - // If we only have compiled regex cached, use that otherwise compile the original // string query if its not empty p.search_state.search_term = if search_result.compiled_regex.is_some() { @@ -436,11 +434,19 @@ pub fn handle_io_command( } compiled_regex } else { + command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); return Ok(()); }; - p.format_lines(); + p.reformat_display(); + p.upper_mark = *p + .search_state + .search_idx + .iter() + .nth(p.search_state.search_mark) + .unwrap(); command_queue.push_back(Command::Io(IoCommand::RedrawDisplay)); + command_queue.push_back(Command::Io(IoCommand::RedrawPrompt)); } } Ok(()) @@ -570,7 +576,7 @@ mod tests { let _ = writeln!(t, "line {idx}"); t }); - ps.format_lines(); + ps.reformat_display(); ps.upper_mark = 3; ps.selection_anchor = Some(Selection { absolute_row: 3, @@ -608,7 +614,7 @@ mod tests { let _ = writeln!(t, "line {idx}"); t }); - ps.format_lines(); + ps.reformat_display(); ps.upper_mark = 3; ps.selection_anchor = Some(Selection { absolute_row: 3, @@ -646,7 +652,7 @@ mod tests { let _ = writeln!(t, "line {idx}"); t }); - ps.format_lines(); + ps.reformat_display(); ps.upper_mark = 2; ps.selection_anchor = Some(Selection { absolute_row: 2, diff --git a/src/core/utils/display/mod.rs b/src/core/utils/display/mod.rs index 5d48624..a1eca3d 100644 --- a/src/core/utils/display/mod.rs +++ b/src/core/utils/display/mod.rs @@ -6,7 +6,7 @@ use crossterm::{ terminal::{Clear, ClearType}, }; -use std::{cmp::Ordering, convert::TryInto, io::Write}; +use std::{cmp::Ordering, convert::TryInto, fmt::Display, io::Write}; use super::term; use crate::screen::Row; @@ -219,9 +219,9 @@ pub fn draw_append_text( /// the number of lines of text data. This rule is disobeyed in only one special case which is if number of lines of /// text is less than available rows. In this situation, upper mark is always 0. #[allow(clippy::too_many_arguments)] -pub fn write_text_checked( +pub fn write_text_checked>( out: &mut impl Write, - lines: &[String], + lines: &[L], mut upper_mark: usize, rows: usize, cols: usize, @@ -246,8 +246,7 @@ pub fn write_text_checked( lower_mark = upper_mark.saturating_add(writable_rows.min(line_count)); } - // Add \r to ensure cursor is placed at the beginning of each row - let display_lines: &[String] = &lines[upper_mark..lower_mark]; + let display_lines = &lines[upper_mark..lower_mark]; term::move_cursor(out, 0, 0, false)?; term::clear_entire_screen(out, false)?; @@ -280,9 +279,9 @@ pub fn write_from_pagerstate(out: &mut impl Write, ps: &mut PagerState) -> Resul write_raw_lines(out, &display_lines, Some("\r")) } -pub fn write_lines( +pub fn write_lines>( out: &mut impl Write, - lines: &[String], + lines: &[L], cols: usize, line_wrapping: bool, left_mark: usize, @@ -296,28 +295,29 @@ pub fn write_lines( } } -pub fn write_lines_in_horizontal_scroll( +pub fn write_lines_in_horizontal_scroll>( out: &mut impl Write, - lines: &[String], + lines: &[L], cols: usize, start: usize, line_numbers: bool, line_count: usize, ) -> crate::Result { for line in lines { + let line_str = line.as_ref(); let (first_end, second_start, second_end) = - get_horizontal_scroll_bounds(line, cols, start, line_numbers, line_count); + get_horizontal_scroll_bounds(line_str, cols, start, line_numbers, line_count); - if start < line.len() { + if start < line.as_ref().len() { if line_numbers { writeln!( out, "\r{}{}", - &line[0..first_end], - &line[second_start..second_end] + &line_str[0..first_end], + &line_str[second_start..second_end] )?; } else { - writeln!(out, "\r{}", &line[second_start..second_end])?; + writeln!(out, "\r{}", &line_str[second_start..second_end])?; } } else { writeln!(out, "\r")?; @@ -393,9 +393,9 @@ pub fn get_horizontal_scroll_bounds( /// `initial` tells any extra text to be inserted before each line. For functions that use this /// function over terminals, this should be set to `\r` to avoid broken display. /// The `\r` resets the cursor to the start of the line. -pub fn write_raw_lines( +pub fn write_raw_lines( out: &mut impl Write, - lines: &[String], + lines: &[L], initial: Option<&str>, ) -> Result<(), MinusError> { for line in lines { diff --git a/src/core/utils/display/tests.rs b/src/core/utils/display/tests.rs index eaa2ee2..c96c7db 100644 --- a/src/core/utils/display/tests.rs +++ b/src/core/utils/display/tests.rs @@ -17,7 +17,7 @@ fn short_no_line_numbers() { let mut pager = PagerState::new().unwrap(); pager.screen.orig_text = lines.to_string(); - pager.format_lines(); + pager.reformat_display(); let mut out = Vec::with_capacity(lines.len()); @@ -53,7 +53,7 @@ fn long_no_line_numbers() { // One extra line for prompt pager.rows = 4; pager.screen.orig_text = lines.to_string(); - pager.format_lines(); + pager.reformat_display(); assert!(write_from_pagerstate(&mut out, &mut pager).is_ok()); @@ -67,7 +67,7 @@ fn long_no_line_numbers() { let mut out = Vec::with_capacity(lines.len()); pager.screen.orig_text = "Another line\nThird line\nFourth line\nFifth line\n".to_string(); pager.upper_mark = 1; - pager.format_lines(); + pager.reformat_display(); assert!(write_from_pagerstate(&mut out, &mut pager).is_ok()); @@ -99,7 +99,7 @@ fn short_with_line_numbers() { let mut pager = PagerState::new().unwrap(); pager.screen.orig_text = lines.to_string(); pager.line_numbers = LineNumbers::Enabled; - pager.format_lines(); + pager.reformat_display(); assert!(write_from_pagerstate(&mut out, &mut pager).is_ok()); @@ -134,7 +134,7 @@ fn long_with_line_numbers() { pager.rows = 4; pager.screen.orig_text = lines.to_string(); pager.line_numbers = LineNumbers::Enabled; - pager.format_lines(); + pager.reformat_display(); assert!(write_from_pagerstate(&mut out, &mut pager).is_ok()); @@ -186,7 +186,7 @@ fn big_line_numbers_are_padded() { pager.rows = 11; pager.screen.orig_text = lines; pager.line_numbers = LineNumbers::AlwaysOn; - pager.format_lines(); + pager.reformat_display(); assert!(write_from_pagerstate(&mut out, &mut pager).is_ok()); @@ -229,7 +229,7 @@ fn draw_short_no_line_numbers() { let mut pager = PagerState::new().unwrap(); pager.screen.orig_text = lines.to_string(); pager.line_numbers = LineNumbers::AlwaysOff; - pager.format_lines(); + pager.reformat_display(); assert!(draw_full(&mut out, &mut pager).is_ok()); @@ -264,7 +264,7 @@ fn draw_long_no_line_numbers() { let mut pager = PagerState::new().unwrap(); pager.rows = 3; pager.screen.orig_text = lines.to_string(); - pager.format_lines(); + pager.reformat_display(); assert!(draw_full(&mut out, &mut pager).is_ok()); @@ -310,7 +310,7 @@ fn draw_short_with_line_numbers() { let mut pager = PagerState::new().unwrap(); pager.screen.orig_text = lines.to_string(); pager.line_numbers = LineNumbers::Enabled; - pager.format_lines(); + pager.reformat_display(); assert!(draw_full(&mut out, &mut pager).is_ok()); assert!( @@ -345,7 +345,7 @@ fn draw_long_with_line_numbers() { pager.rows = 3; pager.screen.orig_text = lines.to_string(); pager.line_numbers = LineNumbers::Enabled; - pager.format_lines(); + pager.reformat_display(); assert!(draw_full(&mut out, &mut pager).is_ok()); @@ -399,7 +399,7 @@ fn draw_big_line_numbers_are_padded() { pager.upper_mark = 95; pager.screen.orig_text = lines; pager.line_numbers = LineNumbers::Enabled; - pager.format_lines(); + pager.reformat_display(); assert!(draw_full(&mut out, &mut pager).is_ok()); @@ -426,7 +426,7 @@ fn draw_wrapping_line_numbers() { pager.cols = 30; pager.upper_mark = 2; pager.line_numbers = LineNumbers::Enabled; - pager.format_lines(); + pager.reformat_display(); assert!(draw_full(&mut out, &mut pager).is_ok()); @@ -457,7 +457,7 @@ fn test_draw_no_overflow() { let mut out = Vec::with_capacity(TEXT.len()); let mut pager = PagerState::new().unwrap(); pager.screen.orig_text = TEXT.to_string(); - pager.format_lines(); + pager.reformat_display(); draw_full(&mut out, &mut pager).unwrap(); assert!( String::from_utf8(out) @@ -488,7 +488,7 @@ mod draw_for_change_tests { let mut ps = PagerState::new().unwrap(); ps.upper_mark = 0; ps.screen.orig_text = lines; - ps.format_lines(); + ps.reformat_display(); ps.format_prompt(); ps } diff --git a/src/search.rs b/src/search.rs index 6a91e83..3471382 100644 --- a/src/search.rs +++ b/src/search.rs @@ -62,6 +62,7 @@ use crossterm::{ terminal::{Clear, ClearType}, }; use regex::Regex; +use std::borrow::Cow; use std::collections::BTreeSet; use std::{ convert::{TryFrom, TryInto}, @@ -248,12 +249,12 @@ fn line_matches_query(line: &str, query: &Regex) -> bool { query.is_match(stripped.as_ref()) } -fn preview_line( +fn preview_line<'a>( iso: &IncrementalSearchOpts<'_>, query: &Regex, line_idx: usize, - line: &str, - visible_lines: &mut Vec, + line: &'a str, + visible_lines: &mut Vec>, upper_mark: &mut Option, wrapped: bool, ) { @@ -275,13 +276,16 @@ fn preview_line( .enumerate() .map(|(i, fr)| { let h = highlight_matches_args(&fr.row, query, false); - let match_row = format!("{h}"); - if h.is_match && (wrapped || line_idx + i > iso.initial_upper_mark) { - match_row_idx = Some(line_idx + i); + if h.is_match { + if wrapped || line_idx + i > iso.initial_upper_mark { + match_row_idx = Some(line_idx + i); + } + Cow::Owned(format!("{h}")) + } else { + fr.row } - match_row }) - .collect::>(); + .collect::>>(); if upper_mark.is_none() { if match_row_idx.is_none() { @@ -300,13 +304,16 @@ fn preview_line( } } -fn incremental_preview(iso: &IncrementalSearchOpts<'_>, query: &Regex) -> Option> { +fn incremental_preview<'a>( + iso: &IncrementalSearchOpts<'a>, + query: &'a Regex, +) -> Option>> { if iso.writable_rows == 0 { return None; } let start_line_idx = iso.lines_to_row_map.row_to_line(iso.initial_upper_mark)?; - let mut visible_lines = Vec::with_capacity(iso.writable_rows); + let mut visible_lines: Vec> = Vec::with_capacity(iso.writable_rows); let mut upper_mark = None; for (line_idx, line) in iso @@ -344,15 +351,16 @@ fn incremental_preview(iso: &IncrementalSearchOpts<'_>, query: &Regex) -> Option .saturating_sub(iso.writable_rows); let to_insert = um.saturating_sub(start); let shift = visible_lines.len(); - visible_lines.resize(iso.writable_rows, String::new()); - visible_lines.rotate_left(shift); - for (d, s) in visible_lines - .iter_mut() - .zip(iso.screen.formatted_lines.iter().skip(start)) + for l in iso + .screen + .formatted_lines + .iter() + .skip(start) .take(to_insert) { - d.clone_from(s); + visible_lines.push(Cow::Borrowed(l.as_str())); } + visible_lines.rotate_left(shift); } if upper_mark.is_none() { @@ -907,7 +915,7 @@ pub(crate) fn next_nth_match( let position_of_next_match = if jump == 0 { start_idx } else { - start_idx.saturating_add(jump).saturating_sub(1) % search_idx.len() + start_idx.saturating_add(jump - 1) % search_idx.len() }; Some(position_of_next_match) diff --git a/src/state.rs b/src/state.rs index 60a4426..087bb44 100644 --- a/src/state.rs +++ b/src/state.rs @@ -247,7 +247,7 @@ impl PagerState { Ok(ps) } - pub(crate) fn format_lines(&mut self) { + pub(crate) fn reformat_display(&mut self) { let format_result = screen::format_lines_into( &mut self.screen.formatted_lines, &self.screen.orig_text, @@ -579,7 +579,7 @@ impl PagerState { ); if self.line_numbers.is_on() && (new_lc_dgts != old_lc_dgts && old_lc_dgts != 0) { - self.format_lines(); + self.reformat_display(); return AppendStyle::FullRedraw; } @@ -665,7 +665,7 @@ mod tests { ps.screen.line_wrapping = false; ps.left_mark = 3; ps.screen.orig_text = "abcdefghij\nklmnopqrst\nuvwxyz\n".to_string(); - ps.format_lines(); + ps.reformat_display(); let padding = ps.line_number_padding() as u16; ps.selection_anchor = ps.selection_from_coordinates(padding + 1, 0); @@ -682,7 +682,7 @@ mod tests { let mut ps = PagerState::new().unwrap(); ps.cols = 6; ps.screen.orig_text = "abcdefghi\njklmnop\n".to_string(); - ps.format_lines(); + ps.reformat_display(); ps.selection_anchor = Some(Selection { absolute_row: 0, col: 2, diff --git a/src/tests.rs b/src/tests.rs index 2386a12..e30a70a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -195,7 +195,7 @@ mod pager_append_str { ); ps.screen.orig_text = TEST.to_string(); - ps.format_lines(); + ps.reformat_display(); assert_eq!(ps.screen.orig_text, TEST.to_owned()); assert_eq!( @@ -231,7 +231,7 @@ mod pager_append_str { let row1_ptr = ps.screen.formatted_lines[1].as_ptr(); ps.screen.orig_text = "short\nlines".to_string(); - ps.format_lines(); + ps.reformat_display(); assert_eq!(ps.screen.formatted_lines, vec!["short", "lines"]); assert_eq!(ps.screen.formatted_lines.as_ptr(), vec_ptr); @@ -252,7 +252,7 @@ mod pager_append_str { let row1_ptr = ps.screen.formatted_lines[1].as_ptr(); ps.screen.orig_text = "one\ntwo\nthree".to_string(); - ps.format_lines(); + ps.reformat_display(); assert_eq!(ps.screen.formatted_lines, vec!["one", "two", "three"]); assert_eq!(ps.screen.formatted_lines.as_ptr(), vec_ptr);