From 9ec09277012956503d5c206480b10de4930b3aed Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 5 May 2024 13:01:52 +0300 Subject: [PATCH] Respect LSP completion triggers when copilot suggestion is on (#11401) --- crates/copilot/Cargo.toml | 1 + .../src/copilot_completion_provider.rs | 118 ++++++++++++++++++ crates/editor/src/editor.rs | 21 ++-- crates/multi_buffer/src/multi_buffer.rs | 10 +- 4 files changed, 137 insertions(+), 13 deletions(-) diff --git a/crates/copilot/Cargo.toml b/crates/copilot/Cargo.toml index 3f38a81f5b..033240028c 100644 --- a/crates/copilot/Cargo.toml +++ b/crates/copilot/Cargo.toml @@ -53,6 +53,7 @@ clock.workspace = true indoc.workspace = true serde_json.workspace = true collections = { workspace = true, features = ["test-support"] } +editor = { workspace = true, features = ["test-support"] } fs = { workspace = true, features = ["test-support"] } gpui = { workspace = true, features = ["test-support"] } language = { workspace = true, features = ["test-support"] } diff --git a/crates/copilot/src/copilot_completion_provider.rs b/crates/copilot/src/copilot_completion_provider.rs index 970145a10f..8e4f90e11d 100644 --- a/crates/copilot/src/copilot_completion_provider.rs +++ b/crates/copilot/src/copilot_completion_provider.rs @@ -840,6 +840,124 @@ mod tests { }); } + #[gpui::test] + async fn test_copilot_does_not_prevent_completion_triggers( + executor: BackgroundExecutor, + cx: &mut TestAppContext, + ) { + init_test(cx, |_| {}); + + let (copilot, copilot_lsp) = Copilot::fake(cx); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string(), ":".to_string()]), + ..lsp::CompletionOptions::default() + }), + ..lsp::ServerCapabilities::default() + }, + cx, + ) + .await; + let copilot_provider = cx.new_model(|_| CopilotCompletionProvider::new(copilot)); + cx.update_editor(|editor, cx| { + editor.set_inline_completion_provider(Some(copilot_provider), cx) + }); + + cx.set_state(indoc! {" + one + twˇ + three + "}); + + let _ = handle_completion_request( + &mut cx, + indoc! {" + one + tw|<> + three + "}, + vec!["completion_a", "completion_b"], + ); + handle_copilot_completion_request( + &copilot_lsp, + vec![crate::request::Completion { + text: "two.foo()".into(), + range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 2)), + ..Default::default() + }], + vec![], + ); + cx.update_editor(|editor, cx| editor.next_inline_completion(&Default::default(), cx)); + executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(!editor.context_menu_visible(), "Even there are some completions available, those are not triggered when active copilot suggestion is present"); + assert!(editor.has_active_inline_completion(cx)); + assert_eq!(editor.display_text(cx), "one\ntwo.foo()\nthree\n"); + assert_eq!(editor.text(cx), "one\ntw\nthree\n"); + }); + + cx.simulate_keystroke("o"); + let _ = handle_completion_request( + &mut cx, + indoc! {" + one + two|<> + three + "}, + vec!["completion_a_2", "completion_b_2"], + ); + handle_copilot_completion_request( + &copilot_lsp, + vec![crate::request::Completion { + text: "two.foo()".into(), + range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 3)), + ..Default::default() + }], + vec![], + ); + executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(!editor.context_menu_visible()); + assert!(editor.has_active_inline_completion(cx)); + assert_eq!(editor.display_text(cx), "one\ntwo.foo()\nthree\n"); + assert_eq!(editor.text(cx), "one\ntwo\nthree\n"); + }); + + cx.simulate_keystroke("."); + let _ = handle_completion_request( + &mut cx, + indoc! {" + one + two.|<> + three + "}, + vec!["something_else()"], + ); + handle_copilot_completion_request( + &copilot_lsp, + vec![crate::request::Completion { + text: "two.foo()".into(), + range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 4)), + ..Default::default() + }], + vec![], + ); + executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!( + editor.context_menu_visible(), + "On completion trigger input, the completions should be fetched and visible" + ); + assert!( + !editor.has_active_inline_completion(cx), + "On completion trigger input, copilot suggestion should be dismissed" + ); + assert_eq!(editor.display_text(cx), "one\ntwo.\nthree\n"); + assert_eq!(editor.text(cx), "one\ntwo.\nthree\n"); + }); + } + #[gpui::test] async fn test_copilot_disabled_globs(executor: BackgroundExecutor, cx: &mut TestAppContext) { init_test(cx, |settings| { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 5cc0a3b29b..d1d8c697fa 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2695,15 +2695,9 @@ impl Editor { } } - if had_active_inline_completion { - this.refresh_inline_completion(true, cx); - if !this.has_active_inline_completion(cx) { - this.trigger_completion_on_input(&text, cx); - } - } else { - this.trigger_completion_on_input(&text, cx); - this.refresh_inline_completion(true, cx); - } + let trigger_in_words = !had_active_inline_completion; + this.trigger_completion_on_input(&text, trigger_in_words, cx); + this.refresh_inline_completion(true, cx); }); } @@ -3056,7 +3050,12 @@ impl Editor { }); } - fn trigger_completion_on_input(&mut self, text: &str, cx: &mut ViewContext) { + fn trigger_completion_on_input( + &mut self, + text: &str, + trigger_in_words: bool, + cx: &mut ViewContext, + ) { if !EditorSettings::get_global(cx).show_completions_on_input { return; } @@ -3065,7 +3064,7 @@ impl Editor { if self .buffer .read(cx) - .is_completion_trigger(selection.head(), text, cx) + .is_completion_trigger(selection.head(), text, trigger_in_words, cx) { self.show_completions(&ShowCompletions, cx); } else { diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 38bb3deed0..6ebcf98526 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -1522,7 +1522,13 @@ impl MultiBuffer { .map(|state| state.buffer.clone()) } - pub fn is_completion_trigger(&self, position: Anchor, text: &str, cx: &AppContext) -> bool { + pub fn is_completion_trigger( + &self, + position: Anchor, + text: &str, + trigger_in_words: bool, + cx: &AppContext, + ) -> bool { let mut chars = text.chars(); let char = if let Some(char) = chars.next() { char @@ -1536,7 +1542,7 @@ impl MultiBuffer { let snapshot = self.snapshot(cx); let position = position.to_offset(&snapshot); let scope = snapshot.language_scope_at(position); - if char_kind(&scope, char) == CharKind::Word { + if trigger_in_words && char_kind(&scope, char) == CharKind::Word { return true; }