diff --git a/crates/activity_indicator/src/activity_indicator.rs b/crates/activity_indicator/src/activity_indicator.rs index 84d1291dad..09cc2fb956 100644 --- a/crates/activity_indicator/src/activity_indicator.rs +++ b/crates/activity_indicator/src/activity_indicator.rs @@ -11,7 +11,7 @@ use language::{ LanguageServerStatusUpdate, ServerHealth, }; use project::{ - LanguageServerProgress, LspStoreEvent, Project, ProjectEnvironmentEvent, + LanguageServerProgress, LspStoreEvent, ProgressToken, Project, ProjectEnvironmentEvent, git_store::{GitStoreEvent, Repository}, }; use smallvec::SmallVec; @@ -61,7 +61,7 @@ struct ServerStatus { struct PendingWork<'a> { language_server_id: LanguageServerId, - progress_token: &'a str, + progress_token: &'a ProgressToken, progress: &'a LanguageServerProgress, } @@ -313,9 +313,9 @@ impl ActivityIndicator { let mut pending_work = status .pending_work .iter() - .map(|(token, progress)| PendingWork { + .map(|(progress_token, progress)| PendingWork { language_server_id: server_id, - progress_token: token.as_str(), + progress_token, progress, }) .collect::>(); @@ -358,11 +358,7 @@ impl ActivityIndicator { .. }) = pending_work.next() { - let mut message = progress - .title - .as_deref() - .unwrap_or(progress_token) - .to_string(); + let mut message = progress.title.clone().unwrap_or(progress_token.to_string()); if let Some(percentage) = progress.percentage { write!(&mut message, " ({}%)", percentage).unwrap(); @@ -773,7 +769,7 @@ impl Render for ActivityIndicator { let Some(content) = self.content_to_render(cx) else { return result; }; - let this = cx.entity().downgrade(); + let activity_indicator = cx.entity().downgrade(); let truncate_content = content.message.len() > MAX_MESSAGE_LEN; result.gap_2().child( PopoverMenu::new("activity-indicator-popover") @@ -815,22 +811,21 @@ impl Render for ActivityIndicator { ) .anchor(gpui::Corner::BottomLeft) .menu(move |window, cx| { - let strong_this = this.upgrade()?; + let strong_this = activity_indicator.upgrade()?; let mut has_work = false; let menu = ContextMenu::build(window, cx, |mut menu, _, cx| { for work in strong_this.read(cx).pending_language_server_work(cx) { has_work = true; - let this = this.clone(); + let activity_indicator = activity_indicator.clone(); let mut title = work .progress .title - .as_deref() - .unwrap_or(work.progress_token) - .to_owned(); + .clone() + .unwrap_or(work.progress_token.to_string()); if work.progress.is_cancellable { let language_server_id = work.language_server_id; - let token = work.progress_token.to_string(); + let token = work.progress_token.clone(); let title = SharedString::from(title); menu = menu.custom_entry( move |_, _| { @@ -842,18 +837,23 @@ impl Render for ActivityIndicator { .into_any_element() }, move |_, cx| { - this.update(cx, |this, cx| { - this.project.update(cx, |project, cx| { - project.cancel_language_server_work( - language_server_id, - Some(token.clone()), + let token = token.clone(); + activity_indicator + .update(cx, |activity_indicator, cx| { + activity_indicator.project.update( cx, + |project, cx| { + project.cancel_language_server_work( + language_server_id, + Some(token), + cx, + ); + }, ); - }); - this.context_menu_handle.hide(cx); - cx.notify(); - }) - .ok(); + activity_indicator.context_menu_handle.hide(cx); + cx.notify(); + }) + .ok(); }, ); } else { diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index 0cd18c049e..73fdd8da88 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/crates/collab/src/tests/editor_tests.rs @@ -17,12 +17,14 @@ use editor::{ use fs::Fs; use futures::{SinkExt, StreamExt, channel::mpsc, lock::Mutex}; use git::repository::repo_path; -use gpui::{App, Rgba, TestAppContext, UpdateGlobal, VisualContext, VisualTestContext}; +use gpui::{ + App, Rgba, SharedString, TestAppContext, UpdateGlobal, VisualContext, VisualTestContext, +}; use indoc::indoc; use language::FakeLspAdapter; use lsp::LSP_REQUEST_TIMEOUT; use project::{ - ProjectPath, SERVER_PROGRESS_THROTTLE_TIMEOUT, + ProgressToken, ProjectPath, SERVER_PROGRESS_THROTTLE_TIMEOUT, lsp_store::lsp_ext_command::{ExpandedMacro, LspExtExpandMacro}, }; use recent_projects::disconnected_overlay::DisconnectedOverlay; @@ -1283,12 +1285,14 @@ async fn test_language_server_statuses(cx_a: &mut TestAppContext, cx_b: &mut Tes }); executor.run_until_parked(); + let token = ProgressToken::String(SharedString::from("the-token")); + project_a.read_with(cx_a, |project, cx| { let status = project.language_server_statuses(cx).next().unwrap().1; assert_eq!(status.name.0, "the-language-server"); assert_eq!(status.pending_work.len(), 1); assert_eq!( - status.pending_work["the-token"].message.as_ref().unwrap(), + status.pending_work[&token].message.as_ref().unwrap(), "the-message" ); }); @@ -1322,7 +1326,7 @@ async fn test_language_server_statuses(cx_a: &mut TestAppContext, cx_b: &mut Tes assert_eq!(status.name.0, "the-language-server"); assert_eq!(status.pending_work.len(), 1); assert_eq!( - status.pending_work["the-token"].message.as_ref().unwrap(), + status.pending_work[&token].message.as_ref().unwrap(), "the-message-2" ); }); @@ -1332,7 +1336,7 @@ async fn test_language_server_statuses(cx_a: &mut TestAppContext, cx_b: &mut Tes assert_eq!(status.name.0, "the-language-server"); assert_eq!(status.pending_work.len(), 1); assert_eq!( - status.pending_work["the-token"].message.as_ref().unwrap(), + status.pending_work[&token].message.as_ref().unwrap(), "the-message-2" ); }); diff --git a/crates/editor/src/inlays/inlay_hints.rs b/crates/editor/src/inlays/inlay_hints.rs index d0d92cbce6..74fe998876 100644 --- a/crates/editor/src/inlays/inlay_hints.rs +++ b/crates/editor/src/inlays/inlay_hints.rs @@ -1181,17 +1181,17 @@ pub mod tests { }) .unwrap(); - let progress_token = "test_progress_token"; + let progress_token = 42; fake_server .request::(lsp::WorkDoneProgressCreateParams { - token: lsp::ProgressToken::String(progress_token.to_string()), + token: lsp::ProgressToken::Number(progress_token), }) .await .into_response() .expect("work done progress create request failed"); cx.executor().run_until_parked(); fake_server.notify::(lsp::ProgressParams { - token: lsp::ProgressToken::String(progress_token.to_string()), + token: lsp::ProgressToken::Number(progress_token), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::Begin( lsp::WorkDoneProgressBegin::default(), )), @@ -1211,7 +1211,7 @@ pub mod tests { .unwrap(); fake_server.notify::(lsp::ProgressParams { - token: lsp::ProgressToken::String(progress_token.to_string()), + token: lsp::ProgressToken::Number(progress_token), value: lsp::ProgressParamsValue::WorkDone(lsp::WorkDoneProgress::End( lsp::WorkDoneProgressEnd::default(), )), diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 3263395f40..762070796f 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -138,6 +138,54 @@ pub use worktree::{ const SERVER_LAUNCHING_BEFORE_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); pub const SERVER_PROGRESS_THROTTLE_TIMEOUT: Duration = Duration::from_millis(100); +const WORKSPACE_DIAGNOSTICS_TOKEN_START: &str = "id:"; + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize)] +pub enum ProgressToken { + Number(i32), + String(SharedString), +} + +impl std::fmt::Display for ProgressToken { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Number(number) => write!(f, "{number}"), + Self::String(string) => write!(f, "{string}"), + } + } +} + +impl ProgressToken { + fn from_lsp(value: lsp::NumberOrString) -> Self { + match value { + lsp::NumberOrString::Number(number) => Self::Number(number), + lsp::NumberOrString::String(string) => Self::String(SharedString::new(string)), + } + } + + fn to_lsp(&self) -> lsp::NumberOrString { + match self { + Self::Number(number) => lsp::NumberOrString::Number(*number), + Self::String(string) => lsp::NumberOrString::String(string.to_string()), + } + } + + fn from_proto(value: proto::ProgressToken) -> Option { + Some(match value.value? { + proto::progress_token::Value::Number(number) => Self::Number(number), + proto::progress_token::Value::String(string) => Self::String(SharedString::new(string)), + }) + } + + fn to_proto(&self) -> proto::ProgressToken { + proto::ProgressToken { + value: Some(match self { + Self::Number(number) => proto::progress_token::Value::Number(*number), + Self::String(string) => proto::progress_token::Value::String(string.to_string()), + }), + } + } +} #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum FormatTrigger { @@ -712,9 +760,10 @@ impl LocalLspStore { async move { this.update(&mut cx, |this, _| { if let Some(status) = this.language_server_statuses.get_mut(&server_id) - && let lsp::NumberOrString::String(token) = params.token { - status.progress_tokens.insert(token); + status + .progress_tokens + .insert(ProgressToken::from_lsp(params.token)); } })?; @@ -3632,9 +3681,9 @@ pub enum LspStoreEvent { #[derive(Clone, Debug, Serialize)] pub struct LanguageServerStatus { pub name: LanguageServerName, - pub pending_work: BTreeMap, + pub pending_work: BTreeMap, pub has_pending_diagnostic_updates: bool, - progress_tokens: HashSet, + progress_tokens: HashSet, pub worktree: Option, } @@ -4484,7 +4533,7 @@ impl LspStore { this.update(cx, |this, cx| { this.on_lsp_work_start( language_server.server_id(), - id.to_string(), + ProgressToken::Number(id), LanguageServerProgress { is_disk_based_diagnostics_progress: false, is_cancellable: false, @@ -4502,7 +4551,11 @@ impl LspStore { Some(defer(|| { cx.update(|cx| { this.update(cx, |this, cx| { - this.on_lsp_work_end(language_server.server_id(), id.to_string(), cx); + this.on_lsp_work_end( + language_server.server_id(), + ProgressToken::Number(id), + cx, + ); }) }) .log_err(); @@ -8925,7 +8978,8 @@ impl LspStore { proto::update_language_server::Variant::WorkStart(payload) => { lsp_store.on_lsp_work_start( language_server_id, - payload.token, + ProgressToken::from_proto(payload.token.context("missing progress token")?) + .context("invalid progress token value")?, LanguageServerProgress { title: payload.title, is_disk_based_diagnostics_progress: false, @@ -8940,7 +8994,8 @@ impl LspStore { proto::update_language_server::Variant::WorkProgress(payload) => { lsp_store.on_lsp_work_progress( language_server_id, - payload.token, + ProgressToken::from_proto(payload.token.context("missing progress token")?) + .context("invalid progress token value")?, LanguageServerProgress { title: None, is_disk_based_diagnostics_progress: false, @@ -8954,7 +9009,12 @@ impl LspStore { } proto::update_language_server::Variant::WorkEnd(payload) => { - lsp_store.on_lsp_work_end(language_server_id, payload.token, cx); + lsp_store.on_lsp_work_end( + language_server_id, + ProgressToken::from_proto(payload.token.context("missing progress token")?) + .context("invalid progress token value")?, + cx, + ); } proto::update_language_server::Variant::DiskBasedDiagnosticsUpdating(_) => { @@ -9347,31 +9407,28 @@ impl LspStore { fn on_lsp_progress( &mut self, - progress: lsp::ProgressParams, + progress_params: lsp::ProgressParams, language_server_id: LanguageServerId, disk_based_diagnostics_progress_token: Option, cx: &mut Context, ) { - let token = match progress.token { - lsp::NumberOrString::String(token) => token, - lsp::NumberOrString::Number(token) => { - log::info!("skipping numeric progress token {}", token); - return; - } - }; - - match progress.value { + match progress_params.value { lsp::ProgressParamsValue::WorkDone(progress) => { self.handle_work_done_progress( progress, language_server_id, disk_based_diagnostics_progress_token, - token, + ProgressToken::from_lsp(progress_params.token), cx, ); } lsp::ProgressParamsValue::WorkspaceDiagnostic(report) => { - let identifier = token.split_once("id:").map(|(_, id)| id.to_owned()); + let identifier = match progress_params.token { + lsp::NumberOrString::Number(_) => None, + lsp::NumberOrString::String(token) => token + .split_once(WORKSPACE_DIAGNOSTICS_TOKEN_START) + .map(|(_, id)| id.to_owned()), + }; if let Some(LanguageServerState::Running { workspace_diagnostics_refresh_tasks, .. @@ -9393,7 +9450,7 @@ impl LspStore { progress: lsp::WorkDoneProgress, language_server_id: LanguageServerId, disk_based_diagnostics_progress_token: Option, - token: String, + token: ProgressToken, cx: &mut Context, ) { let language_server_status = @@ -9407,9 +9464,14 @@ impl LspStore { return; } - let is_disk_based_diagnostics_progress = disk_based_diagnostics_progress_token - .as_ref() - .is_some_and(|disk_based_token| token.starts_with(disk_based_token)); + let is_disk_based_diagnostics_progress = + if let (Some(disk_based_token), ProgressToken::String(token)) = + (&disk_based_diagnostics_progress_token, &token) + { + token.starts_with(disk_based_token) + } else { + false + }; match progress { lsp::WorkDoneProgress::Begin(report) => { @@ -9456,7 +9518,7 @@ impl LspStore { fn on_lsp_work_start( &mut self, language_server_id: LanguageServerId, - token: String, + token: ProgressToken, progress: LanguageServerProgress, cx: &mut Context, ) { @@ -9470,7 +9532,7 @@ impl LspStore { .language_server_adapter_for_id(language_server_id) .map(|adapter| adapter.name()), message: proto::update_language_server::Variant::WorkStart(proto::LspWorkStart { - token, + token: Some(token.to_proto()), title: progress.title, message: progress.message, percentage: progress.percentage.map(|p| p as u32), @@ -9482,7 +9544,7 @@ impl LspStore { fn on_lsp_work_progress( &mut self, language_server_id: LanguageServerId, - token: String, + token: ProgressToken, progress: LanguageServerProgress, cx: &mut Context, ) { @@ -9522,7 +9584,7 @@ impl LspStore { .map(|adapter| adapter.name()), message: proto::update_language_server::Variant::WorkProgress( proto::LspWorkProgress { - token, + token: Some(token.to_proto()), message: progress.message, percentage: progress.percentage.map(|p| p as u32), is_cancellable: Some(progress.is_cancellable), @@ -9535,7 +9597,7 @@ impl LspStore { fn on_lsp_work_end( &mut self, language_server_id: LanguageServerId, - token: String, + token: ProgressToken, cx: &mut Context, ) { if let Some(status) = self.language_server_statuses.get_mut(&language_server_id) { @@ -9552,7 +9614,9 @@ impl LspStore { name: self .language_server_adapter_for_id(language_server_id) .map(|adapter| adapter.name()), - message: proto::update_language_server::Variant::WorkEnd(proto::LspWorkEnd { token }), + message: proto::update_language_server::Variant::WorkEnd(proto::LspWorkEnd { + token: Some(token.to_proto()), + }), }) } @@ -9964,25 +10028,33 @@ impl LspStore { } pub async fn handle_cancel_language_server_work( - this: Entity, + lsp_store: Entity, envelope: TypedEnvelope, mut cx: AsyncApp, ) -> Result { - this.update(&mut cx, |this, cx| { + lsp_store.update(&mut cx, |lsp_store, cx| { if let Some(work) = envelope.payload.work { match work { proto::cancel_language_server_work::Work::Buffers(buffers) => { let buffers = - this.buffer_ids_to_buffers(buffers.buffer_ids.into_iter(), cx); - this.cancel_language_server_work_for_buffers(buffers, cx); + lsp_store.buffer_ids_to_buffers(buffers.buffer_ids.into_iter(), cx); + lsp_store.cancel_language_server_work_for_buffers(buffers, cx); } proto::cancel_language_server_work::Work::LanguageServerWork(work) => { let server_id = LanguageServerId::from_proto(work.language_server_id); - this.cancel_language_server_work(server_id, work.token, cx); + let token = work + .token + .map(|token| { + ProgressToken::from_proto(token) + .context("invalid work progress token") + }) + .transpose()?; + lsp_store.cancel_language_server_work(server_id, token, cx); } } } - })?; + anyhow::Ok(()) + })??; Ok(proto::Ack {}) } @@ -11093,7 +11165,7 @@ impl LspStore { pub(crate) fn cancel_language_server_work( &mut self, server_id: LanguageServerId, - token_to_cancel: Option, + token_to_cancel: Option, cx: &mut Context, ) { if let Some(local) = self.as_local() { @@ -11111,7 +11183,7 @@ impl LspStore { server .notify::( WorkDoneProgressCancelParams { - token: lsp::NumberOrString::String(token.clone()), + token: token.to_lsp(), }, ) .ok(); @@ -11125,7 +11197,7 @@ impl LspStore { proto::cancel_language_server_work::Work::LanguageServerWork( proto::cancel_language_server_work::LanguageServerWork { language_server_id: server_id.to_proto(), - token: token_to_cancel, + token: token_to_cancel.map(|token| token.to_proto()), }, ), ), @@ -12504,7 +12576,7 @@ fn lsp_workspace_diagnostics_refresh( let token = if let Some(identifier) = ®istration_id { format!( - "workspace/diagnostic/{}/{requests}/id:{identifier}", + "workspace/diagnostic/{}/{requests}/{WORKSPACE_DIAGNOSTICS_TOKEN_START}{identifier}", server.server_id(), ) } else { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 910e217a67..e188ebd5e3 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -146,7 +146,7 @@ pub use buffer_store::ProjectTransaction; pub use lsp_store::{ DiagnosticSummary, InvalidationStrategy, LanguageServerLogType, LanguageServerProgress, LanguageServerPromptRequest, LanguageServerStatus, LanguageServerToQuery, LspStore, - LspStoreEvent, SERVER_PROGRESS_THROTTLE_TIMEOUT, + LspStoreEvent, ProgressToken, SERVER_PROGRESS_THROTTLE_TIMEOUT, }; pub use toolchain_store::{ToolchainStore, Toolchains}; const MAX_PROJECT_SEARCH_HISTORY_SIZE: usize = 500; @@ -3451,7 +3451,7 @@ impl Project { pub fn cancel_language_server_work( &mut self, server_id: LanguageServerId, - token_to_cancel: Option, + token_to_cancel: Option, cx: &mut Context, ) { self.lsp_store.update(cx, |lsp_store, cx| { diff --git a/crates/proto/proto/lsp.proto b/crates/proto/proto/lsp.proto index 7e446a915f..3005943109 100644 --- a/crates/proto/proto/lsp.proto +++ b/crates/proto/proto/lsp.proto @@ -552,23 +552,33 @@ message UpdateLanguageServer { } } +message ProgressToken { + oneof value { + int32 number = 1; + string string = 2; + } +} + message LspWorkStart { - string token = 1; + reserved 1; optional string title = 4; optional string message = 2; optional uint32 percentage = 3; optional bool is_cancellable = 5; + ProgressToken token = 6; } message LspWorkProgress { - string token = 1; + reserved 1; optional string message = 2; optional uint32 percentage = 3; optional bool is_cancellable = 4; + ProgressToken token = 5; } message LspWorkEnd { - string token = 1; + reserved 1; + ProgressToken token = 2; } message LspDiskBasedDiagnosticsUpdating {} @@ -708,7 +718,8 @@ message CancelLanguageServerWork { message LanguageServerWork { uint64 language_server_id = 1; - optional string token = 2; + reserved 2; + optional ProgressToken token = 3; } } diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index 4010d033c0..969363fb2b 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -10,7 +10,7 @@ use language_model::LanguageModelToolResultContent; use extension::ExtensionHostProxy; use fs::{FakeFs, Fs}; -use gpui::{AppContext as _, Entity, SemanticVersion, TestAppContext}; +use gpui::{AppContext as _, Entity, SemanticVersion, SharedString, TestAppContext}; use http_client::{BlockedHttpClient, FakeHttpClient}; use language::{ Buffer, FakeLspAdapter, LanguageConfig, LanguageMatcher, LanguageRegistry, LineEnding, @@ -19,7 +19,7 @@ use language::{ use lsp::{CompletionContext, CompletionResponse, CompletionTriggerKind, LanguageServerName}; use node_runtime::NodeRuntime; use project::{ - Project, + ProgressToken, Project, agent_server_store::AgentServerCommand, search::{SearchQuery, SearchResult}, }; @@ -710,7 +710,11 @@ async fn test_remote_cancel_language_server_work( cx.executor().run_until_parked(); project.update(cx, |project, cx| { - project.cancel_language_server_work(server_id, Some(progress_token.into()), cx) + project.cancel_language_server_work( + server_id, + Some(ProgressToken::String(SharedString::from(progress_token))), + cx, + ) }); cx.executor().run_until_parked(); @@ -721,7 +725,7 @@ async fn test_remote_cancel_language_server_work( .await; assert_eq!( cancel_notification.token, - lsp::NumberOrString::String(progress_token.into()) + lsp::NumberOrString::String(progress_token.to_owned()) ); } }