From 55721c65d44f9ba413a464de7c1a7d7456d5a6b5 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Wed, 22 Jan 2025 10:00:43 -0300 Subject: [PATCH 1/2] Fix single line edit prediction detection (#23456) #23411 introduced an "Accept" callout for single line edits, but the logic to detect them was incorrect causing it to trigger for multiline insertions, this PR fixes that. Release Notes: - N/A --- crates/editor/src/display_map/block_map.rs | 1 - crates/editor/src/editor.rs | 40 ++++++++++++-------- crates/editor/src/element.rs | 30 +++------------ crates/editor/src/inline_completion_tests.rs | 6 ++- crates/gpui/src/elements/div.rs | 1 - 5 files changed, 35 insertions(+), 43 deletions(-) diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 7de2797079..f3b5cef853 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -1757,7 +1757,6 @@ impl<'a> BlockChunks<'a> { pub struct StickyHeaderExcerpt<'a> { pub excerpt: &'a ExcerptInfo, pub next_excerpt_controls_present: bool, - // TODO az remove option pub next_buffer_row: Option, } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 923e61754c..db4a4cba01 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -486,7 +486,10 @@ enum InlineCompletionText { } enum InlineCompletion { - Edit(Vec<(Range, String)>), + Edit { + edits: Vec<(Range, String)>, + single_line: bool, + }, Move(Anchor), } @@ -4686,7 +4689,10 @@ impl Editor { selections.select_anchor_ranges([position..position]); }); } - InlineCompletion::Edit(edits) => { + InlineCompletion::Edit { + edits, + single_line: _, + } => { if let Some(provider) = self.inline_completion_provider() { provider.accept(cx); } @@ -4733,7 +4739,10 @@ impl Editor { selections.select_anchor_ranges([position..position]); }); } - InlineCompletion::Edit(edits) => { + InlineCompletion::Edit { + edits, + single_line: _, + } => { // Find an insertion that starts at the cursor position. let snapshot = self.buffer.read(cx).snapshot(cx); let cursor_offset = self.selections.newest::(cx).head(); @@ -4883,16 +4892,12 @@ impl Editor { } let first_edit_start = edits.first().unwrap().0.start; - let edit_start_row = first_edit_start - .to_point(&multibuffer) - .row - .saturating_sub(2); + let first_edit_start_point = first_edit_start.to_point(&multibuffer); + let edit_start_row = first_edit_start_point.row.saturating_sub(2); let last_edit_end = edits.last().unwrap().0.end; - let edit_end_row = cmp::min( - multibuffer.max_point().row, - last_edit_end.to_point(&multibuffer).row + 2, - ); + let last_edit_end_point = last_edit_end.to_point(&multibuffer); + let edit_end_row = cmp::min(multibuffer.max_point().row, last_edit_end_point.row + 2); let cursor_row = cursor.to_point(&multibuffer).row; @@ -4935,7 +4940,11 @@ impl Editor { } invalidation_row_range = edit_start_row..edit_end_row; - completion = InlineCompletion::Edit(edits); + + let single_line = first_edit_start_point.row == last_edit_end_point.row + && !edits.iter().any(|(_, edit)| edit.contains('\n')); + + completion = InlineCompletion::Edit { edits, single_line }; }; let invalidation_range = multibuffer @@ -4976,9 +4985,10 @@ impl Editor { let editor_snapshot = self.snapshot(cx); let text = match &self.active_inline_completion.as_ref()?.completion { - InlineCompletion::Edit(edits) => { - inline_completion_edit_text(&editor_snapshot, edits, true, cx) - } + InlineCompletion::Edit { + edits, + single_line: _, + } => inline_completion_edit_text(&editor_snapshot, edits, true, cx), InlineCompletion::Move(target) => { let target_point = target.to_point(&editor_snapshot.display_snapshot.buffer_snapshot); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index e8a7098cd4..ae02b81474 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1592,7 +1592,6 @@ impl EditorElement { &self, display_row: DisplayRow, display_snapshot: &DisplaySnapshot, - buffer_snapshot: &MultiBufferSnapshot, line_layout: &LineWithInvisibles, crease_trailer: Option<&CreaseTrailerLayout>, em_width: Pixels, @@ -1628,11 +1627,9 @@ impl EditorElement { if let Some(inline_completion) = editor.active_inline_completion.as_ref() { match &inline_completion.completion { - InlineCompletion::Edit(edits) - if single_line_edit(&edits, buffer_snapshot).is_some() => - { - padding += INLINE_ACCEPT_SUGGESTION_EM_WIDTHS - } + InlineCompletion::Edit { + single_line: true, .. + } => padding += INLINE_ACCEPT_SUGGESTION_EM_WIDTHS, _ => {} } } @@ -3389,7 +3386,7 @@ impl EditorElement { Some(element) } } - InlineCompletion::Edit(edits) => { + InlineCompletion::Edit { edits, single_line } => { if self.editor.read(cx).has_active_completions_menu() { return None; } @@ -3413,7 +3410,7 @@ impl EditorElement { return None; } - if let Some(range) = single_line_edit(&edits, &editor_snapshot.buffer_snapshot) { + if let (true, Some((range, _))) = (single_line, edits.first()) { let mut element = inline_completion_tab_indicator("Accept", None, cx); let target_display_point = range.end.to_display_point(editor_snapshot); @@ -5250,22 +5247,6 @@ fn inline_completion_tab_indicator( .into_any() } -fn single_line_edit<'a>( - edits: &'a [(Range, String)], - snapshot: &MultiBufferSnapshot, -) -> Option<&'a Range> { - let [(range, _)] = edits else { - return None; - }; - - let point_range = range.to_point(&snapshot); - if point_range.start.row == point_range.end.row { - Some(range) - } else { - None - } -} - fn all_edits_insertions_or_deletions( edits: &Vec<(Range, String)>, snapshot: &MultiBufferSnapshot, @@ -6550,7 +6531,6 @@ impl Element for EditorElement { inline_blame = self.layout_inline_blame( display_row, &snapshot.display_snapshot, - &snapshot.buffer_snapshot, line_layout, crease_trailer_layout, em_width, diff --git a/crates/editor/src/inline_completion_tests.rs b/crates/editor/src/inline_completion_tests.rs index d7c44dc95e..92944ae57a 100644 --- a/crates/editor/src/inline_completion_tests.rs +++ b/crates/editor/src/inline_completion_tests.rs @@ -286,7 +286,11 @@ fn assert_editor_active_edit_completion( .as_ref() .expect("editor has no active completion"); - if let InlineCompletion::Edit(edits) = &completion_state.completion { + if let InlineCompletion::Edit { + edits, + single_line: _, + } = &completion_state.completion + { assert(editor.buffer().read(cx).snapshot(cx), edits); } else { panic!("expected edit completion"); diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index cf33668ba3..bf0f0f9c6c 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -1211,7 +1211,6 @@ impl Element for Div { state.child_bounds = Vec::with_capacity(request_layout.child_layout_ids.len()); state.bounds = bounds; let requested = state.requested_scroll_top.take(); - // TODO az for (ix, child_layout_id) in request_layout.child_layout_ids.iter().enumerate() { let child_bounds = cx.layout_bounds(*child_layout_id); From 636df12b748d081ca0a2dd233834da6dd4d96978 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Wed, 22 Jan 2025 11:01:25 -0300 Subject: [PATCH 2/2] Only show accept callout if suggestion is all insertions/deletions (#23461) Release Notes: - N/A --- crates/editor/src/element.rs | 44 +++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index ae02b81474..504790f30d 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -3410,28 +3410,30 @@ impl EditorElement { return None; } - if let (true, Some((range, _))) = (single_line, edits.first()) { - let mut element = inline_completion_tab_indicator("Accept", None, cx); - - let target_display_point = range.end.to_display_point(editor_snapshot); - let target_line_end = DisplayPoint::new( - target_display_point.row(), - editor_snapshot.line_len(target_display_point.row()), - ); - let origin = self.editor.update(cx, |editor, cx| { - editor.display_to_pixel_point(target_line_end, editor_snapshot, cx) - })?; - - element.prepaint_as_root( - text_bounds.origin + origin + point(PADDING_X, px(0.)), - AvailableSpace::min_size(), - cx, - ); - - return Some(element); - } - if all_edits_insertions_or_deletions(edits, &editor_snapshot.buffer_snapshot) { + if *single_line { + let range = &edits.first()?.0; + let target_display_point = range.end.to_display_point(editor_snapshot); + + let target_line_end = DisplayPoint::new( + target_display_point.row(), + editor_snapshot.line_len(target_display_point.row()), + ); + let origin = self.editor.update(cx, |editor, cx| { + editor.display_to_pixel_point(target_line_end, editor_snapshot, cx) + })?; + + let mut element = inline_completion_tab_indicator("Accept", None, cx); + + element.prepaint_as_root( + text_bounds.origin + origin + point(PADDING_X, px(0.)), + AvailableSpace::min_size(), + cx, + ); + + return Some(element); + } + return None; }