From 03115c8d719183e23bf6c19c9a44ceb65d18ffe7 Mon Sep 17 00:00:00 2001 From: Julia Date: Thu, 10 Nov 2022 15:28:11 -0500 Subject: [PATCH 1/3] Skip LSP additional completion edits which fall within primary edit --- crates/project/src/project.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 3c28f6b512e38f9f803398641b7b9676beec234f..0675610dcb7a27e3c08d687089c39068b0cdec16 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3453,29 +3453,39 @@ 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::(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 within_primary = primary.start.cmp(&range.start, buffer).is_ge() + && primary.end.cmp(&range.end, buffer).is_le(); + let within_additional = range.start.cmp(&primary.start, buffer).is_ge() + && range.end.cmp(&primary.end, buffer).is_le(); + + if !within_primary && !within_additional { + 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 { From 44c3cedc48e5f3245429e694eee7aa9af9db0a78 Mon Sep 17 00:00:00 2001 From: Julia Date: Thu, 10 Nov 2022 18:53:37 -0500 Subject: [PATCH 2/3] Skip additional completions on any kind of overlap with primary edit --- crates/project/src/project.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 0675610dcb7a27e3c08d687089c39068b0cdec16..0e2723201a7dab20fdd2c5f71048e81ad4a4af8f 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3476,12 +3476,12 @@ impl Project { for (range, text) in edits { let primary = &completion.old_range; - let within_primary = primary.start.cmp(&range.start, buffer).is_ge() - && primary.end.cmp(&range.end, buffer).is_le(); - let within_additional = range.start.cmp(&primary.start, buffer).is_ge() - && range.end.cmp(&primary.end, buffer).is_le(); + 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(); - if !within_primary && !within_additional { + if !start_within && !end_within { buffer.edit([(range, text)], None, cx); } } From ad698fd11002c7ff24f5657d9637796d66ce4dc8 Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 11 Nov 2022 10:28:07 -0500 Subject: [PATCH 3/3] Test for filtering out of faulty LSP completion additional edits --- crates/editor/src/editor_tests.rs | 47 +++++++++++++++++++++---------- crates/project/src/project.rs | 2 ++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 763917a46489739ad41e6c5eb501ab5275c819ed..7bd5dda522f6298067619074ce79647e07f1b133 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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>, ) { - 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::>() }); cx.handle_request::(move |_, _, _| { - let edit = edit.clone(); + let edits = edits.clone(); async move { Ok(lsp::CompletionItem { - additional_text_edits: edit, + additional_text_edits: edits, ..Default::default() }) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 0e2723201a7dab20fdd2c5f71048e81ad4a4af8f..1563cb9de4268b66802e239107ce310b32b5ec21 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3481,6 +3481,8 @@ impl Project { 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); }