Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Revert "lsp: Do not notify all language servers on file save" (#19183)

Browse source 
Reverts zed-industries/zed#17756. According to the existing
implementations of the LSP specification, namely
[Helix](a7651f5bf0/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
This commit is contained in:
Stanislav Alekseev 2024-10-16 13:41:01 +03:00 • committed by GitHub
parent 128619899e
commit eddf70b5c4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 8 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

16
crates/project/src/lsp_store.rs
View file

@ -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())

55
crates/project/src/project_tests.rs
View file

@ -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::request::RegisterCapability>(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::<lsp::notification::DidOpenTextDocument>()
@ -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::request::RegisterCapability>(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::<lsp::notification::DidOpenTextDocument>()
@ -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::<lsp::notification::DidSaveTextDocument>()
.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(