From 1aa1b2bedea86524de2e45136c5caa1e8d84d538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Thu, 17 Apr 2025 15:37:38 -0300 Subject: [PATCH] Fix multiline completions when surroundings don't match completion text (#28995) Follow up to the scenarios I overlooked in https://github.com/zed-industries/zed/pull/28586. Release Notes: - N/A --- crates/editor/src/editor.rs | 26 +++---- crates/editor/src/editor_tests.rs | 113 ++++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 23 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 55f9ad2620..77560c484b 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -4816,19 +4816,18 @@ impl Editor { let suffix = &old_text[lookbehind.min(old_text.len())..]; let selections = self.selections.all::(cx); - let mut edits = Vec::new(); + let mut ranges = Vec::new(); let mut linked_edits = HashMap::<_, Vec<_>>::default(); for selection in &selections { - let edit = if selection.id == newest_anchor.id { - (replace_range_multibuffer.clone(), new_text.as_str()) + let range = if selection.id == newest_anchor.id { + replace_range_multibuffer.clone() } else { let mut range = selection.range(); - let mut text = new_text.as_str(); // if prefix is present, don't duplicate it if snapshot.contains_str_at(range.start.saturating_sub(lookbehind), prefix) { - text = &new_text[lookbehind.min(new_text.len())..]; + range.start = range.start.saturating_sub(lookbehind); // if suffix is also present, mimic the newest cursor and replace it if selection.id != newest_anchor.id @@ -4837,10 +4836,10 @@ impl Editor { range.end += lookahead; } } - (range, text) + range }; - edits.push(edit); + ranges.push(range); if !self.linked_edit_ranges.is_empty() { let start_anchor = snapshot.anchor_before(selection.head()); @@ -4866,19 +4865,14 @@ impl Editor { self.transact(window, cx, |this, window, cx| { if let Some(mut snippet) = snippet { snippet.text = new_text.to_string(); - let ranges = edits - .iter() - .map(|(range, _)| range.clone()) - .collect::>(); this.insert_snippet(&ranges, snippet, window, cx).log_err(); } else { this.buffer.update(cx, |buffer, cx| { - let auto_indent = if completion.insert_text_mode == Some(InsertTextMode::AS_IS) - { - None - } else { - this.autoindent_mode.clone() + let auto_indent = match completion.insert_text_mode { + Some(InsertTextMode::AS_IS) => None, + _ => this.autoindent_mode.clone(), }; + let edits = ranges.into_iter().map(|range| (range, new_text.as_str())); buffer.edit(edits, auto_indent, cx); }); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index c1f154a4f1..e67b7cc539 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -10104,7 +10104,7 @@ async fn test_completion_with_mode_specified_by_action(cx: &mut TestAppContext) } #[gpui::test] -async fn test_completion_replacing_suffix_in_multicursors(cx: &mut TestAppContext) { +async fn test_completion_replacing_surrounding_text_with_multicursors(cx: &mut TestAppContext) { init_test(cx, |_| {}); let mut cx = EditorLspTestContext::new_rust( lsp::ServerCapabilities { @@ -10118,6 +10118,8 @@ async fn test_completion_replacing_suffix_in_multicursors(cx: &mut TestAppContex ) .await; + // scenario: surrounding text matches completion text + let completion_text = "to_offset"; let initial_state = indoc! {" 1. buf.to_offˇsuffix 2. buf.to_offˇsuf @@ -10148,7 +10150,6 @@ async fn test_completion_replacing_suffix_in_multicursors(cx: &mut TestAppContex buf. // newest cursor "}; - let completion_text = "to_offset"; let expected = indoc! {" 1. buf.to_offsetˇ 2. buf.to_offsetˇsuf @@ -10164,24 +10165,122 @@ async fn test_completion_replacing_suffix_in_multicursors(cx: &mut TestAppContex buf.to_offsetˇ // newest cursor "}; - cx.set_state(initial_state); cx.update_editor(|editor, window, cx| { editor.show_completions(&ShowCompletions { trigger: None }, window, cx); }); - - let counter = Arc::new(AtomicUsize::new(0)); handle_completion_request_with_insert_and_replace( &mut cx, completion_marked_buffer, vec![completion_text], - counter.clone(), + Arc::new(AtomicUsize::new(0)), ) .await; cx.condition(|editor, _| editor.context_menu_visible()) .await; - assert_eq!(counter.load(atomic::Ordering::Acquire), 1); + let apply_additional_edits = cx.update_editor(|editor, window, cx| { + editor + .confirm_completion_replace(&ConfirmCompletionReplace, window, cx) + .unwrap() + }); + cx.assert_editor_state(expected); + handle_resolve_completion_request(&mut cx, None).await; + apply_additional_edits.await.unwrap(); + // scenario: surrounding text matches surroundings of newest cursor, inserting at the end + let completion_text = "foo_and_bar"; + let initial_state = indoc! {" + 1. ooanbˇ + 2. zooanbˇ + 3. ooanbˇz + 4. zooanbˇz + 5. ooanˇ + 6. oanbˇ + + ooanbˇ + "}; + let completion_marked_buffer = indoc! {" + 1. ooanb + 2. zooanb + 3. ooanbz + 4. zooanbz + 5. ooan + 6. oanb + + + "}; + let expected = indoc! {" + 1. foo_and_barˇ + 2. zfoo_and_barˇ + 3. foo_and_barˇz + 4. zfoo_and_barˇz + 5. ooanfoo_and_barˇ + 6. oanbfoo_and_barˇ + + foo_and_barˇ + "}; + cx.set_state(initial_state); + cx.update_editor(|editor, window, cx| { + editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + }); + handle_completion_request_with_insert_and_replace( + &mut cx, + completion_marked_buffer, + vec![completion_text], + Arc::new(AtomicUsize::new(0)), + ) + .await; + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + let apply_additional_edits = cx.update_editor(|editor, window, cx| { + editor + .confirm_completion_replace(&ConfirmCompletionReplace, window, cx) + .unwrap() + }); + cx.assert_editor_state(expected); + handle_resolve_completion_request(&mut cx, None).await; + apply_additional_edits.await.unwrap(); + + // scenario: surrounding text matches surroundings of newest cursor, inserted at the middle + // (expects the same as if it was inserted at the end) + let completion_text = "foo_and_bar"; + let initial_state = indoc! {" + 1. ooˇanb + 2. zooˇanb + 3. ooˇanbz + 4. zooˇanbz + + ooˇanb + "}; + let completion_marked_buffer = indoc! {" + 1. ooanb + 2. zooanb + 3. ooanbz + 4. zooanbz + + + "}; + let expected = indoc! {" + 1. foo_and_barˇ + 2. zfoo_and_barˇ + 3. foo_and_barˇz + 4. zfoo_and_barˇz + + foo_and_barˇ + "}; + cx.set_state(initial_state); + cx.update_editor(|editor, window, cx| { + editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + }); + handle_completion_request_with_insert_and_replace( + &mut cx, + completion_marked_buffer, + vec![completion_text], + Arc::new(AtomicUsize::new(0)), + ) + .await; + cx.condition(|editor, _| editor.context_menu_visible()) + .await; let apply_additional_edits = cx.update_editor(|editor, window, cx| { editor .confirm_completion_replace(&ConfirmCompletionReplace, window, cx)