From 949944acb4b33595f329126060132a2cdaca1de9 Mon Sep 17 00:00:00 2001 From: Atchyut Preetham Pulavarthi Date: Mon, 16 Mar 2026 15:22:57 +0530 Subject: [PATCH] editor: Fix jumbled auto-imports when completing with multiple cursors (#50320) When accepting an autocomplete suggestion with multiple active cursors using `CMD+D`, Zed applies the primary completion edit to all cursors. However, the overlap check for LSP `additionalTextEdits` only verifies the replace range of the newest cursor. If user has a cursor inside an existing import statement at the top of the file and another cursor further down, Zed fails to detect the overlap at the top of the file. When the user auto-completes the import statement ends up jumbled. This fix updates the completion logic to calculate the commit ranges for all active cursors and passes them to the LSP store. The overlap check now iterates over all commit ranges to ensure auto-imports are correctly discarded if they intersect with any of the user's multi-cursor edits. Closes https://github.com/zed-industries/zed/issues/50314 ### Before https://github.com/user-attachments/assets/8d0f71ec-37ab-4714-a318-897d9ee5e56b ### After https://github.com/user-attachments/assets/4c978167-3065-48c0-bc3c-547a2dd22ac3 Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [x] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed an issue where accepting an autocomplete suggestion with multiple cursors could result in duplicated or jumbled text in import statements. --- crates/editor/src/editor.rs | 13 ++++- crates/editor/src/editor_tests.rs | 94 +++++++++++++++++++++++++++++++ crates/project/src/lsp_store.rs | 32 ++++++++--- crates/proto/proto/lsp.proto | 1 + 4 files changed, 129 insertions(+), 11 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index fd830c254877463da84e98d21dd39b0e644ca433..2512c362f9c06dc94b231a2ea56168df9e13bf7e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6481,6 +6481,7 @@ impl Editor { .selections .all::(&self.display_snapshot(cx)); let mut ranges = Vec::new(); + let mut all_commit_ranges = Vec::new(); let mut linked_edits = LinkedEdits::new(); let text: Arc = new_text.clone().into(); @@ -6506,10 +6507,12 @@ impl Editor { ranges.push(range.clone()); + let start_anchor = snapshot.anchor_before(range.start); + let end_anchor = snapshot.anchor_after(range.end); + let anchor_range = start_anchor.text_anchor..end_anchor.text_anchor; + all_commit_ranges.push(anchor_range.clone()); + if !self.linked_edit_ranges.is_empty() { - let start_anchor = snapshot.anchor_before(range.start); - let end_anchor = snapshot.anchor_after(range.end); - let anchor_range = start_anchor.text_anchor..end_anchor.text_anchor; linked_edits.push(&self, anchor_range, text.clone(), cx); } } @@ -6596,6 +6599,7 @@ impl Editor { completions_menu.completions.clone(), candidate_id, true, + all_commit_ranges, cx, ); @@ -26575,6 +26579,7 @@ pub trait CompletionProvider { _completions: Rc>>, _completion_index: usize, _push_to_history: bool, + _all_commit_ranges: Vec>, _cx: &mut Context, ) -> Task>> { Task::ready(Ok(None)) @@ -26943,6 +26948,7 @@ impl CompletionProvider for Entity { completions: Rc>>, completion_index: usize, push_to_history: bool, + all_commit_ranges: Vec>, cx: &mut Context, ) -> Task>> { self.update(cx, |project, cx| { @@ -26952,6 +26958,7 @@ impl CompletionProvider for Entity { completions, completion_index, push_to_history, + all_commit_ranges, cx, ) }) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index f497881531bf4ba39cb22aca4cf90923f7d10b81..683995e8ff0817e9f11c276fba1e85eef29eee7a 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -19888,6 +19888,100 @@ async fn test_completions_with_additional_edits(cx: &mut TestAppContext) { cx.assert_editor_state("fn main() { let a = Some(2)ˇ; }"); } +#[gpui::test] +async fn test_completions_with_additional_edits_and_multiple_cursors(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorLspTestContext::new_typescript( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + resolve_provider: Some(true), + ..Default::default() + }), + ..Default::default() + }, + cx, + ) + .await; + + cx.set_state( + "import { «Fooˇ» } from './types';\n\nclass Bar {\n method(): «Fooˇ» { return new Foo(); }\n}", + ); + + cx.simulate_keystroke("F"); + cx.simulate_keystroke("o"); + + let completion_item = lsp::CompletionItem { + label: "FooBar".into(), + kind: Some(lsp::CompletionItemKind::CLASS), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 3, + character: 14, + }, + end: lsp::Position { + line: 3, + character: 16, + }, + }, + new_text: "FooBar".to_string(), + })), + additional_text_edits: Some(vec![lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 9, + }, + end: lsp::Position { + line: 0, + character: 11, + }, + }, + new_text: "FooBar".to_string(), + }]), + ..Default::default() + }; + + let closure_completion_item = completion_item.clone(); + let mut request = cx.set_request_handler::(move |_, _, _| { + let task_completion_item = closure_completion_item.clone(); + async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + task_completion_item, + ]))) + } + }); + + request.next().await; + + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + let apply_additional_edits = cx.update_editor(|editor, window, cx| { + editor + .confirm_completion(&ConfirmCompletion::default(), window, cx) + .unwrap() + }); + + cx.assert_editor_state( + "import { FooBarˇ } from './types';\n\nclass Bar {\n method(): FooBarˇ { return new Foo(); }\n}", + ); + + cx.set_request_handler::(move |_, _, _| { + let task_completion_item = completion_item.clone(); + async move { Ok(task_completion_item) } + }) + .next() + .await + .unwrap(); + + apply_additional_edits.await.unwrap(); + + cx.assert_editor_state( + "import { FooBarˇ } from './types';\n\nclass Bar {\n method(): FooBarˇ { return new Foo(); }\n}", + ); +} + #[gpui::test] async fn test_completions_resolve_updates_labels_if_filter_text_matches(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 8b4f3d7e8e1a6f68a1263fc11dc2e61c4a4890aa..25a614052789c85b8c418086e803b9b5cb9e6fae 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -6643,6 +6643,7 @@ impl LspStore { completions: Rc>>, completion_index: usize, push_to_history: bool, + all_commit_ranges: Vec>, cx: &mut Context, ) -> Task>> { if let Some((client, project_id)) = self.upstream_client() { @@ -6659,6 +6660,11 @@ impl LspStore { new_text: completion.new_text, source: completion.source, })), + all_commit_ranges: all_commit_ranges + .iter() + .cloned() + .map(language::proto::serialize_anchor_range) + .collect(), } }; @@ -6752,12 +6758,15 @@ impl LspStore { let has_overlap = if is_file_start_auto_import { false } else { - let start_within = primary.start.cmp(&range.start, buffer).is_le() - && primary.end.cmp(&range.start, buffer).is_ge(); - let end_within = range.start.cmp(&primary.end, buffer).is_le() - && range.end.cmp(&primary.end, buffer).is_ge(); - let result = start_within || end_within; - result + all_commit_ranges.iter().any(|commit_range| { + let start_within = + commit_range.start.cmp(&range.start, buffer).is_le() + && commit_range.end.cmp(&range.start, buffer).is_ge(); + let end_within = + range.start.cmp(&commit_range.end, buffer).is_le() + && range.end.cmp(&commit_range.end, buffer).is_ge(); + start_within || end_within + }) }; //Skip additional edits which overlap with the primary completion edit @@ -10418,13 +10427,19 @@ impl LspStore { envelope: TypedEnvelope, mut cx: AsyncApp, ) -> Result { - let (buffer, completion) = this.update(&mut cx, |this, cx| { + let (buffer, completion, all_commit_ranges) = this.update(&mut cx, |this, cx| { let buffer_id = BufferId::new(envelope.payload.buffer_id)?; let buffer = this.buffer_store.read(cx).get_existing(buffer_id)?; let completion = Self::deserialize_completion( envelope.payload.completion.context("invalid completion")?, )?; - anyhow::Ok((buffer, completion)) + let all_commit_ranges = envelope + .payload + .all_commit_ranges + .into_iter() + .map(language::proto::deserialize_anchor_range) + .collect::, _>>()?; + anyhow::Ok((buffer, completion, all_commit_ranges)) })?; let apply_additional_edits = this.update(&mut cx, |this, cx| { @@ -10444,6 +10459,7 @@ impl LspStore { }]))), 0, false, + all_commit_ranges, cx, ) }); diff --git a/crates/proto/proto/lsp.proto b/crates/proto/proto/lsp.proto index 226373a111b6e29e4731edd638a5317dcd244273..813f9e9ec652a7b97281bea29f368b0dcf37d537 100644 --- a/crates/proto/proto/lsp.proto +++ b/crates/proto/proto/lsp.proto @@ -230,6 +230,7 @@ message ApplyCompletionAdditionalEdits { uint64 project_id = 1; uint64 buffer_id = 2; Completion completion = 3; + repeated AnchorRange all_commit_ranges = 4; } message ApplyCompletionAdditionalEditsResponse {