Truncate code actions with a long label and show full label aside (#45268)

Gaauwe Rombouts created

Closes #43355

Fixes the issue were code actions with long labels would get cut off
without being able to see the full description. We now properly truncate
those labels with an ellipsis and show the full description in an aside.

Release Notes:

- Added ellipsis to truncated code actions and an aside showing the full
action description.

Change summary

crates/editor/src/code_context_menus.rs     | 151 +++++++++++-----------
crates/gpui/src/text_system/line_wrapper.rs |  45 ++++--
2 files changed, 106 insertions(+), 90 deletions(-)

Detailed changes

crates/editor/src/code_context_menus.rs 🔗

@@ -51,6 +51,8 @@ pub const MENU_ASIDE_MIN_WIDTH: Pixels = px(260.);
 pub const MENU_ASIDE_MAX_WIDTH: Pixels = px(500.);
 pub const COMPLETION_MENU_MIN_WIDTH: Pixels = px(280.);
 pub const COMPLETION_MENU_MAX_WIDTH: Pixels = px(540.);
+pub const CODE_ACTION_MENU_MIN_WIDTH: Pixels = px(220.);
+pub const CODE_ACTION_MENU_MAX_WIDTH: Pixels = px(540.);
 
 // Constants for the markdown cache. The purpose of this cache is to reduce flickering due to
 // documentation not yet being parsed.
@@ -179,7 +181,7 @@ impl CodeContextMenu {
     ) -> Option<AnyElement> {
         match self {
             CodeContextMenu::Completions(menu) => menu.render_aside(max_size, window, cx),
-            CodeContextMenu::CodeActions(_) => None,
+            CodeContextMenu::CodeActions(menu) => menu.render_aside(max_size, window, cx),
         }
     }
 
@@ -1419,26 +1421,6 @@ pub enum CodeActionsItem {
 }
 
 impl CodeActionsItem {
-    fn as_task(&self) -> Option<&ResolvedTask> {
-        let Self::Task(_, task) = self else {
-            return None;
-        };
-        Some(task)
-    }
-
-    fn as_code_action(&self) -> Option<&CodeAction> {
-        let Self::CodeAction { action, .. } = self else {
-            return None;
-        };
-        Some(action)
-    }
-    fn as_debug_scenario(&self) -> Option<&DebugScenario> {
-        let Self::DebugScenario(scenario) = self else {
-            return None;
-        };
-        Some(scenario)
-    }
-
     pub fn label(&self) -> String {
         match self {
             Self::CodeAction { action, .. } => action.lsp_action.title().to_owned(),
@@ -1446,6 +1428,14 @@ impl CodeActionsItem {
             Self::DebugScenario(scenario) => scenario.label.to_string(),
         }
     }
+
+    pub fn menu_label(&self) -> String {
+        match self {
+            Self::CodeAction { action, .. } => action.lsp_action.title().replace("\n", ""),
+            Self::Task(_, task) => task.resolved_label.replace("\n", ""),
+            Self::DebugScenario(scenario) => format!("debug: {}", scenario.label),
+        }
+    }
 }
 
 pub struct CodeActionsMenu {
@@ -1555,60 +1545,33 @@ impl CodeActionsMenu {
                         let item_ix = range.start + ix;
                         let selected = item_ix == selected_item;
                         let colors = cx.theme().colors();
-                        div().min_w(px(220.)).max_w(px(540.)).child(
-                            ListItem::new(item_ix)
-                                .inset(true)
-                                .toggle_state(selected)
-                                .when_some(action.as_code_action(), |this, action| {
-                                    this.child(
-                                        h_flex()
-                                            .overflow_hidden()
-                                            .when(is_quick_action_bar, |this| this.text_ui(cx))
-                                            .child(
-                                                // TASK: It would be good to make lsp_action.title a SharedString to avoid allocating here.
-                                                action.lsp_action.title().replace("\n", ""),
-                                            )
-                                            .when(selected, |this| {
-                                                this.text_color(colors.text_accent)
-                                            }),
-                                    )
-                                })
-                                .when_some(action.as_task(), |this, task| {
-                                    this.child(
-                                        h_flex()
-                                            .overflow_hidden()
-                                            .when(is_quick_action_bar, |this| this.text_ui(cx))
-                                            .child(task.resolved_label.replace("\n", ""))
-                                            .when(selected, |this| {
-                                                this.text_color(colors.text_accent)
-                                            }),
-                                    )
-                                })
-                                .when_some(action.as_debug_scenario(), |this, scenario| {
-                                    this.child(
-                                        h_flex()
-                                            .overflow_hidden()
-                                            .when(is_quick_action_bar, |this| this.text_ui(cx))
-                                            .child("debug: ")
-                                            .child(scenario.label.clone())
-                                            .when(selected, |this| {
-                                                this.text_color(colors.text_accent)
-                                            }),
-                                    )
-                                })
-                                .on_click(cx.listener(move |editor, _, window, cx| {
-                                    cx.stop_propagation();
-                                    if let Some(task) = editor.confirm_code_action(
-                                        &ConfirmCodeAction {
-                                            item_ix: Some(item_ix),
-                                        },
-                                        window,
-                                        cx,
-                                    ) {
-                                        task.detach_and_log_err(cx)
-                                    }
-                                })),
-                        )
+
+                        ListItem::new(item_ix)
+                            .inset(true)
+                            .toggle_state(selected)
+                            .overflow_x()
+                            .child(
+                                div()
+                                    .min_w(CODE_ACTION_MENU_MIN_WIDTH)
+                                    .max_w(CODE_ACTION_MENU_MAX_WIDTH)
+                                    .overflow_hidden()
+                                    .text_ellipsis()
+                                    .when(is_quick_action_bar, |this| this.text_ui(cx))
+                                    .when(selected, |this| this.text_color(colors.text_accent))
+                                    .child(action.menu_label()),
+                            )
+                            .on_click(cx.listener(move |editor, _, window, cx| {
+                                cx.stop_propagation();
+                                if let Some(task) = editor.confirm_code_action(
+                                    &ConfirmCodeAction {
+                                        item_ix: Some(item_ix),
+                                    },
+                                    window,
+                                    cx,
+                                ) {
+                                    task.detach_and_log_err(cx)
+                                }
+                            }))
                     })
                     .collect()
             }),
