Revert PR #8327 - Fix autocomplete completions being cut in half (#10084)

Max Brunsfeld created

This reverts https://github.com/zed-industries/zed/pull/8327

That PR introduced a regression where completions' syntax highlighting
would be corrupted in a non-deterministic way, such that it varied from
frame to frame:

In the screenshot below, many of the field names (e.g. `cursor`,
`depth`) are incorrectly colored as types instead of fields. The
`finished_states` field has highlighting that changes at the wrong
offset. All of these values changed from frame to frame, creating a
strange flickering effect:

<img width="599" alt="Screenshot 2024-04-01 at 5 56 36 PM"
src="https://github.com/zed-industries/zed/assets/326587/b6a48f02-f146-4f76-92e6-32fb417d86c0">

Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs      | 428 +++++----------------------------
crates/gpui/src/elements/text.rs |  24 -
2 files changed, 78 insertions(+), 374 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -64,12 +64,11 @@ use gpui::{
     AnyElement, AppContext, AsyncWindowContext, AvailableSpace, BackgroundExecutor, Bounds,
     ClipboardItem, Context, DispatchPhase, ElementId, EventEmitter, FocusHandle, FocusableView,
     FontId, FontStyle, FontWeight, HighlightStyle, Hsla, InteractiveText, KeyContext, Model,
-    MouseButton, ParentElement, Pixels, Render, SharedString, Styled, StyledText, Subscription,
-    Task, TextStyle, UnderlineStyle, UniformListScrollHandle, View, ViewContext, ViewInputHandler,
-    VisualContext, WeakView, WhiteSpace, WindowContext,
+    MouseButton, ParentElement, Pixels, Render, SharedString, StrikethroughStyle, Styled,
+    StyledText, Subscription, Task, TextStyle, UnderlineStyle, UniformListScrollHandle, View,
+    ViewContext, ViewInputHandler, VisualContext, WeakView, WhiteSpace, WindowContext,
 };
 use highlight_matching_bracket::refresh_matching_bracket_highlights;
-use hover_links::{HoverLink, HoveredLinkState, InlayHighlight};
 use hover_popover::{hide_hover, HoverState};
 use inlay_hint_cache::{InlayHintCache, InlaySplice, InvalidationStrategy};
 pub use inline_completion_provider::*;
@@ -82,6 +81,8 @@ use language::{
     CodeLabel, Completion, CursorShape, Diagnostic, Documentation, IndentKind, IndentSize,
     Language, OffsetRangeExt, Point, Selection, SelectionGoal, TransactionId,
 };
+
+use hover_links::{HoverLink, HoveredLinkState, InlayHighlight};
 use lsp::{DiagnosticSeverity, LanguageServerId};
 use mouse_context_menu::MouseContextMenu;
 use movement::TextLayoutDetails;
