From 71551d395b4e47804df2d8ecea99e34dbbf16157 Mon Sep 17 00:00:00 2001 From: Philipp Mildenberger Date: Mon, 23 May 2022 18:10:48 +0200 Subject: Adds support for multiple language servers per language. Language Servers are now configured in a separate table in `languages.toml`: ```toml [langauge-server.mylang-lsp] command = "mylang-lsp" args = ["--stdio"] config = { provideFormatter = true } [language-server.efm-lsp-prettier] command = "efm-langserver" [language-server.efm-lsp-prettier.config] documentFormatting = true languages = { typescript = [ { formatCommand ="prettier --stdin-filepath ${INPUT}", formatStdin = true } ] } ``` The language server for a language is configured like this (`typescript-language-server` is configured by default): ```toml [[language]] name = "typescript" language-servers = [ { name = "efm-lsp-prettier", only-features = [ "format" ] }, "typescript-language-server" ] ``` or equivalent: ```toml [[language]] name = "typescript" language-servers = [ { name = "typescript-language-server", except-features = [ "format" ] }, "efm-lsp-prettier" ] ``` Each requested LSP feature is priorized in the order of the `language-servers` array. For example the first `goto-definition` supported language server (in this case `typescript-language-server`) will be taken for the relevant LSP request (command `goto_definition`). If no `except-features` or `only-features` is given all features for the language server are enabled, as long as the language server supports these. If it doesn't the next language server which supports the feature is tried. The list of supported features are: - `format` - `goto-definition` - `goto-declaration` - `goto-type-definition` - `goto-reference` - `goto-implementation` - `signature-help` - `hover` - `document-highlight` - `completion` - `code-action` - `workspace-command` - `document-symbols` - `workspace-symbols` - `diagnostics` - `rename-symbol` - `inlay-hints` Another side-effect/difference that comes with this PR, is that only one language server instance is started if different languages use the same language server. --- helix-term/src/commands/lsp.rs | 799 +++++++++++++++++++++------------------ helix-term/src/commands/typed.rs | 106 ++++-- 2 files changed, 511 insertions(+), 394 deletions(-) (limited to 'helix-term/src/commands') diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 0ad6fb7e..efef1211 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1,4 +1,4 @@ -use futures_util::FutureExt; +use futures_util::{future::BoxFuture, stream::FuturesUnordered, FutureExt}; use helix_lsp::{ block_on, lsp::{ @@ -8,6 +8,8 @@ use helix_lsp::{ util::{diagnostic_to_lsp_diagnostic, lsp_range_to_range, range_to_lsp_range}, OffsetEncoding, }; +use serde_json::Value; +use tokio_stream::StreamExt; use tui::{ text::{Span, Spans}, widgets::Row, @@ -15,7 +17,9 @@ use tui::{ use super::{align_view, push_jump, Align, Context, Editor, Open}; -use helix_core::{path, text_annotations::InlineAnnotation, Selection}; +use helix_core::{ + path, syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection, +}; use helix_view::{ document::{DocumentInlayHints, DocumentInlayHintsId, Mode}, editor::Action, @@ -25,6 +29,7 @@ use helix_view::{ use crate::{ compositor::{self, Compositor}, + job::Callback, ui::{ self, lsp::SignatureHelp, overlay::overlaid, DynamicPicker, FileLocation, FilePicker, Popup, PromptEvent, @@ -35,24 +40,6 @@ use std::{ cmp::Ordering, collections::BTreeMap, fmt::Write, future::Future, path::PathBuf, sync::Arc, }; -/// Gets the language server that is attached to a document, and -/// if it's not active displays a status message. Using this macro -/// in a context where the editor automatically queries the LSP -/// (instead of when the user explicitly does so via a keybind like -/// `gd`) will spam the "LSP inactive" status message confusingly. -#[macro_export] -macro_rules! language_server { - ($editor:expr, $doc:expr) => { - match $doc.language_server() { - Some(language_server) => language_server, - None => { - $editor.set_status("Language server not active for current buffer"); - return; - } - } - }; -} - impl ui::menu::Item for lsp::Location { /// Current working directory. type Data = PathBuf; @@ -87,20 +74,30 @@ impl ui::menu::Item for lsp::Location { } } -impl ui::menu::Item for lsp::SymbolInformation { +struct SymbolInformationItem { + symbol: lsp::SymbolInformation, + offset_encoding: OffsetEncoding, +} + +impl ui::menu::Item for SymbolInformationItem { /// Path to currently focussed document type Data = Option; fn format(&self, current_doc_path: &Self::Data) -> Row { - if current_doc_path.as_ref() == Some(&self.location.uri) { - self.name.as_str().into() + if current_doc_path.as_ref() == Some(&self.symbol.location.uri) { + self.symbol.name.as_str().into() } else { - match self.location.uri.to_file_path() { + match self.symbol.location.uri.to_file_path() { Ok(path) => { let get_relative_path = path::get_relative_path(path.as_path()); - format!("{} ({})", &self.name, get_relative_path.to_string_lossy()).into() + format!( + "{} ({})", + &self.symbol.name, + get_relative_path.to_string_lossy() + ) + .into() } - Err(_) => format!("{} ({})", &self.name, &self.location.uri).into(), + Err(_) => format!("{} ({})", &self.symbol.name, &self.symbol.location.uri).into(), } } } @@ -116,6 +113,7 @@ struct DiagnosticStyles { struct PickerDiagnostic { url: lsp::Url, diag: lsp::Diagnostic, + offset_encoding: OffsetEncoding, } impl ui::menu::Item for PickerDiagnostic { @@ -211,21 +209,19 @@ fn jump_to_location( align_view(doc, view, Align::Center); } -fn sym_picker( - symbols: Vec, - current_path: Option, - offset_encoding: OffsetEncoding, -) -> FilePicker { +type SymbolPicker = FilePicker; + +fn sym_picker(symbols: Vec, current_path: Option) -> SymbolPicker { // TODO: drop current_path comparison and instead use workspace: bool flag? FilePicker::new( symbols, current_path.clone(), - move |cx, symbol, action| { + move |cx, item, action| { let (view, doc) = current!(cx.editor); push_jump(view, doc); - if current_path.as_ref() != Some(&symbol.location.uri) { - let uri = &symbol.location.uri; + if current_path.as_ref() != Some(&item.symbol.location.uri) { + let uri = &item.symbol.location.uri; let path = match uri.to_file_path() { Ok(path) => path, Err(_) => { @@ -245,7 +241,7 @@ fn sym_picker( let (view, doc) = current!(cx.editor); if let Some(range) = - lsp_range_to_range(doc.text(), symbol.location.range, offset_encoding) + lsp_range_to_range(doc.text(), item.symbol.location.range, item.offset_encoding) { // we flip the range so that the cursor sits on the start of the symbol // (for example start of the function). @@ -253,7 +249,7 @@ fn sym_picker( align_view(doc, view, Align::Center); } }, - move |_editor, symbol| Some(location_to_file_location(&symbol.location)), + move |_editor, item| Some(location_to_file_location(&item.symbol.location)), ) .truncate_start(false) } @@ -266,10 +262,9 @@ enum DiagnosticsFormat { fn diag_picker( cx: &Context, - diagnostics: BTreeMap>, + diagnostics: BTreeMap>, current_path: Option, format: DiagnosticsFormat, - offset_encoding: OffsetEncoding, ) -> FilePicker { // TODO: drop current_path comparison and instead use workspace: bool flag? @@ -277,10 +272,11 @@ fn diag_picker( let mut flat_diag = Vec::new(); for (url, diags) in diagnostics { flat_diag.reserve(diags.len()); - for diag in diags { + for (diag, _, offset_encoding) in diags { flat_diag.push(PickerDiagnostic { url: url.clone(), diag, + offset_encoding, }); } } @@ -295,7 +291,13 @@ fn diag_picker( FilePicker::new( flat_diag, (styles, format), - move |cx, PickerDiagnostic { url, diag }, action| { + move |cx, + PickerDiagnostic { + url, + diag, + offset_encoding, + }, + action| { if current_path.as_ref() == Some(url) { let (view, doc) = current!(cx.editor); push_jump(view, doc); @@ -306,14 +308,14 @@ fn diag_picker( let (view, doc) = current!(cx.editor); - if let Some(range) = lsp_range_to_range(doc.text(), diag.range, offset_encoding) { + if let Some(range) = lsp_range_to_range(doc.text(), diag.range, *offset_encoding) { // we flip the range so that the cursor sits on the start of the symbol // (for example start of the function). doc.set_selection(view.id, Selection::single(range.head, range.anchor)); align_view(doc, view, Align::Center); } }, - move |_editor, PickerDiagnostic { url, diag }| { + move |_editor, PickerDiagnostic { url, diag, .. }| { let location = lsp::Location::new(url.clone(), diag.range); Some(location_to_file_location(&location)) }, @@ -323,126 +325,149 @@ fn diag_picker( pub fn symbol_picker(cx: &mut Context) { fn nested_to_flat( - list: &mut Vec, + list: &mut Vec, file: &lsp::TextDocumentIdentifier, symbol: lsp::DocumentSymbol, + offset_encoding: OffsetEncoding, ) { #[allow(deprecated)] - list.push(lsp::SymbolInformation { - name: symbol.name, - kind: symbol.kind, - tags: symbol.tags, - deprecated: symbol.deprecated, - location: lsp::Location::new(file.uri.clone(), symbol.selection_range), - container_name: None, + list.push(SymbolInformationItem { + symbol: lsp::SymbolInformation { + name: symbol.name, + kind: symbol.kind, + tags: symbol.tags, + deprecated: symbol.deprecated, + location: lsp::Location::new(file.uri.clone(), symbol.selection_range), + container_name: None, + }, + offset_encoding, }); for child in symbol.children.into_iter().flatten() { - nested_to_flat(list, file, child); + nested_to_flat(list, file, child, offset_encoding); } } let doc = doc!(cx.editor); - let language_server = language_server!(cx.editor, doc); + let mut futures: FuturesUnordered<_> = doc + .language_servers_with_feature(LanguageServerFeature::DocumentSymbols) + .iter() + .filter_map(|ls| { + let request = ls.document_symbols(doc.identifier())?; + Some((request, ls.offset_encoding(), doc.identifier())) + }) + .map(|(request, offset_encoding, doc_id)| async move { + let json = request.await?; + let response: Option = serde_json::from_value(json)?; + let symbols = match response { + Some(symbols) => symbols, + None => return anyhow::Ok(vec![]), + }; + // lsp has two ways to represent symbols (flat/nested) + // convert the nested variant to flat, so that we have a homogeneous list + let symbols = match symbols { + lsp::DocumentSymbolResponse::Flat(symbols) => symbols + .into_iter() + .map(|symbol| SymbolInformationItem { + symbol, + offset_encoding, + }) + .collect(), + lsp::DocumentSymbolResponse::Nested(symbols) => { + let mut flat_symbols = Vec::new(); + for symbol in symbols { + nested_to_flat(&mut flat_symbols, &doc_id, symbol, offset_encoding) + } + flat_symbols + } + }; + Ok(symbols) + }) + .collect(); let current_url = doc.url(); - let offset_encoding = language_server.offset_encoding(); - let future = match language_server.document_symbols(doc.identifier()) { - Some(future) => future, - None => { - cx.editor - .set_error("Language server does not support document symbols"); - return; - } - }; - - cx.callback( - future, - move |editor, compositor, response: Option| { - if let Some(symbols) = response { - // lsp has two ways to represent symbols (flat/nested) - // convert the nested variant to flat, so that we have a homogeneous list - let symbols = match symbols { - lsp::DocumentSymbolResponse::Flat(symbols) => symbols, - lsp::DocumentSymbolResponse::Nested(symbols) => { - let doc = doc!(editor); - let mut flat_symbols = Vec::new(); - for symbol in symbols { - nested_to_flat(&mut flat_symbols, &doc.identifier(), symbol) - } - flat_symbols - } - }; + if futures.is_empty() { + cx.editor + .set_error("No Language server does support document symbols"); + return; + } - let picker = sym_picker(symbols, current_url, offset_encoding); - compositor.push(Box::new(overlaid(picker))) + cx.jobs.callback(async move { + let mut symbols = Vec::new(); + // TODO if one symbol request errors, all other requests are discarded (even if they're valid) + while let Some(mut lsp_items) = futures.try_next().await? { + symbols.append(&mut lsp_items); + } + let call = move |editor: &mut Editor, compositor: &mut Compositor| { + if symbols.is_empty() { + editor.set_error("No symbols available"); + return; } - }, - ) + let picker = sym_picker(symbols, current_url); + compositor.push(Box::new(overlaid(picker))) + }; + + Ok(Callback::EditorCompositor(Box::new(call))) + }); } pub fn workspace_symbol_picker(cx: &mut Context) { let doc = doc!(cx.editor); - let current_url = doc.url(); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - let future = match language_server.workspace_symbols("".to_string()) { - Some(future) => future, - None => { - cx.editor - .set_error("Language server does not support workspace symbols"); - return; + + let get_symbols = move |pattern: String, editor: &mut Editor| { + let doc = doc!(editor); + let mut futures: FuturesUnordered<_> = doc + .language_servers_with_feature(LanguageServerFeature::WorkspaceSymbols) + .iter() + .filter_map(|ls| Some((ls.workspace_symbols(pattern.clone())?, ls.offset_encoding()))) + .map(|(request, offset_encoding)| async move { + let json = request.await?; + + let response = serde_json::from_value::>>(json)? + .unwrap_or_default() + .into_iter() + .map(|symbol| SymbolInformationItem { + symbol, + offset_encoding, + }) + .collect(); + + anyhow::Ok(response) + }) + .collect(); + + if futures.is_empty() { + editor.set_error("No Language server does support workspace symbols"); } - }; - cx.callback( - future, - move |_editor, compositor, response: Option>| { - let symbols = response.unwrap_or_default(); - let picker = sym_picker(symbols, current_url, offset_encoding); - let get_symbols = |query: String, editor: &mut Editor| { - let doc = doc!(editor); - let language_server = match doc.language_server() { - Some(s) => s, - None => { - // This should not generally happen since the picker will not - // even open in the first place if there is no server. - return async move { Err(anyhow::anyhow!("LSP not active")) }.boxed(); - } - }; - let symbol_request = match language_server.workspace_symbols(query) { - Some(future) => future, - None => { - // This should also not happen since the language server must have - // supported workspace symbols before to reach this block. - return async move { - Err(anyhow::anyhow!( - "Language server does not support workspace symbols" - )) - } - .boxed(); - } - }; + async move { + let mut symbols = Vec::new(); + // TODO if one symbol request errors, all other requests are discarded (even if they're valid) + while let Some(mut lsp_items) = futures.try_next().await? { + symbols.append(&mut lsp_items); + } + anyhow::Ok(symbols) + } + .boxed() + }; - let future = async move { - let json = symbol_request.await?; - let response: Option> = - serde_json::from_value(json)?; + let current_url = doc.url(); + let initial_symbols = get_symbols("".to_owned(), cx.editor); - Ok(response.unwrap_or_default()) - }; - future.boxed() - }; + cx.jobs.callback(async move { + let symbols = initial_symbols.await?; + let call = move |_editor: &mut Editor, compositor: &mut Compositor| { + let picker = sym_picker(symbols, current_url); let dyn_picker = DynamicPicker::new(picker, Box::new(get_symbols)); compositor.push(Box::new(overlaid(dyn_picker))) - }, - ) + }; + + Ok(Callback::EditorCompositor(Box::new(call))) + }); } pub fn diagnostics_picker(cx: &mut Context) { let doc = doc!(cx.editor); - let language_server = language_server!(cx.editor, doc); if let Some(current_url) = doc.url() { - let offset_encoding = language_server.offset_encoding(); let diagnostics = cx .editor .diagnostics @@ -454,7 +479,6 @@ pub fn diagnostics_picker(cx: &mut Context) { [(current_url.clone(), diagnostics)].into(), Some(current_url), DiagnosticsFormat::HideSourcePath, - offset_encoding, ); cx.push_layer(Box::new(overlaid(picker))); } @@ -462,24 +486,28 @@ pub fn diagnostics_picker(cx: &mut Context) { pub fn workspace_diagnostics_picker(cx: &mut Context) { let doc = doc!(cx.editor); - let language_server = language_server!(cx.editor, doc); let current_url = doc.url(); - let offset_encoding = language_server.offset_encoding(); + // TODO not yet filtered by LanguageServerFeature, need to do something similar as Document::shown_diagnostics here for all open documents let diagnostics = cx.editor.diagnostics.clone(); let picker = diag_picker( cx, diagnostics, current_url, DiagnosticsFormat::ShowSourcePath, - offset_encoding, ); cx.push_layer(Box::new(overlaid(picker))); } -impl ui::menu::Item for lsp::CodeActionOrCommand { +struct CodeActionOrCommandItem { + lsp_item: lsp::CodeActionOrCommand, + offset_encoding: OffsetEncoding, + language_server_id: usize, +} + +impl ui::menu::Item for CodeActionOrCommandItem { type Data = (); fn format(&self, _data: &Self::Data) -> Row { - match self { + match &self.lsp_item { lsp::CodeActionOrCommand::CodeAction(action) => action.title.as_str().into(), lsp::CodeActionOrCommand::Command(command) => command.title.as_str().into(), } @@ -546,45 +574,40 @@ fn action_fixes_diagnostics(action: &CodeActionOrCommand) -> bool { pub fn code_action(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let selection_range = doc.selection(view.id).primary(); - let offset_encoding = language_server.offset_encoding(); - - let range = range_to_lsp_range(doc.text(), selection_range, offset_encoding); - let future = match language_server.code_actions( - doc.identifier(), - range, - // Filter and convert overlapping diagnostics - lsp::CodeActionContext { - diagnostics: doc - .diagnostics() - .iter() - .filter(|&diag| { - selection_range - .overlaps(&helix_core::Range::new(diag.range.start, diag.range.end)) - }) - .map(|diag| diagnostic_to_lsp_diagnostic(doc.text(), diag, offset_encoding)) - .collect(), - only: None, - trigger_kind: Some(CodeActionTriggerKind::INVOKED), - }, - ) { - Some(future) => future, - None => { - cx.editor - .set_error("Language server does not support code actions"); - return; - } - }; - - cx.callback( - future, - move |editor, compositor, response: Option| { + let mut futures: FuturesUnordered<_> = doc + .language_servers_with_feature(LanguageServerFeature::CodeAction) + .iter() + // TODO this should probably already been filtered in something like "language_servers_with_feature" + .filter_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let language_server_id = language_server.id(); + let range = range_to_lsp_range(doc.text(), selection_range, offset_encoding); + // Filter and convert overlapping diagnostics + let code_action_context = lsp::CodeActionContext { + diagnostics: doc + .diagnostics() + .iter() + .filter(|&diag| { + selection_range + .overlaps(&helix_core::Range::new(diag.range.start, diag.range.end)) + }) + .map(|diag| diagnostic_to_lsp_diagnostic(doc.text(), diag, offset_encoding)) + .collect(), + only: None, + trigger_kind: Some(CodeActionTriggerKind::INVOKED), + }; + let code_action_request = + language_server.code_actions(doc.identifier(), range, code_action_context)?; + Some((code_action_request, offset_encoding, language_server_id)) + }) + .map(|(request, offset_encoding, ls_id)| async move { + let json = request.await?; + let response: Option = serde_json::from_value(json)?; let mut actions = match response { Some(a) => a, - None => return, + None => return anyhow::Ok(Vec::new()), }; // remove disabled code actions @@ -596,11 +619,6 @@ pub fn code_action(cx: &mut Context) { ) }); - if actions.is_empty() { - editor.set_status("No code actions available"); - return; - } - // 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: @@ -636,18 +654,48 @@ pub fn code_action(cx: &mut Context) { .reverse() }); - let mut picker = ui::Menu::new(actions, (), move |editor, code_action, event| { + Ok(actions + .into_iter() + .map(|lsp_item| CodeActionOrCommandItem { + lsp_item, + offset_encoding, + language_server_id: ls_id, + }) + .collect()) + }) + .collect(); + + if futures.is_empty() { + cx.editor + .set_error("No Language server does support code actions"); + return; + } + + cx.jobs.callback(async move { + let mut actions = Vec::new(); + // TODO if one code action request errors, all other requests are ignored (even if they're valid) + while let Some(mut lsp_items) = futures.try_next().await? { + actions.append(&mut lsp_items); + } + + let call = move |editor: &mut Editor, compositor: &mut Compositor| { + if actions.is_empty() { + editor.set_error("No code actions available"); + return; + } + let mut picker = ui::Menu::new(actions, (), move |editor, action, event| { if event != PromptEvent::Validate { return; } // always present here - let code_action = code_action.unwrap(); + let action = action.unwrap(); + let offset_encoding = action.offset_encoding; - match code_action { + match &action.lsp_item { lsp::CodeActionOrCommand::Command(command) => { log::debug!("code action command: {:?}", command); - execute_lsp_command(editor, command.clone()); + execute_lsp_command(editor, action.language_server_id, command.clone()); } lsp::CodeActionOrCommand::CodeAction(code_action) => { log::debug!("code action: {:?}", code_action); @@ -659,7 +707,7 @@ pub fn code_action(cx: &mut Context) { // 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 { - execute_lsp_command(editor, command.clone()); + execute_lsp_command(editor, action.language_server_id, command.clone()); } } } @@ -668,8 +716,10 @@ pub fn code_action(cx: &mut Context) { let popup = Popup::new("code-action", picker).with_scrollbar(false); compositor.replace_or_push("code-action", popup); - }, - ) + }; + + Ok(Callback::EditorCompositor(Box::new(call))) + }); } impl ui::menu::Item for lsp::Command { @@ -679,13 +729,14 @@ impl ui::menu::Item for lsp::Command { } } -pub fn execute_lsp_command(editor: &mut Editor, cmd: lsp::Command) { - let doc = doc!(editor); - let language_server = language_server!(editor, doc); - +pub fn execute_lsp_command(editor: &mut Editor, language_server_id: usize, cmd: lsp::Command) { // the command is executed on the server and communicated back // to the client asynchronously using workspace edits - let future = match language_server.command(cmd) { + let future = match editor + .language_servers + .get_by_id(language_server_id) + .and_then(|language_server| language_server.command(cmd)) + { Some(future) => future, None => { editor.set_error("Language server does not support executing commands"); @@ -977,18 +1028,22 @@ fn to_locations(definitions: Option) -> Vec future, + let (future, offset_encoding) = match doc + .language_servers_with_feature(LanguageServerFeature::GotoDeclaration) + .iter() + .find_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = language_server.goto_declaration(doc.identifier(), pos, None)?; + Some((future, offset_encoding)) + }) { + Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor - .set_error("Language server does not support goto-declaration"); + .set_error("No language server supports goto-declaration"); return; } }; @@ -1004,16 +1059,19 @@ pub fn goto_declaration(cx: &mut Context) { pub fn goto_definition(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - - let pos = doc.position(view.id, offset_encoding); - - let future = match language_server.goto_definition(doc.identifier(), pos, None) { - Some(future) => future, + let (future, offset_encoding) = match doc + .language_servers_with_feature(LanguageServerFeature::GotoDefinition) + .iter() + .find_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = language_server.goto_definition(doc.identifier(), pos, None)?; + Some((future, offset_encoding)) + }) { + Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor - .set_error("Language server does not support goto-definition"); + .set_error("No language server supports goto-definition"); return; } }; @@ -1029,16 +1087,19 @@ pub fn goto_definition(cx: &mut Context) { pub fn goto_type_definition(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - - let pos = doc.position(view.id, offset_encoding); - - let future = match language_server.goto_type_definition(doc.identifier(), pos, None) { - Some(future) => future, + let (future, offset_encoding) = match doc + .language_servers_with_feature(LanguageServerFeature::GotoTypeDefinition) + .iter() + .find_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = language_server.goto_type_definition(doc.identifier(), pos, None)?; + Some((future, offset_encoding)) + }) { + Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor - .set_error("Language server does not support goto-type-definition"); + .set_error("No language server supports goto-type-definition"); return; } }; @@ -1054,16 +1115,19 @@ pub fn goto_type_definition(cx: &mut Context) { pub fn goto_implementation(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - - let pos = doc.position(view.id, offset_encoding); - - let future = match language_server.goto_implementation(doc.identifier(), pos, None) { - Some(future) => future, + let (future, offset_encoding) = match doc + .language_servers_with_feature(LanguageServerFeature::GotoImplementation) + .iter() + .find_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = language_server.goto_implementation(doc.identifier(), pos, None)?; + Some((future, offset_encoding)) + }) { + Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor - .set_error("Language server does not support goto-implementation"); + .set_error("no language server supports goto-implementation"); return; } }; @@ -1080,21 +1144,24 @@ pub fn goto_implementation(cx: &mut Context) { pub fn goto_reference(cx: &mut Context) { let config = cx.editor.config(); let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - - let pos = doc.position(view.id, offset_encoding); - - let future = match language_server.goto_reference( - doc.identifier(), - pos, - config.lsp.goto_reference_include_declaration, - None, - ) { - Some(future) => future, + let (future, offset_encoding) = match doc + .language_servers_with_feature(LanguageServerFeature::GotoReference) + .iter() + .find_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = language_server.goto_reference( + doc.identifier(), + pos, + config.lsp.goto_reference_include_declaration, + None, + )?; + Some((future, offset_encoding)) + }) { + Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor - .set_error("Language server does not support goto-reference"); + .set_error("language server supports goto-reference"); return; } }; @@ -1108,7 +1175,7 @@ pub fn goto_reference(cx: &mut Context) { ); } -#[derive(PartialEq, Eq)] +#[derive(PartialEq, Eq, Clone, Copy)] pub enum SignatureHelpInvoked { Manual, Automatic, @@ -1120,35 +1187,34 @@ pub fn signature_help(cx: &mut Context) { pub fn signature_help_impl(cx: &mut Context, invoked: SignatureHelpInvoked) { let (view, doc) = current!(cx.editor); - let was_manually_invoked = invoked == SignatureHelpInvoked::Manual; - let language_server = match doc.language_server() { - Some(language_server) => language_server, + // TODO merge multiple language server signature help into one instead of just taking the first language server that supports it + let future = match doc + .language_servers_with_feature(LanguageServerFeature::SignatureHelp) + .iter() + .find_map(|language_server| { + let pos = doc.position(view.id, language_server.offset_encoding()); + language_server.text_document_signature_help(doc.identifier(), pos, None) + }) { + Some(future) => future.boxed(), None => { // Do not show the message if signature help was invoked // automatically on backspace, trigger characters, etc. - if was_manually_invoked { + if invoked == SignatureHelpInvoked::Manual { cx.editor - .set_status("Language server not active for current buffer"); - } - return; - } - }; - let offset_encoding = language_server.offset_encoding(); - - let pos = doc.position(view.id, offset_encoding); - - let future = match language_server.text_document_signature_help(doc.identifier(), pos, None) { - Some(f) => f, - None => { - if was_manually_invoked { - cx.editor - .set_error("Language server does not support signature-help"); + .set_error("No language server supports signature-help"); } return; } }; + signature_help_impl_with_future(cx, future, invoked); +} +pub fn signature_help_impl_with_future( + cx: &mut Context, + future: BoxFuture<'static, helix_lsp::Result>, + invoked: SignatureHelpInvoked, +) { cx.callback( future, move |editor, compositor, response: Option| { @@ -1156,7 +1222,7 @@ pub fn signature_help_impl(cx: &mut Context, invoked: SignatureHelpInvoked) { if !(config.lsp.auto_signature_help || SignatureHelp::visible_popup(compositor).is_some() - || was_manually_invoked) + || invoked == SignatureHelpInvoked::Manual) { return; } @@ -1165,7 +1231,7 @@ pub fn signature_help_impl(cx: &mut Context, invoked: SignatureHelpInvoked) { // it very probably means the server was a little slow to respond and the user has // already moved on to something else, making a signature help popup will just be an // annoyance, see https://github.com/helix-editor/helix/issues/3112 - if !was_manually_invoked && editor.mode != Mode::Insert { + if invoked == SignatureHelpInvoked::Automatic && editor.mode != Mode::Insert { return; } @@ -1255,18 +1321,20 @@ pub fn signature_help_impl(cx: &mut Context, invoked: SignatureHelpInvoked) { pub fn hover(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); // TODO: factor out a doc.position_identifier() that returns lsp::TextDocumentPositionIdentifier + let request = doc + .language_servers_with_feature(LanguageServerFeature::Hover) + .iter() + .find_map(|language_server| { + let pos = doc.position(view.id, language_server.offset_encoding()); + language_server.text_document_hover(doc.identifier(), pos, None) + }); - let pos = doc.position(view.id, offset_encoding); - - let future = match language_server.text_document_hover(doc.identifier(), pos, None) { + let future = match request { Some(future) => future, None => { - cx.editor - .set_error("Language server does not support hover"); + cx.editor.set_error("No language server supports hover"); return; } }; @@ -1349,7 +1417,11 @@ pub fn rename_symbol(cx: &mut Context) { } } - fn create_rename_prompt(editor: &Editor, prefill: String) -> Box { + fn create_rename_prompt( + editor: &Editor, + prefill: String, + language_server_id: Option, + ) -> Box { let prompt = ui::Prompt::new( "rename-to:".into(), None, @@ -1358,27 +1430,36 @@ pub fn rename_symbol(cx: &mut Context) { if event != PromptEvent::Validate { return; } - let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - - let pos = doc.position(view.id, offset_encoding); - - let future = - match language_server.rename_symbol(doc.identifier(), pos, input.to_string()) { - Some(future) => future, - None => { - cx.editor - .set_error("Language server does not support symbol renaming"); - return; + let request = doc + .language_servers_with_feature(LanguageServerFeature::RenameSymbol) + .iter() + .find_map(|language_server| { + if let Some(language_server_id) = language_server_id { + if language_server.id() != language_server_id { + return None; + } } - }; - match block_on(future) { - Ok(edits) => { - let _ = apply_workspace_edit(cx.editor, offset_encoding, &edits); + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = language_server.rename_symbol( + doc.identifier(), + pos, + input.to_string(), + )?; + Some((future, offset_encoding)) + }); + + if let Some((future, offset_encoding)) = request { + match block_on(future) { + Ok(edits) => { + let _ = apply_workspace_edit(cx.editor, offset_encoding, &edits); + } + Err(err) => cx.editor.set_error(err.to_string()), } - Err(err) => cx.editor.set_error(err.to_string()), + } else { + cx.editor + .set_error("No language server supports symbol renaming"); } }, ) @@ -1388,20 +1469,20 @@ pub fn rename_symbol(cx: &mut Context) { } let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - - if !language_server.supports_rename() { - cx.editor - .set_error("Language server does not support symbol renaming"); - return; - } - let pos = doc.position(view.id, offset_encoding); + let prepare_rename_request = doc + .language_servers_with_feature(LanguageServerFeature::RenameSymbol) + .iter() + .find_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = language_server.prepare_rename(doc.identifier(), pos)?; + Some((future, offset_encoding, language_server.id())) + }); - match language_server.prepare_rename(doc.identifier(), pos) { + match prepare_rename_request { // Language server supports textDocument/prepareRename, use it. - Some(future) => cx.callback( + Some((future, offset_encoding, ls_id)) => cx.callback( future, move |editor, compositor, response: Option| { let prefill = match get_prefill_from_lsp_response(editor, offset_encoding, response) @@ -1413,7 +1494,7 @@ pub fn rename_symbol(cx: &mut Context) { } }; - let prompt = create_rename_prompt(editor, prefill); + let prompt = create_rename_prompt(editor, prefill, Some(ls_id)); compositor.push(prompt); }, @@ -1423,7 +1504,7 @@ pub fn rename_symbol(cx: &mut Context) { None => { let prefill = get_prefill_from_word_boundary(cx.editor); - let prompt = create_rename_prompt(cx.editor, prefill); + let prompt = create_rename_prompt(cx.editor, prefill, None); cx.push_layer(prompt); } @@ -1432,17 +1513,20 @@ pub fn rename_symbol(cx: &mut Context) { pub fn select_references_to_symbol_under_cursor(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let language_server = language_server!(cx.editor, doc); - let offset_encoding = language_server.offset_encoding(); - - let pos = doc.position(view.id, offset_encoding); - - let future = match language_server.text_document_document_highlight(doc.identifier(), pos, None) - { + let (future, offset_encoding) = match doc + .language_servers_with_feature(LanguageServerFeature::DocumentHighlight) + .iter() + .find_map(|language_server| { + let offset_encoding = language_server.offset_encoding(); + let pos = doc.position(view.id, offset_encoding); + let future = + language_server.text_document_document_highlight(doc.identifier(), pos, None)?; + Some((future, offset_encoding)) + }) { Some(future) => future, None => { cx.editor - .set_error("Language server does not support document highlight"); + .set_error("No language server supports document-highlight"); return; } }; @@ -1455,8 +1539,6 @@ pub fn select_references_to_symbol_under_cursor(cx: &mut Context) { _ => return, }; let (view, doc) = current!(editor); - let language_server = language_server!(editor, doc); - let offset_encoding = language_server.offset_encoding(); let text = doc.text(); let pos = doc.selection(view.id).primary().head; @@ -1502,63 +1584,58 @@ fn compute_inlay_hints_for_view( let view_id = view.id; let doc_id = view.doc; - let language_server = doc.language_server()?; - - let capabilities = language_server.capabilities(); - - let (future, new_doc_inlay_hints_id) = match capabilities.inlay_hint_provider { - Some( - lsp::OneOf::Left(true) - | lsp::OneOf::Right(lsp::InlayHintServerCapabilities::Options(_)), - ) => { - let doc_text = doc.text(); - let len_lines = doc_text.len_lines(); - - // Compute ~3 times the current view height of inlay hints, that way some scrolling - // will not show half the view with hints and half without while still being faster - // than computing all the hints for the full file (which could be dozens of time - // longer than the view is). - let view_height = view.inner_height(); - let first_visible_line = - doc_text.char_to_line(view.offset.anchor.min(doc_text.len_chars())); - let first_line = first_visible_line.saturating_sub(view_height); - let last_line = first_visible_line - .saturating_add(view_height.saturating_mul(2)) - .min(len_lines); - - let new_doc_inlay_hint_id = DocumentInlayHintsId { - first_line, - last_line, - }; - // Don't recompute the annotations in case nothing has changed about the view - if !doc.inlay_hints_oudated - && doc - .inlay_hints(view_id) - .map_or(false, |dih| dih.id == new_doc_inlay_hint_id) - { - return None; - } + let language_servers = doc.language_servers_with_feature(LanguageServerFeature::InlayHints); + let language_server = language_servers.iter().find(|language_server| { + matches!( + language_server.capabilities().inlay_hint_provider, + Some( + lsp::OneOf::Left(true) + | lsp::OneOf::Right(lsp::InlayHintServerCapabilities::Options(_)) + ) + ) + })?; + + let doc_text = doc.text(); + let len_lines = doc_text.len_lines(); + + // Compute ~3 times the current view height of inlay hints, that way some scrolling + // will not show half the view with hints and half without while still being faster + // than computing all the hints for the full file (which could be dozens of time + // longer than the view is). + let view_height = view.inner_height(); + let first_visible_line = doc_text.char_to_line(view.offset.anchor.min(doc_text.len_chars())); + let first_line = first_visible_line.saturating_sub(view_height); + let last_line = first_visible_line + .saturating_add(view_height.saturating_mul(2)) + .min(len_lines); + + let new_doc_inlay_hints_id = DocumentInlayHintsId { + first_line, + last_line, + }; + // Don't recompute the annotations in case nothing has changed about the view + if !doc.inlay_hints_oudated + && doc + .inlay_hints(view_id) + .map_or(false, |dih| dih.id == new_doc_inlay_hints_id) + { + return None; + } - let doc_slice = doc_text.slice(..); - let first_char_in_range = doc_slice.line_to_char(first_line); - let last_char_in_range = doc_slice.line_to_char(last_line); + let doc_slice = doc_text.slice(..); + let first_char_in_range = doc_slice.line_to_char(first_line); + let last_char_in_range = doc_slice.line_to_char(last_line); - let range = helix_lsp::util::range_to_lsp_range( - doc_text, - helix_core::Range::new(first_char_in_range, last_char_in_range), - language_server.offset_encoding(), - ); + let range = helix_lsp::util::range_to_lsp_range( + doc_text, + helix_core::Range::new(first_char_in_range, last_char_in_range), + language_server.offset_encoding(), + ); - ( - language_server.text_document_range_inlay_hints(doc.identifier(), range, None), - new_doc_inlay_hint_id, - ) - } - _ => return None, - }; + let offset_encoding = language_server.offset_encoding(); let callback = super::make_job_callback( - future?, + language_server.text_document_range_inlay_hints(doc.identifier(), range, None)?, move |editor, _compositor, response: Option>| { // The config was modified or the window was closed while the request was in flight if !editor.config().lsp.display_inlay_hints || editor.tree.try_get(view_id).is_none() { @@ -1572,8 +1649,8 @@ fn compute_inlay_hints_for_view( }; // If we have neither hints nor an LSP, empty the inlay hints since they're now oudated - let (mut hints, offset_encoding) = match (response, doc.language_server()) { - (Some(h), Some(ls)) if !h.is_empty() => (h, ls.offset_encoding()), + let mut hints = match response { + Some(hints) if !hints.is_empty() => hints, _ => { doc.set_inlay_hints( view_id, diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 81a24059..b78de772 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1329,23 +1329,20 @@ fn lsp_workspace_command( if event != PromptEvent::Validate { return Ok(()); } - - let (_, doc) = current!(cx.editor); - - let language_server = match doc.language_server() { - Some(language_server) => language_server, - None => { - cx.editor - .set_status("Language server not active for current buffer"); - return Ok(()); - } - }; - - let options = match &language_server.capabilities().execute_command_provider { - Some(options) => options, + let doc = doc!(cx.editor); + let language_servers = + doc.language_servers_with_feature(LanguageServerFeature::WorkspaceCommand); + let (language_server_id, options) = match language_servers.iter().find_map(|ls| { + ls.capabilities() + .execute_command_provider + .as_ref() + .map(|options| (ls.id(), options)) + }) { + Some(id_options) => id_options, None => { - cx.editor - .set_status("Workspace commands are not supported for this language server"); + cx.editor.set_status( + "No active language servers for this document support workspace commands", + ); return Ok(()); } }; @@ -1362,8 +1359,8 @@ fn lsp_workspace_command( let callback = async move { let call: job::Callback = Callback::EditorCompositor(Box::new( move |_editor: &mut Editor, compositor: &mut Compositor| { - let picker = ui::Picker::new(commands, (), |cx, command, _action| { - execute_lsp_command(cx.editor, command.clone()); + let picker = ui::Picker::new(commands, (), move |cx, command, _action| { + execute_lsp_command(cx.editor, language_server_id, command.clone()); }); compositor.push(Box::new(overlaid(picker))) }, @@ -1376,6 +1373,7 @@ fn lsp_workspace_command( if options.commands.iter().any(|c| c == &command) { execute_lsp_command( cx.editor, + language_server_id, helix_lsp::lsp::Command { title: command.clone(), arguments: None, @@ -1426,7 +1424,7 @@ fn lsp_restart( .collect(); for document_id in document_ids_to_refresh { - cx.editor.refresh_language_server(document_id); + cx.editor.refresh_language_servers(document_id); } Ok(()) @@ -1443,21 +1441,63 @@ fn lsp_stop( let doc = doc!(cx.editor); - let ls_id = doc - .language_server() - .map(|ls| ls.id()) - .context("LSP not running for the current document")?; + // TODO this stops language servers which may be used in another doc/language type that uses the same language servers + // I'm not sure if this is really what we want + let ls_shutdown_names = doc + .language_servers() + .iter() + .map(|ls| ls.name()) + .collect::>(); - let config = doc - .language_config() - .context("LSP not defined for the current document")?; - cx.editor.language_servers.stop(config); + for ls_name in &ls_shutdown_names { + cx.editor.language_servers.stop(ls_name); + } + + let doc_ids_active_clients: Vec<_> = cx + .editor + .documents() + .filter_map(|doc| { + let doc_active_ls_ids: Vec<_> = doc + .language_servers() + .iter() + .filter(|ls| !ls_shutdown_names.contains(&ls.name())) + .map(|ls| ls.id()) + .collect(); + + let active_clients: Vec<_> = cx + .editor + .language_servers + .iter_clients() + .filter(|client| doc_active_ls_ids.contains(&client.id())) + .map(Clone::clone) + .collect(); + + if active_clients.len() != doc.language_servers().len() { + Some((doc.id(), active_clients)) + } else { + None + } + }) + .collect(); + + for (doc_id, active_clients) in doc_ids_active_clients { + let doc = cx.editor.documents.get_mut(&doc_id).unwrap(); + + let stopped_clients: Vec<_> = doc + .language_servers() + .iter() + .filter(|ls| { + !active_clients + .iter() + .any(|active_ls| active_ls.id() == ls.id()) + }) + .map(|ls| ls.id()) + .collect(); // is necessary because of borrow-checking - for doc in cx.editor.documents_mut() { - if doc.language_server().map_or(false, |ls| ls.id() == ls_id) { - doc.set_language_server(None); - doc.set_diagnostics(Default::default()); + for client_id in stopped_clients { + doc.clear_diagnostics(client_id) } + doc.set_language_servers(active_clients); } Ok(()) @@ -1850,7 +1890,7 @@ fn language( doc.detect_indent_and_line_ending(); let id = doc.id(); - cx.editor.refresh_language_server(id); + cx.editor.refresh_language_servers(id); Ok(()) } @@ -2588,7 +2628,7 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[ TypableCommand { name: "lsp-restart", aliases: &[], - doc: "Restarts the Language Server that is in use by the current doc", + doc: "Restarts the language servers used by the current doc", fun: lsp_restart, signature: CommandSignature::none(), }, -- cgit v1.2.3-70-g09d2 From f9b08656f41cbb9573ffb144f5dc2e24ea764ac9 Mon Sep 17 00:00:00 2001 From: Philipp Mildenberger Date: Fri, 17 Mar 2023 15:30:49 +0100 Subject: Fix sorting issues of the editor wide diagnostics and apply diagnostics related review suggestions Co-authored-by: Pascal Kuthe --- helix-term/src/application.rs | 21 +++++++++++---------- helix-term/src/commands/lsp.rs | 17 ++++++++++------- helix-term/src/ui/statusline.rs | 2 +- helix-view/src/editor.rs | 2 +- 4 files changed, 23 insertions(+), 19 deletions(-) (limited to 'helix-term/src/commands') diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index e159cb83..728aa46a 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -721,7 +721,7 @@ impl Application { )); } } - Notification::PublishDiagnostics(mut params) => { + Notification::PublishDiagnostics(params) => { let path = match params.uri.to_file_path() { Ok(path) => path, Err(_) => { @@ -841,15 +841,10 @@ impl Application { doc.replace_diagnostics(diagnostics, server_id); } - // Sort diagnostics first by severity and then by line numbers. - // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order - params - .diagnostics - .sort_unstable_by_key(|d| (d.severity, d.range.start)); - let diagnostics = params + let mut diagnostics = params .diagnostics .into_iter() - .map(|d| (d, server_id, offset_encoding)) + .map(|d| (d, server_id)) .collect(); // Insert the original lsp::Diagnostics here because we may have no open document @@ -859,10 +854,16 @@ impl Application { Entry::Occupied(o) => { let current_diagnostics = o.into_mut(); // there may entries of other language servers, which is why we can't overwrite the whole entry - current_diagnostics.retain(|(_, lsp_id, _)| *lsp_id != server_id); - current_diagnostics.extend(diagnostics); + current_diagnostics.retain(|(_, lsp_id)| *lsp_id != server_id); + current_diagnostics.append(&mut diagnostics); + // Sort diagnostics first by severity and then by line numbers. + // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order + current_diagnostics + .sort_unstable_by_key(|(d, _)| (d.severity, d.range.start)); } Entry::Vacant(v) => { + diagnostics + .sort_unstable_by_key(|(d, _)| (d.severity, d.range.start)); v.insert(diagnostics); } }; diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index efef1211..1a1233a9 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -262,7 +262,7 @@ enum DiagnosticsFormat { fn diag_picker( cx: &Context, - diagnostics: BTreeMap>, + diagnostics: BTreeMap>, current_path: Option, format: DiagnosticsFormat, ) -> FilePicker { @@ -272,12 +272,15 @@ fn diag_picker( let mut flat_diag = Vec::new(); for (url, diags) in diagnostics { flat_diag.reserve(diags.len()); - for (diag, _, offset_encoding) in diags { - flat_diag.push(PickerDiagnostic { - url: url.clone(), - diag, - offset_encoding, - }); + + for (diag, ls) in diags { + if let Some(ls) = cx.editor.language_servers.get_by_id(ls) { + flat_diag.push(PickerDiagnostic { + url: url.clone(), + diag, + offset_encoding: ls.offset_encoding(), + }); + } } } diff --git a/helix-term/src/ui/statusline.rs b/helix-term/src/ui/statusline.rs index b10e8076..60997956 100644 --- a/helix-term/src/ui/statusline.rs +++ b/helix-term/src/ui/statusline.rs @@ -266,7 +266,7 @@ where .diagnostics .values() .flatten() - .fold((0, 0), |mut counts, (diag, _, _)| { + .fold((0, 0), |mut counts, (diag, _)| { match diag.severity { Some(DiagnosticSeverity::WARNING) => counts.0 += 1, Some(DiagnosticSeverity::ERROR) | None => counts.1 += 1, diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 697d4459..2bd48af8 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -818,7 +818,7 @@ pub struct Editor { pub macro_recording: Option<(char, Vec)>, pub macro_replaying: Vec, pub language_servers: helix_lsp::Registry, - pub diagnostics: BTreeMap>, + pub diagnostics: BTreeMap>, pub diff_providers: DiffProviderRegistry, pub debugger: Option, -- cgit v1.2.3-70-g09d2 From dd2f74794a2ba6c45693d218079349d27828caec Mon Sep 17 00:00:00 2001 From: Philipp Mildenberger Date: Sat, 18 Mar 2023 16:45:07 +0100 Subject: Fix error messages when no language server is available Co-authored-by: Skyler Hawthorne --- helix-term/src/commands/lsp.rs | 4 ++-- helix-view/src/editor.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'helix-term/src/commands') diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 1a1233a9..25a54aba 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1130,7 +1130,7 @@ pub fn goto_implementation(cx: &mut Context) { Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor - .set_error("no language server supports goto-implementation"); + .set_error("No language server supports goto-implementation"); return; } }; @@ -1164,7 +1164,7 @@ pub fn goto_reference(cx: &mut Context) { Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor - .set_error("language server supports goto-reference"); + .set_error("No language server supports goto-reference"); return; } }; diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 2bd48af8..366cc01c 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -48,7 +48,7 @@ use helix_core::{ }; use helix_core::{Position, Selection}; use helix_dap as dap; -use helix_lsp::{lsp, OffsetEncoding}; +use helix_lsp::lsp; use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; -- cgit v1.2.3-70-g09d2 From 76b5cab52479daf25ffa0af798c1ebcf6a4f0004 Mon Sep 17 00:00:00 2001 From: Philipp Mildenberger Date: Sat, 18 Mar 2023 20:12:20 +0100 Subject: Refactored doc.language_servers and doc.language_servers_with_feature to return an iterator and refactor LanguageServerFeature handling to a HashMap (language server name maps to features) Co-authored-by: Pascal Kuthe --- helix-core/src/syntax.rs | 97 ++++++++++++++++++++++++++++++++-------- helix-lsp/src/lib.rs | 20 ++++----- helix-term/src/application.rs | 8 ++-- helix-term/src/commands.rs | 11 ++--- helix-term/src/commands/lsp.rs | 55 +++++++++++------------ helix-term/src/commands/typed.rs | 24 +++++----- helix-term/src/health.rs | 8 ++-- helix-term/src/ui/mod.rs | 8 ++-- helix-term/src/ui/statusline.rs | 5 +-- helix-view/src/document.rs | 52 +++++---------------- helix-view/src/editor.rs | 5 ++- 11 files changed, 155 insertions(+), 138 deletions(-) (limited to 'helix-term/src/commands') diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index f45a38cc..a4e6d990 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -16,7 +16,7 @@ use slotmap::{DefaultKey as LayerId, HopSlotMap}; use std::{ borrow::Cow, cell::RefCell, - collections::{HashMap, VecDeque}, + collections::{HashMap, HashSet, VecDeque}, fmt::{self, Display}, hash::{Hash, Hasher}, mem::{replace, transmute}, @@ -26,7 +26,7 @@ use std::{ }; use once_cell::sync::{Lazy, OnceCell}; -use serde::{Deserialize, Serialize}; +use serde::{ser::SerializeSeq, Deserialize, Serialize}; use helix_loader::grammar::{get_language, load_runtime_file}; @@ -110,8 +110,13 @@ pub struct LanguageConfiguration { #[serde(skip)] pub(crate) highlight_config: OnceCell>>, // tags_config OnceCell<> https://github.com/tree-sitter/tree-sitter/pull/583 - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub language_servers: Vec, + #[serde( + default, + skip_serializing_if = "HashMap::is_empty", + serialize_with = "serialize_lang_features", + deserialize_with = "deserialize_lang_features" + )] + pub language_servers: HashMap, #[serde(skip_serializing_if = "Option::is_none")] pub indent: Option, @@ -211,7 +216,7 @@ impl<'de> Deserialize<'de> for FileType { } } -#[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)] #[serde(rename_all = "kebab-case")] pub enum LanguageServerFeature { Format, @@ -261,18 +266,81 @@ impl Display for LanguageServerFeature { #[derive(Debug, Serialize, Deserialize)] #[serde(untagged, rename_all = "kebab-case", deny_unknown_fields)] -pub enum LanguageServerFeatureConfiguration { +enum LanguageServerFeatureConfiguration { #[serde(rename_all = "kebab-case")] Features { - #[serde(default, skip_serializing_if = "Vec::is_empty")] - only_features: Vec, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - except_features: Vec, + #[serde(default, skip_serializing_if = "HashSet::is_empty")] + only_features: HashSet, + #[serde(default, skip_serializing_if = "HashSet::is_empty")] + except_features: HashSet, name: String, }, Simple(String), } +#[derive(Debug, Default)] +pub struct LanguageServerFeatures { + pub only: HashSet, + pub excluded: HashSet, +} + +impl LanguageServerFeatures { + pub fn has_feature(&self, feature: LanguageServerFeature) -> bool { + self.only.is_empty() || self.only.contains(&feature) && !self.excluded.contains(&feature) + } +} + +fn deserialize_lang_features<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let raw: Vec = Deserialize::deserialize(deserializer)?; + let res = raw + .into_iter() + .map(|config| match config { + LanguageServerFeatureConfiguration::Simple(name) => { + (name, LanguageServerFeatures::default()) + } + LanguageServerFeatureConfiguration::Features { + only_features, + except_features, + name, + } => ( + name, + LanguageServerFeatures { + only: only_features, + excluded: except_features, + }, + ), + }) + .collect(); + Ok(res) +} +fn serialize_lang_features( + map: &HashMap, + serializer: S, +) -> Result +where + S: serde::Serializer, +{ + let mut serializer = serializer.serialize_seq(Some(map.len()))?; + for (name, features) in map { + let features = if features.only.is_empty() && features.excluded.is_empty() { + LanguageServerFeatureConfiguration::Simple(name.to_owned()) + } else { + LanguageServerFeatureConfiguration::Features { + only_features: features.only.clone(), + except_features: features.excluded.clone(), + name: name.to_owned(), + } + }; + serializer.serialize_element(&features)?; + } + serializer.end() +} + #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct LanguageServerConfiguration { @@ -650,15 +718,6 @@ pub struct SoftWrap { pub wrap_at_text_width: Option, } -impl LanguageServerFeatureConfiguration { - pub fn name(&self) -> &String { - match self { - LanguageServerFeatureConfiguration::Simple(name) => name, - LanguageServerFeatureConfiguration::Features { name, .. } => name, - } - } -} - // Expose loader as Lazy<> global since it's always static? #[derive(Debug)] diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 12e63255..ba0c3fee 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -689,12 +689,10 @@ impl Registry { ) -> Result>> { language_config .language_servers - .iter() - .filter_map(|config| { - let name = config.name().clone(); - + .keys() + .filter_map(|name| { #[allow(clippy::map_entry)] - if self.inner.contains_key(&name) { + if self.inner.contains_key(name) { let client = match self.start_client( name.clone(), language_config, @@ -705,7 +703,10 @@ impl Registry { Ok(client) => client, error => return Some(error), }; - let old_clients = self.inner.insert(name, vec![client.clone()]).unwrap(); + let old_clients = self + .inner + .insert(name.clone(), vec![client.clone()]) + .unwrap(); // TODO what if there are different language servers for different workspaces, // I think the language servers will be stopped without being restarted, which is not intended @@ -742,9 +743,8 @@ impl Registry { ) -> Result>> { language_config .language_servers - .iter() - .map(|features| { - let name = features.name(); + .keys() + .map(|name| { if let Some(clients) = self.inner.get_mut(name) { if let Some((_, client)) = clients.iter_mut().enumerate().find(|(i, client)| { client.try_add_doc(&language_config.roots, root_dirs, doc_path, *i == 0) @@ -759,7 +759,7 @@ impl Registry { root_dirs, enable_snippets, )?; - let clients = self.inner.entry(features.name().clone()).or_default(); + let clients = self.inner.entry(name.clone()).or_default(); clients.push(client.clone()); Ok(client) }) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 728aa46a..83473179 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -699,9 +699,10 @@ impl Application { tokio::spawn(language_server.did_change_configuration(config.clone())); } - let docs = self.editor.documents().filter(|doc| { - doc.language_servers().iter().any(|l| l.id() == server_id) - }); + let docs = self + .editor + .documents() + .filter(|doc| doc.language_servers().any(|l| l.id() == server_id)); // trigger textDocument/didOpen for docs that are already open for doc in docs { @@ -970,7 +971,6 @@ impl Application { .filter_map(|doc| { if doc .language_servers() - .iter() .any(|server| server.id() == server_id) { doc.clear_diagnostics(server_id); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 14a68490..060c9d83 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3235,7 +3235,6 @@ pub mod insert { let doc = doc_mut!(cx.editor); let trigger_completion = doc .language_servers_with_feature(LanguageServerFeature::Completion) - .iter() .any(|ls| { let capabilities = ls.capabilities(); @@ -3264,7 +3263,6 @@ pub mod insert { // TODO support multiple language servers (not just the first that is found) let future = doc .language_servers_with_feature(LanguageServerFeature::SignatureHelp) - .iter() .find_map(|ls| { let capabilities = ls.capabilities(); @@ -4067,10 +4065,8 @@ fn format_selections(cx: &mut Context) { .set_error("format_selections only supports a single selection for now"); return; } - - let (future, offset_encoding) = match doc + let future_offset_encoding = doc .language_servers_with_feature(LanguageServerFeature::Format) - .iter() .find_map(|language_server| { let offset_encoding = language_server.offset_encoding(); let ranges: Vec = doc @@ -4091,7 +4087,9 @@ fn format_selections(cx: &mut Context) { None, )?; Some((future, offset_encoding)) - }) { + }); + + let (future, offset_encoding) = match future_offset_encoding { Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor @@ -4247,7 +4245,6 @@ pub fn completion(cx: &mut Context) { let mut futures: FuturesUnordered<_> = doc .language_servers_with_feature(LanguageServerFeature::Completion) - .iter() // TODO this should probably already been filtered in something like "language_servers_with_feature" .filter_map(|language_server| { let language_server_id = language_server.id(); diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 25a54aba..6553ce16 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -353,7 +353,6 @@ pub fn symbol_picker(cx: &mut Context) { let mut futures: FuturesUnordered<_> = doc .language_servers_with_feature(LanguageServerFeature::DocumentSymbols) - .iter() .filter_map(|ls| { let request = ls.document_symbols(doc.identifier())?; Some((request, ls.offset_encoding(), doc.identifier())) @@ -420,7 +419,6 @@ pub fn workspace_symbol_picker(cx: &mut Context) { let doc = doc!(editor); let mut futures: FuturesUnordered<_> = doc .language_servers_with_feature(LanguageServerFeature::WorkspaceSymbols) - .iter() .filter_map(|ls| Some((ls.workspace_symbols(pattern.clone())?, ls.offset_encoding()))) .map(|(request, offset_encoding)| async move { let json = request.await?; @@ -581,7 +579,6 @@ pub fn code_action(cx: &mut Context) { let mut futures: FuturesUnordered<_> = doc .language_servers_with_feature(LanguageServerFeature::CodeAction) - .iter() // TODO this should probably already been filtered in something like "language_servers_with_feature" .filter_map(|language_server| { let offset_encoding = language_server.offset_encoding(); @@ -1034,15 +1031,15 @@ fn to_locations(definitions: Option) -> Vec future_offset_encoding, None => { cx.editor @@ -1062,15 +1059,15 @@ pub fn goto_declaration(cx: &mut Context) { pub fn goto_definition(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let (future, offset_encoding) = match doc + let future_offset_encoding = doc .language_servers_with_feature(LanguageServerFeature::GotoDefinition) - .iter() .find_map(|language_server| { let offset_encoding = language_server.offset_encoding(); let pos = doc.position(view.id, offset_encoding); let future = language_server.goto_definition(doc.identifier(), pos, None)?; Some((future, offset_encoding)) - }) { + }); + let (future, offset_encoding) = match future_offset_encoding { Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor @@ -1090,15 +1087,15 @@ pub fn goto_definition(cx: &mut Context) { pub fn goto_type_definition(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let (future, offset_encoding) = match doc + let future_offset_encoding = doc .language_servers_with_feature(LanguageServerFeature::GotoTypeDefinition) - .iter() .find_map(|language_server| { let offset_encoding = language_server.offset_encoding(); let pos = doc.position(view.id, offset_encoding); let future = language_server.goto_type_definition(doc.identifier(), pos, None)?; Some((future, offset_encoding)) - }) { + }); + let (future, offset_encoding) = match future_offset_encoding { Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor @@ -1118,15 +1115,15 @@ pub fn goto_type_definition(cx: &mut Context) { pub fn goto_implementation(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let (future, offset_encoding) = match doc + let future_offset_encoding = doc .language_servers_with_feature(LanguageServerFeature::GotoImplementation) - .iter() .find_map(|language_server| { let offset_encoding = language_server.offset_encoding(); let pos = doc.position(view.id, offset_encoding); let future = language_server.goto_implementation(doc.identifier(), pos, None)?; Some((future, offset_encoding)) - }) { + }); + let (future, offset_encoding) = match future_offset_encoding { Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor @@ -1147,9 +1144,8 @@ pub fn goto_implementation(cx: &mut Context) { pub fn goto_reference(cx: &mut Context) { let config = cx.editor.config(); let (view, doc) = current!(cx.editor); - let (future, offset_encoding) = match doc + let future_offset_encoding = doc .language_servers_with_feature(LanguageServerFeature::GotoReference) - .iter() .find_map(|language_server| { let offset_encoding = language_server.offset_encoding(); let pos = doc.position(view.id, offset_encoding); @@ -1160,7 +1156,8 @@ pub fn goto_reference(cx: &mut Context) { None, )?; Some((future, offset_encoding)) - }) { + }); + let (future, offset_encoding) = match future_offset_encoding { Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor @@ -1192,13 +1189,14 @@ pub fn signature_help_impl(cx: &mut Context, invoked: SignatureHelpInvoked) { let (view, doc) = current!(cx.editor); // TODO merge multiple language server signature help into one instead of just taking the first language server that supports it - let future = match doc + let future = doc .language_servers_with_feature(LanguageServerFeature::SignatureHelp) - .iter() .find_map(|language_server| { let pos = doc.position(view.id, language_server.offset_encoding()); language_server.text_document_signature_help(doc.identifier(), pos, None) - }) { + }); + + let future = match future { Some(future) => future.boxed(), None => { // Do not show the message if signature help was invoked @@ -1328,7 +1326,6 @@ pub fn hover(cx: &mut Context) { // TODO: factor out a doc.position_identifier() that returns lsp::TextDocumentPositionIdentifier let request = doc .language_servers_with_feature(LanguageServerFeature::Hover) - .iter() .find_map(|language_server| { let pos = doc.position(view.id, language_server.offset_encoding()); language_server.text_document_hover(doc.identifier(), pos, None) @@ -1436,7 +1433,6 @@ pub fn rename_symbol(cx: &mut Context) { let (view, doc) = current!(cx.editor); let request = doc .language_servers_with_feature(LanguageServerFeature::RenameSymbol) - .iter() .find_map(|language_server| { if let Some(language_server_id) = language_server_id { if language_server.id() != language_server_id { @@ -1475,7 +1471,6 @@ pub fn rename_symbol(cx: &mut Context) { let prepare_rename_request = doc .language_servers_with_feature(LanguageServerFeature::RenameSymbol) - .iter() .find_map(|language_server| { let offset_encoding = language_server.offset_encoding(); let pos = doc.position(view.id, offset_encoding); @@ -1516,17 +1511,17 @@ pub fn rename_symbol(cx: &mut Context) { pub fn select_references_to_symbol_under_cursor(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let (future, offset_encoding) = match doc + let future_offset_encoding = doc .language_servers_with_feature(LanguageServerFeature::DocumentHighlight) - .iter() .find_map(|language_server| { let offset_encoding = language_server.offset_encoding(); let pos = doc.position(view.id, offset_encoding); let future = language_server.text_document_document_highlight(doc.identifier(), pos, None)?; Some((future, offset_encoding)) - }) { - Some(future) => future, + }); + let (future, offset_encoding) = match future_offset_encoding { + Some(future_offset_encoding) => future_offset_encoding, None => { cx.editor .set_error("No language server supports document-highlight"); @@ -1587,8 +1582,8 @@ fn compute_inlay_hints_for_view( let view_id = view.id; let doc_id = view.doc; - let language_servers = doc.language_servers_with_feature(LanguageServerFeature::InlayHints); - let language_server = language_servers.iter().find(|language_server| { + let mut language_servers = doc.language_servers_with_feature(LanguageServerFeature::InlayHints); + let language_server = language_servers.find(|language_server| { matches!( language_server.capabilities().inlay_hint_provider, Some( diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index b78de772..38058ed5 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1330,14 +1330,16 @@ fn lsp_workspace_command( return Ok(()); } let doc = doc!(cx.editor); - let language_servers = - doc.language_servers_with_feature(LanguageServerFeature::WorkspaceCommand); - let (language_server_id, options) = match language_servers.iter().find_map(|ls| { - ls.capabilities() - .execute_command_provider - .as_ref() - .map(|options| (ls.id(), options)) - }) { + let id_options = doc + .language_servers_with_feature(LanguageServerFeature::WorkspaceCommand) + .find_map(|ls| { + ls.capabilities() + .execute_command_provider + .as_ref() + .map(|options| (ls.id(), options)) + }); + + let (language_server_id, options) = match id_options { Some(id_options) => id_options, None => { cx.editor.set_status( @@ -1346,6 +1348,7 @@ fn lsp_workspace_command( return Ok(()); } }; + if args.is_empty() { let commands = options .commands @@ -1445,7 +1448,6 @@ fn lsp_stop( // I'm not sure if this is really what we want let ls_shutdown_names = doc .language_servers() - .iter() .map(|ls| ls.name()) .collect::>(); @@ -1459,7 +1461,6 @@ fn lsp_stop( .filter_map(|doc| { let doc_active_ls_ids: Vec<_> = doc .language_servers() - .iter() .filter(|ls| !ls_shutdown_names.contains(&ls.name())) .map(|ls| ls.id()) .collect(); @@ -1472,7 +1473,7 @@ fn lsp_stop( .map(Clone::clone) .collect(); - if active_clients.len() != doc.language_servers().len() { + if active_clients.len() != doc.language_servers().count() { Some((doc.id(), active_clients)) } else { None @@ -1485,7 +1486,6 @@ fn lsp_stop( let stopped_clients: Vec<_> = doc .language_servers() - .iter() .filter(|ls| { !active_clients .iter() diff --git a/helix-term/src/health.rs b/helix-term/src/health.rs index 6b9f8517..5b22ea55 100644 --- a/helix-term/src/health.rs +++ b/helix-term/src/health.rs @@ -194,10 +194,10 @@ pub fn languages_all() -> std::io::Result<()> { // TODO multiple language servers (check binary for each supported language server, not just the first) - let lsp = lang.language_servers.first().and_then(|lsp| { + let lsp = lang.language_servers.keys().next().and_then(|ls_name| { syn_loader_conf .language_server - .get(lsp.name()) + .get(ls_name) .map(|config| config.command.clone()) }); check_binary(lsp); @@ -271,10 +271,10 @@ pub fn language(lang_str: String) -> std::io::Result<()> { // TODO multiple language servers probe_protocol( "language server", - lang.language_servers.first().and_then(|lsp| { + lang.language_servers.keys().next().and_then(|ls_name| { syn_loader_conf .language_server - .get(lsp.name()) + .get(ls_name) .map(|config| config.command.clone()) }), )?; diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 118836c0..6f7ed174 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -394,13 +394,11 @@ pub mod completers { pub fn lsp_workspace_command(editor: &Editor, input: &str) -> Vec { let matcher = Matcher::default(); - let language_servers = - doc!(editor).language_servers_with_feature(LanguageServerFeature::WorkspaceCommand); - let options = match language_servers - .into_iter() + let options = match doc!(editor) + .language_servers_with_feature(LanguageServerFeature::WorkspaceCommand) .find_map(|ls| ls.capabilities().execute_command_provider.as_ref()) { - Some(id_options) => id_options, + Some(options) => options, None => { return vec![]; } diff --git a/helix-term/src/ui/statusline.rs b/helix-term/src/ui/statusline.rs index 60997956..4aa64634 100644 --- a/helix-term/src/ui/statusline.rs +++ b/helix-term/src/ui/statusline.rs @@ -202,11 +202,10 @@ fn render_lsp_spinner(context: &mut RenderContext, write: F) where F: Fn(&mut RenderContext, String, Option