Skip to content

Commit

Permalink
Make completion menu entries mutable (#22880)
Browse files Browse the repository at this point in the history 
Release Notes:

- N/A
  • Loading branch information
@mgsloan
mgsloan authored Jan 9, 2025
1 parent 05bc6b2 commit af1a3cb
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 51 deletions.
64 changes: 32 additions & 32 deletions crates/editor/src/code_context_menus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub struct CompletionsMenu {
pub buffer: Model<Buffer>,
pub completions: Rc<RefCell<Box<[Completion]>>>,
match_candidates: Rc<[StringMatchCandidate]>,
pub entries: Rc<[CompletionEntry]>,
pub entries: Rc<RefCell<Vec<CompletionEntry>>>,
pub selected_item: usize,
scroll_handle: UniformListScrollHandle,
resolve_completions: bool,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -244,15 +244,15 @@ impl CompletionsMenu {
string: completion.clone(),
})
})
.collect();
.collect::<Vec<_>>();
Self {
id,
sort_completions,
initial_position: selection.start,
buffer,
completions: RefCell::new(completions).into(),
match_candidates,
entries,
entries: RefCell::new(entries).into(),
selected_item: 0,
scroll_handle: UniformListScrollHandle::new(),
resolve_completions: false,
Expand Down Expand Up @@ -290,7 +290,8 @@ impl CompletionsMenu {
provider: Option<&dyn CompletionProvider>,
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(
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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())
})
};

Expand All @@ -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::<Vec<usize>>(),
Some(selected_candidate_id) => iter::once(selected_candidate_id)
.chain(candidate_ids.filter(|id| *id != selected_candidate_id))
.collect::<Vec<usize>>(),
};
drop(entries);

if candidate_ids.is_empty() {
return;
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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)| {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
Expand Down
20 changes: 11 additions & 9 deletions crates/editor/src/editor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3815,10 +3815,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);
Expand All @@ -3832,12 +3830,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();

Expand Down Expand Up @@ -3986,7 +3986,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,
);
Expand Down Expand Up @@ -5110,9 +5110,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,
})
}
Expand Down
19 changes: 9 additions & 10 deletions crates/editor/src/editor_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -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 {
Expand All @@ -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");
}
Expand All @@ -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<String> {
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()
Expand Down

0 comments on commit af1a3cb

Please sign in to comment.