From baefec384975c00fd7f7caeada277a41944231c5 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 14 Jun 2025 01:33:02 +0300 Subject: [PATCH] Move r-a status into the activity indicator (#32726) Deals with the noisy pop-ups by moving r-a **status messages** into the activity indicator, where the rest of the LSP statuses is displayed. https://github.com/user-attachments/assets/e16fb374-d34d-4d03-b5f1-41f71f61c7c7 https://github.com/user-attachments/assets/67c611aa-8b73-4adb-a76d-b0c8ce3e2f94 Release Notes: - N/A *or* Added/Fixed/Improved ... --- .../src/activity_indicator.rs | 234 +++++++++++++----- .../src/extension_store_test.rs | 19 +- crates/language/src/language.rs | 4 +- crates/language/src/language_registry.rs | 38 ++- .../src/extension_lsp_adapter.rs | 10 +- crates/project/src/lsp_store.rs | 7 +- .../src/lsp_store/rust_analyzer_ext.rs | 71 ++---- 7 files changed, 247 insertions(+), 136 deletions(-) diff --git a/crates/activity_indicator/src/activity_indicator.rs b/crates/activity_indicator/src/activity_indicator.rs index 86d60d2640..24762cb727 100644 --- a/crates/activity_indicator/src/activity_indicator.rs +++ b/crates/activity_indicator/src/activity_indicator.rs @@ -7,7 +7,10 @@ use gpui::{ InteractiveElement as _, ParentElement as _, Render, SharedString, StatefulInteractiveElement, Styled, Transformation, Window, actions, percentage, }; -use language::{BinaryStatus, LanguageRegistry, LanguageServerId}; +use language::{ + BinaryStatus, LanguageRegistry, LanguageServerId, LanguageServerName, + LanguageServerStatusUpdate, ServerHealth, +}; use project::{ EnvironmentErrorMessage, LanguageServerProgress, LspStoreEvent, Project, ProjectEnvironmentEvent, @@ -16,6 +19,7 @@ use project::{ use smallvec::SmallVec; use std::{ cmp::Reverse, + collections::HashSet, fmt::Write, path::Path, sync::Arc, @@ -30,9 +34,9 @@ const GIT_OPERATION_DELAY: Duration = Duration::from_millis(0); actions!(activity_indicator, [ShowErrorMessage]); pub enum Event { - ShowError { - server_name: SharedString, - error: String, + ShowStatus { + server_name: LanguageServerName, + status: SharedString, }, } @@ -45,8 +49,8 @@ pub struct ActivityIndicator { #[derive(Debug)] struct ServerStatus { - name: SharedString, - status: BinaryStatus, + name: LanguageServerName, + status: LanguageServerStatusUpdate, } struct PendingWork<'a> { @@ -145,19 +149,19 @@ impl ActivityIndicator { }); cx.subscribe_in(&this, window, move |_, _, event, window, cx| match event { - Event::ShowError { server_name, error } => { + Event::ShowStatus { + server_name, + status, + } => { let create_buffer = project.update(cx, |project, cx| project.create_buffer(cx)); let project = project.clone(); - let error = error.clone(); + let status = status.clone(); let server_name = server_name.clone(); cx.spawn_in(window, async move |workspace, cx| { let buffer = create_buffer.await?; buffer.update(cx, |buffer, cx| { buffer.edit( - [( - 0..0, - format!("Language server error: {}\n\n{}", server_name, error), - )], + [(0..0, format!("Language server {server_name}:\n\n{status}"))], None, cx, ); @@ -166,7 +170,10 @@ impl ActivityIndicator { workspace.update_in(cx, |workspace, window, cx| { workspace.add_item_to_active_pane( Box::new(cx.new(|cx| { - Editor::for_buffer(buffer, Some(project.clone()), window, cx) + let mut editor = + Editor::for_buffer(buffer, Some(project.clone()), window, cx); + editor.set_read_only(true); + editor })), None, true, @@ -185,19 +192,34 @@ impl ActivityIndicator { } fn show_error_message(&mut self, _: &ShowErrorMessage, _: &mut Window, cx: &mut Context) { - self.statuses.retain(|status| { - if let BinaryStatus::Failed { error } = &status.status { - cx.emit(Event::ShowError { + let mut status_message_shown = false; + self.statuses.retain(|status| match &status.status { + LanguageServerStatusUpdate::Binary(BinaryStatus::Failed { error }) + if !status_message_shown => + { + cx.emit(Event::ShowStatus { server_name: status.name.clone(), - error: error.clone(), + status: SharedString::from(error), }); + status_message_shown = true; false - } else { - true } + LanguageServerStatusUpdate::Health( + ServerHealth::Error | ServerHealth::Warning, + status_string, + ) if !status_message_shown => match status_string { + Some(error) => { + cx.emit(Event::ShowStatus { + server_name: status.name.clone(), + status: error.clone(), + }); + status_message_shown = true; + false + } + None => false, + }, + _ => true, }); - - cx.notify(); } fn dismiss_error_message( @@ -267,48 +289,52 @@ impl ActivityIndicator { }); } // Show any language server has pending activity. - let mut pending_work = self.pending_language_server_work(cx); - if let Some(PendingWork { - progress_token, - progress, - .. - }) = pending_work.next() { - let mut message = progress - .title - .as_deref() - .unwrap_or(progress_token) - .to_string(); + let mut pending_work = self.pending_language_server_work(cx); + if let Some(PendingWork { + progress_token, + progress, + .. + }) = pending_work.next() + { + let mut message = progress + .title + .as_deref() + .unwrap_or(progress_token) + .to_string(); - if let Some(percentage) = progress.percentage { - write!(&mut message, " ({}%)", percentage).unwrap(); + if let Some(percentage) = progress.percentage { + write!(&mut message, " ({}%)", percentage).unwrap(); + } + + if let Some(progress_message) = progress.message.as_ref() { + message.push_str(": "); + message.push_str(progress_message); + } + + let additional_work_count = pending_work.count(); + if additional_work_count > 0 { + write!(&mut message, " + {} more", additional_work_count).unwrap(); + } + + return Some(Content { + icon: Some( + Icon::new(IconName::ArrowCircle) + .size(IconSize::Small) + .with_animation( + "arrow-circle", + Animation::new(Duration::from_secs(2)).repeat(), + |icon, delta| { + icon.transform(Transformation::rotate(percentage(delta))) + }, + ) + .into_any_element(), + ), + message, + on_click: Some(Arc::new(Self::toggle_language_server_work_context_menu)), + tooltip_message: None, + }); } - - if let Some(progress_message) = progress.message.as_ref() { - message.push_str(": "); - message.push_str(progress_message); - } - - let additional_work_count = pending_work.count(); - if additional_work_count > 0 { - write!(&mut message, " + {} more", additional_work_count).unwrap(); - } - - return Some(Content { - icon: Some( - Icon::new(IconName::ArrowCircle) - .size(IconSize::Small) - .with_animation( - "arrow-circle", - Animation::new(Duration::from_secs(2)).repeat(), - |icon, delta| icon.transform(Transformation::rotate(percentage(delta))), - ) - .into_any_element(), - ), - message, - on_click: Some(Arc::new(Self::toggle_language_server_work_context_menu)), - tooltip_message: None, - }); } if let Some(session) = self @@ -369,14 +395,38 @@ impl ActivityIndicator { let mut downloading = SmallVec::<[_; 3]>::new(); let mut checking_for_update = SmallVec::<[_; 3]>::new(); let mut failed = SmallVec::<[_; 3]>::new(); + let mut health_messages = SmallVec::<[_; 3]>::new(); + let mut servers_to_clear_statuses = HashSet::::default(); for status in &self.statuses { - match status.status { - BinaryStatus::CheckingForUpdate => checking_for_update.push(status.name.clone()), - BinaryStatus::Downloading => downloading.push(status.name.clone()), - BinaryStatus::Failed { .. } => failed.push(status.name.clone()), - BinaryStatus::None => {} + match &status.status { + LanguageServerStatusUpdate::Binary(BinaryStatus::CheckingForUpdate) => { + checking_for_update.push(status.name.clone()); + } + LanguageServerStatusUpdate::Binary(BinaryStatus::Downloading) => { + downloading.push(status.name.clone()); + } + LanguageServerStatusUpdate::Binary(BinaryStatus::Failed { .. }) => { + failed.push(status.name.clone()); + } + LanguageServerStatusUpdate::Binary(BinaryStatus::None) => {} + LanguageServerStatusUpdate::Health(health, server_status) => match server_status { + Some(server_status) => { + health_messages.push((status.name.clone(), *health, server_status.clone())); + } + None => { + servers_to_clear_statuses.insert(status.name.clone()); + } + }, } } + self.statuses + .retain(|status| !servers_to_clear_statuses.contains(&status.name)); + + health_messages.sort_by_key(|(_, health, _)| match health { + ServerHealth::Error => 2, + ServerHealth::Warning => 1, + ServerHealth::Ok => 0, + }); if !downloading.is_empty() { return Some(Content { @@ -457,7 +507,7 @@ impl ActivityIndicator { }), ), on_click: Some(Arc::new(|this, window, cx| { - this.show_error_message(&Default::default(), window, cx) + this.show_error_message(&ShowErrorMessage, window, cx) })), tooltip_message: None, }); @@ -471,7 +521,7 @@ impl ActivityIndicator { .size(IconSize::Small) .into_any_element(), ), - message: format!("Formatting failed: {}. Click to see logs.", failure), + message: format!("Formatting failed: {failure}. Click to see logs."), on_click: Some(Arc::new(|indicator, window, cx| { indicator.project.update(cx, |project, cx| { project.reset_last_formatting_failure(cx); @@ -482,6 +532,56 @@ impl ActivityIndicator { }); } + // Show any health messages for the language servers + if let Some((server_name, health, message)) = health_messages.pop() { + let health_str = match health { + ServerHealth::Ok => format!("({server_name}) "), + ServerHealth::Warning => format!("({server_name}) Warning: "), + ServerHealth::Error => format!("({server_name}) Error: "), + }; + let single_line_message = message + .lines() + .filter_map(|line| { + let line = line.trim(); + if line.is_empty() { None } else { Some(line) } + }) + .collect::>() + .join(" "); + let mut altered_message = single_line_message != message; + let truncated_message = truncate_and_trailoff( + &single_line_message, + MAX_MESSAGE_LEN.saturating_sub(health_str.len()), + ); + altered_message |= truncated_message != single_line_message; + let final_message = format!("{health_str}{truncated_message}"); + + let tooltip_message = if altered_message { + Some(format!("{health_str}{message}")) + } else { + None + }; + + return Some(Content { + icon: Some( + Icon::new(IconName::Warning) + .size(IconSize::Small) + .into_any_element(), + ), + message: final_message, + tooltip_message, + on_click: Some(Arc::new(move |activity_indicator, window, cx| { + if altered_message { + activity_indicator.show_error_message(&ShowErrorMessage, window, cx) + } else { + activity_indicator + .statuses + .retain(|status| status.name != server_name); + cx.notify(); + } + })), + }); + } + // Show any application auto-update info. if let Some(updater) = &self.auto_updater { return match &updater.read(cx).status() { diff --git a/crates/extension_host/src/extension_store_test.rs b/crates/extension_host/src/extension_store_test.rs index 43dd130fe2..fd91080a5c 100644 --- a/crates/extension_host/src/extension_store_test.rs +++ b/crates/extension_host/src/extension_store_test.rs @@ -8,9 +8,9 @@ use collections::BTreeMap; use extension::ExtensionHostProxy; use fs::{FakeFs, Fs, RealFs}; use futures::{AsyncReadExt, StreamExt, io::BufReader}; -use gpui::{AppContext as _, SemanticVersion, SharedString, TestAppContext}; +use gpui::{AppContext as _, SemanticVersion, TestAppContext}; use http_client::{FakeHttpClient, Response}; -use language::{BinaryStatus, LanguageMatcher, LanguageRegistry}; +use language::{BinaryStatus, LanguageMatcher, LanguageRegistry, LanguageServerStatusUpdate}; use lsp::LanguageServerName; use node_runtime::NodeRuntime; use parking_lot::Mutex; @@ -717,9 +717,18 @@ async fn test_extension_store_with_test_extension(cx: &mut TestAppContext) { status_updates.next().await.unwrap(), ], [ - (SharedString::new("gleam"), BinaryStatus::CheckingForUpdate), - (SharedString::new("gleam"), BinaryStatus::Downloading), - (SharedString::new("gleam"), BinaryStatus::None) + ( + LanguageServerName::new_static("gleam"), + LanguageServerStatusUpdate::Binary(BinaryStatus::CheckingForUpdate) + ), + ( + LanguageServerName::new_static("gleam"), + LanguageServerStatusUpdate::Binary(BinaryStatus::Downloading) + ), + ( + LanguageServerName::new_static("gleam"), + LanguageServerStatusUpdate::Binary(BinaryStatus::None) + ) ] ); diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index f4c90c64e7..549eedc05a 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -32,7 +32,9 @@ use futures::Future; use gpui::{App, AsyncApp, Entity, SharedString, Task}; pub use highlight_map::HighlightMap; use http_client::HttpClient; -pub use language_registry::{LanguageName, LoadedLanguage}; +pub use language_registry::{ + LanguageName, LanguageServerStatusUpdate, LoadedLanguage, ServerHealth, +}; use lsp::{CodeActionKind, InitializeParams, LanguageServerBinary, LanguageServerBinaryOptions}; pub use manifest::{ManifestDelegate, ManifestName, ManifestProvider, ManifestQuery}; use parking_lot::Mutex; diff --git a/crates/language/src/language_registry.rs b/crates/language/src/language_registry.rs index 6a515e0166..4d0837d8e3 100644 --- a/crates/language/src/language_registry.rs +++ b/crates/language/src/language_registry.rs @@ -107,7 +107,7 @@ pub struct LanguageRegistry { state: RwLock, language_server_download_dir: Option>, executor: BackgroundExecutor, - lsp_binary_status_tx: BinaryStatusSender, + lsp_binary_status_tx: ServerStatusSender, } struct LanguageRegistryState { @@ -138,6 +138,20 @@ pub struct FakeLanguageServerEntry { pub _server: Option, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum LanguageServerStatusUpdate { + Binary(BinaryStatus), + Health(ServerHealth, Option), +} + +#[derive(Debug, PartialEq, Eq, Deserialize, Serialize, Clone, Copy)] +#[serde(rename_all = "camelCase")] +pub enum ServerHealth { + Ok, + Warning, + Error, +} + #[derive(Clone, Debug, PartialEq, Eq)] pub enum BinaryStatus { None, @@ -233,8 +247,8 @@ pub struct LanguageQueries { } #[derive(Clone, Default)] -struct BinaryStatusSender { - txs: Arc>>>, +struct ServerStatusSender { + txs: Arc>>>, } pub struct LoadedLanguage { @@ -1071,8 +1085,12 @@ impl LanguageRegistry { self.state.read().all_lsp_adapters.get(name).cloned() } - pub fn update_lsp_status(&self, server_name: LanguageServerName, status: BinaryStatus) { - self.lsp_binary_status_tx.send(server_name.0, status); + pub fn update_lsp_status( + &self, + server_name: LanguageServerName, + status: LanguageServerStatusUpdate, + ) { + self.lsp_binary_status_tx.send(server_name, status); } pub fn next_language_server_id(&self) -> LanguageServerId { @@ -1127,7 +1145,7 @@ impl LanguageRegistry { pub fn language_server_binary_statuses( &self, - ) -> mpsc::UnboundedReceiver<(SharedString, BinaryStatus)> { + ) -> mpsc::UnboundedReceiver<(LanguageServerName, LanguageServerStatusUpdate)> { self.lsp_binary_status_tx.subscribe() } @@ -1241,14 +1259,16 @@ impl LanguageRegistryState { } } -impl BinaryStatusSender { - fn subscribe(&self) -> mpsc::UnboundedReceiver<(SharedString, BinaryStatus)> { +impl ServerStatusSender { + fn subscribe( + &self, + ) -> mpsc::UnboundedReceiver<(LanguageServerName, LanguageServerStatusUpdate)> { let (tx, rx) = mpsc::unbounded(); self.txs.lock().push(tx); rx } - fn send(&self, name: SharedString, status: BinaryStatus) { + fn send(&self, name: LanguageServerName, status: LanguageServerStatusUpdate) { let mut txs = self.txs.lock(); txs.retain(|tx| tx.unbounded_send((name.clone(), status.clone())).is_ok()); } diff --git a/crates/language_extension/src/extension_lsp_adapter.rs b/crates/language_extension/src/extension_lsp_adapter.rs index ae325edbd8..e6c0b2b796 100644 --- a/crates/language_extension/src/extension_lsp_adapter.rs +++ b/crates/language_extension/src/extension_lsp_adapter.rs @@ -12,8 +12,8 @@ use fs::Fs; use futures::{Future, FutureExt}; use gpui::AsyncApp; use language::{ - BinaryStatus, CodeLabel, HighlightId, Language, LanguageName, LanguageToolchainStore, - LspAdapter, LspAdapterDelegate, + BinaryStatus, CodeLabel, HighlightId, Language, LanguageName, LanguageServerStatusUpdate, + LanguageToolchainStore, LspAdapter, LspAdapterDelegate, }; use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerBinaryOptions, LanguageServerName}; use serde::Serialize; @@ -82,8 +82,10 @@ impl ExtensionLanguageServerProxy for LanguageServerRegistryProxy { language_server_id: LanguageServerName, status: BinaryStatus, ) { - self.language_registry - .update_lsp_status(language_server_id, status); + self.language_registry.update_lsp_status( + language_server_id, + LanguageServerStatusUpdate::Binary(status), + ); } } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 0224fec863..d583257859 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -41,8 +41,9 @@ use itertools::Itertools as _; use language::{ Bias, BinaryStatus, Buffer, BufferSnapshot, CachedLspAdapter, CodeLabel, Diagnostic, DiagnosticEntry, DiagnosticSet, DiagnosticSourceKind, Diff, File as _, Language, LanguageName, - LanguageRegistry, LanguageToolchainStore, LocalFile, LspAdapter, LspAdapterDelegate, Patch, - PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, Unclipped, + LanguageRegistry, LanguageServerStatusUpdate, LanguageToolchainStore, LocalFile, LspAdapter, + LspAdapterDelegate, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, + Unclipped, language_settings::{ FormatOnSave, Formatter, LanguageSettings, SelectedFormatter, language_settings, }, @@ -10744,7 +10745,7 @@ impl LspAdapterDelegate for LocalLspAdapterDelegate { fn update_status(&self, server_name: LanguageServerName, status: language::BinaryStatus) { self.language_registry - .update_lsp_status(server_name, status); + .update_lsp_status(server_name, LanguageServerStatusUpdate::Binary(status)); } fn registered_lsp_adapters(&self) -> Vec> { diff --git a/crates/project/src/lsp_store/rust_analyzer_ext.rs b/crates/project/src/lsp_store/rust_analyzer_ext.rs index deea5d5dd7..78401ac797 100644 --- a/crates/project/src/lsp_store/rust_analyzer_ext.rs +++ b/crates/project/src/lsp_store/rust_analyzer_ext.rs @@ -1,12 +1,11 @@ use ::serde::{Deserialize, Serialize}; use anyhow::Context as _; -use gpui::{App, Entity, PromptLevel, Task, WeakEntity}; +use gpui::{App, Entity, SharedString, Task, WeakEntity}; +use language::{LanguageServerStatusUpdate, ServerHealth}; use lsp::LanguageServer; use rpc::proto; -use crate::{ - LanguageServerPromptRequest, LspStore, LspStoreEvent, Project, ProjectPath, lsp_store, -}; +use crate::{LspStore, Project, ProjectPath, lsp_store}; pub const RUST_ANALYZER_NAME: &str = "rust-analyzer"; pub const CARGO_DIAGNOSTICS_SOURCE_NAME: &str = "rustc"; @@ -17,20 +16,10 @@ pub const CARGO_DIAGNOSTICS_SOURCE_NAME: &str = "rustc"; #[derive(Debug)] enum ServerStatus {} -/// Other(String) variant to handle unknown values due to this still being experimental -#[derive(Debug, PartialEq, Deserialize, Serialize, Clone)] -#[serde(rename_all = "camelCase")] -enum ServerHealthStatus { - Ok, - Warning, - Error, - Other(String), -} - #[derive(Debug, PartialEq, Deserialize, Serialize, Clone)] #[serde(rename_all = "camelCase")] struct ServerStatusParams { - pub health: ServerHealthStatus, + pub health: ServerHealth, pub message: Option, } @@ -45,40 +34,28 @@ pub fn register_notifications(lsp_store: WeakEntity, language_server: language_server .on_notification::({ - let name = name.to_string(); + let name = name.clone(); move |params, cx| { - let name = name.to_string(); - if let Some(ref message) = params.message { - let message = message.trim(); - if !message.is_empty() { - let formatted_message = format!( - "Language server {name} (id {server_id}) status update: {message}" - ); - match params.health { - ServerHealthStatus::Ok => log::info!("{formatted_message}"), - ServerHealthStatus::Warning => log::warn!("{formatted_message}"), - ServerHealthStatus::Error => { - log::error!("{formatted_message}"); - let (tx, _rx) = smol::channel::bounded(1); - let request = LanguageServerPromptRequest { - level: PromptLevel::Critical, - message: params.message.unwrap_or_default(), - actions: Vec::new(), - response_channel: tx, - lsp_name: name.clone(), - }; - lsp_store - .update(cx, |_, cx| { - cx.emit(LspStoreEvent::LanguageServerPrompt(request)); - }) - .ok(); - } - ServerHealthStatus::Other(status) => { - log::info!("Unknown server health: {status}\n{formatted_message}") - } - } - } + let status = params.message; + let log_message = + format!("Language server {name} (id {server_id}) status update: {status:?}"); + match ¶ms.health { + ServerHealth::Ok => log::info!("{log_message}"), + ServerHealth::Warning => log::warn!("{log_message}"), + ServerHealth::Error => log::error!("{log_message}"), } + + lsp_store + .update(cx, |lsp_store, _| { + lsp_store.languages.update_lsp_status( + name.clone(), + LanguageServerStatusUpdate::Health( + params.health, + status.map(SharedString::from), + ), + ); + }) + .ok(); } }) .detach();