Avoid extra completion requests (#11875)

Gus created

Do not spawn a second completion request when completion menu is open and a new edit is made.

Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs       |  23 +++++-
crates/editor/src/editor_tests.rs | 117 ++++++++++++++++++++++++++++++++
2 files changed, 135 insertions(+), 5 deletions(-)

Detailed changes

crates/editor/src/editor.rs ๐Ÿ”—

@@ -2041,6 +2041,7 @@ impl Editor {
         &mut self,
         local: bool,
         old_cursor_position: &Anchor,
+        show_completions: bool,
         cx: &mut ViewContext<Self>,
     ) {
         // Copy selections to primary selection buffer
@@ -2142,7 +2143,9 @@ impl Editor {
                     })
                     .detach();
 
-                    self.show_completions(&ShowCompletions, cx);
+                    if show_completions {
+                        self.show_completions(&ShowCompletions, cx);
+                    }
                 } else {
                     drop(context_menu);
                     self.hide_context_menu(cx);
@@ -2182,6 +2185,16 @@ impl Editor {
         autoscroll: Option<Autoscroll>,
         cx: &mut ViewContext<Self>,
         change: impl FnOnce(&mut MutableSelectionsCollection<'_>) -> R,
+    ) -> R {
+        self.change_selections_inner(autoscroll, true, cx, change)
+    }
+
+    pub fn change_selections_inner<R>(
+        &mut self,
+        autoscroll: Option<Autoscroll>,
+        request_completions: bool,
+        cx: &mut ViewContext<Self>,
+        change: impl FnOnce(&mut MutableSelectionsCollection<'_>) -> R,
     ) -> R {
         let old_cursor_position = self.selections.newest_anchor().head();
         self.push_to_selection_history();
@@ -2192,7 +2205,7 @@ impl Editor {
             if let Some(autoscroll) = autoscroll {
                 self.request_autoscroll(autoscroll, cx);
             }
-            self.selections_did_change(true, &old_cursor_position, cx);
+            self.selections_did_change(true, &old_cursor_position, request_completions, cx);
         }
 
         result
@@ -2852,7 +2865,9 @@ impl Editor {
 
             drop(snapshot);
             let had_active_inline_completion = this.has_active_inline_completion(cx);
-            this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections));
+            this.change_selections_inner(Some(Autoscroll::fit()), false, cx, |s| {
+                s.select(new_selections)
+            });
 
             if brace_inserted {
                 // If we inserted a brace while composing text (i.e. typing `"` on a
@@ -9165,7 +9180,7 @@ impl Editor {
                 s.clear_pending();
             }
         });
-        self.selections_did_change(false, &old_cursor_position, cx);
+        self.selections_did_change(false, &old_cursor_position, true, cx);
     }
 
     fn push_to_selection_history(&mut self) {

crates/editor/src/editor_tests.rs ๐Ÿ”—

@@ -6531,6 +6531,7 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
         cx,
     )
     .await;
+    let counter = Arc::new(AtomicUsize::new(0));
 
     cx.set_state(indoc! {"
         oneห‡
@@ -6546,10 +6547,13 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
             three
         "},
         vec!["first_completion", "second_completion"],
+        counter.clone(),
     )
     .await;
     cx.condition(|editor, _| editor.context_menu_visible())
         .await;
+    assert_eq!(counter.load(atomic::Ordering::Acquire), 1);
+
     let apply_additional_edits = cx.update_editor(|editor, cx| {
         editor.context_menu_next(&Default::default(), cx);
         editor
@@ -6620,10 +6624,12 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
             additional edit
         "},
         vec!["fourth_completion", "fifth_completion", "sixth_completion"],
+        counter.clone(),
     )
     .await;
     cx.condition(|editor, _| editor.context_menu_visible())
         .await;
+    assert_eq!(counter.load(atomic::Ordering::Acquire), 2);
 
     cx.simulate_keystroke("i");
 
@@ -6636,10 +6642,12 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
             additional edit
         "},
         vec!["fourth_completion", "fifth_completion", "sixth_completion"],
+        counter.clone(),
     )
     .await;
     cx.condition(|editor, _| editor.context_menu_visible())
         .await;
+    assert_eq!(counter.load(atomic::Ordering::Acquire), 3);
 
     let apply_additional_edits = cx.update_editor(|editor, cx| {
         editor
@@ -6674,9 +6682,17 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
     cx.update_editor(|editor, cx| {
         editor.show_completions(&ShowCompletions, cx);
     });
-    handle_completion_request(&mut cx, "editor.<clo|>", vec!["close", "clobber"]).await;
+    handle_completion_request(
+        &mut cx,
+        "editor.<clo|>",
+        vec!["close", "clobber"],
+        counter.clone(),
+    )
+    .await;
     cx.condition(|editor, _| editor.context_menu_visible())
         .await;
+    assert_eq!(counter.load(atomic::Ordering::Acquire), 4);
+
     let apply_additional_edits = cx.update_editor(|editor, cx| {
         editor
             .confirm_completion(&ConfirmCompletion::default(), cx)
@@ -6687,6 +6703,103 @@ async fn test_completion(cx: &mut gpui::TestAppContext) {
     apply_additional_edits.await.unwrap();
 }
 
+#[gpui::test]
+async fn test_no_duplicated_completion_requests(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+
+    let mut cx = EditorLspTestContext::new_rust(
+        lsp::ServerCapabilities {
+            completion_provider: Some(lsp::CompletionOptions {
+                trigger_characters: Some(vec![".".to_string()]),
+                resolve_provider: Some(true),
+                ..Default::default()
+            }),
+            ..Default::default()
+        },
+        cx,
+    )
+    .await;
+
+    cx.set_state(indoc! {"fn main() { let a = 2ห‡; }"});
+    cx.simulate_keystroke(".");
+    let completion_item = lsp::CompletionItem {
+        label: "Some".into(),
+        kind: Some(lsp::CompletionItemKind::SNIPPET),
+        detail: Some("Wrap the expression in an `Option::Some`".to_string()),
+        documentation: Some(lsp::Documentation::MarkupContent(lsp::MarkupContent {
+            kind: lsp::MarkupKind::Markdown,
+            value: "```rust\nSome(2)\n```".to_string(),
+        })),
+        deprecated: Some(false),
+        sort_text: Some("Some".to_string()),
+        filter_text: Some("Some".to_string()),
+        insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
+        text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+            range: lsp::Range {
+                start: lsp::Position {
+                    line: 0,
+                    character: 22,
+                },
+                end: lsp::Position {
+                    line: 0,
+                    character: 22,
+                },
+            },
+            new_text: "Some(2)".to_string(),
+        })),
+        additional_text_edits: Some(vec![lsp::TextEdit {
+            range: lsp::Range {
+                start: lsp::Position {
+                    line: 0,
+                    character: 20,
+                },
+                end: lsp::Position {
+                    line: 0,
+                    character: 22,
+                },
+            },
+            new_text: "".to_string(),
+        }]),
+        ..Default::default()
+    };
+
+    let closure_completion_item = completion_item.clone();
+    let counter = Arc::new(AtomicUsize::new(0));
+    let counter_clone = counter.clone();
+    let mut request = cx.handle_request::<lsp::request::Completion, _, _>(move |_, _, _| {
+        let task_completion_item = closure_completion_item.clone();
+        counter_clone.fetch_add(1, atomic::Ordering::Release);
+        async move {
+            Ok(Some(lsp::CompletionResponse::Array(vec![
+                task_completion_item,
+            ])))
+        }
+    });
+
+    cx.condition(|editor, _| editor.context_menu_visible())
+        .await;
+    cx.assert_editor_state(indoc! {"fn main() { let a = 2.ห‡; }"});
+    assert!(request.next().await.is_some());
+    assert_eq!(counter.load(atomic::Ordering::Acquire), 1);
+
+    cx.simulate_keystroke("S");
+    cx.simulate_keystroke("o");
+    cx.simulate_keystroke("m");
+    cx.condition(|editor, _| editor.context_menu_visible())
+        .await;
+    cx.assert_editor_state(indoc! {"fn main() { let a = 2.Somห‡; }"});
+    assert!(request.next().await.is_some());
+    assert!(request.next().await.is_some());
+    assert!(request.next().await.is_some());
+    request.close();
+    assert!(request.next().await.is_none());
+    assert_eq!(
+        counter.load(atomic::Ordering::Acquire),
+        4,
+        "With the completions menu open, only one LSP request should happen per input"
+    );
+}
+
 #[gpui::test]
 async fn test_toggle_comment(cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});
@@ -11358,6 +11471,7 @@ pub fn handle_completion_request(
     cx: &mut EditorLspTestContext,
     marked_string: &str,
     completions: Vec<&'static str>,
+    counter: Arc<AtomicUsize>,
 ) -> impl Future<Output = ()> {
     let complete_from_marker: TextRangeMarker = '|'.into();
     let replace_range_marker: TextRangeMarker = ('<', '>').into();
@@ -11373,6 +11487,7 @@ pub fn handle_completion_request(
 
     let mut request = cx.handle_request::<lsp::request::Completion, _, _>(move |url, params, _| {
         let completions = completions.clone();
+        counter.fetch_add(1, atomic::Ordering::Release);
         async move {
             assert_eq!(params.text_document_position.text_document.uri, url.clone());
             assert_eq!(