Move r-a status into the activity indicator (#32726)

Kirill Bulatov created

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 ...

Change summary

crates/activity_indicator/src/activity_indicator.rs    | 232 ++++++++---
crates/extension_host/src/extension_store_test.rs      |  19 
crates/language/src/language.rs                        |   4 
crates/language/src/language_registry.rs               |  38 +
crates/language_extension/src/extension_lsp_adapter.rs |  10 
crates/project/src/lsp_store.rs                        |   7 
crates/project/src/lsp_store/rust_analyzer_ext.rs      |  71 +--
7 files changed, 246 insertions(+), 135 deletions(-)

Detailed changes

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>) {
-        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();
-
-            if let Some(percentage) = progress.percentage {
-                write!(&mut message, " ({}%)", percentage).unwrap();
-            }
+            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(progress_message) = progress.message.as_ref() {
-                message.push_str(": ");
-                message.push_str(progress_message);
-            }
+                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();
-            }
+                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,
-            });
+                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::<LanguageServerName>::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::<Vec<_>>()
+                .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() {

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)
+            )
         ]
     );
 

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;

crates/language/src/language_registry.rs 🔗

@@ -107,7 +107,7 @@ pub struct LanguageRegistry {
     state: RwLock<LanguageRegistryState>,
     language_server_download_dir: Option<Arc<Path>>,
     executor: BackgroundExecutor,
-    lsp_binary_status_tx: BinaryStatusSender,
+    lsp_binary_status_tx: ServerStatusSender,
 }
 
 struct LanguageRegistryState {
@@ -138,6 +138,20 @@ pub struct FakeLanguageServerEntry {
     pub _server: Option<lsp::FakeLanguageServer>,
 }
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum LanguageServerStatusUpdate {
+    Binary(BinaryStatus),
+    Health(ServerHealth, Option<SharedString>),
+}
+
+#[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<Mutex<Vec<mpsc::UnboundedSender<(SharedString, BinaryStatus)>>>>,
+struct ServerStatusSender {
+    txs: Arc<Mutex<Vec<mpsc::UnboundedSender<(LanguageServerName, LanguageServerStatusUpdate)>>>>,
 }
 
 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());
     }

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),
+        );
     }
 }
 

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<Arc<dyn LspAdapter>> {

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<String>,
 }
 
@@ -45,40 +34,28 @@ pub fn register_notifications(lsp_store: WeakEntity<LspStore>, language_server:
 
     language_server
         .on_notification::<ServerStatus, _>({
-            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 &params.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();