From 399451b676afd319469434d89d3fe040716676d0 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 10:26:43 +0200 Subject: [PATCH 1/9] Capture copilot behavior in a editor unit test --- crates/copilot/Cargo.toml | 12 ++ crates/copilot/src/copilot.rs | 39 ++-- crates/copilot/src/request.rs | 2 +- crates/editor/Cargo.toml | 2 + crates/editor/src/editor.rs | 3 +- crates/editor/src/editor_tests.rs | 334 +++++++++++++++++++++++------- 6 files changed, 299 insertions(+), 93 deletions(-) diff --git a/crates/copilot/Cargo.toml b/crates/copilot/Cargo.toml index 52b9c223bd..1c37b6e298 100644 --- a/crates/copilot/Cargo.toml +++ b/crates/copilot/Cargo.toml @@ -8,6 +8,17 @@ publish = false path = "src/copilot.rs" doctest = false +[features] +test-support = [ + "client/test-support", + "collections/test-support", + "gpui/test-support", + "language/test-support", + "lsp/test-support", + "settings/test-support", + "util/test-support", +] + [dependencies] collections = { path = "../collections" } context_menu = { path = "../context_menu" } @@ -30,6 +41,7 @@ smol = "1.2.5" futures = "0.3" [dev-dependencies] +collections = { path = "../collections", features = ["test-support"] } gpui = { path = "../gpui", features = ["test-support"] } language = { path = "../language", features = ["test-support"] } settings = { path = "../settings", features = ["test-support"] } diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index bcf3188482..c0c5afc068 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -1,4 +1,4 @@ -mod request; +pub mod request; mod sign_in; use anyhow::{anyhow, Context, Result}; @@ -125,12 +125,8 @@ enum CopilotServer { #[derive(Clone, Debug)] enum SignInStatus { - Authorized { - _user: String, - }, - Unauthorized { - _user: String, - }, + Authorized, + Unauthorized, SigningIn { prompt: Option, task: Shared>>>, @@ -238,6 +234,23 @@ impl Copilot { } } + #[cfg(any(test, feature = "test-support"))] + pub fn fake(cx: &mut gpui::TestAppContext) -> (ModelHandle, lsp::FakeLanguageServer) { + let (server, fake_server) = + LanguageServer::fake("copilot".into(), Default::default(), cx.to_async()); + let http = util::http::FakeHttpClient::create(|_| async { unreachable!() }); + let this = cx.add_model(|cx| Self { + http: http.clone(), + node_runtime: NodeRuntime::new(http, cx.background().clone()), + server: CopilotServer::Started { + server: Arc::new(server), + status: SignInStatus::Authorized, + subscriptions_by_buffer_id: Default::default(), + }, + }); + (this, fake_server) + } + fn start_language_server( http: Arc, node_runtime: Arc, @@ -617,14 +630,10 @@ impl Copilot { ) { if let CopilotServer::Started { status, .. } = &mut self.server { *status = match lsp_status { - request::SignInStatus::Ok { user } - | request::SignInStatus::MaybeOk { user } - | request::SignInStatus::AlreadySignedIn { user } => { - SignInStatus::Authorized { _user: user } - } - request::SignInStatus::NotAuthorized { user } => { - SignInStatus::Unauthorized { _user: user } - } + request::SignInStatus::Ok { .. } + | request::SignInStatus::MaybeOk { .. } + | request::SignInStatus::AlreadySignedIn { .. } => SignInStatus::Authorized, + request::SignInStatus::NotAuthorized { .. } => SignInStatus::Unauthorized, request::SignInStatus::NotSignedIn => SignInStatus::SignedOut, }; cx.notify(); diff --git a/crates/copilot/src/request.rs b/crates/copilot/src/request.rs index e946c8f5cc..415f160ea3 100644 --- a/crates/copilot/src/request.rs +++ b/crates/copilot/src/request.rs @@ -117,7 +117,7 @@ pub struct GetCompletionsResult { pub completions: Vec, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Completion { pub text: String, diff --git a/crates/editor/Cargo.toml b/crates/editor/Cargo.toml index ef2489d7ec..82b7082576 100644 --- a/crates/editor/Cargo.toml +++ b/crates/editor/Cargo.toml @@ -11,6 +11,7 @@ doctest = false [features] test-support = [ "rand", + "copilot/test-support", "text/test-support", "language/test-support", "gpui/test-support", @@ -65,6 +66,7 @@ tree-sitter-javascript = { version = "*", optional = true } tree-sitter-typescript = { git = "https://github.com/tree-sitter/tree-sitter-typescript", rev = "5d20856f34315b068c41edaee2ac8a100081d259", optional = true } [dev-dependencies] +copilot = { path = "../copilot", features = ["test-support"] } text = { path = "../text", features = ["test-support"] } language = { path = "../language", features = ["test-support"] } lsp = { path = "../lsp", features = ["test-support"] } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 84d18bafe8..dd32d7420c 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -95,6 +95,7 @@ const CURSOR_BLINK_INTERVAL: Duration = Duration::from_millis(500); const MAX_LINE_LEN: usize = 1024; const MIN_NAVIGATION_HISTORY_ROW_DELTA: i64 = 10; const MAX_SELECTION_HISTORY_LEN: usize = 1024; +const COPILOT_DEBOUNCE_TIMEOUT: Duration = Duration::from_millis(75); pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); @@ -2799,7 +2800,7 @@ impl Editor { let (buffer, buffer_position) = self.buffer.read(cx).text_anchor_for_position(cursor, cx)?; self.copilot_state.pending_refresh = cx.spawn_weak(|this, mut cx| async move { - cx.background().timer(Duration::from_millis(75)).await; + cx.background().timer(COPILOT_DEBOUNCE_TIMEOUT).await; let (completion, completions_cycling) = copilot.update(&mut cx, |copilot, cx| { ( copilot.completions(&buffer, buffer_position, cx), diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 292c24f79a..7f7f7ec7ed 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -16,7 +16,7 @@ use language::{BracketPairConfig, FakeLspAdapter, LanguageConfig, LanguageRegist use parking_lot::Mutex; use project::FakeFs; use settings::EditorSettings; -use std::{cell::RefCell, rc::Rc, time::Instant}; +use std::{cell::RefCell, future::Future, rc::Rc, time::Instant}; use unindent::Unindent; use util::{ assert_set_eq, @@ -4585,81 +4585,6 @@ async fn test_completion(cx: &mut gpui::TestAppContext) { cx.assert_editor_state("editor.closeˇ"); handle_resolve_completion_request(&mut cx, None).await; apply_additional_edits.await.unwrap(); - - // Handle completion request passing a marked string specifying where the completion - // should be triggered from using '|' character, what range should be replaced, and what completions - // should be returned using '<' and '>' to delimit the range - async fn handle_completion_request<'a>( - cx: &mut EditorLspTestContext<'a>, - marked_string: &str, - completions: Vec<&'static str>, - ) { - let complete_from_marker: TextRangeMarker = '|'.into(); - let replace_range_marker: TextRangeMarker = ('<', '>').into(); - let (_, mut marked_ranges) = marked_text_ranges_by( - marked_string, - vec![complete_from_marker.clone(), replace_range_marker.clone()], - ); - - let complete_from_position = - cx.to_lsp(marked_ranges.remove(&complete_from_marker).unwrap()[0].start); - let replace_range = - cx.to_lsp_range(marked_ranges.remove(&replace_range_marker).unwrap()[0].clone()); - - cx.handle_request::(move |url, params, _| { - let completions = completions.clone(); - async move { - assert_eq!(params.text_document_position.text_document.uri, url.clone()); - assert_eq!( - params.text_document_position.position, - complete_from_position - ); - Ok(Some(lsp::CompletionResponse::Array( - completions - .iter() - .map(|completion_text| lsp::CompletionItem { - label: completion_text.to_string(), - text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: replace_range, - new_text: completion_text.to_string(), - })), - ..Default::default() - }) - .collect(), - ))) - } - }) - .next() - .await; - } - - async fn handle_resolve_completion_request<'a>( - cx: &mut EditorLspTestContext<'a>, - edits: Option>, - ) { - 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 edits = edits.clone(); - async move { - Ok(lsp::CompletionItem { - additional_text_edits: edits, - ..Default::default() - }) - } - }) - .next() - .await; - } } #[gpui::test] @@ -5956,6 +5881,160 @@ async fn test_move_to_enclosing_bracket(cx: &mut gpui::TestAppContext) { ); } +#[gpui::test] +async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppContext) { + let (copilot, copilot_lsp) = Copilot::fake(cx); + cx.update(|cx| cx.set_global(copilot)); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string(), ":".to_string()]), + ..Default::default() + }), + ..Default::default() + }, + cx, + ) + .await; + + cx.set_state(indoc! {" + oneˇ + two + three + "}); + + // When inserting, ensure autocompletion is favored over Copilot suggestions. + cx.simulate_keystroke("."); + let _ = handle_completion_request( + &mut cx, + indoc! {" + one.|<> + two + three + "}, + vec!["completion_a", "completion_b"], + ); + handle_copilot_completion_request( + &copilot_lsp, + vec![copilot::request::Completion { + text: "copilot1".into(), + position: lsp::Position::new(0, 5), + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 5)), + ..Default::default() + }], + vec![], + ); + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(editor.context_menu_visible()); + assert!(!editor.has_active_copilot_suggestion(cx)); + + // Confirming a completion inserts it and hides the context menu, without showing + // the copilot suggestion afterwards. + editor + .confirm_completion(&Default::default(), cx) + .unwrap() + .detach(); + assert!(!editor.context_menu_visible()); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.text(cx), "one.completion_a\ntwo\nthree\n"); + assert_eq!(editor.display_text(cx), "one.completion_a\ntwo\nthree\n"); + }); + + cx.set_state(indoc! {" + oneˇ + two + three + "}); + + // When inserting, ensure autocompletion is favored over Copilot suggestions. + cx.simulate_keystroke("."); + let _ = handle_completion_request( + &mut cx, + indoc! {" + one.|<> + two + three + "}, + vec!["completion_a", "completion_b"], + ); + handle_copilot_completion_request( + &copilot_lsp, + vec![copilot::request::Completion { + text: "one.copilot1".into(), + position: lsp::Position::new(0, 4), + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 4)), + ..Default::default() + }], + vec![], + ); + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(editor.context_menu_visible()); + assert!(!editor.has_active_copilot_suggestion(cx)); + + // When hiding the context menu, the Copilot suggestion becomes visible. + editor.hide_context_menu(cx); + assert!(!editor.context_menu_visible()); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot1\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.\ntwo\nthree\n"); + }); + + // Ensure existing completion is interpolated when inserting again. + cx.simulate_keystroke("c"); + deterministic.run_until_parked(); + cx.update_editor(|editor, cx| { + assert!(!editor.context_menu_visible()); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot1\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + }); + + // After debouncing, new Copilot completions should be requested. + handle_copilot_completion_request( + &copilot_lsp, + vec![copilot::request::Completion { + text: "one.copilot2".into(), + position: lsp::Position::new(0, 5), + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 5)), + ..Default::default() + }], + vec![], + ); + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(!editor.context_menu_visible()); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + + // Canceling should remove the active Copilot suggestion. + editor.cancel(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.c\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + + // After canceling, tabbing shouldn't insert the previously shown suggestion. + editor.tab(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.c \ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c \ntwo\nthree\n"); + + // When undoing the previously active suggestion is shown again. + editor.undo(&Default::default(), cx); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + + // Tabbing when there is an active suggestion inserts it. + editor.tab(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.copilot2\ntwo\nthree\n"); + }); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(row as u32, column as u32); point..point @@ -5971,3 +6050,106 @@ fn assert_selection_ranges(marked_text: &str, view: &mut Editor, cx: &mut ViewCo marked_text ); } + +/// Handle completion request passing a marked string specifying where the completion +/// should be triggered from using '|' character, what range should be replaced, and what completions +/// should be returned using '<' and '>' to delimit the range +fn handle_completion_request<'a>( + cx: &mut EditorLspTestContext<'a>, + marked_string: &str, + completions: Vec<&'static str>, +) -> impl Future { + let complete_from_marker: TextRangeMarker = '|'.into(); + let replace_range_marker: TextRangeMarker = ('<', '>').into(); + let (_, mut marked_ranges) = marked_text_ranges_by( + marked_string, + vec![complete_from_marker.clone(), replace_range_marker.clone()], + ); + + let complete_from_position = + cx.to_lsp(marked_ranges.remove(&complete_from_marker).unwrap()[0].start); + let replace_range = + cx.to_lsp_range(marked_ranges.remove(&replace_range_marker).unwrap()[0].clone()); + + let mut request = cx.handle_request::(move |url, params, _| { + let completions = completions.clone(); + async move { + assert_eq!(params.text_document_position.text_document.uri, url.clone()); + assert_eq!( + params.text_document_position.position, + complete_from_position + ); + Ok(Some(lsp::CompletionResponse::Array( + completions + .iter() + .map(|completion_text| lsp::CompletionItem { + label: completion_text.to_string(), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: replace_range, + new_text: completion_text.to_string(), + })), + ..Default::default() + }) + .collect(), + ))) + } + }); + + async move { + request.next().await; + } +} + +fn handle_resolve_completion_request<'a>( + cx: &mut EditorLspTestContext<'a>, + edits: Option>, +) -> impl Future { + 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::>() + }); + + let mut request = + cx.handle_request::(move |_, _, _| { + let edits = edits.clone(); + async move { + Ok(lsp::CompletionItem { + additional_text_edits: edits, + ..Default::default() + }) + } + }); + + async move { + request.next().await; + } +} + +fn handle_copilot_completion_request( + lsp: &lsp::FakeLanguageServer, + completions: Vec, + completions_cycling: Vec, +) { + lsp.handle_request::(move |_params, _cx| { + let completions = completions.clone(); + async move { + Ok(copilot::request::GetCompletionsResult { + completions: completions.clone(), + }) + } + }); + lsp.handle_request::(move |_params, _cx| { + let completions_cycling = completions_cycling.clone(); + async move { + Ok(copilot::request::GetCompletionsResult { + completions: completions_cycling.clone(), + }) + } + }); +} From 7a7dc956113b05c68f084f9c6308f0f254187204 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 10:27:19 +0200 Subject: [PATCH 2/9] Refresh copilot suggestions when undoing/redoing --- crates/editor/src/editor.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index dd32d7420c..041f0b50c6 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3991,6 +3991,7 @@ impl Editor { } self.request_autoscroll(Autoscroll::fit(), cx); self.unmark_text(cx); + self.refresh_copilot_suggestions(cx); cx.emit(Event::Edited); } } @@ -4005,6 +4006,7 @@ impl Editor { } self.request_autoscroll(Autoscroll::fit(), cx); self.unmark_text(cx); + self.refresh_copilot_suggestions(cx); cx.emit(Event::Edited); } } From 661be7ba51bc296977894427856942a37cfb6dfc Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 10:28:00 +0200 Subject: [PATCH 3/9] Refresh copilot suggestions when accepting a completion --- crates/editor/src/editor.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 041f0b50c6..75de823551 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2489,6 +2489,8 @@ impl Editor { ); }); } + + this.refresh_copilot_suggestions(cx); }); let project = self.project.clone()?; From 6e821eea4b1f8453a49e94bc4a9883c766a5c919 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 10:31:11 +0200 Subject: [PATCH 4/9] Prevent `tab` from accepting a copilot suggestion when it isn't visible --- crates/editor/src/display_map.rs | 9 +++++--- .../editor/src/display_map/suggestion_map.rs | 17 ++++++++++----- crates/editor/src/editor.rs | 21 +++++++------------ 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index c5e6b371ed..f54e2e980c 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -7,7 +7,7 @@ mod wrap_map; use crate::{Anchor, AnchorRangeExt, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint}; pub use block_map::{BlockMap, BlockPoint}; use collections::{HashMap, HashSet}; -use fold_map::FoldMap; +use fold_map::{FoldMap, FoldOffset}; use gpui::{ color::Color, fonts::{FontId, HighlightStyle}, @@ -238,19 +238,22 @@ impl DisplayMap { &self, new_suggestion: Option>, cx: &mut ModelContext, - ) where + ) -> Option> + where T: ToPoint, { let snapshot = self.buffer.read(cx).snapshot(cx); let edits = self.buffer_subscription.consume().into_inner(); let tab_size = Self::tab_size(&self.buffer, cx); let (snapshot, edits) = self.fold_map.read(snapshot, edits); - let (snapshot, edits) = self.suggestion_map.replace(new_suggestion, snapshot, edits); + let (snapshot, edits, old_suggestion) = + self.suggestion_map.replace(new_suggestion, snapshot, edits); let (snapshot, edits) = self.tab_map.sync(snapshot, edits, tab_size); let (snapshot, edits) = self .wrap_map .update(cx, |map, cx| map.sync(snapshot, edits, cx)); self.block_map.read(snapshot, edits); + old_suggestion } pub fn set_font(&self, font_id: FontId, font_size: f32, cx: &mut ModelContext) -> bool { diff --git a/crates/editor/src/display_map/suggestion_map.rs b/crates/editor/src/display_map/suggestion_map.rs index ccc07bf591..17dbae6031 100644 --- a/crates/editor/src/display_map/suggestion_map.rs +++ b/crates/editor/src/display_map/suggestion_map.rs @@ -79,7 +79,11 @@ impl SuggestionMap { new_suggestion: Option>, fold_snapshot: FoldSnapshot, fold_edits: Vec, - ) -> (SuggestionSnapshot, Vec) + ) -> ( + SuggestionSnapshot, + Vec, + Option>, + ) where T: ToPoint, { @@ -99,7 +103,8 @@ impl SuggestionMap { let mut snapshot = self.0.lock(); let mut patch = Patch::new(edits); - if let Some(suggestion) = snapshot.suggestion.take() { + let old_suggestion = snapshot.suggestion.take(); + if let Some(suggestion) = &old_suggestion { patch = patch.compose([SuggestionEdit { old: SuggestionOffset(suggestion.position.0) ..SuggestionOffset(suggestion.position.0 + suggestion.text.len()), @@ -119,7 +124,7 @@ impl SuggestionMap { snapshot.suggestion = new_suggestion; snapshot.version += 1; - (snapshot.clone(), patch.into_inner()) + (snapshot.clone(), patch.into_inner(), old_suggestion) } pub fn sync( @@ -589,7 +594,7 @@ mod tests { let (suggestion_map, suggestion_snapshot) = SuggestionMap::new(fold_snapshot.clone()); assert_eq!(suggestion_snapshot.text(), "abcdefghi"); - let (suggestion_snapshot, _) = suggestion_map.replace( + let (suggestion_snapshot, _, _) = suggestion_map.replace( Some(Suggestion { position: 3, text: "123\n456".into(), @@ -854,7 +859,9 @@ mod tests { }; log::info!("replacing suggestion with {:?}", new_suggestion); - self.replace(new_suggestion, fold_snapshot, Default::default()) + let (snapshot, edits, _) = + self.replace(new_suggestion, fold_snapshot, Default::default()); + (snapshot, edits) } } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 75de823551..d9b2b1a8db 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -53,7 +53,7 @@ pub use language::{char_kind, CharKind}; use language::{ AutoindentMode, BracketPair, Buffer, CodeAction, CodeLabel, Completion, CursorShape, Diagnostic, DiagnosticSeverity, IndentKind, IndentSize, Language, OffsetRangeExt, OffsetUtf16, - Point, Selection, SelectionGoal, TransactionId, + Point, Rope, Selection, SelectionGoal, TransactionId, }; use link_go_to_definition::{ hide_link_definition, show_link_definition, LinkDefinitionKind, LinkGoToDefinitionState, @@ -1834,7 +1834,7 @@ impl Editor { return; } - if self.hide_copilot_suggestion(cx) { + if self.hide_copilot_suggestion(cx).is_some() { return; } @@ -2865,14 +2865,8 @@ impl Editor { } fn accept_copilot_suggestion(&mut self, cx: &mut ViewContext) -> bool { - let snapshot = self.buffer.read(cx).snapshot(cx); - let cursor = self.selections.newest_anchor().head(); - if let Some(text) = self - .copilot_state - .text_for_active_completion(cursor, &snapshot) - { + if let Some(text) = self.hide_copilot_suggestion(cx) { self.insert_with_autoindent_mode(&text.to_string(), None, cx); - self.hide_copilot_suggestion(cx); true } else { false @@ -2883,14 +2877,15 @@ impl Editor { self.display_map.read(cx).has_suggestion() } - fn hide_copilot_suggestion(&mut self, cx: &mut ViewContext) -> bool { + fn hide_copilot_suggestion(&mut self, cx: &mut ViewContext) -> Option { if self.has_active_copilot_suggestion(cx) { - self.display_map + let old_suggestion = self + .display_map .update(cx, |map, cx| map.replace_suggestion::(None, cx)); cx.notify(); - true + old_suggestion.map(|suggestion| suggestion.text) } else { - false + None } } From dcd8bdfc882d68ca17f5d36e1baef8c84c2c192f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 11:00:08 +0200 Subject: [PATCH 5/9] Update visible suggestion if edit occurs outside the current editor --- crates/editor/src/editor.rs | 3 +++ crates/editor/src/editor_tests.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d9b2b1a8db..0474371af6 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6411,6 +6411,9 @@ impl Editor { multi_buffer::Event::Edited => { self.refresh_active_diagnostics(cx); self.refresh_code_actions(cx); + if self.has_active_copilot_suggestion(cx) { + self.update_visible_copilot_suggestion(cx); + } cx.emit(Event::BufferEdited); } multi_buffer::Event::ExcerptsAdded { diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 7f7f7ec7ed..8c882f3ea8 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -6026,12 +6026,41 @@ async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppC assert!(editor.has_active_copilot_suggestion(cx)); assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + }); + + // If an edit occurs outside of this editor, the suggestion is still correctly interpolated. + cx.update_buffer(|buffer, cx| buffer.edit([(5..5, "o")], None, cx)); + cx.update_editor(|editor, cx| { + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.co\ntwo\nthree\n"); // Tabbing when there is an active suggestion inserts it. editor.tab(&Default::default(), cx); assert!(!editor.has_active_copilot_suggestion(cx)); assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); assert_eq!(editor.text(cx), "one.copilot2\ntwo\nthree\n"); + + // When undoing the previously active suggestion is shown again. + editor.undo(&Default::default(), cx); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.co\ntwo\nthree\n"); + + // Hide suggestion. + editor.cancel(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.co\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.co\ntwo\nthree\n"); + }); + + // If an edit occurs outside of this editor but no suggestion is being shown, + // we won't make it visible. + cx.update_buffer(|buffer, cx| buffer.edit([(6..6, "p")], None, cx)); + cx.update_editor(|editor, cx| { + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.cop\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.cop\ntwo\nthree\n"); }); } From 908a7cf47efd93fad1bffe4dc903b63a3d5a49a5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 13:59:19 +0200 Subject: [PATCH 6/9] :lipstick: --- crates/editor/src/editor.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 0474371af6..c19c98e448 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2833,14 +2833,13 @@ impl Editor { } fn next_copilot_suggestion(&mut self, _: &copilot::NextSuggestion, cx: &mut ViewContext) { - if self.copilot_state.completions.is_empty() { + if !self.has_active_copilot_suggestion(cx) { self.refresh_copilot_suggestions(cx); return; } self.copilot_state.active_completion_index = (self.copilot_state.active_completion_index + 1) % self.copilot_state.completions.len(); - self.update_visible_copilot_suggestion(cx); } @@ -2849,7 +2848,7 @@ impl Editor { _: &copilot::PreviousSuggestion, cx: &mut ViewContext, ) { - if self.copilot_state.completions.is_empty() { + if !self.has_active_copilot_suggestion(cx) { self.refresh_copilot_suggestions(cx); return; } @@ -2860,7 +2859,6 @@ impl Editor { } else { self.copilot_state.active_completion_index - 1 }; - self.update_visible_copilot_suggestion(cx); } From f920e02d965dedd52f270f273b8b67214aa8f98c Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 13:59:41 +0200 Subject: [PATCH 7/9] Indent instead of accepting suggestion if cursor is in leading whitespace --- crates/editor/src/editor.rs | 19 +++++++++----- crates/editor/src/editor_tests.rs | 42 ++++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c19c98e448..248e6fed95 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3230,10 +3230,6 @@ impl Editor { } pub fn tab(&mut self, _: &Tab, cx: &mut ViewContext) { - if self.accept_copilot_suggestion(cx) { - return; - } - if self.move_to_next_snippet_tabstop(cx) { return; } @@ -3263,8 +3259,8 @@ impl Editor { // If the selection is empty and the cursor is in the leading whitespace before the // suggested indentation, then auto-indent the line. let cursor = selection.head(); + let current_indent = snapshot.indent_size_for_line(cursor.row); if let Some(suggested_indent) = suggested_indents.get(&cursor.row).copied() { - let current_indent = snapshot.indent_size_for_line(cursor.row); if cursor.column < suggested_indent.len && cursor.column <= current_indent.len && current_indent.len <= suggested_indent.len @@ -3283,6 +3279,16 @@ impl Editor { } } + // Accept copilot suggestion if there is only one selection and the cursor is + // in the leading whitespace. + if self.selections.count() == 1 + && selection.start.column >= current_indent.len + && self.has_active_copilot_suggestion(cx) + { + self.accept_copilot_suggestion(cx); + return; + } + // Otherwise, insert a hard or soft tab. let settings = cx.global::(); let language_name = buffer.language_at(cursor, cx).map(|l| l.name()); @@ -3306,7 +3312,8 @@ impl Editor { self.transact(cx, |this, cx| { this.buffer.update(cx, |b, cx| b.edit(edits, None, cx)); - this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(selections)) + this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(selections)); + this.refresh_copilot_suggestions(cx); }); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 8c882f3ea8..6220ad3354 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -5881,7 +5881,7 @@ async fn test_move_to_enclosing_bracket(cx: &mut gpui::TestAppContext) { ); } -#[gpui::test] +#[gpui::test(iterations = 10)] async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppContext) { let (copilot, copilot_lsp) = Copilot::fake(cx); cx.update(|cx| cx.set_global(copilot)); @@ -5918,7 +5918,6 @@ async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppC &copilot_lsp, vec![copilot::request::Completion { text: "copilot1".into(), - position: lsp::Position::new(0, 5), range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 5)), ..Default::default() }], @@ -5962,7 +5961,6 @@ async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppC &copilot_lsp, vec![copilot::request::Completion { text: "one.copilot1".into(), - position: lsp::Position::new(0, 4), range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 4)), ..Default::default() }], @@ -5996,7 +5994,6 @@ async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppC &copilot_lsp, vec![copilot::request::Completion { text: "one.copilot2".into(), - position: lsp::Position::new(0, 5), range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 5)), ..Default::default() }], @@ -6062,6 +6059,43 @@ async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppC assert_eq!(editor.display_text(cx), "one.cop\ntwo\nthree\n"); assert_eq!(editor.text(cx), "one.cop\ntwo\nthree\n"); }); + + // Reset the editor to verify how suggestions behave when tabbing on leading indentation. + cx.update_editor(|editor, cx| { + editor.set_text("fn foo() {\n \n}", cx); + editor.change_selections(None, cx, |s| { + s.select_ranges([Point::new(1, 2)..Point::new(1, 2)]) + }); + }); + handle_copilot_completion_request( + &copilot_lsp, + vec![copilot::request::Completion { + text: " let x = 4;".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_copilot_suggestion(&Default::default(), cx)); + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "fn foo() {\n let x = 4;\n}"); + assert_eq!(editor.text(cx), "fn foo() {\n \n}"); + + // Tabbing inside of leading whitespace inserts indentation without accepting the suggestion. + editor.tab(&Default::default(), cx); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.text(cx), "fn foo() {\n \n}"); + assert_eq!(editor.display_text(cx), "fn foo() {\n let x = 4;\n}"); + + // Tabbing again accepts the suggestion. + editor.tab(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.text(cx), "fn foo() {\n let x = 4;\n}"); + assert_eq!(editor.display_text(cx), "fn foo() {\n let x = 4;\n}"); + }); } fn empty_range(row: usize, column: usize) -> Range { From dd416cdfd28ccef1b2543ff7ed3c4d7e1653b28e Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 14:14:43 +0200 Subject: [PATCH 8/9] :memo: --- crates/editor/src/editor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 248e6fed95..b2232d8df2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3279,7 +3279,7 @@ impl Editor { } } - // Accept copilot suggestion if there is only one selection and the cursor is + // Accept copilot suggestion if there is only one selection and the cursor is not // in the leading whitespace. if self.selections.count() == 1 && selection.start.column >= current_indent.len From 03a4c9d6d52251fe3bb9a2624984aa00ba7381bd Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 14:15:58 +0200 Subject: [PATCH 9/9] Use the cursor variable instead of selection.start --- crates/editor/src/editor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b2232d8df2..f28c82407a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3282,7 +3282,7 @@ impl Editor { // Accept copilot suggestion if there is only one selection and the cursor is not // in the leading whitespace. if self.selections.count() == 1 - && selection.start.column >= current_indent.len + && cursor.column >= current_indent.len && self.has_active_copilot_suggestion(cx) { self.accept_copilot_suggestion(cx);