Make space for documentation aside during followup completion select (#21716)

The goal of #7115 appears to be to limit the disruptiveness of
completion documentation load causing the completion selector to move
around. The approach was to debounce load of documentation via a setting
`completion_documentation_secondary_query_debounce`. This particularly
had a nonideal interaction with #21286, where now this debounce interval
was used between the documentation fetches of every individual
completion item.

I think a better solution is to continue making space for documentation
to be shown as soon as any documentation is shown. #21704 implemented
part of this, but it did not persist across followup completions.

Release Notes:

- Fixed completion list moving around on load of documentation. The
previous approach to mitigating this was to rate-limit the fetch of
docs, configured by a
`completion_documentation_secondary_query_debounce` setting, which is
now deprecated.

assets/settings/default.json

@@ -150,9 +150,6 @@
   // Whether to display inline and alongside documentation for items in the
   // completions menu
   "show_completion_documentation": true,
-  // The debounce delay before re-querying the language server for completion
-  // documentation when not included in original completion list.
-  "completion_documentation_secondary_query_debounce": 300,
   // Show method signatures in the editor, when inside parentheses.
   "auto_signature_help": false,
   /// Whether to show the signature help after completion or a bracket pair inserted.

crates/editor/src/debounced_delay.rs

@@ -1,46 +0,0 @@
-use std::time::Duration;
-
-use futures::{channel::oneshot, FutureExt};
-use gpui::{Task, ViewContext};
-
-use crate::Editor;
-
-#[derive(Debug)]
-pub struct DebouncedDelay {
-    task: Option<Task<()>>,
-    cancel_channel: Option<oneshot::Sender<()>>,
-}
-
-impl DebouncedDelay {
-    pub fn new() -> DebouncedDelay {
-        DebouncedDelay {
-            task: None,
-            cancel_channel: None,
-        }
-    }
-
-    pub fn fire_new<F>(&mut self, delay: Duration, cx: &mut ViewContext<Editor>, func: F)
-    where
-        F: 'static + Send + FnOnce(&mut Editor, &mut ViewContext<Editor>) -> Task<()>,
-    {
-        if let Some(channel) = self.cancel_channel.take() {
-            _ = channel.send(());
-        }
-
-        let (sender, mut receiver) = oneshot::channel::<()>();
-        self.cancel_channel = Some(sender);
-
-        drop(self.task.take());
-        self.task = Some(cx.spawn(move |model, mut cx| async move {
-            let mut timer = cx.background_executor().timer(delay).fuse();
-            futures::select_biased! {
-                _ = receiver => return,
-                _ = timer => {}
-            }
-
-            if let Ok(task) = model.update(&mut cx, |project, cx| (func)(project, cx)) {
-                task.await;
-            }
-        }));
-    }
-}

crates/editor/src/editor.rs

@@ -16,7 +16,6 @@ pub mod actions;
 mod blame_entry_tooltip;
 mod blink_manager;
 mod clangd_ext;
-mod debounced_delay;
 pub mod display_map;
 mod editor_settings;
 mod editor_settings_controls;
@@ -58,7 +57,6 @@ use client::{Collaborator, ParticipantIndex};
 use clock::ReplicaId;
 use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque};
 use convert_case::{Case, Casing};
