diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index d177e5ac3e..8966a936b0 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -1,7 +1,7 @@ use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ div, pulsating_between, px, uniform_list, Animation, AnimationExt, AnyElement, - BackgroundExecutor, Div, FontWeight, Hsla, ListSizingBehavior, Model, ScrollStrategy, + AsyncWindowContext, Div, FontWeight, Hsla, ListSizingBehavior, Model, ScrollStrategy, SharedString, Size, StrikethroughStyle, StyledText, TextStyleRefinement, UniformListScrollHandle, ViewContext, WeakView, }; @@ -43,20 +43,26 @@ pub enum CodeContextMenu { CodeActions(CodeActionsMenu), } +#[must_use] +#[derive(Debug, Clone, Copy)] +pub enum InlineCompletionVisibilityChange { + Unchanged, + Hide, + Show, +} + impl CodeContextMenu { pub fn select_first( &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) -> bool { - if self.visible() { - match self { - CodeContextMenu::Completions(menu) => menu.select_first(provider, cx), - CodeContextMenu::CodeActions(menu) => menu.select_first(cx), + ) -> InlineCompletionVisibilityChange { + match self { + CodeContextMenu::Completions(menu) => menu.select_first(provider, cx), + CodeContextMenu::CodeActions(menu) => { + menu.select_first(cx); + InlineCompletionVisibilityChange::Unchanged } - true - } else { - false } } @@ -64,15 +70,13 @@ impl CodeContextMenu { &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) -> bool { - if self.visible() { - match self { - CodeContextMenu::Completions(menu) => menu.select_prev(provider, cx), - CodeContextMenu::CodeActions(menu) => menu.select_prev(cx), + ) -> InlineCompletionVisibilityChange { + match self { + CodeContextMenu::Completions(menu) => menu.select_prev(provider, cx), + CodeContextMenu::CodeActions(menu) => { + menu.select_prev(cx); + InlineCompletionVisibilityChange::Unchanged } - true - } else { - false } } @@ -80,15 +84,13 @@ impl CodeContextMenu { &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) -> bool { - if self.visible() { - match self { - CodeContextMenu::Completions(menu) => menu.select_next(provider, cx), - CodeContextMenu::CodeActions(menu) => menu.select_next(cx), + ) -> InlineCompletionVisibilityChange { + match self { + CodeContextMenu::Completions(menu) => menu.select_next(provider, cx), + CodeContextMenu::CodeActions(menu) => { + menu.select_next(cx); + InlineCompletionVisibilityChange::Unchanged } - true - } else { - false } } @@ -96,15 +98,13 @@ impl CodeContextMenu { &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) -> bool { - if self.visible() { - match self { - CodeContextMenu::Completions(menu) => menu.select_last(provider, cx), - CodeContextMenu::CodeActions(menu) => menu.select_last(cx), + ) -> InlineCompletionVisibilityChange { + match self { + CodeContextMenu::Completions(menu) => menu.select_last(provider, cx), + CodeContextMenu::CodeActions(menu) => { + menu.select_last(cx); + InlineCompletionVisibilityChange::Unchanged } - true - } else { - false } } @@ -180,6 +180,12 @@ pub(crate) enum CompletionEntry { InlineCompletionHint(InlineCompletionMenuHint), } +#[derive(Clone, Debug)] +pub enum SelectionType { + InlineCompletionSelected, + LspCompletionSelected, +} + impl CompletionsMenu { pub fn new( id: CompletionId, @@ -188,12 +194,17 @@ impl CompletionsMenu { initial_position: Anchor, buffer: Model, completions: Box<[Completion]>, + hint: Option, ) -> Self { let match_candidates = completions .iter() .enumerate() .map(|(id, completion)| StringMatchCandidate::new(id, &completion.label.filter_text())) .collect(); + let entries = match hint { + Some(hint) => vec![CompletionEntry::InlineCompletionHint(hint)], + None => Vec::new(), + }; Self { id, @@ -203,7 +214,7 @@ impl CompletionsMenu { show_completion_documentation, completions: RefCell::new(completions).into(), match_candidates, - entries: RefCell::new(Vec::new()).into(), + entries: RefCell::new(entries).into(), selected_item: 0, scroll_handle: UniformListScrollHandle::new(), resolve_completions: true, @@ -273,79 +284,91 @@ impl CompletionsMenu { &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) { + ) -> InlineCompletionVisibilityChange { let index = if self.scroll_handle.y_flipped() { self.entries.borrow().len() - 1 } else { 0 }; - self.update_selection_index(index, provider, cx); + self.set_selection(index, provider, cx) } fn select_last( &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) { + ) -> InlineCompletionVisibilityChange { let index = if self.scroll_handle.y_flipped() { 0 } else { self.entries.borrow().len() - 1 }; - self.update_selection_index(index, provider, cx); + self.set_selection(index, provider, cx) } fn select_prev( &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) { + ) -> InlineCompletionVisibilityChange { let index = if self.scroll_handle.y_flipped() { self.next_match_index() } else { self.prev_match_index() }; - self.update_selection_index(index, provider, cx); + self.set_selection(index, provider, cx) } fn select_next( &mut self, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) { + ) -> InlineCompletionVisibilityChange { let index = if self.scroll_handle.y_flipped() { self.prev_match_index() } else { self.next_match_index() }; - self.update_selection_index(index, provider, cx); + self.set_selection(index, provider, cx) } - pub fn select_initial_lsp_completion( + pub fn set_selection( &mut self, + index: usize, provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, - ) { - let index = if self.inline_completion_present() { - 1 - } else { - 0 - }; - self.update_selection_index(index, provider, cx); - } - - fn update_selection_index( - &mut self, - match_index: usize, - provider: Option<&dyn CompletionProvider>, - cx: &mut ViewContext, - ) { - if self.selected_item != match_index { - self.selected_item = match_index; + ) -> InlineCompletionVisibilityChange { + if self.selected_item != index { + let entries_count = self.entries.borrow().len(); + if index >= entries_count { + log::error!("index {index} is out of bounds, entries_count = {entries_count}"); + return InlineCompletionVisibilityChange::Unchanged; + } + let inline_completion_was_visible = self.inline_completion_visible_in_buffer(); + self.selected_item = index; self.scroll_handle .scroll_to_item(self.selected_item, ScrollStrategy::Top); self.resolve_visible_completions(provider, cx); cx.notify(); + self.inline_completion_visibility_change(inline_completion_was_visible) + } else { + InlineCompletionVisibilityChange::Unchanged + } + } + + fn inline_completion_visibility_change( + &self, + was_visible: bool, + ) -> InlineCompletionVisibilityChange { + let is_visible = self.inline_completion_visible_in_buffer(); + if is_visible == was_visible { + InlineCompletionVisibilityChange::Unchanged + } else { + if is_visible { + InlineCompletionVisibilityChange::Show + } else { + InlineCompletionVisibilityChange::Hide + } } } @@ -365,17 +388,25 @@ impl CompletionsMenu { } } - pub fn show_inline_completion_hint(&mut self, hint: InlineCompletionMenuHint) { + pub fn show_inline_completion_hint(&mut self, hint: InlineCompletionMenuHint) -> bool { let hint = CompletionEntry::InlineCompletionHint(hint); if self.inline_completion_present() { self.entries.borrow_mut()[0] = hint; } else { self.entries.borrow_mut().insert(0, hint); - // When `y_flipped`, need to scroll to bring it into view. - if self.selected_item == 0 { - self.scroll_handle - .scroll_to_item(self.selected_item, ScrollStrategy::Top); - } + self.selected_item = + (self.selected_item + 1).min(self.entries.borrow().len().saturating_sub(1)); + } + self.inline_completion_visible_in_buffer() + } + + /// Get whether whether LSP completion or inline completion is selected. Used to preserve this + /// across instances of the context menu. + pub fn selection_type(&self) -> SelectionType { + if self.inline_completion_selected() { + SelectionType::InlineCompletionSelected + } else { + SelectionType::LspCompletionSelected } } @@ -392,7 +423,7 @@ impl CompletionsMenu { }) } - fn inline_completion_selected_and_loaded(&self) -> bool { + fn inline_completion_visible_in_buffer(&self) -> bool { self.selected_item == 0 && self.entries.borrow().first().map_or(false, |entry| { matches!( @@ -490,7 +521,13 @@ impl CompletionsMenu { } pub fn visible(&self) -> bool { - !self.entries.borrow().is_empty() + if self.entries.borrow().is_empty() { + return false; + } + if self.entries.borrow().len() == 1 { + return !self.inline_completion_present(); + } + return true; } fn origin(&self, cursor_position: DisplayPoint) -> ContextMenuOrigin { @@ -532,7 +569,7 @@ impl CompletionsMenu { .map(|(ix, _)| ix); drop(completions); - let translucent = self.inline_completion_selected_and_loaded(); + let translucent = self.inline_completion_visible_in_buffer(); if translucent { max_height_in_lines = max_height_in_lines.min(2); } @@ -666,8 +703,7 @@ impl CompletionsMenu { .on_click(cx.listener(move |editor, _event, cx| { cx.stop_propagation(); if translucent { - editor - .context_menu_select_initial_lsp_completion(cx); + editor.context_menu_set_selection(1, cx); } else if let Some(task) = editor.confirm_completion( &ConfirmCompletion { item_ix: Some(item_ix), @@ -849,9 +885,12 @@ impl CompletionsMenu { ) } - pub async fn filter(&mut self, query: Option<&str>, executor: BackgroundExecutor) { - let inline_completion_was_selected = self.inline_completion_selected(); - + pub async fn filter( + &mut self, + query: Option<&str>, + prior_selection_type: Option, + cx: &AsyncWindowContext, + ) -> InlineCompletionVisibilityChange { let mut matches = if let Some(query) = query { fuzzy::match_strings( &self.match_candidates, @@ -859,7 +898,7 @@ impl CompletionsMenu { query.chars().any(|c| c.is_uppercase()), 100, &Default::default(), - executor, + cx.background_executor().clone(), ) .await } else { @@ -945,11 +984,23 @@ impl CompletionsMenu { } drop(completions); + let should_select_inline_completion = match prior_selection_type { + // When there's just one entry for the inline completion, this is the initial load of the + // menu. In that case, the inline completion should be selected if it's loaded, otherwise + // the LSP completion should be selected. The rationale for this is to avoid "perceptual + // data races" where the user is attempting to confirm the inline completion, and the LSP + // completion arrives asynchronously during this. + None if self.entries.borrow().len() == 1 => self.inline_completion_visible_in_buffer(), + None => self.inline_completion_selected(), + Some(SelectionType::InlineCompletionSelected) => true, + Some(SelectionType::LspCompletionSelected) => false, + }; + let mut entries = self.entries.borrow_mut(); let new_selection = if let Some(CompletionEntry::InlineCompletionHint(_)) = entries.first() { entries.truncate(1); - if inline_completion_was_selected || matches.is_empty() { + if should_select_inline_completion || matches.is_empty() { 0 } else { 1 @@ -959,9 +1010,18 @@ impl CompletionsMenu { 0 }; entries.extend(matches.into_iter().map(CompletionEntry::Match)); + drop(entries); + self.selected_item = new_selection; - self.scroll_handle - .scroll_to_item(new_selection, ScrollStrategy::Top); + // Scroll to 0 even if the LSP completion is the only one selected. This keeps the display + // consistent when y_flipped. + self.scroll_handle.scroll_to_item(0, ScrollStrategy::Top); + if self.inline_completion_visible_in_buffer() { + // TODO: Inefficient: This will cause a re-show even though often it's already shown. + InlineCompletionVisibilityChange::Show + } else { + InlineCompletionVisibilityChange::Hide + } } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index bebb3fb33e..297048a6f3 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -74,7 +74,7 @@ use zed_predict_tos::ZedPredictTos; use code_context_menus::{ AvailableCodeAction, CodeActionContents, CodeActionsItem, CodeActionsMenu, CodeContextMenu, - CompletionEntry, CompletionsMenu, ContextMenuOrigin, + CompletionEntry, CompletionsMenu, ContextMenuOrigin, InlineCompletionVisibilityChange, }; use git::blame::GitBlame; use gpui::{ @@ -1964,8 +1964,9 @@ impl Editor { 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()) + let prior_selection_type = completion_menu.selection_type(); + let inline_completion_visibility_change = completion_menu + .filter(query.as_deref(), Some(prior_selection_type), &cx) .await; this.update(&mut cx, |this, cx| { @@ -1981,6 +1982,12 @@ impl Editor { *context_menu = Some(CodeContextMenu::Completions(completion_menu)); drop(context_menu); + + this.handle_inline_completion_visibility_change( + inline_completion_visibility_change, + cx, + ); + cx.notify(); }) }) @@ -3753,11 +3760,27 @@ impl Editor { let id = post_inc(&mut self.next_completion_id); let task = cx.spawn(|editor, mut cx| { async move { - editor.update(&mut cx, |this, _| { + let prior_selection_type = editor.update(&mut cx, |this, _| { this.completion_tasks.retain(|(task_id, _)| *task_id >= id); + match this.context_menu.borrow().as_ref() { + Some(CodeContextMenu::Completions(prev_menu)) => { + Some(prev_menu.selection_type()) + } + _ => None, + } })?; let completions = completions.await.log_err(); let menu = if let Some(completions) = completions { + let inline_completion_menu_hint = editor + .update(&mut cx, |editor, cx| { + if editor.show_inline_completions_in_menu(cx) { + editor.inline_completion_menu_hint(cx) + } else { + None + } + }) + .ok() + .flatten(); let mut menu = CompletionsMenu::new( id, sort_completions, @@ -3765,12 +3788,15 @@ impl Editor { position, buffer.clone(), completions.into(), + inline_completion_menu_hint, ); - menu.filter(query.as_deref(), cx.background_executor().clone()) + let inline_completion_visibility_change = menu + .filter(query.as_deref(), prior_selection_type, &mut cx) .await; - menu.visible().then_some(menu) + menu.visible() + .then_some((menu, inline_completion_visibility_change)) } else { None }; @@ -3787,20 +3813,21 @@ impl Editor { } if editor.focus_handle.is_focused(cx) && menu.is_some() { - let mut menu = menu.unwrap(); + let (mut menu, inline_completion_visibility_change) = menu.unwrap(); menu.resolve_visible_completions(editor.completion_provider.as_deref(), cx); - if editor.show_inline_completions_in_menu(cx) { - if let Some(hint) = editor.inline_completion_menu_hint(cx) { - menu.show_inline_completion_hint(hint); - } - } else { + if !editor.show_inline_completions_in_menu(cx) { editor.discard_inline_completion(false, cx); } *editor.context_menu.borrow_mut() = Some(CodeContextMenu::Completions(menu)); + editor.handle_inline_completion_visibility_change( + inline_completion_visibility_change, + cx, + ); + cx.notify(); } else if editor.completion_tasks.len() <= 1 { // If there are no more completion tasks and the last menu was @@ -4841,6 +4868,38 @@ impl Editor { self.active_inline_completion.is_some() } + pub fn handle_inline_completion_visibility_change( + &mut self, + change: InlineCompletionVisibilityChange, + cx: &mut ViewContext, + ) { + match change { + InlineCompletionVisibilityChange::Unchanged => {} + InlineCompletionVisibilityChange::Hide => { + self.hide_active_inline_completion(cx); + } + InlineCompletionVisibilityChange::Show => { + // TODO: May be more efficient to have this store the info needed to show, instead + // of recomputing. + self.update_visible_inline_completion(cx); + } + } + } + + fn hide_active_inline_completion(&mut self, cx: &mut ViewContext) { + let should_clear = match &mut self.active_inline_completion { + Some(ref mut active_inline_completion) => { + let inlay_ids = std::mem::take(&mut active_inline_completion.inlay_ids); + Some(inlay_ids) + } + _ => None, + }; + if let Some(inlay_ids) = should_clear { + self.splice_inlays(inlay_ids, Default::default(), cx); + self.clear_highlights::(cx); + } + } + fn take_active_inline_completion( &mut self, cx: &mut ViewContext, @@ -4981,11 +5040,16 @@ impl Editor { if self.show_inline_completions_in_menu(cx) && self.has_active_completions_menu() { if let Some(hint) = self.inline_completion_menu_hint(cx) { - match self.context_menu.borrow_mut().as_mut() { + let should_be_visible = match self.context_menu.borrow_mut().as_mut() { Some(CodeContextMenu::Completions(menu)) => { - menu.show_inline_completion_hint(hint); + menu.show_inline_completion_hint(hint) } - _ => {} + _ => true, + }; + if !should_be_visible { + // TODO: Inefficient to hide the splices / highlights that were just inserted + // above. + self.hide_active_inline_completion(cx); } } } @@ -7387,11 +7451,17 @@ impl Editor { if self .context_menu - .borrow_mut() - .as_mut() - .map(|menu| menu.select_first(self.completion_provider.as_deref(), cx)) - .unwrap_or(false) + .borrow() + .as_ref() + .map_or(false, |context_menu| context_menu.visible()) { + let change = self + .context_menu + .borrow_mut() + .as_mut() + .unwrap() + .select_first(self.completion_provider.as_deref(), cx); + self.handle_inline_completion_visibility_change(change, cx); return; } @@ -7496,11 +7566,17 @@ impl Editor { if self .context_menu - .borrow_mut() - .as_mut() - .map(|menu| menu.select_last(self.completion_provider.as_deref(), cx)) - .unwrap_or(false) + .borrow() + .as_ref() + .map_or(false, |context_menu| context_menu.visible()) { + let change = self + .context_menu + .borrow_mut() + .as_mut() + .unwrap() + .select_last(self.completion_provider.as_deref(), cx); + self.handle_inline_completion_visibility_change(change, cx); return; } @@ -7548,40 +7624,53 @@ impl Editor { }); } - pub fn context_menu_select_initial_lsp_completion(&mut self, cx: &mut ViewContext) { + pub fn context_menu_set_selection(&mut self, index: usize, cx: &mut ViewContext) { + let mut change = InlineCompletionVisibilityChange::Unchanged; if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { match context_menu { CodeContextMenu::Completions(completions_menu) => { - completions_menu - .select_initial_lsp_completion(self.completion_provider.as_deref(), cx); + change = completions_menu.set_selection( + index, + self.completion_provider.as_deref(), + cx, + ); } CodeContextMenu::CodeActions(_) => {} } } + self.handle_inline_completion_visibility_change(change, cx); } pub fn context_menu_first(&mut self, _: &ContextMenuFirst, cx: &mut ViewContext) { + let mut change = InlineCompletionVisibilityChange::Unchanged; if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { - context_menu.select_first(self.completion_provider.as_deref(), cx); + change = context_menu.select_first(self.completion_provider.as_deref(), cx); } + self.handle_inline_completion_visibility_change(change, cx); } pub fn context_menu_prev(&mut self, _: &ContextMenuPrev, cx: &mut ViewContext) { + let mut change = InlineCompletionVisibilityChange::Unchanged; if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { - context_menu.select_prev(self.completion_provider.as_deref(), cx); + change = context_menu.select_prev(self.completion_provider.as_deref(), cx); } + self.handle_inline_completion_visibility_change(change, cx); } pub fn context_menu_next(&mut self, _: &ContextMenuNext, cx: &mut ViewContext) { + let mut change = InlineCompletionVisibilityChange::Unchanged; if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { - context_menu.select_next(self.completion_provider.as_deref(), cx); + change = context_menu.select_next(self.completion_provider.as_deref(), cx); } + self.handle_inline_completion_visibility_change(change, cx); } pub fn context_menu_last(&mut self, _: &ContextMenuLast, cx: &mut ViewContext) { + let mut change = InlineCompletionVisibilityChange::Unchanged; if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { - context_menu.select_last(self.completion_provider.as_deref(), cx); + change = context_menu.select_last(self.completion_provider.as_deref(), cx); } + self.handle_inline_completion_visibility_change(change, cx); } pub fn move_to_previous_word_start(