From 96c5be5fa5f9997a66595370902fd029be5207df Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Wed, 8 Jan 2025 18:21:56 -0700 Subject: [PATCH] Make completion menu entries mutable (#22880) Release Notes: - N/A --- crates/editor/src/code_context_menus.rs | 64 ++++++++++++------------- crates/editor/src/editor.rs | 20 ++++---- crates/editor/src/editor_tests.rs | 19 ++++---- 3 files changed, 52 insertions(+), 51 deletions(-) diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index a6e93351c4..0408d10a20 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -158,7 +158,7 @@ pub struct CompletionsMenu { pub buffer: Model, pub completions: Rc>>, match_candidates: Rc<[StringMatchCandidate]>, - pub entries: Rc<[CompletionEntry]>, + pub entries: Rc>>, pub selected_item: usize, scroll_handle: UniformListScrollHandle, resolve_completions: bool, @@ -195,7 +195,7 @@ impl CompletionsMenu { show_completion_documentation, completions: RefCell::new(completions).into(), match_candidates, - entries: Vec::new().into(), + entries: RefCell::new(Vec::new()).into(), selected_item: 0, scroll_handle: UniformListScrollHandle::new(), resolve_completions: true, @@ -244,7 +244,7 @@ impl CompletionsMenu { string: completion.clone(), }) }) - .collect(); + .collect::>(); Self { id, sort_completions, @@ -252,7 +252,7 @@ impl CompletionsMenu { buffer, completions: RefCell::new(completions).into(), match_candidates, - entries, + entries: RefCell::new(entries).into(), selected_item: 0, scroll_handle: UniformListScrollHandle::new(), resolve_completions: false, @@ -290,7 +290,8 @@ impl CompletionsMenu { provider: Option<&dyn CompletionProvider>, cx: &mut ViewContext, ) { - self.update_selection_index(self.entries.len() - 1, provider, cx); + let index = self.entries.borrow().len() - 1; + self.update_selection_index(index, provider, cx); } fn update_selection_index( @@ -312,12 +313,12 @@ impl CompletionsMenu { if self.selected_item > 0 { self.selected_item - 1 } else { - self.entries.len() - 1 + self.entries.borrow().len() - 1 } } fn next_match_index(&self) -> usize { - if self.selected_item + 1 < self.entries.len() { + if self.selected_item + 1 < self.entries.borrow().len() { self.selected_item + 1 } else { 0 @@ -326,24 +327,18 @@ impl CompletionsMenu { pub fn show_inline_completion_hint(&mut self, hint: InlineCompletionMenuHint) { let hint = CompletionEntry::InlineCompletionHint(hint); - - self.entries = match self.entries.first() { + let mut entries = self.entries.borrow_mut(); + match entries.first() { Some(CompletionEntry::InlineCompletionHint { .. }) => { - let mut entries = Vec::from(&*self.entries); entries[0] = hint; - entries } _ => { if self.selected_item != 0 { self.selected_item += 1; } - let mut entries = Vec::with_capacity(self.entries.len() + 1); - entries.push(hint); - entries.extend_from_slice(&self.entries); - entries + entries.insert(0, hint); } } - .into(); } pub fn resolve_visible_completions( @@ -369,13 +364,14 @@ impl CompletionsMenu { let visible_count = last_rendered_range .clone() .map_or(APPROXIMATE_VISIBLE_COUNT, |range| range.count()); + let entries = self.entries.borrow(); let entry_range = if self.selected_item == 0 { - 0..min(visible_count, self.entries.len()) - } else if self.selected_item == self.entries.len() - 1 { - self.entries.len().saturating_sub(visible_count)..self.entries.len() + 0..min(visible_count, entries.len()) + } else if self.selected_item == entries.len() - 1 { + entries.len().saturating_sub(visible_count)..entries.len() } else { last_rendered_range.map_or(0..0, |range| { - min(range.start, self.entries.len())..min(range.end, self.entries.len()) + min(range.start, entries.len())..min(range.end, entries.len()) }) }; @@ -386,24 +382,25 @@ impl CompletionsMenu { entry_range.clone(), EXTRA_TO_RESOLVE, EXTRA_TO_RESOLVE, - self.entries.len(), + entries.len(), ); // Avoid work by sometimes filtering out completions that already have documentation. // This filtering doesn't happen if the completions are currently being updated. let completions = self.completions.borrow(); let candidate_ids = entry_indices - .flat_map(|i| Self::entry_candidate_id(&self.entries[i])) + .flat_map(|i| Self::entry_candidate_id(&entries[i])) .filter(|i| completions[*i].documentation.is_none()); // Current selection is always resolved even if it already has documentation, to handle // out-of-spec language servers that return more results later. - let candidate_ids = match Self::entry_candidate_id(&self.entries[self.selected_item]) { + let candidate_ids = match Self::entry_candidate_id(&entries[self.selected_item]) { None => candidate_ids.collect::>(), Some(selected_candidate_id) => iter::once(selected_candidate_id) .chain(candidate_ids.filter(|id| *id != selected_candidate_id)) .collect::>(), }; + drop(entries); if candidate_ids.is_empty() { return; @@ -432,7 +429,7 @@ impl CompletionsMenu { } pub fn visible(&self) -> bool { - !self.entries.is_empty() + !self.entries.borrow().is_empty() } fn origin(&self, cursor_position: DisplayPoint) -> ContextMenuOrigin { @@ -449,6 +446,7 @@ impl CompletionsMenu { let show_completion_documentation = self.show_completion_documentation; let widest_completion_ix = self .entries + .borrow() .iter() .enumerate() .max_by_key(|(_, mat)| match mat { @@ -475,19 +473,19 @@ impl CompletionsMenu { let selected_item = self.selected_item; let completions = self.completions.clone(); - let matches = self.entries.clone(); + let entries = self.entries.clone(); let last_rendered_range = self.last_rendered_range.clone(); let style = style.clone(); let list = uniform_list( cx.view().clone(), "completions", - matches.len(), + self.entries.borrow().len(), move |_editor, range, cx| { last_rendered_range.borrow_mut().replace(range.clone()); let start_ix = range.start; let completions_guard = completions.borrow_mut(); - matches[range] + entries.borrow()[range] .iter() .enumerate() .map(|(ix, mat)| { @@ -623,7 +621,7 @@ impl CompletionsMenu { return None; } - let multiline_docs = match &self.entries[self.selected_item] { + let multiline_docs = match &self.entries.borrow()[self.selected_item] { CompletionEntry::Match(mat) => { match self.completions.borrow_mut()[mat.candidate_id] .documentation @@ -769,12 +767,14 @@ impl CompletionsMenu { } drop(completions); - let mut new_entries: Vec<_> = matches.into_iter().map(CompletionEntry::Match).collect(); - if let Some(CompletionEntry::InlineCompletionHint(hint)) = self.entries.first() { - new_entries.insert(0, CompletionEntry::InlineCompletionHint(hint.clone())); + let mut entries = self.entries.borrow_mut(); + if let Some(CompletionEntry::InlineCompletionHint(_)) = entries.first() { + entries.truncate(1); + } else { + entries.truncate(0); } + entries.extend(matches.into_iter().map(CompletionEntry::Match)); - self.entries = new_entries.into(); self.selected_item = 0; } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index afc273b39b..13e66e837e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3830,10 +3830,8 @@ impl Editor { return None; }; - let mat = completions_menu - .entries - .get(item_ix.unwrap_or(completions_menu.selected_item))?; - + let entries = completions_menu.entries.borrow(); + let mat = entries.get(item_ix.unwrap_or(completions_menu.selected_item))?; let mat = match mat { CompletionEntry::InlineCompletionHint { .. } => { self.accept_inline_completion(&AcceptInlineCompletion, cx); @@ -3847,12 +3845,14 @@ impl Editor { mat } }; + let candidate_id = mat.candidate_id; + drop(entries); let buffer_handle = completions_menu.buffer; let completion = completions_menu .completions .borrow() - .get(mat.candidate_id)? + .get(candidate_id)? .clone(); cx.stop_propagation(); @@ -4001,7 +4001,7 @@ impl Editor { let apply_edits = provider.apply_additional_edits_for_completion( buffer_handle, completions_menu.completions.clone(), - mat.candidate_id, + candidate_id, true, cx, ); @@ -5158,9 +5158,11 @@ impl Editor { .borrow() .as_ref() .map_or(false, |menu| match menu { - CodeContextMenu::Completions(menu) => menu.entries.first().map_or(false, |entry| { - matches!(entry, CompletionEntry::InlineCompletionHint(_)) - }), + CodeContextMenu::Completions(menu) => { + menu.entries.borrow().first().map_or(false, |entry| { + matches!(entry, CompletionEntry::InlineCompletionHint(_)) + }) + } CodeContextMenu::CodeActions(_) => false, }) } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 16909ef58b..0ba540f2b1 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -8473,7 +8473,7 @@ async fn test_completion_page_up_down_keys(cx: &mut gpui::TestAppContext) { cx.update_editor(|editor, _| { if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() { - assert_eq!(completion_menu_entries(&menu.entries), &["first", "last"]); + assert_eq!(completion_menu_entries(&menu), &["first", "last"]); } else { panic!("expected completion menu to be open"); } @@ -8566,7 +8566,7 @@ async fn test_completion_sort(cx: &mut gpui::TestAppContext) { if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() { assert_eq!( - completion_menu_entries(&menu.entries), + completion_menu_entries(&menu), &["r", "ret", "Range", "return"] ); } else { @@ -11080,6 +11080,7 @@ async fn test_completions_default_resolve_data_handling(cx: &mut gpui::TestAppCo assert_eq!( completions_menu .entries + .borrow() .iter() .flat_map(|c| match c { CompletionEntry::Match(mat) => Some(mat.string.clone()), @@ -11190,7 +11191,7 @@ async fn test_completions_in_languages_with_extra_word_characters(cx: &mut gpui: if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() { assert_eq!( - completion_menu_entries(&menu.entries), + completion_menu_entries(&menu), &["bg-red", "bg-blue", "bg-yellow"] ); } else { @@ -11203,10 +11204,7 @@ async fn test_completions_in_languages_with_extra_word_characters(cx: &mut gpui: cx.update_editor(|editor, _| { if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() { - assert_eq!( - completion_menu_entries(&menu.entries), - &["bg-blue", "bg-yellow"] - ); + assert_eq!(completion_menu_entries(&menu), &["bg-blue", "bg-yellow"]); } else { panic!("expected completion menu to be open"); } @@ -11220,18 +11218,19 @@ async fn test_completions_in_languages_with_extra_word_characters(cx: &mut gpui: cx.update_editor(|editor, _| { if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() { - assert_eq!(completion_menu_entries(&menu.entries), &["bg-yellow"]); + assert_eq!(completion_menu_entries(&menu), &["bg-yellow"]); } else { panic!("expected completion menu to be open"); } }); } -fn completion_menu_entries(entries: &[CompletionEntry]) -> Vec<&str> { +fn completion_menu_entries(menu: &CompletionsMenu) -> Vec { + let entries = menu.entries.borrow(); entries .iter() .flat_map(|e| match e { - CompletionEntry::Match(mat) => Some(mat.string.as_str()), + CompletionEntry::Match(mat) => Some(mat.string.clone()), _ => None, }) .collect()