From 971c88db809f448cada0fcdfea53051fedac84f3 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 4 Apr 2023 17:14:52 +0200 Subject: [PATCH 1/3] Return the previous suggestion when replacing it --- crates/editor/src/display_map.rs | 9 ++++++--- crates/editor/src/display_map/suggestion_map.rs | 17 ++++++++++++----- 2 files changed, 18 insertions(+), 8 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) } } } From b58ac815a8eb39ff8d1ca8dd73fcbaf32a1725ea Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 4 Apr 2023 17:21:45 +0200 Subject: [PATCH 2/3] Trigger copilot only on insertion and deletion Also, avoid showing the suggestion if a completion is in progress to avoid flickering. --- crates/editor/src/editor.rs | 124 ++++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 56 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 7a03c44af5..7431c86815 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1459,7 +1459,7 @@ impl Editor { self.refresh_code_actions(cx); self.refresh_document_highlights(cx); refresh_matching_bracket_highlights(self, cx); - self.refresh_copilot_suggestions(cx); + self.hide_copilot_suggestion(cx); } self.blink_manager.update(cx, BlinkManager::pause_blinking); @@ -1833,7 +1833,7 @@ impl Editor { return; } - if self.clear_copilot_suggestions(cx) { + if self.hide_copilot_suggestion(cx) { return; } @@ -2012,8 +2012,18 @@ impl Editor { } drop(snapshot); + let had_active_copilot_suggestion = this.has_active_copilot_suggestion(cx); this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections)); - this.trigger_completion_on_input(&text, cx); + + if had_active_copilot_suggestion { + this.refresh_copilot_suggestions(cx); + if !this.has_active_copilot_suggestion(cx) { + this.trigger_completion_on_input(&text, cx); + } + } else { + this.trigger_completion_on_input(&text, cx); + this.refresh_copilot_suggestions(cx); + } }); } @@ -2094,6 +2104,7 @@ impl Editor { .collect(); this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections)); + this.refresh_copilot_suggestions(cx); }); } @@ -2186,9 +2197,7 @@ impl Editor { } fn trigger_completion_on_input(&mut self, text: &str, cx: &mut ViewContext) { - if !cx.global::().show_completions_on_input - || self.has_active_copilot_suggestion(cx) - { + if !cx.global::().show_completions_on_input { return; } @@ -2375,8 +2384,8 @@ impl Editor { this.completion_tasks.retain(|(id, _)| *id > menu.id); if this.focused && !menu.matches.is_empty() { this.show_context_menu(ContextMenu::Completions(menu), cx); - } else { - this.hide_context_menu(cx); + } else if this.hide_context_menu(cx).is_none() { + this.update_visible_copilot_suggestion(cx); } }); } @@ -2774,16 +2783,16 @@ impl Editor { fn refresh_copilot_suggestions(&mut self, cx: &mut ViewContext) -> Option<()> { let copilot = Copilot::global(cx)?; if self.mode != EditorMode::Full || !copilot.read(cx).status().is_authorized() { - self.clear_copilot_suggestions(cx); + self.hide_copilot_suggestion(cx); return None; } - self.refresh_active_copilot_suggestion(cx); + self.update_visible_copilot_suggestion(cx); let snapshot = self.buffer.read(cx).snapshot(cx); let cursor = self.selections.newest_anchor().head(); let language_name = snapshot.language_at(cursor).map(|language| language.name()); if !cx.global::().copilot_on(language_name.as_deref()) { - self.clear_copilot_suggestions(cx); + self.hide_copilot_suggestion(cx); return None; } @@ -2810,7 +2819,7 @@ impl Editor { for completion in completions { this.copilot_state.push_completion(completion); } - this.refresh_active_copilot_suggestion(cx); + this.update_visible_copilot_suggestion(cx); } }); @@ -2829,7 +2838,7 @@ impl Editor { self.copilot_state.active_completion_index = (self.copilot_state.active_completion_index + 1) % self.copilot_state.completions.len(); - self.refresh_active_copilot_suggestion(cx); + self.update_visible_copilot_suggestion(cx); } fn previous_copilot_suggestion( @@ -2849,18 +2858,51 @@ impl Editor { self.copilot_state.active_completion_index - 1 }; - self.refresh_active_copilot_suggestion(cx); + self.update_visible_copilot_suggestion(cx); } - fn refresh_active_copilot_suggestion(&mut self, cx: &mut ViewContext) { + 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) + { + self.insert_with_autoindent_mode(&text.to_string(), None, cx); + self.hide_copilot_suggestion(cx); + true + } else { + false + } + } + + fn has_active_copilot_suggestion(&self, cx: &AppContext) -> bool { + self.display_map.read(cx).has_suggestion() + } + + fn hide_copilot_suggestion(&mut self, cx: &mut ViewContext) -> bool { + let old_suggestion = self + .display_map + .update(cx, |map, cx| map.replace_suggestion::(None, cx)); + + if old_suggestion.is_some() { + cx.notify(); + true + } else { + false + } + } + + fn update_visible_copilot_suggestion(&mut self, cx: &mut ViewContext) { let snapshot = self.buffer.read(cx).snapshot(cx); let selection = self.selections.newest_anchor(); let cursor = selection.head(); - if self.context_menu.is_some() || selection.start != selection.end { - self.display_map - .update(cx, |map, cx| map.replace_suggestion::(None, cx)); - cx.notify(); + if self.context_menu.is_some() + || !self.completion_tasks.is_empty() + || selection.start != selection.end + { + self.hide_copilot_suggestion(cx); } else if let Some(text) = self .copilot_state .text_for_active_completion(cursor, &snapshot) @@ -2875,44 +2917,11 @@ impl Editor { ) }); cx.notify(); - } else if self.has_active_copilot_suggestion(cx) { - self.display_map - .update(cx, |map, cx| map.replace_suggestion::(None, cx)); - cx.notify(); - } - } - - 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) - { - self.insert_with_autoindent_mode(&text.to_string(), None, cx); - self.clear_copilot_suggestions(cx); - true } else { - false + self.hide_copilot_suggestion(cx); } } - fn clear_copilot_suggestions(&mut self, cx: &mut ViewContext) -> bool { - self.display_map - .update(cx, |map, cx| map.replace_suggestion::(None, cx)); - let was_empty = self.copilot_state.completions.is_empty(); - self.copilot_state.completions.clear(); - self.copilot_state.active_completion_index = 0; - self.copilot_state.pending_refresh = Task::ready(None); - self.copilot_state.excerpt_id = None; - cx.notify(); - !was_empty - } - - fn has_active_copilot_suggestion(&self, cx: &AppContext) -> bool { - self.display_map.read(cx).has_suggestion() - } - pub fn render_code_actions_indicator( &self, style: &EditorStyle, @@ -3028,7 +3037,7 @@ impl Editor { self.completion_tasks.clear(); } self.context_menu = Some(menu); - self.refresh_active_copilot_suggestion(cx); + self.hide_copilot_suggestion(cx); cx.notify(); } @@ -3036,7 +3045,9 @@ impl Editor { cx.notify(); self.completion_tasks.clear(); let context_menu = self.context_menu.take(); - self.refresh_active_copilot_suggestion(cx); + if context_menu.is_some() { + self.update_visible_copilot_suggestion(cx); + } context_menu } @@ -3196,6 +3207,7 @@ impl Editor { this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(selections)); this.insert("", cx); + this.refresh_copilot_suggestions(cx); }); } @@ -3211,6 +3223,7 @@ impl Editor { }) }); this.insert("", cx); + this.refresh_copilot_suggestions(cx); }); } @@ -6400,7 +6413,6 @@ impl Editor { multi_buffer::Event::Edited => { self.refresh_active_diagnostics(cx); self.refresh_code_actions(cx); - self.refresh_copilot_suggestions(cx); cx.emit(Event::BufferEdited); } multi_buffer::Event::ExcerptsAdded { From 65fd605b828ae4c68abb1a9846f225d1d209e6e2 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 4 Apr 2023 18:59:21 +0200 Subject: [PATCH 3/3] Revert "Return the previous suggestion when replacing it" This reverts commit 971c88db809f448cada0fcdfea53051fedac84f3. --- crates/editor/src/display_map.rs | 9 +++------ crates/editor/src/display_map/suggestion_map.rs | 17 +++++------------ crates/editor/src/editor.rs | 8 +++----- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index f54e2e980c..c5e6b371ed 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, FoldOffset}; +use fold_map::FoldMap; use gpui::{ color::Color, fonts::{FontId, HighlightStyle}, @@ -238,22 +238,19 @@ impl DisplayMap { &self, new_suggestion: Option>, cx: &mut ModelContext, - ) -> Option> - where + ) 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, old_suggestion) = - self.suggestion_map.replace(new_suggestion, snapshot, edits); + let (snapshot, edits) = 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 17dbae6031..ccc07bf591 100644 --- a/crates/editor/src/display_map/suggestion_map.rs +++ b/crates/editor/src/display_map/suggestion_map.rs @@ -79,11 +79,7 @@ impl SuggestionMap { new_suggestion: Option>, fold_snapshot: FoldSnapshot, fold_edits: Vec, - ) -> ( - SuggestionSnapshot, - Vec, - Option>, - ) + ) -> (SuggestionSnapshot, Vec) where T: ToPoint, { @@ -103,8 +99,7 @@ impl SuggestionMap { let mut snapshot = self.0.lock(); let mut patch = Patch::new(edits); - let old_suggestion = snapshot.suggestion.take(); - if let Some(suggestion) = &old_suggestion { + if let Some(suggestion) = snapshot.suggestion.take() { patch = patch.compose([SuggestionEdit { old: SuggestionOffset(suggestion.position.0) ..SuggestionOffset(suggestion.position.0 + suggestion.text.len()), @@ -124,7 +119,7 @@ impl SuggestionMap { snapshot.suggestion = new_suggestion; snapshot.version += 1; - (snapshot.clone(), patch.into_inner(), old_suggestion) + (snapshot.clone(), patch.into_inner()) } pub fn sync( @@ -594,7 +589,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(), @@ -859,9 +854,7 @@ mod tests { }; log::info!("replacing suggestion with {:?}", new_suggestion); - let (snapshot, edits, _) = - self.replace(new_suggestion, fold_snapshot, Default::default()); - (snapshot, edits) + self.replace(new_suggestion, fold_snapshot, Default::default()) } } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 7431c86815..84d18bafe8 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2881,11 +2881,9 @@ impl Editor { } fn hide_copilot_suggestion(&mut self, cx: &mut ViewContext) -> bool { - let old_suggestion = self - .display_map - .update(cx, |map, cx| map.replace_suggestion::(None, cx)); - - if old_suggestion.is_some() { + if self.has_active_copilot_suggestion(cx) { + self.display_map + .update(cx, |map, cx| map.replace_suggestion::(None, cx)); cx.notify(); true } else {