Skip to content

fix: Fuzzy completion with Unicode chars #887

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

fix: Fuzzy completion with Unicode chars #887

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b62ad02
fix: consider graphemes when split match str
ysthakur Mar 13, 2025
c86a1ba
Rename shortest_base_string to match_width
ysthakur Mar 13, 2025
6606347
Test suggestions shorter than typed
ysthakur Mar 14, 2025
bedb982
Use number of graphemes rather than width
ysthakur Mar 16, 2025
2bf4299
Test empty match
ysthakur Mar 16, 2025
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
32 changes: 10 additions & 22 deletions src/menu/columnar_menu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Menu, MenuBuilder, MenuEvent, MenuSettings};
use super::{menu_functions::split_suggestion, Menu, MenuBuilder, MenuEvent, MenuSettings};
use crate::{
core_editor::Editor,
menu_functions::{can_partially_complete, completer_input, replace_in_buffer},
Expand Down Expand Up @@ -38,8 +38,8 @@ struct ColumnDetails {
pub columns: u16,
/// Column width
pub col_width: usize,
/// The shortest of the strings, which the suggestions are based on
pub shortest_base_string: String,
/// The display width of the shortest string, which the suggestions are based on
pub shortest_base_string: usize,
}

/// Menu to present suggestions in a columnar fashion
Expand Down Expand Up @@ -299,23 +299,8 @@ impl ColumnarMenu {
use_ansi_coloring: bool,
) -> String {
if use_ansi_coloring {
// strip quotes
let is_quote = |c: char| "`'\"".contains(c);
let shortest_base = &self.working_details.shortest_base_string;
let shortest_base = shortest_base
.strip_prefix(is_quote)
.unwrap_or(shortest_base);
let match_len = shortest_base.len();

// Split string so the match text can be styled
let skip_len = suggestion
.value
.chars()
.take_while(|c| is_quote(*c))
.count();
let (match_str, remaining_str) = suggestion
.value
.split_at((match_len + skip_len).min(suggestion.value.len()));
let match_width = self.working_details.shortest_base_string;
let (match_str, remaining_str) = split_suggestion(&suggestion.value, match_width);

let suggestion_style_prefix = suggestion
.style
Expand Down Expand Up @@ -512,8 +497,11 @@ impl Menu for ColumnarMenu {
self.values = values;
self.working_details.shortest_base_string = base_ranges
.iter()
.map(|range| editor.get_buffer()[range.clone()].to_string())
.min_by_key(|s| s.width())
.map(|range| {
let s = &editor.get_buffer()[range.clone()];
s.strip_prefix(['`', '\'', '"']).unwrap_or(s).width()
})
.min()
.unwrap_or_default();

self.reset_position();
Expand Down
36 changes: 14 additions & 22 deletions src/menu/ide_menu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Menu, MenuBuilder, MenuEvent, MenuSettings};
use super::{menu_functions::split_suggestion, Menu, MenuBuilder, MenuEvent, MenuSettings};
use crate::{
core_editor::Editor,
menu_functions::{can_partially_complete, completer_input, replace_in_buffer},
Expand Down Expand Up @@ -125,8 +125,8 @@ struct IdeMenuDetails {
pub space_right: u16,
/// Corrected description offset, based on the available space
pub description_offset: u16,
/// The shortest of the strings, which the suggestions are based on
pub shortest_base_string: String,
/// The display width of the shortest string, which the suggestions are based on
pub shortest_base_string: usize,
}

/// Menu to present suggestions like similar to Ide completion menus
Expand Down Expand Up @@ -512,18 +512,8 @@ impl IdeMenu {
};

if use_ansi_coloring {
// strip quotes
let is_quote = |c: char| "`'\"".contains(c);
let shortest_base = &self.working_details.shortest_base_string;
let shortest_base = shortest_base
.strip_prefix(is_quote)
.unwrap_or(shortest_base);
let match_len = shortest_base.len().min(string.len());

// Split string so the match text can be styled
let skip_len = string.chars().take_while(|c| is_quote(*c)).count();
let (match_str, remaining_str) =
string.split_at((match_len + skip_len).min(string.len()));
let match_width = self.working_details.shortest_base_string;
let (match_str, remaining_str) = split_suggestion(&suggestion.value, match_width);

let suggestion_style_prefix = suggestion
.style
Expand Down Expand Up @@ -644,8 +634,11 @@ impl Menu for IdeMenu {
self.values = values;
self.working_details.shortest_base_string = base_ranges
.iter()
.map(|range| editor.get_buffer()[range.clone()].to_string())
.min_by_key(|s| s.len())
.map(|range| {
let s = &editor.get_buffer()[range.clone()];
s.strip_prefix(['`', '\'', '"']).unwrap_or(s).width()
})
.min()
.unwrap_or_default();

self.reset_position();
Expand Down Expand Up @@ -703,9 +696,8 @@ impl Menu for IdeMenu {
let mut cursor_pos = self.working_details.cursor_col;

if self.default_details.correct_cursor_pos {
let base_string = &self.working_details.shortest_base_string;

cursor_pos = cursor_pos.saturating_sub(base_string.width() as u16);
cursor_pos =
cursor_pos.saturating_sub(self.working_details.shortest_base_string as u16);
}

let border_width = if self.default_details.border.is_some() {
Expand Down Expand Up @@ -1432,7 +1424,7 @@ mod tests {
space_left: 50,
space_right: 50,
description_offset: 50,
shortest_base_string: String::new(),
shortest_base_string: 0,
};
let mut editor = Editor::default();
// backtick at the end of the line
Expand Down Expand Up @@ -1460,7 +1452,7 @@ mod tests {
space_left: 50,
space_right: 50,
description_offset: 50,
shortest_base_string: String::new(),
shortest_base_string: 0,
};
let mut editor = Editor::default();

Expand Down
38 changes: 38 additions & 0 deletions src/menu/menu_functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//! Collection of common functions that can be used to create menus
use unicode_segmentation::UnicodeSegmentation;
use unicode_width::UnicodeWidthStr;

use crate::{Editor, Suggestion, UndoBehavior};

/// Index result obtained from parsing a string with an index marker
Expand Down Expand Up @@ -354,6 +357,27 @@ pub fn can_partially_complete(values: &[Suggestion], editor: &mut Editor) -> boo
}
}

/// Split suggestion based on the display width of the matched part
///
/// For highlighting prefix-matched suggestions
pub fn split_suggestion(sugg: &str, match_width: usize) -> (&str, &str) {
let mut match_end = sugg.len();
let mut curr_width = 0;
for (offset, grapheme) in sugg.grapheme_indices(true) {
if curr_width >= match_width {
match_end = offset;
break;
}
// Strip quotes from the beginning
if offset == 0 && (grapheme == "`" || grapheme == "'" || grapheme == "\"") {
continue;
}
curr_width += grapheme.width();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess with this code doing operations on Unicode width would not panic.

But I have a question if this misbehaves if the user tries to complete starting from " etc. or " being a particular suggestion? (Is this relying on a particular behavior of our Completer implementations to take out all quotes?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess with this code doing operations on Unicode width would not panic.

But I have a question if this misbehaves if the user tries to complete starting from " etc. or " being a particular suggestion? (Is this relying on a particular behavior of our Completer implementations to take out all quotes?)

True, that's one of reasons why this is just a temporary fix.

Copy link
Member Author

@ysthakur ysthakur Mar 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this won't work well if a quote is part of what the user types. As blindFS said, it's a temporary fix. I was planning on allowing adding a separate display value on Suggestions, so that when showing suggestions for files, the displayed values could be unquoted and unescaped. Then, Reedline would no longer have to strip quotes.

This won't solve the problem of getting a common prefix that accounts for quotes, though, so that's going to need some more thought. One obvious thing I can think of is pushing the burden of finding a common prefix onto completers.

This current quote stripping is really for Nushell, and I assume it won't hurt other users of Reedline either.

}

sugg.split_at(match_end)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -697,4 +721,18 @@ mod tests {
assert_eq!(orig_buffer, editor.get_buffer());
assert_eq!(orig_insertion_point, editor.insertion_point());
}

#[rstest]
#[case("おはよう", "おは", "おは")]
#[case("`'おはよう)", "'お", "`'お")]
#[case("おはよう", "a", "お")]
#[case("abcd", "お", "ab")]
fn test_split_suggestions(
#[case] sugg: &str,
#[case] typed: &str,
#[case] exp_match_str: &str,
) {
let (got_match_str, _) = split_suggestion(sugg, typed.width());
assert_eq!(exp_match_str, got_match_str)
}
}