From 6d96c6ef51e011b487088b3df119ff1a22811acf Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 12 Jul 2023 18:32:03 +0300 Subject: [PATCH 1/3] Draft the postfix completions support --- crates/editor/src/editor_tests.rs | 91 +++++++++++++++++++++++++++++++ crates/project/src/lsp_command.rs | 20 +++---- 2 files changed, 101 insertions(+), 10 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 7b36287dca5ab84a695c9ecb8c701cf84c71a043..890d34af13e7c23d9597104064b5f6fd099d7c1f 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -7223,6 +7223,97 @@ async fn test_language_server_restart_due_to_settings_change(cx: &mut gpui::Test ); } +#[gpui::test] +async fn test_completions_with_extra_edits(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string()]), + ..Default::default() + }), + ..Default::default() + }, + cx, + ) + .await; + + cx.set_state(indoc! {"fn main() { let a = 2ˇ; }"}); + cx.simulate_keystroke("."); + let completion_item = lsp::CompletionItem { + label: "some".into(), + kind: Some(lsp::CompletionItemKind::SNIPPET), + detail: Some("Wrap the expression in an `Option::Some`".to_string()), + documentation: Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { + kind: lsp::MarkupKind::Markdown, + value: "```rust\nSome(2)\n```".to_string(), + })), + deprecated: Some(false), + sort_text: Some("fffffff2".to_string()), + filter_text: Some("some".to_string()), + insert_text_format: Some(lsp::InsertTextFormat::SNIPPET), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 22, + }, + end: lsp::Position { + line: 0, + character: 22, + }, + }, + new_text: "Some(2)".to_string(), + })), + additional_text_edits: Some(vec![lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 20, + }, + end: lsp::Position { + line: 0, + character: 22, + }, + }, + new_text: "".to_string(), + }]), + ..Default::default() + }; + + let closure_completion_item = completion_item.clone(); + let mut request = cx.handle_request::(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, cx| { + editor + .confirm_completion(&ConfirmCompletion::default(), cx) + .unwrap() + }); + cx.assert_editor_state(indoc! {"fn main() { let a = 2.Some(2)ˇ; }"}); + + cx.handle_request::(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(indoc! {"fn main() { let a = Some(2)ˇ; }"}); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(row as u32, column as u32); point..point diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index eec64beb5a3f25300d3147cde2033a58c235d53d..56a6c4e88d1eef775ae5ebb1192eb3d0e8698121 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -1349,7 +1349,6 @@ impl LspCommand for GetCompletions { } else { Default::default() }; - let completions = buffer.read_with(&cx, |buffer, _| { let language = buffer.language().cloned(); let snapshot = buffer.snapshot(); @@ -1358,15 +1357,16 @@ impl LspCommand for GetCompletions { completions .into_iter() .filter_map(move |mut lsp_completion| { - // For now, we can only handle additional edits if they are returned - // when resolving the completion, not if they are present initially. - if lsp_completion - .additional_text_edits - .as_ref() - .map_or(false, |edits| !edits.is_empty()) - { - return None; - } + // TODO kb store these? at least, should only allow this when we have resolve + // // For now, we can only handle additional edits if they are returned + // // when resolving the completion, not if they are present initially. + // if lsp_completion + // .additional_text_edits + // .as_ref() + // .map_or(false, |edits| !edits.is_empty()) + // { + // return None; + // } let (old_range, mut new_text) = match lsp_completion.text_edit.as_ref() { // If the language server provides a range to overwrite, then From c732aa1617a084cfe849d6aecacf5e43243532db Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 12 Jul 2023 19:29:20 +0300 Subject: [PATCH 2/3] Do not resolve completions if extra edits are available --- crates/editor/src/editor_tests.rs | 86 +++++++++++++++++++++++++++++++ crates/project/src/lsp_command.rs | 12 +---- crates/project/src/project.rs | 15 ++++-- 3 files changed, 98 insertions(+), 15 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 890d34af13e7c23d9597104064b5f6fd099d7c1f..525623738c13d8b6f228b54d8e1776c8ee106322 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -7294,6 +7294,92 @@ async fn test_completions_with_extra_edits(cx: &mut gpui::TestAppContext) { request.next().await; + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + let apply_additional_edits = cx.update_editor(|editor, cx| { + editor + .confirm_completion(&ConfirmCompletion::default(), cx) + .unwrap() + }); + cx.assert_editor_state(indoc! {"fn main() { let a = 2.Some(2)ˇ; }"}); + apply_additional_edits.await.unwrap(); + cx.assert_editor_state(indoc! {"fn main() { let a = Some(2)ˇ; }"}); +} + +#[gpui::test] +async fn test_completions_with_extra_resolved_edits(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string()]), + ..Default::default() + }), + ..Default::default() + }, + cx, + ) + .await; + + cx.set_state(indoc! {"fn main() { let a = 2ˇ; }"}); + cx.simulate_keystroke("."); + let completion_item = lsp::CompletionItem { + label: "some".into(), + kind: Some(lsp::CompletionItemKind::SNIPPET), + detail: Some("Wrap the expression in an `Option::Some`".to_string()), + documentation: Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { + kind: lsp::MarkupKind::Markdown, + value: "```rust\nSome(2)\n```".to_string(), + })), + deprecated: Some(false), + sort_text: Some("fffffff2".to_string()), + filter_text: Some("some".to_string()), + insert_text_format: Some(lsp::InsertTextFormat::SNIPPET), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 22, + }, + end: lsp::Position { + line: 0, + character: 22, + }, + }, + new_text: "Some(2)".to_string(), + })), + additional_text_edits: Some(vec![lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 20, + }, + end: lsp::Position { + line: 0, + character: 22, + }, + }, + new_text: "".to_string(), + }]), + ..Default::default() + }; + + let closure_completion_item = completion_item.clone(); + let mut request = cx.handle_request::(move |_, _, _| { + let task_completion_item = closure_completion_item.clone(); + async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + lsp::CompletionItem { + additional_text_edits: None, + ..task_completion_item + }, + ]))) + } + }); + + request.next().await; + cx.condition(|editor, _| editor.context_menu_visible()) .await; let apply_additional_edits = cx.update_editor(|editor, cx| { diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 56a6c4e88d1eef775ae5ebb1192eb3d0e8698121..08261b64f17c8714d1305fbff303080ddc82f0ab 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -1349,6 +1349,7 @@ impl LspCommand for GetCompletions { } else { Default::default() }; + let completions = buffer.read_with(&cx, |buffer, _| { let language = buffer.language().cloned(); let snapshot = buffer.snapshot(); @@ -1357,17 +1358,6 @@ impl LspCommand for GetCompletions { completions .into_iter() .filter_map(move |mut lsp_completion| { - // TODO kb store these? at least, should only allow this when we have resolve - // // For now, we can only handle additional edits if they are returned - // // when resolving the completion, not if they are present initially. - // if lsp_completion - // .additional_text_edits - // .as_ref() - // .map_or(false, |edits| !edits.is_empty()) - // { - // return None; - // } - let (old_range, mut new_text) = match lsp_completion.text_edit.as_ref() { // If the language server provides a range to overwrite, then // check that the range is valid. diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 364b19e3a9fc26f51186e99b51184d47dfedbaec..fd933f11c6a7eaedafb8fdf611503e3aba238ca6 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4446,11 +4446,18 @@ impl Project { }; cx.spawn(|this, mut cx| async move { - let resolved_completion = lang_server - .request::(completion.lsp_completion) - .await?; + let additional_text_edits = if let Some(edits) = + completion.lsp_completion.additional_text_edits.as_ref() + { + Some(edits.clone()) + } else { + lang_server + .request::(completion.lsp_completion) + .await? + .additional_text_edits + }; - if let Some(edits) = resolved_completion.additional_text_edits { + if let Some(edits) = additional_text_edits { let edits = this .update(&mut cx, |this, cx| { this.edits_from_lsp( From 0c7949bdeec2fa2ec48a9cf65f9f6cee74e85c57 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 12 Jul 2023 21:10:01 +0300 Subject: [PATCH 3/3] Force resolve all completions, to ensure their edits are up-to-date co-authored-by: Max Brunsfeld --- crates/editor/src/editor_tests.rs | 88 +------------------------------ crates/project/src/project.rs | 15 ++---- 2 files changed, 5 insertions(+), 98 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 525623738c13d8b6f228b54d8e1776c8ee106322..260b0ccc40bde32816e8c6d5e10bb8be9075b2a1 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -7224,7 +7224,7 @@ async fn test_language_server_restart_due_to_settings_change(cx: &mut gpui::Test } #[gpui::test] -async fn test_completions_with_extra_edits(cx: &mut gpui::TestAppContext) { +async fn test_completions_with_additional_edits(cx: &mut gpui::TestAppContext) { init_test(cx, |_| {}); let mut cx = EditorLspTestContext::new_rust( @@ -7294,92 +7294,6 @@ async fn test_completions_with_extra_edits(cx: &mut gpui::TestAppContext) { request.next().await; - cx.condition(|editor, _| editor.context_menu_visible()) - .await; - let apply_additional_edits = cx.update_editor(|editor, cx| { - editor - .confirm_completion(&ConfirmCompletion::default(), cx) - .unwrap() - }); - cx.assert_editor_state(indoc! {"fn main() { let a = 2.Some(2)ˇ; }"}); - apply_additional_edits.await.unwrap(); - cx.assert_editor_state(indoc! {"fn main() { let a = Some(2)ˇ; }"}); -} - -#[gpui::test] -async fn test_completions_with_extra_resolved_edits(cx: &mut gpui::TestAppContext) { - init_test(cx, |_| {}); - - let mut cx = EditorLspTestContext::new_rust( - lsp::ServerCapabilities { - completion_provider: Some(lsp::CompletionOptions { - trigger_characters: Some(vec![".".to_string()]), - ..Default::default() - }), - ..Default::default() - }, - cx, - ) - .await; - - cx.set_state(indoc! {"fn main() { let a = 2ˇ; }"}); - cx.simulate_keystroke("."); - let completion_item = lsp::CompletionItem { - label: "some".into(), - kind: Some(lsp::CompletionItemKind::SNIPPET), - detail: Some("Wrap the expression in an `Option::Some`".to_string()), - documentation: Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { - kind: lsp::MarkupKind::Markdown, - value: "```rust\nSome(2)\n```".to_string(), - })), - deprecated: Some(false), - sort_text: Some("fffffff2".to_string()), - filter_text: Some("some".to_string()), - insert_text_format: Some(lsp::InsertTextFormat::SNIPPET), - text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: lsp::Range { - start: lsp::Position { - line: 0, - character: 22, - }, - end: lsp::Position { - line: 0, - character: 22, - }, - }, - new_text: "Some(2)".to_string(), - })), - additional_text_edits: Some(vec![lsp::TextEdit { - range: lsp::Range { - start: lsp::Position { - line: 0, - character: 20, - }, - end: lsp::Position { - line: 0, - character: 22, - }, - }, - new_text: "".to_string(), - }]), - ..Default::default() - }; - - let closure_completion_item = completion_item.clone(); - let mut request = cx.handle_request::(move |_, _, _| { - let task_completion_item = closure_completion_item.clone(); - async move { - Ok(Some(lsp::CompletionResponse::Array(vec![ - lsp::CompletionItem { - additional_text_edits: None, - ..task_completion_item - }, - ]))) - } - }); - - request.next().await; - cx.condition(|editor, _| editor.context_menu_visible()) .await; let apply_additional_edits = cx.update_editor(|editor, cx| { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index fd933f11c6a7eaedafb8fdf611503e3aba238ca6..7ad8f121b7420e5d8b4225e295e4fb0298bce04c 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4446,17 +4446,10 @@ impl Project { }; cx.spawn(|this, mut cx| async move { - let additional_text_edits = if let Some(edits) = - completion.lsp_completion.additional_text_edits.as_ref() - { - Some(edits.clone()) - } else { - lang_server - .request::(completion.lsp_completion) - .await? - .additional_text_edits - }; - + let additional_text_edits = lang_server + .request::(completion.lsp_completion) + .await? + .additional_text_edits; if let Some(edits) = additional_text_edits { let edits = this .update(&mut cx, |this, cx| {