Merge pull request #1871 from zed-industries/skip-additional-edit-within-primary

Julia created 3 years ago

Skip LSP additional completion edits which fall within primary edit

Change summary

crates/editor/src/editor_tests.rs | 47 ++++++++++++++++++++++----------
crates/project/src/project.rs     | 24 ++++++++++++----
2 files changed, 50 insertions(+), 21 deletions(-)

Detailed changes

crates/editor/src/editor_tests.rs

@@ -4146,14 +4146,26 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
 
     handle_resolve_completion_request(
         &mut cx,
-        Some((
-            indoc! {"
-                one.second_completion
-                two
-                threeˇ
-            "},
-            "\nadditional edit",
-        )),
+        Some(vec![
+            (
+                //This overlaps with the primary completion edit which is
+                //misbehavior from the LSP spec, test that we filter it out
+                indoc! {"
+                    one.second_ˇcompletion
+                    two
+                    threeˇ
+                "},
+                "overlapping aditional edit",
+            ),
+            (
+                indoc! {"
+                    one.second_completion
+                    two
+                    threeˇ
+                "},
+                "\nadditional edit",
+            ),
+        ]),
     )
     .await;
     apply_additional_edits.await.unwrap();
@@ -4303,19 +4315,24 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
 
     async fn handle_resolve_completion_request<'a>(
         cx: &mut EditorLspTestContext<'a>,
-        edit: Option<(&'static str, &'static str)>,
+        edits: Option<Vec<(&'static str, &'static str)>>,
     ) {
-        let edit = edit.map(|(marked_string, new_text)| {
-            let (_, marked_ranges) = marked_text_ranges(marked_string, false);
-            let replace_range = cx.to_lsp_range(marked_ranges[0].clone());
-            vec![lsp::TextEdit::new(replace_range, new_text.to_string())]
+        let edits = edits.map(|edits| {
+            edits
+                .iter()
+                .map(|(marked_string, new_text)| {
+                    let (_, marked_ranges) = marked_text_ranges(marked_string, false);
+                    let replace_range = cx.to_lsp_range(marked_ranges[0].clone());
+                    lsp::TextEdit::new(replace_range, new_text.to_string())
+                })
+                .collect::<Vec<_>>()
         });
 
         cx.handle_request::<lsp::request::ResolveCompletionItem, _, _>(move |_, _, _| {
-            let edit = edit.clone();
+            let edits = edits.clone();
             async move {
                 Ok(lsp::CompletionItem {
-                    additional_text_edits: edit,
+                    additional_text_edits: edits,
                     ..Default::default()
                 })
             }

crates/project/src/project.rs

@@ -3453,29 +3453,41 @@ impl Project {
         let buffer_id = buffer.remote_id();
 
         if self.is_local() {
-            let lang_server = if let Some((_, server)) = self.language_server_for_buffer(buffer, cx)
-            {
-                server.clone()
-            } else {
-                return Task::ready(Ok(Default::default()));
+            let lang_server = match self.language_server_for_buffer(buffer, cx) {
+                Some((_, server)) => server.clone(),
+                _ => return Task::ready(Ok(Default::default())),
             };
 
             cx.spawn(|this, mut cx| async move {
                 let resolved_completion = lang_server
                     .request::<lsp::request::ResolveCompletionItem>(completion.lsp_completion)
                     .await?;
+
                 if let Some(edits) = resolved_completion.additional_text_edits {
                     let edits = this
                         .update(&mut cx, |this, cx| {
                             this.edits_from_lsp(&buffer_handle, edits, None, cx)
                         })
                         .await?;
+
                     buffer_handle.update(&mut cx, |buffer, cx| {
                         buffer.finalize_last_transaction();
                         buffer.start_transaction();
+
                         for (range, text) in edits {
-                            buffer.edit([(range, text)], None, cx);
+                            let primary = &completion.old_range;
+                            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();
+
+                            //Skip addtional edits which overlap with the primary completion edit
+                            //https://github.com/zed-industries/zed/pull/1871
+                            if !start_within && !end_within {
+                                buffer.edit([(range, text)], None, cx);
+                            }
                         }
+
                         let transaction = if buffer.end_transaction(cx).is_some() {
                             let transaction = buffer.finalize_last_transaction().unwrap().clone();
                             if !push_to_history {