Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust fuzzy matching to be more inline with other editors #3969

Merged
merged 2 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion book/src/keymap.md
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,6 @@ Keys to use within picker. Remapping currently not supported.
| `PageDown`, `Ctrl-d` | Page down |
| `Home` | Go to first entry |
| `End` | Go to last entry |
| `Ctrl-space` | Filter options |
| `Enter` | Open selected |
| `Ctrl-s` | Open horizontally |
| `Ctrl-v` | Open vertically |
Expand Down
74 changes: 74 additions & 0 deletions helix-term/src/ui/fuzzy_match.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use fuzzy_matcher::skim::SkimMatcherV2 as Matcher;
use fuzzy_matcher::FuzzyMatcher;

#[cfg(test)]
mod test;

pub struct FuzzyQuery {
queries: Vec<String>,
}

impl FuzzyQuery {
pub fn new(query: &str) -> FuzzyQuery {
let mut saw_backslash = false;
let queries = query
.split(|c| {
saw_backslash = match c {
' ' if !saw_backslash => return true,
'\\' => true,
_ => false,
};
false
})
.filter_map(|query| {
if query.is_empty() {
None
} else {
Some(query.replace("\\ ", " "))
}
})
.collect();
FuzzyQuery { queries }
}
pascalkuthe marked this conversation as resolved.
Show resolved Hide resolved

pub fn fuzzy_match(&self, item: &str, matcher: &Matcher) -> Option<i64> {
// use the rank of the first query for the rank, because merging ranks is not really possible
// this behaviour matches fzf and skim
let score = matcher.fuzzy_match(item, self.queries.get(0)?)?;
if self
.queries
.iter()
.any(|query| matcher.fuzzy_match(item, query).is_none())
{
return None;
}
Some(score)
}

pub fn fuzzy_indicies(&self, item: &str, matcher: &Matcher) -> Option<(i64, Vec<usize>)> {
if self.queries.len() == 1 {
return matcher.fuzzy_indices(item, &self.queries[0]);
}

// use the rank of the first query for the rank, because merging ranks is not really possible
// this behaviour matches fzf and skim
let (score, mut indicies) = matcher.fuzzy_indices(item, self.queries.get(0)?)?;

// fast path for the common case of not using a space
// during matching this branch should be free thanks to branch prediction
if self.queries.len() == 1 {
return Some((score, indicies));
}

for query in &self.queries[1..] {
let (_, matched_indicies) = matcher.fuzzy_indices(item, query)?;
indicies.extend_from_slice(&matched_indicies);
}

// deadup and remove duplicate matches
indicies.sort_unstable();
indicies.dedup();

Some((score, indicies))
}
}
47 changes: 47 additions & 0 deletions helix-term/src/ui/fuzzy_match/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use crate::ui::fuzzy_match::FuzzyQuery;
use crate::ui::fuzzy_match::Matcher;

fn run_test<'a>(query: &str, items: &'a [&'a str]) -> Vec<String> {
let query = FuzzyQuery::new(query);
let matcher = Matcher::default();
items
.iter()
.filter_map(|item| {
let (_, indicies) = query.fuzzy_indicies(item, &matcher)?;
let matched_string = indicies
.iter()
.map(|&pos| item.chars().nth(pos).unwrap())
.collect();
Some(matched_string)
})
.collect()
}

#[test]
fn match_single_value() {
let matches = run_test("foo", &["foobar", "foo", "bar"]);
assert_eq!(matches, &["foo", "foo"])
}

#[test]
fn match_multiple_values() {
let matches = run_test(
"foo bar",
&["foo bar", "foo bar", "bar foo", "bar", "foo"],
);
assert_eq!(matches, &["foobar", "foobar", "barfoo"])
}

#[test]
fn space_escape() {
let matches = run_test(r"foo\ bar", &["bar foo", "foo bar", "foobar"]);
assert_eq!(matches, &["foo bar"])
}

