Skip to content

Commit

Permalink
lsp: Do not notify all language servers on file save (#17756)
Browse files Browse the repository at this point in the history
This is not an ideal solution to
https://github.com/fasterthanlime/zed-diags-readme, but current status
quo is not great either; we were just going through all of the language
servers and notifying them, whereas we should ideally do it based on a
glob.
/cc @fasterthanlime

Release Notes:

- N/A
  • Loading branch information
osiewicz authored Sep 26, 2024
1 parent 31902a1 commit db92a31
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 8 deletions.
16 changes: 16 additions & 0 deletions crates/project/src/lsp_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2892,11 +2892,27 @@ 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())
Expand Down
55 changes: 47 additions & 8 deletions crates/project/src/project_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,34 @@ 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>()
Expand Down Expand Up @@ -433,6 +461,24 @@ 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>()
Expand Down Expand Up @@ -483,7 +529,7 @@ async fn test_managing_language_servers(cx: &mut gpui::TestAppContext) {
)
);

// Save notifications are reported to all servers.
// Save notifications are reported only to servers that signed up for a given extension.
project
.update(cx, |project, cx| project.save_buffer(toml_buffer, cx))
.await
Expand All @@ -495,13 +541,6 @@ 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(
Expand Down

0 comments on commit db92a31

Please sign in to comment.