Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
34 changes: 11 additions & 23 deletions src/menu/columnar_menu.rs
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 match_width: 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_str, remaining_str) =
split_suggestion(&suggestion.value, self.working_details.match_width);

let suggestion_style_prefix = suggestion
.style
Expand Down Expand Up @@ -510,10 +495,13 @@ impl Menu for ColumnarMenu {
let (values, base_ranges) = completer.complete_with_base_ranges(&input, pos);

self.values = values;
self.working_details.shortest_base_string = base_ranges
self.working_details.match_width = 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
35 changes: 13 additions & 22 deletions src/menu/ide_menu.rs
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 match_width: 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()));
split_suggestion(&suggestion.value, self.working_details.match_width);

let suggestion_style_prefix = suggestion
.style
Expand Down Expand Up @@ -642,10 +632,13 @@ impl Menu for IdeMenu {
let (values, base_ranges) = completer.complete_with_base_ranges(&input, pos);

self.values = values;
self.working_details.shortest_base_string = base_ranges
self.working_details.match_width = 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,7 @@ 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.match_width as u16);
}

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

Expand Down
46 changes: 46 additions & 0 deletions src/menu/menu_functions.rs
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

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,26 @@ 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)
}

#[test]
fn test_split_suggestions_shorter_than_typed() {
let sugg = "a";
let typed = "abcd";
let (got_match_str, _) = split_suggestion(sugg, typed.width());
assert_eq!("a", got_match_str)
}
}