Merge pull request #2383 from zed-industries/show-copilot-more-often

Antonio Scandurra created

Clean up completion tasks, even if they fail or return no results

Change summary

crates/editor/src/editor.rs       | 93 ++++++++++++++++++--------------
crates/editor/src/editor_tests.rs | 41 ++++++++++++-
2 files changed, 89 insertions(+), 45 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -20,7 +20,7 @@ mod editor_tests;
 pub mod test;
 
 use aho_corasick::AhoCorasick;
-use anyhow::Result;
+use anyhow::{anyhow, Result};
 use blink_manager::BlinkManager;
 use clock::ReplicaId;
 use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque};
@@ -2352,53 +2352,66 @@ impl Editor {
         let id = post_inc(&mut self.next_completion_id);
         let task = cx.spawn_weak(|this, mut cx| {
             async move {
-                let completions = completions.await?;
-                if completions.is_empty() {
-                    return Ok(());
-                }
-
-                let mut menu = CompletionsMenu {
-                    id,
-                    initial_position: position,
-                    match_candidates: completions
-                        .iter()
-                        .enumerate()
-                        .map(|(id, completion)| {
-                            StringMatchCandidate::new(
-                                id,
-                                completion.label.text[completion.label.filter_range.clone()].into(),
-                            )
-                        })
-                        .collect(),
-                    buffer,
-                    completions: completions.into(),
-                    matches: Vec::new().into(),
-                    selected_item: 0,
-                    list: Default::default(),
+                let menu = if let Some(completions) = completions.await.log_err() {
+                    let mut menu = CompletionsMenu {
+                        id,
+                        initial_position: position,
+                        match_candidates: completions
+                            .iter()
+                            .enumerate()
+                            .map(|(id, completion)| {
+                                StringMatchCandidate::new(
+                                    id,
+                                    completion.label.text[completion.label.filter_range.clone()]
+                                        .into(),
+                                )
+                            })
+                            .collect(),
+                        buffer,
+                        completions: completions.into(),
+                        matches: Vec::new().into(),
+                        selected_item: 0,
+                        list: Default::default(),
+                    };
+                    menu.filter(query.as_deref(), cx.background()).await;
+                    if menu.matches.is_empty() {
+                        None
+                    } else {
+                        Some(menu)
+                    }
+                } else {
+                    None
                 };
 
-                menu.filter(query.as_deref(), cx.background()).await;
+                let this = this
+                    .upgrade(&cx)
+                    .ok_or_else(|| anyhow!("editor was dropped"))?;
+                this.update(&mut cx, |this, cx| {
+                    this.completion_tasks.retain(|(task_id, _)| *task_id > id);
 
-                if let Some(this) = this.upgrade(&cx) {
-                    this.update(&mut cx, |this, cx| {
-                        match this.context_menu.as_ref() {
-                            None => {}
-                            Some(ContextMenu::Completions(prev_menu)) => {
-                                if prev_menu.id > menu.id {
-                                    return;
-                                }
+                    match this.context_menu.as_ref() {
+                        None => {}
+                        Some(ContextMenu::Completions(prev_menu)) => {
+                            if prev_menu.id > id {
+                                return;
                             }
-                            _ => return,
                         }
+                        _ => return,
+                    }
 
-                        this.completion_tasks.retain(|(id, _)| *id > menu.id);
-                        if this.focused && !menu.matches.is_empty() {
-                            this.show_context_menu(ContextMenu::Completions(menu), cx);
-                        } else if this.hide_context_menu(cx).is_none() {
+                    if this.focused && menu.is_some() {
+                        let menu = menu.unwrap();
+                        this.show_context_menu(ContextMenu::Completions(menu), cx);
+                    } else if this.completion_tasks.is_empty() {
+                        // If there are no more completion tasks and the last menu was
+                        // empty, we should hide it. If it was already hidden, we should
+                        // also show the copilot suggestion when available.
+                        if this.hide_context_menu(cx).is_none() {
                             this.update_visible_copilot_suggestion(cx);
                         }
-                    });
-                }
+                    }
+                });
+
                 Ok::<_, anyhow::Error>(())
             }
             .log_err()

crates/editor/src/editor_tests.rs 🔗

@@ -5897,13 +5897,12 @@ async fn test_copilot(deterministic: Arc<Deterministic>, cx: &mut gpui::TestAppC
     )
     .await;
 
+    // When inserting, ensure autocompletion is favored over Copilot suggestions.
     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,
@@ -5917,8 +5916,8 @@ async fn test_copilot(deterministic: Arc<Deterministic>, cx: &mut gpui::TestAppC
     handle_copilot_completion_request(
         &copilot_lsp,
         vec![copilot::request::Completion {
-            text: "copilot1".into(),
-            range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 5)),
+            text: "one.copilot1".into(),
+            range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 4)),
             ..Default::default()
         }],
         vec![],
@@ -5940,13 +5939,45 @@ async fn test_copilot(deterministic: Arc<Deterministic>, cx: &mut gpui::TestAppC
         assert_eq!(editor.display_text(cx), "one.completion_a\ntwo\nthree\n");
     });
 
+    // Ensure Copilot suggestions are shown right away if no autocompletion is available.
     cx.set_state(indoc! {"
         oneˇ
         two
         three
     "});
+    cx.simulate_keystroke(".");
+    let _ = handle_completion_request(
+        &mut cx,
+        indoc! {"
+            one.|<>
+            two
+            three
+        "},
+        vec![],
+    );
+    handle_copilot_completion_request(
+        &copilot_lsp,
+        vec![copilot::request::Completion {
+            text: "one.copilot1".into(),
+            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));
+        assert_eq!(editor.display_text(cx), "one.copilot1\ntwo\nthree\n");
+        assert_eq!(editor.text(cx), "one.\ntwo\nthree\n");
+    });
 
-    // When inserting, ensure autocompletion is favored over Copilot suggestions.
+    // Reset editor, and ensure autocompletion is still favored over Copilot suggestions.
+    cx.set_state(indoc! {"
+        oneˇ
+        two
+        three
+    "});
     cx.simulate_keystroke(".");
     let _ = handle_completion_request(
         &mut cx,