mirror of
https://github.com/nushell/nushell.git
synced 2025-05-09 12:34:26 +02:00
# Description Closes #14904. The bug there was introduced by #14846, which replaced skim with Nucleo. It turns out that Nucleo's `Pattern::new` function doesn't treat the needle as a single atom - it splits on spaces and makes each word its own atom. This PR fixes the problem by creating a single `Atom` for the whole needle rather than creating a `Pattern`. Because of the bug, when you typed `lines <TAB>` (with a space at the end), the suggestion `lines` was also matched. This suggestion was shorter than the original typed needle, which would cause an out-of-bounds error. This also meant that if you typed `foo bar<TAB>`, `foo aaaaa bar` would be shown before `foo bar aaa`. At the time, I didn't realize that it was more intuitive to have `foo bar aaa` be put first. # User-Facing Changes Typing something like `lines <TAB>` should no longer cause a panic. # Tests + Formatting - Added a test to ensure spaces are respected when fuzzy matching - Updated a test with the changed sort order for subcommand suggestions # After Submitting No need to update docs.
This commit is contained in:
parent
299453ecb7
commit
22a01d7e76
@ -2,7 +2,7 @@ use nu_parser::trim_quotes_str;
|
|||||||
use nu_protocol::{CompletionAlgorithm, CompletionSort};
|
use nu_protocol::{CompletionAlgorithm, CompletionSort};
|
||||||
use nu_utils::IgnoreCaseExt;
|
use nu_utils::IgnoreCaseExt;
|
||||||
use nucleo_matcher::{
|
use nucleo_matcher::{
|
||||||
pattern::{AtomKind, CaseMatching, Normalization, Pattern},
|
pattern::{Atom, AtomKind, CaseMatching, Normalization},
|
||||||
Config, Matcher, Utf32Str,
|
Config, Matcher, Utf32Str,
|
||||||
};
|
};
|
||||||
use std::{borrow::Cow, fmt::Display};
|
use std::{borrow::Cow, fmt::Display};
|
||||||
@ -38,9 +38,9 @@ enum State<T> {
|
|||||||
},
|
},
|
||||||
Fuzzy {
|
Fuzzy {
|
||||||
matcher: Matcher,
|
matcher: Matcher,
|
||||||
pat: Pattern,
|
atom: Atom,
|
||||||
/// Holds (haystack, item, score)
|
/// Holds (haystack, item, score)
|
||||||
items: Vec<(String, T, u32)>,
|
items: Vec<(String, T, u16)>,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -65,7 +65,7 @@ impl<T> NuMatcher<T> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
MatchAlgorithm::Fuzzy => {
|
MatchAlgorithm::Fuzzy => {
|
||||||
let pat = Pattern::new(
|
let atom = Atom::new(
|
||||||
needle,
|
needle,
|
||||||
if options.case_sensitive {
|
if options.case_sensitive {
|
||||||
CaseMatching::Respect
|
CaseMatching::Respect
|
||||||
@ -74,13 +74,14 @@ impl<T> NuMatcher<T> {
|
|||||||
},
|
},
|
||||||
Normalization::Smart,
|
Normalization::Smart,
|
||||||
AtomKind::Fuzzy,
|
AtomKind::Fuzzy,
|
||||||
|
false,
|
||||||
);
|
);
|
||||||
NuMatcher {
|
NuMatcher {
|
||||||
options,
|
options,
|
||||||
needle: needle.to_owned(),
|
needle: needle.to_owned(),
|
||||||
state: State::Fuzzy {
|
state: State::Fuzzy {
|
||||||
matcher: Matcher::new(Config::DEFAULT),
|
matcher: Matcher::new(Config::DEFAULT),
|
||||||
pat,
|
atom,
|
||||||
items: Vec::new(),
|
items: Vec::new(),
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
@ -115,13 +116,13 @@ impl<T> NuMatcher<T> {
|
|||||||
}
|
}
|
||||||
State::Fuzzy {
|
State::Fuzzy {
|
||||||
matcher,
|
matcher,
|
||||||
pat,
|
atom,
|
||||||
items,
|
items,
|
||||||
} => {
|
} => {
|
||||||
let mut haystack_buf = Vec::new();
|
let mut haystack_buf = Vec::new();
|
||||||
let haystack_utf32 = Utf32Str::new(trim_quotes_str(haystack), &mut haystack_buf);
|
let haystack_utf32 = Utf32Str::new(trim_quotes_str(haystack), &mut haystack_buf);
|
||||||
let mut indices = Vec::new();
|
let mut indices = Vec::new();
|
||||||
let Some(score) = pat.indices(haystack_utf32, matcher, &mut indices) else {
|
let Some(score) = atom.indices(haystack_utf32, matcher, &mut indices) else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
if let Some(item) = item {
|
if let Some(item) = item {
|
||||||
@ -292,4 +293,22 @@ mod test {
|
|||||||
// Sort by score, then in alphabetical order
|
// Sort by score, then in alphabetical order
|
||||||
assert_eq!(vec!["fob", "foo bar", "foo/bar"], matcher.results());
|
assert_eq!(vec!["fob", "foo bar", "foo/bar"], matcher.results());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn match_algorithm_fuzzy_sort_strip() {
|
||||||
|
let options = CompletionOptions {
|
||||||
|
match_algorithm: MatchAlgorithm::Fuzzy,
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
let mut matcher = NuMatcher::new("'love spaces' ", options);
|
||||||
|
for item in [
|
||||||
|
"'i love spaces'",
|
||||||
|
"'i love spaces' so much",
|
||||||
|
"'lovespaces' ",
|
||||||
|
] {
|
||||||
|
matcher.add(item, item);
|
||||||
|
}
|
||||||
|
// Make sure the spaces are respected
|
||||||
|
assert_eq!(vec!["'i love spaces' so much"], matcher.results());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -1038,8 +1038,8 @@ fn subcommand_completions() {
|
|||||||
match_suggestions(
|
match_suggestions(
|
||||||
&vec![
|
&vec![
|
||||||
"food bar".to_string(),
|
"food bar".to_string(),
|
||||||
"foo-test-command aagap bcr".to_string(),
|
|
||||||
"foo-test-command bar".to_string(),
|
"foo-test-command bar".to_string(),
|
||||||
|
"foo-test-command aagap bcr".to_string(),
|
||||||
],
|
],
|
||||||
&suggestions,
|
&suggestions,
|
||||||
);
|
);
|
||||||
|
Loading…
Reference in New Issue
Block a user