
Hi folks! @absynce and I paired a bit to improve the
`elm-language-server` configuration. We have realised that sometimes
`elm-language-server` settings were being reset to default. We had been
configuring `elm-language-server` like this:
```json
"lsp": {
"elm-language-server": {
"initialization_options": {
"disableElmLSDiagnostics": true,
"onlyUpdateDiagnosticsOnSave": true,
"elmReviewDiagnostics": "warning"
}
}
}
```
And then we noticed that the following communication happened:
```
// Send:
{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{}}}
// Receive:
{"jsonrpc":"2.0","id":5,"method":"workspace/configuration","params":{"items":[{"section":"elmLS"}]}}
// Send:
{"jsonrpc":"2.0","id":5,"result":[null],"error":null}
```
In `elm-language-server` the settings from `didChangeConfiguration`
[replace the initial
settings](edd6813388/src/common/providers/diagnostics/diagnosticsProvider.ts (L188)
).
Setting the value to `{}` effectively resets the configuration options
to defaults.
In Zed, `initialization_options` and `workspace_configuration` are two
different things, but in `elm-language-server` they are coupled.
Additionally, `elm-language-server` is requesting workspace
configuration for the `elmLS` section that doesn't exist.
This PR:
1. Fixes settings reset on `didChangeConfiguration` by populating
`workspace_configuration` from `initialization_options`
2. Makes workspace configuration requests work by inserting an extra
copy of the settings under the `elmLS` key in `workspace_configuration`
— this is a bit ugly, but we're not sure how to make both kinds of
configuration messages work in the current setup.
This is how communication looks like after the proposed changes:
```
// Send:
{
"jsonrpc": "2.0",
"method": "workspace/didChangeConfiguration",
"params": {
"settings": {
"disableElmLSDiagnostics": true,
"onlyUpdateDiagnosticsOnSave": true,
"elmReviewDiagnostics": "warning",
"elmLS": {
"disableElmLSDiagnostics": true,
"onlyUpdateDiagnosticsOnSave": true,
"elmReviewDiagnostics": "warning"
}
}
}
}
// Receive:
{
"jsonrpc": "2.0",
"id": 4,
"method": "workspace/configuration",
"params": {
"items": [
{
"section": "elmLS"
}
]
}
}
// Send:
{
"jsonrpc": "2.0",
"id": 4,
"result": [
{
"disableElmLSDiagnostics": true,
"onlyUpdateDiagnosticsOnSave": true,
"elmReviewDiagnostics": "warning"
}
],
"error": null
}
```
Things we have considered:
1. Extracting the `elm-language-server` settings into a separate
section: we haven't found this being widely used in Zed, seems that all
language server configuration should fall under the top level `lsp`
section
2. Changing the way `elm-language-server` configuration works:
`elm-language-server` has got integrations with multiple editors,
changing the configuration behaviour would mean updating all the
existing integrations. Plus we are not exactly sure if it's doing
anything wrong.
Release Notes:
- Improved elm-language-server configuration options
Co-authored-by: Jared M. Smith <absynce@gmail.com>
148 lines
4.5 KiB
Rust
148 lines
4.5 KiB
Rust
use anyhow::{anyhow, Result};
|
|
use async_trait::async_trait;
|
|
use futures::StreamExt;
|
|
use gpui::AppContext;
|
|
use language::{LanguageServerName, LspAdapter, LspAdapterDelegate};
|
|
use lsp::LanguageServerBinary;
|
|
use node_runtime::NodeRuntime;
|
|
use project::project_settings::ProjectSettings;
|
|
use serde_json::Value;
|
|
use settings::Settings;
|
|
use smol::fs;
|
|
use std::{
|
|
any::Any,
|
|
ffi::OsString,
|
|
path::{Path, PathBuf},
|
|
sync::Arc,
|
|
};
|
|
use util::ResultExt;
|
|
|
|
const SERVER_NAME: &'static str = "elm-language-server";
|
|
const SERVER_PATH: &'static str = "node_modules/@elm-tooling/elm-language-server/out/node/index.js";
|
|
|
|
fn server_binary_arguments(server_path: &Path) -> Vec<OsString> {
|
|
vec![server_path.into(), "--stdio".into()]
|
|
}
|
|
|
|
pub struct ElmLspAdapter {
|
|
node: Arc<dyn NodeRuntime>,
|
|
}
|
|
|
|
impl ElmLspAdapter {
|
|
pub fn new(node: Arc<dyn NodeRuntime>) -> Self {
|
|
ElmLspAdapter { node }
|
|
}
|
|
}
|
|
|
|
#[async_trait]
|
|
impl LspAdapter for ElmLspAdapter {
|
|
fn name(&self) -> LanguageServerName {
|
|
LanguageServerName(SERVER_NAME.into())
|
|
}
|
|
|
|
fn short_name(&self) -> &'static str {
|
|
"elmLS"
|
|
}
|
|
|
|
async fn fetch_latest_server_version(
|
|
&self,
|
|
_: &dyn LspAdapterDelegate,
|
|
) -> Result<Box<dyn 'static + Any + Send>> {
|
|
Ok(Box::new(
|
|
self.node
|
|
.npm_package_latest_version("@elm-tooling/elm-language-server")
|
|
.await?,
|
|
) as Box<_>)
|
|
}
|
|
|
|
async fn fetch_server_binary(
|
|
&self,
|
|
version: Box<dyn 'static + Send + Any>,
|
|
container_dir: PathBuf,
|
|
_: &dyn LspAdapterDelegate,
|
|
) -> Result<LanguageServerBinary> {
|
|
let version = version.downcast::<String>().unwrap();
|
|
let server_path = container_dir.join(SERVER_PATH);
|
|
|
|
if fs::metadata(&server_path).await.is_err() {
|
|
self.node
|
|
.npm_install_packages(
|
|
&container_dir,
|
|
&[("@elm-tooling/elm-language-server", version.as_str())],
|
|
)
|
|
.await?;
|
|
}
|
|
|
|
Ok(LanguageServerBinary {
|
|
path: self.node.binary_path().await?,
|
|
arguments: server_binary_arguments(&server_path),
|
|
})
|
|
}
|
|
|
|
async fn cached_server_binary(
|
|
&self,
|
|
container_dir: PathBuf,
|
|
_: &dyn LspAdapterDelegate,
|
|
) -> Option<LanguageServerBinary> {
|
|
get_cached_server_binary(container_dir, &*self.node).await
|
|
}
|
|
|
|
async fn installation_test_binary(
|
|
&self,
|
|
container_dir: PathBuf,
|
|
) -> Option<LanguageServerBinary> {
|
|
get_cached_server_binary(container_dir, &*self.node).await
|
|
}
|
|
|
|
fn workspace_configuration(&self, _workspace_root: &Path, cx: &mut AppContext) -> Value {
|
|
// elm-language-server expects workspace didChangeConfiguration notification
|
|
// params to be the same as lsp initialization_options
|
|
let override_options = ProjectSettings::get_global(cx)
|
|
.lsp
|
|
.get(SERVER_NAME)
|
|
.and_then(|s| s.initialization_options.clone())
|
|
.unwrap_or_default();
|
|
|
|
match override_options.clone().as_object_mut() {
|
|
Some(op) => {
|
|
// elm-language-server requests workspace configuration
|
|
// for the `elmLS` section, so we have to nest
|
|
// another copy of initialization_options there
|
|
op.insert("elmLS".into(), override_options);
|
|
serde_json::to_value(op).unwrap_or_default()
|
|
}
|
|
None => override_options,
|
|
}
|
|
}
|
|
}
|
|
|
|
async fn get_cached_server_binary(
|
|
container_dir: PathBuf,
|
|
node: &dyn NodeRuntime,
|
|
) -> Option<LanguageServerBinary> {
|
|
(|| async move {
|
|
let mut last_version_dir = None;
|
|
let mut entries = fs::read_dir(&container_dir).await?;
|
|
while let Some(entry) = entries.next().await {
|
|
let entry = entry?;
|
|
if entry.file_type().await?.is_dir() {
|
|
last_version_dir = Some(entry.path());
|
|
}
|
|
}
|
|
let last_version_dir = last_version_dir.ok_or_else(|| anyhow!("no cached binary"))?;
|
|
let server_path = last_version_dir.join(SERVER_PATH);
|
|
if server_path.exists() {
|
|
Ok(LanguageServerBinary {
|
|
path: node.binary_path().await?,
|
|
arguments: server_binary_arguments(&server_path),
|
|
})
|
|
} else {
|
|
Err(anyhow!(
|
|
"missing executable in directory {:?}",
|
|
last_version_dir
|
|
))
|
|
}
|
|
})()
|
|
.await
|
|
.log_err()
|
|
}
|