Reduce spamming of inline completion discard events (#11999)

Joseph T. Lyons created 2 years ago

I'm not a huge fan of passing around a boolean all around the place, but
this will tame the events for now until we have a better solution.

Release Notes:

- N/A

Change summary

crates/copilot/src/copilot_completion_provider.rs       | 33 +++++-----
crates/editor/src/editor.rs                             | 28 ++++++---
crates/editor/src/inline_completion_provider.rs         | 10 ++-
crates/supermaven/src/supermaven_completion_provider.rs | 26 +++++---
crates/vim/src/insert.rs                                |  2 
5 files changed, 57 insertions(+), 42 deletions(-)

Detailed changes

crates/copilot/src/copilot_completion_provider.rs

@@ -22,7 +22,6 @@ pub struct CopilotCompletionProvider {
     pending_cycling_refresh: Task<Result<()>>,
     copilot: Model<Copilot>,
     telemetry: Option<Arc<Telemetry>>,
-    should_allow_event_to_send: bool,
 }
 
 impl CopilotCompletionProvider {
@@ -37,7 +36,6 @@ impl CopilotCompletionProvider {
             pending_cycling_refresh: Task::ready(Ok(())),
             copilot,
             telemetry: None,
-            should_allow_event_to_send: false,
         }
     }
 
@@ -105,7 +103,6 @@ impl InlineCompletionProvider for CopilotCompletionProvider {
 
             this.update(&mut cx, |this, cx| {
                 if !completions.is_empty() {
-                    this.should_allow_event_to_send = true;
                     this.cycled = false;
                     this.pending_cycling_refresh = Task::ready(Ok(()));
                     this.completions.clear();
@@ -193,8 +190,8 @@ impl InlineCompletionProvider for CopilotCompletionProvider {
             self.copilot
                 .update(cx, |copilot, cx| copilot.accept_completion(completion, cx))
                 .detach_and_log_err(cx);
-            if let Some(telemetry) = self.telemetry.as_ref() {
-                if self.should_allow_event_to_send {
+            if self.active_completion().is_some() {
+                if let Some(telemetry) = self.telemetry.as_ref() {
                     telemetry.report_inline_completion_event(
                         Self::name().to_string(),
                         true,
@@ -203,11 +200,13 @@ impl InlineCompletionProvider for CopilotCompletionProvider {
                 }
             }
         }
-
-        self.should_allow_event_to_send = false;
     }
 
-    fn discard(&mut self, cx: &mut ModelContext<Self>) {
+    fn discard(
+        &mut self,
+        should_report_inline_completion_event: bool,
+        cx: &mut ModelContext<Self>,
+    ) {
         let settings = AllLanguageSettings::get_global(cx);
 
         let copilot_enabled = settings.inline_completions_enabled(None, None);
@@ -222,17 +221,17 @@ impl InlineCompletionProvider for CopilotCompletionProvider {
             })
             .detach_and_log_err(cx);
 
-        if let Some(telemetry) = self.telemetry.as_ref() {
-            if self.should_allow_event_to_send {
-                telemetry.report_inline_completion_event(
-                    Self::name().to_string(),
-                    false,
-                    self.file_extension.clone(),
-                );
+        if should_report_inline_completion_event {
+            if self.active_completion().is_some() {
+                if let Some(telemetry) = self.telemetry.as_ref() {
+                    telemetry.report_inline_completion_event(
+                        Self::name().to_string(),
+                        false,
+                        self.file_extension.clone(),
+                    );
+                }
             }
         }
-
-        self.should_allow_event_to_send = false;
     }
 
     fn active_completion_text<'a>(

crates/editor/src/editor.rs

@@ -2161,7 +2161,7 @@ impl Editor {
             self.refresh_code_actions(cx);
             self.refresh_document_highlights(cx);
             refresh_matching_bracket_highlights(self, cx);
-            self.discard_inline_completion(cx);
+            self.discard_inline_completion(false, cx);
             if self.git_blame_inline_enabled {
                 self.start_inline_blame_timer(cx);
             }
@@ -2566,7 +2566,7 @@ impl Editor {
 
     pub fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext<Self>) {
         self.clear_expanded_diff_hunks(cx);
-        if self.dismiss_menus_and_popups(cx) {
+        if self.dismiss_menus_and_popups(true, cx) {
             return;
         }
 
@@ -2579,7 +2579,11 @@ impl Editor {
         cx.propagate();
     }
 
-    pub fn dismiss_menus_and_popups(&mut self, cx: &mut ViewContext<Self>) -> bool {
+    pub fn dismiss_menus_and_popups(
+        &mut self,
+        should_report_inline_completion_event: bool,
+        cx: &mut ViewContext<Self>,
+    ) -> bool {
         if self.take_rename(false, cx).is_some() {
             return true;
         }
@@ -2592,7 +2596,7 @@ impl Editor {
             return true;
         }
 
-        if self.discard_inline_completion(cx) {
+        if self.discard_inline_completion(should_report_inline_completion_event, cx) {
             return true;
         }
 
@@ -3704,7 +3708,7 @@ impl Editor {
                         let menu = menu.unwrap();
                         *context_menu = Some(ContextMenu::Completions(menu));
                         drop(context_menu);
-                        this.discard_inline_completion(cx);
+                        this.discard_inline_completion(false, cx);
                         cx.notify();
                     } else if this.completion_tasks.len() <= 1 {
                         // If there are no more completion tasks and the last menu was
@@ -3918,7 +3922,7 @@ impl Editor {
                     }
 
                     this.completion_tasks.clear();
-                    this.discard_inline_completion(cx);
+                    this.discard_inline_completion(false, cx);
                     let task_context = tasks.as_ref().zip(this.workspace.clone()).and_then(
                         |(tasks, (workspace, _))| {
                             let position = Point::new(buffer_row, tasks.1.column);
@@ -4313,7 +4317,7 @@ impl Editor {
         if !self.show_inline_completions
             || !provider.is_enabled(&buffer, cursor_buffer_position, cx)
         {
-            self.discard_inline_completion(cx);
+            self.discard_inline_completion(false, cx);
             return None;
         }
 
@@ -4448,9 +4452,13 @@ impl Editor {
         }
     }
 
-    fn discard_inline_completion(&mut self, cx: &mut ViewContext<Self>) -> bool {
+    fn discard_inline_completion(
+        &mut self,
+        should_report_inline_completion_event: bool,
+        cx: &mut ViewContext<Self>,
+    ) -> bool {
         if let Some(provider) = self.inline_completion_provider() {
-            provider.discard(cx);
+            provider.discard(should_report_inline_completion_event, cx);
         }
 
         self.take_active_inline_completion(cx).is_some()
@@ -4513,7 +4521,7 @@ impl Editor {
             }
         }
 
-        self.discard_inline_completion(cx);
+        self.discard_inline_completion(false, cx);
     }
 
     fn inline_completion_provider(&self) -> Option<Arc<dyn InlineCompletionProviderHandle>> {

crates/editor/src/inline_completion_provider.rs

@@ -25,7 +25,7 @@ pub trait InlineCompletionProvider: 'static + Sized {
         cx: &mut ModelContext<Self>,
     );
     fn accept(&mut self, cx: &mut ModelContext<Self>);
-    fn discard(&mut self, cx: &mut ModelContext<Self>);
+    fn discard(&mut self, should_report_inline_completion_event: bool, cx: &mut ModelContext<Self>);
     fn active_completion_text<'a>(
         &'a self,
         buffer: &Model<Buffer>,
@@ -56,7 +56,7 @@ pub trait InlineCompletionProviderHandle {
         cx: &mut AppContext,
     );
     fn accept(&self, cx: &mut AppContext);
-    fn discard(&self, cx: &mut AppContext);
+    fn discard(&self, should_report_inline_completion_event: bool, cx: &mut AppContext);
     fn active_completion_text<'a>(
         &'a self,
         buffer: &Model<Buffer>,
@@ -106,8 +106,10 @@ where
         self.update(cx, |this, cx| this.accept(cx))
     }
 
-    fn discard(&self, cx: &mut AppContext) {
-        self.update(cx, |this, cx| this.discard(cx))
+    fn discard(&self, should_report_inline_completion_event: bool, cx: &mut AppContext) {
+        self.update(cx, |this, cx| {
+            this.discard(should_report_inline_completion_event, cx)
+        })
     }
 
     fn active_completion_text<'a>(

crates/supermaven/src/supermaven_completion_provider.rs

@@ -100,8 +100,8 @@ impl InlineCompletionProvider for SupermavenCompletionProvider {
     }
 
     fn accept(&mut self, _cx: &mut ModelContext<Self>) {
-        if let Some(telemetry) = self.telemetry.as_ref() {
-            if let Some(_) = self.completion_id {
+        if self.completion_id.is_some() {
+            if let Some(telemetry) = self.telemetry.as_ref() {
                 telemetry.report_inline_completion_event(
                     Self::name().to_string(),
                     true,
@@ -113,14 +113,20 @@ impl InlineCompletionProvider for SupermavenCompletionProvider {
         self.completion_id = None;
     }
 
-    fn discard(&mut self, _cx: &mut ModelContext<Self>) {
-        if let Some(telemetry) = self.telemetry.as_ref() {
-            if let Some(_) = self.completion_id {
-                telemetry.report_inline_completion_event(
-                    Self::name().to_string(),
-                    false,
-                    self.file_extension.clone(),
-                );
+    fn discard(
+        &mut self,
+        should_report_inline_completion_event: bool,
+        _cx: &mut ModelContext<Self>,
+    ) {
+        if should_report_inline_completion_event {
+            if self.completion_id.is_some() {
+                if let Some(telemetry) = self.telemetry.as_ref() {
+                    telemetry.report_inline_completion_event(
+                        Self::name().to_string(),
+                        false,
+                        self.file_extension.clone(),
+                    );
+                }
             }
         }
 

crates/vim/src/insert.rs

@@ -16,7 +16,7 @@ fn normal_before(_: &mut Workspace, action: &NormalBefore, cx: &mut ViewContext<
         vim.stop_recording_immediately(action.boxed_clone());
         if count <= 1 || vim.workspace_state.replaying {
             vim.update_active_editor(cx, |_, editor, cx| {
-                editor.dismiss_menus_and_popups(cx);
+                editor.dismiss_menus_and_popups(false, cx);
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                     s.move_cursors_with(|map, mut cursor, _| {
                         *cursor.column_mut() = cursor.column().saturating_sub(1);