Skip to content

Commit

Permalink
perf(text): Avoid useless conversins in formatted_line
Browse files Browse the repository at this point in the history
  • Loading branch information
AMythicDev committed Feb 7, 2024
1 parent fe985e8 commit 00af2ec
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 52 deletions.
70 changes: 20 additions & 50 deletions src/core/utils/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
//!
//! [`PagerState::lines`]: crate::state::PagerState::lines
use std::collections::HashMap;
use std::{borrow::Cow, collections::HashMap};

use crate::{minus_core, LineNumbers};

Expand Down Expand Up @@ -315,15 +315,15 @@ pub fn format_text_block(mut opts: FormatOpts<'_>) -> FormatResult {
/// [`PagerState::lines`]: crate::state::PagerState::lines
#[allow(clippy::too_many_arguments)]
#[allow(clippy::uninlined_format_args)]
pub fn formatted_line(
line: &str,
pub fn formatted_line<'a>(
line: &'a str,
len_line_number: usize,
idx: usize,
line_numbers: LineNumbers,
#[cfg(feature = "search")] formatted_idx: usize,
#[cfg(feature = "search")] search_idx: &mut BTreeSet<usize>,
#[cfg(feature = "search")] search_idx: &'a mut BTreeSet<usize>,
cols: usize,
#[cfg(feature = "search")] search_term: &Option<regex::Regex>,
#[cfg(feature = "search")] search_term: &'a Option<regex::Regex>,
) -> Vec<String> {
assert!(
!line.contains('\n'),
Expand All @@ -344,30 +344,26 @@ pub fn formatted_line(

// Wrap the line and return an iterator over all the rows
let mut enumerated_rows = if line_numbers {
wrap_str(line, cols.saturating_sub(padding + 2))
.into_iter()
.enumerate()
textwrap::wrap(line, cols.saturating_sub(padding + 2))
} else {
wrap_str(line, cols).into_iter().enumerate()
};
textwrap::wrap(line, cols)
}
.into_iter()
.enumerate();

// highlight the lines with matching search terms
// If a match is found, add this line's index to PagerState::search_idx
#[cfg_attr(not(feature = "search"), allow(unused_mut))]
#[cfg_attr(not(feature = "search"), allow(unused_variables))]
let mut handle_search = |row: String, wrap_idx: usize| {
let mut handle_search = |row: &mut Cow<'a, str>, wrap_idx: usize| {
#[cfg(feature = "search")]
if let Some(st) = search_term.as_ref() {
let (highlighted_row, is_match) = search::highlight_line_matches(&row, st, false);
let (highlighted_row, is_match) = search::highlight_line_matches(row, st, false);
if is_match {
*row.to_mut() = highlighted_row;
search_idx.insert(formatted_idx + wrap_idx);
}
highlighted_row
} else {
row
}
#[cfg(not(feature = "search"))]
row
};

if line_numbers {
Expand All @@ -377,7 +373,7 @@ pub fn formatted_line(
// * If minus is run under test, ascii codes for making the numbers bol is not inserted because they add
// extra difficulty while writing tests
// * Line number is added only to the first row of a line. This makes a better UI overall
let formatter = |row: String, is_first_row: bool, idx: usize| {
let formatter = |row: Cow<'_, str>, is_first_row: bool, idx: usize| {
format!(
"{bold}{number: >len$}{reset} {row}",
bold = if cfg!(not(test)) && is_first_row {
Expand Down Expand Up @@ -405,7 +401,7 @@ pub fn formatted_line(
let first_row = {
#[cfg_attr(not(feature = "search"), allow(unused_mut))]
let mut row = enumerated_rows.next().unwrap().1;
row = handle_search(row, 0);
handle_search(&mut row, 0);
formatter(row, true, idx)
};
formatted_rows.push(first_row);
Expand All @@ -414,7 +410,7 @@ pub fn formatted_line(
#[cfg_attr(not(feature = "search"), allow(unused_variables))]
let mut rows_left = enumerated_rows
.map(|(wrap_idx, mut row)| {
row = handle_search(row, wrap_idx);
handle_search(&mut row, wrap_idx);
formatter(row, false, 0)
})
.collect::<Vec<String>>();
Expand All @@ -425,20 +421,14 @@ pub fn formatted_line(
// If line numbers aren't active, simply return the rows with search matches highlighted if search is active
#[cfg_attr(not(feature = "search"), allow(unused_variables))]
enumerated_rows
.map(|(wrap_idx, row)| handle_search(row, wrap_idx))
.map(|(wrap_idx, mut row)| {
handle_search(&mut row, wrap_idx);
row.to_string()
})
.collect::<Vec<String>>()
}
}

/// Wrap a line of string into a rows of a terminal based on the number of columns
/// Read the [`textwrap::wrap`] function for more info
pub fn wrap_str(line: &str, cols: usize) -> Vec<String> {
textwrap::wrap(line, cols)
.iter()
.map(ToString::to_string)
.collect::<Vec<String>>()
}

pub fn make_format_lines(
text: &String,
line_numbers: LineNumbers,
Expand Down Expand Up @@ -651,23 +641,3 @@ third line\n",
assert_eq!(3, append_style.num_unterminated);
}
}

mod wrapping {
// Test wrapping functions
#[test]
fn test_wrap_str() {
let test = {
let mut line = String::with_capacity(200);
for _ in 1..=200 {
line.push('#');
}
line
};
let result = super::wrap_str(&test, 80);
assert_eq!(result.len(), 3);
assert_eq!(
(80, 80, 40),
(result[0].len(), result[1].len(), result[2].len()),
);
}
}
2 changes: 1 addition & 1 deletion src/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl Screen {
///
/// NOTE: This operation might be expensive if the text data is too large.
#[must_use]
pub fn get_line_count(&self) -> usize {
pub const fn get_line_count(&self) -> usize {
self.line_count
}
/// Returns all the text within the bounds
Expand Down
2 changes: 1 addition & 1 deletion src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ impl PagerState {
append_props.lines_formatted,
);

let new_lc = old_lc + lines_formatted.saturating_sub(if clean_append { 0 } else { 1 });
let new_lc = old_lc + lines_formatted.saturating_sub(usize::from(!clean_append));
self.screen.line_count = new_lc;
let new_lc_dgts = minus_core::digits(new_lc);

Expand Down

0 comments on commit 00af2ec

Please sign in to comment.