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

editor: Revert flattening of code actions in mouse context menu (#28988)

Browse source 
In light of making context not move dynamically, reverting back these
changes.

- Doing it async will lead to a loading state, which moves the context
menu.
- Doing it sync introduces noticeable lag in opening the context menu.
   
Future idea is to introduce fixed code actions like refactor, rewrite,
etc depending on code action kind [(see
more)](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionKind)
which will use submenus.
 
Release Notes:

- N/A
This commit is contained in:
Smit Barmase 2025-04-17 23:48:51 +05:30 • committed by GitHub
parent 7e928dd615
commit ba588161d9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 170 additions and 393 deletions
Download patch file Download diff file Expand all files Collapse all files

10
crates/collab/src/tests/editor_tests.rs
View file

@ -694,15 +694,7 @@ async fn test_collaborating_with_code_actions(
// Confirming the code action will trigger a resolve request.
let confirm_action = editor_b
.update_in(cx_b, |editor, window, cx| {
Editor::confirm_code_action(
editor,
&ConfirmCodeAction {
item_ix: Some(0),
from_mouse_context_menu: false,
},
window,
cx,
)
Editor::confirm_code_action(editor, &ConfirmCodeAction { item_ix: Some(0) }, window, cx)
})
.unwrap();
fake_language_server.set_request_handler::<lsp::request::CodeActionResolveRequest, _, _>(

3
crates/editor/src/actions.rs
View file

@ -99,9 +99,6 @@ pub struct ComposeCompletion {
pub struct ConfirmCodeAction {
#[serde(default)]
pub item_ix: Option<usize>,
#[serde(default)]
#[serde(skip)]
pub from_mouse_context_menu: bool,
}
#[derive(PartialEq, Clone, Deserialize, Default, JsonSchema)]

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

@ -775,7 +775,7 @@ pub struct AvailableCodeAction {
pub provider: Rc<dyn CodeActionProvider>,
}
#[derive(Clone, Default)]
#[derive(Clone)]
pub struct CodeActionContents {
pub tasks: Option<Rc<ResolvedTasks>>,
pub actions: Option<Rc<[AvailableCodeAction]>>,
@ -791,7 +791,7 @@ impl CodeActionContents {
}
}
pub fn is_empty(&self) -> bool {
fn is_empty(&self) -> bool {
match (&self.tasks, &self.actions) {
(Some(tasks), Some(actions)) => actions.is_empty() && tasks.templates.is_empty(),
(Some(tasks), None) => tasks.templates.is_empty(),
@ -800,7 +800,7 @@ impl CodeActionContents {
}
}
pub fn iter(&self) -> impl Iterator<Item = CodeActionsItem> + '_ {
fn iter(&self) -> impl Iterator<Item = CodeActionsItem> + '_ {
self.tasks
.iter()
.flat_map(|tasks| {
@ -868,14 +868,14 @@ pub enum CodeActionsItem {
}
impl CodeActionsItem {
pub fn as_task(&self) -> Option<&ResolvedTask> {
fn as_task(&self) -> Option<&ResolvedTask> {
let Self::Task(_, task) = self else {
return None;
};
Some(task)
}
pub fn as_code_action(&self) -> Option<&CodeAction> {
fn as_code_action(&self) -> Option<&CodeAction> {
let Self::CodeAction { action, .. } = self else {
return None;
};
@ -1015,7 +1015,6 @@ impl CodeActionsMenu {
if let Some(task) = editor.confirm_code_action(
&ConfirmCodeAction {
item_ix: Some(item_ix),
from_mouse_context_menu: false,
},
window,
cx,
@ -1041,7 +1040,6 @@ impl CodeActionsMenu {
if let Some(task) = editor.confirm_code_action(
&ConfirmCodeAction {
item_ix: Some(item_ix),
from_mouse_context_menu: false,
},
window,
cx,

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

@ -1790,7 +1790,6 @@ impl Editor {
self,
crate::mouse_context_menu::MenuPosition::PinnedToScreen(position),
context_menu,
None,
window,
cx,
));
@ -4933,89 +4932,6 @@ impl Editor {
}))
}
fn prepare_code_actions_task(
&mut self,
action: &ToggleCodeActions,
window: &mut Window,
cx: &mut Context<Self>,
) -> Task<Option<(Entity<Buffer>, CodeActionContents)>> {
let snapshot = self.snapshot(window, cx);
let multibuffer_point = action
.deployed_from_indicator
.map(|row| DisplayPoint::new(row, 0).to_point(&snapshot))
.unwrap_or_else(|| self.selections.newest::<Point>(cx).head());
let Some((buffer, buffer_row)) = snapshot
.buffer_snapshot
.buffer_line_for_row(MultiBufferRow(multibuffer_point.row))
.and_then(|(buffer_snapshot, range)| {
self.buffer
.read(cx)
.buffer(buffer_snapshot.remote_id())
.map(|buffer| (buffer, range.start.row))
})
else {
return Task::ready(None);
};
let (_, code_actions) = self
.available_code_actions
.clone()
.and_then(|(location, code_actions)| {
let snapshot = location.buffer.read(cx).snapshot();
let point_range = location.range.to_point(&snapshot);
let point_range = point_range.start.row..=point_range.end.row;
if point_range.contains(&buffer_row) {
Some((location, code_actions))
} else {
None
}
})
.unzip();
let buffer_id = buffer.read(cx).remote_id();
let tasks = self
.tasks
.get(&(buffer_id, buffer_row))
.map(|t| Arc::new(t.to_owned()));
if tasks.is_none() && code_actions.is_none() {
return Task::ready(None);
}
self.completion_tasks.clear();
self.discard_inline_completion(false, cx);
let task_context = tasks
.as_ref()
.zip(self.project.clone())
.map(|(tasks, project)| {
Self::build_tasks_context(&project, &buffer, buffer_row, tasks, cx)
});
cx.spawn_in(window, async move |_, _| {
let task_context = match task_context {
Some(task_context) => task_context.await,
None => None,
};
let resolved_tasks = tasks.zip(task_context).map(|(tasks, task_context)| {
Rc::new(ResolvedTasks {
templates: tasks.resolve(&task_context).collect(),
position: snapshot
.buffer_snapshot
.anchor_before(Point::new(multibuffer_point.row, tasks.column)),
})
});
Some((
buffer,
CodeActionContents {
actions: code_actions,
tasks: resolved_tasks,
},
))
})
}
pub fn toggle_code_actions(
&mut self,
action: &ToggleCodeActions,
@ -5036,58 +4952,113 @@ impl Editor {
}
}
drop(context_menu);
let snapshot = self.snapshot(window, cx);
let deployed_from_indicator = action.deployed_from_indicator;
let mut task = self.code_actions_task.take();
let action = action.clone();
cx.spawn_in(window, async move |editor, cx| {
while let Some(prev_task) = task {
prev_task.await.log_err();
task = editor.update(cx, |this, _| this.code_actions_task.take())?;
}
let context_menu_task = editor.update_in(cx, |editor, window, cx| {
if !editor.focus_handle.is_focused(window) {
return Some(Task::ready(Ok(())));
}
let debugger_flag = cx.has_flag::<Debugger>();
let code_actions_task = editor.prepare_code_actions_task(&action, window, cx);
Some(cx.spawn_in(window, async move |editor, cx| {
if let Some((buffer, code_action_contents)) = code_actions_task.await {
let spawn_straight_away =
code_action_contents.tasks.as_ref().map_or(false, |tasks| {
tasks
.templates
.iter()
.filter(|task| {
if matches!(task.1.task_type(), task::TaskType::Debug(_)) {
debugger_flag
} else {
true
}
})
.count()
== 1
}) && code_action_contents
.actions
.as_ref()
.map_or(true, |actions| actions.is_empty());
let spawned_test_task = editor.update_in(cx, |editor, window, cx| {
if editor.focus_handle.is_focused(window) {
let multibuffer_point = action
.deployed_from_indicator
.map(|row| DisplayPoint::new(row, 0).to_point(&snapshot))
.unwrap_or_else(|| editor.selections.newest::<Point>(cx).head());
let (buffer, buffer_row) = snapshot
.buffer_snapshot
.buffer_line_for_row(MultiBufferRow(multibuffer_point.row))
.and_then(|(buffer_snapshot, range)| {
editor
.buffer
.read(cx)
.buffer(buffer_snapshot.remote_id())
.map(|buffer| (buffer, range.start.row))
})?;
let (_, code_actions) = editor
.available_code_actions
.clone()
.and_then(|(location, code_actions)| {
let snapshot = location.buffer.read(cx).snapshot();
let point_range = location.range.to_point(&snapshot);
let point_range = point_range.start.row..=point_range.end.row;
if point_range.contains(&buffer_row) {
Some((location, code_actions))
} else {
None
}
})
.unzip();
let buffer_id = buffer.read(cx).remote_id();
let tasks = editor
.tasks
.get(&(buffer_id, buffer_row))
.map(|t| Arc::new(t.to_owned()));
if tasks.is_none() && code_actions.is_none() {
return None;
}
editor.completion_tasks.clear();
editor.discard_inline_completion(false, cx);
let task_context =
tasks
.as_ref()
.zip(editor.project.clone())
.map(|(tasks, project)| {
Self::build_tasks_context(&project, &buffer, buffer_row, tasks, cx)
});
let debugger_flag = cx.has_flag::<Debugger>();
Some(cx.spawn_in(window, async move |editor, cx| {
let task_context = match task_context {
Some(task_context) => task_context.await,
None => None,
};
let resolved_tasks =
tasks.zip(task_context).map(|(tasks, task_context)| {
Rc::new(ResolvedTasks {
templates: tasks.resolve(&task_context).collect(),
position: snapshot.buffer_snapshot.anchor_before(Point::new(
multibuffer_point.row,
tasks.column,
)),
})
});
let spawn_straight_away = resolved_tasks.as_ref().map_or(false, |tasks| {
tasks
.templates
.iter()
.filter(|task| {
if matches!(task.1.task_type(), task::TaskType::Debug(_)) {
debugger_flag
} else {
true
}
})
.count()
== 1
}) && code_actions
.as_ref()
.map_or(true, |actions| actions.is_empty());
if let Ok(task) = editor.update_in(cx, |editor, window, cx| {
*editor.context_menu.borrow_mut() =
Some(CodeContextMenu::CodeActions(CodeActionsMenu {
buffer,
actions: code_action_contents,
actions: CodeActionContents {
tasks: resolved_tasks,
actions: code_actions,
},
selected_item: Default::default(),
scroll_handle: UniformListScrollHandle::default(),
deployed_from_indicator,
}));
if spawn_straight_away {
if let Some(task) = editor.confirm_code_action(
&ConfirmCodeAction {
item_ix: Some(0),
from_mouse_context_menu: false,
},
&ConfirmCodeAction { item_ix: Some(0) },
window,
cx,
) {
@ -5102,12 +5073,12 @@ impl Editor {
} else {
Ok(())
}
} else {
Ok(())
}
}))
}))
} else {
Some(Task::ready(Ok(())))
}
})?;
if let Some(task) = context_menu_task {
if let Some(task) = spawned_test_task {
task.await?;
}
@ -5124,27 +5095,17 @@ impl Editor {
) -> Option<Task<Result<()>>> {
self.hide_mouse_cursor(&HideMouseCursorOrigin::TypingAction);
let (action, buffer) = if action.from_mouse_context_menu {
if let Some(menu) = self.mouse_context_menu.take() {
let code_action = menu.code_action?;
let index = action.item_ix?;
let action = code_action.actions.get(index)?;
(action, code_action.buffer)
} else {
return None;
}
} else {
let actions_menu =
if let CodeContextMenu::CodeActions(menu) = self.hide_context_menu(window, cx)? {
let action_ix = action.item_ix.unwrap_or(menu.selected_item);
let action = menu.actions.get(action_ix)?;
let buffer = menu.buffer;
(action, buffer)
menu
} else {
return None;
}
};
};
let action_ix = action.item_ix.unwrap_or(actions_menu.selected_item);
let action = actions_menu.actions.get(action_ix)?;
let title = action.label();
let buffer = actions_menu.buffer;
let workspace = self.workspace()?;
match action {
@ -8941,7 +8902,6 @@ impl Editor {
self,
source,
clicked_point,
None,
context_menu,
window,
cx,

300
crates/editor/src/mouse_context_menu.rs
View file

@ -1,22 +1,15 @@
use crate::{
ConfirmCodeAction, Copy, CopyAndTrim, CopyPermalinkToLine, Cut, DebuggerEvaluateSelectedText,
DisplayPoint, DisplaySnapshot, Editor, FindAllReferences, GoToDeclaration, GoToDefinition,
Copy, CopyAndTrim, CopyPermalinkToLine, Cut, DebuggerEvaluateSelectedText, DisplayPoint,
DisplaySnapshot, Editor, FindAllReferences, GoToDeclaration, GoToDefinition,
GoToImplementation, GoToTypeDefinition, Paste, Rename, RevealInFileManager, SelectMode,
SelectionExt, ToDisplayPoint, ToggleCodeActions,
actions::{Format, FormatSelections},
code_context_menus::CodeActionContents,
selections_collection::SelectionsCollection,
};
use feature_flags::{Debugger, FeatureFlagAppExt as _};
use gpui::prelude::FluentBuilder;
use gpui::{
Context, DismissEvent, Entity, FocusHandle, Focusable as _, Pixels, Point, Subscription, Task,
Window,
};
use gpui::{Context, DismissEvent, Entity, Focusable as _, Pixels, Point, Subscription, Window};
use std::ops::Range;
use text::PointUtf16;
use ui::ContextMenu;
use util::ResultExt;
use workspace::OpenInTerminal;
#[derive(Debug)]
@ -32,24 +25,13 @@ pub enum MenuPosition {
},
}
pub struct MouseCodeAction {
pub actions: CodeActionContents,
pub buffer: Entity<language::Buffer>,
}
pub struct MouseContextMenu {
pub(crate) position: MenuPosition,
pub(crate) context_menu: Entity<ui::ContextMenu>,
pub(crate) code_action: Option<MouseCodeAction>,
_dismiss_subscription: Subscription,
_cursor_move_subscription: Subscription,
}
enum CodeActionLoadState {
Loading,
Loaded(CodeActionContents),
}
impl std::fmt::Debug for MouseContextMenu {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("MouseContextMenu")
@ -64,7 +46,6 @@ impl MouseContextMenu {
editor: &mut Editor,
source: multi_buffer::Anchor,
position: Point<Pixels>,
code_action: Option<MouseCodeAction>,
context_menu: Entity<ui::ContextMenu>,
window: &mut Window,
cx: &mut Context<Editor>,
@ -84,7 +65,6 @@ impl MouseContextMenu {
editor,
menu_position,
context_menu,
code_action,
window,
cx,
));
@ -94,7 +74,6 @@ impl MouseContextMenu {
editor: &Editor,
position: MenuPosition,
context_menu: Entity<ui::ContextMenu>,
code_action: Option<MouseCodeAction>,
window: &mut Window,
cx: &mut Context<Editor>,
) -> Self {
@ -141,7 +120,6 @@ impl MouseContextMenu {
Self {
position,
context_menu,
code_action,
_dismiss_subscription,
_cursor_move_subscription,
}
@ -181,13 +159,13 @@ pub fn deploy_context_menu(
let display_map = editor.selections.display_map(cx);
let source_anchor = display_map.display_point_to_anchor(point, text::Bias::Right);
if let Some(custom) = editor.custom_context_menu.take() {
let context_menu = if let Some(custom) = editor.custom_context_menu.take() {
let menu = custom(editor, point, window, cx);
editor.custom_context_menu = Some(custom);
let Some(menu) = menu else {
return;
};
set_context_menu(editor, menu, source_anchor, position, None, window, cx);
menu
} else {
// Don't show the context menu if there isn't a project associated with this editor
let Some(project) = editor.project.clone() else {
@ -226,222 +204,75 @@ pub fn deploy_context_menu(
!filter.is_hidden(&DebuggerEvaluateSelectedText)
});
let menu = build_context_menu(
focus,
has_selections,
has_reveal_target,
has_git_repo,
evaluate_selection,
Some(CodeActionLoadState::Loading),
window,
cx,
);
set_context_menu(editor, menu, source_anchor, position, None, window, cx);
let mut actions_task = editor.code_actions_task.take();
cx.spawn_in(window, async move |editor, cx| {
while let Some(prev_task) = actions_task {
prev_task.await.log_err();
actions_task = editor.update(cx, |this, _| this.code_actions_task.take())?;
}
let action = ToggleCodeActions {
deployed_from_indicator: Some(point.row()),
};
let context_menu_task = editor.update_in(cx, |editor, window, cx| {
let code_actions_task = editor.prepare_code_actions_task(&action, window, cx);
Some(cx.spawn_in(window, async move |editor, cx| {
let code_action_result = code_actions_task.await;
if let Ok(editor_task) = editor.update_in(cx, |editor, window, cx| {
let Some(mouse_context_menu) = editor.mouse_context_menu.take() else {
return Task::ready(Ok::<_, anyhow::Error>(()));
};
if mouse_context_menu
.context_menu
.focus_handle(cx)
.contains_focused(window, cx)
{
window.focus(&editor.focus_handle(cx));
}
drop(mouse_context_menu);
let (state, code_action) =
if let Some((buffer, actions)) = code_action_result {
(
CodeActionLoadState::Loaded(actions.clone()),
Some(MouseCodeAction { actions, buffer }),
)
} else {
(
CodeActionLoadState::Loaded(CodeActionContents::default()),
None,
)
};
let menu = build_context_menu(
window.focused(cx),
has_selections,
has_reveal_target,
has_git_repo,
evaluate_selection,
Some(state),
window,
cx,
);
set_context_menu(
editor,
menu,
source_anchor,
position,
code_action,
window,
cx,
);
Task::ready(Ok(()))
}) {
editor_task.await
ui::ContextMenu::build(window, cx, |menu, _window, _cx| {
let builder = menu
.on_blur_subscription(Subscription::new(|| {}))
.action(
"Show Code Actions",
Box::new(ToggleCodeActions {
deployed_from_indicator: None,
}),
)
.separator()
.when(evaluate_selection && has_selections, |builder| {
builder
.action("Evaluate Selection", Box::new(DebuggerEvaluateSelectedText))
.separator()
})
.action("Go to Definition", Box::new(GoToDefinition))
.action("Go to Declaration", Box::new(GoToDeclaration))
.action("Go to Type Definition", Box::new(GoToTypeDefinition))
.action("Go to Implementation", Box::new(GoToImplementation))
.action("Find All References", Box::new(FindAllReferences))
.separator()
.action("Rename Symbol", Box::new(Rename))
.action("Format Buffer", Box::new(Format))
.when(has_selections, |cx| {
cx.action("Format Selections", Box::new(FormatSelections))
})
.separator()
.action("Cut", Box::new(Cut))
.action("Copy", Box::new(Copy))
.action("Copy and trim", Box::new(CopyAndTrim))
.action("Paste", Box::new(Paste))
.separator()
.map(|builder| {
let reveal_in_finder_label = if cfg!(target_os = "macos") {
"Reveal in Finder"
} else {
Ok(())
}
}))
})?;
if let Some(task) = context_menu_task {
task.await?;
}
Ok::<_, anyhow::Error>(())
})
.detach_and_log_err(cx);
};
}
fn build_context_menu(
focus: Option<FocusHandle>,
has_selections: bool,
has_reveal_target: bool,
has_git_repo: bool,
evaluate_selection: bool,
code_action_load_state: Option<CodeActionLoadState>,
window: &mut Window,
cx: &mut Context<Editor>,
) -> Entity<ContextMenu> {
ui::ContextMenu::build(window, cx, |menu, _window, cx| {
let menu = menu
.on_blur_subscription(Subscription::new(|| {}))
.when(evaluate_selection && has_selections, |builder| {
builder
.action("Evaluate Selection", Box::new(DebuggerEvaluateSelectedText))
.separator()
})
.action("Go to Definition", Box::new(GoToDefinition))
.action("Go to Declaration", Box::new(GoToDeclaration))
.action("Go to Type Definition", Box::new(GoToTypeDefinition))
.action("Go to Implementation", Box::new(GoToImplementation))
.action("Find All References", Box::new(FindAllReferences))
.separator()
.action("Rename Symbol", Box::new(Rename))
.action("Format Buffer", Box::new(Format))
.when(has_selections, |cx| {
cx.action("Format Selections", Box::new(FormatSelections))
})
.separator()
.action("Cut", Box::new(Cut))
.action("Copy", Box::new(Copy))
.action("Copy and trim", Box::new(CopyAndTrim))
.action("Paste", Box::new(Paste))
.separator()
.map(|builder| {
let reveal_in_finder_label = if cfg!(target_os = "macos") {
"Reveal in Finder"
} else {
"Reveal in File Manager"
};
const OPEN_IN_TERMINAL_LABEL: &str = "Open in Terminal";
if has_reveal_target {
builder
.action(reveal_in_finder_label, Box::new(RevealInFileManager))
.action(OPEN_IN_TERMINAL_LABEL, Box::new(OpenInTerminal))
} else {
builder
.disabled_action(reveal_in_finder_label, Box::new(RevealInFileManager))
.disabled_action(OPEN_IN_TERMINAL_LABEL, Box::new(OpenInTerminal))
}
})
.map(|builder| {
const COPY_PERMALINK_LABEL: &str = "Copy Permalink";
if has_git_repo {
builder.action(COPY_PERMALINK_LABEL, Box::new(CopyPermalinkToLine))
} else {
builder.disabled_action(COPY_PERMALINK_LABEL, Box::new(CopyPermalinkToLine))
}
})
.when_some(code_action_load_state, |menu, state| {
menu.separator().map(|menu| match state {
CodeActionLoadState::Loading => menu.disabled_action(
"Loading code actions...",
Box::new(ConfirmCodeAction {
item_ix: None,
from_mouse_context_menu: true,
}),
),
CodeActionLoadState::Loaded(actions) => {
if actions.is_empty() {
menu.disabled_action(
"No code actions available",
Box::new(ConfirmCodeAction {
item_ix: None,
from_mouse_context_menu: true,
}),
)
} else {
actions
.iter()
.filter(|action| {
if action
.as_task()
.map(|task| {
matches!(task.task_type(), task::TaskType::Debug(_))
})
.unwrap_or(false)
{
cx.has_flag::<Debugger>()
} else {
true
}
})
.enumerate()
.fold(menu, |menu, (ix, action)| {
menu.action(
action.label(),
Box::new(ConfirmCodeAction {
item_ix: Some(ix),
from_mouse_context_menu: true,
}),
)
})
}
"Reveal in File Manager"
};
const OPEN_IN_TERMINAL_LABEL: &str = "Open in Terminal";
if has_reveal_target {
builder
.action(reveal_in_finder_label, Box::new(RevealInFileManager))
.action(OPEN_IN_TERMINAL_LABEL, Box::new(OpenInTerminal))
} else {
builder
.disabled_action(reveal_in_finder_label, Box::new(RevealInFileManager))
.disabled_action(OPEN_IN_TERMINAL_LABEL, Box::new(OpenInTerminal))
}
})
});
match focus {
Some(focus) => menu.context(focus),
None => menu,
}
})
}
.map(|builder| {
const COPY_PERMALINK_LABEL: &str = "Copy Permalink";
if has_git_repo {
builder.action(COPY_PERMALINK_LABEL, Box::new(CopyPermalinkToLine))
} else {
builder.disabled_action(COPY_PERMALINK_LABEL, Box::new(CopyPermalinkToLine))
}
});
match focus {
Some(focus) => builder.context(focus),
None => builder,
}
})
};
fn set_context_menu(
editor: &mut Editor,
context_menu: Entity<ui::ContextMenu>,
source_anchor: multi_buffer::Anchor,
position: Option<Point<Pixels>>,
code_action: Option<MouseCodeAction>,
window: &mut Window,
cx: &mut Context<Editor>,
) {
editor.mouse_context_menu = match position {
Some(position) => MouseContextMenu::pinned_to_editor(
editor,
source_anchor,
position,
code_action,
context_menu,
window,
cx,
@ -456,7 +287,6 @@ fn set_context_menu(
editor,
menu_position,
context_menu,
code_action,
window,
cx,
))