Skip to content

Commit

Permalink
Use single atom for fuzzy matching (fix #14904) (#14913)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
ysthakur authored Jan 25, 2025
1 parent 299453e commit 22a01d7
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
33 changes: 26 additions & 7 deletions crates/nu-cli/src/completions/completion_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use nu_parser::trim_quotes_str;
use nu_protocol::{CompletionAlgorithm, CompletionSort};
use nu_utils::IgnoreCaseExt;
use nucleo_matcher::{
pattern::{AtomKind, CaseMatching, Normalization, Pattern},
pattern::{Atom, AtomKind, CaseMatching, Normalization},
Config, Matcher, Utf32Str,
};
use std::{borrow::Cow, fmt::Display};
Expand Down Expand Up @@ -38,9 +38,9 @@ enum State<T> {
},
Fuzzy {
matcher: Matcher,
pat: Pattern,
atom: Atom,
/// Holds (haystack, item, score)
items: Vec<(String, T, u32)>,
items: Vec<(String, T, u16)>,
},
}

Expand All @@ -65,7 +65,7 @@ impl<T> NuMatcher<T> {
}
}
MatchAlgorithm::Fuzzy => {
let pat = Pattern::new(
let atom = Atom::new(
needle,
if options.case_sensitive {
CaseMatching::Respect
Expand All @@ -74,13 +74,14 @@ impl<T> NuMatcher<T> {
},
Normalization::Smart,
AtomKind::Fuzzy,
false,
);
NuMatcher {
options,
needle: needle.to_owned(),
state: State::Fuzzy {
matcher: Matcher::new(Config::DEFAULT),
pat,
atom,
items: Vec::new(),
},
}
Expand Down Expand Up @@ -115,13 +116,13 @@ impl<T> NuMatcher<T> {
}
State::Fuzzy {
matcher,
pat,
atom,
items,
} => {
let mut haystack_buf = Vec::new();
let haystack_utf32 = Utf32Str::new(trim_quotes_str(haystack), &mut haystack_buf);
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;
};
if let Some(item) = item {
Expand Down Expand Up @@ -292,4 +293,22 @@ mod test {
// Sort by score, then in alphabetical order
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());
}
}
2 changes: 1 addition & 1 deletion crates/nu-cli/tests/completions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,8 +1038,8 @@ fn subcommand_completions() {
match_suggestions(
&vec![
"food bar".to_string(),
"foo-test-command aagap bcr".to_string(),
"foo-test-command bar".to_string(),
"foo-test-command aagap bcr".to_string(),
],
&suggestions,
);
Expand Down

0 comments on commit 22a01d7

Please sign in to comment.