Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make completion menu entries mutable (#22880)

Browse source 
Release Notes:

- N/A
This commit is contained in:
Michael Sloan 2025-01-08 18:21:56 -07:00 • committed by GitHub
parent 05bc6b2abd
commit af1a3cbaac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 52 additions and 51 deletions
Download patch file Download diff file Expand all files Collapse all files

64
crates/editor/src/code_context_menus.rs
View file

@ -158,7 +158,7 @@ pub struct CompletionsMenu {
pub buffer: Model<Buffer>, pub buffer: Model<Buffer>,
pub completions: Rc<RefCell<Box<[Completion]>>>, pub completions: Rc<RefCell<Box<[Completion]>>>,
match_candidates: Rc<[StringMatchCandidate]>, match_candidates: Rc<[StringMatchCandidate]>,
pub entries: Rc<[CompletionEntry]>, pub entries: Rc<RefCell<Vec<CompletionEntry>>>,
pub selected_item: usize, pub selected_item: usize,
scroll_handle: UniformListScrollHandle, scroll_handle: UniformListScrollHandle,
resolve_completions: bool, resolve_completions: bool,
@ -195,7 +195,7 @@ impl CompletionsMenu {
show_completion_documentation, show_completion_documentation,
completions: RefCell::new(completions).into(), completions: RefCell::new(completions).into(),
match_candidates, match_candidates,
entries: Vec::new().into(), entries: RefCell::new(Vec::new()).into(),
selected_item: 0, selected_item: 0,
scroll_handle: UniformListScrollHandle::new(), scroll_handle: UniformListScrollHandle::new(),
resolve_completions: true, resolve_completions: true,
@ -244,7 +244,7 @@ impl CompletionsMenu {
string: completion.clone(), string: completion.clone(),
}) })
}) })
.collect(); .collect::<Vec<_>>();
Self { Self {
id, id,
sort_completions, sort_completions,
@ -252,7 +252,7 @@ impl CompletionsMenu {
buffer, buffer,
completions: RefCell::new(completions).into(), completions: RefCell::new(completions).into(),
match_candidates, match_candidates,
entries, entries: RefCell::new(entries).into(),
selected_item: 0, selected_item: 0,
scroll_handle: UniformListScrollHandle::new(), scroll_handle: UniformListScrollHandle::new(),
resolve_completions: false, resolve_completions: false,
@ -290,7 +290,8 @@ impl CompletionsMenu {
provider: Option<&dyn CompletionProvider>, provider: Option<&dyn CompletionProvider>,
cx: &mut ViewContext<Editor>, cx: &mut ViewContext<Editor>,
) { ) {
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( fn update_selection_index(
@ -312,12 +313,12 @@ impl CompletionsMenu {
if self.selected_item > 0 { if self.selected_item > 0 {
self.selected_item - 1 self.selected_item - 1
} else { } else {
self.entries.len() - 1 self.entries.borrow().len() - 1
} }
} }
fn next_match_index(&self) -> usize { 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 self.selected_item + 1
} else { } else {
0 0
@ -326,24 +327,18 @@ impl CompletionsMenu {
pub fn show_inline_completion_hint(&mut self, hint: InlineCompletionMenuHint) { pub fn show_inline_completion_hint(&mut self, hint: InlineCompletionMenuHint) {
let hint = CompletionEntry::InlineCompletionHint(hint); let hint = CompletionEntry::InlineCompletionHint(hint);
let mut entries = self.entries.borrow_mut();
self.entries = match self.entries.first() { match entries.first() {
Some(CompletionEntry::InlineCompletionHint { .. }) => { Some(CompletionEntry::InlineCompletionHint { .. }) => {
let mut entries = Vec::from(&*self.entries);
entries[0] = hint; entries[0] = hint;
entries
} }
_ => { _ => {
if self.selected_item != 0 { if self.selected_item != 0 {
self.selected_item += 1; self.selected_item += 1;
} }
let mut entries = Vec::with_capacity(self.entries.len() + 1); entries.insert(0, hint);
entries.push(hint);
entries.extend_from_slice(&self.entries);
entries
} }
} }
.into();
} }
pub fn resolve_visible_completions( pub fn resolve_visible_completions(
@ -369,13 +364,14 @@ impl CompletionsMenu {
let visible_count = last_rendered_range let visible_count = last_rendered_range
.clone() .clone()
.map_or(APPROXIMATE_VISIBLE_COUNT, |range| range.count()); .map_or(APPROXIMATE_VISIBLE_COUNT, |range| range.count());
let entries = self.entries.borrow();
let entry_range = if self.selected_item == 0 { let entry_range = if self.selected_item == 0 {
0..min(visible_count, self.entries.len()) 0..min(visible_count, entries.len())
} else if self.selected_item == self.entries.len() - 1 { } else if self.selected_item == entries.len() - 1 {
self.entries.len().saturating_sub(visible_count)..self.entries.len() entries.len().saturating_sub(visible_count)..entries.len()
} else { } else {
last_rendered_range.map_or(0..0, |range| { 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(), entry_range.clone(),
EXTRA_TO_RESOLVE, EXTRA_TO_RESOLVE,
EXTRA_TO_RESOLVE, EXTRA_TO_RESOLVE,
self.entries.len(), entries.len(),
); );
// Avoid work by sometimes filtering out completions that already have documentation. // Avoid work by sometimes filtering out completions that already have documentation.
// This filtering doesn't happen if the completions are currently being updated. // This filtering doesn't happen if the completions are currently being updated.
let completions = self.completions.borrow(); let completions = self.completions.borrow();
let candidate_ids = entry_indices 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()); .filter(|i| completions[*i].documentation.is_none());
// Current selection is always resolved even if it already has documentation, to handle // Current selection is always resolved even if it already has documentation, to handle
// out-of-spec language servers that return more results later. // 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::<Vec<usize>>(), None => candidate_ids.collect::<Vec<usize>>(),
Some(selected_candidate_id) => iter::once(selected_candidate_id) Some(selected_candidate_id) => iter::once(selected_candidate_id)
.chain(candidate_ids.filter(|id| *id != selected_candidate_id)) .chain(candidate_ids.filter(|id| *id != selected_candidate_id))
.collect::<Vec<usize>>(), .collect::<Vec<usize>>(),
}; };
drop(entries);
if candidate_ids.is_empty() { if candidate_ids.is_empty() {
return; return;
@ -432,7 +429,7 @@ impl CompletionsMenu {
} }
pub fn visible(&self) -> bool { pub fn visible(&self) -> bool {
!self.entries.is_empty() !self.entries.borrow().is_empty()
} }
fn origin(&self, cursor_position: DisplayPoint) -> ContextMenuOrigin { fn origin(&self, cursor_position: DisplayPoint) -> ContextMenuOrigin {
@ -449,6 +446,7 @@ impl CompletionsMenu {
let show_completion_documentation = self.show_completion_documentation; let show_completion_documentation = self.show_completion_documentation;
let widest_completion_ix = self let widest_completion_ix = self
.entries .entries
.borrow()
.iter() .iter()
.enumerate() .enumerate()
.max_by_key(|(_, mat)| match mat { .max_by_key(|(_, mat)| match mat {
@ -475,19 +473,19 @@ impl CompletionsMenu {
let selected_item = self.selected_item; let selected_item = self.selected_item;
let completions = self.completions.clone(); 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 last_rendered_range = self.last_rendered_range.clone();
let style = style.clone(); let style = style.clone();
let list = uniform_list( let list = uniform_list(
cx.view().clone(), cx.view().clone(),
"completions", "completions",
matches.len(), self.entries.borrow().len(),
move |_editor, range, cx| { move |_editor, range, cx| {
last_rendered_range.borrow_mut().replace(range.clone()); last_rendered_range.borrow_mut().replace(range.clone());
let start_ix = range.start; let start_ix = range.start;
let completions_guard = completions.borrow_mut(); let completions_guard = completions.borrow_mut();
matches[range] entries.borrow()[range]
.iter() .iter()
.enumerate() .enumerate()
.map(|(ix, mat)| { .map(|(ix, mat)| {
@ -623,7 +621,7 @@ impl CompletionsMenu {
return None; return None;
} }
let multiline_docs = match &self.entries[self.selected_item] { let multiline_docs = match &self.entries.borrow()[self.selected_item] {
CompletionEntry::Match(mat) => { CompletionEntry::Match(mat) => {
match self.completions.borrow_mut()[mat.candidate_id] match self.completions.borrow_mut()[mat.candidate_id]
.documentation .documentation
@ -769,12 +767,14 @@ impl CompletionsMenu {
} }
drop(completions); drop(completions);
let mut new_entries: Vec<_> = matches.into_iter().map(CompletionEntry::Match).collect(); let mut entries = self.entries.borrow_mut();
if let Some(CompletionEntry::InlineCompletionHint(hint)) = self.entries.first() { if let Some(CompletionEntry::InlineCompletionHint(_)) = entries.first() {
new_entries.insert(0, CompletionEntry::InlineCompletionHint(hint.clone())); entries.truncate(1);
} else {
entries.truncate(0);
} }
entries.extend(matches.into_iter().map(CompletionEntry::Match));
self.entries = new_entries.into();
self.selected_item = 0; self.selected_item = 0;
} }
} }

18
crates/editor/src/editor.rs
View file

@ -3815,10 +3815,8 @@ impl Editor {
return None; return None;
}; };
let mat = completions_menu let entries = completions_menu.entries.borrow();
.entries let mat = entries.get(item_ix.unwrap_or(completions_menu.selected_item))?;
.get(item_ix.unwrap_or(completions_menu.selected_item))?;
let mat = match mat { let mat = match mat {
CompletionEntry::InlineCompletionHint { .. } => { CompletionEntry::InlineCompletionHint { .. } => {
self.accept_inline_completion(&AcceptInlineCompletion, cx); self.accept_inline_completion(&AcceptInlineCompletion, cx);
@ -3832,12 +3830,14 @@ impl Editor {
mat mat
} }
}; };
let candidate_id = mat.candidate_id;
drop(entries);
let buffer_handle = completions_menu.buffer; let buffer_handle = completions_menu.buffer;
let completion = completions_menu let completion = completions_menu
.completions .completions
.borrow() .borrow()
.get(mat.candidate_id)? .get(candidate_id)?
.clone(); .clone();
cx.stop_propagation(); cx.stop_propagation();
@ -3986,7 +3986,7 @@ impl Editor {
let apply_edits = provider.apply_additional_edits_for_completion( let apply_edits = provider.apply_additional_edits_for_completion(
buffer_handle, buffer_handle,
completions_menu.completions.clone(), completions_menu.completions.clone(),
mat.candidate_id, candidate_id,
true, true,
cx, cx,
); );
@ -5110,9 +5110,11 @@ impl Editor {
.borrow() .borrow()
.as_ref() .as_ref()
.map_or(false, |menu| match menu { .map_or(false, |menu| match menu {
CodeContextMenu::Completions(menu) => menu.entries.first().map_or(false, |entry| { CodeContextMenu::Completions(menu) => {
menu.entries.borrow().first().map_or(false, |entry| {
matches!(entry, CompletionEntry::InlineCompletionHint(_)) matches!(entry, CompletionEntry::InlineCompletionHint(_))
}), })
}
CodeContextMenu::CodeActions(_) => false, CodeContextMenu::CodeActions(_) => false,
}) })
} }

19
crates/editor/src/editor_tests.rs
View file

@ -8473,7 +8473,7 @@ async fn test_completion_page_up_down_keys(cx: &mut gpui::TestAppContext) {
cx.update_editor(|editor, _| { cx.update_editor(|editor, _| {
if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() 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 { } else {
panic!("expected completion menu to be open"); 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() if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref()
{ {
assert_eq!( assert_eq!(
completion_menu_entries(&menu.entries), completion_menu_entries(&menu),
&["r", "ret", "Range", "return"] &["r", "ret", "Range", "return"]
); );
} else { } else {
@ -11080,6 +11080,7 @@ async fn test_completions_default_resolve_data_handling(cx: &mut gpui::TestAppCo
assert_eq!( assert_eq!(
completions_menu completions_menu
.entries .entries
.borrow()
.iter() .iter()
.flat_map(|c| match c { .flat_map(|c| match c {
CompletionEntry::Match(mat) => Some(mat.string.clone()), 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() if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref()
{ {
assert_eq!( assert_eq!(
completion_menu_entries(&menu.entries), completion_menu_entries(&menu),
&["bg-red", "bg-blue", "bg-yellow"] &["bg-red", "bg-blue", "bg-yellow"]
); );
} else { } else {
@ -11203,10 +11204,7 @@ async fn test_completions_in_languages_with_extra_word_characters(cx: &mut gpui:
cx.update_editor(|editor, _| { cx.update_editor(|editor, _| {
if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref()
{ {
assert_eq!( assert_eq!(completion_menu_entries(&menu), &["bg-blue", "bg-yellow"]);
completion_menu_entries(&menu.entries),
&["bg-blue", "bg-yellow"]
);
} else { } else {
panic!("expected completion menu to be open"); 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, _| { cx.update_editor(|editor, _| {
if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() 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 { } else {
panic!("expected completion menu to be open"); panic!("expected completion menu to be open");
} }
}); });
} }
fn completion_menu_entries(entries: &[CompletionEntry]) -> Vec<&str> { fn completion_menu_entries(menu: &CompletionsMenu) -> Vec<String> {
let entries = menu.entries.borrow();
entries entries
.iter() .iter()
.flat_map(|e| match e { .flat_map(|e| match e {
CompletionEntry::Match(mat) => Some(mat.string.as_str()), CompletionEntry::Match(mat) => Some(mat.string.clone()),
_ => None, _ => None,
}) })
.collect() .collect()