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 6b0042638dc7a1a96f24af92510fd72571bb027d..8c4fb1f349b47f7707d274c8a01414fd81d8d5b0 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 17eeae1c3dac939076220cdd6ed90b4a2778e3f8..8795af4cb44bd5bd55f26ff219be36d3bbcd7782 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(