From c00cf238afe3dbd43327fd74bd8a9ff2dd9c21db Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Wed, 1 Sep 2021 14:38:47 +0900 Subject: Simplify textDocument/didClose, we don't need to look up LSP again --- helix-view/src/editor.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'helix-view/src') diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 562c3c60..050f2645 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -287,14 +287,9 @@ impl Editor { if close_buffer { // get around borrowck issues - let language_servers = &mut self.language_servers; let doc = &self.documents[view.doc]; - let language_server = doc - .language - .as_ref() - .and_then(|language| language_servers.get(language).ok()); - if let Some(language_server) = language_server { + if let Some(language_server) = doc.language_server() { tokio::spawn(language_server.text_document_did_close(doc.identifier())); } self.documents.remove(view.doc); -- cgit v1.2.3-70-g09d2 From 184637c55acca49380372ca118f13b3390bcb003 Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Wed, 1 Sep 2021 14:49:48 +0900 Subject: lsp: refactor format so we stop cloning the language_server --- helix-lsp/src/client.rs | 14 +++++++++----- helix-view/src/document.rs | 23 +++++++++++++---------- 2 files changed, 22 insertions(+), 15 deletions(-) (limited to 'helix-view/src') diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index ac6ae70a..fdff553f 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -583,19 +583,19 @@ impl Client { // formatting - pub async fn text_document_formatting( + pub fn text_document_formatting( &self, text_document: lsp::TextDocumentIdentifier, options: lsp::FormattingOptions, work_done_token: Option, - ) -> anyhow::Result> { + ) -> Option>>> { let capabilities = self.capabilities.get().unwrap(); // check if we're able to format match capabilities.document_formatting_provider { Some(lsp::OneOf::Left(true)) | Some(lsp::OneOf::Right(_)) => (), // None | Some(false) - _ => return Ok(Vec::new()), + _ => return None, }; // TODO: return err::unavailable so we can fall back to tree sitter formatting @@ -605,9 +605,13 @@ impl Client { work_done_progress_params: lsp::WorkDoneProgressParams { work_done_token }, }; - let response = self.request::(params).await?; + let request = self.call::(params); - Ok(response.unwrap_or_default()) + Some(async move { + let json = request.await?; + let response: Vec = serde_json::from_value(json)?; + Ok(response) + }) } pub async fn text_document_range_formatting( diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index b2c02927..a27be8e6 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -386,21 +386,24 @@ impl Document { /// If supported, returns the changes that should be applied to this document in order /// to format it nicely. pub fn format(&self) -> Option + 'static> { - if let Some(language_server) = self.language_server.clone() { + if let Some(language_server) = self.language_server() { let text = self.text.clone(); - let id = self.identifier(); + let offset_encoding = language_server.offset_encoding(); + let request = language_server.text_document_formatting( + self.identifier(), + lsp::FormattingOptions::default(), + None, + )?; + let fut = async move { - let edits = language_server - .text_document_formatting(id, lsp::FormattingOptions::default(), None) - .await - .unwrap_or_else(|e| { - log::warn!("LSP formatting failed: {}", e); - Default::default() - }); + let edits = request.await.unwrap_or_else(|e| { + log::warn!("LSP formatting failed: {}", e); + Default::default() + }); LspFormatting { doc: text, edits, - offset_encoding: language_server.offset_encoding(), + offset_encoding, } }; Some(fut) -- cgit v1.2.3-70-g09d2 From 800d79b584cd09020488b8a614e5214b929d8f5d Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Wed, 1 Sep 2021 14:54:11 +0900 Subject: ls: Refactor textDocument/didSave in a similar vein --- helix-lsp/src/client.rs | 19 ++++++++++--------- helix-view/src/document.rs | 8 ++++---- 2 files changed, 14 insertions(+), 13 deletions(-) (limited to 'helix-view/src') diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index fdff553f..b8fbfddb 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -490,11 +490,11 @@ impl Client { // will_save / will_save_wait_until - pub async fn text_document_did_save( + pub fn text_document_did_save( &self, text_document: lsp::TextDocumentIdentifier, text: &Rope, - ) -> Result<()> { + ) -> Option>> { let capabilities = self.capabilities.get().unwrap(); let include_text = match &capabilities.text_document_sync { @@ -507,17 +507,18 @@ impl Client { include_text, }) => include_text.unwrap_or(false), // Supported(false) - _ => return Ok(()), + _ => return None, }, // unsupported - _ => return Ok(()), + _ => return None, }; - self.notify::(lsp::DidSaveTextDocumentParams { - text_document, - text: include_text.then(|| text.into()), - }) - .await + Some(self.notify::( + lsp::DidSaveTextDocumentParams { + text_document, + text: include_text.then(|| text.into()), + }, + )) } pub fn completion( diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index a27be8e6..5677eb44 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -471,10 +471,10 @@ impl Document { let mut file = File::create(path).await?; to_writer(&mut file, encoding, &text).await?; - if let Some(language_server) = language_server { - language_server - .text_document_did_save(identifier, &text) - .await?; + if let Some(notification) = language_server.and_then(|language_server| { + language_server.text_document_did_save(identifier, &text) + }) { + notification.await?; } Ok(()) -- cgit v1.2.3-70-g09d2 From 10b690b5bd5d3e9ee477782ebfe3f6ff8d11cb3f Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Thu, 2 Sep 2021 10:49:23 +0900 Subject: Drop some &mut bounds where & would have sufficed --- helix-term/src/job.rs | 4 ++-- helix-view/src/document.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'helix-view/src') diff --git a/helix-term/src/job.rs b/helix-term/src/job.rs index 2ac41926..4fa38174 100644 --- a/helix-term/src/job.rs +++ b/helix-term/src/job.rs @@ -61,7 +61,7 @@ impl Jobs { } pub fn handle_callback( - &mut self, + &self, editor: &mut Editor, compositor: &mut Compositor, call: anyhow::Result>, @@ -84,7 +84,7 @@ impl Jobs { } } - pub fn add(&mut self, j: Job) { + pub fn add(&self, j: Job) { if j.wait { self.wait_futures.push(j.future); } else { diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 5677eb44..71f6680c 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -649,7 +649,7 @@ impl Document { // } // emit lsp notification - if let Some(language_server) = &self.language_server { + if let Some(language_server) = self.language_server() { let notify = language_server.text_document_did_change( self.versioned_identifier(), &old_doc, -- cgit v1.2.3-70-g09d2 From dc7799b980826ffe33ed635968def79daf20bd10 Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Thu, 2 Sep 2021 11:28:40 +0900 Subject: lsp: Refactor code that could use document_by_path_mut --- helix-term/src/application.rs | 11 +++-------- helix-view/src/editor.rs | 5 +++++ 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'helix-view/src') diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 8241ce3a..d3b65a4f 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -276,15 +276,10 @@ impl Application { match notification { Notification::PublishDiagnostics(params) => { - let path = Some(params.uri.to_file_path().unwrap()); + let path = params.uri.to_file_path().unwrap(); + let doc = self.editor.document_by_path_mut(&path); - let doc = self - .editor - .documents - .iter_mut() - .find(|(_, doc)| doc.path() == path.as_ref()); - - if let Some((_, doc)) = doc { + if let Some(doc) = doc { let text = doc.text(); let diagnostics = params diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 050f2645..0d914e45 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -340,6 +340,11 @@ impl Editor { .find(|doc| doc.path().map(|p| p == path.as_ref()).unwrap_or(false)) } + pub fn document_by_path_mut>(&mut self, path: P) -> Option<&mut Document> { + self.documents_mut() + .find(|doc| doc.path().map(|p| p == path.as_ref()).unwrap_or(false)) + } + pub fn cursor(&self) -> (Option, CursorKind) { let view = view!(self); let doc = &self.documents[view.doc]; -- cgit v1.2.3-70-g09d2 From 59ed1c8c78ab996273bcc910c11724bc8402f711 Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Thu, 2 Sep 2021 12:52:32 +0900 Subject: Simplify documents & documents_mut() --- helix-view/src/editor.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'helix-view/src') diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 0d914e45..c8abd5b5 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -319,20 +319,24 @@ impl Editor { view.ensure_cursor_in_view(doc, self.config.scrolloff) } + #[inline] pub fn document(&self, id: DocumentId) -> Option<&Document> { self.documents.get(id) } + #[inline] pub fn document_mut(&mut self, id: DocumentId) -> Option<&mut Document> { self.documents.get_mut(id) } + #[inline] pub fn documents(&self) -> impl Iterator { - self.documents.iter().map(|(_id, doc)| doc) + self.documents.values() } + #[inline] pub fn documents_mut(&mut self) -> impl Iterator { - self.documents.iter_mut().map(|(_id, doc)| doc) + self.documents.values_mut() } pub fn document_by_path>(&self, path: P) -> Option<&Document> { -- cgit v1.2.3-70-g09d2 From 46f3c69f06cc55f36bcc6244a9f96c2481836dea Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Thu, 2 Sep 2021 13:55:08 +0900 Subject: lsp: Don't send notifications until initialize completes Then send open events for all documents with the LSP attached. --- helix-lsp/src/lib.rs | 98 +++++++++++++++++++++---------------------- helix-lsp/src/transport.rs | 29 ++++++++++++- helix-term/src/application.rs | 31 ++++++++++++++ helix-view/src/editor.rs | 5 ++- 4 files changed, 111 insertions(+), 52 deletions(-) (limited to 'helix-view/src') diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index e10c107b..7357c885 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -226,6 +226,8 @@ impl MethodCall { #[derive(Debug, PartialEq, Clone)] pub enum Notification { + // we inject this notification to signal the LSP is ready + Initialized, PublishDiagnostics(lsp::PublishDiagnosticsParams), ShowMessage(lsp::ShowMessageParams), LogMessage(lsp::LogMessageParams), @@ -237,6 +239,7 @@ impl Notification { use lsp::notification::Notification as _; let notification = match method { + lsp::notification::Initialized::METHOD => Self::Initialized, lsp::notification::PublishDiagnostics::METHOD => { let params: lsp::PublishDiagnosticsParams = params .parse() @@ -294,7 +297,7 @@ impl Registry { } } - pub fn get_by_id(&mut self, id: usize) -> Option<&Client> { + pub fn get_by_id(&self, id: usize) -> Option<&Client> { self.inner .values() .find(|(client_id, _)| client_id == &id) @@ -302,55 +305,52 @@ impl Registry { } pub fn get(&mut self, language_config: &LanguageConfiguration) -> Result> { - if let Some(config) = &language_config.language_server { - // avoid borrow issues - let inner = &mut self.inner; - let s_incoming = &mut self.incoming; - - match inner.entry(language_config.scope.clone()) { - Entry::Occupied(entry) => Ok(entry.get().1.clone()), - Entry::Vacant(entry) => { - // initialize a new client - let id = self.counter.fetch_add(1, Ordering::Relaxed); - let (client, incoming, initialize_notify) = Client::start( - &config.command, - &config.args, - serde_json::from_str(language_config.config.as_deref().unwrap_or("")).ok(), - id, - )?; - s_incoming.push(UnboundedReceiverStream::new(incoming)); - let client = Arc::new(client); - - let _client = client.clone(); - // Initialize the client asynchronously - tokio::spawn(async move { - use futures_util::TryFutureExt; - let value = _client - .capabilities - .get_or_try_init(|| { - _client - .initialize() - .map_ok(|response| response.capabilities) - }) - .await; - - value.expect("failed to initialize capabilities"); - - // next up, notify - _client - .notify::(lsp::InitializedParams {}) - .await - .unwrap(); - - initialize_notify.notify_one(); - }); - - entry.insert((id, client.clone())); - Ok(client) - } + let config = match &language_config.language_server { + Some(config) => config, + None => return Err(Error::LspNotDefined), + }; + + match self.inner.entry(language_config.scope.clone()) { + Entry::Occupied(entry) => Ok(entry.get().1.clone()), + Entry::Vacant(entry) => { + // initialize a new client + let id = self.counter.fetch_add(1, Ordering::Relaxed); + let (client, incoming, initialize_notify) = Client::start( + &config.command, + &config.args, + serde_json::from_str(language_config.config.as_deref().unwrap_or("")).ok(), + id, + )?; + self.incoming.push(UnboundedReceiverStream::new(incoming)); + let client = Arc::new(client); + + // Initialize the client asynchronously + let _client = client.clone(); + tokio::spawn(async move { + use futures_util::TryFutureExt; + let value = _client + .capabilities + .get_or_try_init(|| { + _client + .initialize() + .map_ok(|response| response.capabilities) + }) + .await; + + value.expect("failed to initialize capabilities"); + + // next up, notify + _client + .notify::(lsp::InitializedParams {}) + .await + .unwrap(); + + initialize_notify.notify_one(); + }); + + entry.insert((id, client.clone())); + Ok(client) } - } else { - Err(Error::LspNotDefined) } } diff --git a/helix-lsp/src/transport.rs b/helix-lsp/src/transport.rs index 071c5b93..cf7e66a8 100644 --- a/helix-lsp/src/transport.rs +++ b/helix-lsp/src/transport.rs @@ -64,11 +64,16 @@ impl Transport { let transport = Arc::new(transport); - tokio::spawn(Self::recv(transport.clone(), server_stdout, client_tx)); + tokio::spawn(Self::recv( + transport.clone(), + server_stdout, + client_tx.clone(), + )); tokio::spawn(Self::err(transport.clone(), server_stderr)); tokio::spawn(Self::send( transport, server_stdin, + client_tx, client_rx, notify.clone(), )); @@ -269,6 +274,7 @@ impl Transport { async fn send( transport: Arc, mut server_stdin: BufWriter, + mut client_tx: UnboundedSender<(usize, jsonrpc::Call)>, mut client_rx: UnboundedReceiver, initialize_notify: Arc, ) { @@ -303,6 +309,22 @@ impl Transport { _ = initialize_notify.notified() => { // TODO: notified is technically not cancellation safe // server successfully initialized is_pending = false; + + use lsp_types::notification::Notification; + // Hack: inject an initialized notification so we trigger code that needs to happen after init + let notification = ServerMessage::Call(jsonrpc::Call::Notification(jsonrpc::Notification { + jsonrpc: None, + + method: lsp_types::notification::Initialized::METHOD.to_string(), + params: jsonrpc::Params::None, + })); + match transport.process_server_message(&mut client_tx, notification).await { + Ok(_) => {} + Err(err) => { + error!("err: <- {:?}", err); + } + } + // drain the pending queue and send payloads to server for msg in pending_messages.drain(..) { log::info!("Draining pending message {:?}", msg); @@ -317,6 +339,11 @@ impl Transport { msg = client_rx.recv() => { if let Some(msg) = msg { if is_pending && !is_initialize(&msg) { + // ignore notifications + if let Payload::Notification(_) = msg { + continue; + } + log::info!("Language server not initialized, delaying request"); pending_messages.push(msg); } else { diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index d3b65a4f..e21c5504 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -275,6 +275,37 @@ impl Application { }; match notification { + Notification::Initialized => { + let language_server = + match self.editor.language_servers.get_by_id(server_id) { + Some(language_server) => language_server, + None => { + warn!("can't find language server with id `{}`", server_id); + return; + } + }; + + let docs = self.editor.documents().filter(|doc| { + doc.language_server().map(|server| server.id()) == Some(server_id) + }); + + // trigger textDocument/didOpen for docs that are already open + for doc in docs { + // TODO: extract and share with editor.open + let language_id = doc + .language() + .and_then(|s| s.split('.').last()) // source.rust + .map(ToOwned::to_owned) + .unwrap_or_default(); + + tokio::spawn(language_server.text_document_did_open( + doc.url().unwrap(), + doc.version(), + doc.text(), + language_id, + )); + } + } Notification::PublishDiagnostics(params) => { let path = params.uri.to_file_path().unwrap(); let doc = self.editor.document_by_path_mut(&path); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c8abd5b5..3d2d4a87 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -255,20 +255,21 @@ impl Editor { .and_then(|language| self.language_servers.get(language).ok()); if let Some(language_server) = language_server { - doc.set_language_server(Some(language_server.clone())); - let language_id = doc .language() .and_then(|s| s.split('.').last()) // source.rust .map(ToOwned::to_owned) .unwrap_or_default(); + // TODO: this now races with on_init code if the init happens too quickly tokio::spawn(language_server.text_document_did_open( doc.url().unwrap(), doc.version(), doc.text(), language_id, )); + + doc.set_language_server(Some(language_server)); } let id = self.documents.insert(doc); -- cgit v1.2.3-70-g09d2 From 37606bad47fe0e197cb34fc7d52856597c32ce33 Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Thu, 2 Sep 2021 13:55:55 +0900 Subject: lsp: doc.language_server() is None until initialize completes --- helix-lsp/src/client.rs | 4 ++++ helix-view/src/document.rs | 13 +++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) (limited to 'helix-view/src') diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 27e4697c..f2bb0059 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -93,6 +93,10 @@ impl Client { } } + pub fn is_initialized(&self) -> bool { + self.capabilities.get().is_some() + } + pub fn capabilities(&self) -> &lsp::ServerCapabilities { self.capabilities .get() diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 71f6680c..362a433e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -798,9 +798,18 @@ impl Document { self.version } - #[inline] pub fn language_server(&self) -> Option<&helix_lsp::Client> { - self.language_server.as_deref() + let server = self.language_server.as_deref(); + let initialized = server + .map(|server| server.is_initialized()) + .unwrap_or(false); + + // only resolve language_server if it's initialized + if initialized { + server + } else { + None + } } #[inline] -- cgit v1.2.3-70-g09d2 From 64099af3f1f5957de8f4203c665063d02ba4bac9 Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Thu, 2 Sep 2021 16:07:21 +0900 Subject: Don't panic on save if language_server isn't initialized --- helix-view/src/document.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'helix-view/src') diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 362a433e..6de60995 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -471,10 +471,15 @@ impl Document { let mut file = File::create(path).await?; to_writer(&mut file, encoding, &text).await?; - if let Some(notification) = language_server.and_then(|language_server| { - language_server.text_document_did_save(identifier, &text) - }) { - notification.await?; + if let Some(language_server) = language_server { + if language_server.is_initialized() { + return Ok(()); + } + if let Some(notification) = + language_server.text_document_did_save(identifier, &text) + { + notification.await?; + } } Ok(()) -- cgit v1.2.3-70-g09d2