Merge pull request #1132 from zed-industries/refresh-on-save

Don't refresh the project diagnostics until the user saves
This commit is contained in:
Max Brunsfeld 2022-06-07 14:28:07 -07:00 committed by GitHub
commit 26cd20e38b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 189 additions and 111 deletions

View File

@ -1,7 +1,7 @@
pub mod items;
use anyhow::Result;
use collections::{BTreeSet, HashSet};
use collections::{BTreeMap, HashSet};
use editor::{
diagnostic_block_renderer,
display_map::{BlockDisposition, BlockId, BlockProperties, RenderBlock},
@ -23,7 +23,6 @@ use smallvec::SmallVec;
use std::{
any::{Any, TypeId},
cmp::Ordering,
mem,
ops::Range,
path::PathBuf,
sync::Arc,
@ -52,7 +51,7 @@ struct ProjectDiagnosticsEditor {
summary: DiagnosticSummary,
excerpts: ModelHandle<MultiBuffer>,
path_states: Vec<PathState>,
paths_to_update: BTreeSet<ProjectPath>,
paths_to_update: BTreeMap<ProjectPath, usize>,
}
struct PathState {
@ -114,8 +113,8 @@ impl View for ProjectDiagnosticsEditor {
"summary": project.diagnostic_summary(cx),
}),
"summary": self.summary,
"paths_to_update": self.paths_to_update.iter().map(|path|
path.path.to_string_lossy()
"paths_to_update": self.paths_to_update.iter().map(|(path, server_id)|
(path.path.to_string_lossy(), server_id)
).collect::<Vec<_>>(),
"paths_states": self.path_states.iter().map(|state|
json!({
@ -139,12 +138,16 @@ impl ProjectDiagnosticsEditor {
cx: &mut ViewContext<Self>,
) -> Self {
cx.subscribe(&project_handle, |this, _, event, cx| match event {
project::Event::DiskBasedDiagnosticsFinished => {
this.update_excerpts(cx);
project::Event::DiskBasedDiagnosticsFinished { language_server_id } => {
this.update_excerpts(Some(*language_server_id), cx);
this.update_title(cx);
}
project::Event::DiagnosticsUpdated(path) => {
this.paths_to_update.insert(path.clone());
project::Event::DiagnosticsUpdated {
language_server_id,
path,
} => {
this.paths_to_update
.insert(path.clone(), *language_server_id);
}
_ => {}
})
@ -161,7 +164,10 @@ impl ProjectDiagnosticsEditor {
.detach();
let project = project_handle.read(cx);
let paths_to_update = project.diagnostic_summaries(cx).map(|e| e.0).collect();
let paths_to_update = project
.diagnostic_summaries(cx)
.map(|e| (e.0, e.1.language_server_id))
.collect();
let summary = project.diagnostic_summary(cx);
let mut this = Self {
project: project_handle,
@ -172,7 +178,7 @@ impl ProjectDiagnosticsEditor {
path_states: Default::default(),
paths_to_update,
};
this.update_excerpts(cx);
this.update_excerpts(None, cx);
this
}
@ -212,8 +218,18 @@ impl ProjectDiagnosticsEditor {
.detach()
}
fn update_excerpts(&mut self, cx: &mut ViewContext<Self>) {
let paths = mem::take(&mut self.paths_to_update);
fn update_excerpts(&mut self, language_server_id: Option<usize>, cx: &mut ViewContext<Self>) {
let mut paths = Vec::new();
self.paths_to_update.retain(|path, server_id| {
if language_server_id
.map_or(true, |language_server_id| language_server_id == *server_id)
{
paths.push(path.clone());
false
} else {
true
}
});
let project = self.project.clone();
cx.spawn(|this, mut cx| {
async move {
@ -221,7 +237,7 @@ impl ProjectDiagnosticsEditor {
let buffer = project
.update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx))
.await?;
this.update(&mut cx, |view, cx| view.populate_excerpts(path, buffer, cx))
this.update(&mut cx, |this, cx| this.populate_excerpts(path, buffer, cx))
}
Result::<_, anyhow::Error>::Ok(())
}
@ -838,6 +854,7 @@ mod tests {
project.update(cx, |project, cx| {
project
.update_diagnostic_entries(
0,
PathBuf::from("/test/main.rs"),
None,
vec![
@ -986,9 +1003,10 @@ mod tests {
// Diagnostics are added for another earlier path.
project.update(cx, |project, cx| {
project.disk_based_diagnostics_started(cx);
project.disk_based_diagnostics_started(0, cx);
project
.update_diagnostic_entries(
0,
PathBuf::from("/test/consts.rs"),
None,
vec![DiagnosticEntry {
@ -1005,7 +1023,7 @@ mod tests {
cx,
)
.unwrap();
project.disk_based_diagnostics_finished(cx);
project.disk_based_diagnostics_finished(0, cx);
});
view.next_notification(&cx).await;
@ -1085,9 +1103,10 @@ mod tests {
// Diagnostics are added to the first path
project.update(cx, |project, cx| {
project.disk_based_diagnostics_started(cx);
project.disk_based_diagnostics_started(0, cx);
project
.update_diagnostic_entries(
0,
PathBuf::from("/test/consts.rs"),
None,
vec![
@ -1118,7 +1137,7 @@ mod tests {
cx,
)
.unwrap();
project.disk_based_diagnostics_finished(cx);
project.disk_based_diagnostics_finished(0, cx);
});
view.next_notification(&cx).await;

View File

@ -1,3 +1,4 @@
use collections::HashSet;
use editor::{Editor, GoToNextDiagnostic};
use gpui::{
elements::*, platform::CursorStyle, serde_json, Entity, ModelHandle, MutableAppContext,
@ -12,7 +13,7 @@ pub struct DiagnosticIndicator {
summary: project::DiagnosticSummary,
active_editor: Option<WeakViewHandle<Editor>>,
current_diagnostic: Option<Diagnostic>,
check_in_progress: bool,
in_progress_checks: HashSet<usize>,
_observe_active_editor: Option<Subscription>,
}
@ -23,16 +24,13 @@ pub fn init(cx: &mut MutableAppContext) {
impl DiagnosticIndicator {
pub fn new(project: &ModelHandle<Project>, cx: &mut ViewContext<Self>) -> Self {
cx.subscribe(project, |this, project, event, cx| match event {
project::Event::DiskBasedDiagnosticsUpdated => {
project::Event::DiskBasedDiagnosticsStarted { language_server_id } => {
this.in_progress_checks.insert(*language_server_id);
cx.notify();
}
project::Event::DiskBasedDiagnosticsStarted => {
this.check_in_progress = true;
cx.notify();
}
project::Event::DiskBasedDiagnosticsFinished => {
project::Event::DiskBasedDiagnosticsFinished { language_server_id } => {
this.summary = project.read(cx).diagnostic_summary(cx);
this.check_in_progress = false;
this.in_progress_checks.remove(language_server_id);
cx.notify();
}
_ => {}
@ -40,7 +38,10 @@ impl DiagnosticIndicator {
.detach();
Self {
summary: project.read(cx).diagnostic_summary(cx),
check_in_progress: project.read(cx).is_running_disk_based_diagnostics(),
in_progress_checks: project
.read(cx)
.language_servers_running_disk_based_diagnostics()
.collect(),
active_editor: None,
current_diagnostic: None,
_observe_active_editor: None,
@ -85,7 +86,7 @@ impl View for DiagnosticIndicator {
enum Summary {}
enum Message {}
let in_progress = self.check_in_progress;
let in_progress = !self.in_progress_checks.is_empty();
let mut element = Flex::row().with_child(
MouseEventHandler::new::<Summary, _, _>(0, cx, |state, cx| {
let style = &cx

View File

@ -154,10 +154,16 @@ pub enum Event {
ActiveEntryChanged(Option<ProjectEntryId>),
WorktreeAdded,
WorktreeRemoved(WorktreeId),
DiskBasedDiagnosticsStarted,
DiskBasedDiagnosticsUpdated,
DiskBasedDiagnosticsFinished,
DiagnosticsUpdated(ProjectPath),
DiskBasedDiagnosticsStarted {
language_server_id: usize,
},
DiskBasedDiagnosticsFinished {
language_server_id: usize,
},
DiagnosticsUpdated {
path: ProjectPath,
language_server_id: usize,
},
RemoteIdChanged(Option<u64>),
CollaboratorLeft(PeerId),
ContactRequestedJoin(Arc<User>),
@ -187,6 +193,7 @@ pub struct ProjectPath {
#[derive(Copy, Clone, Debug, Default, PartialEq, Serialize)]
pub struct DiagnosticSummary {
pub language_server_id: usize,
pub error_count: usize,
pub warning_count: usize,
}
@ -220,8 +227,12 @@ pub struct Symbol {
pub struct ProjectTransaction(pub HashMap<ModelHandle<Buffer>, language::Transaction>);
impl DiagnosticSummary {
fn new<'a, T: 'a>(diagnostics: impl IntoIterator<Item = &'a DiagnosticEntry<T>>) -> Self {
fn new<'a, T: 'a>(
language_server_id: usize,
diagnostics: impl IntoIterator<Item = &'a DiagnosticEntry<T>>,
) -> Self {
let mut this = Self {
language_server_id,
error_count: 0,
warning_count: 0,
};
@ -246,6 +257,7 @@ impl DiagnosticSummary {
pub fn to_proto(&self, path: &Path) -> proto::DiagnosticSummary {
proto::DiagnosticSummary {
path: path.to_string_lossy().to_string(),
language_server_id: self.language_server_id as u64,
error_count: self.error_count as u32,
warning_count: self.warning_count as u32,
}
@ -1742,6 +1754,24 @@ impl Project {
)
.log_err();
}
// After saving a buffer, simulate disk-based diagnostics being finished for languages
// that don't support a disk-based progress token.
let (lsp_adapter, language_server) =
self.language_server_for_buffer(buffer.read(cx), cx)?;
if lsp_adapter
.disk_based_diagnostics_progress_token()
.is_none()
{
let server_id = language_server.server_id();
self.disk_based_diagnostics_finished(server_id, cx);
self.broadcast_language_server_update(
server_id,
proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(
proto::LspDiskBasedDiagnosticsUpdated {},
),
);
}
}
_ => {}
}
@ -1827,11 +1857,7 @@ impl Project {
if let Some(this) = this.upgrade(&cx) {
this.update(&mut cx, |this, cx| {
this.on_lsp_diagnostics_published(
server_id,
params,
&adapter,
disk_based_diagnostics_progress_token,
cx,
server_id, params, &adapter, cx,
);
});
}
@ -2061,30 +2087,16 @@ impl Project {
server_id: usize,
mut params: lsp::PublishDiagnosticsParams,
adapter: &Arc<dyn LspAdapter>,
disk_based_diagnostics_progress_token: Option<&str>,
cx: &mut ModelContext<Self>,
) {
adapter.process_diagnostics(&mut params);
if disk_based_diagnostics_progress_token.is_none() {
self.disk_based_diagnostics_started(cx);
self.broadcast_language_server_update(
server_id,
proto::update_language_server::Variant::DiskBasedDiagnosticsUpdating(
proto::LspDiskBasedDiagnosticsUpdating {},
),
);
}
self.update_diagnostics(params, adapter.disk_based_diagnostic_sources(), cx)
.log_err();
if disk_based_diagnostics_progress_token.is_none() {
self.disk_based_diagnostics_finished(cx);
self.broadcast_language_server_update(
server_id,
proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(
proto::LspDiskBasedDiagnosticsUpdated {},
),
);
}
self.update_diagnostics(
server_id,
params,
adapter.disk_based_diagnostic_sources(),
cx,
)
.log_err();
}
fn on_lsp_progress(
@ -2115,7 +2127,7 @@ impl Project {
if Some(token.as_str()) == disk_based_diagnostics_progress_token {
language_server_status.pending_diagnostic_updates += 1;
if language_server_status.pending_diagnostic_updates == 1 {
self.disk_based_diagnostics_started(cx);
self.disk_based_diagnostics_started(server_id, cx);
self.broadcast_language_server_update(
server_id,
proto::update_language_server::Variant::DiskBasedDiagnosticsUpdating(
@ -2161,7 +2173,7 @@ impl Project {
if Some(token.as_str()) == disk_based_diagnostics_progress_token {
language_server_status.pending_diagnostic_updates -= 1;
if language_server_status.pending_diagnostic_updates == 0 {
self.disk_based_diagnostics_finished(cx);
self.disk_based_diagnostics_finished(server_id, cx);
self.broadcast_language_server_update(
server_id,
proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(
@ -2297,6 +2309,7 @@ impl Project {
pub fn update_diagnostics(
&mut self,
language_server_id: usize,
params: lsp::PublishDiagnosticsParams,
disk_based_sources: &[&str],
cx: &mut ModelContext<Self>,
@ -2401,12 +2414,19 @@ impl Project {
}
}
self.update_diagnostic_entries(abs_path, params.version, diagnostics, cx)?;
self.update_diagnostic_entries(
language_server_id,
abs_path,
params.version,
diagnostics,
cx,
)?;
Ok(())
}
pub fn update_diagnostic_entries(
&mut self,
language_server_id: usize,
abs_path: PathBuf,
version: Option<i32>,
diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
@ -2431,10 +2451,18 @@ impl Project {
worktree
.as_local_mut()
.ok_or_else(|| anyhow!("not a local worktree"))?
.update_diagnostics(project_path.path.clone(), diagnostics, cx)
.update_diagnostics(
language_server_id,
project_path.path.clone(),
diagnostics,
cx,
)
})?;
if updated {
cx.emit(Event::DiagnosticsUpdated(project_path));
cx.emit(Event::DiagnosticsUpdated {
language_server_id,
path: project_path,
});
}
Ok(())
}
@ -3970,10 +3998,18 @@ impl Project {
}
}
pub fn is_running_disk_based_diagnostics(&self) -> bool {
pub fn language_servers_running_disk_based_diagnostics<'a>(
&'a self,
) -> impl 'a + Iterator<Item = usize> {
self.language_server_statuses
.values()
.any(|status| status.pending_diagnostic_updates > 0)
.iter()
.filter_map(|(id, status)| {
if status.pending_diagnostic_updates > 0 {
Some(*id)
} else {
None
}
})
}
pub fn diagnostic_summary(&self, cx: &AppContext) -> DiagnosticSummary {
@ -3998,29 +4034,20 @@ impl Project {
})
}
pub fn disk_based_diagnostics_started(&mut self, cx: &mut ModelContext<Self>) {
if self
.language_server_statuses
.values()
.map(|status| status.pending_diagnostic_updates)
.sum::<isize>()
== 1
{
cx.emit(Event::DiskBasedDiagnosticsStarted);
}
pub fn disk_based_diagnostics_started(
&mut self,
language_server_id: usize,
cx: &mut ModelContext<Self>,
) {
cx.emit(Event::DiskBasedDiagnosticsStarted { language_server_id });
}
pub fn disk_based_diagnostics_finished(&mut self, cx: &mut ModelContext<Self>) {
cx.emit(Event::DiskBasedDiagnosticsUpdated);
if self
.language_server_statuses
.values()
.map(|status| status.pending_diagnostic_updates)
.sum::<isize>()
== 0
{
cx.emit(Event::DiskBasedDiagnosticsFinished);
}
pub fn disk_based_diagnostics_finished(
&mut self,
language_server_id: usize,
cx: &mut ModelContext<Self>,
) {
cx.emit(Event::DiskBasedDiagnosticsFinished { language_server_id });
}
pub fn active_entry(&self) -> Option<ProjectEntryId> {
@ -4351,7 +4378,10 @@ impl Project {
.unwrap()
.update_diagnostic_summary(project_path.path.clone(), &summary);
});
cx.emit(Event::DiagnosticsUpdated(project_path));
cx.emit(Event::DiagnosticsUpdated {
language_server_id: summary.language_server_id as usize,
path: project_path,
});
}
}
Ok(())
@ -4420,11 +4450,13 @@ impl Project {
}
proto::update_language_server::Variant::DiskBasedDiagnosticsUpdating(_) => {
this.update(&mut cx, |this, cx| {
this.disk_based_diagnostics_started(cx);
this.disk_based_diagnostics_started(language_server_id, cx);
})
}
proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(_) => {
this.update(&mut cx, |this, cx| this.disk_based_diagnostics_finished(cx));
this.update(&mut cx, |this, cx| {
this.disk_based_diagnostics_finished(language_server_id, cx)
});
}
}
@ -6064,6 +6096,7 @@ mod tests {
project.update(cx, |project, cx| {
project
.update_diagnostics(
0,
lsp::PublishDiagnosticsParams {
uri: Url::from_file_path("/dir/a.rs").unwrap(),
version: None,
@ -6083,6 +6116,7 @@ mod tests {
.unwrap();
project
.update_diagnostics(
0,
lsp::PublishDiagnosticsParams {
uri: Url::from_file_path("/dir/b.rs").unwrap(),
version: None,
@ -6178,7 +6212,9 @@ mod tests {
fake_server.start_progress(progress_token).await;
assert_eq!(
events.next().await.unwrap(),
Event::DiskBasedDiagnosticsStarted
Event::DiskBasedDiagnosticsStarted {
language_server_id: 0,
}
);
fake_server.start_progress(progress_token).await;
@ -6199,18 +6235,19 @@ mod tests {
);
assert_eq!(
events.next().await.unwrap(),
Event::DiagnosticsUpdated((worktree_id, Path::new("a.rs")).into())
Event::DiagnosticsUpdated {
language_server_id: 0,
path: (worktree_id, Path::new("a.rs")).into()
}
);
fake_server.end_progress(progress_token).await;
fake_server.end_progress(progress_token).await;
assert_eq!(
events.next().await.unwrap(),
Event::DiskBasedDiagnosticsUpdated
);
assert_eq!(
events.next().await.unwrap(),
Event::DiskBasedDiagnosticsFinished
Event::DiskBasedDiagnosticsFinished {
language_server_id: 0
}
);
let buffer = project
@ -6248,7 +6285,10 @@ mod tests {
);
assert_eq!(
events.next().await.unwrap(),
Event::DiagnosticsUpdated((worktree_id, Path::new("a.rs")).into())
Event::DiagnosticsUpdated {
language_server_id: 0,
path: (worktree_id, Path::new("a.rs")).into()
}
);
fake_server.notify::<lsp::notification::PublishDiagnostics>(
@ -6306,22 +6346,35 @@ mod tests {
fake_server.start_progress(progress_token).await;
assert_eq!(
events.next().await.unwrap(),
Event::DiskBasedDiagnosticsStarted
Event::DiskBasedDiagnosticsStarted {
language_server_id: 1
}
);
project.read_with(cx, |project, _| {
assert_eq!(
project
.language_servers_running_disk_based_diagnostics()
.collect::<Vec<_>>(),
[1]
);
});
// All diagnostics are considered done, despite the old server's diagnostic
// task never completing.
fake_server.end_progress(progress_token).await;
assert_eq!(
events.next().await.unwrap(),
Event::DiskBasedDiagnosticsUpdated
);
assert_eq!(
events.next().await.unwrap(),
Event::DiskBasedDiagnosticsFinished
Event::DiskBasedDiagnosticsFinished {
language_server_id: 1
}
);
project.read_with(cx, |project, _| {
assert!(!project.is_running_disk_based_diagnostics());
assert_eq!(
project
.language_servers_running_disk_based_diagnostics()
.collect::<Vec<_>>(),
[0; 0]
);
});
}
@ -8002,7 +8055,7 @@ mod tests {
};
project
.update(cx, |p, cx| p.update_diagnostics(message, &[], cx))
.update(cx, |p, cx| p.update_diagnostics(0, message, &[], cx))
.unwrap();
let buffer = buffer.read_with(cx, |buffer, _| buffer.snapshot());

View File

@ -210,6 +210,7 @@ impl Worktree {
(
PathKey(PathBuf::from(summary.path).into()),
DiagnosticSummary {
language_server_id: summary.language_server_id as usize,
error_count: summary.error_count as usize,
warning_count: summary.warning_count as usize,
},
@ -528,6 +529,7 @@ impl LocalWorktree {
pub fn update_diagnostics(
&mut self,
language_server_id: usize,
worktree_path: Arc<Path>,
diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
_: &mut ModelContext<Worktree>,
@ -537,7 +539,7 @@ impl LocalWorktree {
.diagnostic_summaries
.remove(&PathKey(worktree_path.clone()))
.unwrap_or_default();
let new_summary = DiagnosticSummary::new(&diagnostics);
let new_summary = DiagnosticSummary::new(language_server_id, &diagnostics);
if !new_summary.is_empty() {
self.diagnostic_summaries
.insert(PathKey(worktree_path.clone()), new_summary);
@ -553,6 +555,7 @@ impl LocalWorktree {
worktree_id: self.id().to_proto(),
summary: Some(proto::DiagnosticSummary {
path: worktree_path.to_string_lossy().to_string(),
language_server_id: language_server_id as u64,
error_count: new_summary.error_count as u32,
warning_count: new_summary.warning_count as u32,
}),
@ -1065,6 +1068,7 @@ impl RemoteWorktree {
summary: &proto::DiagnosticSummary,
) {
let summary = DiagnosticSummary {
language_server_id: summary.language_server_id as usize,
error_count: summary.error_count as usize,
warning_count: summary.warning_count as usize,
};

View File

@ -516,8 +516,9 @@ message UpdateDiagnosticSummary {
message DiagnosticSummary {
string path = 1;
uint32 error_count = 2;
uint32 warning_count = 3;
uint64 language_server_id = 2;
uint32 error_count = 3;
uint32 warning_count = 4;
}
message UpdateLanguageServer {

View File

@ -6,4 +6,4 @@ pub use conn::Connection;
pub use peer::*;
mod macros;
pub const PROTOCOL_VERSION: u32 = 22;
pub const PROTOCOL_VERSION: u32 = 23;