From 90b220337778fed267aaeaa8e170a60428a2d80a Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 17 Sep 2024 11:31:32 -0600 Subject: [PATCH 1/4] Faster completion support for remotes Before this change we cancled every completion task on each keystroke. This works fine when the latency to the LSP is low, but on a remote that may be a few hundred ms away, this means you are always having to wait until the completions show. Co-Authored-By: Maan2003 --- crates/editor/src/editor.rs | 43 +++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 61a59665c15e3e..f218e4b5cde98b 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -550,6 +550,7 @@ pub struct Editor { signature_help_state: SignatureHelpState, auto_signature_help: Option, find_all_references_task_sources: Vec, + backup_completion_task: CompletionId, next_completion_id: CompletionId, completion_documentation_pre_resolve_debounce: DebouncedDelay, available_code_actions: Option<(Location, Arc<[CodeAction]>)>, @@ -1895,6 +1896,7 @@ impl Editor { signature_help_state: SignatureHelpState::default(), auto_signature_help: None, find_all_references_task_sources: Vec::new(), + backup_completion_task: 0, next_completion_id: 0, completion_documentation_pre_resolve_debounce: DebouncedDelay::new(), next_inlay_id: 0, @@ -4218,16 +4220,42 @@ impl Editor { }), trigger_kind, }; + let snapshot = buffer.read(cx).snapshot(); + let classifier = snapshot.char_classifier_at(&buffer_position); + let first_char = options + .trigger + .as_ref() + .and_then(|trigger| trigger.chars().next()); + let should_cancel_backup = first_char.is_some_and(|char| !classifier.is_word(char)); let completions = provider.completions(&buffer, buffer_position, completion_context, cx); let sort_completions = provider.sort_completions(); let id = post_inc(&mut self.next_completion_id); + if should_cancel_backup { + self.backup_completion_task = id; + } + dbg!( + id, + &options.trigger, + &should_cancel_backup, + self.backup_completion_task + ); let task = cx.spawn(|this, mut cx| { async move { this.update(&mut cx, |this, _| { - this.completion_tasks.retain(|(task_id, _)| *task_id >= id); + this.completion_tasks.retain(|(task_id, _)| { + if *task_id >= id || *task_id == this.backup_completion_task { + true + } else { + dbg!("dropping", id); + false + } + }); })?; let completions = completions.await.log_err(); + cx.background_executor() + .timer(Duration::from_millis(1000)) + .await; let menu = if let Some(completions) = completions { let mut menu = CompletionsMenu { id, @@ -4253,13 +4281,24 @@ impl Editor { DebouncedDelay::new(), )), }; - menu.filter(query.as_deref(), cx.background_executor().clone()) + let completion_query = this.update(&mut cx, |this, cx| { + Self::completion_query(&this.buffer.read(cx).read(cx), position) + })?; + let query = completion_query.or(query); + menu.filter(dbg!(query.as_deref()), cx.background_executor().clone()) .await; + this.update(&mut cx, |editor, cx| { + dbg!("got here!"); + editor.backup_completion_task = editor.next_completion_id.saturating_sub(1); + })?; + if menu.matches.is_empty() { None } else { this.update(&mut cx, |editor, cx| { + editor.backup_completion_task = + editor.next_completion_id.saturating_sub(1); let completions = menu.completions.clone(); let matches = menu.matches.clone(); From c36f84217cb4eb92a888c2a4080c9a6173b677b4 Mon Sep 17 00:00:00 2001 From: maan2003 Date: Wed, 18 Sep 2024 14:06:48 +0530 Subject: [PATCH 2/4] wip --- crates/editor/src/editor.rs | 73 +++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index f218e4b5cde98b..d80d561112ce0b 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -546,11 +546,12 @@ pub struct Editor { nav_history: Option, context_menu: RwLock>, mouse_context_menu: Option, - completion_tasks: Vec<(CompletionId, Task>)>, signature_help_state: SignatureHelpState, auto_signature_help: Option, find_all_references_task_sources: Vec, - backup_completion_task: CompletionId, + // invariant: backup_completion_task.is_some() -> latest_completion_task.is_some() + backup_completion_task: Option<(CompletionId, Task>)>, + latest_completion_task: Option<(CompletionId, Task>)>, next_completion_id: CompletionId, completion_documentation_pre_resolve_debounce: DebouncedDelay, available_code_actions: Option<(Location, Arc<[CodeAction]>)>, @@ -1892,11 +1893,11 @@ impl Editor { nav_history: None, context_menu: RwLock::new(None), mouse_context_menu: None, - completion_tasks: Default::default(), + latest_completion_task: None, + backup_completion_task: None, signature_help_state: SignatureHelpState::default(), auto_signature_help: None, find_all_references_task_sources: Vec::new(), - backup_completion_task: 0, next_completion_id: 0, completion_documentation_pre_resolve_debounce: DebouncedDelay::new(), next_inlay_id: 0, @@ -4231,27 +4232,20 @@ impl Editor { let sort_completions = provider.sort_completions(); let id = post_inc(&mut self.next_completion_id); - if should_cancel_backup { - self.backup_completion_task = id; - } dbg!( id, &options.trigger, &should_cancel_backup, - self.backup_completion_task + &self.backup_completion_task, + &self.latest_completion_task, ); + if should_cancel_backup { + self.backup_completion_task = None; + } let task = cx.spawn(|this, mut cx| { async move { - this.update(&mut cx, |this, _| { - this.completion_tasks.retain(|(task_id, _)| { - if *task_id >= id || *task_id == this.backup_completion_task { - true - } else { - dbg!("dropping", id); - false - } - }); - })?; + // TODO: if a menu exists already, then filter it before wait for completions. + // looks like this is something else filtering the completion menu. let completions = completions.await.log_err(); cx.background_executor() .timer(Duration::from_millis(1000)) @@ -4288,17 +4282,10 @@ impl Editor { menu.filter(dbg!(query.as_deref()), cx.background_executor().clone()) .await; - this.update(&mut cx, |editor, cx| { - dbg!("got here!"); - editor.backup_completion_task = editor.next_completion_id.saturating_sub(1); - })?; - if menu.matches.is_empty() { None } else { this.update(&mut cx, |editor, cx| { - editor.backup_completion_task = - editor.next_completion_id.saturating_sub(1); let completions = menu.completions.clone(); let matches = menu.matches.clone(); @@ -4338,13 +4325,33 @@ impl Editor { _ => return, } + let is_latest = if this + .latest_completion_task + .as_ref() + .map_or(false, |(task_id, _)| *task_id == id) + { + // we drop the both tasks + this.backup_completion_task = None; + this.latest_completion_task = None; + true + } else if this + .backup_completion_task + .as_ref() + .map_or(false, |(task_id, _)| *task_id == id) + { + this.backup_completion_task = None; + false + } else { + // the task didn't get cancelled, so we manually return + return; + }; if this.focus_handle.is_focused(cx) && menu.is_some() { let menu = menu.unwrap(); *context_menu = Some(ContextMenu::Completions(menu)); drop(context_menu); this.discard_inline_completion(false, cx); cx.notify(); - } else if this.completion_tasks.len() <= 1 { + } else if is_latest { // 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 completion when available. @@ -4359,8 +4366,10 @@ impl Editor { } .log_err() }); - - self.completion_tasks.push((id, task)); + let prev_task = std::mem::replace(&mut self.latest_completion_task, Some((id, task))); + if self.backup_completion_task.is_none() { + self.backup_completion_task = prev_task; + } } pub fn confirm_completion( @@ -4622,7 +4631,8 @@ impl Editor { return None; } - editor.completion_tasks.clear(); + editor.latest_completion_task = None; + editor.backup_completion_task = None; editor.discard_inline_completion(false, cx); let task_context = tasks @@ -5230,7 +5240,7 @@ impl Editor { let excerpt_id = cursor.excerpt_id; if self.context_menu.read().is_none() - && self.completion_tasks.is_empty() + && self.latest_completion_task.is_none() && selection.start == selection.end { if let Some(provider) = self.inline_completion_provider() { @@ -5402,7 +5412,8 @@ impl Editor { fn hide_context_menu(&mut self, cx: &mut ViewContext) -> Option { cx.notify(); - self.completion_tasks.clear(); + self.latest_completion_task = None; + self.backup_completion_task = None; let context_menu = self.context_menu.write().take(); if context_menu.is_some() { self.update_visible_inline_completion(cx); From 2ffca51c440558b8dd708e799161de8958fa7049 Mon Sep 17 00:00:00 2001 From: maan2003 Date: Wed, 18 Sep 2024 15:12:41 +0530 Subject: [PATCH 3/4] wip --- crates/editor/src/editor.rs | 120 ++++++++++++++++++++++++------------ 1 file changed, 82 insertions(+), 38 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d80d561112ce0b..a5354303921bba 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -549,9 +549,7 @@ pub struct Editor { signature_help_state: SignatureHelpState, auto_signature_help: Option, find_all_references_task_sources: Vec, - // invariant: backup_completion_task.is_some() -> latest_completion_task.is_some() - backup_completion_task: Option<(CompletionId, Task>)>, - latest_completion_task: Option<(CompletionId, Task>)>, + completion_tasks: CompletionsTasks, next_completion_id: CompletionId, completion_documentation_pre_resolve_debounce: DebouncedDelay, available_code_actions: Option<(Location, Arc<[CodeAction]>)>, @@ -855,6 +853,22 @@ enum ContextMenu { CodeActions(CodeActionsMenu), } +#[derive(Default, Debug)] +enum CompletionsTasks { + Single { + id: CompletionId, + task: Task>, + }, + WithBackup { + latest_id: CompletionId, + latest_task: Task>, + backup_id: CompletionId, + backup_task: Task>, + }, + #[default] + None, +} + impl ContextMenu { fn select_first( &mut self, @@ -1893,8 +1907,7 @@ impl Editor { nav_history: None, context_menu: RwLock::new(None), mouse_context_menu: None, - latest_completion_task: None, - backup_completion_task: None, + completion_tasks: CompletionsTasks::None, signature_help_state: SignatureHelpState::default(), auto_signature_help: None, find_all_references_task_sources: Vec::new(), @@ -4227,7 +4240,7 @@ impl Editor { .trigger .as_ref() .and_then(|trigger| trigger.chars().next()); - let should_cancel_backup = first_char.is_some_and(|char| !classifier.is_word(char)); + let should_cancel_previous = first_char.is_some_and(|char| !classifier.is_word(char)); let completions = provider.completions(&buffer, buffer_position, completion_context, cx); let sort_completions = provider.sort_completions(); @@ -4235,12 +4248,12 @@ impl Editor { dbg!( id, &options.trigger, - &should_cancel_backup, - &self.backup_completion_task, - &self.latest_completion_task, + &should_cancel_previous, + &self.completion_tasks, ); - if should_cancel_backup { - self.backup_completion_task = None; + if should_cancel_previous { + self.completion_tasks = CompletionsTasks::None; + // maybe even close the menu? } let task = cx.spawn(|this, mut cx| { async move { @@ -4325,25 +4338,33 @@ impl Editor { _ => return, } - let is_latest = if this - .latest_completion_task - .as_ref() - .map_or(false, |(task_id, _)| *task_id == id) - { - // we drop the both tasks - this.backup_completion_task = None; - this.latest_completion_task = None; - true - } else if this - .backup_completion_task - .as_ref() - .map_or(false, |(task_id, _)| *task_id == id) - { - this.backup_completion_task = None; - false - } else { - // the task didn't get cancelled, so we manually return - return; + let is_latest = match std::mem::take(&mut this.completion_tasks) { + CompletionsTasks::Single { id: latest_id, .. } + | CompletionsTasks::WithBackup { latest_id, .. } + if latest_id == id => + { + // if this is latest completion task, then drop all + this.completion_tasks = CompletionsTasks::None; + true + } + CompletionsTasks::WithBackup { + backup_id, + latest_id, + latest_task, + .. + } if backup_id == id => { + // convert to single task + this.completion_tasks = CompletionsTasks::Single { + id: latest_id, + task: latest_task, + }; + false + } + // neither latest nor backup task + _ => { + // the task didn't get cancelled, so we manually return + return; + } }; if this.focus_handle.is_focused(cx) && menu.is_some() { let menu = menu.unwrap(); @@ -4366,9 +4387,34 @@ impl Editor { } .log_err() }); - let prev_task = std::mem::replace(&mut self.latest_completion_task, Some((id, task))); - if self.backup_completion_task.is_none() { - self.backup_completion_task = prev_task; + match std::mem::take(&mut self.completion_tasks) { + CompletionsTasks::None => self.completion_tasks = CompletionsTasks::Single { id, task }, + CompletionsTasks::Single { + id: prev_id, + task: prev_task, + } => { + // move prev task to backup + self.completion_tasks = CompletionsTasks::WithBackup { + latest_id: id, + latest_task: task, + backup_id: prev_id, + backup_task: prev_task, + } + } + CompletionsTasks::WithBackup { + latest_id: prev_latest_id, + latest_task: prev_latest_task, + backup_id, + backup_task, + } => { + // replace the latest task + self.completion_tasks = &CompletionsTasks::WithBackup { + latest_id: id, + latest_task: task, + backup_id, + backup_task, + } + } } } @@ -4631,8 +4677,7 @@ impl Editor { return None; } - editor.latest_completion_task = None; - editor.backup_completion_task = None; + editor.completion_tasks = CompletionsTasks::None; editor.discard_inline_completion(false, cx); let task_context = tasks @@ -5240,7 +5285,7 @@ impl Editor { let excerpt_id = cursor.excerpt_id; if self.context_menu.read().is_none() - && self.latest_completion_task.is_none() + && matches!(self.completion_tasks, CompletionsTasks::None) && selection.start == selection.end { if let Some(provider) = self.inline_completion_provider() { @@ -5412,8 +5457,7 @@ impl Editor { fn hide_context_menu(&mut self, cx: &mut ViewContext) -> Option { cx.notify(); - self.latest_completion_task = None; - self.backup_completion_task = None; + self.completion_tasks = CompletionsTasks::None; let context_menu = self.context_menu.write().take(); if context_menu.is_some() { self.update_visible_inline_completion(cx); From e9415d201744dc2399f3502f0cce2a280cc18811 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 20 Sep 2024 13:47:45 -0600 Subject: [PATCH 4/4] tidy up --- crates/editor/src/editor.rs | 177 +++++++++++++++--------------------- 1 file changed, 74 insertions(+), 103 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a5354303921bba..286cc9db8c01cb 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -549,7 +549,7 @@ pub struct Editor { signature_help_state: SignatureHelpState, auto_signature_help: Option, find_all_references_task_sources: Vec, - completion_tasks: CompletionsTasks, + completion_tasks: Option, next_completion_id: CompletionId, completion_documentation_pre_resolve_debounce: DebouncedDelay, available_code_actions: Option<(Location, Arc<[CodeAction]>)>, @@ -853,20 +853,12 @@ enum ContextMenu { CodeActions(CodeActionsMenu), } -#[derive(Default, Debug)] -enum CompletionsTasks { - Single { - id: CompletionId, - task: Task>, - }, - WithBackup { - latest_id: CompletionId, - latest_task: Task>, - backup_id: CompletionId, - backup_task: Task>, - }, - #[default] - None, +#[derive(Debug)] +struct CompletionTasks { + id: CompletionId, + task: Task<()>, + backup_id: Option, + backup_task: Option>, } impl ContextMenu { @@ -1907,7 +1899,7 @@ impl Editor { nav_history: None, context_menu: RwLock::new(None), mouse_context_menu: None, - completion_tasks: CompletionsTasks::None, + completion_tasks: None, signature_help_state: SignatureHelpState::default(), auto_signature_help: None, find_all_references_task_sources: Vec::new(), @@ -2487,7 +2479,7 @@ impl Editor { let mut completion_menu = completion_menu.clone(); drop(context_menu); - let query = Self::completion_query(buffer, cursor_position); + let (_, query) = Self::completion_query(buffer, cursor_position); cx.spawn(move |this, mut cx| async move { completion_menu .filter(query.as_deref(), cx.background_executor().clone()) @@ -3950,10 +3942,13 @@ impl Editor { }); } - fn completion_query(buffer: &MultiBufferSnapshot, position: impl ToOffset) -> Option { + fn completion_query( + buffer: &MultiBufferSnapshot, + position: impl ToOffset, + ) -> (multi_buffer::Anchor, Option) { let offset = position.to_offset(buffer); let (word_range, kind) = buffer.surrounding_word(offset, true); - if offset > word_range.start && kind == Some(CharKind::Word) { + let query = if offset > word_range.start && kind == Some(CharKind::Word) { Some( buffer .text_for_range(word_range.start..offset) @@ -3961,7 +3956,8 @@ impl Editor { ) } else { None - } + }; + (buffer.anchor_before(word_range.start), query) } pub fn toggle_inlay_hints(&mut self, _: &ToggleInlayHints, cx: &mut ViewContext) { @@ -4208,7 +4204,8 @@ impl Editor { return; }; - let query = Self::completion_query(&self.buffer.read(cx).read(cx), position); + let (initial_position, query) = + Self::completion_query(&self.buffer.read(cx).read(cx), position); let is_followup_invoke = { let context_menu_state = self.context_menu.read(); matches!( @@ -4244,28 +4241,16 @@ impl Editor { let completions = provider.completions(&buffer, buffer_position, completion_context, cx); let sort_completions = provider.sort_completions(); - let id = post_inc(&mut self.next_completion_id); - dbg!( - id, - &options.trigger, - &should_cancel_previous, - &self.completion_tasks, - ); + let completion_id = post_inc(&mut self.next_completion_id); if should_cancel_previous { - self.completion_tasks = CompletionsTasks::None; - // maybe even close the menu? + self.completion_tasks = None; } let task = cx.spawn(|this, mut cx| { - async move { - // TODO: if a menu exists already, then filter it before wait for completions. - // looks like this is something else filtering the completion menu. + let run = async move { let completions = completions.await.log_err(); - cx.background_executor() - .timer(Duration::from_millis(1000)) - .await; let menu = if let Some(completions) = completions { let mut menu = CompletionsMenu { - id, + id: completion_id, sort_completions, initial_position: position, match_candidates: completions @@ -4288,14 +4273,25 @@ impl Editor { DebouncedDelay::new(), )), }; - let completion_query = this.update(&mut cx, |this, cx| { - Self::completion_query(&this.buffer.read(cx).read(cx), position) + let (cursor_moved, query) = this.update(&mut cx, |this, cx| { + let position = this.selections.newest_anchor().head(); + let (new_position, new_query) = + Self::completion_query(&this.buffer.read(cx).read(cx), position); + if initial_position != new_position + && this + .completion_tasks + .as_ref() + .is_some_and(|tasks| tasks.backup_id == Some(completion_id)) + { + return (true, None); + } + (false, new_query) })?; - let query = completion_query.or(query); - menu.filter(dbg!(query.as_deref()), cx.background_executor().clone()) + + menu.filter(query.as_deref(), cx.background_executor().clone()) .await; - if menu.matches.is_empty() { + if menu.matches.is_empty() || cursor_moved { None } else { this.update(&mut cx, |editor, cx| { @@ -4330,7 +4326,7 @@ impl Editor { None => {} Some(ContextMenu::Completions(prev_menu)) => { - if prev_menu.id > id { + if prev_menu.id > completion_id { return; } } @@ -4338,34 +4334,21 @@ impl Editor { _ => return, } - let is_latest = match std::mem::take(&mut this.completion_tasks) { - CompletionsTasks::Single { id: latest_id, .. } - | CompletionsTasks::WithBackup { latest_id, .. } - if latest_id == id => - { - // if this is latest completion task, then drop all - this.completion_tasks = CompletionsTasks::None; - true - } - CompletionsTasks::WithBackup { - backup_id, - latest_id, - latest_task, - .. - } if backup_id == id => { - // convert to single task - this.completion_tasks = CompletionsTasks::Single { - id: latest_id, - task: latest_task, - }; - false - } - // neither latest nor backup task - _ => { - // the task didn't get cancelled, so we manually return - return; + let is_latest = this + .completion_tasks + .as_ref() + .is_some_and(|tasks| tasks.id == completion_id); + if let Some(tasks) = this.completion_tasks.take() { + if Some(completion_id) == tasks.backup_id { + this.completion_tasks = Some(CompletionTasks { + id: tasks.id, + task: tasks.task, + backup_id: None, + backup_task: None, + }); } - }; + } + if this.focus_handle.is_focused(cx) && menu.is_some() { let menu = menu.unwrap(); *context_menu = Some(ContextMenu::Completions(menu)); @@ -4385,37 +4368,25 @@ impl Editor { Ok::<_, anyhow::Error>(()) } - .log_err() - }); - match std::mem::take(&mut self.completion_tasks) { - CompletionsTasks::None => self.completion_tasks = CompletionsTasks::Single { id, task }, - CompletionsTasks::Single { - id: prev_id, - task: prev_task, - } => { - // move prev task to backup - self.completion_tasks = CompletionsTasks::WithBackup { - latest_id: id, - latest_task: task, - backup_id: prev_id, - backup_task: prev_task, - } - } - CompletionsTasks::WithBackup { - latest_id: prev_latest_id, - latest_task: prev_latest_task, - backup_id, - backup_task, - } => { - // replace the latest task - self.completion_tasks = &CompletionsTasks::WithBackup { - latest_id: id, - latest_task: task, - backup_id, - backup_task, - } + .log_err(); + async move { + run.await; } - } + }); + let (backup_id, backup_task) = if let Some(tasks) = self.completion_tasks.take() { + ( + Some(tasks.backup_id.unwrap_or(tasks.id)), + Some(tasks.backup_task.unwrap_or(tasks.task)), + ) + } else { + (None, None) + }; + self.completion_tasks = Some(CompletionTasks { + id: completion_id, + task, + backup_id, + backup_task, + }); } pub fn confirm_completion( @@ -4677,7 +4648,7 @@ impl Editor { return None; } - editor.completion_tasks = CompletionsTasks::None; + editor.completion_tasks = None; editor.discard_inline_completion(false, cx); let task_context = tasks @@ -5285,7 +5256,7 @@ impl Editor { let excerpt_id = cursor.excerpt_id; if self.context_menu.read().is_none() - && matches!(self.completion_tasks, CompletionsTasks::None) + && matches!(self.completion_tasks, None) && selection.start == selection.end { if let Some(provider) = self.inline_completion_provider() { @@ -5457,7 +5428,7 @@ impl Editor { fn hide_context_menu(&mut self, cx: &mut ViewContext) -> Option { cx.notify(); - self.completion_tasks = CompletionsTasks::None; + self.completion_tasks = None; let context_menu = self.context_menu.write().take(); if context_menu.is_some() { self.update_visible_inline_completion(cx);