From 6e49a2460eb97ba18e68a062a62512b8237ff725 Mon Sep 17 00:00:00 2001 From: Ephram Date: Mon, 1 Apr 2024 13:20:51 -0600 Subject: [PATCH] Fix autocomplete completions being cut in half (#8327) Release Notes: - Fixed LSP completions being cut in half ([#8126](https://github.com/zed-industries/zed/issues/8126)). Previously, autocomplete suggestions were covered by autocomplete documentation, which only appeared after a short delay. Now, when an autocomplete suggestion is too long to fit, the name is truncated with ellipses like how VSCode does it: ![image](https://github.com/zed-industries/zed/assets/50590465/bf3c6271-7d7a-44b1-ab76-647df5620fcd) Additionally `completion_documentation_secondary_query_debounce`'s default was changed from 300ms to 0ms, which makes the editor feel significantly faster (in my opinion). Before: https://github.com/zed-industries/zed/assets/50590465/6443670b-fe25-4428-9a39-54405d9a7cec After: https://github.com/zed-industries/zed/assets/50590465/72572487-3eb4-4a96-a2f9-608e563a1f05 --------- Co-authored-by: Marshall Bowers Co-authored-by: Conrad Co-authored-by: Conrad Irwin --- crates/editor/src/editor.rs | 431 +++++++++++++++++++++++++------ crates/gpui/src/elements/text.rs | 24 +- 2 files changed, 376 insertions(+), 79 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 4802c4e91b..e05b20c37f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -64,11 +64,12 @@ 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, StrikethroughStyle, Styled, - StyledText, Subscription, Task, TextStyle, UnderlineStyle, UniformListScrollHandle, View, - ViewContext, ViewInputHandler, VisualContext, WeakView, WhiteSpace, WindowContext, + MouseButton, ParentElement, Pixels, Render, SharedString, 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::*; @@ -81,8 +82,6 @@ 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; @@ -861,25 +860,41 @@ impl CompletionsMenu { let settings = EditorSettings::get_global(cx); let show_completion_documentation = settings.show_completion_documentation; - let widest_completion_ix = self + 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 .matches .iter() - .enumerate() - .max_by_key(|(_, mat)| { + .map(|mat| { let completions = self.completions.read(); let completion = &completions[mat.candidate_id]; let documentation = &completion.documentation; - let mut len = completion.label.text.chars().count(); - if let Some(Documentation::SingleLine(text)) = documentation { - if show_completion_documentation { - len += text.chars().count(); - } + 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; } - len + if let Some(Documentation::SingleLine(documentation_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 + padding_width).min(max_completion_len.0 as f32) }) - .map(|(ix, _)| ix); + .fold(190_f32, |a, b| a.max(b)); let completions = self.completions.clone(); let matches = self.matches.clone(); @@ -933,7 +948,7 @@ impl CompletionsMenu { .map(|(ix, mat)| { let item_ix = start_ix + ix; let candidate_id = mat.candidate_id; - let completion = &completions_guard[candidate_id]; + let completion = completions_guard[candidate_id].clone(); let documentation = if show_completion_documentation { &completion.documentation @@ -941,63 +956,36 @@ impl CompletionsMenu { &None }; - 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::
(documentation_label), - ) + 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), + ) }) .collect() }, @@ -1005,7 +993,7 @@ impl CompletionsMenu { .occlude() .max_h(max_height) .track_scroll(self.scroll_handle.clone()) - .with_width_from_item(widest_completion_ix); + .min_w(px(widest_completion_pixels + padding_width)); Popover::new() .child(list) @@ -1015,6 +1003,295 @@ impl CompletionsMenu { .into_any_element() } + fn truncate_completion( + style: &EditorStyle, + cx: &mut ViewContext, + mat: &StringMatch, + completion: &mut Completion, + documentation: &Option, + 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 { + // truncate second part only + 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::() + + "…"; + } + } else { + // truncate first part (and optionally second part too) + 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::() + + "…"; + 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::() + + "…"; + } + } + } + } + } + } 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 { + // truncate second part only + + if let Some(type_annotation_truncation_index) = + completion_layout_line + .index_for_x(max_completion_len - ellipsis_width.width) + { + // variable_name_end = type_annotation_truncation_index + 2; + completion_label_text = completion + .label + .text + .chars() + .take(type_annotation_truncation_index) + .collect::() + + "…"; + } + } else { + // truncate first part (and optionally second part too) + 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::() + + "…" + + 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::() + + "…"; + } + } + } + } + } + } 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( @@ -3302,7 +3579,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(); @@ -9372,7 +9649,9 @@ 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"); } diff --git a/crates/gpui/src/elements/text.rs b/crates/gpui/src/elements/text.rs index 735e8dc858..8ca5c39026 100644 --- a/crates/gpui/src/elements/text.rs +++ b/crates/gpui/src/elements/text.rs @@ -1,8 +1,8 @@ use crate::{ ActiveTooltip, AnyTooltip, AnyView, Bounds, DispatchPhase, Element, ElementContext, ElementId, - HighlightStyle, Hitbox, IntoElement, LayoutId, MouseDownEvent, MouseMoveEvent, MouseUpEvent, - Pixels, Point, SharedString, Size, TextRun, TextStyle, WhiteSpace, WindowContext, WrappedLine, - TOOLTIP_DELAY, + HighlightStyle, Hitbox, IntoElement, LayoutId, LineLayout, MouseDownEvent, MouseMoveEvent, + MouseUpEvent, Pixels, Point, SharedString, Size, TextRun, TextStyle, WhiteSpace, WindowContext, + WrappedLine, TOOLTIP_DELAY, }; use anyhow::anyhow; use parking_lot::{Mutex, MutexGuard}; @@ -118,6 +118,12 @@ impl StyledText { } } + /// Lets you modify the text + pub fn with_text(mut self, text: impl Into) -> 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( @@ -145,6 +151,18 @@ impl StyledText { self.runs = Some(runs); self } + + /// line_layout returns metadata about how the line will be rendered + pub fn layout_line( + &self, + font_size: Pixels, + cx: &WindowContext, + ) -> anyhow::Result> { + 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 {