From 0926db91115ddfa10535ae16b4c528d2a99247b4 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Mon, 20 Nov 2023 15:51:36 -0500 Subject: [PATCH 01/31] Add app events --- crates/client/src/telemetry.rs | 38 ++++++++++++++++++++++----------- crates/client2/src/telemetry.rs | 38 ++++++++++++++++++++++----------- crates/zed/src/main.rs | 23 ++++++++++++++++---- crates/zed2/src/main.rs | 20 +++++++++++++---- 4 files changed, 87 insertions(+), 32 deletions(-) diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 8f7fbeb83d..a3e7449cf8 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -109,6 +109,10 @@ pub enum ClickhouseEvent { virtual_memory_in_bytes: u64, milliseconds_since_first_event: i64, }, + App { + operation: &'static str, + milliseconds_since_first_event: i64, + }, } #[cfg(debug_assertions)] @@ -168,13 +172,8 @@ impl Telemetry { let mut state = self.state.lock(); state.installation_id = installation_id.map(|id| id.into()); state.session_id = Some(session_id.into()); - let has_clickhouse_events = !state.clickhouse_events_queue.is_empty(); drop(state); - if has_clickhouse_events { - self.flush_clickhouse_events(); - } - let this = self.clone(); cx.spawn(|mut cx| async move { // Avoiding calling `System::new_all()`, as there have been crashes related to it @@ -256,7 +255,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_copilot_event( @@ -273,7 +272,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_assistant_event( @@ -290,7 +289,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_call_event( @@ -307,7 +306,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_cpu_event( @@ -322,7 +321,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_memory_event( @@ -337,7 +336,21 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) + } + + // app_events are called at app open and app close, so flush is set to immediately send + pub fn report_app_event( + self: &Arc, + telemetry_settings: TelemetrySettings, + operation: &'static str, + ) { + let event = ClickhouseEvent::App { + operation, + milliseconds_since_first_event: self.milliseconds_since_first_event(), + }; + + self.report_clickhouse_event(event, telemetry_settings, true) } fn milliseconds_since_first_event(&self) -> i64 { @@ -358,6 +371,7 @@ impl Telemetry { self: &Arc, event: ClickhouseEvent, telemetry_settings: TelemetrySettings, + immediate_flush: bool, ) { if !telemetry_settings.metrics { return; @@ -370,7 +384,7 @@ impl Telemetry { .push(ClickhouseEventWrapper { signed_in, event }); if state.installation_id.is_some() { - if state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { + if immediate_flush || state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { drop(state); self.flush_clickhouse_events(); } else { diff --git a/crates/client2/src/telemetry.rs b/crates/client2/src/telemetry.rs index 9c88d1102c..9bd24293a0 100644 --- a/crates/client2/src/telemetry.rs +++ b/crates/client2/src/telemetry.rs @@ -107,6 +107,10 @@ pub enum ClickhouseEvent { virtual_memory_in_bytes: u64, milliseconds_since_first_event: i64, }, + App { + operation: &'static str, + milliseconds_since_first_event: i64, + }, } #[cfg(debug_assertions)] @@ -163,13 +167,8 @@ impl Telemetry { let mut state = self.state.lock(); state.installation_id = installation_id.map(|id| id.into()); state.session_id = Some(session_id.into()); - let has_clickhouse_events = !state.clickhouse_events_queue.is_empty(); drop(state); - if has_clickhouse_events { - self.flush_clickhouse_events(); - } - let this = self.clone(); cx.spawn(|cx| async move { // Avoiding calling `System::new_all()`, as there have been crashes related to it @@ -257,7 +256,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_copilot_event( @@ -274,7 +273,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_assistant_event( @@ -291,7 +290,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_call_event( @@ -308,7 +307,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_cpu_event( @@ -323,7 +322,7 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_memory_event( @@ -338,7 +337,21 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings) + self.report_clickhouse_event(event, telemetry_settings, false) + } + + // app_events are called at app open and app close, so flush is set to immediately send + pub fn report_app_event( + self: &Arc, + telemetry_settings: TelemetrySettings, + operation: &'static str, + ) { + let event = ClickhouseEvent::App { + operation, + milliseconds_since_first_event: self.milliseconds_since_first_event(), + }; + + self.report_clickhouse_event(event, telemetry_settings, true) } fn milliseconds_since_first_event(&self) -> i64 { @@ -359,6 +372,7 @@ impl Telemetry { self: &Arc, event: ClickhouseEvent, telemetry_settings: TelemetrySettings, + immediate_flush: bool, ) { if !telemetry_settings.metrics { return; @@ -371,7 +385,7 @@ impl Telemetry { .push(ClickhouseEventWrapper { signed_in, event }); if state.installation_id.is_some() { - if state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { + if immediate_flush || state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { drop(state); self.flush_clickhouse_events(); } else { diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 5f2a7c525e..992a433a74 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -65,7 +65,8 @@ fn main() { log::info!("========== starting zed =========="); let mut app = gpui::App::new(Assets).unwrap(); - let installation_id = app.background().block(installation_id()).ok(); + let (installation_id, existing_installation_id_found) = + app.background().block(installation_id()).ok().unzip(); let session_id = Uuid::new_v4().to_string(); init_panic_hook(&app, installation_id.clone(), session_id.clone()); @@ -166,6 +167,20 @@ fn main() { .detach(); client.telemetry().start(installation_id, session_id, cx); + // TODO: + // Cleanly identify open / first open + // What should we do if we fail when looking for installation_id? + // - set to true, false, or skip? + // Report closed + // Copy logic to zed2 + let telemetry_settings = *settings::get::(cx); + let event_operation = match existing_installation_id_found { + Some(true) => "open", + _ => "first open", + }; + client + .telemetry() + .report_app_event(telemetry_settings, event_operation); let app_state = Arc::new(AppState { languages, @@ -317,11 +332,11 @@ async fn authenticate(client: Arc, cx: &AsyncAppContext) -> Result<()> { Ok::<_, anyhow::Error>(()) } -async fn installation_id() -> Result { +async fn installation_id() -> Result<(String, bool)> { let legacy_key_name = "device_id"; if let Ok(Some(installation_id)) = KEY_VALUE_STORE.read_kvp(legacy_key_name) { - Ok(installation_id) + Ok((installation_id, true)) } else { let installation_id = Uuid::new_v4().to_string(); @@ -329,7 +344,7 @@ async fn installation_id() -> Result { .write_kvp(legacy_key_name.to_string(), installation_id.clone()) .await?; - Ok(installation_id) + Ok((installation_id, false)) } } diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index 0532d62c38..aacb3ed83c 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -71,7 +71,11 @@ fn main() { log::info!("========== starting zed =========="); let app = App::production(Arc::new(Assets)); - let installation_id = app.background_executor().block(installation_id()).ok(); + let (installation_id, existing_installation_id_found) = app + .background_executor() + .block(installation_id()) + .ok() + .unzip(); let session_id = Uuid::new_v4().to_string(); init_panic_hook(&app, installation_id.clone(), session_id.clone()); @@ -173,6 +177,14 @@ fn main() { // .detach(); client.telemetry().start(installation_id, session_id, cx); + let telemetry_settings = *settings::get::(cx); + let event_operation = match existing_installation_id_found { + Some(true) => "open", + _ => "first open", + }; + client + .telemetry() + .report_app_event(telemetry_settings, event_operation); let app_state = Arc::new(AppState { languages, @@ -333,11 +345,11 @@ fn main() { // Ok::<_, anyhow::Error>(()) // } -async fn installation_id() -> Result { +async fn installation_id() -> Result<(String, bool)> { let legacy_key_name = "device_id"; if let Ok(Some(installation_id)) = KEY_VALUE_STORE.read_kvp(legacy_key_name) { - Ok(installation_id) + Ok((installation_id, true)) } else { let installation_id = Uuid::new_v4().to_string(); @@ -345,7 +357,7 @@ async fn installation_id() -> Result { .write_kvp(legacy_key_name.to_string(), installation_id.clone()) .await?; - Ok(installation_id) + Ok((installation_id, false)) } } From db3f48747420b220b1d0f37916cdecd560106f5c Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Mon, 20 Nov 2023 16:00:05 -0500 Subject: [PATCH 02/31] Fix zed2 compile error --- crates/zed2/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index aacb3ed83c..0f6075f62e 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -177,7 +177,7 @@ fn main() { // .detach(); client.telemetry().start(installation_id, session_id, cx); - let telemetry_settings = *settings::get::(cx); + let telemetry_settings = *client::TelemetrySettings::get_global(cx); let event_operation = match existing_installation_id_found { Some(true) => "open", _ => "first open", From a0dcc9618ed5575ceaa426165a2406b4ad765232 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Mon, 20 Nov 2023 16:04:15 -0500 Subject: [PATCH 03/31] Mark app event as `open` if we fail to get installation_id If we find a previous installation_id, then we send `open`. If we don't find a previous installation_id, then we sent as `first open`. If we fail, we mark it as `open` so that we don't accidentally bloat our `first open` stats. --- crates/zed/src/main.rs | 4 ++-- crates/zed2/src/main.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 992a433a74..b953a782ce 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -175,8 +175,8 @@ fn main() { // Copy logic to zed2 let telemetry_settings = *settings::get::(cx); let event_operation = match existing_installation_id_found { - Some(true) => "open", - _ => "first open", + Some(false) => "first open", + _ => "open", }; client .telemetry() diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index 0f6075f62e..9e851f1008 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -179,8 +179,8 @@ fn main() { client.telemetry().start(installation_id, session_id, cx); let telemetry_settings = *client::TelemetrySettings::get_global(cx); let event_operation = match existing_installation_id_found { - Some(true) => "open", - _ => "first open", + Some(false) => "first open", + _ => "open", }; client .telemetry() From daddb03e7a8fda0cb4adce35cfb629cd98ea271d Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Mon, 20 Nov 2023 16:04:32 -0500 Subject: [PATCH 04/31] Remove comments --- crates/zed/src/main.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index b953a782ce..20b93ae6bb 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -167,12 +167,6 @@ fn main() { .detach(); client.telemetry().start(installation_id, session_id, cx); - // TODO: - // Cleanly identify open / first open - // What should we do if we fail when looking for installation_id? - // - set to true, false, or skip? - // Report closed - // Copy logic to zed2 let telemetry_settings = *settings::get::(cx); let event_operation = match existing_installation_id_found { Some(false) => "first open", From 170291ff96baefded554a3524e64fe2681621d5b Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 21 Nov 2023 19:57:24 +0100 Subject: [PATCH 05/31] Start decoupling workspace and call crates --- Cargo.lock | 1 + crates/workspace2/Cargo.toml | 1 + crates/workspace2/src/pane_group.rs | 7 +- crates/workspace2/src/workspace2.rs | 382 ++++++++++++++++++---------- 4 files changed, 246 insertions(+), 145 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6aa94b08d0..85f474b046 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11319,6 +11319,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-recursion 1.0.5", + "async-trait", "bincode", "call2", "client2", diff --git a/crates/workspace2/Cargo.toml b/crates/workspace2/Cargo.toml index f3f10d2015..bddf019eb5 100644 --- a/crates/workspace2/Cargo.toml +++ b/crates/workspace2/Cargo.toml @@ -37,6 +37,7 @@ theme2 = { path = "../theme2" } util = { path = "../util" } ui = { package = "ui2", path = "../ui2" } +async-trait.workspace = true async-recursion = "1.0.0" itertools = "0.10" bincode = "1.2.1" diff --git a/crates/workspace2/src/pane_group.rs b/crates/workspace2/src/pane_group.rs index bd827a6dd7..80e002a429 100644 --- a/crates/workspace2/src/pane_group.rs +++ b/crates/workspace2/src/pane_group.rs @@ -127,7 +127,6 @@ impl PaneGroup { &self, project: &Model, follower_states: &HashMap, FollowerState>, - active_call: Option<&Model>, active_pane: &View, zoomed: Option<&AnyWeakView>, app_state: &Arc, @@ -137,7 +136,6 @@ impl PaneGroup { project, 0, follower_states, - active_call, active_pane, zoomed, app_state, @@ -199,7 +197,6 @@ impl Member { project: &Model, basis: usize, follower_states: &HashMap, FollowerState>, - active_call: Option<&Model>, active_pane: &View, zoomed: Option<&AnyWeakView>, app_state: &Arc, @@ -234,7 +231,6 @@ impl Member { project, basis + 1, follower_states, - active_call, active_pane, zoomed, app_state, @@ -556,7 +552,7 @@ impl PaneAxis { project: &Model, basis: usize, follower_states: &HashMap, FollowerState>, - active_call: Option<&Model>, + active_pane: &View, zoomed: Option<&AnyWeakView>, app_state: &Arc, @@ -578,7 +574,6 @@ impl PaneAxis { project, basis, follower_states, - active_call, active_pane, zoomed, app_state, diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index 22a7b57058..64f6e5963d 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -15,7 +15,8 @@ mod status_bar; mod toolbar; mod workspace_settings; -use anyhow::{anyhow, Context as _, Result}; +use anyhow::{anyhow, bail, Context as _, Result}; +use async_trait::async_trait; use call2::ActiveCall; use client2::{ proto::{self, PeerId}, @@ -33,8 +34,8 @@ use gpui::{ AsyncWindowContext, Bounds, Context, Div, Entity, EntityId, EventEmitter, FocusHandle, FocusableView, GlobalPixels, InteractiveElement, KeyContext, ManagedView, Model, ModelContext, ParentElement, PathPromptOptions, Point, PromptLevel, Render, Size, Styled, Subscription, Task, - View, ViewContext, VisualContext, WeakView, WindowBounds, WindowContext, WindowHandle, - WindowOptions, + View, ViewContext, VisualContext, WeakModel, WeakView, WindowBounds, WindowContext, + WindowHandle, WindowOptions, }; use item::{FollowableItem, FollowableItemHandle, Item, ItemHandle, ItemSettings, ProjectItem}; use itertools::Itertools; @@ -408,6 +409,177 @@ pub enum Event { WorkspaceCreated(WeakView), } +#[async_trait(?Send)] +trait CallHandler { + fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()>; + fn shared_screen_for_peer( + &self, + peer_id: PeerId, + pane: &View, + cx: &mut ViewContext, + ) -> Option>; + fn follower_states_mut(&mut self) -> &mut HashMap, FollowerState>; + fn follower_states(&self) -> &HashMap, FollowerState>; + fn room_id(&self, cx: &AppContext) -> Option; + fn is_in_room(&self, cx: &mut ViewContext) -> bool { + self.room_id(cx).is_some() + } + fn hang_up(&self, cx: AsyncWindowContext) -> Result>>; + fn active_project(&self, cx: &AppContext) -> Option>; +} +struct Call { + follower_states: HashMap, FollowerState>, + active_call: Option<(Model, Vec)>, + parent_workspace: WeakView, +} + +impl Call { + fn new(parent_workspace: WeakView, cx: &mut ViewContext<'_, Workspace>) -> Self { + let mut active_call = None; + if cx.has_global::>() { + let call = cx.global::>().clone(); + let subscriptions = vec![cx.subscribe(&call, Self::on_active_call_event)]; + active_call = Some((call, subscriptions)); + } + Self { + follower_states: Default::default(), + active_call, + parent_workspace, + } + } + fn on_active_call_event( + workspace: &mut Workspace, + _: Model, + event: &call2::room::Event, + cx: &mut ViewContext, + ) { + match event { + call2::room::Event::ParticipantLocationChanged { participant_id } + | call2::room::Event::RemoteVideoTracksChanged { participant_id } => { + workspace.leader_updated(*participant_id, cx); + } + _ => {} + } + } +} + +#[async_trait(?Send)] +impl CallHandler for Call { + fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()> { + cx.notify(); + + let (call, _) = self.active_call.as_ref()?; + let room = call.read(cx).room()?.read(cx); + let participant = room.remote_participant_for_peer_id(leader_id)?; + let mut items_to_activate = Vec::new(); + + let leader_in_this_app; + let leader_in_this_project; + match participant.location { + call2::ParticipantLocation::SharedProject { project_id } => { + leader_in_this_app = true; + leader_in_this_project = Some(project_id) + == self + .parent_workspace + .update(cx, |this, cx| this.project.read(cx).remote_id()) + .log_err() + .flatten(); + } + call2::ParticipantLocation::UnsharedProject => { + leader_in_this_app = true; + leader_in_this_project = false; + } + call2::ParticipantLocation::External => { + leader_in_this_app = false; + leader_in_this_project = false; + } + }; + + for (pane, state) in &self.follower_states { + if state.leader_id != leader_id { + continue; + } + if let (Some(active_view_id), true) = (state.active_view_id, leader_in_this_app) { + if let Some(item) = state.items_by_leader_view_id.get(&active_view_id) { + if leader_in_this_project || !item.is_project_item(cx) { + items_to_activate.push((pane.clone(), item.boxed_clone())); + } + } else { + log::warn!( + "unknown view id {:?} for leader {:?}", + active_view_id, + leader_id + ); + } + continue; + } + // todo!() + // if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) { + // items_to_activate.push((pane.clone(), Box::new(shared_screen))); + // } + } + + for (pane, item) in items_to_activate { + let pane_was_focused = pane.read(cx).has_focus(cx); + if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) { + pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx)); + } else { + pane.update(cx, |pane, mut cx| { + pane.add_item(item.boxed_clone(), false, false, None, &mut cx) + }); + } + + if pane_was_focused { + pane.update(cx, |pane, cx| pane.focus_active_item(cx)); + } + } + + None + } + + fn shared_screen_for_peer( + &self, + peer_id: PeerId, + pane: &View, + cx: &mut ViewContext, + ) -> Option> { + let (call, _) = self.active_call.as_ref()?; + let room = call.read(cx).room()?.read(cx); + let participant = room.remote_participant_for_peer_id(peer_id)?; + let track = participant.video_tracks.values().next()?.clone(); + let user = participant.user.clone(); + todo!(); + // for item in pane.read(cx).items_of_type::() { + // if item.read(cx).peer_id == peer_id { + // return Box::new(Some(item)); + // } + // } + + // Some(Box::new(cx.build_view(|cx| { + // SharedScreen::new(&track, peer_id, user.clone(), cx) + // }))) + } + + fn follower_states_mut(&mut self) -> &mut HashMap, FollowerState> { + &mut self.follower_states + } + fn follower_states(&self) -> &HashMap, FollowerState> { + &self.follower_states + } + fn room_id(&self, cx: &AppContext) -> Option { + Some(self.active_call.as_ref()?.0.read(cx).room()?.read(cx).id()) + } + fn hang_up(&self, mut cx: AsyncWindowContext) -> Result>> { + let Some((call, _)) = self.active_call.as_ref() else { + bail!("Cannot exit a call; not in a call"); + }; + + call.update(&mut cx, |this, cx| this.hang_up(cx)) + } + fn active_project(&self, cx: &AppContext) -> Option> { + ActiveCall::global(cx).read(cx).location().cloned() + } +} pub struct Workspace { window_self: WindowHandle, weak_self: WeakView, @@ -428,10 +600,9 @@ pub struct Workspace { titlebar_item: Option, notifications: Vec<(TypeId, usize, Box)>, project: Model, - follower_states: HashMap, FollowerState>, + call_handler: Box, last_leaders_by_pane: HashMap, PeerId>, window_edited: bool, - active_call: Option<(Model, Vec)>, leader_updates_tx: mpsc::UnboundedSender<(PeerId, proto::UpdateFollowers)>, database_id: WorkspaceId, app_state: Arc, @@ -550,9 +721,19 @@ impl Workspace { mpsc::unbounded::<(PeerId, proto::UpdateFollowers)>(); let _apply_leader_updates = cx.spawn(|this, mut cx| async move { while let Some((leader_id, update)) = leader_updates_rx.next().await { - Self::process_leader_update(&this, leader_id, update, &mut cx) + let mut cx2 = cx.clone(); + let t = this.clone(); + + Workspace::process_leader_update(&this, leader_id, update, &mut cx) .await .log_err(); + + // this.update(&mut cx, |this, cxx| { + // this.call_handler + // .process_leader_update(leader_id, update, cx2) + // })? + // .await + // .log_err(); } Ok(()) @@ -585,14 +766,6 @@ impl Workspace { // drag_and_drop.register_container(weak_handle.clone()); // }); - let mut active_call = None; - if cx.has_global::>() { - let call = cx.global::>().clone(); - let mut subscriptions = Vec::new(); - subscriptions.push(cx.subscribe(&call, Self::on_active_call_event)); - active_call = Some((call, subscriptions)); - } - let subscriptions = vec![ cx.observe_window_activation(Self::on_window_activation_changed), cx.observe_window_bounds(move |_, cx| { @@ -652,10 +825,11 @@ impl Workspace { bottom_dock, right_dock, project: project.clone(), - follower_states: Default::default(), + last_leaders_by_pane: Default::default(), window_edited: false, - active_call, + + call_handler: Box::new(Call::new(weak_handle.clone(), cx)), database_id: workspace_id, app_state, _observe_current_user, @@ -1102,7 +1276,7 @@ impl Workspace { cx: &mut ViewContext, ) -> Task> { //todo!(saveing) - let active_call = self.active_call().cloned(); + let window = cx.window_handle(); cx.spawn(|this, mut cx| async move { @@ -1113,27 +1287,27 @@ impl Workspace { .count() })?; - if let Some(active_call) = active_call { - if !quitting - && workspace_count == 1 - && active_call.read_with(&cx, |call, _| call.room().is_some())? - { - let answer = window.update(&mut cx, |_, cx| { - cx.prompt( - PromptLevel::Warning, - "Do you want to leave the current call?", - &["Close window and hang up", "Cancel"], - ) - })?; + if !quitting + && workspace_count == 1 + && this + .update(&mut cx, |this, cx| this.call_handler.is_in_room(cx)) + .log_err() + .unwrap_or_default() + { + let answer = window.update(&mut cx, |_, cx| { + cx.prompt( + PromptLevel::Warning, + "Do you want to leave the current call?", + &["Close window and hang up", "Cancel"], + ) + })?; - if answer.await.log_err() == Some(1) { - return anyhow::Ok(false); - } else { - active_call - .update(&mut cx, |call, cx| call.hang_up(cx))? - .await - .log_err(); - } + if answer.await.log_err() == Some(1) { + return anyhow::Ok(false); + } else { + this.update(&mut cx, |this, cx| this.call_handler.hang_up(cx.to_async()))?? + .await + .log_err(); } } @@ -2238,7 +2412,7 @@ impl Workspace { } fn collaborator_left(&mut self, peer_id: PeerId, cx: &mut ViewContext) { - self.follower_states.retain(|_, state| { + self.call_handler.follower_states_mut().retain(|_, state| { if state.leader_id == peer_id { for item in state.items_by_leader_view_id.values() { item.set_leader_peer_id(None, cx); @@ -2391,19 +2565,19 @@ impl Workspace { // } pub fn unfollow(&mut self, pane: &View, cx: &mut ViewContext) -> Option { - let state = self.follower_states.remove(pane)?; + let follower_states = self.call_handler.follower_states_mut(); + let state = follower_states.remove(pane)?; let leader_id = state.leader_id; for (_, item) in state.items_by_leader_view_id { item.set_leader_peer_id(None, cx); } - if self - .follower_states + if follower_states .values() .all(|state| state.leader_id != state.leader_id) { let project_id = self.project.read(cx).remote_id(); - let room_id = self.active_call()?.read(cx).room()?.read(cx).id(); + let room_id = self.call_handler.room_id(cx)?; self.app_state .client .send(proto::Unfollow { @@ -2614,7 +2788,7 @@ impl Workspace { match update.variant.ok_or_else(|| anyhow!("invalid update"))? { proto::update_followers::Variant::UpdateActiveView(update_active_view) => { this.update(cx, |this, _| { - for (_, state) in &mut this.follower_states { + for (_, state) in this.call_handler.follower_states_mut() { if state.leader_id == leader_id { state.active_view_id = if let Some(active_view_id) = update_active_view.id.clone() { @@ -2637,7 +2811,7 @@ impl Workspace { let mut tasks = Vec::new(); this.update(cx, |this, cx| { let project = this.project.clone(); - for (_, state) in &mut this.follower_states { + for (_, state) in this.call_handler.follower_states_mut() { if state.leader_id == leader_id { let view_id = ViewId::from_proto(id.clone())?; if let Some(item) = state.items_by_leader_view_id.get(&view_id) { @@ -2651,7 +2825,8 @@ impl Workspace { } proto::update_followers::Variant::CreateView(view) => { let panes = this.update(cx, |this, _| { - this.follower_states + this.call_handler + .follower_states() .iter() .filter_map(|(pane, state)| (state.leader_id == leader_id).then_some(pane)) .cloned() @@ -2711,7 +2886,7 @@ impl Workspace { for (pane, (item_tasks, leader_view_ids)) in item_tasks_by_pane { let items = futures::future::try_join_all(item_tasks).await?; this.update(cx, |this, cx| { - let state = this.follower_states.get_mut(&pane)?; + let state = this.call_handler.follower_states_mut().get_mut(&pane)?; for (id, item) in leader_view_ids.into_iter().zip(items) { item.set_leader_peer_id(Some(leader_id), cx); state.items_by_leader_view_id.insert(id, item); @@ -2768,74 +2943,14 @@ impl Workspace { } pub fn leader_for_pane(&self, pane: &View) -> Option { - self.follower_states.get(pane).map(|state| state.leader_id) + self.call_handler + .follower_states() + .get(pane) + .map(|state| state.leader_id) } fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()> { - cx.notify(); - - let call = self.active_call()?; - let room = call.read(cx).room()?.read(cx); - let participant = room.remote_participant_for_peer_id(leader_id)?; - let mut items_to_activate = Vec::new(); - - let leader_in_this_app; - let leader_in_this_project; - match participant.location { - call2::ParticipantLocation::SharedProject { project_id } => { - leader_in_this_app = true; - leader_in_this_project = Some(project_id) == self.project.read(cx).remote_id(); - } - call2::ParticipantLocation::UnsharedProject => { - leader_in_this_app = true; - leader_in_this_project = false; - } - call2::ParticipantLocation::External => { - leader_in_this_app = false; - leader_in_this_project = false; - } - }; - - for (pane, state) in &self.follower_states { - if state.leader_id != leader_id { - continue; - } - if let (Some(active_view_id), true) = (state.active_view_id, leader_in_this_app) { - if let Some(item) = state.items_by_leader_view_id.get(&active_view_id) { - if leader_in_this_project || !item.is_project_item(cx) { - items_to_activate.push((pane.clone(), item.boxed_clone())); - } - } else { - log::warn!( - "unknown view id {:?} for leader {:?}", - active_view_id, - leader_id - ); - } - continue; - } - // todo!() - // if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) { - // items_to_activate.push((pane.clone(), Box::new(shared_screen))); - // } - } - - for (pane, item) in items_to_activate { - let pane_was_focused = pane.read(cx).has_focus(cx); - if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) { - pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx)); - } else { - pane.update(cx, |pane, cx| { - pane.add_item(item.boxed_clone(), false, false, None, cx) - }); - } - - if pane_was_focused { - pane.update(cx, |pane, cx| pane.focus_active_item(cx)); - } - } - - None + self.call_handler.leader_updated(leader_id, cx) } // todo!() @@ -2886,25 +3001,6 @@ impl Workspace { } } - fn active_call(&self) -> Option<&Model> { - self.active_call.as_ref().map(|(call, _)| call) - } - - fn on_active_call_event( - &mut self, - _: Model, - event: &call2::room::Event, - cx: &mut ViewContext, - ) { - match event { - call2::room::Event::ParticipantLocationChanged { participant_id } - | call2::room::Event::RemoteVideoTracksChanged { participant_id } => { - self.leader_updated(*participant_id, cx); - } - _ => {} - } - } - pub fn database_id(&self) -> WorkspaceId { self.database_id } @@ -3671,8 +3767,7 @@ impl Render for Workspace { .flex_1() .child(self.center.render( &self.project, - &self.follower_states, - self.active_call(), + &self.call_handler.follower_states(), &self.active_pane, self.zoomed.as_ref(), &self.app_state, @@ -3845,11 +3940,12 @@ impl WorkspaceStore { update: proto::update_followers::Variant, cx: &AppContext, ) -> Option<()> { - if !cx.has_global::>() { - return None; - } - - let room_id = ActiveCall::global(cx).read(cx).room()?.read(cx).id(); + let room_id = self.workspaces.iter().next().and_then(|workspace| { + workspace + .read_with(cx, |this, cx| this.call_handler.room_id(cx)) + .log_err() + .flatten() + })?; let follower_ids: Vec<_> = self .followers .iter() @@ -3885,9 +3981,17 @@ impl WorkspaceStore { project_id: envelope.payload.project_id, peer_id: envelope.original_sender_id()?, }; - let active_project = ActiveCall::global(cx).read(cx).location().cloned(); - let mut response = proto::FollowResponse::default(); + let active_project = this + .workspaces + .iter() + .next() + .and_then(|workspace| { + workspace + .read_with(cx, |this, cx| this.call_handler.active_project(cx)) + .log_err() + }) + .flatten(); for workspace in &this.workspaces { workspace .update(cx, |workspace, cx| { From ebccdb64bcf7b62d65d99e3065bb932c599b1d27 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 21 Nov 2023 20:18:35 +0100 Subject: [PATCH 06/31] Move CallHandler impl into call2 --- Cargo.lock | 1 + crates/call2/Cargo.toml | 2 +- crates/call2/src/call2.rs | 154 +++++++++++++++++++++ crates/workspace2/Cargo.toml | 1 - crates/workspace2/src/pane_group.rs | 1 - crates/workspace2/src/workspace2.rs | 202 ++++++---------------------- 6 files changed, 196 insertions(+), 165 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 85f474b046..17bda4458c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1193,6 +1193,7 @@ dependencies = [ "serde_json", "settings2", "util", + "workspace2", ] [[package]] diff --git a/crates/call2/Cargo.toml b/crates/call2/Cargo.toml index 9e13463680..500931cc11 100644 --- a/crates/call2/Cargo.toml +++ b/crates/call2/Cargo.toml @@ -31,7 +31,7 @@ media = { path = "../media" } project = { package = "project2", path = "../project2" } settings = { package = "settings2", path = "../settings2" } util = { path = "../util" } - +workspace = {package = "workspace2", path = "../workspace2"} anyhow.workspace = true async-broadcast = "0.4" futures.workspace = true diff --git a/crates/call2/src/call2.rs b/crates/call2/src/call2.rs index 1f11e0650d..34a9aabe14 100644 --- a/crates/call2/src/call2.rs +++ b/crates/call2/src/call2.rs @@ -505,6 +505,160 @@ pub fn report_call_event_for_channel( ) } +struct Call { + follower_states: HashMap, FollowerState>, + active_call: Option<(Model, Vec)>, + parent_workspace: WeakView, +} + +impl Call { + fn new(parent_workspace: WeakView, cx: &mut ViewContext<'_, Workspace>) -> Self { + let mut active_call = None; + if cx.has_global::>() { + let call = cx.global::>().clone(); + let subscriptions = vec![cx.subscribe(&call, Self::on_active_call_event)]; + active_call = Some((call, subscriptions)); + } + Self { + follower_states: Default::default(), + active_call, + parent_workspace, + } + } + fn on_active_call_event( + workspace: &mut Workspace, + _: Model, + event: &call2::room::Event, + cx: &mut ViewContext, + ) { + match event { + call2::room::Event::ParticipantLocationChanged { participant_id } + | call2::room::Event::RemoteVideoTracksChanged { participant_id } => { + workspace.leader_updated(*participant_id, cx); + } + _ => {} + } + } +} + +#[async_trait(?Send)] +impl CallHandler for Call { + fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()> { + cx.notify(); + + let (call, _) = self.active_call.as_ref()?; + let room = call.read(cx).room()?.read(cx); + let participant = room.remote_participant_for_peer_id(leader_id)?; + let mut items_to_activate = Vec::new(); + + let leader_in_this_app; + let leader_in_this_project; + match participant.location { + call2::ParticipantLocation::SharedProject { project_id } => { + leader_in_this_app = true; + leader_in_this_project = Some(project_id) + == self + .parent_workspace + .update(cx, |this, cx| this.project.read(cx).remote_id()) + .log_err() + .flatten(); + } + call2::ParticipantLocation::UnsharedProject => { + leader_in_this_app = true; + leader_in_this_project = false; + } + call2::ParticipantLocation::External => { + leader_in_this_app = false; + leader_in_this_project = false; + } + }; + + for (pane, state) in &self.follower_states { + if state.leader_id != leader_id { + continue; + } + if let (Some(active_view_id), true) = (state.active_view_id, leader_in_this_app) { + if let Some(item) = state.items_by_leader_view_id.get(&active_view_id) { + if leader_in_this_project || !item.is_project_item(cx) { + items_to_activate.push((pane.clone(), item.boxed_clone())); + } + } else { + log::warn!( + "unknown view id {:?} for leader {:?}", + active_view_id, + leader_id + ); + } + continue; + } + // todo!() + // if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) { + // items_to_activate.push((pane.clone(), Box::new(shared_screen))); + // } + } + + for (pane, item) in items_to_activate { + let pane_was_focused = pane.read(cx).has_focus(cx); + if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) { + pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx)); + } else { + pane.update(cx, |pane, mut cx| { + pane.add_item(item.boxed_clone(), false, false, None, &mut cx) + }); + } + + if pane_was_focused { + pane.update(cx, |pane, cx| pane.focus_active_item(cx)); + } + } + + None + } + + fn shared_screen_for_peer( + &self, + peer_id: PeerId, + pane: &View, + cx: &mut ViewContext, + ) -> Option> { + let (call, _) = self.active_call.as_ref()?; + let room = call.read(cx).room()?.read(cx); + let participant = room.remote_participant_for_peer_id(peer_id)?; + let track = participant.video_tracks.values().next()?.clone(); + let user = participant.user.clone(); + todo!(); + // for item in pane.read(cx).items_of_type::() { + // if item.read(cx).peer_id == peer_id { + // return Box::new(Some(item)); + // } + // } + + // Some(Box::new(cx.build_view(|cx| { + // SharedScreen::new(&track, peer_id, user.clone(), cx) + // }))) + } + + fn follower_states_mut(&mut self) -> &mut HashMap, FollowerState> { + &mut self.follower_states + } + fn follower_states(&self) -> &HashMap, FollowerState> { + &self.follower_states + } + fn room_id(&self, cx: &AppContext) -> Option { + Some(self.active_call.as_ref()?.0.read(cx).room()?.read(cx).id()) + } + fn hang_up(&self, mut cx: AsyncWindowContext) -> Result>> { + let Some((call, _)) = self.active_call.as_ref() else { + bail!("Cannot exit a call; not in a call"); + }; + + call.update(&mut cx, |this, cx| this.hang_up(cx)) + } + fn active_project(&self, cx: &AppContext) -> Option> { + ActiveCall::global(cx).read(cx).location().cloned() + } +} + #[cfg(test)] mod test { use gpui::TestAppContext; diff --git a/crates/workspace2/Cargo.toml b/crates/workspace2/Cargo.toml index bddf019eb5..c327132a78 100644 --- a/crates/workspace2/Cargo.toml +++ b/crates/workspace2/Cargo.toml @@ -20,7 +20,6 @@ test-support = [ [dependencies] db2 = { path = "../db2" } -call2 = { path = "../call2" } client2 = { path = "../client2" } collections = { path = "../collections" } # context_menu = { path = "../context_menu" } diff --git a/crates/workspace2/src/pane_group.rs b/crates/workspace2/src/pane_group.rs index 80e002a429..eeea0bd365 100644 --- a/crates/workspace2/src/pane_group.rs +++ b/crates/workspace2/src/pane_group.rs @@ -1,6 +1,5 @@ use crate::{AppState, FollowerState, Pane, Workspace}; use anyhow::{anyhow, bail, Result}; -use call2::ActiveCall; use collections::HashMap; use db2::sqlez::{ bindable::{Bind, Column, StaticColumnCount}, diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index 64f6e5963d..05e994b74f 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -17,7 +17,6 @@ mod workspace_settings; use anyhow::{anyhow, bail, Context as _, Result}; use async_trait::async_trait; -use call2::ActiveCall; use client2::{ proto::{self, PeerId}, Client, TypedEnvelope, UserStore, @@ -410,7 +409,7 @@ pub enum Event { } #[async_trait(?Send)] -trait CallHandler { +pub trait CallHandler { fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()>; fn shared_screen_for_peer( &self, @@ -427,159 +426,7 @@ trait CallHandler { fn hang_up(&self, cx: AsyncWindowContext) -> Result>>; fn active_project(&self, cx: &AppContext) -> Option>; } -struct Call { - follower_states: HashMap, FollowerState>, - active_call: Option<(Model, Vec)>, - parent_workspace: WeakView, -} -impl Call { - fn new(parent_workspace: WeakView, cx: &mut ViewContext<'_, Workspace>) -> Self { - let mut active_call = None; - if cx.has_global::>() { - let call = cx.global::>().clone(); - let subscriptions = vec![cx.subscribe(&call, Self::on_active_call_event)]; - active_call = Some((call, subscriptions)); - } - Self { - follower_states: Default::default(), - active_call, - parent_workspace, - } - } - fn on_active_call_event( - workspace: &mut Workspace, - _: Model, - event: &call2::room::Event, - cx: &mut ViewContext, - ) { - match event { - call2::room::Event::ParticipantLocationChanged { participant_id } - | call2::room::Event::RemoteVideoTracksChanged { participant_id } => { - workspace.leader_updated(*participant_id, cx); - } - _ => {} - } - } -} - -#[async_trait(?Send)] -impl CallHandler for Call { - fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()> { - cx.notify(); - - let (call, _) = self.active_call.as_ref()?; - let room = call.read(cx).room()?.read(cx); - let participant = room.remote_participant_for_peer_id(leader_id)?; - let mut items_to_activate = Vec::new(); - - let leader_in_this_app; - let leader_in_this_project; - match participant.location { - call2::ParticipantLocation::SharedProject { project_id } => { - leader_in_this_app = true; - leader_in_this_project = Some(project_id) - == self - .parent_workspace - .update(cx, |this, cx| this.project.read(cx).remote_id()) - .log_err() - .flatten(); - } - call2::ParticipantLocation::UnsharedProject => { - leader_in_this_app = true; - leader_in_this_project = false; - } - call2::ParticipantLocation::External => { - leader_in_this_app = false; - leader_in_this_project = false; - } - }; - - for (pane, state) in &self.follower_states { - if state.leader_id != leader_id { - continue; - } - if let (Some(active_view_id), true) = (state.active_view_id, leader_in_this_app) { - if let Some(item) = state.items_by_leader_view_id.get(&active_view_id) { - if leader_in_this_project || !item.is_project_item(cx) { - items_to_activate.push((pane.clone(), item.boxed_clone())); - } - } else { - log::warn!( - "unknown view id {:?} for leader {:?}", - active_view_id, - leader_id - ); - } - continue; - } - // todo!() - // if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) { - // items_to_activate.push((pane.clone(), Box::new(shared_screen))); - // } - } - - for (pane, item) in items_to_activate { - let pane_was_focused = pane.read(cx).has_focus(cx); - if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) { - pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx)); - } else { - pane.update(cx, |pane, mut cx| { - pane.add_item(item.boxed_clone(), false, false, None, &mut cx) - }); - } - - if pane_was_focused { - pane.update(cx, |pane, cx| pane.focus_active_item(cx)); - } - } - - None - } - - fn shared_screen_for_peer( - &self, - peer_id: PeerId, - pane: &View, - cx: &mut ViewContext, - ) -> Option> { - let (call, _) = self.active_call.as_ref()?; - let room = call.read(cx).room()?.read(cx); - let participant = room.remote_participant_for_peer_id(peer_id)?; - let track = participant.video_tracks.values().next()?.clone(); - let user = participant.user.clone(); - todo!(); - // for item in pane.read(cx).items_of_type::() { - // if item.read(cx).peer_id == peer_id { - // return Box::new(Some(item)); - // } - // } - - // Some(Box::new(cx.build_view(|cx| { - // SharedScreen::new(&track, peer_id, user.clone(), cx) - // }))) - } - - fn follower_states_mut(&mut self) -> &mut HashMap, FollowerState> { - &mut self.follower_states - } - fn follower_states(&self) -> &HashMap, FollowerState> { - &self.follower_states - } - fn room_id(&self, cx: &AppContext) -> Option { - Some(self.active_call.as_ref()?.0.read(cx).room()?.read(cx).id()) - } - fn hang_up(&self, mut cx: AsyncWindowContext) -> Result>> { - let Some((call, _)) = self.active_call.as_ref() else { - bail!("Cannot exit a call; not in a call"); - }; - - call.update(&mut cx, |this, cx| this.hang_up(cx)) - } - fn active_project(&self, cx: &AppContext) -> Option> { - ActiveCall::global(cx).read(cx).location().cloned() - } -} pub struct Workspace { window_self: WindowHandle, weak_self: WeakView, @@ -611,6 +458,7 @@ pub struct Workspace { _observe_current_user: Task>, _schedule_serialize: Option>, pane_history_timestamp: Arc, + call_factory: CallFactory, } impl EventEmitter for Workspace {} @@ -630,11 +478,13 @@ struct FollowerState { enum WorkspaceBounds {} +type CallFactory = fn(WeakView, &mut ViewContext) -> Box; impl Workspace { pub fn new( workspace_id: WorkspaceId, project: Model, app_state: Arc, + call_factory: CallFactory, cx: &mut ViewContext, ) -> Self { cx.observe(&project, |_, _, cx| cx.notify()).detach(); @@ -829,7 +679,7 @@ impl Workspace { last_leaders_by_pane: Default::default(), window_edited: false, - call_handler: Box::new(Call::new(weak_handle.clone(), cx)), + call_handler: call_factory(weak_handle.clone(), cx), database_id: workspace_id, app_state, _observe_current_user, @@ -839,6 +689,7 @@ impl Workspace { subscriptions, pane_history_timestamp, workspace_actions: Default::default(), + call_factory, } } @@ -846,6 +697,7 @@ impl Workspace { abs_paths: Vec, app_state: Arc, requesting_window: Option>, + call_factory: CallFactory, cx: &mut AppContext, ) -> Task< anyhow::Result<( @@ -896,7 +748,13 @@ impl Workspace { let window = if let Some(window) = requesting_window { cx.update_window(window.into(), |old_workspace, cx| { cx.replace_root_view(|cx| { - Workspace::new(workspace_id, project_handle.clone(), app_state.clone(), cx) + Workspace::new( + workspace_id, + project_handle.clone(), + app_state.clone(), + call_factory, + cx, + ) }); })?; window @@ -942,7 +800,13 @@ impl Workspace { let project_handle = project_handle.clone(); move |cx| { cx.build_view(|cx| { - Workspace::new(workspace_id, project_handle, app_state, cx) + Workspace::new( + workspace_id, + project_handle, + app_state, + call_factory, + cx, + ) }) } })? @@ -1203,7 +1067,13 @@ impl Workspace { if self.project.read(cx).is_local() { Task::Ready(Some(Ok(callback(self, cx)))) } else { - let task = Self::new_local(Vec::new(), self.app_state.clone(), None, cx); + let task = Self::new_local( + Vec::new(), + self.app_state.clone(), + None, + self.call_factory, + cx, + ); cx.spawn(|_vh, mut cx| async move { let (workspace, _) = task.await?; workspace.update(&mut cx, callback) @@ -1432,7 +1302,7 @@ impl Workspace { Some(self.prepare_to_close(false, cx)) }; let app_state = self.app_state.clone(); - + let call_factory = self.call_factory; cx.spawn(|_, mut cx| async move { let window_to_replace = if let Some(close_task) = close_task { if !close_task.await? { @@ -1442,7 +1312,7 @@ impl Workspace { } else { None }; - cx.update(|_, cx| open_paths(&paths, &app_state, window_to_replace, cx))? + cx.update(|_, cx| open_paths(&paths, &app_state, window_to_replace, call_factory, cx))? .await?; Ok(()) }) @@ -4331,6 +4201,7 @@ pub fn open_paths( abs_paths: &[PathBuf], app_state: &Arc, requesting_window: Option>, + call_factory: CallFactory, cx: &mut AppContext, ) -> Task< anyhow::Result<( @@ -4357,7 +4228,13 @@ pub fn open_paths( todo!() } else { cx.update(move |cx| { - Workspace::new_local(abs_paths, app_state.clone(), requesting_window, cx) + Workspace::new_local( + abs_paths, + app_state.clone(), + requesting_window, + call_factory, + cx, + ) })? .await } @@ -4368,8 +4245,9 @@ pub fn open_new( app_state: &Arc, cx: &mut AppContext, init: impl FnOnce(&mut Workspace, &mut ViewContext) + 'static + Send, + call_factory: CallFactory, ) -> Task<()> { - let task = Workspace::new_local(Vec::new(), app_state.clone(), None, cx); + let task = Workspace::new_local(Vec::new(), app_state.clone(), None, call_factory, cx); cx.spawn(|mut cx| async move { if let Some((workspace, opened_paths)) = task.await.log_err() { workspace From abe5a9c85f909414d044bc1691af756fd64177be Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 21 Nov 2023 20:51:53 +0100 Subject: [PATCH 07/31] Finish up decoupling workspace from call --- Cargo.lock | 1 + crates/call2/Cargo.toml | 1 + crates/call2/src/call2.rs | 149 ++++++++++------------------ crates/workspace2/src/workspace2.rs | 126 ++++++++++++----------- crates/zed2/src/main.rs | 2 +- 5 files changed, 126 insertions(+), 153 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17bda4458c..b96e12a0fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1175,6 +1175,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-broadcast", + "async-trait", "audio2", "client2", "collections", diff --git a/crates/call2/Cargo.toml b/crates/call2/Cargo.toml index 500931cc11..43e19b4ccb 100644 --- a/crates/call2/Cargo.toml +++ b/crates/call2/Cargo.toml @@ -32,6 +32,7 @@ project = { package = "project2", path = "../project2" } settings = { package = "settings2", path = "../settings2" } util = { path = "../util" } workspace = {package = "workspace2", path = "../workspace2"} +async-trait.workspace = true anyhow.workspace = true async-broadcast = "0.4" futures.workspace = true diff --git a/crates/call2/src/call2.rs b/crates/call2/src/call2.rs index 34a9aabe14..18576d4657 100644 --- a/crates/call2/src/call2.rs +++ b/crates/call2/src/call2.rs @@ -2,24 +2,29 @@ pub mod call_settings; pub mod participant; pub mod room; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; +use async_trait::async_trait; use audio::Audio; use call_settings::CallSettings; -use client::{proto, Client, TelemetrySettings, TypedEnvelope, User, UserStore, ZED_ALWAYS_ACTIVE}; +use client::{ + proto::{self, PeerId}, + Client, TelemetrySettings, TypedEnvelope, User, UserStore, ZED_ALWAYS_ACTIVE, +}; use collections::HashSet; use futures::{channel::oneshot, future::Shared, Future, FutureExt}; use gpui::{ - AppContext, AsyncAppContext, Context, EventEmitter, Model, ModelContext, Subscription, Task, - WeakModel, + AppContext, AsyncAppContext, AsyncWindowContext, Context, EventEmitter, Model, ModelContext, + Subscription, Task, View, ViewContext, WeakModel, WeakView, }; +pub use participant::ParticipantLocation; use postage::watch; use project::Project; use room::Event; +pub use room::Room; use settings::Settings; use std::sync::Arc; - -pub use participant::ParticipantLocation; -pub use room::Room; +use util::ResultExt; +use workspace::{item::ItemHandle, CallHandler, Pane, Workspace}; pub fn init(client: Arc, user_store: Model, cx: &mut AppContext) { CallSettings::register(cx); @@ -505,35 +510,36 @@ pub fn report_call_event_for_channel( ) } -struct Call { - follower_states: HashMap, FollowerState>, +pub struct Call { active_call: Option<(Model, Vec)>, parent_workspace: WeakView, } impl Call { - fn new(parent_workspace: WeakView, cx: &mut ViewContext<'_, Workspace>) -> Self { + pub fn new( + parent_workspace: WeakView, + cx: &mut ViewContext<'_, Workspace>, + ) -> Box { let mut active_call = None; if cx.has_global::>() { let call = cx.global::>().clone(); let subscriptions = vec![cx.subscribe(&call, Self::on_active_call_event)]; active_call = Some((call, subscriptions)); } - Self { - follower_states: Default::default(), + Box::new(Self { active_call, parent_workspace, - } + }) } fn on_active_call_event( workspace: &mut Workspace, _: Model, - event: &call2::room::Event, + event: &room::Event, cx: &mut ViewContext, ) { match event { - call2::room::Event::ParticipantLocationChanged { participant_id } - | call2::room::Event::RemoteVideoTracksChanged { participant_id } => { + room::Event::ParticipantLocationChanged { participant_id } + | room::Event::RemoteVideoTracksChanged { participant_id } => { workspace.leader_updated(*participant_id, cx); } _ => {} @@ -543,78 +549,6 @@ impl Call { #[async_trait(?Send)] impl CallHandler for Call { - fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()> { - cx.notify(); - - let (call, _) = self.active_call.as_ref()?; - let room = call.read(cx).room()?.read(cx); - let participant = room.remote_participant_for_peer_id(leader_id)?; - let mut items_to_activate = Vec::new(); - - let leader_in_this_app; - let leader_in_this_project; - match participant.location { - call2::ParticipantLocation::SharedProject { project_id } => { - leader_in_this_app = true; - leader_in_this_project = Some(project_id) - == self - .parent_workspace - .update(cx, |this, cx| this.project.read(cx).remote_id()) - .log_err() - .flatten(); - } - call2::ParticipantLocation::UnsharedProject => { - leader_in_this_app = true; - leader_in_this_project = false; - } - call2::ParticipantLocation::External => { - leader_in_this_app = false; - leader_in_this_project = false; - } - }; - - for (pane, state) in &self.follower_states { - if state.leader_id != leader_id { - continue; - } - if let (Some(active_view_id), true) = (state.active_view_id, leader_in_this_app) { - if let Some(item) = state.items_by_leader_view_id.get(&active_view_id) { - if leader_in_this_project || !item.is_project_item(cx) { - items_to_activate.push((pane.clone(), item.boxed_clone())); - } - } else { - log::warn!( - "unknown view id {:?} for leader {:?}", - active_view_id, - leader_id - ); - } - continue; - } - // todo!() - // if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) { - // items_to_activate.push((pane.clone(), Box::new(shared_screen))); - // } - } - - for (pane, item) in items_to_activate { - let pane_was_focused = pane.read(cx).has_focus(cx); - if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) { - pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx)); - } else { - pane.update(cx, |pane, mut cx| { - pane.add_item(item.boxed_clone(), false, false, None, &mut cx) - }); - } - - if pane_was_focused { - pane.update(cx, |pane, cx| pane.focus_active_item(cx)); - } - } - - None - } - fn shared_screen_for_peer( &self, peer_id: PeerId, @@ -638,12 +572,6 @@ impl CallHandler for Call { // }))) } - fn follower_states_mut(&mut self) -> &mut HashMap, FollowerState> { - &mut self.follower_states - } - fn follower_states(&self) -> &HashMap, FollowerState> { - &self.follower_states - } fn room_id(&self, cx: &AppContext) -> Option { Some(self.active_call.as_ref()?.0.read(cx).room()?.read(cx).id()) } @@ -657,6 +585,39 @@ impl CallHandler for Call { fn active_project(&self, cx: &AppContext) -> Option> { ActiveCall::global(cx).read(cx).location().cloned() } + fn peer_state( + &mut self, + leader_id: PeerId, + cx: &mut ViewContext, + ) -> Option<(bool, bool)> { + let (call, _) = self.active_call.as_ref()?; + let room = call.read(cx).room()?.read(cx); + let participant = room.remote_participant_for_peer_id(leader_id)?; + + let leader_in_this_app; + let leader_in_this_project; + match participant.location { + ParticipantLocation::SharedProject { project_id } => { + leader_in_this_app = true; + leader_in_this_project = Some(project_id) + == self + .parent_workspace + .update(cx, |this, cx| this.project().read(cx).remote_id()) + .log_err() + .flatten(); + } + ParticipantLocation::UnsharedProject => { + leader_in_this_app = true; + leader_in_this_project = false; + } + ParticipantLocation::External => { + leader_in_this_app = false; + leader_in_this_project = false; + } + }; + + Some((leader_in_this_project, leader_in_this_app)) + } } #[cfg(test)] diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index 05e994b74f..754988a605 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -15,7 +15,7 @@ mod status_bar; mod toolbar; mod workspace_settings; -use anyhow::{anyhow, bail, Context as _, Result}; +use anyhow::{anyhow, Context as _, Result}; use async_trait::async_trait; use client2::{ proto::{self, PeerId}, @@ -207,10 +207,10 @@ pub fn init_settings(cx: &mut AppContext) { ItemSettings::register(cx); } -pub fn init(app_state: Arc, cx: &mut AppContext) { +pub fn init(app_state: Arc, cx: &mut AppContext, call_factory: CallFactory) { init_settings(cx); notifications::init(cx); - + cx.set_global(call_factory); // cx.add_global_action({ // let app_state = Arc::downgrade(&app_state); // move |_: &Open, cx: &mut AppContext| { @@ -410,15 +410,13 @@ pub enum Event { #[async_trait(?Send)] pub trait CallHandler { - fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()>; + fn peer_state(&mut self, id: PeerId, cx: &mut ViewContext) -> Option<(bool, bool)>; fn shared_screen_for_peer( &self, peer_id: PeerId, pane: &View, cx: &mut ViewContext, ) -> Option>; - fn follower_states_mut(&mut self) -> &mut HashMap, FollowerState>; - fn follower_states(&self) -> &HashMap, FollowerState>; fn room_id(&self, cx: &AppContext) -> Option; fn is_in_room(&self, cx: &mut ViewContext) -> bool { self.room_id(cx).is_some() @@ -448,6 +446,7 @@ pub struct Workspace { notifications: Vec<(TypeId, usize, Box)>, project: Model, call_handler: Box, + follower_states: HashMap, FollowerState>, last_leaders_by_pane: HashMap, PeerId>, window_edited: bool, leader_updates_tx: mpsc::UnboundedSender<(PeerId, proto::UpdateFollowers)>, @@ -458,7 +457,6 @@ pub struct Workspace { _observe_current_user: Task>, _schedule_serialize: Option>, pane_history_timestamp: Arc, - call_factory: CallFactory, } impl EventEmitter for Workspace {} @@ -484,7 +482,6 @@ impl Workspace { workspace_id: WorkspaceId, project: Model, app_state: Arc, - call_factory: CallFactory, cx: &mut ViewContext, ) -> Self { cx.observe(&project, |_, _, cx| cx.notify()).detach(); @@ -656,6 +653,7 @@ impl Workspace { ]; cx.defer(|this, cx| this.update_window_title(cx)); + let call_factory = cx.global::(); Workspace { window_self: window_handle, weak_self: weak_handle.clone(), @@ -675,7 +673,7 @@ impl Workspace { bottom_dock, right_dock, project: project.clone(), - + follower_states: Default::default(), last_leaders_by_pane: Default::default(), window_edited: false, @@ -689,7 +687,6 @@ impl Workspace { subscriptions, pane_history_timestamp, workspace_actions: Default::default(), - call_factory, } } @@ -697,7 +694,6 @@ impl Workspace { abs_paths: Vec, app_state: Arc, requesting_window: Option>, - call_factory: CallFactory, cx: &mut AppContext, ) -> Task< anyhow::Result<( @@ -748,13 +744,7 @@ impl Workspace { let window = if let Some(window) = requesting_window { cx.update_window(window.into(), |old_workspace, cx| { cx.replace_root_view(|cx| { - Workspace::new( - workspace_id, - project_handle.clone(), - app_state.clone(), - call_factory, - cx, - ) + Workspace::new(workspace_id, project_handle.clone(), app_state.clone(), cx) }); })?; window @@ -800,13 +790,7 @@ impl Workspace { let project_handle = project_handle.clone(); move |cx| { cx.build_view(|cx| { - Workspace::new( - workspace_id, - project_handle, - app_state, - call_factory, - cx, - ) + Workspace::new(workspace_id, project_handle, app_state, cx) }) } })? @@ -1067,13 +1051,7 @@ impl Workspace { if self.project.read(cx).is_local() { Task::Ready(Some(Ok(callback(self, cx)))) } else { - let task = Self::new_local( - Vec::new(), - self.app_state.clone(), - None, - self.call_factory, - cx, - ); + let task = Self::new_local(Vec::new(), self.app_state.clone(), None, cx); cx.spawn(|_vh, mut cx| async move { let (workspace, _) = task.await?; workspace.update(&mut cx, callback) @@ -1302,7 +1280,7 @@ impl Workspace { Some(self.prepare_to_close(false, cx)) }; let app_state = self.app_state.clone(); - let call_factory = self.call_factory; + cx.spawn(|_, mut cx| async move { let window_to_replace = if let Some(close_task) = close_task { if !close_task.await? { @@ -1312,7 +1290,7 @@ impl Workspace { } else { None }; - cx.update(|_, cx| open_paths(&paths, &app_state, window_to_replace, call_factory, cx))? + cx.update(|_, cx| open_paths(&paths, &app_state, window_to_replace, cx))? .await?; Ok(()) }) @@ -2282,7 +2260,7 @@ impl Workspace { } fn collaborator_left(&mut self, peer_id: PeerId, cx: &mut ViewContext) { - self.call_handler.follower_states_mut().retain(|_, state| { + self.follower_states.retain(|_, state| { if state.leader_id == peer_id { for item in state.items_by_leader_view_id.values() { item.set_leader_peer_id(None, cx); @@ -2435,7 +2413,7 @@ impl Workspace { // } pub fn unfollow(&mut self, pane: &View, cx: &mut ViewContext) -> Option { - let follower_states = self.call_handler.follower_states_mut(); + let follower_states = &mut self.follower_states; let state = follower_states.remove(pane)?; let leader_id = state.leader_id; for (_, item) in state.items_by_leader_view_id { @@ -2658,7 +2636,7 @@ impl Workspace { match update.variant.ok_or_else(|| anyhow!("invalid update"))? { proto::update_followers::Variant::UpdateActiveView(update_active_view) => { this.update(cx, |this, _| { - for (_, state) in this.call_handler.follower_states_mut() { + for (_, state) in &mut this.follower_states { if state.leader_id == leader_id { state.active_view_id = if let Some(active_view_id) = update_active_view.id.clone() { @@ -2681,7 +2659,7 @@ impl Workspace { let mut tasks = Vec::new(); this.update(cx, |this, cx| { let project = this.project.clone(); - for (_, state) in this.call_handler.follower_states_mut() { + for (_, state) in &mut this.follower_states { if state.leader_id == leader_id { let view_id = ViewId::from_proto(id.clone())?; if let Some(item) = state.items_by_leader_view_id.get(&view_id) { @@ -2695,8 +2673,7 @@ impl Workspace { } proto::update_followers::Variant::CreateView(view) => { let panes = this.update(cx, |this, _| { - this.call_handler - .follower_states() + this.follower_states .iter() .filter_map(|(pane, state)| (state.leader_id == leader_id).then_some(pane)) .cloned() @@ -2756,7 +2733,7 @@ impl Workspace { for (pane, (item_tasks, leader_view_ids)) in item_tasks_by_pane { let items = futures::future::try_join_all(item_tasks).await?; this.update(cx, |this, cx| { - let state = this.call_handler.follower_states_mut().get_mut(&pane)?; + let state = this.follower_states.get_mut(&pane)?; for (id, item) in leader_view_ids.into_iter().zip(items) { item.set_leader_peer_id(Some(leader_id), cx); state.items_by_leader_view_id.insert(id, item); @@ -2813,14 +2790,55 @@ impl Workspace { } pub fn leader_for_pane(&self, pane: &View) -> Option { - self.call_handler - .follower_states() - .get(pane) - .map(|state| state.leader_id) + self.follower_states.get(pane).map(|state| state.leader_id) } - fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()> { - self.call_handler.leader_updated(leader_id, cx) + pub fn leader_updated(&mut self, leader_id: PeerId, cx: &mut ViewContext) -> Option<()> { + cx.notify(); + + let (leader_in_this_project, leader_in_this_app) = + self.call_handler.peer_state(leader_id, cx)?; + let mut items_to_activate = Vec::new(); + for (pane, state) in &self.follower_states { + if state.leader_id != leader_id { + continue; + } + if let (Some(active_view_id), true) = (state.active_view_id, leader_in_this_app) { + if let Some(item) = state.items_by_leader_view_id.get(&active_view_id) { + if leader_in_this_project || !item.is_project_item(cx) { + items_to_activate.push((pane.clone(), item.boxed_clone())); + } + } else { + log::warn!( + "unknown view id {:?} for leader {:?}", + active_view_id, + leader_id + ); + } + continue; + } + // todo!() + // if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) { + // items_to_activate.push((pane.clone(), Box::new(shared_screen))); + // } + } + + for (pane, item) in items_to_activate { + let pane_was_focused = pane.read(cx).has_focus(cx); + if let Some(index) = pane.update(cx, |pane, _| pane.index_for_item(item.as_ref())) { + pane.update(cx, |pane, cx| pane.activate_item(index, false, false, cx)); + } else { + pane.update(cx, |pane, mut cx| { + pane.add_item(item.boxed_clone(), false, false, None, &mut cx) + }); + } + + if pane_was_focused { + pane.update(cx, |pane, cx| pane.focus_active_item(cx)); + } + } + + None } // todo!() @@ -3637,7 +3655,7 @@ impl Render for Workspace { .flex_1() .child(self.center.render( &self.project, - &self.call_handler.follower_states(), + &self.follower_states, &self.active_pane, self.zoomed.as_ref(), &self.app_state, @@ -4201,7 +4219,6 @@ pub fn open_paths( abs_paths: &[PathBuf], app_state: &Arc, requesting_window: Option>, - call_factory: CallFactory, cx: &mut AppContext, ) -> Task< anyhow::Result<( @@ -4228,13 +4245,7 @@ pub fn open_paths( todo!() } else { cx.update(move |cx| { - Workspace::new_local( - abs_paths, - app_state.clone(), - requesting_window, - call_factory, - cx, - ) + Workspace::new_local(abs_paths, app_state.clone(), requesting_window, cx) })? .await } @@ -4245,9 +4256,8 @@ pub fn open_new( app_state: &Arc, cx: &mut AppContext, init: impl FnOnce(&mut Workspace, &mut ViewContext) + 'static + Send, - call_factory: CallFactory, ) -> Task<()> { - let task = Workspace::new_local(Vec::new(), app_state.clone(), None, call_factory, cx); + let task = Workspace::new_local(Vec::new(), app_state.clone(), None, cx); cx.spawn(|mut cx| async move { if let Some((workspace, opened_paths)) = task.await.log_err() { workspace diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index 9c42badb85..62d337a716 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -189,7 +189,7 @@ fn main() { // audio::init(Assets, cx); auto_update::init(http.clone(), client::ZED_SERVER_URL.clone(), cx); - workspace::init(app_state.clone(), cx); + workspace::init(app_state.clone(), cx, call::Call::new); // recent_projects::init(cx); go_to_line::init(cx); From e557eb4afe05195f7196f118270816d7a1ad73d6 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 21 Nov 2023 12:45:25 -0800 Subject: [PATCH 08/31] Fix no window showing up on startup co-authored-by: Marshall --- crates/zed2/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index 9c42badb85..c1db4eace4 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -355,7 +355,7 @@ async fn restore_or_create_workspace(app_state: &Arc, mut cx: AsyncApp cx.update(|cx| workspace::open_paths(location.paths().as_ref(), app_state, None, cx))? .await .log_err(); - } else if matches!(KEY_VALUE_STORE.read_kvp("******* THIS IS A BAD KEY PLEASE UNCOMMENT BELOW TO FIX THIS VERY LONG LINE *******"), Ok(None)) { + // todo!(welcome) //} else if matches!(KEY_VALUE_STORE.read_kvp(FIRST_OPEN), Ok(None)) { //todo!() From 7e7a778d116938d9ef2ff219f7a4e73c952e00ae Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 21 Nov 2023 22:04:02 +0100 Subject: [PATCH 09/31] Move CallFactory into AppState Fix crash caused by double borrow of window handle --- crates/workspace2/src/workspace2.rs | 17 ++++++----------- crates/zed2/src/main.rs | 3 ++- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index 754988a605..e1e79c4d3e 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -207,10 +207,9 @@ pub fn init_settings(cx: &mut AppContext) { ItemSettings::register(cx); } -pub fn init(app_state: Arc, cx: &mut AppContext, call_factory: CallFactory) { +pub fn init(app_state: Arc, cx: &mut AppContext) { init_settings(cx); notifications::init(cx); - cx.set_global(call_factory); // cx.add_global_action({ // let app_state = Arc::downgrade(&app_state); // move |_: &Open, cx: &mut AppContext| { @@ -304,6 +303,7 @@ pub struct AppState { pub user_store: Model, pub workspace_store: Model, pub fs: Arc, + pub call_factory: CallFactory, pub build_window_options: fn(Option, Option, &mut AppContext) -> WindowOptions, pub node_runtime: Arc, @@ -653,7 +653,6 @@ impl Workspace { ]; cx.defer(|this, cx| this.update_window_title(cx)); - let call_factory = cx.global::(); Workspace { window_self: window_handle, weak_self: weak_handle.clone(), @@ -677,7 +676,7 @@ impl Workspace { last_leaders_by_pane: Default::default(), window_edited: false, - call_handler: call_factory(weak_handle.clone(), cx), + call_handler: (app_state.call_factory)(weak_handle.clone(), cx), database_id: workspace_id, app_state, _observe_current_user, @@ -2784,8 +2783,9 @@ impl Workspace { } else { None }; + let room_id = self.call_handler.room_id(cx)?; self.app_state().workspace_store.update(cx, |store, cx| { - store.update_followers(project_id, update, cx) + store.update_followers(project_id, room_id, update, cx) }) } @@ -3825,15 +3825,10 @@ impl WorkspaceStore { pub fn update_followers( &self, project_id: Option, + room_id: u64, update: proto::update_followers::Variant, cx: &AppContext, ) -> Option<()> { - let room_id = self.workspaces.iter().next().and_then(|workspace| { - workspace - .read_with(cx, |this, cx| this.call_handler.room_id(cx)) - .log_err() - .flatten() - })?; let follower_ids: Vec<_> = self .followers .iter() diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index 62d337a716..b0a03d8684 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -180,6 +180,7 @@ fn main() { user_store, fs, build_window_options, + call_factory: call::Call::new, // background_actions: todo!("ask Mikayla"), workspace_store, node_runtime, @@ -189,7 +190,7 @@ fn main() { // audio::init(Assets, cx); auto_update::init(http.clone(), client::ZED_SERVER_URL.clone(), cx); - workspace::init(app_state.clone(), cx, call::Call::new); + workspace::init(app_state.clone(), cx); // recent_projects::init(cx); go_to_line::init(cx); From 469b05684f261149c20a8e8438c4da9d813ce4ab Mon Sep 17 00:00:00 2001 From: Mikayla Date: Tue, 21 Nov 2023 17:11:38 -0800 Subject: [PATCH 10/31] Fix a few identity mixups in GPUI co-authored-by: nathan --- crates/diagnostics2/src/items.rs | 2 +- crates/gpui2/src/element.rs | 6 +-- crates/gpui2/src/view.rs | 22 ++++------ crates/gpui2/src/window.rs | 68 ++++++++++++++++++++++++----- crates/workspace2/src/dock.rs | 2 +- crates/workspace2/src/pane.rs | 4 +- crates/workspace2/src/pane_group.rs | 24 +++++----- 7 files changed, 84 insertions(+), 44 deletions(-) diff --git a/crates/diagnostics2/src/items.rs b/crates/diagnostics2/src/items.rs index bbcfa748d4..ac24b7ad50 100644 --- a/crates/diagnostics2/src/items.rs +++ b/crates/diagnostics2/src/items.rs @@ -44,7 +44,7 @@ impl Render for DiagnosticIndicator { }; h_stack() - .id(cx.entity_id()) + .id("diagnostic-indicator") .on_action(cx.listener(Self::go_to_next_diagnostic)) .rounded_md() .flex_none() diff --git a/crates/gpui2/src/element.rs b/crates/gpui2/src/element.rs index 5cd015503d..1045e6218c 100644 --- a/crates/gpui2/src/element.rs +++ b/crates/gpui2/src/element.rs @@ -432,10 +432,6 @@ impl AnyElement { AnyElement(Box::new(Some(DrawableElement::new(element))) as Box) } - pub fn element_id(&self) -> Option { - self.0.element_id() - } - pub fn layout(&mut self, cx: &mut WindowContext) -> LayoutId { self.0.layout(cx) } @@ -490,7 +486,7 @@ impl RenderOnce for AnyElement { type Element = Self; fn element_id(&self) -> Option { - AnyElement::element_id(self) + None } fn render_once(self) -> Self::Element { diff --git a/crates/gpui2/src/view.rs b/crates/gpui2/src/view.rs index efa40627ac..c46707c7e2 100644 --- a/crates/gpui2/src/view.rs +++ b/crates/gpui2/src/view.rs @@ -248,7 +248,7 @@ impl RenderOnce for View { type Element = View; fn element_id(&self) -> Option { - Some(self.model.entity_id.into()) + Some(ElementId::from_entity_id(self.model.entity_id)) } fn render_once(self) -> Self::Element { @@ -260,7 +260,7 @@ impl RenderOnce for AnyView { type Element = Self; fn element_id(&self) -> Option { - Some(self.model.entity_id.into()) + Some(ElementId::from_entity_id(self.model.entity_id)) } fn render_once(self) -> Self::Element { @@ -308,27 +308,23 @@ where } mod any_view { - use crate::{AnyElement, AnyView, BorrowWindow, Element, LayoutId, Render, WindowContext}; + use crate::{AnyElement, AnyView, Element, LayoutId, Render, WindowContext}; pub(crate) fn layout( view: &AnyView, cx: &mut WindowContext, ) -> (LayoutId, AnyElement) { - cx.with_element_id(Some(view.model.entity_id), |cx| { - let view = view.clone().downcast::().unwrap(); - let mut element = view.update(cx, |view, cx| view.render(cx).into_any()); - let layout_id = element.layout(cx); - (layout_id, element) - }) + let view = view.clone().downcast::().unwrap(); + let mut element = view.update(cx, |view, cx| view.render(cx).into_any()); + let layout_id = element.layout(cx); + (layout_id, element) } pub(crate) fn paint( - view: &AnyView, + _view: &AnyView, element: AnyElement, cx: &mut WindowContext, ) { - cx.with_element_id(Some(view.model.entity_id), |cx| { - element.paint(cx); - }) + element.paint(cx); } } diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 483a8fdbee..1973aa14a9 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -230,9 +230,15 @@ pub struct Window { pub(crate) focus: Option, } +pub(crate) struct ElementStateBox { + inner: Box, + #[cfg(debug_assertions)] + type_name: &'static str, +} + // #[derive(Default)] pub(crate) struct Frame { - pub(crate) element_states: HashMap>, + pub(crate) element_states: HashMap, mouse_listeners: HashMap>, pub(crate) dispatch_tree: DispatchTree, pub(crate) focus_listeners: Vec, @@ -1815,10 +1821,37 @@ pub trait BorrowWindow: BorrowMut + BorrowMut { .remove(&global_id) }) { + let ElementStateBox { + inner, + + #[cfg(debug_assertions)] + type_name + } = any; // Using the extra inner option to avoid needing to reallocate a new box. - let mut state_box = any + let mut state_box = inner .downcast::>() - .expect("invalid element state type for id"); + .map_err(|_| { + #[cfg(debug_assertions)] + { + anyhow!( + "invalid element state type for id, requested_type {:?}, actual type: {:?}", + std::any::type_name::(), + type_name + ) + } + + #[cfg(not(debug_assertions))] + { + anyhow!( + "invalid element state type for id, requested_type {:?}", + std::any::type_name::(), + ) + } + }) + .unwrap(); + + // Actual: Option <- View + // Requested: () <- AnyElemet let state = state_box .take() .expect("element state is already on the stack"); @@ -1827,14 +1860,27 @@ pub trait BorrowWindow: BorrowMut + BorrowMut { cx.window_mut() .current_frame .element_states - .insert(global_id, state_box); + .insert(global_id, ElementStateBox { + inner: state_box, + + #[cfg(debug_assertions)] + type_name + }); result } else { let (result, state) = f(None, cx); cx.window_mut() .current_frame .element_states - .insert(global_id, Box::new(Some(state))); + .insert(global_id, + ElementStateBox { + inner: Box::new(Some(state)), + + #[cfg(debug_assertions)] + type_name: std::any::type_name::() + } + + ); result } }) @@ -2599,6 +2645,12 @@ pub enum ElementId { FocusHandle(FocusId), } +impl ElementId { + pub(crate) fn from_entity_id(entity_id: EntityId) -> Self { + ElementId::View(entity_id) + } +} + impl TryInto for ElementId { type Error = anyhow::Error; @@ -2611,12 +2663,6 @@ impl TryInto for ElementId { } } -impl From for ElementId { - fn from(id: EntityId) -> Self { - ElementId::View(id) - } -} - impl From for ElementId { fn from(id: usize) -> Self { ElementId::Integer(id) diff --git a/crates/workspace2/src/dock.rs b/crates/workspace2/src/dock.rs index b44178ceb7..8bc07b6e90 100644 --- a/crates/workspace2/src/dock.rs +++ b/crates/workspace2/src/dock.rs @@ -721,7 +721,7 @@ impl Render for PanelButtons { let panel = panel.clone(); menu = menu.entry( ListItem::new( - panel.entity_id(), + position.to_label(), Label::new(format!("Dock {}", position.to_label())), ), move |_, cx| { diff --git a/crates/workspace2/src/pane.rs b/crates/workspace2/src/pane.rs index cdc341d7dc..f742be10f4 100644 --- a/crates/workspace2/src/pane.rs +++ b/crates/workspace2/src/pane.rs @@ -1350,7 +1350,7 @@ impl Pane { let id = item.item_id(); div() - .id(item.item_id()) + .id(ix) .invisible() .group_hover("", |style| style.visible()) .child( @@ -1382,7 +1382,7 @@ impl Pane { div() .group("") - .id(item.item_id()) + .id(ix) .cursor_pointer() .when_some(item.tab_tooltip_text(cx), |div, text| { div.tooltip(move |cx| cx.build_view(|cx| Tooltip::new(text.clone())).into()) diff --git a/crates/workspace2/src/pane_group.rs b/crates/workspace2/src/pane_group.rs index bd827a6dd7..3f11421778 100644 --- a/crates/workspace2/src/pane_group.rs +++ b/crates/workspace2/src/pane_group.rs @@ -214,7 +214,7 @@ impl Member { // Some(pane) // }; - div().size_full().child(pane.clone()) + div().size_full().child(pane.clone()).into_any() // Stack::new() // .with_child(pane_element.contained().with_border(leader_border)) @@ -230,16 +230,18 @@ impl Member { // .bg(cx.theme().colors().editor) // .children(); } - Member::Axis(axis) => axis.render( - project, - basis + 1, - follower_states, - active_call, - active_pane, - zoomed, - app_state, - cx, - ), + Member::Axis(axis) => axis + .render( + project, + basis + 1, + follower_states, + active_call, + active_pane, + zoomed, + app_state, + cx, + ) + .into_any(), } // enum FollowIntoExternalProject {} From a4a1e6ba98cd68f66e07fa01e0f24885913c1bbe Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Tue, 21 Nov 2023 17:29:06 -0500 Subject: [PATCH 11/31] WIP Co-authored-by: Mikayla --- crates/call2/src/call2.rs | 4 ++-- crates/client2/src/client2.rs | 2 +- crates/client2/src/telemetry.rs | 18 +++++++++++++++++- crates/collab2/src/tests/test_server.rs | 2 +- crates/gpui2/src/app.rs | 17 +++++++++++++++++ crates/project2/src/worktree_tests.rs | 6 +++--- crates/zed/src/main.rs | 8 ++++++++ 7 files changed, 49 insertions(+), 8 deletions(-) diff --git a/crates/call2/src/call2.rs b/crates/call2/src/call2.rs index 1f11e0650d..14cb28c32d 100644 --- a/crates/call2/src/call2.rs +++ b/crates/call2/src/call2.rs @@ -464,7 +464,7 @@ impl ActiveCall { &self.pending_invites } - pub fn report_call_event(&self, operation: &'static str, cx: &AppContext) { + pub fn report_call_event(&self, operation: &'static str, cx: &mut AppContext) { if let Some(room) = self.room() { let room = room.read(cx); report_call_event_for_room(operation, room.id(), room.channel_id(), &self.client, cx); @@ -477,7 +477,7 @@ pub fn report_call_event_for_room( room_id: u64, channel_id: Option, client: &Arc, - cx: &AppContext, + cx: &mut AppContext, ) { let telemetry = client.telemetry(); let telemetry_settings = *TelemetrySettings::get_global(cx); diff --git a/crates/client2/src/client2.rs b/crates/client2/src/client2.rs index b4279b023e..4ad354f2f9 100644 --- a/crates/client2/src/client2.rs +++ b/crates/client2/src/client2.rs @@ -382,7 +382,7 @@ impl settings::Settings for TelemetrySettings { } impl Client { - pub fn new(http: Arc, cx: &AppContext) -> Arc { + pub fn new(http: Arc, cx: &mut AppContext) -> Arc { Arc::new(Self { id: AtomicU64::new(0), peer: Peer::new(0), diff --git a/crates/client2/src/telemetry.rs b/crates/client2/src/telemetry.rs index 9bd24293a0..ddad1d5fda 100644 --- a/crates/client2/src/telemetry.rs +++ b/crates/client2/src/telemetry.rs @@ -1,5 +1,6 @@ use crate::{TelemetrySettings, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; use chrono::{DateTime, Utc}; +use futures::Future; use gpui::{serde_json, AppContext, AppMetadata, BackgroundExecutor, Task}; use lazy_static::lazy_static; use parking_lot::Mutex; @@ -126,12 +127,13 @@ const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(1); const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); impl Telemetry { - pub fn new(client: Arc, cx: &AppContext) -> Arc { + pub fn new(client: Arc, cx: &mut AppContext) -> Arc { let release_channel = if cx.has_global::() { Some(cx.global::().display_name()) } else { None }; + // TODO: Replace all hardware stuff with nested SystemSpecs json let this = Arc::new(Self { http_client: client, @@ -151,9 +153,22 @@ impl Telemetry { }), }); + // We should only ever have one instance of Telemetry, leak the subscription to keep it alive + // rather than store in TelemetryState, complicating spawn as subscriptions are not Send + std::mem::forget(cx.on_app_quit({ + let this = this.clone(); + move |cx| this.shutdown_telemetry(cx) + })); + this } + fn shutdown_telemetry(self: &Arc, cx: &mut AppContext) -> impl Future { + let telemetry_settings = TelemetrySettings::get_global(cx).clone(); + self.report_app_event(telemetry_settings, "close"); + Task::ready(()) + } + pub fn log_file_path(&self) -> Option { Some(self.state.lock().log_file.as_ref()?.path().to_path_buf()) } @@ -455,6 +470,7 @@ impl Telemetry { release_channel: state.release_channel, events, }; + dbg!(&request_body); json_bytes.clear(); serde_json::to_writer(&mut json_bytes, &request_body)?; } diff --git a/crates/collab2/src/tests/test_server.rs b/crates/collab2/src/tests/test_server.rs index 090a32d4ca..6bb57e11ab 100644 --- a/crates/collab2/src/tests/test_server.rs +++ b/crates/collab2/src/tests/test_server.rs @@ -149,7 +149,7 @@ impl TestServer { .user_id }; let client_name = name.to_string(); - let mut client = cx.read(|cx| Client::new(http.clone(), cx)); + let mut client = cx.update(|cx| Client::new(http.clone(), cx)); let server = self.server.clone(); let db = self.app_state.db.clone(); let connection_killers = self.connection_killers.clone(); diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index ff601db372..e928c22e49 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -10,6 +10,7 @@ pub use entity_map::*; pub use model_context::*; use refineable::Refineable; use smallvec::SmallVec; +use smol::future::FutureExt; #[cfg(any(test, feature = "test-support"))] pub use test_context::*; @@ -983,6 +984,22 @@ impl AppContext { pub fn all_action_names(&self) -> &[SharedString] { self.actions.all_action_names() } + + pub fn on_app_quit( + &mut self, + mut on_quit: impl FnMut(&mut AppContext) -> Fut + 'static, + ) -> Subscription + where + Fut: 'static + Future, + { + self.quit_observers.insert( + (), + Box::new(move |cx| { + let future = on_quit(cx); + async move { future.await }.boxed_local() + }), + ) + } } impl Context for AppContext { diff --git a/crates/project2/src/worktree_tests.rs b/crates/project2/src/worktree_tests.rs index df7307f694..a77f5396e1 100644 --- a/crates/project2/src/worktree_tests.rs +++ b/crates/project2/src/worktree_tests.rs @@ -1056,7 +1056,7 @@ async fn test_create_directory_during_initial_scan(cx: &mut TestAppContext) { async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { init_test(cx); cx.executor().allow_parking(); - let client_fake = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); + let client_fake = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let fs_fake = FakeFs::new(cx.background_executor.clone()); fs_fake @@ -1096,7 +1096,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { assert!(tree.entry_for_path("a/b/").unwrap().is_dir()); }); - let client_real = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); + let client_real = cx.update(|cx| Client::new(FakeHttpClient::with_404_response(), cx)); let fs_real = Arc::new(RealFs); let temp_root = temp_tree(json!({ @@ -2181,7 +2181,7 @@ async fn test_propagate_git_statuses(cx: &mut TestAppContext) { fn build_client(cx: &mut TestAppContext) -> Arc { let http_client = FakeHttpClient::with_404_response(); - cx.read(|cx| Client::new(http_client, cx)) + cx.update(|cx| Client::new(http_client, cx)) } #[track_caller] diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 20b93ae6bb..6fb6b2476f 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -766,3 +766,11 @@ pub fn background_actions() -> &'static [(&'static str, &'static dyn Action)] { ("Change your settings", &zed_actions::OpenSettings), ] } + +// TODO: +// Cleanly identify open / first open +// What should we do if we fail when looking for installation_id? +// - set to true, false, or skip? +// Report closed +// Copy logic to zed2 +// If we don't add an app close, we should prob add back the flush on startup? From eaf90a4fbd24423da6f9202da65f42a0d696f32d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Nov 2023 18:32:02 +0100 Subject: [PATCH 12/31] Fix drawing uniform list elements when scrolling --- crates/gpui2/src/elements/uniform_list.rs | 42 +++++++++++------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/crates/gpui2/src/elements/uniform_list.rs b/crates/gpui2/src/elements/uniform_list.rs index 3c3222c8a0..3649727648 100644 --- a/crates/gpui2/src/elements/uniform_list.rs +++ b/crates/gpui2/src/elements/uniform_list.rs @@ -1,7 +1,7 @@ use crate::{ - point, px, size, AnyElement, AvailableSpace, Bounds, Element, ElementId, InteractiveElement, - InteractiveElementState, Interactivity, LayoutId, Pixels, Point, Render, RenderOnce, Size, - StyleRefinement, Styled, View, ViewContext, WindowContext, + point, px, size, AnyElement, AvailableSpace, BorrowWindow, Bounds, ContentMask, Element, + ElementId, InteractiveElement, InteractiveElementState, Interactivity, LayoutId, Pixels, Point, + Render, RenderOnce, Size, StyleRefinement, Styled, View, ViewContext, WindowContext, }; use smallvec::SmallVec; use std::{cell::RefCell, cmp, ops::Range, rc::Rc}; @@ -210,31 +210,31 @@ impl Element for UniformList { scroll_offset: shared_scroll_offset, }); } - let visible_item_count = if item_height > px(0.) { - (padded_bounds.size.height / item_height).ceil() as usize + 1 - } else { - 0 - }; let first_visible_element_ix = (-scroll_offset.y / item_height).floor() as usize; + let last_visible_element_ix = + ((-scroll_offset.y + padded_bounds.size.height) / item_height).ceil() + as usize; let visible_range = first_visible_element_ix - ..cmp::min( - first_visible_element_ix + visible_item_count, - self.item_count, - ); + ..cmp::min(last_visible_element_ix, self.item_count); let items = (self.render_items)(visible_range.clone(), cx); cx.with_z_index(1, |cx| { - for (item, ix) in items.into_iter().zip(visible_range) { - let item_origin = padded_bounds.origin - + point(px(0.), item_height * ix + scroll_offset.y); - let available_space = size( - AvailableSpace::Definite(padded_bounds.size.width), - AvailableSpace::Definite(item_height), - ); - item.draw(item_origin, available_space, cx); - } + let content_mask = ContentMask { + bounds: padded_bounds, + }; + cx.with_content_mask(Some(content_mask), |cx| { + for (item, ix) in items.into_iter().zip(visible_range) { + let item_origin = padded_bounds.origin + + point(px(0.), item_height * ix + scroll_offset.y); + let available_space = size( + AvailableSpace::Definite(padded_bounds.size.width), + AvailableSpace::Definite(item_height), + ); + item.draw(item_origin, available_space, cx); + } + }); }); } }) From 2c8d243d2223ab7e5ab3bfd1ba06cb1d49269e3a Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Wed, 22 Nov 2023 12:41:06 -0500 Subject: [PATCH 13/31] Comment out `todo!()` to fix panic when opening context menus --- crates/project_panel2/src/project_panel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/project_panel2/src/project_panel.rs b/crates/project_panel2/src/project_panel.rs index 4d1a6ee8f7..0550fc7bd2 100644 --- a/crates/project_panel2/src/project_panel.rs +++ b/crates/project_panel2/src/project_panel.rs @@ -371,7 +371,7 @@ impl ProjectPanel { _entry_id: ProjectEntryId, _cx: &mut ViewContext, ) { - todo!() + // todo!() // let project = self.project.read(cx); // let worktree_id = if let Some(id) = project.worktree_id_for_entry(entry_id, cx) { From 031fca4105b84b6c1227e238a850773ca207f829 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Wed, 22 Nov 2023 12:41:29 -0500 Subject: [PATCH 14/31] Simplify `ContextMenu` by not storing list components --- crates/terminal_view2/src/terminal_view.rs | 9 ++---- crates/ui2/src/components/context_menu.rs | 32 +++++++++---------- crates/ui2/src/components/list.rs | 17 ---------- .../src/components/stories/context_menu.rs | 20 +++++------- crates/workspace2/src/dock.rs | 9 ++---- 5 files changed, 29 insertions(+), 58 deletions(-) diff --git a/crates/terminal_view2/src/terminal_view.rs b/crates/terminal_view2/src/terminal_view.rs index 5a5f74f9e1..9f3ed31388 100644 --- a/crates/terminal_view2/src/terminal_view.rs +++ b/crates/terminal_view2/src/terminal_view.rs @@ -31,7 +31,7 @@ use workspace::{ notifications::NotifyResultExt, register_deserializable_item, searchable::{SearchEvent, SearchOptions, SearchableItem}, - ui::{ContextMenu, Icon, IconElement, Label, ListItem}, + ui::{ContextMenu, Icon, IconElement, Label}, CloseActiveItem, NewCenterTerminal, Pane, ToolbarItemLocation, Workspace, WorkspaceId, }; @@ -299,11 +299,8 @@ impl TerminalView { cx: &mut ViewContext, ) { self.context_menu = Some(ContextMenu::build(cx, |menu, _| { - menu.action(ListItem::new("clear", Label::new("Clear")), Box::new(Clear)) - .action( - ListItem::new("close", Label::new("Close")), - Box::new(CloseActiveItem { save_intent: None }), - ) + menu.action("Clear", Box::new(Clear)) + .action("Close", Box::new(CloseActiveItem { save_intent: None })) })); dbg!(&position); // todo!() diff --git a/crates/ui2/src/components/context_menu.rs b/crates/ui2/src/components/context_menu.rs index 8bb5b2e5d2..81cc3892ee 100644 --- a/crates/ui2/src/components/context_menu.rs +++ b/crates/ui2/src/components/context_menu.rs @@ -1,7 +1,7 @@ use std::cell::RefCell; use std::rc::Rc; -use crate::{prelude::*, v_stack, List}; +use crate::{prelude::*, v_stack, Label, List}; use crate::{ListItem, ListSeparator, ListSubHeader}; use gpui::{ overlay, px, Action, AnchorCorner, AnyElement, AppContext, Bounds, ClickEvent, DispatchPhase, @@ -10,9 +10,9 @@ use gpui::{ }; pub enum ContextMenuItem { - Separator(ListSeparator), - Header(ListSubHeader), - Entry(ListItem, Rc), + Separator, + Header(SharedString), + Entry(SharedString, Rc), } pub struct ContextMenu { @@ -46,29 +46,30 @@ impl ContextMenu { } pub fn header(mut self, title: impl Into) -> Self { - self.items - .push(ContextMenuItem::Header(ListSubHeader::new(title))); + self.items.push(ContextMenuItem::Header(title.into())); self } pub fn separator(mut self) -> Self { - self.items.push(ContextMenuItem::Separator(ListSeparator)); + self.items.push(ContextMenuItem::Separator); self } pub fn entry( mut self, - view: ListItem, + label: impl Into, on_click: impl Fn(&ClickEvent, &mut WindowContext) + 'static, ) -> Self { self.items - .push(ContextMenuItem::Entry(view, Rc::new(on_click))); + .push(ContextMenuItem::Entry(label.into(), Rc::new(on_click))); self } - pub fn action(self, view: ListItem, action: Box) -> Self { + pub fn action(self, label: impl Into, action: Box) -> Self { // todo: add the keybindings to the list entry - self.entry(view, move |_, cx| cx.dispatch_action(action.boxed_clone())) + self.entry(label.into(), move |_, cx| { + cx.dispatch_action(action.boxed_clone()) + }) } pub fn confirm(&mut self, _: &menu::Confirm, cx: &mut ViewContext) { @@ -104,16 +105,15 @@ impl Render for ContextMenu { // .border_color(cx.theme().colors().border) .child( List::new().children(self.items.iter().map(|item| match item { - ContextMenuItem::Separator(separator) => { - separator.clone().render_into_any() + ContextMenuItem::Separator => ListSeparator::new().render_into_any(), + ContextMenuItem::Header(header) => { + ListSubHeader::new(header.clone()).render_into_any() } - ContextMenuItem::Header(header) => header.clone().render_into_any(), ContextMenuItem::Entry(entry, callback) => { let callback = callback.clone(); let dismiss = cx.listener(|_, _, cx| cx.emit(Manager::Dismiss)); - entry - .clone() + ListItem::new(entry.clone(), Label::new(entry.clone())) .on_click(move |event, cx| { callback(event, cx); dismiss(event, cx) diff --git a/crates/ui2/src/components/list.rs b/crates/ui2/src/components/list.rs index 0266ae3342..2cf37d3b65 100644 --- a/crates/ui2/src/components/list.rs +++ b/crates/ui2/src/components/list.rs @@ -254,23 +254,6 @@ pub struct ListItem { on_click: Option>, } -impl Clone for ListItem { - fn clone(&self) -> Self { - Self { - id: self.id.clone(), - disabled: self.disabled, - indent_level: self.indent_level, - label: self.label.clone(), - left_slot: self.left_slot.clone(), - overflow: self.overflow, - size: self.size, - toggle: self.toggle, - variant: self.variant, - on_click: self.on_click.clone(), - } - } -} - impl ListItem { pub fn new(id: impl Into, label: Label) -> Self { Self { diff --git a/crates/ui2/src/components/stories/context_menu.rs b/crates/ui2/src/components/stories/context_menu.rs index dd0bc03a21..98faea70aa 100644 --- a/crates/ui2/src/components/stories/context_menu.rs +++ b/crates/ui2/src/components/stories/context_menu.rs @@ -2,7 +2,7 @@ use gpui::{actions, Action, AnchorCorner, Div, Render, View}; use story::Story; use crate::prelude::*; -use crate::{menu_handle, ContextMenu, Label, ListItem}; +use crate::{menu_handle, ContextMenu, Label}; actions!(PrintCurrentDate, PrintBestFood); @@ -10,17 +10,13 @@ fn build_menu(cx: &mut WindowContext, header: impl Into) -> View Date: Wed, 22 Nov 2023 12:44:51 -0500 Subject: [PATCH 15/31] Use `children` for `ListItem`s --- crates/ui2/src/components/context_menu.rs | 3 ++- crates/ui2/src/components/list.rs | 14 ++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/ui2/src/components/context_menu.rs b/crates/ui2/src/components/context_menu.rs index 81cc3892ee..2473bff610 100644 --- a/crates/ui2/src/components/context_menu.rs +++ b/crates/ui2/src/components/context_menu.rs @@ -113,7 +113,8 @@ impl Render for ContextMenu { let callback = callback.clone(); let dismiss = cx.listener(|_, _, cx| cx.emit(Manager::Dismiss)); - ListItem::new(entry.clone(), Label::new(entry.clone())) + ListItem::new(entry.clone()) + .child(Label::new(entry.clone())) .on_click(move |event, cx| { callback(event, cx); dismiss(event, cx) diff --git a/crates/ui2/src/components/list.rs b/crates/ui2/src/components/list.rs index 2cf37d3b65..7319640b9e 100644 --- a/crates/ui2/src/components/list.rs +++ b/crates/ui2/src/components/list.rs @@ -245,28 +245,28 @@ pub struct ListItem { // TODO: Reintroduce this // disclosure_control_style: DisclosureControlVisibility, indent_level: u32, - label: Label, left_slot: Option, overflow: OverflowStyle, size: ListEntrySize, toggle: Toggle, variant: ListItemVariant, on_click: Option>, + children: SmallVec<[AnyElement; 2]>, } impl ListItem { - pub fn new(id: impl Into, label: Label) -> Self { + pub fn new(id: impl Into) -> Self { Self { id: id.into(), disabled: false, indent_level: 0, - label, left_slot: None, overflow: OverflowStyle::Hidden, size: ListEntrySize::default(), toggle: Toggle::NotToggleable, variant: ListItemVariant::default(), on_click: Default::default(), + children: SmallVec::new(), } } @@ -377,11 +377,17 @@ impl Component for ListItem { .relative() .child(disclosure_control(self.toggle)) .children(left_content) - .child(self.label), + .children(self.children), ) } } +impl ParentElement for ListItem { + fn children_mut(&mut self) -> &mut SmallVec<[AnyElement; 2]> { + &mut self.children + } +} + #[derive(RenderOnce, Clone)] pub struct ListSeparator; From 7b0b87380d18861d2260ae76a4b6547a5b00938d Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Nov 2023 12:57:32 -0500 Subject: [PATCH 16/31] v0.115.x dev --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8f36f2445d..9cf13b56c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11493,7 +11493,7 @@ dependencies = [ [[package]] name = "zed" -version = "0.114.0" +version = "0.115.0" dependencies = [ "activity_indicator", "ai", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index ab8d5b7efe..0512eaca7a 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] description = "The fast, collaborative code editor." edition = "2021" name = "zed" -version = "0.114.0" +version = "0.115.0" publish = false [lib] From 524f892fb0a6d8e19c6d3ce3e0d0f8aac555d27b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Nov 2023 19:02:44 +0100 Subject: [PATCH 17/31] Correctly swap position of context menu --- crates/editor2/src/element.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/editor2/src/element.rs b/crates/editor2/src/element.rs index 7f6135087a..2cc6749e6b 100644 --- a/crates/editor2/src/element.rs +++ b/crates/editor2/src/element.rs @@ -1051,7 +1051,7 @@ impl EditorElement { } if list_origin.y + list_height > text_bounds.lower_right().y { - list_origin.y -= layout.position_map.line_height - list_height; + list_origin.y -= layout.position_map.line_height + list_height; } context_menu.draw(list_origin, available_space, cx); From 10c4df20e93bd91ffb7be1a4fa514fd25c0f4477 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Nov 2023 13:05:29 -0500 Subject: [PATCH 18/31] collab 0.29.0 --- Cargo.lock | 2 +- crates/collab/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9cf13b56c1..8a279b2450 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1664,7 +1664,7 @@ dependencies = [ [[package]] name = "collab" -version = "0.28.0" +version = "0.29.0" dependencies = [ "anyhow", "async-trait", diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index dea6e09245..bbaf521e15 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] default-run = "collab" edition = "2021" name = "collab" -version = "0.28.0" +version = "0.29.0" publish = false [[bin]] From 9abce4bdd908fa288d2daa6de1dfbab44358375b Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 22 Nov 2023 13:16:52 -0500 Subject: [PATCH 19/31] zed1: Cancel completion resolution when new list Co-Authored-By: Max Brunsfeld --- crates/editor/src/editor.rs | 43 +++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 2558aec121..9e01814698 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1001,17 +1001,18 @@ impl CompletionsMenu { fn pre_resolve_completion_documentation( &self, - project: Option>, + editor: &Editor, cx: &mut ViewContext, - ) { + ) -> Option> { let settings = settings::get::(cx); if !settings.show_completion_documentation { - return; + return None; } - let Some(project) = project else { - return; + let Some(project) = editor.project.clone() else { + return None; }; + let client = project.read(cx).client(); let language_registry = project.read(cx).languages().clone(); @@ -1021,7 +1022,7 @@ impl CompletionsMenu { let completions = self.completions.clone(); let completion_indices: Vec<_> = self.matches.iter().map(|m| m.candidate_id).collect(); - cx.spawn(move |this, mut cx| async move { + Some(cx.spawn(move |this, mut cx| async move { if is_remote { let Some(project_id) = project_id else { log::error!("Remote project without remote_id"); @@ -1083,8 +1084,7 @@ impl CompletionsMenu { _ = this.update(&mut cx, |_, cx| cx.notify()); } } - }) - .detach(); + })) } fn attempt_resolve_selected_completion_documentation( @@ -3580,7 +3580,8 @@ impl Editor { let id = post_inc(&mut self.next_completion_id); let task = cx.spawn(|this, mut cx| { async move { - let menu = if let Some(completions) = completions.await.log_err() { + let completions = completions.await.log_err(); + let (menu, pre_resolve_task) = if let Some(completions) = completions { let mut menu = CompletionsMenu { id, initial_position: position, @@ -3601,21 +3602,26 @@ impl Editor { selected_item: 0, list: Default::default(), }; + menu.filter(query.as_deref(), cx.background()).await; + if menu.matches.is_empty() { - None + (None, None) } else { - _ = this.update(&mut cx, |editor, cx| { - menu.pre_resolve_completion_documentation(editor.project.clone(), cx); - }); - Some(menu) + let pre_resolve_task = this + .update(&mut cx, |editor, cx| { + menu.pre_resolve_completion_documentation(editor, cx) + }) + .ok() + .flatten(); + (Some(menu), pre_resolve_task) } } else { - None + (None, None) }; this.update(&mut cx, |this, cx| { - this.completion_tasks.retain(|(task_id, _)| *task_id > id); + this.completion_tasks.retain(|(task_id, _)| *task_id >= id); let mut context_menu = this.context_menu.write(); match context_menu.as_ref() { @@ -3647,10 +3653,15 @@ impl Editor { } })?; + if let Some(pre_resolve_task) = pre_resolve_task { + pre_resolve_task.await; + } + Ok::<_, anyhow::Error>(()) } .log_err() }); + self.completion_tasks.push((id, task)); } From fa74c49dbbccc113e6a8d9f464a04e1193e30c2f Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Wed, 22 Nov 2023 19:23:58 +0100 Subject: [PATCH 20/31] Add dummy call handler for tests --- crates/collab2/src/tests/test_server.rs | 1 + crates/workspace2/src/workspace2.rs | 32 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/crates/collab2/src/tests/test_server.rs b/crates/collab2/src/tests/test_server.rs index 090a32d4ca..f620662f71 100644 --- a/crates/collab2/src/tests/test_server.rs +++ b/crates/collab2/src/tests/test_server.rs @@ -221,6 +221,7 @@ impl TestServer { fs: fs.clone(), build_window_options: |_, _, _| Default::default(), node_runtime: FakeNodeRuntime::new(), + call_factory: |_, _| Box::new(workspace::TestCallHandler), }); cx.update(|cx| { diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index e1e79c4d3e..b09b47d24c 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -322,6 +322,36 @@ struct Follower { peer_id: PeerId, } +#[cfg(any(test, feature = "test-support"))] +pub struct TestCallHandler; + +#[cfg(any(test, feature = "test-support"))] +impl CallHandler for TestCallHandler { + fn peer_state(&mut self, id: PeerId, cx: &mut ViewContext) -> Option<(bool, bool)> { + None + } + + fn shared_screen_for_peer( + &self, + peer_id: PeerId, + pane: &View, + cx: &mut ViewContext, + ) -> Option> { + None + } + + fn room_id(&self, cx: &AppContext) -> Option { + None + } + + fn hang_up(&self, cx: AsyncWindowContext) -> Result>> { + anyhow::bail!("TestCallHandler should not be hanging up") + } + + fn active_project(&self, cx: &AppContext) -> Option> { + None + } +} impl AppState { #[cfg(any(test, feature = "test-support"))] pub fn test(cx: &mut AppContext) -> Arc { @@ -352,6 +382,7 @@ impl AppState { workspace_store, node_runtime: FakeNodeRuntime::new(), build_window_options: |_, _, _| Default::default(), + call_factory: |_, _| Box::new(TestCallHandler), }) } } @@ -3298,6 +3329,7 @@ impl Workspace { fs: project.read(cx).fs().clone(), build_window_options: |_, _, _| Default::default(), node_runtime: FakeNodeRuntime::new(), + call_factory: |_, _| Box::new(TestCallHandler), }); let workspace = Self::new(0, project, app_state, cx); workspace.active_pane.update(cx, |pane, cx| pane.focus(cx)); From b45234eecea255cc3fef12b1c5fbf58dba40892e Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Wed, 22 Nov 2023 19:24:38 +0100 Subject: [PATCH 21/31] Fix warnings in unimplemented function --- crates/call2/src/call2.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/call2/src/call2.rs b/crates/call2/src/call2.rs index 18576d4657..9579552d5a 100644 --- a/crates/call2/src/call2.rs +++ b/crates/call2/src/call2.rs @@ -552,14 +552,14 @@ impl CallHandler for Call { fn shared_screen_for_peer( &self, peer_id: PeerId, - pane: &View, + _pane: &View, cx: &mut ViewContext, ) -> Option> { let (call, _) = self.active_call.as_ref()?; let room = call.read(cx).room()?.read(cx); let participant = room.remote_participant_for_peer_id(peer_id)?; - let track = participant.video_tracks.values().next()?.clone(); - let user = participant.user.clone(); + let _track = participant.video_tracks.values().next()?.clone(); + let _user = participant.user.clone(); todo!(); // for item in pane.read(cx).items_of_type::() { // if item.read(cx).peer_id == peer_id { From f0c7b3e6ee8618441386b7bdb6096512c8a2943d Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 22 Nov 2023 14:03:43 -0500 Subject: [PATCH 22/31] Update copilot when we are the last task --- crates/editor/src/editor.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9e01814698..17712b7e78 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3642,10 +3642,10 @@ impl Editor { drop(context_menu); this.discard_copilot_suggestion(cx); cx.notify(); - } else if this.completion_tasks.is_empty() { - // If there are no more completion tasks and the last menu was - // empty, we should hide it. If it was already hidden, we should - // also show the copilot suggestion when available. + } else if this.completion_tasks.len() <= 1 { + // If there are no more completion tasks (omitting ourself) and + // the last menu was empty, we should hide it. If it was already + // hidden, we should also show the copilot suggestion when available. drop(context_menu); if this.hide_context_menu(cx).is_none() { this.update_visible_copilot_suggestion(cx); From 6e4268a471749449fff232fab5d65fe38bab57b2 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Tue, 21 Nov 2023 21:11:17 -0500 Subject: [PATCH 23/31] Convert telemetry into a model Co-Authored-By: Julia <30666851+ForLoveOfCats@users.noreply.github.com> --- crates/auto_update2/src/auto_update.rs | 6 +- crates/call2/src/call2.rs | 17 +- crates/client2/src/client2.rs | 12 +- crates/client2/src/telemetry.rs | 261 ++++++++++++------------- crates/client2/src/user.rs | 12 +- crates/editor2/src/editor.rs | 40 ++-- crates/gpui2/src/app.rs | 31 ++- crates/zed2/src/main.rs | 18 +- crates/zed2/src/zed2.rs | 10 +- 9 files changed, 208 insertions(+), 199 deletions(-) diff --git a/crates/auto_update2/src/auto_update.rs b/crates/auto_update2/src/auto_update.rs index aeff68965f..88f225e412 100644 --- a/crates/auto_update2/src/auto_update.rs +++ b/crates/auto_update2/src/auto_update.rs @@ -302,7 +302,11 @@ impl AutoUpdater { let mut dmg_file = File::create(&dmg_path).await?; let (installation_id, release_channel, telemetry) = cx.update(|cx| { - let installation_id = cx.global::>().telemetry().installation_id(); + let installation_id = cx + .global::>() + .telemetry() + .read(cx) + .installation_id(); let release_channel = cx .has_global::() .then(|| cx.global::().display_name()); diff --git a/crates/call2/src/call2.rs b/crates/call2/src/call2.rs index 14cb28c32d..6a956a73d2 100644 --- a/crates/call2/src/call2.rs +++ b/crates/call2/src/call2.rs @@ -482,27 +482,26 @@ pub fn report_call_event_for_room( let telemetry = client.telemetry(); let telemetry_settings = *TelemetrySettings::get_global(cx); - telemetry.report_call_event(telemetry_settings, operation, Some(room_id), channel_id) + telemetry.update(cx, |this, cx| { + this.report_call_event(telemetry_settings, operation, Some(room_id), channel_id, cx) + }); } pub fn report_call_event_for_channel( operation: &'static str, channel_id: u64, client: &Arc, - cx: &AppContext, + cx: &mut AppContext, ) { let room = ActiveCall::global(cx).read(cx).room(); + let room_id = room.map(|r| r.read(cx).id()); let telemetry = client.telemetry(); - let telemetry_settings = *TelemetrySettings::get_global(cx); - telemetry.report_call_event( - telemetry_settings, - operation, - room.map(|r| r.read(cx).id()), - Some(channel_id), - ) + telemetry.update(cx, |this, cx| { + this.report_call_event(telemetry_settings, operation, room_id, Some(channel_id), cx) + }); } #[cfg(test)] diff --git a/crates/client2/src/client2.rs b/crates/client2/src/client2.rs index 4ad354f2f9..f7d0b787c0 100644 --- a/crates/client2/src/client2.rs +++ b/crates/client2/src/client2.rs @@ -121,7 +121,7 @@ pub struct Client { id: AtomicU64, peer: Arc, http: Arc, - telemetry: Arc, + telemetry: Model, state: RwLock, #[allow(clippy::type_complexity)] @@ -501,8 +501,12 @@ impl Client { })); } Status::SignedOut | Status::UpgradeRequired => { - cx.update(|cx| self.telemetry.set_authenticated_user_info(None, false, cx)) - .log_err(); + cx.update(|cx| { + self.telemetry.update(cx, |this, cx| { + this.set_authenticated_user_info(None, false, cx) + }) + }) + .log_err(); state._reconnect_task.take(); } _ => {} @@ -1320,7 +1324,7 @@ impl Client { } } - pub fn telemetry(&self) -> &Arc { + pub fn telemetry(&self) -> &Model { &self.telemetry } } diff --git a/crates/client2/src/telemetry.rs b/crates/client2/src/telemetry.rs index ddad1d5fda..ca7ddcca97 100644 --- a/crates/client2/src/telemetry.rs +++ b/crates/client2/src/telemetry.rs @@ -1,12 +1,12 @@ use crate::{TelemetrySettings, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; use chrono::{DateTime, Utc}; use futures::Future; -use gpui::{serde_json, AppContext, AppMetadata, BackgroundExecutor, Task}; +use gpui::{serde_json, AppContext, AppMetadata, Context, Model, ModelContext, Task}; use lazy_static::lazy_static; -use parking_lot::Mutex; use serde::Serialize; use settings::Settings; -use std::{env, io::Write, mem, path::PathBuf, sync::Arc, time::Duration}; +use std::io::Write; +use std::{env, mem, path::PathBuf, sync::Arc, time::Duration}; use sysinfo::{ CpuRefreshKind, Pid, PidExt, ProcessExt, ProcessRefreshKind, RefreshKind, System, SystemExt, }; @@ -16,11 +16,6 @@ use util::{channel::ReleaseChannel, TryFutureExt}; pub struct Telemetry { http_client: Arc, - executor: BackgroundExecutor, - state: Mutex, -} - -struct TelemetryState { metrics_id: Option>, // Per logged-in user installation_id: Option>, // Per app installation (different for dev, nightly, preview, and stable) session_id: Option>, // Per app launch @@ -127,7 +122,7 @@ const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(1); const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); impl Telemetry { - pub fn new(client: Arc, cx: &mut AppContext) -> Arc { + pub fn new(client: Arc, cx: &mut AppContext) -> Model { let release_channel = if cx.has_global::() { Some(cx.global::().display_name()) } else { @@ -135,57 +130,48 @@ impl Telemetry { }; // TODO: Replace all hardware stuff with nested SystemSpecs json - let this = Arc::new(Self { + let this = cx.build_model(|cx| Self { http_client: client, - executor: cx.background_executor().clone(), - state: Mutex::new(TelemetryState { - app_metadata: cx.app_metadata(), - architecture: env::consts::ARCH, - release_channel, - installation_id: None, - metrics_id: None, - session_id: None, - clickhouse_events_queue: Default::default(), - flush_clickhouse_events_task: Default::default(), - log_file: None, - is_staff: None, - first_event_datetime: None, - }), + app_metadata: cx.app_metadata(), + architecture: env::consts::ARCH, + release_channel, + installation_id: None, + metrics_id: None, + session_id: None, + clickhouse_events_queue: Default::default(), + flush_clickhouse_events_task: Default::default(), + log_file: None, + is_staff: None, + first_event_datetime: None, }); // We should only ever have one instance of Telemetry, leak the subscription to keep it alive // rather than store in TelemetryState, complicating spawn as subscriptions are not Send - std::mem::forget(cx.on_app_quit({ - let this = this.clone(); - move |cx| this.shutdown_telemetry(cx) - })); + std::mem::forget(this.update(cx, |_, cx| cx.on_app_quit(Self::shutdown_telemetry))); this } - fn shutdown_telemetry(self: &Arc, cx: &mut AppContext) -> impl Future { + fn shutdown_telemetry(&mut self, cx: &mut ModelContext) -> impl Future { let telemetry_settings = TelemetrySettings::get_global(cx).clone(); - self.report_app_event(telemetry_settings, "close"); + self.report_app_event(telemetry_settings, "close", cx); Task::ready(()) } pub fn log_file_path(&self) -> Option { - Some(self.state.lock().log_file.as_ref()?.path().to_path_buf()) + Some(self.log_file.as_ref()?.path().to_path_buf()) } pub fn start( - self: &Arc, + &mut self, installation_id: Option, session_id: String, - cx: &mut AppContext, + cx: &mut ModelContext, ) { - let mut state = self.state.lock(); - state.installation_id = installation_id.map(|id| id.into()); - state.session_id = Some(session_id.into()); - drop(state); + self.installation_id = installation_id.map(|id| id.into()); + self.session_id = Some(session_id.into()); - let this = self.clone(); - cx.spawn(|cx| async move { + cx.spawn(|this, mut cx| async move { // Avoiding calling `System::new_all()`, as there have been crashes related to it let refresh_kind = RefreshKind::new() .with_memory() // For memory usage @@ -221,23 +207,28 @@ impl Telemetry { break; }; - this.report_memory_event( - telemetry_settings, - process.memory(), - process.virtual_memory(), - ); - this.report_cpu_event( - telemetry_settings, - process.cpu_usage(), - system.cpus().len() as u32, - ); + this.update(&mut cx, |this, cx| { + this.report_memory_event( + telemetry_settings, + process.memory(), + process.virtual_memory(), + cx, + ); + this.report_cpu_event( + telemetry_settings, + process.cpu_usage(), + system.cpus().len() as u32, + cx, + ); + }) + .ok(); } }) .detach(); } pub fn set_authenticated_user_info( - self: &Arc, + &mut self, metrics_id: Option, is_staff: bool, cx: &AppContext, @@ -246,21 +237,20 @@ impl Telemetry { return; } - let mut state = self.state.lock(); let metrics_id: Option> = metrics_id.map(|id| id.into()); - state.metrics_id = metrics_id.clone(); - state.is_staff = Some(is_staff); - drop(state); + self.metrics_id = metrics_id.clone(); + self.is_staff = Some(is_staff); } pub fn report_editor_event( - self: &Arc, + &mut self, telemetry_settings: TelemetrySettings, file_extension: Option, vim_mode: bool, operation: &'static str, copilot_enabled: bool, copilot_enabled_for_language: bool, + cx: &ModelContext, ) { let event = ClickhouseEvent::Editor { file_extension, @@ -271,15 +261,16 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false) + self.report_clickhouse_event(event, telemetry_settings, false, cx) } pub fn report_copilot_event( - self: &Arc, + &mut self, telemetry_settings: TelemetrySettings, suggestion_id: Option, suggestion_accepted: bool, file_extension: Option, + cx: &ModelContext, ) { let event = ClickhouseEvent::Copilot { suggestion_id, @@ -288,15 +279,16 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false) + self.report_clickhouse_event(event, telemetry_settings, false, cx) } pub fn report_assistant_event( - self: &Arc, + &mut self, telemetry_settings: TelemetrySettings, conversation_id: Option, kind: AssistantKind, model: &'static str, + cx: &ModelContext, ) { let event = ClickhouseEvent::Assistant { conversation_id, @@ -305,15 +297,16 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false) + self.report_clickhouse_event(event, telemetry_settings, false, cx) } pub fn report_call_event( - self: &Arc, + &mut self, telemetry_settings: TelemetrySettings, operation: &'static str, room_id: Option, channel_id: Option, + cx: &ModelContext, ) { let event = ClickhouseEvent::Call { operation, @@ -322,14 +315,15 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false) + self.report_clickhouse_event(event, telemetry_settings, false, cx) } pub fn report_cpu_event( - self: &Arc, + &mut self, telemetry_settings: TelemetrySettings, usage_as_percentage: f32, core_count: u32, + cx: &ModelContext, ) { let event = ClickhouseEvent::Cpu { usage_as_percentage, @@ -337,14 +331,15 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false) + self.report_clickhouse_event(event, telemetry_settings, false, cx) } pub fn report_memory_event( - self: &Arc, + &mut self, telemetry_settings: TelemetrySettings, memory_in_bytes: u64, virtual_memory_in_bytes: u64, + cx: &ModelContext, ) { let event = ClickhouseEvent::Memory { memory_in_bytes, @@ -352,94 +347,90 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false) + self.report_clickhouse_event(event, telemetry_settings, false, cx) } // app_events are called at app open and app close, so flush is set to immediately send pub fn report_app_event( - self: &Arc, + &mut self, telemetry_settings: TelemetrySettings, operation: &'static str, + cx: &ModelContext, ) { let event = ClickhouseEvent::App { operation, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, true) + self.report_clickhouse_event(event, telemetry_settings, true, cx) } - fn milliseconds_since_first_event(&self) -> i64 { - let mut state = self.state.lock(); - match state.first_event_datetime { + fn milliseconds_since_first_event(&mut self) -> i64 { + match self.first_event_datetime { Some(first_event_datetime) => { let now: DateTime = Utc::now(); now.timestamp_millis() - first_event_datetime.timestamp_millis() } None => { - state.first_event_datetime = Some(Utc::now()); + self.first_event_datetime = Some(Utc::now()); 0 } } } fn report_clickhouse_event( - self: &Arc, + &mut self, event: ClickhouseEvent, telemetry_settings: TelemetrySettings, immediate_flush: bool, + cx: &ModelContext, ) { if !telemetry_settings.metrics { return; } - let mut state = self.state.lock(); - let signed_in = state.metrics_id.is_some(); - state - .clickhouse_events_queue + let signed_in = self.metrics_id.is_some(); + self.clickhouse_events_queue .push(ClickhouseEventWrapper { signed_in, event }); - if state.installation_id.is_some() { - if immediate_flush || state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { - drop(state); - self.flush_clickhouse_events(); + if self.installation_id.is_some() { + if immediate_flush || self.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { + self.flush_clickhouse_events(cx); } else { - let this = self.clone(); - let executor = self.executor.clone(); - state.flush_clickhouse_events_task = Some(self.executor.spawn(async move { - executor.timer(DEBOUNCE_INTERVAL).await; - this.flush_clickhouse_events(); + self.flush_clickhouse_events_task = Some(cx.spawn(|this, mut cx| async move { + smol::Timer::after(DEBOUNCE_INTERVAL).await; + this.update(&mut cx, |this, cx| this.flush_clickhouse_events(cx)) + .ok(); })); } } } - pub fn metrics_id(self: &Arc) -> Option> { - self.state.lock().metrics_id.clone() + pub fn metrics_id(&self) -> Option> { + self.metrics_id.clone() } - pub fn installation_id(self: &Arc) -> Option> { - self.state.lock().installation_id.clone() + pub fn installation_id(&self) -> Option> { + self.installation_id.clone() } - pub fn is_staff(self: &Arc) -> Option { - self.state.lock().is_staff + pub fn is_staff(&self) -> Option { + self.is_staff } - fn flush_clickhouse_events(self: &Arc) { - let mut state = self.state.lock(); - state.first_event_datetime = None; - let mut events = mem::take(&mut state.clickhouse_events_queue); - state.flush_clickhouse_events_task.take(); - drop(state); + fn flush_clickhouse_events(&mut self, cx: &ModelContext) { + self.first_event_datetime = None; + let mut events = mem::take(&mut self.clickhouse_events_queue); + self.flush_clickhouse_events_task.take(); - let this = self.clone(); - self.executor - .spawn( - async move { - let mut json_bytes = Vec::new(); + let http_client = self.http_client.clone(); - if let Some(file) = &mut this.state.lock().log_file { + cx.spawn(|this, mut cx| { + async move { + let mut json_bytes = Vec::new(); + + this.update(&mut cx, |this, _| { + if let Some(file) = &mut this.log_file { let file = file.as_file_mut(); for event in &mut events { json_bytes.clear(); @@ -449,39 +440,43 @@ impl Telemetry { } } - { - let state = this.state.lock(); - let request_body = ClickhouseEventRequestBody { - token: ZED_SECRET_CLIENT_TOKEN, - installation_id: state.installation_id.clone(), - session_id: state.session_id.clone(), - is_staff: state.is_staff.clone(), - app_version: state - .app_metadata - .app_version - .map(|version| version.to_string()), - os_name: state.app_metadata.os_name, - os_version: state - .app_metadata - .os_version - .map(|version| version.to_string()), - architecture: state.architecture, + std::io::Result::Ok(()) + })??; - release_channel: state.release_channel, - events, - }; - dbg!(&request_body); - json_bytes.clear(); - serde_json::to_writer(&mut json_bytes, &request_body)?; - } + if let Ok(Ok(json_bytes)) = this.update(&mut cx, |this, _| { + let request_body = ClickhouseEventRequestBody { + token: ZED_SECRET_CLIENT_TOKEN, + installation_id: this.installation_id.clone(), + session_id: this.session_id.clone(), + is_staff: this.is_staff.clone(), + app_version: this + .app_metadata + .app_version + .map(|version| version.to_string()), + os_name: this.app_metadata.os_name, + os_version: this + .app_metadata + .os_version + .map(|version| version.to_string()), + architecture: this.architecture, - this.http_client + release_channel: this.release_channel, + events, + }; + json_bytes.clear(); + serde_json::to_writer(&mut json_bytes, &request_body)?; + + std::io::Result::Ok(json_bytes) + }) { + http_client .post_json(CLICKHOUSE_EVENTS_URL.as_str(), json_bytes.into()) .await?; - anyhow::Ok(()) } - .log_err(), - ) - .detach(); + + anyhow::Ok(()) + } + .log_err() + }) + .detach(); } } diff --git a/crates/client2/src/user.rs b/crates/client2/src/user.rs index a5dba03d2d..5d115a3785 100644 --- a/crates/client2/src/user.rs +++ b/crates/client2/src/user.rs @@ -168,11 +168,13 @@ impl UserStore { cx.update(|cx| { if let Some(info) = info { cx.update_flags(info.staff, info.flags); - client.telemetry.set_authenticated_user_info( - Some(info.metrics_id.clone()), - info.staff, - cx, - ) + client.telemetry.update(cx, |this, cx| { + this.set_authenticated_user_info( + Some(info.metrics_id.clone()), + info.staff, + cx, + ) + }) } })?; diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index 5e40f5368e..b53ae376d2 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -8951,7 +8951,7 @@ impl Editor { &self, suggestion_id: Option, suggestion_accepted: bool, - cx: &AppContext, + cx: &mut AppContext, ) { let Some(project) = &self.project else { return }; @@ -8968,12 +8968,15 @@ impl Editor { let telemetry = project.read(cx).client().telemetry().clone(); let telemetry_settings = *TelemetrySettings::get_global(cx); - telemetry.report_copilot_event( - telemetry_settings, - suggestion_id, - suggestion_accepted, - file_extension, - ) + telemetry.update(cx, |this, cx| { + this.report_copilot_event( + telemetry_settings, + suggestion_id, + suggestion_accepted, + file_extension, + cx, + ) + }); } #[cfg(any(test, feature = "test-support"))] @@ -8981,7 +8984,7 @@ impl Editor { &self, _operation: &'static str, _file_extension: Option, - _cx: &AppContext, + _cx: &mut AppContext, ) { } @@ -8990,7 +8993,7 @@ impl Editor { &self, operation: &'static str, file_extension: Option, - cx: &AppContext, + cx: &mut AppContext, ) { let Some(project) = &self.project else { return }; @@ -9020,14 +9023,17 @@ impl Editor { .show_copilot_suggestions; let telemetry = project.read(cx).client().telemetry().clone(); - telemetry.report_editor_event( - telemetry_settings, - file_extension, - vim_mode, - operation, - copilot_enabled, - copilot_enabled_for_language, - ) + telemetry.update(cx, |this, cx| { + this.report_editor_event( + telemetry_settings, + file_extension, + vim_mode, + operation, + copilot_enabled, + copilot_enabled_for_language, + cx, + ) + }); } /// Copy the highlighted chunks to the clipboard as JSON. The format is an array of lines, diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index e928c22e49..41b514c137 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -10,7 +10,6 @@ pub use entity_map::*; pub use model_context::*; use refineable::Refineable; use smallvec::SmallVec; -use smol::future::FutureExt; #[cfg(any(test, feature = "test-support"))] pub use test_context::*; @@ -985,21 +984,21 @@ impl AppContext { self.actions.all_action_names() } - pub fn on_app_quit( - &mut self, - mut on_quit: impl FnMut(&mut AppContext) -> Fut + 'static, - ) -> Subscription - where - Fut: 'static + Future, - { - self.quit_observers.insert( - (), - Box::new(move |cx| { - let future = on_quit(cx); - async move { future.await }.boxed_local() - }), - ) - } + // pub fn on_app_quit( + // &mut self, + // mut on_quit: impl FnMut(&mut AppContext) -> Fut + 'static, + // ) -> Subscription + // where + // Fut: 'static + Future, + // { + // self.quit_observers.insert( + // (), + // Box::new(move |cx| { + // let future = on_quit(cx); + // async move { future.await }.boxed_local() + // }), + // ) + // } } impl Context for AppContext { diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index 9e851f1008..18f7f47861 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -176,15 +176,15 @@ fn main() { // }) // .detach(); - client.telemetry().start(installation_id, session_id, cx); - let telemetry_settings = *client::TelemetrySettings::get_global(cx); - let event_operation = match existing_installation_id_found { - Some(false) => "first open", - _ => "open", - }; - client - .telemetry() - .report_app_event(telemetry_settings, event_operation); + client.telemetry().update(cx, |this, cx| { + this.start(installation_id, session_id, cx); + let telemetry_settings = *client::TelemetrySettings::get_global(cx); + let event_operation = match existing_installation_id_found { + Some(false) => "first open", + _ => "open", + }; + this.report_app_event(telemetry_settings, event_operation, cx); + }); let app_state = Arc::new(AppState { languages, diff --git a/crates/zed2/src/zed2.rs b/crates/zed2/src/zed2.rs index 6427fdabe8..50bd80b818 100644 --- a/crates/zed2/src/zed2.rs +++ b/crates/zed2/src/zed2.rs @@ -10,8 +10,8 @@ pub use assets::*; use collections::VecDeque; use editor::{Editor, MultiBuffer}; use gpui::{ - actions, point, px, AppContext, Context, FocusableView, PromptLevel, TitlebarOptions, - ViewContext, VisualContext, WindowBounds, WindowKind, WindowOptions, + actions, point, px, AppContext, AsyncAppContext, Context, FocusableView, PromptLevel, + TitlebarOptions, ViewContext, VisualContext, WindowBounds, WindowKind, WindowOptions, }; pub use only_instance::*; pub use open_listener::*; @@ -628,12 +628,12 @@ fn open_telemetry_log_file(workspace: &mut Workspace, cx: &mut ViewContext) -> Option { - let path = app_state.client.telemetry().log_file_path()?; + async fn fetch_log_string(app_state: &Arc, cx: &AsyncAppContext) -> Option { + let path = cx.update(|cx| app_state.client.telemetry().read(cx).log_file_path()).ok()??; app_state.fs.load(&path).await.log_err() } - let log = fetch_log_string(&app_state).await.unwrap_or_else(|| "// No data has been collected yet".to_string()); + let log = fetch_log_string(&app_state, &cx).await.unwrap_or_else(|| "// No data has been collected yet".to_string()); const MAX_TELEMETRY_LOG_LEN: usize = 5 * 1024 * 1024; let mut start_offset = log.len().saturating_sub(MAX_TELEMETRY_LOG_LEN); From ee2b6834bdcd88b5fd5aca2ca834870062fb74ad Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Nov 2023 16:16:44 -0500 Subject: [PATCH 24/31] Revert "Convert telemetry into a model" This reverts commit 6e4268a471749449fff232fab5d65fe38bab57b2. --- crates/auto_update2/src/auto_update.rs | 6 +- crates/call2/src/call2.rs | 17 +- crates/client2/src/client2.rs | 12 +- crates/client2/src/telemetry.rs | 261 +++++++++++++------------ crates/client2/src/user.rs | 12 +- crates/editor2/src/editor.rs | 40 ++-- crates/gpui2/src/app.rs | 31 +-- crates/zed2/src/main.rs | 18 +- crates/zed2/src/zed2.rs | 10 +- 9 files changed, 199 insertions(+), 208 deletions(-) diff --git a/crates/auto_update2/src/auto_update.rs b/crates/auto_update2/src/auto_update.rs index 88f225e412..aeff68965f 100644 --- a/crates/auto_update2/src/auto_update.rs +++ b/crates/auto_update2/src/auto_update.rs @@ -302,11 +302,7 @@ impl AutoUpdater { let mut dmg_file = File::create(&dmg_path).await?; let (installation_id, release_channel, telemetry) = cx.update(|cx| { - let installation_id = cx - .global::>() - .telemetry() - .read(cx) - .installation_id(); + let installation_id = cx.global::>().telemetry().installation_id(); let release_channel = cx .has_global::() .then(|| cx.global::().display_name()); diff --git a/crates/call2/src/call2.rs b/crates/call2/src/call2.rs index 6a956a73d2..14cb28c32d 100644 --- a/crates/call2/src/call2.rs +++ b/crates/call2/src/call2.rs @@ -482,26 +482,27 @@ pub fn report_call_event_for_room( let telemetry = client.telemetry(); let telemetry_settings = *TelemetrySettings::get_global(cx); - telemetry.update(cx, |this, cx| { - this.report_call_event(telemetry_settings, operation, Some(room_id), channel_id, cx) - }); + telemetry.report_call_event(telemetry_settings, operation, Some(room_id), channel_id) } pub fn report_call_event_for_channel( operation: &'static str, channel_id: u64, client: &Arc, - cx: &mut AppContext, + cx: &AppContext, ) { let room = ActiveCall::global(cx).read(cx).room(); - let room_id = room.map(|r| r.read(cx).id()); let telemetry = client.telemetry(); + let telemetry_settings = *TelemetrySettings::get_global(cx); - telemetry.update(cx, |this, cx| { - this.report_call_event(telemetry_settings, operation, room_id, Some(channel_id), cx) - }); + telemetry.report_call_event( + telemetry_settings, + operation, + room.map(|r| r.read(cx).id()), + Some(channel_id), + ) } #[cfg(test)] diff --git a/crates/client2/src/client2.rs b/crates/client2/src/client2.rs index f7d0b787c0..4ad354f2f9 100644 --- a/crates/client2/src/client2.rs +++ b/crates/client2/src/client2.rs @@ -121,7 +121,7 @@ pub struct Client { id: AtomicU64, peer: Arc, http: Arc, - telemetry: Model, + telemetry: Arc, state: RwLock, #[allow(clippy::type_complexity)] @@ -501,12 +501,8 @@ impl Client { })); } Status::SignedOut | Status::UpgradeRequired => { - cx.update(|cx| { - self.telemetry.update(cx, |this, cx| { - this.set_authenticated_user_info(None, false, cx) - }) - }) - .log_err(); + cx.update(|cx| self.telemetry.set_authenticated_user_info(None, false, cx)) + .log_err(); state._reconnect_task.take(); } _ => {} @@ -1324,7 +1320,7 @@ impl Client { } } - pub fn telemetry(&self) -> &Model { + pub fn telemetry(&self) -> &Arc { &self.telemetry } } diff --git a/crates/client2/src/telemetry.rs b/crates/client2/src/telemetry.rs index ca7ddcca97..ddad1d5fda 100644 --- a/crates/client2/src/telemetry.rs +++ b/crates/client2/src/telemetry.rs @@ -1,12 +1,12 @@ use crate::{TelemetrySettings, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; use chrono::{DateTime, Utc}; use futures::Future; -use gpui::{serde_json, AppContext, AppMetadata, Context, Model, ModelContext, Task}; +use gpui::{serde_json, AppContext, AppMetadata, BackgroundExecutor, Task}; use lazy_static::lazy_static; +use parking_lot::Mutex; use serde::Serialize; use settings::Settings; -use std::io::Write; -use std::{env, mem, path::PathBuf, sync::Arc, time::Duration}; +use std::{env, io::Write, mem, path::PathBuf, sync::Arc, time::Duration}; use sysinfo::{ CpuRefreshKind, Pid, PidExt, ProcessExt, ProcessRefreshKind, RefreshKind, System, SystemExt, }; @@ -16,6 +16,11 @@ use util::{channel::ReleaseChannel, TryFutureExt}; pub struct Telemetry { http_client: Arc, + executor: BackgroundExecutor, + state: Mutex, +} + +struct TelemetryState { metrics_id: Option>, // Per logged-in user installation_id: Option>, // Per app installation (different for dev, nightly, preview, and stable) session_id: Option>, // Per app launch @@ -122,7 +127,7 @@ const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(1); const DEBOUNCE_INTERVAL: Duration = Duration::from_secs(30); impl Telemetry { - pub fn new(client: Arc, cx: &mut AppContext) -> Model { + pub fn new(client: Arc, cx: &mut AppContext) -> Arc { let release_channel = if cx.has_global::() { Some(cx.global::().display_name()) } else { @@ -130,48 +135,57 @@ impl Telemetry { }; // TODO: Replace all hardware stuff with nested SystemSpecs json - let this = cx.build_model(|cx| Self { + let this = Arc::new(Self { http_client: client, - app_metadata: cx.app_metadata(), - architecture: env::consts::ARCH, - release_channel, - installation_id: None, - metrics_id: None, - session_id: None, - clickhouse_events_queue: Default::default(), - flush_clickhouse_events_task: Default::default(), - log_file: None, - is_staff: None, - first_event_datetime: None, + executor: cx.background_executor().clone(), + state: Mutex::new(TelemetryState { + app_metadata: cx.app_metadata(), + architecture: env::consts::ARCH, + release_channel, + installation_id: None, + metrics_id: None, + session_id: None, + clickhouse_events_queue: Default::default(), + flush_clickhouse_events_task: Default::default(), + log_file: None, + is_staff: None, + first_event_datetime: None, + }), }); // We should only ever have one instance of Telemetry, leak the subscription to keep it alive // rather than store in TelemetryState, complicating spawn as subscriptions are not Send - std::mem::forget(this.update(cx, |_, cx| cx.on_app_quit(Self::shutdown_telemetry))); + std::mem::forget(cx.on_app_quit({ + let this = this.clone(); + move |cx| this.shutdown_telemetry(cx) + })); this } - fn shutdown_telemetry(&mut self, cx: &mut ModelContext) -> impl Future { + fn shutdown_telemetry(self: &Arc, cx: &mut AppContext) -> impl Future { let telemetry_settings = TelemetrySettings::get_global(cx).clone(); - self.report_app_event(telemetry_settings, "close", cx); + self.report_app_event(telemetry_settings, "close"); Task::ready(()) } pub fn log_file_path(&self) -> Option { - Some(self.log_file.as_ref()?.path().to_path_buf()) + Some(self.state.lock().log_file.as_ref()?.path().to_path_buf()) } pub fn start( - &mut self, + self: &Arc, installation_id: Option, session_id: String, - cx: &mut ModelContext, + cx: &mut AppContext, ) { - self.installation_id = installation_id.map(|id| id.into()); - self.session_id = Some(session_id.into()); + let mut state = self.state.lock(); + state.installation_id = installation_id.map(|id| id.into()); + state.session_id = Some(session_id.into()); + drop(state); - cx.spawn(|this, mut cx| async move { + let this = self.clone(); + cx.spawn(|cx| async move { // Avoiding calling `System::new_all()`, as there have been crashes related to it let refresh_kind = RefreshKind::new() .with_memory() // For memory usage @@ -207,28 +221,23 @@ impl Telemetry { break; }; - this.update(&mut cx, |this, cx| { - this.report_memory_event( - telemetry_settings, - process.memory(), - process.virtual_memory(), - cx, - ); - this.report_cpu_event( - telemetry_settings, - process.cpu_usage(), - system.cpus().len() as u32, - cx, - ); - }) - .ok(); + this.report_memory_event( + telemetry_settings, + process.memory(), + process.virtual_memory(), + ); + this.report_cpu_event( + telemetry_settings, + process.cpu_usage(), + system.cpus().len() as u32, + ); } }) .detach(); } pub fn set_authenticated_user_info( - &mut self, + self: &Arc, metrics_id: Option, is_staff: bool, cx: &AppContext, @@ -237,20 +246,21 @@ impl Telemetry { return; } + let mut state = self.state.lock(); let metrics_id: Option> = metrics_id.map(|id| id.into()); - self.metrics_id = metrics_id.clone(); - self.is_staff = Some(is_staff); + state.metrics_id = metrics_id.clone(); + state.is_staff = Some(is_staff); + drop(state); } pub fn report_editor_event( - &mut self, + self: &Arc, telemetry_settings: TelemetrySettings, file_extension: Option, vim_mode: bool, operation: &'static str, copilot_enabled: bool, copilot_enabled_for_language: bool, - cx: &ModelContext, ) { let event = ClickhouseEvent::Editor { file_extension, @@ -261,16 +271,15 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false, cx) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_copilot_event( - &mut self, + self: &Arc, telemetry_settings: TelemetrySettings, suggestion_id: Option, suggestion_accepted: bool, file_extension: Option, - cx: &ModelContext, ) { let event = ClickhouseEvent::Copilot { suggestion_id, @@ -279,16 +288,15 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false, cx) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_assistant_event( - &mut self, + self: &Arc, telemetry_settings: TelemetrySettings, conversation_id: Option, kind: AssistantKind, model: &'static str, - cx: &ModelContext, ) { let event = ClickhouseEvent::Assistant { conversation_id, @@ -297,16 +305,15 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false, cx) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_call_event( - &mut self, + self: &Arc, telemetry_settings: TelemetrySettings, operation: &'static str, room_id: Option, channel_id: Option, - cx: &ModelContext, ) { let event = ClickhouseEvent::Call { operation, @@ -315,15 +322,14 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false, cx) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_cpu_event( - &mut self, + self: &Arc, telemetry_settings: TelemetrySettings, usage_as_percentage: f32, core_count: u32, - cx: &ModelContext, ) { let event = ClickhouseEvent::Cpu { usage_as_percentage, @@ -331,15 +337,14 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false, cx) + self.report_clickhouse_event(event, telemetry_settings, false) } pub fn report_memory_event( - &mut self, + self: &Arc, telemetry_settings: TelemetrySettings, memory_in_bytes: u64, virtual_memory_in_bytes: u64, - cx: &ModelContext, ) { let event = ClickhouseEvent::Memory { memory_in_bytes, @@ -347,90 +352,94 @@ impl Telemetry { milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, false, cx) + self.report_clickhouse_event(event, telemetry_settings, false) } // app_events are called at app open and app close, so flush is set to immediately send pub fn report_app_event( - &mut self, + self: &Arc, telemetry_settings: TelemetrySettings, operation: &'static str, - cx: &ModelContext, ) { let event = ClickhouseEvent::App { operation, milliseconds_since_first_event: self.milliseconds_since_first_event(), }; - self.report_clickhouse_event(event, telemetry_settings, true, cx) + self.report_clickhouse_event(event, telemetry_settings, true) } - fn milliseconds_since_first_event(&mut self) -> i64 { - match self.first_event_datetime { + fn milliseconds_since_first_event(&self) -> i64 { + let mut state = self.state.lock(); + match state.first_event_datetime { Some(first_event_datetime) => { let now: DateTime = Utc::now(); now.timestamp_millis() - first_event_datetime.timestamp_millis() } None => { - self.first_event_datetime = Some(Utc::now()); + state.first_event_datetime = Some(Utc::now()); 0 } } } fn report_clickhouse_event( - &mut self, + self: &Arc, event: ClickhouseEvent, telemetry_settings: TelemetrySettings, immediate_flush: bool, - cx: &ModelContext, ) { if !telemetry_settings.metrics { return; } - let signed_in = self.metrics_id.is_some(); - self.clickhouse_events_queue + let mut state = self.state.lock(); + let signed_in = state.metrics_id.is_some(); + state + .clickhouse_events_queue .push(ClickhouseEventWrapper { signed_in, event }); - if self.installation_id.is_some() { - if immediate_flush || self.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { - self.flush_clickhouse_events(cx); + if state.installation_id.is_some() { + if immediate_flush || state.clickhouse_events_queue.len() >= MAX_QUEUE_LEN { + drop(state); + self.flush_clickhouse_events(); } else { - self.flush_clickhouse_events_task = Some(cx.spawn(|this, mut cx| async move { - smol::Timer::after(DEBOUNCE_INTERVAL).await; - this.update(&mut cx, |this, cx| this.flush_clickhouse_events(cx)) - .ok(); + let this = self.clone(); + let executor = self.executor.clone(); + state.flush_clickhouse_events_task = Some(self.executor.spawn(async move { + executor.timer(DEBOUNCE_INTERVAL).await; + this.flush_clickhouse_events(); })); } } } - pub fn metrics_id(&self) -> Option> { - self.metrics_id.clone() + pub fn metrics_id(self: &Arc) -> Option> { + self.state.lock().metrics_id.clone() } - pub fn installation_id(&self) -> Option> { - self.installation_id.clone() + pub fn installation_id(self: &Arc) -> Option> { + self.state.lock().installation_id.clone() } - pub fn is_staff(&self) -> Option { - self.is_staff + pub fn is_staff(self: &Arc) -> Option { + self.state.lock().is_staff } - fn flush_clickhouse_events(&mut self, cx: &ModelContext) { - self.first_event_datetime = None; - let mut events = mem::take(&mut self.clickhouse_events_queue); - self.flush_clickhouse_events_task.take(); + fn flush_clickhouse_events(self: &Arc) { + let mut state = self.state.lock(); + state.first_event_datetime = None; + let mut events = mem::take(&mut state.clickhouse_events_queue); + state.flush_clickhouse_events_task.take(); + drop(state); - let http_client = self.http_client.clone(); + let this = self.clone(); + self.executor + .spawn( + async move { + let mut json_bytes = Vec::new(); - cx.spawn(|this, mut cx| { - async move { - let mut json_bytes = Vec::new(); - - this.update(&mut cx, |this, _| { - if let Some(file) = &mut this.log_file { + if let Some(file) = &mut this.state.lock().log_file { let file = file.as_file_mut(); for event in &mut events { json_bytes.clear(); @@ -440,43 +449,39 @@ impl Telemetry { } } - std::io::Result::Ok(()) - })??; + { + let state = this.state.lock(); + let request_body = ClickhouseEventRequestBody { + token: ZED_SECRET_CLIENT_TOKEN, + installation_id: state.installation_id.clone(), + session_id: state.session_id.clone(), + is_staff: state.is_staff.clone(), + app_version: state + .app_metadata + .app_version + .map(|version| version.to_string()), + os_name: state.app_metadata.os_name, + os_version: state + .app_metadata + .os_version + .map(|version| version.to_string()), + architecture: state.architecture, - if let Ok(Ok(json_bytes)) = this.update(&mut cx, |this, _| { - let request_body = ClickhouseEventRequestBody { - token: ZED_SECRET_CLIENT_TOKEN, - installation_id: this.installation_id.clone(), - session_id: this.session_id.clone(), - is_staff: this.is_staff.clone(), - app_version: this - .app_metadata - .app_version - .map(|version| version.to_string()), - os_name: this.app_metadata.os_name, - os_version: this - .app_metadata - .os_version - .map(|version| version.to_string()), - architecture: this.architecture, + release_channel: state.release_channel, + events, + }; + dbg!(&request_body); + json_bytes.clear(); + serde_json::to_writer(&mut json_bytes, &request_body)?; + } - release_channel: this.release_channel, - events, - }; - json_bytes.clear(); - serde_json::to_writer(&mut json_bytes, &request_body)?; - - std::io::Result::Ok(json_bytes) - }) { - http_client + this.http_client .post_json(CLICKHOUSE_EVENTS_URL.as_str(), json_bytes.into()) .await?; + anyhow::Ok(()) } - - anyhow::Ok(()) - } - .log_err() - }) - .detach(); + .log_err(), + ) + .detach(); } } diff --git a/crates/client2/src/user.rs b/crates/client2/src/user.rs index 5d115a3785..a5dba03d2d 100644 --- a/crates/client2/src/user.rs +++ b/crates/client2/src/user.rs @@ -168,13 +168,11 @@ impl UserStore { cx.update(|cx| { if let Some(info) = info { cx.update_flags(info.staff, info.flags); - client.telemetry.update(cx, |this, cx| { - this.set_authenticated_user_info( - Some(info.metrics_id.clone()), - info.staff, - cx, - ) - }) + client.telemetry.set_authenticated_user_info( + Some(info.metrics_id.clone()), + info.staff, + cx, + ) } })?; diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index 0b367b7656..fa5f4dfa42 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -8954,7 +8954,7 @@ impl Editor { &self, suggestion_id: Option, suggestion_accepted: bool, - cx: &mut AppContext, + cx: &AppContext, ) { let Some(project) = &self.project else { return }; @@ -8971,15 +8971,12 @@ impl Editor { let telemetry = project.read(cx).client().telemetry().clone(); let telemetry_settings = *TelemetrySettings::get_global(cx); - telemetry.update(cx, |this, cx| { - this.report_copilot_event( - telemetry_settings, - suggestion_id, - suggestion_accepted, - file_extension, - cx, - ) - }); + telemetry.report_copilot_event( + telemetry_settings, + suggestion_id, + suggestion_accepted, + file_extension, + ) } #[cfg(any(test, feature = "test-support"))] @@ -8987,7 +8984,7 @@ impl Editor { &self, _operation: &'static str, _file_extension: Option, - _cx: &mut AppContext, + _cx: &AppContext, ) { } @@ -8996,7 +8993,7 @@ impl Editor { &self, operation: &'static str, file_extension: Option, - cx: &mut AppContext, + cx: &AppContext, ) { let Some(project) = &self.project else { return }; @@ -9026,17 +9023,14 @@ impl Editor { .show_copilot_suggestions; let telemetry = project.read(cx).client().telemetry().clone(); - telemetry.update(cx, |this, cx| { - this.report_editor_event( - telemetry_settings, - file_extension, - vim_mode, - operation, - copilot_enabled, - copilot_enabled_for_language, - cx, - ) - }); + telemetry.report_editor_event( + telemetry_settings, + file_extension, + vim_mode, + operation, + copilot_enabled, + copilot_enabled_for_language, + ) } /// Copy the highlighted chunks to the clipboard as JSON. The format is an array of lines, diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index 1d599eaede..617c0b5600 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -10,6 +10,7 @@ pub use entity_map::*; pub use model_context::*; use refineable::Refineable; use smallvec::SmallVec; +use smol::future::FutureExt; #[cfg(any(test, feature = "test-support"))] pub use test_context::*; @@ -984,21 +985,21 @@ impl AppContext { self.actions.all_action_names() } - // pub fn on_app_quit( - // &mut self, - // mut on_quit: impl FnMut(&mut AppContext) -> Fut + 'static, - // ) -> Subscription - // where - // Fut: 'static + Future, - // { - // self.quit_observers.insert( - // (), - // Box::new(move |cx| { - // let future = on_quit(cx); - // async move { future.await }.boxed_local() - // }), - // ) - // } + pub fn on_app_quit( + &mut self, + mut on_quit: impl FnMut(&mut AppContext) -> Fut + 'static, + ) -> Subscription + where + Fut: 'static + Future, + { + self.quit_observers.insert( + (), + Box::new(move |cx| { + let future = on_quit(cx); + async move { future.await }.boxed_local() + }), + ) + } } impl Context for AppContext { diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index d380d1f47c..46be582129 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -176,15 +176,15 @@ fn main() { // }) // .detach(); - client.telemetry().update(cx, |this, cx| { - this.start(installation_id, session_id, cx); - let telemetry_settings = *client::TelemetrySettings::get_global(cx); - let event_operation = match existing_installation_id_found { - Some(false) => "first open", - _ => "open", - }; - this.report_app_event(telemetry_settings, event_operation, cx); - }); + client.telemetry().start(installation_id, session_id, cx); + let telemetry_settings = *client::TelemetrySettings::get_global(cx); + let event_operation = match existing_installation_id_found { + Some(false) => "first open", + _ => "open", + }; + client + .telemetry() + .report_app_event(telemetry_settings, event_operation); let app_state = Arc::new(AppState { languages, diff --git a/crates/zed2/src/zed2.rs b/crates/zed2/src/zed2.rs index 09880b858f..1286594138 100644 --- a/crates/zed2/src/zed2.rs +++ b/crates/zed2/src/zed2.rs @@ -10,8 +10,8 @@ pub use assets::*; use collections::VecDeque; use editor::{Editor, MultiBuffer}; use gpui::{ - actions, point, px, AppContext, AsyncAppContext, Context, FocusableView, PromptLevel, - TitlebarOptions, ViewContext, VisualContext, WindowBounds, WindowKind, WindowOptions, + actions, point, px, AppContext, Context, FocusableView, PromptLevel, TitlebarOptions, + ViewContext, VisualContext, WindowBounds, WindowKind, WindowOptions, }; pub use only_instance::*; pub use open_listener::*; @@ -628,12 +628,12 @@ fn open_telemetry_log_file(workspace: &mut Workspace, cx: &mut ViewContext, cx: &AsyncAppContext) -> Option { - let path = cx.update(|cx| app_state.client.telemetry().read(cx).log_file_path()).ok()??; + async fn fetch_log_string(app_state: &Arc) -> Option { + let path = app_state.client.telemetry().log_file_path()?; app_state.fs.load(&path).await.log_err() } - let log = fetch_log_string(&app_state, &cx).await.unwrap_or_else(|| "// No data has been collected yet".to_string()); + let log = fetch_log_string(&app_state).await.unwrap_or_else(|| "// No data has been collected yet".to_string()); const MAX_TELEMETRY_LOG_LEN: usize = 5 * 1024 * 1024; let mut start_offset = log.len().saturating_sub(MAX_TELEMETRY_LOG_LEN); From c04f123e44ec49d998ab66b4a5c7216c617f1eec Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Wed, 22 Nov 2023 22:25:26 +0100 Subject: [PATCH 25/31] ci: Add ci-config.toml in .cargo folder. --- .cargo/ci-config.toml | 12 ++++++++++++ .github/workflows/ci.yml | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 .cargo/ci-config.toml diff --git a/.cargo/ci-config.toml b/.cargo/ci-config.toml new file mode 100644 index 0000000000..6dbaf4b446 --- /dev/null +++ b/.cargo/ci-config.toml @@ -0,0 +1,12 @@ +# This config is different from config.toml in this directory, as the latter is recognized by Cargo. +# This file is placed in $HOME/.cargo/config.toml on CI runs. Cargo then merges Zeds .cargo/config.toml with $HOME/.cargo/config.toml +# with preference for settings from Zeds config.toml. +# TL;DR: If a value is set in both ci-config.toml and config.toml, config.toml value takes precedence. +# Arrays are merged together though. See: https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure +# The intent for this file is to configure CI build process with a divergance from Zed developers experience; for example, in this config file +# we use `-D warnings` for rustflags (which makes compilation fail in presence of warnings during build process). Placing that in developers `config.toml` +# would be incovenient. +# We *could* override things like RUSTFLAGS manually by setting them as environment variables, but that is less DRY; worse yet, if you forget to set proper environment variables +# in one spot, that's going to trigger a rebuild of all of the artifacts. Using ci-config.toml we can define these overrides for CI in one spot and not worry about it. +[build] +rustflags = ["-D", "warnings"] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 208d538976..ef718a4fa9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,15 +23,15 @@ jobs: - self-hosted - test steps: - - name: Set up default .cargo/config.toml - run: printf "[build]\nrustflags = [\"-D\", \"warnings\"]" > $HOME/.cargo/config.toml - - name: Checkout repo uses: actions/checkout@v3 with: clean: false submodules: "recursive" + - name: Set up default .cargo/config.toml + run: cp ./.cargo/ci-config.toml ~/.cargo/config.toml + - name: Run rustfmt uses: ./.github/actions/check_formatting From 0def2bc0d2563dae9b6c4904ae06ec27e74230a1 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Nov 2023 16:26:15 -0500 Subject: [PATCH 26/31] Remove dbg --- crates/client2/src/telemetry.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/client2/src/telemetry.rs b/crates/client2/src/telemetry.rs index ddad1d5fda..c7be5f687d 100644 --- a/crates/client2/src/telemetry.rs +++ b/crates/client2/src/telemetry.rs @@ -470,7 +470,6 @@ impl Telemetry { release_channel: state.release_channel, events, }; - dbg!(&request_body); json_bytes.clear(); serde_json::to_writer(&mut json_bytes, &request_body)?; } From 37e3cc12917109f0324724449c0089ba4eed8d10 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 22 Nov 2023 16:26:27 -0500 Subject: [PATCH 27/31] zed2(ish) Cancel completion resolution when new list --- crates/editor2/src/editor.rs | 47 ++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index fa5f4dfa42..5c25279270 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -966,20 +966,22 @@ impl CompletionsMenu { fn pre_resolve_completion_documentation( &self, - project: Option>, - cx: &mut ViewContext, - ) { + _editor: &Editor, + _cx: &mut ViewContext, + ) -> Option> { // todo!("implementation below "); + None } - // ) { + // { // let settings = EditorSettings::get_global(cx); // if !settings.show_completion_documentation { - // return; + // return None; // } - // let Some(project) = project else { - // return; + // let Some(project) = editor.project.clone() else { + // return None; // }; + // let client = project.read(cx).client(); // let language_registry = project.read(cx).languages().clone(); @@ -989,7 +991,7 @@ impl CompletionsMenu { // let completions = self.completions.clone(); // let completion_indices: Vec<_> = self.matches.iter().map(|m| m.candidate_id).collect(); - // cx.spawn(move |this, mut cx| async move { + // Some(cx.spawn(move |this, mut cx| async move { // if is_remote { // let Some(project_id) = project_id else { // log::error!("Remote project without remote_id"); @@ -1051,8 +1053,7 @@ impl CompletionsMenu { // _ = this.update(&mut cx, |_, cx| cx.notify()); // } // } - // }) - // .detach(); + // })) // } fn attempt_resolve_selected_completion_documentation( @@ -3596,7 +3597,8 @@ impl Editor { let id = post_inc(&mut self.next_completion_id); let task = cx.spawn(|this, mut cx| { async move { - let menu = if let Some(completions) = completions.await.log_err() { + let completions = completions.await.log_err(); + let (menu, pre_resolve_task) = if let Some(completions) = completions { let mut menu = CompletionsMenu { id, initial_position: position, @@ -3619,20 +3621,24 @@ impl Editor { }; menu.filter(query.as_deref(), cx.background_executor().clone()) .await; + if menu.matches.is_empty() { - None + (None, None) } else { - _ = this.update(&mut cx, |editor, cx| { - menu.pre_resolve_completion_documentation(editor.project.clone(), cx); - }); - Some(menu) + let pre_resolve_task = this + .update(&mut cx, |editor, cx| { + menu.pre_resolve_completion_documentation(editor, cx) + }) + .ok() + .flatten(); + (Some(menu), pre_resolve_task) } } else { - None + (None, None) }; this.update(&mut cx, |this, cx| { - this.completion_tasks.retain(|(task_id, _)| *task_id > id); + this.completion_tasks.retain(|(task_id, _)| *task_id >= id); let mut context_menu = this.context_menu.write(); match context_menu.as_ref() { @@ -3664,10 +3670,15 @@ impl Editor { } })?; + if let Some(pre_resolve_task) = pre_resolve_task { + pre_resolve_task.await; + } + Ok::<_, anyhow::Error>(()) } .log_err() }); + self.completion_tasks.push((id, task)); } From eb74ad7caaab389a5eefbaac221587a0b157c43e Mon Sep 17 00:00:00 2001 From: Mikayla Date: Wed, 22 Nov 2023 13:41:48 -0800 Subject: [PATCH 28/31] Fix failing test --- crates/diagnostics2/src/diagnostics.rs | 2 +- crates/gpui2/src/element.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/diagnostics2/src/diagnostics.rs b/crates/diagnostics2/src/diagnostics.rs index 7ff8cd84db..01e8762127 100644 --- a/crates/diagnostics2/src/diagnostics.rs +++ b/crates/diagnostics2/src/diagnostics.rs @@ -1550,7 +1550,7 @@ mod tests { block_id: ix, editor_style: &editor::EditorStyle::default(), }) - .element_id()? + .inner_id()? .try_into() .ok()?, diff --git a/crates/gpui2/src/element.rs b/crates/gpui2/src/element.rs index 1045e6218c..912329c3cb 100644 --- a/crates/gpui2/src/element.rs +++ b/crates/gpui2/src/element.rs @@ -463,6 +463,10 @@ impl AnyElement { pub fn into_any(self) -> AnyElement { AnyElement::new(self) } + + pub fn inner_id(&self) -> Option { + self.0.element_id() + } } impl Element for AnyElement { From a876b6f700a73051848e0ffdff601b78a339b82c Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Nov 2023 23:01:18 -0500 Subject: [PATCH 29/31] Remove comments --- crates/zed/src/main.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 6fb6b2476f..20b93ae6bb 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -766,11 +766,3 @@ pub fn background_actions() -> &'static [(&'static str, &'static dyn Action)] { ("Change your settings", &zed_actions::OpenSettings), ] } - -// TODO: -// Cleanly identify open / first open -// What should we do if we fail when looking for installation_id? -// - set to true, false, or skip? -// Report closed -// Copy logic to zed2 -// If we don't add an app close, we should prob add back the flush on startup? From 575ab81409a2378b9148b6f753453e78bee9ff51 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Nov 2023 23:01:31 -0500 Subject: [PATCH 30/31] Disable app close event --- crates/client2/src/telemetry.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/client2/src/telemetry.rs b/crates/client2/src/telemetry.rs index c7be5f687d..31d0a9f5d9 100644 --- a/crates/client2/src/telemetry.rs +++ b/crates/client2/src/telemetry.rs @@ -155,19 +155,19 @@ impl Telemetry { // We should only ever have one instance of Telemetry, leak the subscription to keep it alive // rather than store in TelemetryState, complicating spawn as subscriptions are not Send - std::mem::forget(cx.on_app_quit({ - let this = this.clone(); - move |cx| this.shutdown_telemetry(cx) - })); + // std::mem::forget(cx.on_app_quit({ + // let this = this.clone(); + // move |cx| this.shutdown_telemetry(cx) + // })); this } - fn shutdown_telemetry(self: &Arc, cx: &mut AppContext) -> impl Future { - let telemetry_settings = TelemetrySettings::get_global(cx).clone(); - self.report_app_event(telemetry_settings, "close"); - Task::ready(()) - } + // fn shutdown_telemetry(self: &Arc, cx: &mut AppContext) -> impl Future { + // let telemetry_settings = TelemetrySettings::get_global(cx).clone(); + // self.report_app_event(telemetry_settings, "close"); + // Task::ready(()) + // } pub fn log_file_path(&self) -> Option { Some(self.state.lock().log_file.as_ref()?.path().to_path_buf()) From 35f35dd47646edfef37f432dd971be34f6d02288 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Nov 2023 23:03:11 -0500 Subject: [PATCH 31/31] Remove unused import --- crates/client2/src/telemetry.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/client2/src/telemetry.rs b/crates/client2/src/telemetry.rs index 31d0a9f5d9..37651ebcfb 100644 --- a/crates/client2/src/telemetry.rs +++ b/crates/client2/src/telemetry.rs @@ -1,6 +1,5 @@ use crate::{TelemetrySettings, ZED_SECRET_CLIENT_TOKEN, ZED_SERVER_URL}; use chrono::{DateTime, Utc}; -use futures::Future; use gpui::{serde_json, AppContext, AppMetadata, BackgroundExecutor, Task}; use lazy_static::lazy_static; use parking_lot::Mutex;