From fb131307015c09da2a9ffa658d087dbc7afa6bcd Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Thu, 11 Jan 2024 17:55:50 +0100 Subject: [PATCH] migrate language server config to new config system --- Cargo.lock | 13 ++++--- helix-lsp/Cargo.toml | 3 ++ helix-lsp/src/client.rs | 47 ++++++++++-------------- helix-lsp/src/config.rs | 67 +++++++++++++++++++++++++++++++++++ helix-lsp/src/lib.rs | 22 ++++++++---- helix-term/src/application.rs | 7 ++-- 6 files changed, 117 insertions(+), 42 deletions(-) create mode 100644 helix-lsp/src/config.rs diff --git a/Cargo.lock b/Cargo.lock index 22ea56404..7a1fcdd02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -62,9 +62,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.78" +version = "1.0.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca87830a3e3fb156dc96cfbd31cb620265dd053be734723f22b760d6cc3c3051" +checksum = "080e9890a082662b09c1ad45f567faeeb47f22b5fb23895fbe1e651e718e25ca" [[package]] name = "arc-swap" @@ -1069,6 +1069,7 @@ name = "helix-core" version = "23.10.0" dependencies = [ "ahash", + "anyhow", "arc-swap", "bitflags 2.4.1", "chrono", @@ -1079,6 +1080,7 @@ dependencies = [ "helix-config", "helix-loader", "imara-diff", + "indexmap", "indoc", "log", "nucleo", @@ -1147,6 +1149,7 @@ dependencies = [ name = "helix-lsp" version = "23.10.0" dependencies = [ + "ahash", "anyhow", "futures-executor", "futures-util", @@ -1155,6 +1158,7 @@ dependencies = [ "helix-core", "helix-loader", "helix-parsec", + "indexmap", "log", "lsp-types", "parking_lot", @@ -1358,12 +1362,13 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.0.0" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d5477fe2230a79769d8dc68e0eabf5437907c0457a5614a9e8dddb67f65eb65d" +checksum = "d530e1a18b1cb4c484e6e34556a0d948706958449fca0cab753d649f2bce3d1f" dependencies = [ "equivalent", "hashbrown 0.14.3", + "serde", ] [[package]] diff --git a/helix-lsp/Cargo.toml b/helix-lsp/Cargo.toml index 851351e0e..e40659109 100644 --- a/helix-lsp/Cargo.toml +++ b/helix-lsp/Cargo.toml @@ -14,6 +14,7 @@ homepage.workspace = true [dependencies] helix-core = { path = "../helix-core" } +helix-config = { path = "../helix-config" } helix-loader = { path = "../helix-loader" } helix-parsec = { path = "../helix-parsec" } @@ -30,3 +31,5 @@ tokio = { version = "1.35", features = ["rt", "rt-multi-thread", "io-util", "io- tokio-stream = "0.1.14" which = "5.0.0" parking_lot = "0.12.1" +ahash = "0.8.6" +indexmap = { version = "2.1.0", features = ["serde"] } diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 682d4db66..f3f7e279b 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -1,9 +1,12 @@ use crate::{ + config::LanguageServerConfig, find_lsp_workspace, jsonrpc, transport::{Payload, Transport}, Call, Error, OffsetEncoding, Result, }; +use anyhow::Context; +use helix_config::{self as config, OptionManager}; use helix_core::{find_workspace, path, syntax::LanguageServerFeature, ChangeSet, Rope}; use helix_loader::{self, VERSION_AND_GIT_HASH}; use lsp::{ @@ -13,15 +16,14 @@ use lsp::{ }; use lsp_types as lsp; use parking_lot::Mutex; -use serde::Deserialize; use serde_json::Value; use std::future::Future; +use std::path::PathBuf; use std::process::Stdio; use std::sync::{ atomic::{AtomicU64, Ordering}, Arc, }; -use std::{collections::HashMap, path::PathBuf}; use tokio::{ io::{BufReader, BufWriter}, process::{Child, Command}, @@ -50,13 +52,11 @@ pub struct Client { server_tx: UnboundedSender, request_counter: AtomicU64, pub(crate) capabilities: OnceCell, - config: Option, root_path: std::path::PathBuf, root_uri: Option, workspace_folders: Mutex>, initialize_notify: Arc, - /// workspace folders added while the server is still initializing - req_timeout: u64, + config: Arc, } impl Client { @@ -170,23 +170,20 @@ impl Client { #[allow(clippy::type_complexity, clippy::too_many_arguments)] pub fn start( - cmd: &str, - args: &[String], - config: Option, - server_environment: HashMap, + config: Arc, root_markers: &[String], manual_roots: &[PathBuf], id: usize, name: String, - req_timeout: u64, doc_path: Option<&std::path::PathBuf>, ) -> Result<(Self, UnboundedReceiver<(usize, Call)>, Arc)> { // Resolve path to the binary - let cmd = which::which(cmd).map_err(|err| anyhow::anyhow!(err))?; + let cmd = which::which(config.command().as_deref().context("no command defined")?) + .map_err(|err| anyhow::anyhow!(err))?; let process = Command::new(cmd) - .envs(server_environment) - .args(args) + .envs(config.enviorment().iter().map(|(k, v)| (&**k, &**v))) + .args(config.args().iter().map(|v| &**v)) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -233,7 +230,6 @@ impl Client { request_counter: AtomicU64::new(0), capabilities: OnceCell::new(), config, - req_timeout, root_path, root_uri, workspace_folders: Mutex::new(workspace_folders), @@ -374,8 +370,8 @@ impl Client { .unwrap_or_default() } - pub fn config(&self) -> Option<&Value> { - self.config.as_ref() + pub fn config(&self) -> config::Guard>> { + self.config.server_config() } pub async fn workspace_folders( @@ -404,7 +400,7 @@ impl Client { where R::Params: serde::Serialize, { - self.call_with_timeout::(params, self.req_timeout) + self.call_with_timeout::(params, self.config.timeout()) } fn call_with_timeout( @@ -512,7 +508,7 @@ impl Client { // ------------------------------------------------------------------------------------------- pub(crate) async fn initialize(&self, enable_snippets: bool) -> Result { - if let Some(config) = &self.config { + if let Some(config) = &*self.config() { log::info!("Using custom LSP config: {}", config); } @@ -524,7 +520,7 @@ impl Client { // clients will prefer _uri if possible root_path: self.root_path.to_str().map(|path| path.to_owned()), root_uri: self.root_uri.clone(), - initialization_options: self.config.clone(), + initialization_options: self.config().as_deref().cloned(), capabilities: lsp::ClientCapabilities { workspace: Some(lsp::WorkspaceClientCapabilities { configuration: Some(true), @@ -1152,17 +1148,12 @@ impl Client { }; // merge FormattingOptions with 'config.format' - let config_format = self - .config - .as_ref() - .and_then(|cfg| cfg.get("format")) - .and_then(|fmt| HashMap::::deserialize(fmt).ok()); - - let options = if let Some(mut properties) = config_format { + let mut config_format = self.config.format(); + let options = if !config_format.is_empty() { // passed in options take precedence over 'config.format' - properties.extend(options.properties); + config_format.extend(options.properties); lsp::FormattingOptions { - properties, + properties: config_format, ..options } } else { diff --git a/helix-lsp/src/config.rs b/helix-lsp/src/config.rs new file mode 100644 index 000000000..6d9f80080 --- /dev/null +++ b/helix-lsp/src/config.rs @@ -0,0 +1,67 @@ +use std::collections::HashMap; + +use anyhow::bail; +use helix_config::{options, List, Map, String, Ty, Value}; + +use crate::lsp; + +// TODO: differentiating between Some(null) and None is not really practical +// since the distinction is lost on a roundtrip trough config::Value. +// Porbably better to change our code to treat null the way we currently +// treat None +options! { + struct LanguageServerConfig { + /// The name or path of the language server binary to execute. Binaries must be in `$PATH` + command: Option = None, + /// A list of arguments to pass to the language server binary + #[read = deref] + args: List = List::default(), + /// Any environment variables that will be used when starting the language server + enviorment: Map = Map::default(), + /// LSP initialization options + #[name = "config"] + server_config: Option> = None, + /// LSP initialization options + #[read = copy] + timeout: u64 = 20, + // TODO: merge + /// LSP formatting options + #[name = "config.format"] + #[read = fold(HashMap::new(), fold_format_config, FormatConfig)] + format: Map = Map::default() + } +} + +type FormatConfig = HashMap; + +fn fold_format_config(config: &Map, mut res: FormatConfig) -> FormatConfig { + for (k, v) in config.iter() { + res.entry(k.to_string()).or_insert_with(|| v.0.clone()); + } + res +} + +// damm orphan rules :/ +#[derive(Debug, PartialEq, Clone)] +struct FormattingProperty(lsp::FormattingProperty); + +impl Ty for FormattingProperty { + fn from_value(val: Value) -> anyhow::Result { + match val { + Value::Int(_) => Ok(FormattingProperty(lsp::FormattingProperty::Number( + i32::from_value(val)?, + ))), + Value::Bool(val) => Ok(FormattingProperty(lsp::FormattingProperty::Bool(val))), + Value::String(val) => Ok(FormattingProperty(lsp::FormattingProperty::String(val))), + _ => bail!("expected a string, boolean or integer"), + } + } + + fn to_value(&self) -> Value { + match self.0 { + lsp::FormattingProperty::Bool(val) => Value::Bool(val), + lsp::FormattingProperty::Number(val) => Value::Int(val as _), + lsp::FormattingProperty::String(ref val) => Value::String(val.clone()), + } + } +} diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 34278cd54..92dab5966 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -1,4 +1,5 @@ mod client; +mod config; pub mod file_event; pub mod jsonrpc; pub mod snippet; @@ -11,6 +12,7 @@ pub use lsp::{Position, Url}; pub use lsp_types as lsp; use futures_util::stream::select_all::SelectAll; +use helix_config::OptionRegistry; use helix_core::{ path, syntax::{LanguageConfiguration, LanguageServerConfiguration, LanguageServerFeatures}, @@ -26,6 +28,8 @@ use std::{ use thiserror::Error; use tokio_stream::wrappers::UnboundedReceiverStream; +use crate::config::init_config; + pub type Result = core::result::Result; pub type LanguageServerName = String; @@ -636,17 +640,25 @@ pub struct Registry { counter: usize, pub incoming: SelectAll>, pub file_event_handler: file_event::Handler, + pub config: OptionRegistry, } impl Registry { pub fn new(syn_loader: Arc) -> Self { - Self { + let mut res = Self { inner: HashMap::new(), syn_loader, counter: 0, incoming: SelectAll::new(), file_event_handler: file_event::Handler::new(), - } + config: OptionRegistry::new(), + }; + res.reset_config(); + res + } + + pub fn reset_config(&mut self) { + init_config(&mut self.config); } pub fn get_by_id(&self, id: usize) -> Option<&Client> { @@ -882,15 +894,11 @@ fn start_client( enable_snippets: bool, ) -> Result { let (client, incoming, initialize_notify) = Client::start( - &ls_config.command, - &ls_config.args, - ls_config.config.clone(), - ls_config.environment.clone(), + todo!(), &config.roots, config.workspace_lsp_roots.as_deref().unwrap_or(root_dirs), id, name, - ls_config.timeout, doc_path, )?; diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4eda8097c..7e6cdc880 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -699,7 +699,7 @@ impl Application { // Trigger a workspace/didChangeConfiguration notification after initialization. // This might not be required by the spec but Neovim does this as well, so it's // probably a good idea for compatibility. - if let Some(config) = language_server.config() { + if let Some(config) = language_server.config().as_deref() { tokio::spawn(language_server.did_change_configuration(config.clone())); } @@ -1023,7 +1023,8 @@ impl Application { .items .iter() .map(|item| { - let mut config = language_server.config()?; + let config = language_server.config(); + let mut config = config.as_deref()?; if let Some(section) = item.section.as_ref() { // for some reason some lsps send an empty string (observed in 'vscode-eslint-language-server') if !section.is_empty() { @@ -1032,7 +1033,7 @@ impl Application { } } } - Some(config) + Some(config.to_owned()) }) .collect(); Ok(json!(result))