Skip to content

Commit

Permalink
Make space for documentation aside during followup completion select (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
mgsloan authored Dec 9, 2024
1 parent 2af9fa7 commit 7bd6913
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 100 deletions.
3 changes: 0 additions & 3 deletions assets/settings/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
46 changes: 0 additions & 46 deletions crates/editor/src/debounced_delay.rs

This file was deleted.

56 changes: 21 additions & 35 deletions crates/editor/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::{
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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>,
}

Expand All @@ -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()
Expand All @@ -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()))),
aside_was_displayed: Cell::new(false),
resolve_completions: true,
aside_was_displayed: Cell::new(aside_was_displayed),
}
}

Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -1164,32 +1158,27 @@ impl CompletionsMenu {
provider: Option<&dyn CompletionProvider>,
cx: &mut ViewContext<Editor>,
) {
let completion_index = self.matches[self.selected_item].candidate_id;
let Some(provider) = provider else {
if !self.resolve_completions {
return;
};
let Some(completion_resolve) = self.selected_completion_resolve_debounce.as_ref() else {
}
let Some(provider) = provider else {
return;
};

let completion_index = self.matches[self.selected_item].candidate_id;
let resolve_task = provider.resolve_completions(
self.buffer.clone(),
vec![completion_index],
self.completions.clone(),
cx,
);

let delay_ms =
EditorSettings::get_global(cx).completion_documentation_secondary_query_debounce;
let delay = Duration::from_millis(delay_ms);

completion_resolve.lock().fire_new(delay, cx, |_, cx| {
cx.spawn(move |editor, mut cx| async move {
if let Some(true) = resolve_task.await.log_err() {
editor.update(&mut cx, |_, cx| cx.notify()).ok();
}
})
});
cx.spawn(move |editor, mut cx| async move {
if let Some(true) = resolve_task.await.log_err() {
editor.update(&mut cx, |_, cx| cx.notify()).ok();
}
})
.detach();
}

fn visible(&self) -> bool {
Expand Down Expand Up @@ -4472,12 +4461,9 @@ impl Editor {
};

let query = Self::completion_query(&self.buffer.read(cx).read(cx), position);
let is_followup_invoke = {
let context_menu_state = self.context_menu.read();
matches!(
context_menu_state.deref(),
Some(ContextMenu::Completions(_))
)
let (is_followup_invoke, aside_was_displayed) = match self.context_menu.read().deref() {
Some(ContextMenu::Completions(menu)) => (true, menu.aside_was_displayed.get()),
_ => (false, false),
};
let trigger_kind = match (&options.trigger, is_followup_invoke) {
(_, true) => CompletionTriggerKind::TRIGGER_FOR_INCOMPLETE_COMPLETIONS,
Expand Down Expand Up @@ -4514,6 +4500,7 @@ impl Editor {
position,
buffer.clone(),
completions.into(),
aside_was_displayed,
);
menu.filter(query.as_deref(), cx.background_executor().clone())
.await;
Expand Down Expand Up @@ -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)
.suppress_documentation_resolution(),
CompletionsMenu::new_snippet_choices(id, true, choices, selection, buffer),
));
}
}
Expand Down
6 changes: 0 additions & 6 deletions crates/editor/src/editor_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions docs/src/configuring-zed.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down

0 comments on commit 7bd6913

Please sign in to comment.