mirrors/zed
1
0
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2025-01-23 18:32:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 12

Solve perceptual data race between LSP completions and edit predictions

Browse source 
1. When there is an edit prediction loaded and LSP completions arrive, select the edit prediction.  This handles the perceptual data race where the user sees a completion in the buffer and doesn't even notice the menu popping up.

2. When there is no edit prediction loaded and LSP completions arrive, select the LSP completion.  This makes use of LSP results more convenient.

3. When the edit prediction entry is loaded and selected, the menu becomes transparent with no text, to show the edit prediction inline in the buffer (see image).  No more display in an aside, so no mental overhead of re-locating the change in the buffer.

4. When an LSP completion entry is selected, do not show an edit prediction in the buffer.

**Not implemented here (WIP):** Since 2 and 4 mean edit predictions are often hidden, it may be less obvious to the user that they are present.  To deal with this, show a preview of the edit prediction in the menu.  These would display the change starting at the completion menu target (e.g. character after `.`).
This commit is contained in:
Michael Sloan 2025-01-22 00:56:21 -07:00
parent b7c77784a8
commit 17b5fc6bae
2 changed files with 257 additions and 108 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -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<Editor>,
) -> 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<Editor>,
) -> 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<Editor>,
) -> 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<Editor>,
) -> 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<Buffer>,
completions: Box<[Completion]>,
hint: Option<InlineCompletionMenuHint>,
) -> 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<Editor>,
) {
) -> 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<Editor>,
) {
) -> 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<Editor>,
) {
) -> 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<Editor>,
) {
) -> 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<Editor>,
) {
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<Editor>,
) {
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<SelectionType>,
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
}
}
}

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

@ -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<Self>,
) {
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<Self>) {
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::<InlineCompletionHighlight>(cx);
}
}
fn take_active_inline_completion(
&mut self,
cx: &mut ViewContext<Self>,
@ -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<Self>) {
pub fn context_menu_set_selection(&mut self, index: usize, cx: &mut ViewContext<Self>) {
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<Self>) {
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<Self>) {
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<Self>) {
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<Self>) {
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(