From 1be2836f60176fbdde27d0b26eeb2b0c19f7a17c Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Wed, 5 Feb 2025 19:19:23 +0100 Subject: [PATCH] Refactor session id to use entity instead (#109) * Move common session fields to struct * Use session entity instead of session id inside UI This is to prepare to move the state to the --- crates/collab/src/tests/debug_panel_tests.rs | 5 +- crates/dap/src/session.rs | 66 ++++++------- crates/debugger_ui/src/console.rs | 18 ++-- crates/debugger_ui/src/debugger_panel.rs | 41 ++++---- crates/debugger_ui/src/debugger_panel_item.rs | 97 +++++++++---------- crates/debugger_ui/src/module_list.rs | 10 +- crates/debugger_ui/src/stack_frame_list.rs | 18 ++-- crates/debugger_ui/src/variable_list.rs | 26 ++--- crates/project/src/dap_store.rs | 7 +- 9 files changed, 140 insertions(+), 148 deletions(-) diff --git a/crates/collab/src/tests/debug_panel_tests.rs b/crates/collab/src/tests/debug_panel_tests.rs index c4aea60da8..8ab704be4c 100644 --- a/crates/collab/src/tests/debug_panel_tests.rs +++ b/crates/collab/src/tests/debug_panel_tests.rs @@ -1960,7 +1960,10 @@ async fn test_ignore_breakpoints( let breakpoints_ignored = active_debug_panel_item.read(cx).are_breakpoints_ignored(cx); - assert_eq!(session_id, active_debug_panel_item.read(cx).session_id()); + assert_eq!( + session_id, + active_debug_panel_item.read(cx).session().read(cx).id() + ); assert_eq!(false, breakpoints_ignored); assert_eq!(client.id(), active_debug_panel_item.read(cx).client_id()); assert_eq!(1, active_debug_panel_item.read(cx).thread_id()); diff --git a/crates/dap/src/session.rs b/crates/dap/src/session.rs index ce7f60a1fc..eda6129cb8 100644 --- a/crates/dap/src/session.rs +++ b/crates/dap/src/session.rs @@ -19,14 +19,18 @@ impl DebugSessionId { } } -pub enum DebugSession { +pub struct DebugSession { + id: DebugSessionId, + mode: DebugSessionMode, + ignore_breakpoints: bool, +} + +pub enum DebugSessionMode { Local(LocalDebugSession), Remote(RemoteDebugSession), } pub struct LocalDebugSession { - id: DebugSessionId, - ignore_breakpoints: bool, configuration: DebugAdapterConfig, clients: HashMap>, } @@ -80,77 +84,63 @@ impl LocalDebugSession { pub fn client_ids(&self) -> impl Iterator + '_ { self.clients.keys().cloned() } - - pub fn id(&self) -> DebugSessionId { - self.id - } } pub struct RemoteDebugSession { - id: DebugSessionId, - ignore_breakpoints: bool, label: String, } impl DebugSession { pub fn new_local(id: DebugSessionId, configuration: DebugAdapterConfig) -> Self { - Self::Local(LocalDebugSession { + Self { id, ignore_breakpoints: false, - configuration, - clients: HashMap::default(), - }) + mode: DebugSessionMode::Local(LocalDebugSession { + configuration, + clients: HashMap::default(), + }), + } } pub fn as_local(&self) -> Option<&LocalDebugSession> { - match self { - DebugSession::Local(local) => Some(local), + match &self.mode { + DebugSessionMode::Local(local) => Some(local), _ => None, } } pub fn as_local_mut(&mut self) -> Option<&mut LocalDebugSession> { - match self { - DebugSession::Local(local) => Some(local), + match &mut self.mode { + DebugSessionMode::Local(local) => Some(local), _ => None, } } pub fn new_remote(id: DebugSessionId, label: String, ignore_breakpoints: bool) -> Self { - Self::Remote(RemoteDebugSession { + Self { id, - label: label.clone(), ignore_breakpoints, - }) - } - - pub fn id(&self) -> DebugSessionId { - match self { - DebugSession::Local(local) => local.id, - DebugSession::Remote(remote) => remote.id, + mode: DebugSessionMode::Remote(RemoteDebugSession { label }), } } + pub fn id(&self) -> DebugSessionId { + self.id + } + pub fn name(&self) -> String { - match self { - DebugSession::Local(local) => local.configuration.label.clone(), - DebugSession::Remote(remote) => remote.label.clone(), + match &self.mode { + DebugSessionMode::Local(local) => local.configuration.label.clone(), + DebugSessionMode::Remote(remote) => remote.label.clone(), } } pub fn ignore_breakpoints(&self) -> bool { - match self { - DebugSession::Local(local) => local.ignore_breakpoints, - DebugSession::Remote(remote) => remote.ignore_breakpoints, - } + self.ignore_breakpoints } pub fn set_ignore_breakpoints(&mut self, ignore: bool, cx: &mut Context) { - match self { - DebugSession::Local(local) => local.ignore_breakpoints = ignore, - DebugSession::Remote(remote) => remote.ignore_breakpoints = ignore, - } - + self.ignore_breakpoints = ignore; cx.notify(); } } diff --git a/crates/debugger_ui/src/console.rs b/crates/debugger_ui/src/console.rs index 817bfecfa8..1eb05867c5 100644 --- a/crates/debugger_ui/src/console.rs +++ b/crates/debugger_ui/src/console.rs @@ -3,7 +3,7 @@ use crate::{ variable_list::VariableList, }; use dap::{ - client::DebugAdapterClientId, proto_conversions::ProtoConversion, session::DebugSessionId, + client::DebugAdapterClientId, proto_conversions::ProtoConversion, session::DebugSession, OutputEvent, OutputEventGroup, }; use editor::{ @@ -34,8 +34,8 @@ pub struct Console { groups: Vec, console: Entity, query_bar: Entity, - session_id: DebugSessionId, dap_store: Entity, + session: Entity, client_id: DebugAdapterClientId, _subscriptions: Vec, variable_list: Entity, @@ -44,11 +44,11 @@ pub struct Console { impl Console { pub fn new( - stack_frame_list: &Entity, - session_id: &DebugSessionId, + session: Entity, client_id: &DebugAdapterClientId, - variable_list: Entity, dap_store: Entity, + stack_frame_list: Entity, + variable_list: Entity, window: &mut Window, cx: &mut Context, ) -> Self { @@ -84,18 +84,18 @@ impl Console { }); let _subscriptions = - vec![cx.subscribe(stack_frame_list, Self::handle_stack_frame_list_events)]; + vec![cx.subscribe(&stack_frame_list, Self::handle_stack_frame_list_events)]; Self { + session, console, dap_store, query_bar, variable_list, _subscriptions, + stack_frame_list, client_id: *client_id, groups: Vec::default(), - session_id: *session_id, - stack_frame_list: stack_frame_list.clone(), } } @@ -132,7 +132,7 @@ impl Console { project_id: *project_id, client_id: self.client_id.to_proto(), thread_id: None, - session_id: self.session_id.to_proto(), + session_id: self.session.read(cx).id().to_proto(), variant: Some(proto::update_debug_adapter::Variant::OutputEvent( event.to_proto(), )), diff --git a/crates/debugger_ui/src/debugger_panel.rs b/crates/debugger_ui/src/debugger_panel.rs index 02a8318a7d..e937635d45 100644 --- a/crates/debugger_ui/src/debugger_panel.rs +++ b/crates/debugger_ui/src/debugger_panel.rs @@ -346,7 +346,7 @@ impl DebugPanel { let thread_panel = item.downcast::().unwrap(); let thread_id = thread_panel.read(cx).thread_id(); - let session_id = thread_panel.read(cx).session_id(); + let session_id = thread_panel.read(cx).session().read(cx).id(); let client_id = thread_panel.read(cx).client_id(); self.thread_states.remove(&(client_id, thread_id)); @@ -758,20 +758,17 @@ impl DebugPanel { return; }; - let Some(session_name) = self + let Some(session) = self .dap_store .read(cx) .session_by_id(session_id) - .map(|session| session.read(cx).name()) + .map(|session| session) else { - return; // this can never happen + return; // this can/should never happen }; - let session_id = *session_id; let client_id = *client_id; - let session_name = SharedString::from(session_name); - cx.spawn_in(window, { let event = event.clone(); |this, mut cx| async move { @@ -789,18 +786,17 @@ impl DebugPanel { let existing_item = this.debug_panel_item_by_client(&client_id, thread_id, cx); if existing_item.is_none() { - let debug_panel = cx.entity().clone(); + let debug_panel = cx.entity(); this.pane.update(cx, |pane, cx| { let tab = cx.new(|cx| { DebugPanelItem::new( - debug_panel, - this.workspace.clone(), - this.dap_store.clone(), - thread_state.clone(), - &session_id, + session, &client_id, - session_name, thread_id, + thread_state, + this.dap_store.clone(), + &debug_panel, + this.workspace.clone(), window, cx, ) @@ -1025,6 +1021,10 @@ impl DebugPanel { cx: &mut Context, ) { let session_id = DebugSessionId::from_proto(payload.session_id); + let Some(session) = self.dap_store.read(cx).session_by_id(&session_id) else { + return; + }; + let client_id = DebugAdapterClientId::from_proto(payload.client_id); let thread_id = payload.thread_id; let thread_state = payload.thread_state.clone().unwrap(); @@ -1045,18 +1045,17 @@ impl DebugPanel { self.thread_states .insert((client_id, thread_id), thread_state.clone()); - let debug_panel = cx.entity().clone(); + let debug_panel = cx.entity(); let debug_panel_item = self.pane.update(cx, |pane, cx| { let debug_panel_item = cx.new(|cx| { DebugPanelItem::new( - debug_panel, - self.workspace.clone(), - self.dap_store.clone(), - thread_state, - &session_id, + session, &client_id, - payload.session_name.clone().into(), thread_id, + thread_state, + self.dap_store.clone(), + &debug_panel, + self.workspace.clone(), window, cx, ) diff --git a/crates/debugger_ui/src/debugger_panel_item.rs b/crates/debugger_ui/src/debugger_panel_item.rs index 51f1fca4b4..e25fd62fe1 100644 --- a/crates/debugger_ui/src/debugger_panel_item.rs +++ b/crates/debugger_ui/src/debugger_panel_item.rs @@ -6,7 +6,7 @@ use crate::stack_frame_list::{StackFrameList, StackFrameListEvent}; use crate::variable_list::VariableList; use dap::proto_conversions::{self, ProtoConversion}; -use dap::session::DebugSessionId; +use dap::session::DebugSession; use dap::{ client::DebugAdapterClientId, debugger_settings::DebuggerSettings, Capabilities, ContinuedEvent, LoadedSourceEvent, ModuleEvent, OutputEvent, OutputEventCategory, StoppedEvent, @@ -65,9 +65,8 @@ pub struct DebugPanelItem { console: Entity, focus_handle: FocusHandle, remote_id: Option, - session_name: SharedString, dap_store: Entity, - session_id: DebugSessionId, + session: Entity, show_console_indicator: bool, module_list: Entity, active_thread_item: ThreadItem, @@ -83,14 +82,13 @@ pub struct DebugPanelItem { impl DebugPanelItem { #[allow(clippy::too_many_arguments)] pub fn new( - debug_panel: Entity, - workspace: WeakEntity, - dap_store: Entity, - thread_state: Entity, - session_id: &DebugSessionId, + session: Entity, client_id: &DebugAdapterClientId, - session_name: SharedString, thread_id: u64, + thread_state: Entity, + dap_store: Entity, + debug_panel: &Entity, + workspace: WeakEntity, window: &mut Window, cx: &mut Context, ) -> Self { @@ -100,34 +98,41 @@ impl DebugPanelItem { let stack_frame_list = cx.new(|cx| { StackFrameList::new( - &workspace, &this, &dap_store, client_id, session_id, thread_id, window, cx, + workspace.clone(), + &this, + dap_store.clone(), + session.clone(), + client_id, + thread_id, + window, + cx, ) }); let variable_list = cx.new(|cx| { VariableList::new( - &stack_frame_list, + session.clone(), + client_id, dap_store.clone(), - &client_id, - session_id, + stack_frame_list.clone(), window, cx, ) }); let module_list = - cx.new(|cx| ModuleList::new(dap_store.clone(), &client_id, &session_id, cx)); + cx.new(|cx| ModuleList::new(dap_store.clone(), session.clone(), &client_id, cx)); let loaded_source_list = cx.new(|cx| LoadedSourceList::new(&this, dap_store.clone(), &client_id, cx)); let console = cx.new(|cx| { Console::new( - &stack_frame_list, - session_id, + session.clone(), client_id, - variable_list.clone(), dap_store.clone(), + stack_frame_list.clone(), + variable_list.clone(), window, cx, ) @@ -136,7 +141,7 @@ impl DebugPanelItem { cx.observe(&module_list, |_, _, cx| cx.notify()).detach(); let _subscriptions = vec![ - cx.subscribe_in(&debug_panel, window, { + cx.subscribe_in(debug_panel, window, { move |this: &mut Self, _, event: &DebugPanelEvent, window, cx| { match event { DebugPanelEvent::Stopped { @@ -182,11 +187,11 @@ impl DebugPanelItem { ]; Self { + session, console, thread_id, dap_store, workspace, - session_name, module_list, thread_state, focus_handle, @@ -196,7 +201,6 @@ impl DebugPanelItem { stack_frame_list, loaded_source_list, client_id: *client_id, - session_id: *session_id, show_console_indicator: false, active_thread_item: ThreadItem::Variables, } @@ -209,7 +213,7 @@ impl DebugPanelItem { SetDebuggerPanelItem { project_id, - session_id: self.session_id.to_proto(), + session_id: self.session.read(cx).id().to_proto(), client_id: self.client_id.to_proto(), thread_id: self.thread_id, console: None, @@ -219,7 +223,7 @@ impl DebugPanelItem { variable_list, stack_frame_list, loaded_source_list: None, - session_name: self.session_name.to_string(), + session_name: self.session.read(cx).name(), } } @@ -434,7 +438,7 @@ impl DebugPanelItem { let message = proto_conversions::capabilities_to_proto( &self.dap_store.read(cx).capabilities_by_id(client_id), *project_id, - self.session_id.to_proto(), + self.session.read(cx).id().to_proto(), self.client_id.to_proto(), ); @@ -480,8 +484,8 @@ impl DebugPanelItem { } } - pub fn session_id(&self) -> DebugSessionId { - self.session_id + pub fn session(&self) -> &Entity { + &self.session } pub fn client_id(&self) -> DebugAdapterClientId { @@ -519,8 +523,7 @@ impl DebugPanelItem { #[cfg(any(test, feature = "test-support"))] pub fn are_breakpoints_ignored(&self, cx: &App) -> bool { - self.dap_store - .read_with(cx, |dap, cx| dap.ignore_breakpoints(&self.session_id, cx)) + self.session.read(cx).ignore_breakpoints() } pub fn capabilities(&self, cx: &mut Context) -> Capabilities { @@ -711,7 +714,7 @@ impl DebugPanelItem { self.dap_store.update(cx, |store, cx| { store .terminate_threads( - &self.session_id, + &self.session.read(cx).id(), &self.client_id, Some(vec![self.thread_id; 1]), cx, @@ -729,15 +732,9 @@ impl DebugPanelItem { } pub fn toggle_ignore_breakpoints(&mut self, cx: &mut Context) { - self.workspace - .update(cx, |workspace, cx| { - workspace.project().update(cx, |project, cx| { - project - .toggle_ignore_breakpoints(&self.session_id, &self.client_id, cx) - .detach_and_log_err(cx); - }) - }) - .ok(); + self.session.update(cx, |session, cx| { + session.set_ignore_breakpoints(!session.ignore_breakpoints(), cx); + }); } } @@ -756,21 +753,25 @@ impl Item for DebugPanelItem { &self, params: workspace::item::TabContentParams, _window: &Window, - _: &App, + cx: &App, ) -> AnyElement { - Label::new(format!("{} - Thread {}", self.session_name, self.thread_id)) - .color(if params.selected { - Color::Default - } else { - Color::Muted - }) - .into_any_element() + Label::new(format!( + "{} - Thread {}", + self.session.read(cx).name(), + self.thread_id + )) + .color(if params.selected { + Color::Default + } else { + Color::Muted + }) + .into_any_element() } fn tab_tooltip_text(&self, cx: &App) -> Option { Some(SharedString::from(format!( "{} Thread {} - {:?}", - self.session_name, + self.session.read(cx).name(), self.thread_id, self.thread_state.read(cx).status, ))) @@ -992,9 +993,7 @@ impl Render for DebugPanelItem { .child( IconButton::new( "debug-ignore-breakpoints", - if self.dap_store.update(cx, |store, cx| { - store.ignore_breakpoints(&self.session_id, cx) - }) { + if self.session.read(cx).ignore_breakpoints() { IconName::DebugIgnoreBreakpoints } else { IconName::DebugBreakpoint diff --git a/crates/debugger_ui/src/module_list.rs b/crates/debugger_ui/src/module_list.rs index eb165e5211..13d21189cf 100644 --- a/crates/debugger_ui/src/module_list.rs +++ b/crates/debugger_ui/src/module_list.rs @@ -1,6 +1,6 @@ use anyhow::Result; use dap::{ - client::DebugAdapterClientId, proto_conversions::ProtoConversion, session::DebugSessionId, + client::DebugAdapterClientId, proto_conversions::ProtoConversion, session::DebugSession, Module, ModuleEvent, }; use gpui::{list, AnyElement, Entity, FocusHandle, Focusable, ListState, Task}; @@ -14,15 +14,15 @@ pub struct ModuleList { modules: Vec, focus_handle: FocusHandle, dap_store: Entity, + session: Entity, client_id: DebugAdapterClientId, - session_id: DebugSessionId, } impl ModuleList { pub fn new( dap_store: Entity, + session: Entity, client_id: &DebugAdapterClientId, - session_id: &DebugSessionId, cx: &mut Context, ) -> Self { let weak_entity = cx.weak_entity(); @@ -42,10 +42,10 @@ impl ModuleList { let this = Self { list, + session, dap_store, focus_handle, client_id: *client_id, - session_id: *session_id, modules: Vec::default(), }; @@ -128,7 +128,7 @@ impl ModuleList { fn propagate_updates(&self, cx: &Context) { if let Some((client, id)) = self.dap_store.read(cx).downstream_client() { let request = UpdateDebugAdapter { - session_id: self.session_id.to_proto(), + session_id: self.session.read(cx).id().to_proto(), client_id: self.client_id.to_proto(), project_id: *id, thread_id: None, diff --git a/crates/debugger_ui/src/stack_frame_list.rs b/crates/debugger_ui/src/stack_frame_list.rs index 24c3fdbd83..07949e3af6 100644 --- a/crates/debugger_ui/src/stack_frame_list.rs +++ b/crates/debugger_ui/src/stack_frame_list.rs @@ -3,7 +3,7 @@ use std::path::Path; use anyhow::{anyhow, Result}; use dap::client::DebugAdapterClientId; use dap::proto_conversions::ProtoConversion; -use dap::session::DebugSessionId; +use dap::session::DebugSession; use dap::StackFrame; use gpui::{ list, AnyElement, Entity, EventEmitter, FocusHandle, Focusable, ListState, Subscription, Task, @@ -31,8 +31,8 @@ pub struct StackFrameList { thread_id: u64, list: ListState, focus_handle: FocusHandle, - session_id: DebugSessionId, dap_store: Entity, + session: Entity, stack_frames: Vec, entries: Vec, workspace: WeakEntity, @@ -51,11 +51,11 @@ pub enum StackFrameEntry { impl StackFrameList { #[allow(clippy::too_many_arguments)] pub fn new( - workspace: &WeakEntity, + workspace: WeakEntity, debug_panel_item: &Entity, - dap_store: &Entity, + dap_store: Entity, + session: Entity, client_id: &DebugAdapterClientId, - session_id: &DebugSessionId, thread_id: u64, window: &Window, cx: &mut Context, @@ -85,14 +85,14 @@ impl StackFrameList { Self { list, + session, + workspace, + dap_store, thread_id, focus_handle, _subscriptions, client_id: *client_id, - session_id: *session_id, entries: Default::default(), - workspace: workspace.clone(), - dap_store: dap_store.clone(), fetch_stack_frames_task: None, stack_frames: Default::default(), current_stack_frame_id: Default::default(), @@ -254,7 +254,7 @@ impl StackFrameList { if let Some((client, id)) = self.dap_store.read(cx).downstream_client() { let request = UpdateDebugAdapter { client_id: self.client_id.to_proto(), - session_id: self.session_id.to_proto(), + session_id: self.session.read(cx).id().to_proto(), project_id: *id, thread_id: Some(self.thread_id), variant: Some(rpc::proto::update_debug_adapter::Variant::StackFrameList( diff --git a/crates/debugger_ui/src/variable_list.rs b/crates/debugger_ui/src/variable_list.rs index 1798be2729..5171fae947 100644 --- a/crates/debugger_ui/src/variable_list.rs +++ b/crates/debugger_ui/src/variable_list.rs @@ -1,8 +1,8 @@ use crate::stack_frame_list::{StackFrameId, StackFrameList, StackFrameListEvent}; use anyhow::{anyhow, Result}; use dap::{ - client::DebugAdapterClientId, proto_conversions::ProtoConversion, session::DebugSessionId, - Scope, ScopePresentationHint, Variable, + client::DebugAdapterClientId, proto_conversions::ProtoConversion, session::DebugSession, Scope, + ScopePresentationHint, Variable, }; use editor::{actions::SelectAll, Editor, EditorEvent}; use gpui::{ @@ -330,11 +330,11 @@ pub struct VariableList { list: ListState, focus_handle: FocusHandle, dap_store: Entity, - session_id: DebugSessionId, open_entries: Vec, + session: Entity, client_id: DebugAdapterClientId, - set_variable_editor: Entity, _subscriptions: Vec, + set_variable_editor: Entity, selection: Option, stack_frame_list: Entity, scopes: HashMap>, @@ -347,10 +347,10 @@ pub struct VariableList { impl VariableList { pub fn new( - stack_frame_list: &Entity, - dap_store: Entity, + session: Entity, client_id: &DebugAdapterClientId, - session_id: &DebugSessionId, + dap_store: Entity, + stack_frame_list: Entity, window: &mut Window, cx: &mut Context, ) -> Self { @@ -382,17 +382,18 @@ impl VariableList { .detach(); let _subscriptions = - vec![cx.subscribe(stack_frame_list, Self::handle_stack_frame_list_events)]; + vec![cx.subscribe(&stack_frame_list, Self::handle_stack_frame_list_events)]; Self { list, + session, dap_store, focus_handle, _subscriptions, selection: None, + stack_frame_list, set_variable_editor, client_id: *client_id, - session_id: *session_id, open_context_menu: None, set_variable_state: None, fetch_variables_task: None, @@ -400,7 +401,6 @@ impl VariableList { entries: Default::default(), variables: Default::default(), open_entries: Default::default(), - stack_frame_list: stack_frame_list.clone(), } } @@ -672,7 +672,7 @@ impl VariableList { thread_id, stack_frame_id, scope_id, - self.session_id, + self.session.read(cx).id(), variable.variables_reference, cx, ) @@ -877,7 +877,7 @@ impl VariableList { thread_id, stack_frame_id, scope.variables_reference, - self.session_id, + self.session.read(cx).id(), container_reference, cx, ) @@ -970,7 +970,7 @@ impl VariableList { if let Some((client, project_id)) = self.dap_store.read(cx).downstream_client() { let request = UpdateDebugAdapter { client_id: self.client_id.to_proto(), - session_id: self.session_id.to_proto(), + session_id: self.session.read(cx).id().to_proto(), thread_id: Some(self.stack_frame_list.read(cx).thread_id()), project_id: *project_id, variant: Some(rpc::proto::update_debug_adapter::Variant::VariableList( diff --git a/crates/project/src/dap_store.rs b/crates/project/src/dap_store.rs index 04c1c2ac3b..517fe8cea3 100644 --- a/crates/project/src/dap_store.rs +++ b/crates/project/src/dap_store.rs @@ -2090,10 +2090,11 @@ impl DapStore { for session in local_store .sessions .values() - .filter_map(|session| session.read(cx).as_local()) + .filter(|session| session.read(cx).as_local().is_some()) { - let ignore_breakpoints = self.ignore_breakpoints(&session.id(), cx); - for client in session.clients().collect::>() { + let session = session.read(cx); + let ignore_breakpoints = session.ignore_breakpoints(); + for client in session.as_local().unwrap().clients().collect::>() { tasks.push(self.send_breakpoints( &client.id(), Arc::from(absolute_path.clone()),