@@ -860,41 +861,25 @@ impl CompletionsMenu {
         let settings = EditorSettings::get_global(cx);
         let show_completion_documentation = settings.show_completion_documentation;
 
-        let font_size = style.text.font_size.to_pixels(cx.rem_size());
-        let padding_width = cx.rem_size().0 / 16. * 36.;
-
-        let max_completion_len = px(510.);
-        let widest_completion_pixels = self
+        let widest_completion_ix = self
             .matches
             .iter()
-            .map(|mat| {
+            .enumerate()
+            .max_by_key(|(_, mat)| {
                 let completions = self.completions.read();
                 let completion = &completions[mat.candidate_id];
                 let documentation = &completion.documentation;
 
-                let mut len = completion.label.text.chars().count() as f32;
-                if let Ok(text_width) = cx.text_system().layout_line(
-                    completion.label.text.as_str(),
-                    font_size,
-                    &[style.text.to_run(completion.label.text.as_str().len())],
-                ) {
-                    len = text_width.width.0;
-                }
-
-                if let Some(Documentation::SingleLine(documentation_text)) = documentation {
+                let mut len = completion.label.text.chars().count();
+                if let Some(Documentation::SingleLine(text)) = documentation {
                     if show_completion_documentation {
-                        if let Ok(documentation_width) = cx.text_system().layout_line(
-                            documentation_text.as_str(),
-                            font_size,
-                            &[style.text.to_run(documentation_text.as_str().len())],
-                        ) {
-                            len = documentation_width.width.0 + padding_width;
-                        }
+                        len += text.chars().count();
                     }
                 }
-                (len + padding_width).min(max_completion_len.0 as f32)
+
+                len
             })
-            .fold(190_f32, |a, b| a.max(b));
+            .map(|(ix, _)| ix);
 
         let completions = self.completions.clone();
         let matches = self.matches.clone();
@@ -948,7 +933,7 @@ impl CompletionsMenu {
                     .map(|(ix, mat)| {
                         let item_ix = start_ix + ix;
                         let candidate_id = mat.candidate_id;
-                        let completion = completions_guard[candidate_id].clone();
+                        let completion = &completions_guard[candidate_id];
 
                         let documentation = if show_completion_documentation {
                             &completion.documentation
@@ -956,36 +941,63 @@ impl CompletionsMenu {
                             &None
                         };
 
-                        let (_completion_width, completion_label, documentation_label) =
-                            Self::truncate_completion(
-                                &style,
-                                cx,
-                                mat,
-                                &mut completion.clone(),
-                                documentation,
-                                max_completion_len,
-                            );
-                        div()
-                            .min_w(px(widest_completion_pixels + padding_width))
-                            .max_w(max_completion_len + px(padding_width))
-                            .child(
-                                ListItem::new(mat.candidate_id)
-                                    .inset(true)
-                                    .selected(item_ix == selected_item)
-                                    .on_click(cx.listener(move |editor, _event, cx| {
-                                        cx.stop_propagation();
-                                        if let Some(task) = editor.confirm_completion(
-                                            &ConfirmCompletion {
-                                                item_ix: Some(item_ix),
-                                            },
-                                            cx,
-                                        ) {
-                                            task.detach_and_log_err(cx)
-                                        }
-                                    }))
-                                    .child(h_flex().overflow_hidden().child(completion_label))
-                                    .end_slot(documentation_label),
-                            )
+                        let highlights = gpui::combine_highlights(
+                            mat.ranges().map(|range| (range, FontWeight::BOLD.into())),
+                            styled_runs_for_code_label(&completion.label, &style.syntax).map(
+                                |(range, mut highlight)| {
+                                    // Ignore font weight for syntax highlighting, as we'll use it
+                                    // for fuzzy matches.
+                                    highlight.font_weight = None;
+
+                                    if completion.lsp_completion.deprecated.unwrap_or(false) {
+                                        highlight.strikethrough = Some(StrikethroughStyle {
+                                            thickness: 1.0.into(),
+                                            ..Default::default()
+                                        });
+                                        highlight.color = Some(cx.theme().colors().text_muted);
+                                    }
+
+                                    (range, highlight)
+                                },
+                            ),
+                        );
+                        let completion_label = StyledText::new(completion.label.text.clone())
+                            .with_highlights(&style.text, highlights);
+                        let documentation_label =
+                            if let Some(Documentation::SingleLine(text)) = documentation {
+                                if text.trim().is_empty() {
+                                    None
+                                } else {
+                                    Some(
+                                        h_flex().ml_4().child(
+                                            Label::new(text.clone())
+                                                .size(LabelSize::Small)
+                                                .color(Color::Muted),
+                                        ),
+                                    )
+                                }
+                            } else {
+                                None
+                            };
+
+                        div().min_w(px(220.)).max_w(px(540.)).child(
+                            ListItem::new(mat.candidate_id)
+                                .inset(true)
+                                .selected(item_ix == selected_item)
+                                .on_click(cx.listener(move |editor, _event, cx| {
+                                    cx.stop_propagation();
+                                    if let Some(task) = editor.confirm_completion(
+                                        &ConfirmCompletion {
+                                            item_ix: Some(item_ix),
+                                        },
+                                        cx,
+                                    ) {
+                                        task.detach_and_log_err(cx)
+                                    }
+                                }))
+                                .child(h_flex().overflow_hidden().child(completion_label))
+                                .end_slot::<Div>(documentation_label),
+                        )
                     })
                     .collect()
             },
@@ -993,7 +1005,7 @@ impl CompletionsMenu {
         .occlude()
         .max_h(max_height)
         .track_scroll(self.scroll_handle.clone())
-        .min_w(px(widest_completion_pixels + padding_width));
+        .with_width_from_item(widest_completion_ix);
 
         Popover::new()
             .child(list)
@@ -1003,294 +1015,6 @@ impl CompletionsMenu {
             .into_any_element()
     }
 
-    fn truncate_completion(
-        style: &EditorStyle,
-        cx: &mut ViewContext<Editor>,
-        mat: &StringMatch,
-        completion: &mut Completion,
-        documentation: &Option<Documentation>,
-        max_completion_len: Pixels,
-    ) -> (Pixels, StyledText, StyledText) {
-        let highlights = gpui::combine_highlights(
-            mat.ranges().map(|range| (range, FontWeight::BOLD.into())),
-            styled_runs_for_code_label(&completion.label, &style.syntax).map(
-                |(range, mut highlight)| {
-                    // Ignore font weight for syntax highlighting, as we'll use it
-                    // for fuzzy matches.
-                    highlight.font_weight = None;
-                    (range, highlight)
-                },
-            ),
-        );
-
-        let mut inline_documentation_exists = false;
-
-        let mut documentation_text = if let Some(Documentation::SingleLine(text)) = documentation {
-            inline_documentation_exists = true;
-            text
-        } else {
-            ""
-        }
-        .to_owned();
-        let documentation_style = style.clone().text;
-
-        let documentation_highlight_style = HighlightStyle {
-            color: Some(Color::Muted.color(cx)),
-            ..Default::default()
-        };
-
-        let documentation_highlights = vec![(
-            Range {
-                start: 0,
-                end: documentation_text.len(),
-            },
-            documentation_highlight_style,
-        )];
-        let documentation_label = StyledText::new(documentation_text.clone())
-            .with_highlights(&documentation_style, documentation_highlights);
-
-        let mut completion_label =
-            StyledText::new(completion.label.text.clone()).with_highlights(&style.text, highlights);
-
-        let font_size = style.text.font_size.to_pixels(cx.rem_size());
-
-        let mut variable_name_end = completion.label.filter_range.end;
-        let mut completion_label_text = completion.label.text.clone();
-        let mut variable_name_length_truncated: i32 = 0;
-
-        let mut actual_width: Pixels = px(0.);
-        if let Ok(ellipsis_width) =
-            cx.text_system()
-                .layout_line("…", font_size, &[style.text.to_run("…".len())])
-        {
-            if let Ok(completion_layout_line) = completion_label.layout_line(font_size, cx) {
-                if let Ok(documentation_layout_line) =
-                    documentation_label.layout_line(font_size, cx)
-                {
-                    if inline_documentation_exists {
-                        if completion_layout_line.width + documentation_layout_line.width
-                            > max_completion_len
-                        {
-                            actual_width = max_completion_len;
-                            let width_of_variable_name = completion_layout_line
-                                .x_for_index(completion.label.filter_range.end);
-                            let width_of_documentation =
-                                documentation_layout_line.x_for_index(documentation_text.len());
-
-                            let max_width_of_variable_name =
-                                if width_of_documentation < max_completion_len * 0.2 {
-                                    max_completion_len - width_of_documentation
-                                } else {
-                                    max_completion_len * 0.8
-                                };
-
-                            if width_of_variable_name < max_width_of_variable_name {
-                                // Only truncate the second part.
-                                if let Some(documentation_truncation_index) =
-                                    documentation_layout_line.index_for_x(
-                                        (max_completion_len * 0.65).min(
-                                            max_completion_len
-                                                - ellipsis_width.width
-                                                - width_of_variable_name,
-                                        ),
-                                    )
-                                {
-                                    variable_name_end = documentation_truncation_index + 2;
-                                    documentation_text = documentation_text
-                                        .chars()
-                                        .take(documentation_truncation_index)
-                                        .collect::<String>()
-                                        + "…";
-                                }
-                            } else {
-                                // Truncate the first part (and optionally the second part).
-                                if let Some(variable_name_truncation_index) = completion_layout_line
-                                    .index_for_x(max_width_of_variable_name - ellipsis_width.width)
-                                {
-                                    variable_name_end = variable_name_truncation_index + 2;
-                                    variable_name_length_truncated =
-                                        completion.label.filter_range.end as i32
-                                            - variable_name_end as i32
-                                            - 1;
-                                    completion_label_text = completion
-                                        .label
-                                        .text
-                                        .chars()
-                                        .take(variable_name_truncation_index)
-                                        .collect::<String>()
-                                        + "…";
-                                    completion_label =
-                                        completion_label.with_text(completion_label_text.clone());
-                                    if let Ok(new_completion_layout_line) =
-                                        completion_label.layout_line(font_size, cx)
-                                    {
-                                        let combined_width = new_completion_layout_line
-                                            .x_for_index(completion_label_text.len())
-                                            + width_of_documentation;
-                                        if combined_width > max_completion_len {
-                                            if let Some(documentation_truncation_index) =
-                                                documentation_layout_line.index_for_x(
-                                                    (max_completion_len * 0.65).min(
-                                                        max_completion_len
-                                                            - ellipsis_width.width
-                                                            - max_width_of_variable_name,
-                                                    ),
-                                                )
-                                            {
-                                                documentation_text = documentation_text
-                                                    .chars()
-                                                    .take(documentation_truncation_index)
-                                                    .collect::<String>()
-                                                    + "…";
-                                            }
-                                        }
-                                    }
-                                }
-                            }
-                        } else {
-                            actual_width =
-                                completion_layout_line.width + documentation_layout_line.width;
-                        }
-                    } else {
-                        if completion_layout_line.width > max_completion_len {
-                            actual_width = max_completion_len;
-                            let width_of_variable_name = completion_layout_line
-                                .x_for_index(completion.label.filter_range.end);
-                            let width_of_type_annotation =
-                                completion_layout_line.width - width_of_variable_name;
-
-                            let max_width_of_variable_name =
-                                if width_of_type_annotation < max_completion_len * 0.2 {
-                                    max_completion_len - width_of_type_annotation
-                                } else {
-                                    max_completion_len * 0.8
-                                };
-
-                            if width_of_variable_name < max_width_of_variable_name {
-                                // Only truncate the second part.
-
-                                if let Some(type_annotation_truncation_index) =
-                                    completion_layout_line
-                                        .index_for_x(max_completion_len - ellipsis_width.width)
-                                {
-                                    completion_label_text = completion
-                                        .label
-                                        .text
-                                        .chars()
-                                        .take(type_annotation_truncation_index)
-                                        .collect::<String>()
-                                        + "…";
-                                }
-                            } else {
-                                // Truncate the first part (and optionally the second part).
-                                if let Some(variable_name_truncation_index) = completion_layout_line
-                                    .index_for_x(max_width_of_variable_name - ellipsis_width.width)
-                                {
-                                    variable_name_end = variable_name_truncation_index + 2;
-
-                                    variable_name_length_truncated =
-                                        completion.label.filter_range.end as i32
-                                            - variable_name_end as i32
-                                            - 1;
-
-                                    let second_part_text = &completion.label.text.as_str()
-                                        [completion.label.filter_range.end..];
-
-                                    completion_label_text = completion
-                                        .label
-                                        .text
-                                        .chars()
-                                        .take(variable_name_truncation_index)
-                                        .collect::<String>()
-                                        + "…"
-                                        + second_part_text;
-                                    completion_label =
-                                        completion_label.with_text(completion_label_text.clone());
-
-                                    if let Ok(layout_line) =
-                                        completion_label.layout_line(font_size, cx)
-                                    {
-                                        let combined_width =
-                                            layout_line.x_for_index(completion_label_text.len());
-                                        if combined_width > max_completion_len {
-                                            if let Some(type_annotation_truncation_index) =
-                                                layout_line.index_for_x(
-                                                    max_completion_len - ellipsis_width.width,
-                                                )
-                                            {
-                                                completion_label_text = completion_label_text
-                                                    .chars()
-                                                    .take(type_annotation_truncation_index - 2)
-                                                    .collect::<String>()
-                                                    + "…";
-                                            }
-                                        }
-                                    }
-                                }
-                            }
-                        } else {
-                            actual_width = completion_layout_line.width;
-                        }
-                    }
-                }
-            }
-        };
-
-        // Recompute syntax highlighting.
-        completion.label.text = completion_label_text.clone();
-        if inline_documentation_exists {
-            completion.label.filter_range.end = completion_label_text.len();
-            for run in completion.label.runs.iter_mut() {
-                if run.0.start == 0 {
-                    run.0.start = 0;
-                    run.0.end = completion_label_text.len();
-                }
-            }
-        } else {
-            completion.label.filter_range.end = variable_name_end;
-            for run in completion.label.runs.iter_mut() {
-                if run.0.start == 0 {
-                    run.0.start = 0;
-                    run.0.end = variable_name_end;
-                } else {
-                    run.0.start = (run.0.start as i32 - variable_name_length_truncated) as usize;
-                    run.0.end = (run.0.end as i32 - variable_name_length_truncated) as usize;
-                }
-            }
-        }
-        let highlights = gpui::combine_highlights(
-            mat.ranges().map(|range| (range, FontWeight::NORMAL.into())),
-            styled_runs_for_code_label(&completion.label, &style.syntax).map(
-                |(range, mut highlight)| {
-                    // Ignore font weight for syntax highlighting, as we'll use it
-                    // for fuzzy matches.
-                    highlight.font_weight = None;
-                    (range, highlight)
-                },
-            ),
-        );
-
-        let completion_label =
-            StyledText::new(completion_label_text).with_highlights(&style.text, highlights);
-
-        let documentation_style = style.clone().text;
-        let documentation_highlight_style = HighlightStyle {
-            color: Some(Color::Muted.color(cx)),
-            ..Default::default()
-        };
-        let documentation_highlights = vec![(
-            Range {
-                start: 0,
-                end: documentation_text.len(),
-            },
-            documentation_highlight_style,
-        )];
-
-        let documentation_label = StyledText::new(documentation_text)
-            .with_highlights(&documentation_style, documentation_highlights);
-        (actual_width, completion_label, documentation_label)
-    }
-
     pub async fn filter(&mut self, query: Option<&str>, executor: BackgroundExecutor) {
         let mut matches = if let Some(query) = query {
             fuzzy::match_strings(
@@ -3578,7 +3302,7 @@ impl Editor {
             .text_anchor_for_position(position, cx)?;
 
         // OnTypeFormatting returns a list of edits, no need to pass them between Zed instances,
-        // hence we do LSP request & edit on host side only — add formats to host's history.
+        // hence we do LSP request & edit on host side only — add formats to host's history.
         let push_to_lsp_host_history = true;
         // If this is not the host, append its history with new edits.
         let push_to_client_history = project.read(cx).is_remote();
@@ -9663,9 +9387,7 @@ impl Editor {
                     }
                 }
 
-                let Some(project) = &self.project else {
-                    return;
-                };
+                let Some(project) = &self.project else { return };
                 let telemetry = project.read(cx).client().telemetry().clone();
                 telemetry.log_edit_event("editor");
             }

crates/gpui/src/elements/text.rs 🔗

@@ -1,8 +1,8 @@
 use crate::{
     ActiveTooltip, AnyTooltip, AnyView, Bounds, DispatchPhase, Element, ElementContext, ElementId,
-    HighlightStyle, Hitbox, IntoElement, LayoutId, LineLayout, MouseDownEvent, MouseMoveEvent,
-    MouseUpEvent, Pixels, Point, SharedString, Size, TextRun, TextStyle, WhiteSpace, WindowContext,
-    WrappedLine, TOOLTIP_DELAY,
+    HighlightStyle, Hitbox, IntoElement, LayoutId, MouseDownEvent, MouseMoveEvent, MouseUpEvent,
+    Pixels, Point, SharedString, Size, TextRun, TextStyle, WhiteSpace, WindowContext, WrappedLine,
+    TOOLTIP_DELAY,
 };
 use anyhow::anyhow;
 use parking_lot::{Mutex, MutexGuard};
@@ -118,12 +118,6 @@ impl StyledText {
         }
     }
 
-    /// Sets the text for this [`StyledText`].
-    pub fn with_text(mut self, text: impl Into<SharedString>) -> Self {
-        self.text = text.into();
-        self
-    }
-
     /// Set the styling attributes for the given text, as well as
     /// as any ranges of text that have had their style customized.
     pub fn with_highlights(
@@ -151,18 +145,6 @@ impl StyledText {
         self.runs = Some(runs);
         self
     }
-
-    /// Lays out this line of [`StyledText`] at the specified font size.
-    pub fn layout_line(
-        &self,
-        font_size: Pixels,
-        cx: &WindowContext,
-    ) -> anyhow::Result<Arc<LineLayout>> {
-        let Some(runs) = self.runs.as_ref() else {
-            return Err(anyhow!("must pass runs"));
-        };
-        cx.text_system().layout_line(&self.text, font_size, runs)
-    }
 }
 
 impl Element for StyledText {