From e0dc1314180802c7e9b4486fc01d4ca3cf0b07f6 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Thu, 17 Apr 2025 15:50:37 -0300 Subject: [PATCH] Fix multiline completions when surrounding text doesn't match completion text (cherry-pick #28995) (#28997) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-picked 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 Co-authored-by: João Marcos --- crates/editor/src/editor.rs | 26 +++---- crates/editor/src/editor_tests.rs | 111 ++++++++++++++++++++++++++++-- 2 files changed, 115 insertions(+), 22 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 63b41f87c8e56cfa97f9ab04a7d3c33487258120..fde2bd0b6094e8257d4f32809d61ea4e20abe231 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -4744,19 +4744,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 @@ -4765,10 +4764,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()); @@ -4794,19 +4793,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 3d55e20a8a323255043a0445e8ac5754b365f3d9..655438e639dcd2879cf4e991b05f1ceee8583b44 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9924,7 +9924,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 { @@ -9938,6 +9938,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 @@ -9968,7 +9970,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 @@ -9984,24 +9985,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 + + + "}; + 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 + + + "}; + 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)