fix: Make ColumnarMenu suggestions always take 1 line

src/menu/columnar_menu.rs

@@ -3,7 +3,7 @@ use crate::{
     core_editor::Editor,
     menu_functions::{
         can_partially_complete, completer_input, floor_char_boundary, get_match_indices,
-        replace_in_buffer, style_suggestion,
+        replace_in_buffer, style_suggestion, truncate_no_ansi,
     },
     painting::Painter,
     Completer, Suggestion,
@@ -74,6 +74,8 @@ pub struct ColumnarMenu {
     working_details: ColumnDetails,
     /// Menu cached values
     values: Vec<Suggestion>,
+    /// Cached display width of each suggestion in `values`
+    display_widths: Vec<usize>,
     /// column position of the cursor. Starts from 0
     col_pos: u16,
     /// row position in the menu. Starts from 0
@@ -98,6 +100,7 @@ impl Default for ColumnarMenu {
             min_rows: 3,
             working_details: ColumnDetails::default(),
             values: Vec::new(),
+            display_widths: Vec::new(),
             col_pos: 0,
             row_pos: 0,
             skip_rows: 0,
@@ -381,7 +384,7 @@ impl ColumnarMenu {
         use_ansi_coloring: bool,
     ) -> String {
         let selected = index == self.index();
-        let empty_space = self.get_width().saturating_sub(suggestion.value.width());
+        let empty_space = self.get_width().saturating_sub(self.display_widths[index]);
 
         if use_ansi_coloring {
             // TODO(ysthakur): let the user strip quotes, rather than doing it here
@@ -394,53 +397,56 @@ impl ColumnarMenu {
             let match_indices =
                 get_match_indices(&suggestion.value, &suggestion.match_indices, shortest_base);
 
-            let left_text_size = self.longest_suggestion + self.default_details.col_padding;
+            let left_text_size = self
+                .get_width()
+                .min(self.longest_suggestion + self.default_details.col_padding);
             let description_size = self.get_width().saturating_sub(left_text_size);
-            let padding = left_text_size.saturating_sub(suggestion.value.len());
+            let padding = left_text_size.saturating_sub(self.display_widths[index]);
 
             let value_style = if selected {
                 &self.settings.color.selected_text_style
             } else {
                 &suggestion.style.unwrap_or(self.settings.color.text_style)
             };
+            let value_trunc = truncate_no_ansi(&suggestion.value, left_text_size);
             let styled_value = style_suggestion(
-                &value_style.paint(&suggestion.value).to_string(),
+                &value_style.paint(value_trunc).to_string(),
                 match_indices.as_ref(),
                 &self.settings.color.match_style,
             );
 
-            if let Some(description) = &suggestion.description {
-                let desc_trunc = description
-                    .chars()
-                    .take(description_size)
-                    .collect::<String>()
-                    .replace('\n', " ");
-                if selected {
-                    format!(
-                        "{}{}{}{}{}",
-                        styled_value,
-                        value_style.prefix(),
-                        " ".repeat(padding),
-                        desc_trunc,
-                        RESET,
-                    )
-                } else {
+            match &suggestion.description {
+                Some(desc) if description_size > 3 => {
+                    let desc = desc.replace('\n', "");
+                    let desc_trunc = truncate_no_ansi(desc.as_str(), description_size);
+                    if selected {
+                        format!(
+                            "{}{}{}{}{}",
+                            styled_value,
+                            value_style.prefix(),
+                            " ".repeat(padding),
+                            desc_trunc,
+                            RESET,
+                        )
+                    } else {
+                        format!(
+                            "{}{}{}{}",
+                            styled_value,
+                            " ".repeat(padding),
+                            self.settings.color.description_style.paint(desc_trunc),
+                            RESET,
+                        )
+                    }
+                }
+                _ => {
                     format!(
-                        "{}{}{}{}",
+                        "{}{}{:>empty$}",
                         styled_value,
-                        " ".repeat(padding),
-                        self.settings.color.description_style.paint(desc_trunc),
                         RESET,
+                        "",
+                        empty = empty_space
                     )
                 }
-            } else {
-                format!(
-                    "{}{}{:>empty$}",
-                    styled_value,
-                    RESET,
-                    "",
-                    empty = empty_space
-                )
             }
         } else {
             // If no ansi coloring is found, then the selection word is the line in uppercase
@@ -548,6 +554,7 @@ impl Menu for ColumnarMenu {
         let (values, base_ranges) = completer.complete_with_base_ranges(&input, pos);
 
         self.values = values;
+        self.display_widths = self.values.iter().map(|sugg| sugg.value.width()).collect();
         self.working_details.shortest_base_string = base_ranges
             .iter()
             .map(|range| {
@@ -614,43 +621,25 @@ impl Menu for ColumnarMenu {
                 .iter()
                 .any(|suggestion| suggestion.description.is_some());
 
+            let screen_width = painter.screen_width() as usize;
+            self.longest_suggestion = *self.display_widths.iter().max().unwrap_or(&0);
             if exist_description {
                 self.working_details.columns = 1;
-                self.working_details.col_width = painter.screen_width() as usize;
-
-                self.longest_suggestion = self.get_values().iter().fold(0, |prev, suggestion| {
-                    if prev >= suggestion.value.width() {
-                        prev
-                    } else {
-                        suggestion.value.width()
-                    }
-                });
+                self.working_details.col_width = screen_width;
             } else {
-                let max_width = self.get_values().iter().fold(0, |acc, suggestion| {
-                    let str_len = suggestion.value.width() + self.default_details.col_padding;
-                    if str_len > acc {
-                        str_len
-                    } else {
-                        acc
-                    }
-                });
-
                 // If no default width is found, then the total screen width is used to estimate
                 // the column width based on the default number of columns
                 let default_width = if let Some(col_width) = self.default_details.col_width {
                     col_width
                 } else {
-                    let col_width = painter.screen_width() / self.default_details.columns;
-                    col_width as usize
+                    screen_width / self.default_details.columns as usize
                 };
 
                 // Adjusting the working width of the column based the max line width found
                 // in the menu values
-                if max_width > default_width {
-                    self.working_details.col_width = max_width;
-                } else {
-                    self.working_details.col_width = default_width;
-                };
+                self.working_details.col_width = default_width
+                    .max(self.longest_suggestion + self.default_details.col_padding)
+                    .min(screen_width);
 
                 // The working columns is adjusted based on possible number of columns
                 // that could be fitted in the screen with the calculated column width
@@ -766,6 +755,8 @@ impl Menu for ColumnarMenu {
 
 #[cfg(test)]
 mod tests {
+    use std::io::BufWriter;
+
     use crate::{Span, UndoBehavior};
 
     use super::*;
@@ -873,6 +864,19 @@ mod tests {
         }
     }
 
+    fn setup_menu(
+        menu: &mut ColumnarMenu,
+        editor: &mut Editor,
+        completer: &mut dyn Completer,
+        terminal_size: (u16, u16),
+    ) {
+        let mut painter = Painter::new(BufWriter::new(std::io::stderr()));
+        painter.handle_resize(terminal_size.0, terminal_size.1);
+
+        menu.menu_event(MenuEvent::Activate(false));
+        menu.update_working_details(editor, completer, &painter);
+    }
+
     #[test]
     fn test_menu_replace_backtick() {
         // https://github.com/nushell/nushell/issues/7885
@@ -900,9 +904,9 @@ mod tests {
         let mut completer = FakeCompleter::new(&["おはよう", "`おはよう(`"]);
         let mut menu = ColumnarMenu::default().with_name("testmenu");
         let mut editor = Editor::default();
-
         editor.set_buffer("おは".to_string(), UndoBehavior::CreateUndoPoint);
-        menu.update_values(&mut editor, &mut completer);
+        setup_menu(&mut menu, &mut editor, &mut completer, (10, 10));
+
         assert!(menu.menu_string(2, true).contains("おは"));
     }
 
@@ -912,10 +916,10 @@ mod tests {
         let mut completer = FakeCompleter::new(&["验abc/"]);
         let mut menu = ColumnarMenu::default().with_name("testmenu");
         let mut editor = Editor::default();
-
         editor.set_buffer("ac".to_string(), UndoBehavior::CreateUndoPoint);
-        menu.update_values(&mut editor, &mut completer);
-        assert!(menu.menu_string(10, true).contains("验"));
+        setup_menu(&mut menu, &mut editor, &mut completer, (10, 10));
+
+        assert!(menu.menu_string(2, true).contains("验"));
     }
 
     #[test]
@@ -924,9 +928,9 @@ mod tests {
         let mut completer = FakeCompleter::new(&[&("验".repeat(205) + "abc/")]);
         let mut menu = ColumnarMenu::default().with_name("testmenu");
         let mut editor = Editor::default();
-
         editor.set_buffer("a".to_string(), UndoBehavior::CreateUndoPoint);
-        menu.update_values(&mut editor, &mut completer);
+        setup_menu(&mut menu, &mut editor, &mut completer, (10, 10));
+
         assert!(menu.menu_string(10, true).contains("验"));
     }
 

src/menu/ide_menu.rs

@@ -3,7 +3,7 @@ use crate::{
     core_editor::Editor,
     menu_functions::{
         can_partially_complete, completer_input, floor_char_boundary, get_match_indices,
-        replace_in_buffer, style_suggestion,
+        replace_in_buffer, style_suggestion, truncate_no_ansi,
     },
     painting::Painter,
     Completer, Suggestion,
@@ -506,17 +506,7 @@ impl IdeMenu {
         let max_string_width =
             (self.working_details.completion_width as usize).saturating_sub(border_width + padding);
 
-        let string = if suggestion.value.chars().count() > max_string_width {
-            let mut chars = suggestion
-                .value
-                .chars()
-                .take(max_string_width.saturating_sub(3))
-                .collect::<String>();
-            chars.push_str("...");
-            chars
-        } else {
-            suggestion.value.clone()
-        };
+        let string = truncate_no_ansi(&suggestion.value, max_string_width);
 
         if use_ansi_coloring {
             // TODO(ysthakur): let the user strip quotes, rather than doing it here
@@ -531,42 +521,35 @@ impl IdeMenu {
 
             let suggestion_style = suggestion.style.unwrap_or(self.settings.color.text_style);
 
-            if index == self.index() {
-                format!(
-                    "{}{}{}{}{}{}{}",
-                    vertical_border,
-                    suggestion_style.prefix(),
-                    " ".repeat(padding),
-                    style_suggestion(
-                        &self
-                            .settings
-                            .color
-                            .selected_text_style
-                            .paint(&string)
-                            .to_string(),
-                        &match_indices,
-                        &self.settings.color.selected_match_style,
-                    ),
-                    " ".repeat(padding_right),
-                    RESET,
-                    vertical_border,
+            let styled_string = if index == self.index() {
+                style_suggestion(
+                    &self
+                        .settings
+                        .color
+                        .selected_text_style
+                        .paint(string)
+                        .to_string(),
+                    &match_indices,
+                    &self.settings.color.selected_match_style,
                 )
             } else {
-                format!(
-                    "{}{}{}{}{}{}{}",
-                    vertical_border,
-                    suggestion_style.prefix(),
-                    " ".repeat(padding),
-                    style_suggestion(
-                        &suggestion_style.paint(&string).to_string(),
-                        &match_indices,
-                        &self.settings.color.match_style,
-                    ),
-                    " ".repeat(padding_right),
-                    RESET,
-                    vertical_border,
+                style_suggestion(
+                    &suggestion_style.paint(string).to_string(),
+                    &match_indices,
+                    &self.settings.color.match_style,
                 )
-            }
+            };
+
+            format!(
+                "{}{}{}{}{}{}{}",
+                vertical_border,
+                suggestion_style.prefix(),
+                " ".repeat(padding),
+                styled_string,
+                " ".repeat(padding_right),
+                RESET,
+                vertical_border,
+            )
         } else {
             let marker = if index == self.index() { ">" } else { "" };
 

src/menu/menu_functions.rs

@@ -8,6 +8,7 @@ use itertools::{
 };
 use nu_ansi_term::{ansi::RESET, Style};
 use unicode_segmentation::UnicodeSegmentation;
+use unicode_width::UnicodeWidthStr;
 
 use crate::{Editor, Suggestion, UndoBehavior};
 
@@ -468,6 +469,31 @@ pub fn get_match_indices<'a>(
     }
 }
 
+/// Truncate a string with no ANSI escapes to the given max width, which must be >=3.
+///
+/// If `s` is longer than `max_width`, the resulting string will end in "..."
+/// and have width at most `max_width`.
+pub(crate) fn truncate_no_ansi(s: &str, max_width: usize) -> Cow<'_, str> {
+    if s.width() <= max_width {
+        Cow::Borrowed(s)
+    } else {
+        let trunc_suffix = "...";
+        let suffix_width = trunc_suffix.width();
+
+        let mut res = String::new();
+        let mut curr_width = 0;
+        for grapheme in s.graphemes(true) {
+            curr_width += grapheme.width();
+            if curr_width + suffix_width > max_width {
+                break;
+            }
+            res.push_str(grapheme);
+        }
+        res.push_str(trunc_suffix);
+        Cow::Owned(res)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -856,4 +882,14 @@ mod tests {
             )
         );
     }
+
+    #[rstest]
+    #[case::shorter("asdf", 5, "asdf")]
+    // Ｈ has width 2
+    #[case::exact_width("asdＨ", 5, "asdＨ")]
+    #[case::one_longer("asdfＨ", 5, "as...")]
+    #[case::result_thinner_than_max("aＨＨＨ", 5, "a...")]
+    fn test_truncate(#[case] value: &str, #[case] max_width: usize, #[case] expected: &str) {
+        assert_eq!(expected, truncate_no_ansi(value, max_width));
+    }
 }