@@ -1635,4 +1598,42 @@ impl CodeActionsMenu {
 
         Popover::new().child(list).into_any_element()
     }
+
+    fn render_aside(
+        &mut self,
+        max_size: Size<Pixels>,
+        window: &mut Window,
+        _cx: &mut Context<Editor>,
+    ) -> Option<AnyElement> {
+        let Some(action) = self.actions.get(self.selected_item) else {
+            return None;
+        };
+
+        let label = action.menu_label();
+        let text_system = window.text_system();
+        let mut line_wrapper = text_system.line_wrapper(
+            window.text_style().font(),
+            window.text_style().font_size.to_pixels(window.rem_size()),
+        );
+        let is_truncated =
+            line_wrapper.should_truncate_line(&label, CODE_ACTION_MENU_MAX_WIDTH, "…");
+
+        if is_truncated.is_none() {
+            return None;
+        }
+
+        Some(
+            Popover::new()
+                .child(
+                    div()
+                        .child(label)
+                        .id("code_actions_menu_extended")
+                        .px(MENU_ASIDE_X_PADDING / 2.)
+                        .max_w(max_size.width)
+                        .max_h(max_size.height)
+                        .occlude(),
+                )
+                .into_any_element(),
+        )
+    }
 }

crates/gpui/src/text_system/line_wrapper.rs 🔗

@@ -128,22 +128,21 @@ impl LineWrapper {
         })
     }
 
-    /// Truncate a line of text to the given width with this wrapper's font and font size.
-    pub fn truncate_line<'a>(
+    /// Determines if a line should be truncated based on its width.
+    pub fn should_truncate_line(
         &mut self,
-        line: SharedString,
+        line: &str,
         truncate_width: Pixels,
         truncation_suffix: &str,
-        runs: &'a [TextRun],
-    ) -> (SharedString, Cow<'a, [TextRun]>) {
+    ) -> Option<usize> {
         let mut width = px(0.);
-        let mut suffix_width = truncation_suffix
+        let suffix_width = truncation_suffix
             .chars()
             .map(|c| self.width_for_char(c))
             .fold(px(0.0), |a, x| a + x);
-        let mut char_indices = line.char_indices();
         let mut truncate_ix = 0;
-        for (ix, c) in char_indices {
+
+        for (ix, c) in line.char_indices() {
             if width + suffix_width < truncate_width {
                 truncate_ix = ix;
             }
@@ -152,16 +151,32 @@ impl LineWrapper {
             width += char_width;
 
             if width.floor() > truncate_width {
-                let result =
-                    SharedString::from(format!("{}{}", &line[..truncate_ix], truncation_suffix));
-                let mut runs = runs.to_vec();
-                update_runs_after_truncation(&result, truncation_suffix, &mut runs);
-
-                return (result, Cow::Owned(runs));
+                return Some(truncate_ix);
             }
         }
 
-        (line, Cow::Borrowed(runs))
+        None
+    }
+
+    /// Truncate a line of text to the given width with this wrapper's font and font size.
+    pub fn truncate_line<'a>(
+        &mut self,
+        line: SharedString,
+        truncate_width: Pixels,
+        truncation_suffix: &str,
+        runs: &'a [TextRun],
+    ) -> (SharedString, Cow<'a, [TextRun]>) {
+        if let Some(truncate_ix) =
+            self.should_truncate_line(&line, truncate_width, truncation_suffix)
+        {
+            let result =
+                SharedString::from(format!("{}{}", &line[..truncate_ix], truncation_suffix));
+            let mut runs = runs.to_vec();
+            update_runs_after_truncation(&result, truncation_suffix, &mut runs);
+            (result, Cow::Owned(runs))
+        } else {
+            (line, Cow::Borrowed(runs))
+        }
     }
 
     /// Any character in this list should be treated as a word character,