From eddf70b5c429636f8561814ac4b29922c81bd8ae Mon Sep 17 00:00:00 2001 From: Stanislav Alekseev <43210583+WeetHet@users.noreply.github.com> Date: Wed, 16 Oct 2024 13:41:01 +0300 Subject: [PATCH] Revert "lsp: Do not notify all language servers on file save" (#19183) Reverts zed-industries/zed#17756. According to the existing implementations of the LSP specification, namely [Helix](https://github.com/helix-editor/helix/blob/a7651f5bf027ec98645d571ab05a685d97e1b772/helix-view/src/document.rs#L1038) and, if I'm not wrong, [VSCode](https://github.com/microsoft/vscode-languageserver-node/blob/main/client/src/common/textSynchronization.ts#L580), `textDocument/didSave` has nothing to do with the watched files and should be sent to the language servers connected to the buffers even if the files are not watched by those. As the LSP spec doesn't say anything about `didSave` being related to the watched files, and the reference implementation in VSCode seemingly does not filter the notifications according to those, it seems like this is an incorrect interpretation of the specification This also causes issues with language servers. See [Metals issue](https://github.com/scalameta/metals-zed/issues/28#issuecomment-2410393150) for example Closes #18636 Release Notes: - N/A --- crates/project/src/lsp_store.rs | 16 --------- crates/project/src/project_tests.rs | 55 +++++------------------------ 2 files changed, 8 insertions(+), 63 deletions(-) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 6b0042638dc7a..8c4fb1f349b47 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -2900,27 +2900,11 @@ impl LspStore { let file = File::from_dyn(buffer.read(cx).file())?; let worktree_id = file.worktree_id(cx); let abs_path = file.as_local()?.abs_path(cx); - let worktree_path = file.as_local()?.path(); let text_document = lsp::TextDocumentIdentifier { uri: lsp::Url::from_file_path(abs_path).log_err()?, }; - let watched_paths_for_server = &self.as_local()?.language_server_watched_paths; for server in self.language_servers_for_worktree(worktree_id) { - let should_notify = maybe!({ - Some( - watched_paths_for_server - .get(&server.server_id())? - .read(cx) - .worktree_paths - .get(&worktree_id)? - .is_match(worktree_path), - ) - }) - .unwrap_or_default(); - if !should_notify { - continue; - } if let Some(include_text) = include_text(server.as_ref()) { let text = if include_text { Some(buffer.read(cx).text()) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 17eeae1c3dac9..8795af4cb44bd 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -385,34 +385,6 @@ async fn test_managing_language_servers(cx: &mut gpui::TestAppContext) { // A server is started up, and it is notified about Rust files. let mut fake_rust_server = fake_rust_servers.next().await.unwrap(); - fake_rust_server - .request::(lsp::RegistrationParams { - registrations: vec![lsp::Registration { - id: Default::default(), - method: "workspace/didChangeWatchedFiles".to_string(), - register_options: serde_json::to_value( - lsp::DidChangeWatchedFilesRegistrationOptions { - watchers: vec![ - lsp::FileSystemWatcher { - glob_pattern: lsp::GlobPattern::String( - "/the-root/Cargo.toml".to_string(), - ), - kind: None, - }, - lsp::FileSystemWatcher { - glob_pattern: lsp::GlobPattern::String( - "/the-root/*.rs".to_string(), - ), - kind: None, - }, - ], - }, - ) - .ok(), - }], - }) - .await - .unwrap(); assert_eq!( fake_rust_server .receive_notification::() @@ -460,24 +432,6 @@ async fn test_managing_language_servers(cx: &mut gpui::TestAppContext) { // A json language server is started up and is only notified about the json buffer. let mut fake_json_server = fake_json_servers.next().await.unwrap(); - fake_json_server - .request::(lsp::RegistrationParams { - registrations: vec![lsp::Registration { - id: Default::default(), - method: "workspace/didChangeWatchedFiles".to_string(), - register_options: serde_json::to_value( - lsp::DidChangeWatchedFilesRegistrationOptions { - watchers: vec![lsp::FileSystemWatcher { - glob_pattern: lsp::GlobPattern::String("/the-root/*.json".to_string()), - kind: None, - }], - }, - ) - .ok(), - }], - }) - .await - .unwrap(); assert_eq!( fake_json_server .receive_notification::() @@ -528,7 +482,7 @@ async fn test_managing_language_servers(cx: &mut gpui::TestAppContext) { ) ); - // Save notifications are reported only to servers that signed up for a given extension. + // Save notifications are reported to all servers. project .update(cx, |project, cx| project.save_buffer(toml_buffer, cx)) .await @@ -540,6 +494,13 @@ async fn test_managing_language_servers(cx: &mut gpui::TestAppContext) { .text_document, lsp::TextDocumentIdentifier::new(lsp::Url::from_file_path("/the-root/Cargo.toml").unwrap()) ); + assert_eq!( + fake_json_server + .receive_notification::() + .await + .text_document, + lsp::TextDocumentIdentifier::new(lsp::Url::from_file_path("/the-root/Cargo.toml").unwrap()) + ); // Renames are reported only to servers matching the buffer's language. fs.rename(