edit predictions: Polish up ⌥ preview experience (#24380)

- Do not accept with just `tab` in `when_holding_modifer` mode
- Fix fake cursor for jumps when destination row is outside viewport
- Use current preview state for deciding whether to show modifiers in
popovers
- Stay in preview state if ⌥ isn't released after accepting a jump

Release Notes:

- N/A

assets/keymaps/default-linux.json

@@ -510,7 +510,7 @@
     }
   },
   {
-    "context": "Editor && inline_completion && !showing_completions",
+    "context": "Editor && inline_completion && !inline_completion_requires_modifier",
     "use_key_equivalents": true,
     "bindings": {
       "tab": "editor::AcceptInlineCompletion"

assets/keymaps/default-macos.json

@@ -587,7 +587,7 @@
     }
   },
   {
-    "context": "Editor && inline_completion && !showing_completions",
+    "context": "Editor && inline_completion && !inline_completion_requires_modifier",
     "use_key_equivalents": true,
     "bindings": {
       "tab": "editor::AcceptInlineCompletion"

crates/editor/src/editor.rs

@@ -82,8 +82,8 @@ use gpui::{
     Entity, EntityInputHandler, EventEmitter, FocusHandle, FocusOutEvent, Focusable, FontId,
     FontWeight, Global, HighlightStyle, Hsla, InteractiveText, KeyContext, Modifiers, MouseButton,
     MouseDownEvent, PaintQuad, ParentElement, Pixels, Render, SharedString, Size, Styled,
-    StyledText, Subscription, Task, TextStyle, TextStyleRefinement, UTF16Selection, UnderlineStyle,
+    StyledText, Subscription, Task, TextRun, TextStyle, TextStyleRefinement, UTF16Selection,
-    UniformListScrollHandle, WeakEntity, WeakFocusHandle, Window,
+    UnderlineStyle, UniformListScrollHandle, WeakEntity, WeakFocusHandle, Window,
 };
 use highlight_matching_bracket::refresh_matching_bracket_highlights;
 use hover_popover::{hide_hover, HoverState};
@@ -468,7 +468,7 @@ pub fn make_suggestion_styles(cx: &mut App) -> InlineCompletionStyles {
 type CompletionId = usize;
 
 pub(crate) enum EditDisplayMode {
-    TabAccept(bool),
+    TabAccept,
     DiffPopover,
     Inline,
 }
@@ -493,15 +493,6 @@ struct InlineCompletionState {
     invalidation_range: Range<Anchor>,
 }
 
-impl InlineCompletionState {
-    pub fn is_move(&self) -> bool {
-        match &self.completion {
-            InlineCompletion::Move { .. } => true,
-            _ => false,
-        }
-    }
-}
-
 enum InlineCompletionHighlight {}
 
 pub enum MenuInlineCompletionsPolicy {
@@ -1499,10 +1490,14 @@ impl Editor {
         if self.pending_rename.is_some() {
             key_context.add("renaming");
         }
+
+        let mut showing_completions = false;
+
         match self.context_menu.borrow().as_ref() {
             Some(CodeContextMenu::Completions(_)) => {
                 key_context.add("menu");
                 key_context.add("showing_completions");
+                showing_completions = true;
             }
             Some(CodeContextMenu::CodeActions(_)) => {
                 key_context.add("menu");
@@ -1532,6 +1527,10 @@ impl Editor {
         if self.has_active_inline_completion() {
             key_context.add("copilot_suggestion");
             key_context.add("inline_completion");
+
+            if showing_completions || self.inline_completion_requires_modifier(cx) {
+                key_context.add("inline_completion_requires_modifier");
+            }
         }
 
         if self.selection_mark_mode {
@@ -4664,7 +4663,7 @@ impl Editor {
         }
     }
 
-    fn inline_completion_preview_mode(&self, cx: &App) -> language::InlineCompletionPreviewMode {
+    fn inline_completion_requires_modifier(&self, cx: &App) -> bool {
         let cursor = self.selections.newest_anchor().head();
 
         self.buffer
@@ -4672,8 +4671,9 @@ impl Editor {
             .text_anchor_for_position(cursor, cx)
             .map(|(buffer, _)| {
                 all_language_settings(buffer.read(cx).file(), cx).inline_completions_preview_mode()
+                    == InlineCompletionPreviewMode::WhenHoldingModifier
             })
-            .unwrap_or_default()
+            .unwrap_or(false)
     }
 
     fn should_show_inline_completions_in_buffer(
@@ -5042,9 +5042,7 @@ impl Editor {
             return true;
         }
 
-        has_completion
+        has_completion && self.inline_completion_requires_modifier(cx)
-            && self.inline_completion_preview_mode(cx)
-                == InlineCompletionPreviewMode::WhenHoldingModifier
     }
 
     fn update_inline_completion_preview(
@@ -5053,23 +5051,13 @@ impl Editor {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        // Moves jump directly without a preview step
-        if self
-            .active_inline_completion
-            .as_ref()
-            .map_or(true, |c| c.is_move())
-        {
-            self.previewing_inline_completion = false;
-            cx.notify();
-            return;
-        }
-
         if !self.show_inline_completions_in_menu(cx) {
             return;
         }
 
         self.previewing_inline_completion = modifiers.alt;
         self.update_visible_inline_completion(window, cx);
+        cx.notify();
     }
 
     fn update_visible_inline_completion(
@@ -5198,7 +5186,7 @@ impl Editor {
 
             let display_mode = if all_edits_insertions_or_deletions(&edits, &multibuffer) {
                 if provider.show_tab_accept_marker() {
-                    EditDisplayMode::TabAccept(self.previewing_inline_completion)
+                    EditDisplayMode::TabAccept
                 } else {
                     EditDisplayMode::Inline
                 }
@@ -5494,8 +5482,6 @@ impl Editor {
         min_width: Pixels,
         max_width: Pixels,
         cursor_point: Point,
-        start_row: DisplayRow,
-        line_layouts: &[LineWithInvisibles],
         style: &EditorStyle,
         accept_keystroke: &gpui::Keystroke,
         window: &Window,
@@ -5556,9 +5542,8 @@ impl Editor {
             Some(completion) => self.render_edit_prediction_cursor_popover_preview(
                 completion,
                 cursor_point,
-                start_row,
-                line_layouts,
                 style,
+                window,
                 cx,
             )?,
 
@@ -5566,9 +5551,8 @@ impl Editor {
                 Some(stale_completion) => self.render_edit_prediction_cursor_popover_preview(
                     stale_completion,
                     cursor_point,
-                    start_row,
-                    line_layouts,
                     style,
+                    window,
                     cx,
                 )?,
 
@@ -5599,19 +5583,6 @@ impl Editor {
 
         let has_completion = self.active_inline_completion.is_some();
 
-        let is_move = self
-            .active_inline_completion
-            .as_ref()
-            .map_or(false, |c| c.is_move());
-
-        let modifier_color = if !has_completion {
-            Color::Muted
-        } else if window.modifiers() == accept_keystroke.modifiers {
-            Color::Accent
-        } else {
-            Color::Default
-        };
-
         Some(
             h_flex()
                 .h(self.edit_prediction_cursor_popover_height())
@@ -5631,18 +5602,15 @@ impl Editor {
                             ui::render_modifiers(
                                 &accept_keystroke.modifiers,
                                 PlatformStyle::platform(),
-                                Some(modifier_color),
+                                Some(if !has_completion {
-                                !is_move,
+                                    Color::Muted
+                                } else {
+                                    Color::Default
+                                }),
+                                true,
                             ),
                         ))
-                        .child(if is_move {
+                        .child(Label::new("Preview").into_any_element())
-                            div()
-                                .child(ui::Key::new(&accept_keystroke.key, None))
-                                .font(buffer_font.clone())
-                                .into_any()
-                        } else {
-                            Label::new("Preview").into_any_element()
-                        })
                         .opacity(if has_completion { 1.0 } else { 0.4 }),
                 )
                 .into_any(),
@@ -5653,9 +5621,8 @@ impl Editor {
         &self,
         completion: &InlineCompletionState,
         cursor_point: Point,
-        start_row: DisplayRow,
-        line_layouts: &[LineWithInvisibles],
         style: &EditorStyle,
+        window: &Window,
         cx: &mut Context<Editor>,
     ) -> Option<Div> {
         use text::ToPoint as _;
@@ -5732,6 +5699,7 @@ impl Editor {
 
                 let preview = h_flex()
                     .gap_1()
+                    .min_w_16()
                     .child(styled_text)
                     .when(len_total > first_line_len, |parent| parent.child("…"));
 
@@ -5764,18 +5732,46 @@ impl Editor {
                     None,
                     &style.syntax,
                 );
+                let base = h_flex().gap_3().flex_1().child(render_relative_row_jump(
+                    "Jump ",
+                    cursor_point.row,
+                    target.text_anchor.to_point(&snapshot).row,
+                ));
+
+                if highlighted_text.text.is_empty() {
+                    return Some(base);
+                }
+
                 let cursor_color = self.current_user_player_color(cx).cursor;
 
                 let start_point = range_around_target.start.to_point(&snapshot);
                 let end_point = range_around_target.end.to_point(&snapshot);
                 let target_point = target.text_anchor.to_point(&snapshot);
 
-                let cursor_relative_position = line_layouts
+                let styled_text = highlighted_text.to_styled_text(&style.text);
-                    .get(start_point.row.saturating_sub(start_row.0) as usize)
+                let text_len = highlighted_text.text.len();
+
+                let cursor_relative_position = window
+                    .text_system()
+                    .layout_line(
+                        highlighted_text.text,
+                        style.text.font_size.to_pixels(window.rem_size()),
+                        // We don't need to include highlights
+                        // because we are only using this for the cursor position
+                        &[TextRun {
+                            len: text_len,
+                            font: style.text.font(),
+                            color: style.text.color,
+                            background_color: None,
+                            underline: None,
+                            strikethrough: None,
+                        }],
+                    )
+                    .log_err()
                     .map(|line| {
-                        let start_column_x = line.x_for_index(start_point.column as usize);
+                        line.x_for_index(
-                        let target_column_x = line.x_for_index(target_point.column as usize);
+                            target_point.column.saturating_sub(start_point.column) as usize
-                        target_column_x - start_column_x
+                        )
                     });
 
                 let fade_before = start_point.column > 0;
@@ -5783,56 +5779,40 @@ impl Editor {
 
                 let background = cx.theme().colors().elevated_surface_background;
 
-                Some(
+                let preview = h_flex()
-                    h_flex()
+                    .relative()
-                        .gap_3()
+                    .child(styled_text)
-                        .flex_1()
+                    .when(fade_before, |parent| {
-                        .child(render_relative_row_jump(
+                        parent.child(div().absolute().top_0().left_0().w_4().h_full().bg(
-                            "Jump ",
+                            linear_gradient(
-                            cursor_point.row,
+                                90.,
-                            target.text_anchor.to_point(&snapshot).row,
+                                linear_color_stop(background, 0.),
+                                linear_color_stop(background.opacity(0.), 1.),
+                            ),
                         ))
-                        .when(!highlighted_text.text.is_empty(), |parent| {
+                    })
-                            parent.child(
+                    .when(fade_after, |parent| {
-                                h_flex()
+                        parent.child(div().absolute().top_0().right_0().w_4().h_full().bg(
-                                    .relative()
+                            linear_gradient(
-                                    .child(highlighted_text.to_styled_text(&style.text))
+                                -90.,
-                                    .when(fade_before, |parent| {
+                                linear_color_stop(background, 0.),
-                                        parent.child(
+                                linear_color_stop(background.opacity(0.), 1.),
-                                            div().absolute().top_0().left_0().w_4().h_full().bg(
+                            ),
-                                                linear_gradient(
+                        ))
-                                                    90.,
+                    })
-                                                    linear_color_stop(background, 0.),
+                    .when_some(cursor_relative_position, |parent, position| {
-                                                    linear_color_stop(background.opacity(0.), 1.),
+                        parent.child(
-                                                ),
+                            div()
-                                            ),
+                                .w(px(2.))
-                                        )
+                                .h_full()
-                                    })
+                                .bg(cursor_color)
-                                    .when(fade_after, |parent| {
+                                .absolute()
-                                        parent.child(
+                                .top_0()
-                                            div().absolute().top_0().right_0().w_4().h_full().bg(
+                                .left(position),
-                                                linear_gradient(
+                        )
-                                                    -90.,
+                    });
-                                                    linear_color_stop(background, 0.),
+
-                                                    linear_color_stop(background.opacity(0.), 1.),
+                Some(base.child(preview))
-                                                ),
-                                            ),
-                                        )
-                                    })
-                                    .when_some(cursor_relative_position, |parent, position| {
-                                        parent.child(
-                                            div()
-                                                .w(px(2.))
-                                                .h_full()
-                                                .bg(cursor_color)
-                                                .absolute()
-                                                .top_0()
-                                                .left(position),
-                                        )
-                                    }),
-                            )
-                        }),
-                )
             }
         }
     }

crates/editor/src/element.rs

@@ -1653,7 +1653,7 @@ impl EditorElement {
             if let Some(inline_completion) = editor.active_inline_completion.as_ref() {
                 match &inline_completion.completion {
                     InlineCompletion::Edit {
-                        display_mode: EditDisplayMode::TabAccept(_),
+                        display_mode: EditDisplayMode::TabAccept,
                         ..
                     } => padding += INLINE_ACCEPT_SUGGESTION_EM_WIDTHS,
                     _ => {}
@@ -3238,8 +3238,6 @@ impl EditorElement {
                             min_width,
                             max_width,
                             cursor_point,
-                            start_row,
-                            &line_layouts,
                             style,
                             accept_keystroke.as_ref()?,
                             window,
@@ -3714,8 +3712,7 @@ impl EditorElement {
                 }
 
                 match display_mode {
-                    EditDisplayMode::TabAccept(previewing) => {
+                    EditDisplayMode::TabAccept => {
-                        let previewing = *previewing;
                         let range = &edits.first()?.0;
                         let target_display_point = range.end.to_display_point(editor_snapshot);
 
@@ -3723,14 +3720,22 @@ impl EditorElement {
                             target_display_point.row(),
                             editor_snapshot.line_len(target_display_point.row()),
                         );
-                        let origin = self.editor.update(cx, |editor, _cx| {
+                        let (previewing_inline_completion, origin) =
-                            editor.display_to_pixel_point(target_line_end, editor_snapshot, window)
+                            self.editor.update(cx, |editor, _cx| {
-                        })?;
+                                Some((
+                                    editor.previewing_inline_completion,
+                                    editor.display_to_pixel_point(
+                                        target_line_end,
+                                        editor_snapshot,
+                                        window,
+                                    )?,
+                                ))
+                            })?;
 
                         let mut element = inline_completion_accept_indicator(
                             "Accept",
                             None,
-                            previewing,
+                            previewing_inline_completion,
                             self.editor.focus_handle(cx),
                             window,
                             cx,