From 740686b8830a548b0f2d26b11ddaaefa2484346c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 7 Aug 2025 17:45:41 +0300 Subject: [PATCH 1/7] Batch diagnostics updates (#35794) Diagnostics updates were programmed in Zed based off the r-a LSP push diagnostics, with all related updates happening per file. https://github.com/zed-industries/zed/pull/19230 and especially https://github.com/zed-industries/zed/pull/32269 brought in pull diagnostics that could produce results for thousands files simultaneously. It was noted and addressed on the local side in https://github.com/zed-industries/zed/pull/34022 but the remote side was still not adjusted properly. This PR * removes redundant diagnostics pull updates on remote clients, as buffer diagnostics are updated via buffer sync operations separately * batches all diagnostics-related updates and proto messages, so multiple diagnostic summaries (per file) could be sent at once, specifically, 1 (potentially large) diagnostics summary update instead of N*10^3 small ones. Buffer updates are still sent per buffer and not updated, as happening separately and not offending the collab traffic that much. Release Notes: - Improved diagnostics performance in the collaborative mode --- crates/collab/src/rpc.rs | 36 +- crates/diagnostics/src/diagnostics.rs | 8 +- crates/project/src/lsp_store.rs | 729 +++++++++++++-------- crates/project/src/lsp_store/clangd_ext.rs | 18 +- crates/project/src/project.rs | 50 +- crates/project/src/project_tests.rs | 8 +- crates/proto/proto/lsp.proto | 1 + 7 files changed, 500 insertions(+), 350 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 078632d397..b3603d2619 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1630,15 +1630,15 @@ fn notify_rejoined_projects( } // Stream this worktree's diagnostics. - for summary in worktree.diagnostic_summaries { - session.peer.send( - session.connection_id, - proto::UpdateDiagnosticSummary { - project_id: project.id.to_proto(), - worktree_id: worktree.id, - summary: Some(summary), - }, - )?; + let mut worktree_diagnostics = worktree.diagnostic_summaries.into_iter(); + if let Some(summary) = worktree_diagnostics.next() { + let message = proto::UpdateDiagnosticSummary { + project_id: project.id.to_proto(), + worktree_id: worktree.id, + summary: Some(summary), + more_summaries: worktree_diagnostics.collect(), + }; + session.peer.send(session.connection_id, message)?; } for settings_file in worktree.settings_files { @@ -2060,15 +2060,15 @@ async fn join_project( } // Stream this worktree's diagnostics. - for summary in worktree.diagnostic_summaries { - session.peer.send( - session.connection_id, - proto::UpdateDiagnosticSummary { - project_id: project_id.to_proto(), - worktree_id: worktree.id, - summary: Some(summary), - }, - )?; + let mut worktree_diagnostics = worktree.diagnostic_summaries.into_iter(); + if let Some(summary) = worktree_diagnostics.next() { + let message = proto::UpdateDiagnosticSummary { + project_id: project.id.to_proto(), + worktree_id: worktree.id, + summary: Some(summary), + more_summaries: worktree_diagnostics.collect(), + }; + session.peer.send(session.connection_id, message)?; } for settings_file in worktree.settings_files { diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index ba64ba0eed..e7660920da 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -177,9 +177,9 @@ impl ProjectDiagnosticsEditor { } project::Event::DiagnosticsUpdated { language_server_id, - path, + paths, } => { - this.paths_to_update.insert(path.clone()); + this.paths_to_update.extend(paths.clone()); let project = project.clone(); this.diagnostic_summary_update = cx.spawn(async move |this, cx| { cx.background_executor() @@ -193,9 +193,9 @@ impl ProjectDiagnosticsEditor { cx.emit(EditorEvent::TitleChanged); if this.editor.focus_handle(cx).contains_focused(window, cx) || this.focus_handle.contains_focused(window, cx) { - log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. recording change"); + log::debug!("diagnostics updated for server {language_server_id}, paths {paths:?}. recording change"); } else { - log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. updating excerpts"); + log::debug!("diagnostics updated for server {language_server_id}, paths {paths:?}. updating excerpts"); this.update_stale_excerpts(window, cx); } } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 4489f9f043..b88cf42ff5 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -140,6 +140,20 @@ impl FormatTrigger { } } +#[derive(Debug)] +pub struct DocumentDiagnosticsUpdate<'a, D> { + pub diagnostics: D, + pub result_id: Option, + pub server_id: LanguageServerId, + pub disk_based_sources: Cow<'a, [String]>, +} + +pub struct DocumentDiagnostics { + diagnostics: Vec>>, + document_abs_path: PathBuf, + version: Option, +} + pub struct LocalLspStore { weak: WeakEntity, worktree_store: Entity, @@ -503,12 +517,16 @@ impl LocalLspStore { adapter.process_diagnostics(&mut params, server_id, buffer); } - this.merge_diagnostics( - server_id, - params, - None, + this.merge_lsp_diagnostics( DiagnosticSourceKind::Pushed, - &adapter.disk_based_diagnostic_sources, + vec![DocumentDiagnosticsUpdate { + server_id, + diagnostics: params, + result_id: None, + disk_based_sources: Cow::Borrowed( + &adapter.disk_based_diagnostic_sources, + ), + }], |_, diagnostic, cx| match diagnostic.source_kind { DiagnosticSourceKind::Other | DiagnosticSourceKind::Pushed => { adapter.retain_old_diagnostic(diagnostic, cx) @@ -3610,8 +3628,8 @@ pub enum LspStoreEvent { RefreshInlayHints, RefreshCodeLens, DiagnosticsUpdated { - language_server_id: LanguageServerId, - path: ProjectPath, + server_id: LanguageServerId, + paths: Vec, }, DiskBasedDiagnosticsStarted { language_server_id: LanguageServerId, @@ -4440,17 +4458,24 @@ impl LspStore { pub(crate) fn send_diagnostic_summaries(&self, worktree: &mut Worktree) { if let Some((client, downstream_project_id)) = self.downstream_client.clone() { - if let Some(summaries) = self.diagnostic_summaries.get(&worktree.id()) { - for (path, summaries) in summaries { - for (&server_id, summary) in summaries { - client - .send(proto::UpdateDiagnosticSummary { - project_id: downstream_project_id, - worktree_id: worktree.id().to_proto(), - summary: Some(summary.to_proto(server_id, path)), - }) - .log_err(); - } + if let Some(diangostic_summaries) = self.diagnostic_summaries.get(&worktree.id()) { + let mut summaries = + diangostic_summaries + .into_iter() + .flat_map(|(path, summaries)| { + summaries + .into_iter() + .map(|(server_id, summary)| summary.to_proto(*server_id, path)) + }); + if let Some(summary) = summaries.next() { + client + .send(proto::UpdateDiagnosticSummary { + project_id: downstream_project_id, + worktree_id: worktree.id().to_proto(), + summary: Some(summary), + more_summaries: summaries.collect(), + }) + .log_err(); } } } @@ -6564,7 +6589,7 @@ impl LspStore { &mut self, buffer: Entity, cx: &mut Context, - ) -> Task>> { + ) -> Task>>> { let buffer_id = buffer.read(cx).remote_id(); if let Some((client, upstream_project_id)) = self.upstream_client() { @@ -6575,7 +6600,7 @@ impl LspStore { }, cx, ) { - return Task::ready(Ok(Vec::new())); + return Task::ready(Ok(None)); } let request_task = client.request(proto::MultiLspQuery { buffer_id: buffer_id.to_proto(), @@ -6593,7 +6618,7 @@ impl LspStore { )), }); cx.background_spawn(async move { - Ok(request_task + let _proto_responses = request_task .await? .responses .into_iter() @@ -6606,8 +6631,11 @@ impl LspStore { None } }) - .flat_map(GetDocumentDiagnostics::diagnostics_from_proto) - .collect()) + .collect::>(); + // Proto requests cause the diagnostics to be pulled from language server(s) on the local side + // and then, buffer state updated with the diagnostics received, which will be later propagated to the client. + // Do not attempt to further process the dummy responses here. + Ok(None) }) } else { let server_ids = buffer.update(cx, |buffer, cx| { @@ -6635,7 +6663,7 @@ impl LspStore { for diagnostics in join_all(pull_diagnostics).await { responses.extend(diagnostics?); } - Ok(responses) + Ok(Some(responses)) }) } } @@ -6701,75 +6729,93 @@ impl LspStore { buffer: Entity, cx: &mut Context, ) -> Task> { - let buffer_id = buffer.read(cx).remote_id(); let diagnostics = self.pull_diagnostics(buffer, cx); cx.spawn(async move |lsp_store, cx| { - let diagnostics = diagnostics.await.context("pulling diagnostics")?; + let Some(diagnostics) = diagnostics.await.context("pulling diagnostics")? else { + return Ok(()); + }; lsp_store.update(cx, |lsp_store, cx| { if lsp_store.as_local().is_none() { return; } - for diagnostics_set in diagnostics { - let LspPullDiagnostics::Response { - server_id, - uri, - diagnostics, - } = diagnostics_set - else { - continue; - }; - - let adapter = lsp_store.language_server_adapter_for_id(server_id); - let disk_based_sources = adapter - .as_ref() - .map(|adapter| adapter.disk_based_diagnostic_sources.as_slice()) - .unwrap_or(&[]); - match diagnostics { - PulledDiagnostics::Unchanged { result_id } => { - lsp_store - .merge_diagnostics( - server_id, - lsp::PublishDiagnosticsParams { - uri: uri.clone(), - diagnostics: Vec::new(), - version: None, - }, - Some(result_id), - DiagnosticSourceKind::Pulled, - disk_based_sources, - |_, _, _| true, - cx, - ) - .log_err(); - } - PulledDiagnostics::Changed { + let mut unchanged_buffers = HashSet::default(); + let mut changed_buffers = HashSet::default(); + let server_diagnostics_updates = diagnostics + .into_iter() + .filter_map(|diagnostics_set| match diagnostics_set { + LspPullDiagnostics::Response { + server_id, + uri, diagnostics, - result_id, - } => { - lsp_store - .merge_diagnostics( + } => Some((server_id, uri, diagnostics)), + LspPullDiagnostics::Default => None, + }) + .fold( + HashMap::default(), + |mut acc, (server_id, uri, diagnostics)| { + let (result_id, diagnostics) = match diagnostics { + PulledDiagnostics::Unchanged { result_id } => { + unchanged_buffers.insert(uri.clone()); + (Some(result_id), Vec::new()) + } + PulledDiagnostics::Changed { + result_id, + diagnostics, + } => { + changed_buffers.insert(uri.clone()); + (result_id, diagnostics) + } + }; + let disk_based_sources = Cow::Owned( + lsp_store + .language_server_adapter_for_id(server_id) + .as_ref() + .map(|adapter| adapter.disk_based_diagnostic_sources.as_slice()) + .unwrap_or(&[]) + .to_vec(), + ); + acc.entry(server_id).or_insert_with(Vec::new).push( + DocumentDiagnosticsUpdate { server_id, - lsp::PublishDiagnosticsParams { - uri: uri.clone(), + diagnostics: lsp::PublishDiagnosticsParams { + uri, diagnostics, version: None, }, result_id, - DiagnosticSourceKind::Pulled, disk_based_sources, - |buffer, old_diagnostic, _| match old_diagnostic.source_kind { - DiagnosticSourceKind::Pulled => { - buffer.remote_id() != buffer_id - } - DiagnosticSourceKind::Other - | DiagnosticSourceKind::Pushed => true, - }, - cx, - ) - .log_err(); - } - } + }, + ); + acc + }, + ); + + for diagnostic_updates in server_diagnostics_updates.into_values() { + lsp_store + .merge_lsp_diagnostics( + DiagnosticSourceKind::Pulled, + diagnostic_updates, + |buffer, old_diagnostic, cx| { + File::from_dyn(buffer.file()) + .and_then(|file| { + let abs_path = file.as_local()?.abs_path(cx); + lsp::Url::from_file_path(abs_path).ok() + }) + .is_none_or(|buffer_uri| { + unchanged_buffers.contains(&buffer_uri) + || match old_diagnostic.source_kind { + DiagnosticSourceKind::Pulled => { + !changed_buffers.contains(&buffer_uri) + } + DiagnosticSourceKind::Other + | DiagnosticSourceKind::Pushed => true, + } + }) + }, + cx, + ) + .log_err(); } }) }) @@ -7791,88 +7837,135 @@ impl LspStore { cx: &mut Context, ) -> anyhow::Result<()> { self.merge_diagnostic_entries( - server_id, - abs_path, - result_id, - version, - diagnostics, + vec![DocumentDiagnosticsUpdate { + diagnostics: DocumentDiagnostics { + diagnostics, + document_abs_path: abs_path, + version, + }, + result_id, + server_id, + disk_based_sources: Cow::Borrowed(&[]), + }], |_, _, _| false, cx, )?; Ok(()) } - pub fn merge_diagnostic_entries( + pub fn merge_diagnostic_entries<'a>( &mut self, - server_id: LanguageServerId, - abs_path: PathBuf, - result_id: Option, - version: Option, - mut diagnostics: Vec>>, - filter: impl Fn(&Buffer, &Diagnostic, &App) -> bool + Clone, + diagnostic_updates: Vec>, + merge: impl Fn(&Buffer, &Diagnostic, &App) -> bool + Clone, cx: &mut Context, ) -> anyhow::Result<()> { - let Some((worktree, relative_path)) = - self.worktree_store.read(cx).find_worktree(&abs_path, cx) - else { - log::warn!("skipping diagnostics update, no worktree found for path {abs_path:?}"); - return Ok(()); - }; + let mut diagnostics_summary = None::; + let mut updated_diagnostics_paths = HashMap::default(); + for mut update in diagnostic_updates { + let abs_path = &update.diagnostics.document_abs_path; + let server_id = update.server_id; + let Some((worktree, relative_path)) = + self.worktree_store.read(cx).find_worktree(abs_path, cx) + else { + log::warn!("skipping diagnostics update, no worktree found for path {abs_path:?}"); + return Ok(()); + }; - let project_path = ProjectPath { - worktree_id: worktree.read(cx).id(), - path: relative_path.into(), - }; + let worktree_id = worktree.read(cx).id(); + let project_path = ProjectPath { + worktree_id, + path: relative_path.into(), + }; - if let Some(buffer_handle) = self.buffer_store.read(cx).get_by_path(&project_path) { - let snapshot = buffer_handle.read(cx).snapshot(); - let buffer = buffer_handle.read(cx); - let reused_diagnostics = buffer - .get_diagnostics(server_id) - .into_iter() - .flat_map(|diag| { - diag.iter() - .filter(|v| filter(buffer, &v.diagnostic, cx)) - .map(|v| { - let start = Unclipped(v.range.start.to_point_utf16(&snapshot)); - let end = Unclipped(v.range.end.to_point_utf16(&snapshot)); - DiagnosticEntry { - range: start..end, - diagnostic: v.diagnostic.clone(), - } - }) - }) - .collect::>(); + if let Some(buffer_handle) = self.buffer_store.read(cx).get_by_path(&project_path) { + let snapshot = buffer_handle.read(cx).snapshot(); + let buffer = buffer_handle.read(cx); + let reused_diagnostics = buffer + .get_diagnostics(server_id) + .into_iter() + .flat_map(|diag| { + diag.iter() + .filter(|v| merge(buffer, &v.diagnostic, cx)) + .map(|v| { + let start = Unclipped(v.range.start.to_point_utf16(&snapshot)); + let end = Unclipped(v.range.end.to_point_utf16(&snapshot)); + DiagnosticEntry { + range: start..end, + diagnostic: v.diagnostic.clone(), + } + }) + }) + .collect::>(); - self.as_local_mut() - .context("cannot merge diagnostics on a remote LspStore")? - .update_buffer_diagnostics( - &buffer_handle, + self.as_local_mut() + .context("cannot merge diagnostics on a remote LspStore")? + .update_buffer_diagnostics( + &buffer_handle, + server_id, + update.result_id, + update.diagnostics.version, + update.diagnostics.diagnostics.clone(), + reused_diagnostics.clone(), + cx, + )?; + + update.diagnostics.diagnostics.extend(reused_diagnostics); + } + + let updated = worktree.update(cx, |worktree, cx| { + self.update_worktree_diagnostics( + worktree.id(), server_id, - result_id, - version, - diagnostics.clone(), - reused_diagnostics.clone(), + project_path.path.clone(), + update.diagnostics.diagnostics, cx, - )?; - - diagnostics.extend(reused_diagnostics); + ) + })?; + match updated { + ControlFlow::Continue(new_summary) => { + if let Some((project_id, new_summary)) = new_summary { + match &mut diagnostics_summary { + Some(diagnostics_summary) => { + diagnostics_summary + .more_summaries + .push(proto::DiagnosticSummary { + path: project_path.path.as_ref().to_proto(), + language_server_id: server_id.0 as u64, + error_count: new_summary.error_count, + warning_count: new_summary.warning_count, + }) + } + None => { + diagnostics_summary = Some(proto::UpdateDiagnosticSummary { + project_id: project_id, + worktree_id: worktree_id.to_proto(), + summary: Some(proto::DiagnosticSummary { + path: project_path.path.as_ref().to_proto(), + language_server_id: server_id.0 as u64, + error_count: new_summary.error_count, + warning_count: new_summary.warning_count, + }), + more_summaries: Vec::new(), + }) + } + } + } + updated_diagnostics_paths + .entry(server_id) + .or_insert_with(Vec::new) + .push(project_path); + } + ControlFlow::Break(()) => {} + } } - let updated = worktree.update(cx, |worktree, cx| { - self.update_worktree_diagnostics( - worktree.id(), - server_id, - project_path.path.clone(), - diagnostics, - cx, - ) - })?; - if updated { - cx.emit(LspStoreEvent::DiagnosticsUpdated { - language_server_id: server_id, - path: project_path, - }) + if let Some((diagnostics_summary, (downstream_client, _))) = + diagnostics_summary.zip(self.downstream_client.as_ref()) + { + downstream_client.send(diagnostics_summary).log_err(); + } + for (server_id, paths) in updated_diagnostics_paths { + cx.emit(LspStoreEvent::DiagnosticsUpdated { server_id, paths }); } Ok(()) } @@ -7881,10 +7974,10 @@ impl LspStore { &mut self, worktree_id: WorktreeId, server_id: LanguageServerId, - worktree_path: Arc, + path_in_worktree: Arc, diagnostics: Vec>>, _: &mut Context, - ) -> Result { + ) -> Result>> { let local = match &mut self.mode { LspStoreMode::Local(local_lsp_store) => local_lsp_store, _ => anyhow::bail!("update_worktree_diagnostics called on remote"), @@ -7892,7 +7985,9 @@ impl LspStore { let summaries_for_tree = self.diagnostic_summaries.entry(worktree_id).or_default(); let diagnostics_for_tree = local.diagnostics.entry(worktree_id).or_default(); - let summaries_by_server_id = summaries_for_tree.entry(worktree_path.clone()).or_default(); + let summaries_by_server_id = summaries_for_tree + .entry(path_in_worktree.clone()) + .or_default(); let old_summary = summaries_by_server_id .remove(&server_id) @@ -7900,18 +7995,19 @@ impl LspStore { let new_summary = DiagnosticSummary::new(&diagnostics); if new_summary.is_empty() { - if let Some(diagnostics_by_server_id) = diagnostics_for_tree.get_mut(&worktree_path) { + if let Some(diagnostics_by_server_id) = diagnostics_for_tree.get_mut(&path_in_worktree) + { if let Ok(ix) = diagnostics_by_server_id.binary_search_by_key(&server_id, |e| e.0) { diagnostics_by_server_id.remove(ix); } if diagnostics_by_server_id.is_empty() { - diagnostics_for_tree.remove(&worktree_path); + diagnostics_for_tree.remove(&path_in_worktree); } } } else { summaries_by_server_id.insert(server_id, new_summary); let diagnostics_by_server_id = diagnostics_for_tree - .entry(worktree_path.clone()) + .entry(path_in_worktree.clone()) .or_default(); match diagnostics_by_server_id.binary_search_by_key(&server_id, |e| e.0) { Ok(ix) => { @@ -7924,23 +8020,22 @@ impl LspStore { } if !old_summary.is_empty() || !new_summary.is_empty() { - if let Some((downstream_client, project_id)) = &self.downstream_client { - downstream_client - .send(proto::UpdateDiagnosticSummary { - project_id: *project_id, - worktree_id: worktree_id.to_proto(), - summary: Some(proto::DiagnosticSummary { - path: worktree_path.to_proto(), - language_server_id: server_id.0 as u64, - error_count: new_summary.error_count as u32, - warning_count: new_summary.warning_count as u32, - }), - }) - .log_err(); + if let Some((_, project_id)) = &self.downstream_client { + Ok(ControlFlow::Continue(Some(( + *project_id, + proto::DiagnosticSummary { + path: path_in_worktree.to_proto(), + language_server_id: server_id.0 as u64, + error_count: new_summary.error_count as u32, + warning_count: new_summary.warning_count as u32, + }, + )))) + } else { + Ok(ControlFlow::Continue(None)) } + } else { + Ok(ControlFlow::Break(())) } - - Ok(!old_summary.is_empty() || !new_summary.is_empty()) } pub fn open_buffer_for_symbol( @@ -8793,23 +8888,30 @@ impl LspStore { envelope: TypedEnvelope, mut cx: AsyncApp, ) -> Result<()> { - this.update(&mut cx, |this, cx| { + this.update(&mut cx, |lsp_store, cx| { let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - if let Some(message) = envelope.payload.summary { + let mut updated_diagnostics_paths = HashMap::default(); + let mut diagnostics_summary = None::; + for message_summary in envelope + .payload + .summary + .into_iter() + .chain(envelope.payload.more_summaries) + { let project_path = ProjectPath { worktree_id, - path: Arc::::from_proto(message.path), + path: Arc::::from_proto(message_summary.path), }; let path = project_path.path.clone(); - let server_id = LanguageServerId(message.language_server_id as usize); + let server_id = LanguageServerId(message_summary.language_server_id as usize); let summary = DiagnosticSummary { - error_count: message.error_count as usize, - warning_count: message.warning_count as usize, + error_count: message_summary.error_count as usize, + warning_count: message_summary.warning_count as usize, }; if summary.is_empty() { if let Some(worktree_summaries) = - this.diagnostic_summaries.get_mut(&worktree_id) + lsp_store.diagnostic_summaries.get_mut(&worktree_id) { if let Some(summaries) = worktree_summaries.get_mut(&path) { summaries.remove(&server_id); @@ -8819,31 +8921,55 @@ impl LspStore { } } } else { - this.diagnostic_summaries + lsp_store + .diagnostic_summaries .entry(worktree_id) .or_default() .entry(path) .or_default() .insert(server_id, summary); } - if let Some((downstream_client, project_id)) = &this.downstream_client { - downstream_client - .send(proto::UpdateDiagnosticSummary { - project_id: *project_id, - worktree_id: worktree_id.to_proto(), - summary: Some(proto::DiagnosticSummary { - path: project_path.path.as_ref().to_proto(), - language_server_id: server_id.0 as u64, - error_count: summary.error_count as u32, - warning_count: summary.warning_count as u32, - }), - }) - .log_err(); + + if let Some((_, project_id)) = &lsp_store.downstream_client { + match &mut diagnostics_summary { + Some(diagnostics_summary) => { + diagnostics_summary + .more_summaries + .push(proto::DiagnosticSummary { + path: project_path.path.as_ref().to_proto(), + language_server_id: server_id.0 as u64, + error_count: summary.error_count as u32, + warning_count: summary.warning_count as u32, + }) + } + None => { + diagnostics_summary = Some(proto::UpdateDiagnosticSummary { + project_id: *project_id, + worktree_id: worktree_id.to_proto(), + summary: Some(proto::DiagnosticSummary { + path: project_path.path.as_ref().to_proto(), + language_server_id: server_id.0 as u64, + error_count: summary.error_count as u32, + warning_count: summary.warning_count as u32, + }), + more_summaries: Vec::new(), + }) + } + } } - cx.emit(LspStoreEvent::DiagnosticsUpdated { - language_server_id: LanguageServerId(message.language_server_id as usize), - path: project_path, - }); + updated_diagnostics_paths + .entry(server_id) + .or_insert_with(Vec::new) + .push(project_path); + } + + if let Some((diagnostics_summary, (downstream_client, _))) = + diagnostics_summary.zip(lsp_store.downstream_client.as_ref()) + { + downstream_client.send(diagnostics_summary).log_err(); + } + for (server_id, paths) in updated_diagnostics_paths { + cx.emit(LspStoreEvent::DiagnosticsUpdated { server_id, paths }); } Ok(()) })? @@ -10361,6 +10487,7 @@ impl LspStore { error_count: 0, warning_count: 0, }), + more_summaries: Vec::new(), }) .log_err(); } @@ -10649,52 +10776,80 @@ impl LspStore { ) } + #[cfg(any(test, feature = "test-support"))] pub fn update_diagnostics( &mut self, - language_server_id: LanguageServerId, - params: lsp::PublishDiagnosticsParams, + server_id: LanguageServerId, + diagnostics: lsp::PublishDiagnosticsParams, result_id: Option, source_kind: DiagnosticSourceKind, disk_based_sources: &[String], cx: &mut Context, ) -> Result<()> { - self.merge_diagnostics( - language_server_id, - params, - result_id, + self.merge_lsp_diagnostics( source_kind, - disk_based_sources, + vec![DocumentDiagnosticsUpdate { + diagnostics, + result_id, + server_id, + disk_based_sources: Cow::Borrowed(disk_based_sources), + }], |_, _, _| false, cx, ) } - pub fn merge_diagnostics( + pub fn merge_lsp_diagnostics( &mut self, - language_server_id: LanguageServerId, - mut params: lsp::PublishDiagnosticsParams, - result_id: Option, source_kind: DiagnosticSourceKind, - disk_based_sources: &[String], - filter: impl Fn(&Buffer, &Diagnostic, &App) -> bool + Clone, + lsp_diagnostics: Vec>, + merge: impl Fn(&Buffer, &Diagnostic, &App) -> bool + Clone, cx: &mut Context, ) -> Result<()> { anyhow::ensure!(self.mode.is_local(), "called update_diagnostics on remote"); - let abs_path = params - .uri - .to_file_path() - .map_err(|()| anyhow!("URI is not a file"))?; + let updates = lsp_diagnostics + .into_iter() + .filter_map(|update| { + let abs_path = update.diagnostics.uri.to_file_path().ok()?; + Some(DocumentDiagnosticsUpdate { + diagnostics: self.lsp_to_document_diagnostics( + abs_path, + source_kind, + update.server_id, + update.diagnostics, + &update.disk_based_sources, + ), + result_id: update.result_id, + server_id: update.server_id, + disk_based_sources: update.disk_based_sources, + }) + }) + .collect(); + self.merge_diagnostic_entries(updates, merge, cx)?; + Ok(()) + } + + fn lsp_to_document_diagnostics( + &mut self, + document_abs_path: PathBuf, + source_kind: DiagnosticSourceKind, + server_id: LanguageServerId, + mut lsp_diagnostics: lsp::PublishDiagnosticsParams, + disk_based_sources: &[String], + ) -> DocumentDiagnostics { let mut diagnostics = Vec::default(); let mut primary_diagnostic_group_ids = HashMap::default(); let mut sources_by_group_id = HashMap::default(); let mut supporting_diagnostics = HashMap::default(); - let adapter = self.language_server_adapter_for_id(language_server_id); + let adapter = self.language_server_adapter_for_id(server_id); // Ensure that primary diagnostics are always the most severe - params.diagnostics.sort_by_key(|item| item.severity); + lsp_diagnostics + .diagnostics + .sort_by_key(|item| item.severity); - for diagnostic in ¶ms.diagnostics { + for diagnostic in &lsp_diagnostics.diagnostics { let source = diagnostic.source.as_ref(); let range = range_from_lsp(diagnostic.range); let is_supporting = diagnostic @@ -10716,7 +10871,7 @@ impl LspStore { .map_or(false, |tags| tags.contains(&DiagnosticTag::UNNECESSARY)); let underline = self - .language_server_adapter_for_id(language_server_id) + .language_server_adapter_for_id(server_id) .map_or(true, |adapter| adapter.underline_diagnostic(diagnostic)); if is_supporting { @@ -10758,7 +10913,7 @@ impl LspStore { }); if let Some(infos) = &diagnostic.related_information { for info in infos { - if info.location.uri == params.uri && !info.message.is_empty() { + if info.location.uri == lsp_diagnostics.uri && !info.message.is_empty() { let range = range_from_lsp(info.location.range); diagnostics.push(DiagnosticEntry { range, @@ -10806,16 +10961,11 @@ impl LspStore { } } - self.merge_diagnostic_entries( - language_server_id, - abs_path, - result_id, - params.version, + DocumentDiagnostics { diagnostics, - filter, - cx, - )?; - Ok(()) + document_abs_path, + version: lsp_diagnostics.version, + } } fn insert_newly_running_language_server( @@ -11571,67 +11721,84 @@ impl LspStore { ) { let workspace_diagnostics = GetDocumentDiagnostics::deserialize_workspace_diagnostics_report(report, server_id); - for workspace_diagnostics in workspace_diagnostics { - let LspPullDiagnostics::Response { - server_id, - uri, - diagnostics, - } = workspace_diagnostics.diagnostics - else { - continue; - }; - - let adapter = self.language_server_adapter_for_id(server_id); - let disk_based_sources = adapter - .as_ref() - .map(|adapter| adapter.disk_based_diagnostic_sources.as_slice()) - .unwrap_or(&[]); - - match diagnostics { - PulledDiagnostics::Unchanged { result_id } => { - self.merge_diagnostics( + let mut unchanged_buffers = HashSet::default(); + let mut changed_buffers = HashSet::default(); + let workspace_diagnostics_updates = workspace_diagnostics + .into_iter() + .filter_map( + |workspace_diagnostics| match workspace_diagnostics.diagnostics { + LspPullDiagnostics::Response { server_id, - lsp::PublishDiagnosticsParams { - uri: uri.clone(), - diagnostics: Vec::new(), - version: None, - }, - Some(result_id), - DiagnosticSourceKind::Pulled, - disk_based_sources, - |_, _, _| true, - cx, - ) - .log_err(); - } - PulledDiagnostics::Changed { - diagnostics, - result_id, - } => { - self.merge_diagnostics( - server_id, - lsp::PublishDiagnosticsParams { - uri: uri.clone(), + uri, + diagnostics, + } => Some((server_id, uri, diagnostics, workspace_diagnostics.version)), + LspPullDiagnostics::Default => None, + }, + ) + .fold( + HashMap::default(), + |mut acc, (server_id, uri, diagnostics, version)| { + let (result_id, diagnostics) = match diagnostics { + PulledDiagnostics::Unchanged { result_id } => { + unchanged_buffers.insert(uri.clone()); + (Some(result_id), Vec::new()) + } + PulledDiagnostics::Changed { + result_id, diagnostics, - version: workspace_diagnostics.version, - }, - result_id, - DiagnosticSourceKind::Pulled, - disk_based_sources, - |buffer, old_diagnostic, cx| match old_diagnostic.source_kind { - DiagnosticSourceKind::Pulled => { - let buffer_url = File::from_dyn(buffer.file()) - .map(|f| f.abs_path(cx)) - .and_then(|abs_path| file_path_to_lsp_url(&abs_path).ok()); - buffer_url.is_none_or(|buffer_url| buffer_url != uri) - } - DiagnosticSourceKind::Other | DiagnosticSourceKind::Pushed => true, - }, - cx, - ) - .log_err(); - } - } + } => { + changed_buffers.insert(uri.clone()); + (result_id, diagnostics) + } + }; + let disk_based_sources = Cow::Owned( + self.language_server_adapter_for_id(server_id) + .as_ref() + .map(|adapter| adapter.disk_based_diagnostic_sources.as_slice()) + .unwrap_or(&[]) + .to_vec(), + ); + acc.entry(server_id) + .or_insert_with(Vec::new) + .push(DocumentDiagnosticsUpdate { + server_id, + diagnostics: lsp::PublishDiagnosticsParams { + uri, + diagnostics, + version, + }, + result_id, + disk_based_sources, + }); + acc + }, + ); + + for diagnostic_updates in workspace_diagnostics_updates.into_values() { + self.merge_lsp_diagnostics( + DiagnosticSourceKind::Pulled, + diagnostic_updates, + |buffer, old_diagnostic, cx| { + File::from_dyn(buffer.file()) + .and_then(|file| { + let abs_path = file.as_local()?.abs_path(cx); + lsp::Url::from_file_path(abs_path).ok() + }) + .is_none_or(|buffer_uri| { + unchanged_buffers.contains(&buffer_uri) + || match old_diagnostic.source_kind { + DiagnosticSourceKind::Pulled => { + !changed_buffers.contains(&buffer_uri) + } + DiagnosticSourceKind::Other | DiagnosticSourceKind::Pushed => { + true + } + } + }) + }, + cx, + ) + .log_err(); } } } diff --git a/crates/project/src/lsp_store/clangd_ext.rs b/crates/project/src/lsp_store/clangd_ext.rs index cf3507352e..274b1b8980 100644 --- a/crates/project/src/lsp_store/clangd_ext.rs +++ b/crates/project/src/lsp_store/clangd_ext.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{borrow::Cow, sync::Arc}; use ::serde::{Deserialize, Serialize}; use gpui::WeakEntity; @@ -6,7 +6,7 @@ use language::{CachedLspAdapter, Diagnostic, DiagnosticSourceKind}; use lsp::{LanguageServer, LanguageServerName}; use util::ResultExt as _; -use crate::LspStore; +use crate::{LspStore, lsp_store::DocumentDiagnosticsUpdate}; pub const CLANGD_SERVER_NAME: LanguageServerName = LanguageServerName::new_static("clangd"); const INACTIVE_REGION_MESSAGE: &str = "inactive region"; @@ -81,12 +81,16 @@ pub fn register_notifications( version: params.text_document.version, diagnostics, }; - this.merge_diagnostics( - server_id, - mapped_diagnostics, - None, + this.merge_lsp_diagnostics( DiagnosticSourceKind::Pushed, - &adapter.disk_based_diagnostic_sources, + vec![DocumentDiagnosticsUpdate { + server_id, + diagnostics: mapped_diagnostics, + result_id: None, + disk_based_sources: Cow::Borrowed( + &adapter.disk_based_diagnostic_sources, + ), + }], |_, diag, _| !is_inactive_region(diag), cx, ) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 614d514cd4..b3a9e6fdf5 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -74,9 +74,9 @@ use gpui::{ Task, WeakEntity, Window, }; use language::{ - Buffer, BufferEvent, Capability, CodeLabel, CursorShape, DiagnosticSourceKind, Language, - LanguageName, LanguageRegistry, PointUtf16, ToOffset, ToPointUtf16, Toolchain, ToolchainList, - Transaction, Unclipped, language_settings::InlayHintKind, proto::split_operations, + Buffer, BufferEvent, Capability, CodeLabel, CursorShape, Language, LanguageName, + LanguageRegistry, PointUtf16, ToOffset, ToPointUtf16, Toolchain, ToolchainList, Transaction, + Unclipped, language_settings::InlayHintKind, proto::split_operations, }; use lsp::{ CodeActionKind, CompletionContext, CompletionItemKind, DocumentHighlightKind, InsertTextMode, @@ -305,7 +305,7 @@ pub enum Event { language_server_id: LanguageServerId, }, DiagnosticsUpdated { - path: ProjectPath, + paths: Vec, language_server_id: LanguageServerId, }, RemoteIdChanged(Option), @@ -2895,18 +2895,17 @@ impl Project { cx: &mut Context, ) { match event { - LspStoreEvent::DiagnosticsUpdated { - language_server_id, - path, - } => cx.emit(Event::DiagnosticsUpdated { - path: path.clone(), - language_server_id: *language_server_id, - }), - LspStoreEvent::LanguageServerAdded(language_server_id, name, worktree_id) => cx.emit( - Event::LanguageServerAdded(*language_server_id, name.clone(), *worktree_id), + LspStoreEvent::DiagnosticsUpdated { server_id, paths } => { + cx.emit(Event::DiagnosticsUpdated { + paths: paths.clone(), + language_server_id: *server_id, + }) + } + LspStoreEvent::LanguageServerAdded(server_id, name, worktree_id) => cx.emit( + Event::LanguageServerAdded(*server_id, name.clone(), *worktree_id), ), - LspStoreEvent::LanguageServerRemoved(language_server_id) => { - cx.emit(Event::LanguageServerRemoved(*language_server_id)) + LspStoreEvent::LanguageServerRemoved(server_id) => { + cx.emit(Event::LanguageServerRemoved(*server_id)) } LspStoreEvent::LanguageServerLog(server_id, log_type, string) => cx.emit( Event::LanguageServerLog(*server_id, log_type.clone(), string.clone()), @@ -3829,27 +3828,6 @@ impl Project { }) } - pub fn update_diagnostics( - &mut self, - language_server_id: LanguageServerId, - source_kind: DiagnosticSourceKind, - result_id: Option, - params: lsp::PublishDiagnosticsParams, - disk_based_sources: &[String], - cx: &mut Context, - ) -> Result<(), anyhow::Error> { - self.lsp_store.update(cx, |lsp_store, cx| { - lsp_store.update_diagnostics( - language_server_id, - params, - result_id, - source_kind, - disk_based_sources, - cx, - ) - }) - } - pub fn search(&mut self, query: SearchQuery, cx: &mut Context) -> Receiver { let (result_tx, result_rx) = smol::channel::unbounded(); diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 9c6d9ec979..cb3c9efe60 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -20,8 +20,8 @@ use gpui::{App, BackgroundExecutor, SemanticVersion, UpdateGlobal}; use http_client::Url; use itertools::Itertools; use language::{ - Diagnostic, DiagnosticEntry, DiagnosticSet, DiskState, FakeLspAdapter, LanguageConfig, - LanguageMatcher, LanguageName, LineEnding, OffsetRangeExt, Point, ToPoint, + Diagnostic, DiagnosticEntry, DiagnosticSet, DiagnosticSourceKind, DiskState, FakeLspAdapter, + LanguageConfig, LanguageMatcher, LanguageName, LineEnding, OffsetRangeExt, Point, ToPoint, language_settings::{AllLanguageSettings, LanguageSettingsContent, language_settings}, tree_sitter_rust, tree_sitter_typescript, }; @@ -1619,7 +1619,7 @@ async fn test_disk_based_diagnostics_progress(cx: &mut gpui::TestAppContext) { events.next().await.unwrap(), Event::DiagnosticsUpdated { language_server_id: LanguageServerId(0), - path: (worktree_id, Path::new("a.rs")).into() + paths: vec![(worktree_id, Path::new("a.rs")).into()], } ); @@ -1667,7 +1667,7 @@ async fn test_disk_based_diagnostics_progress(cx: &mut gpui::TestAppContext) { events.next().await.unwrap(), Event::DiagnosticsUpdated { language_server_id: LanguageServerId(0), - path: (worktree_id, Path::new("a.rs")).into() + paths: vec![(worktree_id, Path::new("a.rs")).into()], } ); diff --git a/crates/proto/proto/lsp.proto b/crates/proto/proto/lsp.proto index 164a7d3a50..ea9647feff 100644 --- a/crates/proto/proto/lsp.proto +++ b/crates/proto/proto/lsp.proto @@ -525,6 +525,7 @@ message UpdateDiagnosticSummary { uint64 project_id = 1; uint64 worktree_id = 2; DiagnosticSummary summary = 3; + repeated DiagnosticSummary more_summaries = 4; } message DiagnosticSummary { From 90fa06dd61add35ec4e3a7ff8cc81ee5c5244937 Mon Sep 17 00:00:00 2001 From: localcc Date: Thu, 7 Aug 2025 16:47:19 +0200 Subject: [PATCH 2/7] Fix file unlocking after closing the workspace (#35741) Release Notes: - Fixed folders being locked after closing them in zed --- crates/fs/src/fs_watcher.rs | 189 ++++++++++++++++++++++++++---------- 1 file changed, 137 insertions(+), 52 deletions(-) diff --git a/crates/fs/src/fs_watcher.rs b/crates/fs/src/fs_watcher.rs index 9fdf2ad0b1..c9da4fa957 100644 --- a/crates/fs/src/fs_watcher.rs +++ b/crates/fs/src/fs_watcher.rs @@ -1,6 +1,9 @@ use notify::EventKind; use parking_lot::Mutex; -use std::sync::{Arc, OnceLock}; +use std::{ + collections::HashMap, + sync::{Arc, OnceLock}, +}; use util::{ResultExt, paths::SanitizedPath}; use crate::{PathEvent, PathEventKind, Watcher}; @@ -8,6 +11,7 @@ use crate::{PathEvent, PathEventKind, Watcher}; pub struct FsWatcher { tx: smol::channel::Sender<()>, pending_path_events: Arc>>, + registrations: Mutex, WatcherRegistrationId>>, } impl FsWatcher { @@ -18,10 +22,24 @@ impl FsWatcher { Self { tx, pending_path_events, + registrations: Default::default(), } } } +impl Drop for FsWatcher { + fn drop(&mut self) { + let mut registrations = self.registrations.lock(); + let registrations = registrations.drain(); + + let _ = global(|g| { + for (_, registration) in registrations { + g.remove(registration); + } + }); + } +} + impl Watcher for FsWatcher { fn add(&self, path: &std::path::Path) -> anyhow::Result<()> { let root_path = SanitizedPath::from(path); @@ -29,75 +47,136 @@ impl Watcher for FsWatcher { let tx = self.tx.clone(); let pending_paths = self.pending_path_events.clone(); - use notify::Watcher; + let path: Arc = path.into(); - global({ + if self.registrations.lock().contains_key(&path) { + return Ok(()); + } + + let registration_id = global({ + let path = path.clone(); |g| { - g.add(move |event: ¬ify::Event| { - let kind = match event.kind { - EventKind::Create(_) => Some(PathEventKind::Created), - EventKind::Modify(_) => Some(PathEventKind::Changed), - EventKind::Remove(_) => Some(PathEventKind::Removed), - _ => None, - }; - let mut path_events = event - .paths - .iter() - .filter_map(|event_path| { - let event_path = SanitizedPath::from(event_path); - event_path.starts_with(&root_path).then(|| PathEvent { - path: event_path.as_path().to_path_buf(), - kind, + g.add( + path, + notify::RecursiveMode::NonRecursive, + move |event: ¬ify::Event| { + let kind = match event.kind { + EventKind::Create(_) => Some(PathEventKind::Created), + EventKind::Modify(_) => Some(PathEventKind::Changed), + EventKind::Remove(_) => Some(PathEventKind::Removed), + _ => None, + }; + let mut path_events = event + .paths + .iter() + .filter_map(|event_path| { + let event_path = SanitizedPath::from(event_path); + event_path.starts_with(&root_path).then(|| PathEvent { + path: event_path.as_path().to_path_buf(), + kind, + }) }) - }) - .collect::>(); + .collect::>(); - if !path_events.is_empty() { - path_events.sort(); - let mut pending_paths = pending_paths.lock(); - if pending_paths.is_empty() { - tx.try_send(()).ok(); + if !path_events.is_empty() { + path_events.sort(); + let mut pending_paths = pending_paths.lock(); + if pending_paths.is_empty() { + tx.try_send(()).ok(); + } + util::extend_sorted( + &mut *pending_paths, + path_events, + usize::MAX, + |a, b| a.path.cmp(&b.path), + ); } - util::extend_sorted( - &mut *pending_paths, - path_events, - usize::MAX, - |a, b| a.path.cmp(&b.path), - ); - } - }) + }, + ) } - })?; - - global(|g| { - g.watcher - .lock() - .watch(path, notify::RecursiveMode::NonRecursive) })??; + self.registrations.lock().insert(path, registration_id); + Ok(()) } fn remove(&self, path: &std::path::Path) -> anyhow::Result<()> { - use notify::Watcher; - Ok(global(|w| w.watcher.lock().unwatch(path))??) + let Some(registration) = self.registrations.lock().remove(path) else { + return Ok(()); + }; + + global(|w| w.remove(registration)) } } -pub struct GlobalWatcher { +#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct WatcherRegistrationId(u32); + +struct WatcherRegistrationState { + callback: Box, + path: Arc, +} + +struct WatcherState { // two mutexes because calling watcher.add triggers an watcher.event, which needs watchers. #[cfg(target_os = "linux")] - pub(super) watcher: Mutex, + watcher: notify::INotifyWatcher, #[cfg(target_os = "freebsd")] - pub(super) watcher: Mutex, + watcher: notify::KqueueWatcher, #[cfg(target_os = "windows")] - pub(super) watcher: Mutex, - pub(super) watchers: Mutex>>, + watcher: notify::ReadDirectoryChangesWatcher, + + watchers: HashMap, + path_registrations: HashMap, u32>, + last_registration: WatcherRegistrationId, +} + +pub struct GlobalWatcher { + state: Mutex, } impl GlobalWatcher { - pub(super) fn add(&self, cb: impl Fn(¬ify::Event) + Send + Sync + 'static) { - self.watchers.lock().push(Box::new(cb)) + #[must_use] + fn add( + &self, + path: Arc, + mode: notify::RecursiveMode, + cb: impl Fn(¬ify::Event) + Send + Sync + 'static, + ) -> anyhow::Result { + use notify::Watcher; + let mut state = self.state.lock(); + + state.watcher.watch(&path, mode)?; + + let id = state.last_registration; + state.last_registration = WatcherRegistrationId(id.0 + 1); + + let registration_state = WatcherRegistrationState { + callback: Box::new(cb), + path: path.clone(), + }; + state.watchers.insert(id, registration_state); + *state.path_registrations.entry(path.clone()).or_insert(0) += 1; + + Ok(id) + } + + pub fn remove(&self, id: WatcherRegistrationId) { + use notify::Watcher; + let mut state = self.state.lock(); + let Some(registration_state) = state.watchers.remove(&id) else { + return; + }; + + let Some(count) = state.path_registrations.get_mut(®istration_state.path) else { + return; + }; + *count -= 1; + if *count == 0 { + state.watcher.unwatch(®istration_state.path).log_err(); + state.path_registrations.remove(®istration_state.path); + } } } @@ -114,8 +193,10 @@ fn handle_event(event: Result) { return; }; global::<()>(move |watcher| { - for f in watcher.watchers.lock().iter() { - f(&event) + let state = watcher.state.lock(); + for registration in state.watchers.values() { + let callback = ®istration.callback; + callback(&event); } }) .log_err(); @@ -124,8 +205,12 @@ fn handle_event(event: Result) { pub fn global(f: impl FnOnce(&GlobalWatcher) -> T) -> anyhow::Result { let result = FS_WATCHER_INSTANCE.get_or_init(|| { notify::recommended_watcher(handle_event).map(|file_watcher| GlobalWatcher { - watcher: Mutex::new(file_watcher), - watchers: Default::default(), + state: Mutex::new(WatcherState { + watcher: file_watcher, + watchers: Default::default(), + path_registrations: Default::default(), + last_registration: Default::default(), + }), }) }); match result { From 262365ca241cafc3008b79bd61724f08e93b676c Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Thu, 7 Aug 2025 11:50:11 -0300 Subject: [PATCH 3/7] keymap editor: Refine how we display matching keystrokes (#35796) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit | Before | After | |--------|--------| | CleanShot 2025-08-07 at 10  54 42@2x | CleanShot 2025-08-07 at 11  29 47@2x | Release Notes: - N/A --- crates/settings_ui/src/keybindings.rs | 100 ++++++++++-------- .../src/ui_components/keystroke_input.rs | 2 +- crates/ui_input/src/ui_input.rs | 2 +- 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/crates/settings_ui/src/keybindings.rs b/crates/settings_ui/src/keybindings.rs index 60e527677a..599bb0b18f 100644 --- a/crates/settings_ui/src/keybindings.rs +++ b/crates/settings_ui/src/keybindings.rs @@ -2415,6 +2415,7 @@ impl Render for KeybindingEditorModal { .header( ModalHeader::new().child( v_flex() + .w_full() .pb_1p5() .mb_1() .gap_0p5() @@ -2438,17 +2439,55 @@ impl Render for KeybindingEditorModal { .section( Section::new().child( v_flex() - .gap_2() + .gap_2p5() .child( v_flex() - .child(Label::new("Edit Keystroke")) .gap_1() - .child(self.keybind_editor.clone()), + .child(Label::new("Edit Keystroke")) + .child(self.keybind_editor.clone()) + .child(h_flex().gap_px().when( + matching_bindings_count > 0, + |this| { + let label = format!( + "There {} {} {} with the same keystrokes.", + if matching_bindings_count == 1 { + "is" + } else { + "are" + }, + matching_bindings_count, + if matching_bindings_count == 1 { + "binding" + } else { + "bindings" + } + ); + + this.child( + Label::new(label) + .size(LabelSize::Small) + .color(Color::Muted), + ) + .child( + Button::new("show_matching", "View") + .label_size(LabelSize::Small) + .icon(IconName::ArrowUpRight) + .icon_color(Color::Muted) + .icon_size(IconSize::XSmall) + .on_click(cx.listener( + |this, _, window, cx| { + this.show_matching_bindings( + window, cx, + ); + }, + )), + ) + }, + )), ) .when_some(self.action_arguments_editor.clone(), |this, editor| { this.child( v_flex() - .mt_1p5() .gap_1() .child(Label::new("Edit Arguments")) .child(editor), @@ -2459,50 +2498,25 @@ impl Render for KeybindingEditorModal { this.child( Banner::new() .severity(error.severity) - // For some reason, the div overflows its container to the - //right. The padding accounts for that. - .child( - div() - .size_full() - .pr_2() - .child(Label::new(error.content.clone())), - ), + .child(Label::new(error.content.clone())), ) }), ), ) .footer( - ModalFooter::new() - .start_slot( - div().when(matching_bindings_count > 0, |this| { - this.child( - Button::new("show_matching", format!( - "There {} {} {} with the same keystrokes. Click to view", - if matching_bindings_count == 1 { "is" } else { "are" }, - matching_bindings_count, - if matching_bindings_count == 1 { "binding" } else { "bindings" } - )) - .style(ButtonStyle::Transparent) - .color(Color::Accent) - .on_click(cx.listener(|this, _, window, cx| { - this.show_matching_bindings(window, cx); - })) - ) - }) - ) - .end_slot( - h_flex() - .gap_1() - .child( - Button::new("cancel", "Cancel") - .on_click(cx.listener(|_, _, _, cx| cx.emit(DismissEvent))), - ) - .child(Button::new("save-btn", "Save").on_click(cx.listener( - |this, _event, _window, cx| { - this.save_or_display_error(cx); - }, - ))), - ), + ModalFooter::new().end_slot( + h_flex() + .gap_1() + .child( + Button::new("cancel", "Cancel") + .on_click(cx.listener(|_, _, _, cx| cx.emit(DismissEvent))), + ) + .child(Button::new("save-btn", "Save").on_click(cx.listener( + |this, _event, _window, cx| { + this.save_or_display_error(cx); + }, + ))), + ), ), ) } diff --git a/crates/settings_ui/src/ui_components/keystroke_input.rs b/crates/settings_ui/src/ui_components/keystroke_input.rs index 03d27d0ab9..ee5c4036ea 100644 --- a/crates/settings_ui/src/ui_components/keystroke_input.rs +++ b/crates/settings_ui/src/ui_components/keystroke_input.rs @@ -529,7 +529,7 @@ impl Render for KeystrokeInput { .w_full() .flex_1() .justify_between() - .rounded_lg() + .rounded_sm() .overflow_hidden() .map(|this| { if is_recording { diff --git a/crates/ui_input/src/ui_input.rs b/crates/ui_input/src/ui_input.rs index 309b3f62f6..1a5bebaf1e 100644 --- a/crates/ui_input/src/ui_input.rs +++ b/crates/ui_input/src/ui_input.rs @@ -168,7 +168,7 @@ impl Render for SingleLineInput { .py_1p5() .flex_grow() .text_color(style.text_color) - .rounded_lg() + .rounded_sm() .bg(style.background_color) .border_1() .border_color(style.border_color) From dd840e4b2776a66bc8b2f6ad8379e7fdb914c0c5 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Thu, 7 Aug 2025 17:01:22 +0200 Subject: [PATCH 4/7] editor: Fix multi-buffer headers spilling over at narrow widths (#35800) Release Notes: - N/A --- crates/editor/src/element.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 17a43f9640..034fff970d 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -3682,6 +3682,7 @@ impl EditorElement { .id("path header block") .size_full() .justify_between() + .overflow_hidden() .child( h_flex() .gap_2() From 22342206183005b8dd58ecb6d6cac8efcac54a42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20L=C3=BCthy?= Date: Thu, 7 Aug 2025 17:27:29 +0200 Subject: [PATCH 5/7] completions: Add subtle/eager behavior to Supermaven and Copilot (#35548) This pull request introduces changes to improve the behavior and consistency of multiple completion providers (`CopilotCompletionProvider`, `SupermavenCompletionProvider`) and their integration with UI elements like menus and inline completion buttons. It now allows to see the prediction with the completion menu open whilst pressing `opt` and also enables the subtle/eager setting that was introduced with zeta. Edit: I managed to get the preview working with correct icons! image CleanShot 2025-08-04 at 01 36 31@2x Correct icons are also displayed: image Edit2: I added some comments, would be very happy to receive feedback (still learning rust) Release Notes: - Added Subtle and Eager edit prediction modes to Copilot and Supermaven --- .../src/copilot_completion_provider.rs | 23 +-- crates/edit_prediction/src/edit_prediction.rs | 9 ++ .../src/edit_prediction_button.rs | 7 +- crates/editor/src/edit_prediction_tests.rs | 152 ++++++++++++++++++ crates/editor/src/editor.rs | 141 +++++++++++----- crates/supermaven/src/supermaven.rs | 92 +++++++++-- .../src/supermaven_completion_provider.rs | 11 +- 7 files changed, 372 insertions(+), 63 deletions(-) diff --git a/crates/copilot/src/copilot_completion_provider.rs b/crates/copilot/src/copilot_completion_provider.rs index 2a7225c4e3..2fd6df27b9 100644 --- a/crates/copilot/src/copilot_completion_provider.rs +++ b/crates/copilot/src/copilot_completion_provider.rs @@ -58,11 +58,19 @@ impl EditPredictionProvider for CopilotCompletionProvider { } fn show_completions_in_menu() -> bool { + true + } + + fn show_tab_accept_marker() -> bool { + true + } + + fn supports_jump_to_edit() -> bool { false } fn is_refreshing(&self) -> bool { - self.pending_refresh.is_some() + self.pending_refresh.is_some() && self.completions.is_empty() } fn is_enabled( @@ -343,8 +351,8 @@ mod tests { executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); cx.update_editor(|editor, window, cx| { assert!(editor.context_menu_visible()); - assert!(!editor.has_active_edit_prediction()); - // Since we have both, the copilot suggestion is not shown inline + assert!(editor.has_active_edit_prediction()); + // Since we have both, the copilot suggestion is existing but does not show up as ghost text assert_eq!(editor.text(cx), "one.\ntwo\nthree\n"); assert_eq!(editor.display_text(cx), "one.\ntwo\nthree\n"); @@ -934,8 +942,9 @@ mod tests { executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); cx.update_editor(|editor, _, cx| { assert!(editor.context_menu_visible()); - assert!(!editor.has_active_edit_prediction(),); + assert!(editor.has_active_edit_prediction()); assert_eq!(editor.text(cx), "one\ntwo.\nthree\n"); + assert_eq!(editor.display_text(cx), "one\ntwo.\nthree\n"); }); } @@ -1077,8 +1086,6 @@ mod tests { vec![complete_from_marker.clone(), replace_range_marker.clone()], ); - let complete_from_position = - cx.to_lsp(marked_ranges.remove(&complete_from_marker).unwrap()[0].start); let replace_range = cx.to_lsp_range(marked_ranges.remove(&replace_range_marker).unwrap()[0].clone()); @@ -1087,10 +1094,6 @@ mod tests { let completions = completions.clone(); async move { assert_eq!(params.text_document_position.text_document.uri, url.clone()); - assert_eq!( - params.text_document_position.position, - complete_from_position - ); Ok(Some(lsp::CompletionResponse::Array( completions .iter() diff --git a/crates/edit_prediction/src/edit_prediction.rs b/crates/edit_prediction/src/edit_prediction.rs index fd4e9bb21d..c8502f75de 100644 --- a/crates/edit_prediction/src/edit_prediction.rs +++ b/crates/edit_prediction/src/edit_prediction.rs @@ -61,6 +61,10 @@ pub trait EditPredictionProvider: 'static + Sized { fn show_tab_accept_marker() -> bool { false } + fn supports_jump_to_edit() -> bool { + true + } + fn data_collection_state(&self, _cx: &App) -> DataCollectionState { DataCollectionState::Unsupported } @@ -116,6 +120,7 @@ pub trait EditPredictionProviderHandle { ) -> bool; fn show_completions_in_menu(&self) -> bool; fn show_tab_accept_marker(&self) -> bool; + fn supports_jump_to_edit(&self) -> bool; fn data_collection_state(&self, cx: &App) -> DataCollectionState; fn usage(&self, cx: &App) -> Option; fn toggle_data_collection(&self, cx: &mut App); @@ -166,6 +171,10 @@ where T::show_tab_accept_marker() } + fn supports_jump_to_edit(&self) -> bool { + T::supports_jump_to_edit() + } + fn data_collection_state(&self, cx: &App) -> DataCollectionState { self.read(cx).data_collection_state(cx) } diff --git a/crates/edit_prediction_button/src/edit_prediction_button.rs b/crates/edit_prediction_button/src/edit_prediction_button.rs index 9ab94a4095..3d3b43d71b 100644 --- a/crates/edit_prediction_button/src/edit_prediction_button.rs +++ b/crates/edit_prediction_button/src/edit_prediction_button.rs @@ -491,7 +491,12 @@ impl EditPredictionButton { let subtle_mode = matches!(current_mode, EditPredictionsMode::Subtle); let eager_mode = matches!(current_mode, EditPredictionsMode::Eager); - if matches!(provider, EditPredictionProvider::Zed) { + if matches!( + provider, + EditPredictionProvider::Zed + | EditPredictionProvider::Copilot + | EditPredictionProvider::Supermaven + ) { menu = menu .separator() .header("Display Modes") diff --git a/crates/editor/src/edit_prediction_tests.rs b/crates/editor/src/edit_prediction_tests.rs index 527dfb8832..7bf51e45d7 100644 --- a/crates/editor/src/edit_prediction_tests.rs +++ b/crates/editor/src/edit_prediction_tests.rs @@ -228,6 +228,49 @@ async fn test_edit_prediction_invalidation_range(cx: &mut gpui::TestAppContext) }); } +#[gpui::test] +async fn test_edit_prediction_jump_disabled_for_non_zed_providers(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorTestContext::new(cx).await; + let provider = cx.new(|_| FakeNonZedEditPredictionProvider::default()); + assign_editor_completion_provider_non_zed(provider.clone(), &mut cx); + + // Cursor is 2+ lines above the proposed edit + cx.set_state(indoc! {" + line 0 + line ˇ1 + line 2 + line 3 + line + "}); + + propose_edits_non_zed( + &provider, + vec![(Point::new(4, 3)..Point::new(4, 3), " 4")], + &mut cx, + ); + + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + + // For non-Zed providers, there should be no move completion (jump functionality disabled) + cx.editor(|editor, _, _| { + if let Some(completion_state) = &editor.active_edit_prediction { + // Should be an Edit prediction, not a Move prediction + match &completion_state.completion { + EditPrediction::Edit { .. } => { + // This is expected for non-Zed providers + } + EditPrediction::Move { .. } => { + panic!( + "Non-Zed providers should not show Move predictions (jump functionality)" + ); + } + } + } + }); +} + fn assert_editor_active_edit_completion( cx: &mut EditorTestContext, assert: impl FnOnce(MultiBufferSnapshot, &Vec<(Range, String)>), @@ -301,6 +344,37 @@ fn assign_editor_completion_provider( }) } +fn propose_edits_non_zed( + provider: &Entity, + edits: Vec<(Range, &str)>, + cx: &mut EditorTestContext, +) { + let snapshot = cx.buffer_snapshot(); + let edits = edits.into_iter().map(|(range, text)| { + let range = snapshot.anchor_after(range.start)..snapshot.anchor_before(range.end); + (range, text.into()) + }); + + cx.update(|_, cx| { + provider.update(cx, |provider, _| { + provider.set_edit_prediction(Some(edit_prediction::EditPrediction { + id: None, + edits: edits.collect(), + edit_preview: None, + })) + }) + }); +} + +fn assign_editor_completion_provider_non_zed( + provider: Entity, + cx: &mut EditorTestContext, +) { + cx.update_editor(|editor, window, cx| { + editor.set_edit_prediction_provider(Some(provider), window, cx); + }) +} + #[derive(Default, Clone)] pub struct FakeEditPredictionProvider { pub completion: Option, @@ -325,6 +399,84 @@ impl EditPredictionProvider for FakeEditPredictionProvider { false } + fn supports_jump_to_edit() -> bool { + true + } + + fn is_enabled( + &self, + _buffer: &gpui::Entity, + _cursor_position: language::Anchor, + _cx: &gpui::App, + ) -> bool { + true + } + + fn is_refreshing(&self) -> bool { + false + } + + fn refresh( + &mut self, + _project: Option>, + _buffer: gpui::Entity, + _cursor_position: language::Anchor, + _debounce: bool, + _cx: &mut gpui::Context, + ) { + } + + fn cycle( + &mut self, + _buffer: gpui::Entity, + _cursor_position: language::Anchor, + _direction: edit_prediction::Direction, + _cx: &mut gpui::Context, + ) { + } + + fn accept(&mut self, _cx: &mut gpui::Context) {} + + fn discard(&mut self, _cx: &mut gpui::Context) {} + + fn suggest<'a>( + &mut self, + _buffer: &gpui::Entity, + _cursor_position: language::Anchor, + _cx: &mut gpui::Context, + ) -> Option { + self.completion.clone() + } +} + +#[derive(Default, Clone)] +pub struct FakeNonZedEditPredictionProvider { + pub completion: Option, +} + +impl FakeNonZedEditPredictionProvider { + pub fn set_edit_prediction(&mut self, completion: Option) { + self.completion = completion; + } +} + +impl EditPredictionProvider for FakeNonZedEditPredictionProvider { + fn name() -> &'static str { + "fake-non-zed-provider" + } + + fn display_name() -> &'static str { + "Fake Non-Zed Provider" + } + + fn show_completions_in_menu() -> bool { + false + } + + fn supports_jump_to_edit() -> bool { + false + } + fn is_enabled( &self, _buffer: &gpui::Entity, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 73a81bea19..677acd9fd8 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7760,8 +7760,14 @@ impl Editor { } else { None }; - let is_move = - move_invalidation_row_range.is_some() || self.edit_predictions_hidden_for_vim_mode; + let supports_jump = self + .edit_prediction_provider + .as_ref() + .map(|provider| provider.provider.supports_jump_to_edit()) + .unwrap_or(true); + + let is_move = supports_jump + && (move_invalidation_row_range.is_some() || self.edit_predictions_hidden_for_vim_mode); let completion = if is_move { invalidation_row_range = move_invalidation_row_range.unwrap_or(edit_start_row..edit_end_row); @@ -8799,8 +8805,12 @@ impl Editor { return None; } - let highlighted_edits = - crate::edit_prediction_edit_text(&snapshot, edits, edit_preview.as_ref()?, false, cx); + let highlighted_edits = if let Some(edit_preview) = edit_preview.as_ref() { + crate::edit_prediction_edit_text(&snapshot, edits, edit_preview, false, cx) + } else { + // Fallback for providers without edit_preview + crate::edit_prediction_fallback_text(edits, cx) + }; let styled_text = highlighted_edits.to_styled_text(&style.text); let line_count = highlighted_edits.text.lines().count(); @@ -9068,6 +9078,18 @@ impl Editor { let editor_bg_color = cx.theme().colors().editor_background; editor_bg_color.blend(accent_color.opacity(0.6)) } + fn get_prediction_provider_icon_name( + provider: &Option, + ) -> IconName { + match provider { + Some(provider) => match provider.provider.name() { + "copilot" => IconName::Copilot, + "supermaven" => IconName::Supermaven, + _ => IconName::ZedPredict, + }, + None => IconName::ZedPredict, + } + } fn render_edit_prediction_cursor_popover( &self, @@ -9080,6 +9102,7 @@ impl Editor { cx: &mut Context, ) -> Option { let provider = self.edit_prediction_provider.as_ref()?; + let provider_icon = Self::get_prediction_provider_icon_name(&self.edit_prediction_provider); if provider.provider.needs_terms_acceptance(cx) { return Some( @@ -9106,7 +9129,7 @@ impl Editor { h_flex() .flex_1() .gap_2() - .child(Icon::new(IconName::ZedPredict)) + .child(Icon::new(provider_icon)) .child(Label::new("Accept Terms of Service")) .child(div().w_full()) .child( @@ -9122,12 +9145,8 @@ impl Editor { let is_refreshing = provider.provider.is_refreshing(cx); - fn pending_completion_container() -> Div { - h_flex() - .h_full() - .flex_1() - .gap_2() - .child(Icon::new(IconName::ZedPredict)) + fn pending_completion_container(icon: IconName) -> Div { + h_flex().h_full().flex_1().gap_2().child(Icon::new(icon)) } let completion = match &self.active_edit_prediction { @@ -9157,7 +9176,7 @@ impl Editor { Icon::new(IconName::ZedPredictUp) } } - EditPrediction::Edit { .. } => Icon::new(IconName::ZedPredict), + EditPrediction::Edit { .. } => Icon::new(provider_icon), })) .child( h_flex() @@ -9224,15 +9243,15 @@ impl Editor { cx, )?, - None => { - pending_completion_container().child(Label::new("...").size(LabelSize::Small)) - } + None => pending_completion_container(provider_icon) + .child(Label::new("...").size(LabelSize::Small)), }, - None => pending_completion_container().child(Label::new("No Prediction")), + None => pending_completion_container(provider_icon) + .child(Label::new("...").size(LabelSize::Small)), }; - let completion = if is_refreshing { + let completion = if is_refreshing || self.active_edit_prediction.is_none() { completion .with_animation( "loading-completion", @@ -9332,23 +9351,35 @@ impl Editor { .child(Icon::new(arrow).color(Color::Muted).size(IconSize::Small)) } + let supports_jump = self + .edit_prediction_provider + .as_ref() + .map(|provider| provider.provider.supports_jump_to_edit()) + .unwrap_or(true); + match &completion.completion { EditPrediction::Move { target, snapshot, .. - } => Some( - h_flex() - .px_2() - .gap_2() - .flex_1() - .child( - if target.text_anchor.to_point(&snapshot).row > cursor_point.row { - Icon::new(IconName::ZedPredictDown) - } else { - Icon::new(IconName::ZedPredictUp) - }, - ) - .child(Label::new("Jump to Edit")), - ), + } => { + if !supports_jump { + return None; + } + + Some( + h_flex() + .px_2() + .gap_2() + .flex_1() + .child( + if target.text_anchor.to_point(&snapshot).row > cursor_point.row { + Icon::new(IconName::ZedPredictDown) + } else { + Icon::new(IconName::ZedPredictUp) + }, + ) + .child(Label::new("Jump to Edit")), + ) + } EditPrediction::Edit { edits, @@ -9358,14 +9389,13 @@ impl Editor { } => { let first_edit_row = edits.first()?.0.start.text_anchor.to_point(&snapshot).row; - let (highlighted_edits, has_more_lines) = crate::edit_prediction_edit_text( - &snapshot, - &edits, - edit_preview.as_ref()?, - true, - cx, - ) - .first_line_preview(); + let (highlighted_edits, has_more_lines) = + if let Some(edit_preview) = edit_preview.as_ref() { + crate::edit_prediction_edit_text(&snapshot, &edits, edit_preview, true, cx) + .first_line_preview() + } else { + crate::edit_prediction_fallback_text(&edits, cx).first_line_preview() + }; let styled_text = gpui::StyledText::new(highlighted_edits.text) .with_default_highlights(&style.text, highlighted_edits.highlights); @@ -9376,11 +9406,13 @@ impl Editor { .child(styled_text) .when(has_more_lines, |parent| parent.child("…")); - let left = if first_edit_row != cursor_point.row { + let left = if supports_jump && first_edit_row != cursor_point.row { render_relative_row_jump("", cursor_point.row, first_edit_row) .into_any_element() } else { - Icon::new(IconName::ZedPredict).into_any_element() + let icon_name = + Editor::get_prediction_provider_icon_name(&self.edit_prediction_provider); + Icon::new(icon_name).into_any_element() }; Some( @@ -23270,6 +23302,33 @@ fn edit_prediction_edit_text( edit_preview.highlight_edits(current_snapshot, &edits, include_deletions, cx) } +fn edit_prediction_fallback_text(edits: &[(Range, String)], cx: &App) -> HighlightedText { + // Fallback for providers that don't provide edit_preview (like Copilot/Supermaven) + // Just show the raw edit text with basic styling + let mut text = String::new(); + let mut highlights = Vec::new(); + + let insertion_highlight_style = HighlightStyle { + color: Some(cx.theme().colors().text), + ..Default::default() + }; + + for (_, edit_text) in edits { + let start_offset = text.len(); + text.push_str(edit_text); + let end_offset = text.len(); + + if start_offset < end_offset { + highlights.push((start_offset..end_offset, insertion_highlight_style)); + } + } + + HighlightedText { + text: text.into(), + highlights, + } +} + pub fn diagnostic_style(severity: lsp::DiagnosticSeverity, colors: &StatusColors) -> Hsla { match severity { lsp::DiagnosticSeverity::ERROR => colors.error, diff --git a/crates/supermaven/src/supermaven.rs b/crates/supermaven/src/supermaven.rs index ab500fb79d..a31b96d882 100644 --- a/crates/supermaven/src/supermaven.rs +++ b/crates/supermaven/src/supermaven.rs @@ -234,16 +234,14 @@ fn find_relevant_completion<'a>( } let original_cursor_offset = buffer.clip_offset(state.prefix_offset, text::Bias::Left); - let text_inserted_since_completion_request = - buffer.text_for_range(original_cursor_offset..current_cursor_offset); - let mut trimmed_completion = state_completion; - for chunk in text_inserted_since_completion_request { - if let Some(suffix) = trimmed_completion.strip_prefix(chunk) { - trimmed_completion = suffix; - } else { - continue 'completions; - } - } + let text_inserted_since_completion_request: String = buffer + .text_for_range(original_cursor_offset..current_cursor_offset) + .collect(); + let trimmed_completion = + match state_completion.strip_prefix(&text_inserted_since_completion_request) { + Some(suffix) => suffix, + None => continue 'completions, + }; if best_completion.map_or(false, |best| best.len() > trimmed_completion.len()) { continue; @@ -439,3 +437,77 @@ pub struct SupermavenCompletion { pub id: SupermavenCompletionStateId, pub updates: watch::Receiver<()>, } + +#[cfg(test)] +mod tests { + use super::*; + use collections::BTreeMap; + use gpui::TestAppContext; + use language::Buffer; + + #[gpui::test] + async fn test_find_relevant_completion_no_first_letter_skip(cx: &mut TestAppContext) { + let buffer = cx.new(|cx| Buffer::local("hello world", cx)); + let buffer_snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot()); + + let mut states = BTreeMap::new(); + let state_id = SupermavenCompletionStateId(1); + let (updates_tx, _) = watch::channel(); + + states.insert( + state_id, + SupermavenCompletionState { + buffer_id: buffer.entity_id(), + prefix_anchor: buffer_snapshot.anchor_before(0), // Start of buffer + prefix_offset: 0, + text: "hello".to_string(), + dedent: String::new(), + updates_tx, + }, + ); + + let cursor_position = buffer_snapshot.anchor_after(1); + + let result = find_relevant_completion( + &states, + buffer.entity_id(), + &buffer_snapshot, + cursor_position, + ); + + assert_eq!(result, Some("ello")); + } + + #[gpui::test] + async fn test_find_relevant_completion_with_multiple_chars(cx: &mut TestAppContext) { + let buffer = cx.new(|cx| Buffer::local("hello world", cx)); + let buffer_snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot()); + + let mut states = BTreeMap::new(); + let state_id = SupermavenCompletionStateId(1); + let (updates_tx, _) = watch::channel(); + + states.insert( + state_id, + SupermavenCompletionState { + buffer_id: buffer.entity_id(), + prefix_anchor: buffer_snapshot.anchor_before(0), // Start of buffer + prefix_offset: 0, + text: "hello".to_string(), + dedent: String::new(), + updates_tx, + }, + ); + + let cursor_position = buffer_snapshot.anchor_after(3); + + let result = find_relevant_completion( + &states, + buffer.entity_id(), + &buffer_snapshot, + cursor_position, + ); + + assert_eq!(result, Some("lo")); + } +} diff --git a/crates/supermaven/src/supermaven_completion_provider.rs b/crates/supermaven/src/supermaven_completion_provider.rs index 2660a03e6f..1b1fc54a7a 100644 --- a/crates/supermaven/src/supermaven_completion_provider.rs +++ b/crates/supermaven/src/supermaven_completion_provider.rs @@ -108,6 +108,14 @@ impl EditPredictionProvider for SupermavenCompletionProvider { } fn show_completions_in_menu() -> bool { + true + } + + fn show_tab_accept_marker() -> bool { + true + } + + fn supports_jump_to_edit() -> bool { false } @@ -116,7 +124,7 @@ impl EditPredictionProvider for SupermavenCompletionProvider { } fn is_refreshing(&self) -> bool { - self.pending_refresh.is_some() + self.pending_refresh.is_some() && self.completion_id.is_none() } fn refresh( @@ -197,6 +205,7 @@ impl EditPredictionProvider for SupermavenCompletionProvider { let mut point = cursor_position.to_point(&snapshot); point.column = snapshot.line_len(point.row); let range = cursor_position..snapshot.anchor_after(point); + Some(completion_from_diff( snapshot, completion_text, From efba2cbfd371bdd85dc3bfdd6b98d1d405ad9a89 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Thu, 7 Aug 2025 17:32:06 +0200 Subject: [PATCH 6/7] chore: Bump Rust to 1.89 (#35788) Release Notes: - N/A --------- Co-authored-by: Julia Ryan --- Dockerfile-collab | 2 +- crates/agent2/src/templates.rs | 4 +++ crates/agent2/src/tools/glob.rs | 4 +++ crates/fs/src/fake_git_repo.rs | 4 +-- crates/git/src/repository.rs | 8 +++--- crates/gpui/src/keymap/context.rs | 30 +++++++++++--------- crates/gpui/src/platform/windows/wrapper.rs | 24 +--------------- crates/terminal_view/src/terminal_element.rs | 2 +- flake.lock | 18 ++++++------ rust-toolchain.toml | 2 +- 10 files changed, 43 insertions(+), 55 deletions(-) diff --git a/Dockerfile-collab b/Dockerfile-collab index 2dafe296c7..c1621d6ee6 100644 --- a/Dockerfile-collab +++ b/Dockerfile-collab @@ -1,6 +1,6 @@ # syntax = docker/dockerfile:1.2 -FROM rust:1.88-bookworm as builder +FROM rust:1.89-bookworm as builder WORKDIR app COPY . . diff --git a/crates/agent2/src/templates.rs b/crates/agent2/src/templates.rs index 7d51a626fc..e634d414d6 100644 --- a/crates/agent2/src/templates.rs +++ b/crates/agent2/src/templates.rs @@ -33,6 +33,10 @@ pub trait Template: Sized { } } +#[expect( + dead_code, + reason = "Marked as unused by Rust 1.89 and left as is as of 07 Aug 2025 to let AI team address it." +)] #[derive(Serialize)] pub struct GlobTemplate { pub project_roots: String, diff --git a/crates/agent2/src/tools/glob.rs b/crates/agent2/src/tools/glob.rs index f44ce9f359..4dace7c074 100644 --- a/crates/agent2/src/tools/glob.rs +++ b/crates/agent2/src/tools/glob.rs @@ -19,6 +19,10 @@ struct GlobInput { glob: SharedString, } +#[expect( + dead_code, + reason = "Marked as unused by Rust 1.89 and left as is as of 07 Aug 2025 to let AI team address it." +)] struct GlobTool { project: Entity, templates: Arc, diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 04ba656232..73da63fd47 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -402,11 +402,11 @@ impl GitRepository for FakeGitRepository { &self, _paths: Vec, _env: Arc>, - ) -> BoxFuture> { + ) -> BoxFuture<'_, Result<()>> { unimplemented!() } - fn stash_pop(&self, _env: Arc>) -> BoxFuture> { + fn stash_pop(&self, _env: Arc>) -> BoxFuture<'_, Result<()>> { unimplemented!() } diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index dc7ab0af65..518b6c4f46 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -399,9 +399,9 @@ pub trait GitRepository: Send + Sync { &self, paths: Vec, env: Arc>, - ) -> BoxFuture>; + ) -> BoxFuture<'_, Result<()>>; - fn stash_pop(&self, env: Arc>) -> BoxFuture>; + fn stash_pop(&self, env: Arc>) -> BoxFuture<'_, Result<()>>; fn push( &self, @@ -1203,7 +1203,7 @@ impl GitRepository for RealGitRepository { &self, paths: Vec, env: Arc>, - ) -> BoxFuture> { + ) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); self.executor .spawn(async move { @@ -1227,7 +1227,7 @@ impl GitRepository for RealGitRepository { .boxed() } - fn stash_pop(&self, env: Arc>) -> BoxFuture> { + fn stash_pop(&self, env: Arc>) -> BoxFuture<'_, Result<()>> { let working_directory = self.working_directory(); self.executor .spawn(async move { diff --git a/crates/gpui/src/keymap/context.rs b/crates/gpui/src/keymap/context.rs index f4b878ae77..281035fe97 100644 --- a/crates/gpui/src/keymap/context.rs +++ b/crates/gpui/src/keymap/context.rs @@ -461,6 +461,8 @@ fn skip_whitespace(source: &str) -> &str { #[cfg(test)] mod tests { + use core::slice; + use super::*; use crate as gpui; use KeyBindingContextPredicate::*; @@ -674,11 +676,11 @@ mod tests { assert!(predicate.eval(&contexts)); assert!(!predicate.eval(&[])); - assert!(!predicate.eval(&[child_context.clone()])); + assert!(!predicate.eval(slice::from_ref(&child_context))); assert!(!predicate.eval(&[parent_context])); let zany_predicate = KeyBindingContextPredicate::parse("child > child").unwrap(); - assert!(!zany_predicate.eval(&[child_context.clone()])); + assert!(!zany_predicate.eval(slice::from_ref(&child_context))); assert!(zany_predicate.eval(&[child_context.clone(), child_context.clone()])); } @@ -690,13 +692,13 @@ mod tests { let parent_context = KeyContext::try_from("parent").unwrap(); let child_context = KeyContext::try_from("child").unwrap(); - assert!(not_predicate.eval(&[workspace_context.clone()])); - assert!(!not_predicate.eval(&[editor_context.clone()])); + assert!(not_predicate.eval(slice::from_ref(&workspace_context))); + assert!(!not_predicate.eval(slice::from_ref(&editor_context))); assert!(!not_predicate.eval(&[editor_context.clone(), workspace_context.clone()])); assert!(!not_predicate.eval(&[workspace_context.clone(), editor_context.clone()])); let complex_not = KeyBindingContextPredicate::parse("!editor && workspace").unwrap(); - assert!(complex_not.eval(&[workspace_context.clone()])); + assert!(complex_not.eval(slice::from_ref(&workspace_context))); assert!(!complex_not.eval(&[editor_context.clone(), workspace_context.clone()])); let not_mode_predicate = KeyBindingContextPredicate::parse("!(mode == full)").unwrap(); @@ -709,18 +711,18 @@ mod tests { assert!(not_mode_predicate.eval(&[other_mode_context])); let not_descendant = KeyBindingContextPredicate::parse("!(parent > child)").unwrap(); - assert!(not_descendant.eval(&[parent_context.clone()])); - assert!(not_descendant.eval(&[child_context.clone()])); + assert!(not_descendant.eval(slice::from_ref(&parent_context))); + assert!(not_descendant.eval(slice::from_ref(&child_context))); assert!(!not_descendant.eval(&[parent_context.clone(), child_context.clone()])); let not_descendant = KeyBindingContextPredicate::parse("parent > !child").unwrap(); - assert!(!not_descendant.eval(&[parent_context.clone()])); - assert!(!not_descendant.eval(&[child_context.clone()])); + assert!(!not_descendant.eval(slice::from_ref(&parent_context))); + assert!(!not_descendant.eval(slice::from_ref(&child_context))); assert!(!not_descendant.eval(&[parent_context.clone(), child_context.clone()])); let double_not = KeyBindingContextPredicate::parse("!!editor").unwrap(); - assert!(double_not.eval(&[editor_context.clone()])); - assert!(!double_not.eval(&[workspace_context.clone()])); + assert!(double_not.eval(slice::from_ref(&editor_context))); + assert!(!double_not.eval(slice::from_ref(&workspace_context))); // Test complex descendant cases let workspace_context = KeyContext::try_from("Workspace").unwrap(); @@ -754,9 +756,9 @@ mod tests { // !Workspace - shouldn't match when Workspace is in the context let not_workspace = KeyBindingContextPredicate::parse("!Workspace").unwrap(); - assert!(!not_workspace.eval(&[workspace_context.clone()])); - assert!(not_workspace.eval(&[pane_context.clone()])); - assert!(not_workspace.eval(&[editor_context.clone()])); + assert!(!not_workspace.eval(slice::from_ref(&workspace_context))); + assert!(not_workspace.eval(slice::from_ref(&pane_context))); + assert!(not_workspace.eval(slice::from_ref(&editor_context))); assert!(!not_workspace.eval(&workspace_pane_editor)); } } diff --git a/crates/gpui/src/platform/windows/wrapper.rs b/crates/gpui/src/platform/windows/wrapper.rs index 6015dffdab..a1fe98a392 100644 --- a/crates/gpui/src/platform/windows/wrapper.rs +++ b/crates/gpui/src/platform/windows/wrapper.rs @@ -1,28 +1,6 @@ use std::ops::Deref; -use windows::Win32::{Foundation::HANDLE, UI::WindowsAndMessaging::HCURSOR}; - -#[derive(Debug, Clone, Copy)] -pub(crate) struct SafeHandle { - raw: HANDLE, -} - -unsafe impl Send for SafeHandle {} -unsafe impl Sync for SafeHandle {} - -impl From for SafeHandle { - fn from(value: HANDLE) -> Self { - SafeHandle { raw: value } - } -} - -impl Deref for SafeHandle { - type Target = HANDLE; - - fn deref(&self) -> &Self::Target { - &self.raw - } -} +use windows::Win32::UI::WindowsAndMessaging::HCURSOR; #[derive(Debug, Clone, Copy)] pub(crate) struct SafeCursor { diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index 083c07de9c..6c1be9d5e7 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/crates/terminal_view/src/terminal_element.rs @@ -136,7 +136,7 @@ impl BatchedTextRun { .shape_line( self.text.clone().into(), self.font_size.to_pixels(window.rem_size()), - &[self.style.clone()], + std::slice::from_ref(&self.style), Some(dimensions.cell_width), ) .paint(pos, dimensions.line_height, window, cx); diff --git a/flake.lock b/flake.lock index fa0d51d90d..80022f7b55 100644 --- a/flake.lock +++ b/flake.lock @@ -2,11 +2,11 @@ "nodes": { "crane": { "locked": { - "lastModified": 1750266157, - "narHash": "sha256-tL42YoNg9y30u7zAqtoGDNdTyXTi8EALDeCB13FtbQA=", + "lastModified": 1754269165, + "narHash": "sha256-0tcS8FHd4QjbCVoxN9jI+PjHgA4vc/IjkUSp+N3zy0U=", "owner": "ipetkov", "repo": "crane", - "rev": "e37c943371b73ed87faf33f7583860f81f1d5a48", + "rev": "444e81206df3f7d92780680e45858e31d2f07a08", "type": "github" }, "original": { @@ -33,10 +33,10 @@ "nixpkgs": { "locked": { "lastModified": 315532800, - "narHash": "sha256-j+zO+IHQ7VwEam0pjPExdbLT2rVioyVS3iq4bLO3GEc=", - "rev": "61c0f513911459945e2cb8bf333dc849f1b976ff", + "narHash": "sha256-5VYevX3GccubYeccRGAXvCPA1ktrGmIX1IFC0icX07g=", + "rev": "a683adc19ff5228af548c6539dbc3440509bfed3", "type": "tarball", - "url": "https://releases.nixos.org/nixpkgs/nixpkgs-25.11pre821324.61c0f5139114/nixexprs.tar.xz" + "url": "https://releases.nixos.org/nixpkgs/nixpkgs-25.11pre840248.a683adc19ff5/nixexprs.tar.xz" }, "original": { "type": "tarball", @@ -58,11 +58,11 @@ ] }, "locked": { - "lastModified": 1750964660, - "narHash": "sha256-YQ6EyFetjH1uy5JhdhRdPe6cuNXlYpMAQePFfZj4W7M=", + "lastModified": 1754575663, + "narHash": "sha256-afOx8AG0KYtw7mlt6s6ahBBy7eEHZwws3iCRoiuRQS4=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "04f0fcfb1a50c63529805a798b4b5c21610ff390", + "rev": "6db0fb0e9cec2e9729dc52bf4898e6c135bb8a0f", "type": "github" }, "original": { diff --git a/rust-toolchain.toml b/rust-toolchain.toml index f80eab8fbc..3d87025a27 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,5 +1,5 @@ [toolchain] -channel = "1.88" +channel = "1.89" profile = "minimal" components = [ "rustfmt", "clippy" ] targets = [ From c1d1d1cff6709b51b9bb3f234dc5c21475b13c57 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Thu, 7 Aug 2025 17:43:37 +0200 Subject: [PATCH 7/7] chore: Bump to taffy 0.9 (#35802) Co-authored-by: Anthony Eid Co-authored-by: Lukas Wirth Co-authored-by: Ben Kunkle Release Notes: - N/A Co-authored-by: Anthony Eid Co-authored-by: Lukas Wirth Co-authored-by: Ben Kunkle --- Cargo.lock | 8 ++++---- crates/gpui/Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e6a0b6c75f..8b4bc8752d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7506,9 +7506,9 @@ dependencies = [ [[package]] name = "grid" -version = "0.17.0" +version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71b01d27060ad58be4663b9e4ac9e2d4806918e8876af8912afbddd1a91d5eaa" +checksum = "12101ecc8225ea6d675bc70263074eab6169079621c2186fe0c66590b2df9681" [[package]] name = "group" @@ -16219,9 +16219,9 @@ dependencies = [ [[package]] name = "taffy" -version = "0.8.3" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7aaef0ac998e6527d6d0d5582f7e43953bb17221ac75bb8eb2fcc2db3396db1c" +checksum = "a13e5d13f79d558b5d353a98072ca8ca0e99da429467804de959aa8c83c9a004" dependencies = [ "arrayvec", "grid", diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index 2bf49fa7d8..6e5a76d441 100644 --- a/crates/gpui/Cargo.toml +++ b/crates/gpui/Cargo.toml @@ -121,7 +121,7 @@ smallvec.workspace = true smol.workspace = true strum.workspace = true sum_tree.workspace = true -taffy = "=0.8.3" +taffy = "=0.9.0" thiserror.workspace = true util.workspace = true uuid.workspace = true