From bc27d0113bf582c186ccbd4231c8b57c72fbddd9 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sun, 21 Jun 2026 12:43:26 -0400 Subject: [PATCH] view: Extract code actions into a generic `Fn(&mut Editor)` Code actions can be a thin wrapper around a Fn(&mut Editor) and still support the LSP implementation. This extracts the code actions so that they can be reused by arbitrary core actions. This is an important step towards the spell checking integration: a spelling handler can expose code actions to add a misspelled word to a personal dictionary, or to suggest alternatives. These code actions also only need a closure over an exclusive borrow of the Editor to accomplish either. --- helix-term/src/commands/lsp.rs | 152 +++----------------------------- helix-view/src/action.rs | 154 +++++++++++++++++++++++++++++++++ helix-view/src/lib.rs | 1 + 3 files changed, 166 insertions(+), 141 deletions(-) create mode 100644 helix-view/src/action.rs diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index abade06f..bfb9bb94 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -19,6 +19,7 @@ use helix_core::{ }; use helix_stdx::path; use helix_view::{ + action::Action as CodeActionItem, document::{DocumentInlayHints, DocumentInlayHintsId}, editor::Action, handlers::lsp::SignatureHelpInvoked, @@ -33,7 +34,6 @@ use crate::{ }; use std::{ - cmp::Ordering, collections::{HashSet, VecDeque}, fmt::Display, future::Future, @@ -588,78 +588,13 @@ pub fn workspace_diagnostics_picker(cx: &mut Context) { cx.push_layer(Box::new(overlaid(picker))); } -struct CodeActionOrCommandItem { - lsp_item: lsp::CodeActionOrCommand, - language_server_id: LanguageServerId, -} - -impl ui::menu::Item for CodeActionOrCommandItem { +impl ui::menu::Item for CodeActionItem { type Data = (); fn format(&self, _data: &Self::Data) -> Row<'_> { - match &self.lsp_item { - lsp::CodeActionOrCommand::CodeAction(action) => action.title.as_str().into(), - lsp::CodeActionOrCommand::Command(command) => command.title.as_str().into(), - } - } -} - -/// Determines the category of the `CodeAction` using the `CodeAction::kind` field. -/// Returns a number that represent these categories. -/// Categories with a lower number should be displayed first. -/// -/// -/// While the `kind` field is defined as open ended in the LSP spec (any value may be used) -/// in practice a closed set of common values (mostly suggested in the LSP spec) are used. -/// VSCode displays each of these categories separately (separated by a heading in the codeactions picker) -/// to make them easier to navigate. Helix does not display these headings to the user. -/// However it does sort code actions by their categories to achieve the same order as the VScode picker, -/// just without the headings. -/// -/// The order used here is modeled after the [vscode sourcecode](https://github.com/microsoft/vscode/blob/eaec601dd69aeb4abb63b9601a6f44308c8d8c6e/src/vs/editor/contrib/codeAction/browser/codeActionWidget.ts>) -fn action_category(action: &CodeActionOrCommand) -> u32 { - if let CodeActionOrCommand::CodeAction(CodeAction { - kind: Some(kind), .. - }) = action - { - let mut components = kind.as_str().split('.'); - match components.next() { - Some("quickfix") => 0, - Some("refactor") => match components.next() { - Some("extract") => 1, - Some("inline") => 2, - Some("rewrite") => 3, - Some("move") => 4, - Some("surround") => 5, - _ => 7, - }, - Some("source") => 6, - _ => 7, - } - } else { - 7 + self.title().into() } } -fn action_preferred(action: &CodeActionOrCommand) -> bool { - matches!( - action, - CodeActionOrCommand::CodeAction(CodeAction { - is_preferred: Some(true), - .. - }) - ) -} - -fn action_fixes_diagnostics(action: &CodeActionOrCommand) -> bool { - matches!( - action, - CodeActionOrCommand::CodeAction(CodeAction { - diagnostics: Some(diagnostics), - .. - }) if !diagnostics.is_empty() - ) -} - pub fn code_action(cx: &mut Context) { let (view, doc) = current!(cx.editor); @@ -681,47 +616,9 @@ pub fn code_action(cx: &mut Context) { ) }); - // Sort codeactions into a useful order. This behaviour is only partially described in the LSP spec. - // Many details are modeled after vscode because language servers are usually tested against it. - // VScode sorts the codeaction two times: - // - // First the codeactions that fix some diagnostics are moved to the front. - // If both codeactions fix some diagnostics (or both fix none) the codeaction - // that is marked with `is_preferred` is shown first. The codeactions are then shown in separate - // submenus that only contain a certain category (see `action_category`) of actions. - // - // Below this done in in a single sorting step - actions.sort_by(|action1, action2| { - // sort actions by category - let order = action_category(action1).cmp(&action_category(action2)); - if order != Ordering::Equal { - return order; - } - // within the categories sort by relevancy. - // Modeled after the `codeActionsComparator` function in vscode: - // https://github.com/microsoft/vscode/blob/eaec601dd69aeb4abb63b9601a6f44308c8d8c6e/src/vs/editor/contrib/codeAction/browser/codeAction.ts - - // if one code action fixes a diagnostic but the other one doesn't show it first - let order = action_fixes_diagnostics(action1) - .cmp(&action_fixes_diagnostics(action2)) - .reverse(); - if order != Ordering::Equal { - return order; - } - - // if one of the codeactions is marked as preferred show it first - // otherwise keep the original LSP sorting - action_preferred(action1) - .cmp(&action_preferred(action2)) - .reverse() - }); - Ok(actions .into_iter() - .map(|lsp_item| CodeActionOrCommandItem { - lsp_item, - language_server_id: ls_id, - }) + .map(|lsp_item| CodeActionItem::lsp(ls_id, lsp_item)) .collect()) }) .collect(); @@ -737,11 +634,15 @@ pub fn code_action(cx: &mut Context) { while let Some(output) = futures.next().await { match output { - Ok(mut lsp_items) => actions.append(&mut lsp_items), + Ok(mut items) => actions.append(&mut items), Err(err) => log::error!("while gathering code actions: {err}"), } } + // Sort the gathered actions into a useful order, highest priority first. See + // `lsp_code_action_priority` for how LSP actions are ranked. + actions.sort_by_key(|action| std::cmp::Reverse(action.priority)); + let call = move |editor: &mut Editor, compositor: &mut Compositor| { if actions.is_empty() { editor.set_error("No code actions available"); @@ -751,39 +652,8 @@ pub fn code_action(cx: &mut Context) { if event != PromptEvent::Validate { return; } - - // always present here - let action = action.unwrap(); - let Some(language_server) = editor.language_server_by_id(action.language_server_id) - else { - editor.set_error("Language Server disappeared"); - return; - }; - let offset_encoding = language_server.offset_encoding(); - - match &action.lsp_item { - lsp::CodeActionOrCommand::Command(command) => { - log::debug!("code action command: {:?}", command); - editor.execute_lsp_command(command.clone(), action.language_server_id); - } - lsp::CodeActionOrCommand::CodeAction(code_action) => { - log::debug!("code action: {:?}", code_action); - let resolved_code_action = - resolve_code_action_blocking(code_action, language_server); - - if let Some(ref workspace_edit) = - resolved_code_action.as_ref().unwrap_or(code_action).edit - { - let _ = editor.apply_workspace_edit(offset_encoding, workspace_edit); - } - - // if code action provides both edit and command first the edit - // should be applied and then the command - if let Some(command) = &code_action.command { - editor.execute_lsp_command(command.clone(), action.language_server_id); - } - } - } + // Always present on validate. + action.unwrap().execute(editor); }); picker.move_down(); // pre-select the first item diff --git a/helix-view/src/action.rs b/helix-view/src/action.rs new file mode 100644 index 00000000..474ec8f8 --- /dev/null +++ b/helix-view/src/action.rs @@ -0,0 +1,154 @@ +use std::{borrow::Cow, fmt}; + +use helix_lsp::{lsp, LanguageServerId}; + +use crate::Editor; + +/// A titled action against the editor, shown in the code action menu. +/// +/// LSP code actions are converted into `Action`s, but the type is provider-agnostic: internal +/// features (for example the spell checker) produce `Action`s the same way, so they appear in the +/// same menu without the menu knowing where they came from. +pub struct Action { + title: Cow<'static, str>, + /// Sort key; higher priority actions are shown first. See [`lsp_code_action_priority`]. + pub priority: u8, + action: Box, +} + +impl fmt::Debug for Action { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Action") + .field("title", &self.title) + .field("priority", &self.priority) + .finish_non_exhaustive() + } +} + +impl Action { + pub fn new>, F: Fn(&mut Editor) + Send + Sync + 'static>( + title: T, + priority: u8, + action: F, + ) -> Self { + Self { + title: title.into(), + priority, + action: Box::new(action), + } + } + + pub fn title(&self) -> &str { + &self.title + } + + pub fn execute(&self, editor: &mut Editor) { + (self.action)(editor); + } + + /// Builds an `Action` from an LSP code action or command. + pub fn lsp(server_id: LanguageServerId, action: lsp::CodeActionOrCommand) -> Self { + let title = match &action { + lsp::CodeActionOrCommand::CodeAction(action) => action.title.clone(), + lsp::CodeActionOrCommand::Command(command) => command.title.clone(), + }; + let priority = lsp_code_action_priority(&action); + + Self::new(title, priority, move |editor| { + let Some(language_server) = editor.language_server_by_id(server_id) else { + editor.set_error("Language Server disappeared"); + return; + }; + let offset_encoding = language_server.offset_encoding(); + match &action { + lsp::CodeActionOrCommand::Command(command) => { + log::debug!("code action command: {:?}", command); + editor.execute_lsp_command(command.clone(), server_id); + } + lsp::CodeActionOrCommand::CodeAction(code_action) => { + log::debug!("code action: {:?}", code_action); + // We support lsp "codeAction/resolve" for `edit` and `command` fields. + let resolved; + let code_action = if code_action.edit.is_none() || code_action.command.is_none() + { + resolved = language_server + .resolve_code_action(code_action) + .and_then(|future| helix_lsp::block_on(future).ok()); + resolved.as_ref().unwrap_or(code_action) + } else { + code_action + }; + + if let Some(ref workspace_edit) = code_action.edit { + let _ = editor.apply_workspace_edit(offset_encoding, workspace_edit); + } + + // If a code action provides both an edit and a command the edit is applied + // first, then the command. + if let Some(command) = &code_action.command { + editor.execute_lsp_command(command.clone(), server_id); + } + } + } + }) + } +} + +/// Computes the sort priority for an LSP code action; higher is shown first. +/// +/// This roughly matches VSCode's ordering: code actions are sorted first by the category declared +/// on the `kind` field, then by whether they fix a diagnostic, then by whether they are marked +/// preferred. See . +fn lsp_code_action_priority(action: &lsp::CodeActionOrCommand) -> u8 { + // The `kind` field is open ended in the LSP spec, but in practice a closed set of common values + // (mostly suggested by the spec) is used. VSCode shows these as menu headers; we don't, but we + // sort by them the same way. + let category = if let lsp::CodeActionOrCommand::CodeAction(lsp::CodeAction { + kind: Some(kind), + .. + }) = action + { + let mut components = kind.as_str().split('.'); + match components.next() { + Some("quickfix") => 7, + Some("refactor") => match components.next() { + Some("extract") => 6, + Some("inline") => 5, + Some("rewrite") => 4, + Some("move") => 3, + Some("surround") => 2, + _ => 1, + }, + Some("source") => 1, + _ => 0, + } + } else { + 0 + }; + let fixes_diagnostic = matches!( + action, + lsp::CodeActionOrCommand::CodeAction(lsp::CodeAction { + diagnostics: Some(diagnostics), + .. + }) if !diagnostics.is_empty() + ); + let is_preferred = matches!( + action, + lsp::CodeActionOrCommand::CodeAction(lsp::CodeAction { + is_preferred: Some(true), + .. + }) + ); + + // Weight the criteria so their scores can't overlap: two actions in the same category sort + // closer than actions in different categories, with `fixes_diagnostic` and `is_preferred` + // breaking ties. + let mut priority = category * 4; + if fixes_diagnostic { + priority += 2; + } + if is_preferred { + priority += 1; + } + priority +} diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs index 48d12618..ab3ec32f 100644 --- a/helix-view/src/lib.rs +++ b/helix-view/src/lib.rs @@ -1,6 +1,7 @@ #[macro_use] pub mod macros; +pub mod action; pub mod annotations; pub mod clipboard; pub mod document; -- 2.51.2