Refactor find in terms of clean Record API (#10929)

# Description
Rewrite `find` internals with the same principles as in #10927.

Here we can remove an unnecessary lookup accross all columns when not
narrowing find to particular columns

- Change `find` internal fns to use iterators
- Remove unnecessary quadratic lookup in `find`
- Refactor `find` record highlight logic
# User-Facing Changes
Should provide a small speedup when not providing `find --columns`

# Tests + Formatting
(-)
This commit is contained in:
Stefan Holderbach 2023-11-08 01:06:22 +01:00 committed by GitHub
parent 60da7abbc7
commit f45aed257f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -1,5 +1,4 @@
use crate::help::highlight_search_string; use crate::help::highlight_search_string;
use itertools::Itertools;
use fancy_regex::Regex; use fancy_regex::Regex;
use nu_ansi_term::Style; use nu_ansi_term::Style;
@ -253,26 +252,29 @@ fn find_with_regex(
input.filter( input.filter(
move |value| match value { move |value| match value {
Value::String { val, .. } => re.is_match(val.as_str()).unwrap_or(false) != invert, Value::String { val, .. } => re.is_match(val.as_str()).unwrap_or(false) != invert,
Value::Record { Value::Record { val, .. } => values_match_find(val.values(), &re, &config, invert),
val: Record { vals, .. }, Value::List { vals, .. } => values_match_find(vals, &re, &config, invert),
..
}
| Value::List { vals, .. } => values_match_find(vals, &re, &config, invert),
_ => false, _ => false,
}, },
ctrlc, ctrlc,
) )
} }
fn values_match_find(values: &[Value], re: &Regex, config: &Config, invert: bool) -> bool { fn values_match_find<'a, I>(values: I, re: &Regex, config: &Config, invert: bool) -> bool
where
I: IntoIterator<Item = &'a Value>,
{
match invert { match invert {
true => !record_matches_regex(values, re, config), true => !record_matches_regex(values, re, config),
false => record_matches_regex(values, re, config), false => record_matches_regex(values, re, config),
} }
} }
fn record_matches_regex(values: &[Value], re: &Regex, config: &Config) -> bool { fn record_matches_regex<'a, I>(values: I, re: &Regex, config: &Config) -> bool
values.iter().any(|v| { where
I: IntoIterator<Item = &'a Value>,
{
values.into_iter().any(|v| {
re.is_match(v.into_string(" ", config).as_str()) re.is_match(v.into_string(" ", config).as_str())
.unwrap_or(false) .unwrap_or(false)
}) })
@ -288,46 +290,31 @@ fn highlight_terms_in_record_with_search_columns(
string_style: Style, string_style: Style,
highlight_style: Style, highlight_style: Style,
) -> Value { ) -> Value {
let cols_to_search = if search_cols.is_empty() { let col_select = !search_cols.is_empty();
&record.cols
} else {
search_cols
};
let term_strs: Vec<_> = terms.iter().map(|v| v.into_string("", config)).collect(); let term_strs: Vec<_> = terms.iter().map(|v| v.into_string("", config)).collect();
// TODO: change API to mutate in place
let mut record = record.clone();
// iterator of Ok((val_str, term_str)) pairs if the value should be highlighted, otherwise Err(val) // iterator of Ok((val_str, term_str)) pairs if the value should be highlighted, otherwise Err(val)
let try_val_highlight = record.iter().map(|(col, val)| { for (col, val) in record.iter_mut() {
if col_select && !search_cols.contains(col) {
continue;
}
let val_str = val.into_string("", config); let val_str = val.into_string("", config);
let predicate = cols_to_search.contains(col); let Some(term_str) = term_strs
predicate
.then_some(val_str)
.and_then(|val_str| {
term_strs
.iter() .iter()
.find(|term_str| contains_ignore_case(&val_str, term_str)) .find(|term_str| contains_ignore_case(&val_str, term_str)) else {
.map(|term_str| (val_str, term_str)) continue;
}) };
.ok_or_else(|| val.clone())
});
// turn Ok pairs into vals of highlighted strings, Err vals is original vals
let new_vals = try_val_highlight
.map_ok(|(val_str, term_str)| {
let highlighted_str = let highlighted_str =
highlight_search_string(&val_str, term_str, &string_style, &highlight_style) highlight_search_string(&val_str, term_str, &string_style, &highlight_style)
.unwrap_or_else(|_| string_style.paint(term_str).to_string()); .unwrap_or_else(|_| string_style.paint(term_str).to_string());
Value::string(highlighted_str, span) *val = Value::string(highlighted_str, span);
}) }
.map(|v| v.unwrap_or_else(|v| v));
Value::record( Value::record(record, span)
Record {
cols: record.cols.clone(),
vals: new_vals.collect(),
},
span,
)
} }
fn contains_ignore_case(string: &str, substring: &str) -> bool { fn contains_ignore_case(string: &str, substring: &str) -> bool {
@ -551,13 +538,10 @@ fn record_matches_term(
term: &Value, term: &Value,
span: Span, span: Span,
) -> bool { ) -> bool {
let cols_to_search = if columns_to_search.is_empty() { // Only perform column selection if given columns.
&record.cols let col_select = !columns_to_search.is_empty();
} else {
columns_to_search
};
record.iter().any(|(col, val)| { record.iter().any(|(col, val)| {
if !cols_to_search.contains(col) { if col_select && !columns_to_search.contains(col) {
return false; return false;
} }
let lower_val = if !val.is_error() { let lower_val = if !val.is_error() {