From 5c41f22c2a20a1b8a91ddd6397686bd752591ffc Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Fri, 21 Jul 2023 15:21:21 -0700 Subject: [PATCH] Add support for LSP DidChangeWatchedFiles (#7665) * Add initial support for LSP DidChangeWatchedFiles * Move file event Handler to helix-lsp * Simplify file event handling * Refactor file event handling * Block on future within LSP file event handler * Fully qualify uses of the file_event::Handler type * Rename ops field to options * Revert newline removal from helix-view/Cargo.toml * Ensure file event Handler is cleaned up when lsp client is shutdown --- Cargo.lock | 30 ++--- helix-lsp/Cargo.toml | 1 + helix-lsp/src/client.rs | 13 +++ helix-lsp/src/file_event.rs | 193 +++++++++++++++++++++++++++++++ helix-lsp/src/lib.rs | 11 ++ helix-term/src/application.rs | 70 +++++++++-- helix-term/src/commands/typed.rs | 15 ++- helix-view/src/editor.rs | 21 +++- 8 files changed, 325 insertions(+), 29 deletions(-) create mode 100644 helix-lsp/src/file_event.rs diff --git a/Cargo.lock b/Cargo.lock index a26bdfca..85848f9b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -51,9 +51,9 @@ dependencies = [ [[package]] name = "aho-corasick" -version = "1.0.1" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67fc08ce920c31afb70f013dcce1bfc3a3195de6a228474e45e1f145b36f8d04" +checksum = "43f6cb1bf222025340178f382c426f13757b2960e89779dfcb319c32542a5a41" dependencies = [ "memchr", ] @@ -126,13 +126,12 @@ checksum = "630be753d4e58660abd17930c71b647fe46c27ea6b63cc59e1e3851406972e42" [[package]] name = "bstr" -version = "1.4.0" +version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3d4260bcc2e8fc9df1eac4919a720effeb63a3f0952f5bf4944adfa18897f09" +checksum = "6798148dccfbff0fae41c7574d2fa8f1ef3492fba0face179de5d8d447d67b05" dependencies = [ "memchr", - "once_cell", - "regex-automata 0.1.10", + "regex-automata", "serde", ] @@ -1149,11 +1148,11 @@ dependencies = [ [[package]] name = "globset" -version = "0.4.10" +version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "029d74589adefde59de1a0c4f4732695c32805624aec7b68d91503d4dba79afc" +checksum = "1391ab1f92ffcc08911957149833e682aa3fe252b9f45f966d2ef972274c97df" dependencies = [ - "aho-corasick 0.7.20", + "aho-corasick 1.0.2", "bstr", "fnv", "log", @@ -1292,6 +1291,7 @@ dependencies = [ "anyhow", "futures-executor", "futures-util", + "globset", "helix-core", "helix-loader", "helix-parsec", @@ -1878,25 +1878,19 @@ version = "1.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b2eae68fc220f7cf2532e4494aded17545fce192d59cd996e0fe7887f4ceb575" dependencies = [ - "aho-corasick 1.0.1", + "aho-corasick 1.0.2", "memchr", - "regex-automata 0.3.2", + "regex-automata", "regex-syntax 0.7.3", ] -[[package]] -name = "regex-automata" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" - [[package]] name = "regex-automata" version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "83d3daa6976cffb758ec878f108ba0e062a45b2d6ca3a2cca965338855476caf" dependencies = [ - "aho-corasick 1.0.1", + "aho-corasick 1.0.2", "memchr", "regex-syntax 0.7.3", ] diff --git a/helix-lsp/Cargo.toml b/helix-lsp/Cargo.toml index b4abeb8a..33c63e0a 100644 --- a/helix-lsp/Cargo.toml +++ b/helix-lsp/Cargo.toml @@ -19,6 +19,7 @@ helix-parsec = { version = "0.6", path = "../helix-parsec" } anyhow = "1.0" futures-executor = "0.3" futures-util = { version = "0.3", features = ["std", "async-await"], default-features = false } +globset = "0.4.11" log = "0.4" lsp-types = { version = "0.94" } serde = { version = "1.0", features = ["derive"] } diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 92ab03db..ed9a2f83 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -544,6 +544,10 @@ impl Client { normalizes_line_endings: Some(false), change_annotation_support: None, }), + did_change_watched_files: Some(lsp::DidChangeWatchedFilesClientCapabilities { + dynamic_registration: Some(true), + relative_pattern_support: Some(false), + }), ..Default::default() }), text_document: Some(lsp::TextDocumentClientCapabilities { @@ -1453,4 +1457,13 @@ impl Client { Some(self.call::(params)) } + + pub fn did_change_watched_files( + &self, + changes: Vec, + ) -> impl Future> { + self.notify::(lsp::DidChangeWatchedFilesParams { + changes, + }) + } } diff --git a/helix-lsp/src/file_event.rs b/helix-lsp/src/file_event.rs new file mode 100644 index 00000000..26a27c98 --- /dev/null +++ b/helix-lsp/src/file_event.rs @@ -0,0 +1,193 @@ +use std::{collections::HashMap, path::PathBuf, sync::Weak}; + +use globset::{GlobBuilder, GlobSetBuilder}; +use tokio::sync::mpsc; + +use crate::{lsp, Client}; + +enum Event { + FileChanged { + path: PathBuf, + }, + Register { + client_id: usize, + client: Weak, + registration_id: String, + options: lsp::DidChangeWatchedFilesRegistrationOptions, + }, + Unregister { + client_id: usize, + registration_id: String, + }, + RemoveClient { + client_id: usize, + }, +} + +#[derive(Default)] +struct ClientState { + client: Weak, + registered: HashMap, +} + +/// The Handler uses a dedicated tokio task to respond to file change events by +/// forwarding changes to LSPs that have registered for notifications with a +/// matching glob. +/// +/// When an LSP registers for the DidChangeWatchedFiles notification, the +/// Handler is notified by sending the registration details in addition to a +/// weak reference to the LSP client. This is done so that the Handler can have +/// access to the client without preventing the client from being dropped if it +/// is closed and the Handler isn't properly notified. +#[derive(Clone, Debug)] +pub struct Handler { + tx: mpsc::UnboundedSender, +} + +impl Default for Handler { + fn default() -> Self { + Self::new() + } +} + +impl Handler { + pub fn new() -> Self { + let (tx, rx) = mpsc::unbounded_channel(); + tokio::spawn(Self::run(rx)); + Self { tx } + } + + pub fn register( + &self, + client_id: usize, + client: Weak, + registration_id: String, + options: lsp::DidChangeWatchedFilesRegistrationOptions, + ) { + let _ = self.tx.send(Event::Register { + client_id, + client, + registration_id, + options, + }); + } + + pub fn unregister(&self, client_id: usize, registration_id: String) { + let _ = self.tx.send(Event::Unregister { + client_id, + registration_id, + }); + } + + pub fn file_changed(&self, path: PathBuf) { + let _ = self.tx.send(Event::FileChanged { path }); + } + + pub fn remove_client(&self, client_id: usize) { + let _ = self.tx.send(Event::RemoveClient { client_id }); + } + + async fn run(mut rx: mpsc::UnboundedReceiver) { + let mut state: HashMap = HashMap::new(); + while let Some(event) = rx.recv().await { + match event { + Event::FileChanged { path } => { + log::debug!("Received file event for {:?}", &path); + + state.retain(|id, client_state| { + if !client_state + .registered + .values() + .any(|glob| glob.is_match(&path)) + { + return true; + } + let Some(client) = client_state.client.upgrade() else { + log::warn!("LSP client was dropped: {id}"); + return false; + }; + let Ok(uri) = lsp::Url::from_file_path(&path) else { + return true; + }; + log::debug!( + "Sending didChangeWatchedFiles notification to client '{}'", + client.name() + ); + if let Err(err) = crate::block_on(client + .did_change_watched_files(vec![lsp::FileEvent { + uri, + // We currently always send the CHANGED state + // since we don't actually have more context at + // the moment. + typ: lsp::FileChangeType::CHANGED, + }])) + { + log::warn!("Failed to send didChangeWatchedFiles notification to client: {err}"); + } + true + }); + } + Event::Register { + client_id, + client, + registration_id, + options: ops, + } => { + log::debug!( + "Registering didChangeWatchedFiles for client '{}' with id '{}'", + client_id, + registration_id + ); + + let mut entry = state.entry(client_id).or_insert_with(ClientState::default); + entry.client = client; + + let mut builder = GlobSetBuilder::new(); + for watcher in ops.watchers { + if let lsp::GlobPattern::String(pattern) = watcher.glob_pattern { + if let Ok(glob) = GlobBuilder::new(&pattern).build() { + builder.add(glob); + } + } + } + match builder.build() { + Ok(globset) => { + entry.registered.insert(registration_id, globset); + } + Err(err) => { + // Remove any old state for that registration id and + // remove the entire client if it's now empty. + entry.registered.remove(®istration_id); + if entry.registered.is_empty() { + state.remove(&client_id); + } + log::warn!( + "Unable to build globset for LSP didChangeWatchedFiles {err}" + ) + } + } + } + Event::Unregister { + client_id, + registration_id, + } => { + log::debug!( + "Unregistering didChangeWatchedFiles with id '{}' for client '{}'", + registration_id, + client_id + ); + if let Some(client_state) = state.get_mut(&client_id) { + client_state.registered.remove(®istration_id); + if client_state.registered.is_empty() { + state.remove(&client_id); + } + } + } + Event::RemoveClient { client_id } => { + log::debug!("Removing LSP client: {client_id}"); + state.remove(&client_id); + } + } + } + } +} diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 95c61086..90f0c3fd 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -1,4 +1,5 @@ mod client; +pub mod file_event; pub mod jsonrpc; pub mod snippet; mod transport; @@ -547,6 +548,7 @@ pub enum MethodCall { WorkspaceFolders, WorkspaceConfiguration(lsp::ConfigurationParams), RegisterCapability(lsp::RegistrationParams), + UnregisterCapability(lsp::UnregistrationParams), } impl MethodCall { @@ -570,6 +572,10 @@ impl MethodCall { let params: lsp::RegistrationParams = params.parse()?; Self::RegisterCapability(params) } + lsp::request::UnregisterCapability::METHOD => { + let params: lsp::UnregistrationParams = params.parse()?; + Self::UnregisterCapability(params) + } _ => { return Err(Error::Unhandled); } @@ -629,6 +635,7 @@ pub struct Registry { syn_loader: Arc, counter: usize, pub incoming: SelectAll>, + pub file_event_handler: file_event::Handler, } impl Registry { @@ -638,6 +645,7 @@ impl Registry { syn_loader, counter: 0, incoming: SelectAll::new(), + file_event_handler: file_event::Handler::new(), } } @@ -650,6 +658,7 @@ impl Registry { } pub fn remove_by_id(&mut self, id: usize) { + self.file_event_handler.remove_client(id); self.inner.retain(|_, language_servers| { language_servers.retain(|ls| id != ls.id()); !language_servers.is_empty() @@ -715,6 +724,7 @@ impl Registry { .unwrap(); for old_client in old_clients { + self.file_event_handler.remove_client(old_client.id()); tokio::spawn(async move { let _ = old_client.force_shutdown().await; }); @@ -731,6 +741,7 @@ impl Registry { pub fn stop(&mut self, name: &str) { if let Some(clients) = self.inner.remove(name) { for client in clients { + self.file_event_handler.remove_client(client.id()); tokio::spawn(async move { let _ = client.force_shutdown().await; }); diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index b8950ae0..dc461198 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -5,7 +5,11 @@ use helix_core::{ path::get_relative_path, pos_at_coords, syntax, Selection, }; -use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap}; +use helix_lsp::{ + lsp::{self, notification::Notification}, + util::lsp_pos_to_pos, + LspProgressMap, +}; use helix_view::{ align_view, document::DocumentSavedEventResult, @@ -1080,17 +1084,65 @@ impl Application { .collect(); Ok(json!(result)) } - Ok(MethodCall::RegisterCapability(_params)) => { - log::warn!("Ignoring a client/registerCapability request because dynamic capability registration is not enabled. Please report this upstream to the language server"); - // Language Servers based on the `vscode-languageserver-node` library often send - // client/registerCapability even though we do not enable dynamic registration - // for any capabilities. We should send a MethodNotFound JSONRPC error in this - // case but that rejects the registration promise in the server which causes an - // exit. So we work around this by ignoring the request and sending back an OK - // response. + Ok(MethodCall::RegisterCapability(params)) => { + if let Some(client) = self + .editor + .language_servers + .iter_clients() + .find(|client| client.id() == server_id) + { + for reg in params.registrations { + match reg.method.as_str() { + lsp::notification::DidChangeWatchedFiles::METHOD => { + let Some(options) = reg.register_options else { + continue; + }; + let ops: lsp::DidChangeWatchedFilesRegistrationOptions = + match serde_json::from_value(options) { + Ok(ops) => ops, + Err(err) => { + log::warn!("Failed to deserialize DidChangeWatchedFilesRegistrationOptions: {err}"); + continue; + } + }; + self.editor.language_servers.file_event_handler.register( + client.id(), + Arc::downgrade(client), + reg.id, + ops, + ) + } + _ => { + // Language Servers based on the `vscode-languageserver-node` library often send + // client/registerCapability even though we do not enable dynamic registration + // for most capabilities. We should send a MethodNotFound JSONRPC error in this + // case but that rejects the registration promise in the server which causes an + // exit. So we work around this by ignoring the request and sending back an OK + // response. + log::warn!("Ignoring a client/registerCapability request because dynamic capability registration is not enabled. Please report this upstream to the language server"); + } + } + } + } Ok(serde_json::Value::Null) } + Ok(MethodCall::UnregisterCapability(params)) => { + for unreg in params.unregisterations { + match unreg.method.as_str() { + lsp::notification::DidChangeWatchedFiles::METHOD => { + self.editor + .language_servers + .file_event_handler + .unregister(server_id, unreg.id); + } + _ => { + log::warn!("Received unregistration request for unsupported method: {}", unreg.method); + } + } + } + Ok(serde_json::Value::Null) + } }; tokio::spawn(language_server!().reply(id, reply)); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index dfc71dfd..063658c6 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1283,7 +1283,14 @@ fn reload( doc.reload(view, &cx.editor.diff_providers, redraw_handle) .map(|_| { view.ensure_cursor_in_view(doc, scrolloff); - }) + })?; + if let Some(path) = doc.path() { + cx.editor + .language_servers + .file_event_handler + .file_changed(path.clone()); + } + Ok(()) } fn reload_all( @@ -1324,6 +1331,12 @@ fn reload_all( let redraw_handle = cx.editor.redraw_handle.clone(); doc.reload(view, &cx.editor.diff_providers, redraw_handle)?; + if let Some(path) = doc.path() { + cx.editor + .language_servers + .file_event_handler + .file_changed(path.clone()); + } for view_id in view_ids { let view = view_mut!(cx.editor, view_id); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index affe87dd..20469ae9 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1535,7 +1535,18 @@ impl Editor { let path = path.map(|path| path.into()); let doc = doc_mut!(self, &doc_id); - let future = doc.save(path, force)?; + let doc_save_future = doc.save(path, force)?; + + // When a file is written to, notify the file event handler. + // Note: This can be removed once proper file watching is implemented. + let handler = self.language_servers.file_event_handler.clone(); + let future = async move { + let res = doc_save_future.await; + if let Ok(event) = &res { + handler.file_changed(event.path.clone()); + } + res + }; use futures_util::stream; @@ -1671,6 +1682,14 @@ impl Editor { &self, timeout: Option, ) -> Result<(), tokio::time::error::Elapsed> { + // Remove all language servers from the file event handler. + // Note: this is non-blocking. + for client in self.language_servers.iter_clients() { + self.language_servers + .file_event_handler + .remove_client(client.id()); + } + tokio::time::timeout( Duration::from_millis(timeout.unwrap_or(3000)), future::join_all(