#[test]
fn trim() {
let matches = run_test(r" foo bar ", &["bar foo", "foo bar", "foobar"]);
assert_eq!(matches, &["barfoo", "foobar", "foobar"]);
let matches = run_test(r" foo bar\ ", &["bar foo", "foo bar", "foobar"]);
assert_eq!(matches, &["bar foo"])
}
1 change: 1 addition & 0 deletions helix-term/src/ui/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod completion;
pub(crate) mod editor;
mod fuzzy_match;
mod info;
pub mod lsp;
mod markdown;
Expand Down
37 changes: 8 additions & 29 deletions helix-term/src/ui/picker.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use crate::{
compositor::{Component, Compositor, Context, Event, EventResult},
ctrl, key, shift,
ui::{self, EditorView},
ui::{self, fuzzy_match::FuzzyQuery, EditorView},
};
use tui::{
buffer::Buffer as Surface,
widgets::{Block, BorderType, Borders},
};

use fuzzy_matcher::skim::SkimMatcherV2 as Matcher;
use fuzzy_matcher::FuzzyMatcher;
use tui::widgets::Widget;

use std::time::Instant;
Expand Down Expand Up @@ -287,8 +286,6 @@ pub struct Picker<T: Item> {
matcher: Box<Matcher>,
/// (index, score)
matches: Vec<(usize, i64)>,
/// Filter over original options.
filters: Vec<usize>, // could be optimized into bit but not worth it now

/// Current height of the completions box
completion_height: u16,
Expand Down Expand Up @@ -323,7 +320,6 @@ impl<T: Item> Picker<T> {
editor_data,
matcher: Box::new(Matcher::default()),
matches: Vec::new(),
filters: Vec::new(),
cursor: 0,
prompt,
previous_pattern: String::new(),
Expand Down Expand Up @@ -365,13 +361,14 @@ impl<T: Item> Picker<T> {
.map(|(index, _option)| (index, 0)),
);
} else if pattern.starts_with(&self.previous_pattern) {
let query = FuzzyQuery::new(pattern);
// optimization: if the pattern is a more specific version of the previous one
// then we can score the filtered set.
self.matches.retain_mut(|(index, score)| {
let option = &self.options[*index];
let text = option.sort_text(&self.editor_data);

match self.matcher.fuzzy_match(&text, pattern) {
match query.fuzzy_match(&text, &self.matcher) {
Some(s) => {
// Update the score
*score = s;
Expand All @@ -384,23 +381,17 @@ impl<T: Item> Picker<T> {
self.matches
.sort_unstable_by_key(|(_, score)| Reverse(*score));
} else {
let query = FuzzyQuery::new(pattern);
self.matches.clear();
self.matches.extend(
self.options
.iter()
.enumerate()
.filter_map(|(index, option)| {
// filter options first before matching
if !self.filters.is_empty() {
// TODO: this filters functionality seems inefficient,
// instead store and operate on filters if any
self.filters.binary_search(&index).ok()?;
}

let text = option.filter_text(&self.editor_data);

self.matcher
.fuzzy_match(&text, pattern)
query
.fuzzy_match(&text, &self.matcher)
.map(|score| (index, score))
}),
);
Expand Down Expand Up @@ -460,14 +451,6 @@ impl<T: Item> Picker<T> {
.map(|(index, _score)| &self.options[*index])
}

pub fn save_filter(&mut self, cx: &Context) {
self.filters.clear();
self.filters
.extend(self.matches.iter().map(|(index, _)| *index));
self.filters.sort_unstable(); // used for binary search later
self.prompt.clear(cx.editor);
}

pub fn toggle_preview(&mut self) {
self.show_preview = !self.show_preview;
}
Expand Down Expand Up @@ -545,9 +528,6 @@ impl<T: Item + 'static> Component for Picker<T> {
}
return close_fn;
}
ctrl!(' ') => {
self.save_filter(cx);
}
ctrl!('t') => {
self.toggle_preview();
}
Expand Down Expand Up @@ -630,9 +610,8 @@ impl<T: Item + 'static> Component for Picker<T> {
}

let spans = option.label(&self.editor_data);
let (_score, highlights) = self
.matcher
.fuzzy_indices(&String::from(&spans), self.prompt.line())
let (_score, highlights) = FuzzyQuery::new(self.prompt.line())
.fuzzy_indicies(&String::from(&spans), &self.matcher)
.unwrap_or_default();

spans.0.into_iter().fold(inner, |pos, span| {
Expand Down