Fix multiline completions when surroundings don't match completion text (#28995)

João Marcos created

Follow up to the scenarios I overlooked in
https://github.com/zed-industries/zed/pull/28586.

Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs       |  26 ++----
crates/editor/src/editor_tests.rs | 111 +++++++++++++++++++++++++++++++-
2 files changed, 115 insertions(+), 22 deletions(-)

Detailed changes

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::<usize>(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::<Vec<_>>();
                 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);
                 });
             }

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.<to_off|suffix>  // 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);
     });
+    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();
 
-    let counter = Arc::new(AtomicUsize::new(0));
+    // 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
+
+        <ooanb|>
+    "};
+    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],
-        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, 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
+
+        <oo|anb>
+    "};
+    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)