-use debounced_delay::DebouncedDelay;
 use display_map::*;
 pub use display_map::{DisplayPoint, FoldPlaceholder};
 pub use editor_settings::{
@@ -123,7 +121,7 @@ use multi_buffer::{
     ExpandExcerptDirection, MultiBufferDiffHunk, MultiBufferPoint, MultiBufferRow, ToOffsetUtf16,
 };
 use ordered_float::OrderedFloat;
-use parking_lot::{Mutex, RwLock};
+use parking_lot::RwLock;
 use project::{
     lsp_store::{FormatTarget, FormatTrigger},
     project_settings::{GitGutterSetting, ProjectSettings},
@@ -1006,7 +1004,7 @@ struct CompletionsMenu {
     matches: Arc<[StringMatch]>,
     selected_item: usize,
     scroll_handle: UniformListScrollHandle,
-    selected_completion_resolve_debounce: Option<Arc<Mutex<DebouncedDelay>>>,
+    resolve_completions: bool,
     aside_was_displayed: Cell<bool>,
 }
 
@@ -1017,6 +1015,7 @@ impl CompletionsMenu {
         initial_position: Anchor,
         buffer: Model<Buffer>,
         completions: Box<[Completion]>,
+        aside_was_displayed: bool,
     ) -> Self {
         let match_candidates = completions
             .iter()
@@ -1039,8 +1038,8 @@ impl CompletionsMenu {
             matches: Vec::new().into(),
             selected_item: 0,
             scroll_handle: UniformListScrollHandle::new(),
-            selected_completion_resolve_debounce: Some(Arc::new(Mutex::new(DebouncedDelay::new()))),
+            resolve_completions: true,
-            aside_was_displayed: Cell::new(false),
+            aside_was_displayed: Cell::new(aside_was_displayed),
         }
     }
 
@@ -1093,16 +1092,11 @@ impl CompletionsMenu {
             matches,
             selected_item: 0,
             scroll_handle: UniformListScrollHandle::new(),
-            selected_completion_resolve_debounce: Some(Arc::new(Mutex::new(DebouncedDelay::new()))),
+            resolve_completions: false,
             aside_was_displayed: Cell::new(false),
         }
     }
 
-    fn suppress_documentation_resolution(mut self) -> Self {
-        self.selected_completion_resolve_debounce.take();
-        self
-    }
-
     fn select_first(
         &mut self,
         provider: Option<&dyn CompletionProvider>,
@@ -1164,14 +1158,14 @@ impl CompletionsMenu {
         provider: Option<&dyn CompletionProvider>,
         cx: &mut ViewContext<Editor>,
     ) {
-        let completion_index = self.matches[self.selected_item].candidate_id;
+        if !self.resolve_completions {
+            return;
+        }
         let Some(provider) = provider else {
             return;
         };
-        let Some(completion_resolve) = self.selected_completion_resolve_debounce.as_ref() else {
-            return;
-        };
 
+        let completion_index = self.matches[self.selected_item].candidate_id;
         let resolve_task = provider.resolve_completions(
             self.buffer.clone(),
             vec![completion_index],
@@ -1179,17 +1173,12 @@ impl CompletionsMenu {
             cx,
         );
 
-        let delay_ms =
+        cx.spawn(move |editor, mut cx| async move {
-            EditorSettings::get_global(cx).completion_documentation_secondary_query_debounce;
+            if let Some(true) = resolve_task.await.log_err() {
-        let delay = Duration::from_millis(delay_ms);
+                editor.update(&mut cx, |_, cx| cx.notify()).ok();
-
+            }
-        completion_resolve.lock().fire_new(delay, cx, |_, cx| {
+        })
-            cx.spawn(move |editor, mut cx| async move {
+        .detach();
-                if let Some(true) = resolve_task.await.log_err() {
-                    editor.update(&mut cx, |_, cx| cx.notify()).ok();
-                }
-            })
-        });
     }
 
     fn visible(&self) -> bool {
@@ -4472,12 +4461,9 @@ impl Editor {
             };
 
         let query = Self::completion_query(&self.buffer.read(cx).read(cx), position);
-        let is_followup_invoke = {
+        let (is_followup_invoke, aside_was_displayed) = match self.context_menu.read().deref() {
-            let context_menu_state = self.context_menu.read();
+            Some(ContextMenu::Completions(menu)) => (true, menu.aside_was_displayed.get()),
-            matches!(
+            _ => (false, false),
-                context_menu_state.deref(),
-                Some(ContextMenu::Completions(_))
-            )
         };
         let trigger_kind = match (&options.trigger, is_followup_invoke) {
             (_, true) => CompletionTriggerKind::TRIGGER_FOR_INCOMPLETE_COMPLETIONS,
@@ -4514,6 +4500,7 @@ impl Editor {
                         position,
                         buffer.clone(),
                         completions.into(),
+                        aside_was_displayed,
                     );
                     menu.filter(query.as_deref(), cx.background_executor().clone())
                         .await;
@@ -5858,8 +5845,7 @@ impl Editor {
 
         if let Some(buffer) = buffer {
             *self.context_menu.write() = Some(ContextMenu::Completions(
-                CompletionsMenu::new_snippet_choices(id, true, choices, selection, buffer)
+                CompletionsMenu::new_snippet_choices(id, true, choices, selection, buffer),
-                    .suppress_documentation_resolution(),
             ));
         }
     }

crates/editor/src/editor_settings.rs

@@ -12,7 +12,6 @@ pub struct EditorSettings {
     pub hover_popover_enabled: bool,
     pub show_completions_on_input: bool,
     pub show_completion_documentation: bool,
-    pub completion_documentation_secondary_query_debounce: u64,
     pub toolbar: Toolbar,
     pub scrollbar: Scrollbar,
     pub gutter: Gutter,
@@ -204,11 +203,6 @@ pub struct EditorSettingsContent {
     ///
     /// Default: true
     pub show_completion_documentation: Option<bool>,
-    /// The debounce delay before re-querying the language server for completion
-    /// documentation when not included in original completion list.
-    ///
-    /// Default: 300 ms
-    pub completion_documentation_secondary_query_debounce: Option<u64>,
     /// Toolbar related settings
     pub toolbar: Option<ToolbarContent>,
     /// Scrollbar related settings

docs/src/configuring-zed.md

@@ -1517,16 +1517,6 @@ Or to set a `socks5` proxy:
 
 `boolean` values
 
-## Completion Documentation Debounce Delay
-
-- Description: The debounce delay before re-querying the language server for completion documentation when not included in original completion list.
-- Setting: `completion_documentation_secondary_query_debounce`
-- Default: `300` ms
-
-**Options**
-
-`integer` values
-
 ## Show Inline Completions
 
 - Description: Whether to show inline completions as you type or manually by triggering `editor::ShowInlineCompletion`.