debugger: Always focus the active session whenever it is stopped (#31182)

Piotr Osiewicz created 6 months ago

Closes #ISSUE

Release Notes:

- debugger: Fixed child debug sessions taking precedence over the
parents when spawned.

Change summary

crates/debugger_ui/src/debugger_panel.rs       | 147 +++++++++++--------
crates/debugger_ui/src/session/running.rs      |  15 +
crates/debugger_ui/src/tests/debugger_panel.rs |  33 ++++
3 files changed, 131 insertions(+), 64 deletions(-)

Detailed changes

crates/debugger_ui/src/debugger_panel.rs

@@ -273,7 +273,7 @@ impl DebugPanel {
             let session = session.clone();
             async move |this, cx| {
                 let debug_session =
-                    Self::register_session(this.clone(), session.clone(), cx).await?;
+                    Self::register_session(this.clone(), session.clone(), true, cx).await?;
                 let definition = debug_session
                     .update_in(cx, |debug_session, window, cx| {
                         debug_session.running_state().update(cx, |running, cx| {
@@ -318,69 +318,21 @@ impl DebugPanel {
     pub(crate) async fn register_session(
         this: WeakEntity<Self>,
         session: Entity<Session>,
+        focus: bool,
         cx: &mut AsyncWindowContext,
     ) -> Result<Entity<DebugSession>> {
-        let adapter_name = session.update(cx, |session, _| session.adapter())?;
-        this.update_in(cx, |_, window, cx| {
-            cx.subscribe_in(
-                &session,
-                window,
-                move |this, session, event: &SessionStateEvent, window, cx| match event {
-                    SessionStateEvent::Restart => {
-                        this.handle_restart_request(session.clone(), window, cx);
-                    }
-                    SessionStateEvent::SpawnChildSession { request } => {
-                        this.handle_start_debugging_request(request, session.clone(), window, cx);
-                    }
-                    _ => {}
-                },
-            )
-            .detach();
-        })
-        .ok();
-
-        let serialized_layout = persistence::get_serialized_layout(adapter_name).await;
-
-        let (debug_session, workspace) = this.update_in(cx, |this, window, cx| {
-            this.sessions.retain(|session| {
-                !session
-                    .read(cx)
-                    .running_state()
-                    .read(cx)
-                    .session()
-                    .read(cx)
-                    .is_terminated()
-            });
+        let debug_session = register_session_inner(&this, session, cx).await?;
 
-            let debug_session = DebugSession::running(
-                this.project.clone(),
-                this.workspace.clone(),
-                session,
-                cx.weak_entity(),
-                serialized_layout,
-                this.position(window, cx).axis(),
-                window,
-                cx,
-            );
-
-            // We might want to make this an event subscription and only notify when a new thread is selected
-            // This is used to filter the command menu correctly
-            cx.observe(
-                &debug_session.read(cx).running_state().clone(),
-                |_, _, cx| cx.notify(),
-            )
-            .detach();
-
-            this.sessions.push(debug_session.clone());
-            this.activate_session(debug_session.clone(), window, cx);
+        let workspace = this.update_in(cx, |this, window, cx| {
+            if focus {
+                this.activate_session(debug_session.clone(), window, cx);
+            }
 
-            (debug_session, this.workspace.clone())
+            this.workspace.clone()
         })?;
-
         workspace.update_in(cx, |workspace, window, cx| {
             workspace.focus_panel::<Self>(window, cx);
         })?;
-
         Ok(debug_session)
     }
 
@@ -418,7 +370,7 @@ impl DebugPanel {
                 });
                 (session, task)
             })?;
-            Self::register_session(this, session, cx).await?;
+            Self::register_session(this.clone(), session, true, cx).await?;
             task.await
         })
         .detach_and_log_err(cx);
@@ -441,7 +393,6 @@ impl DebugPanel {
         let adapter = parent_session.read(cx).adapter().clone();
         let mut binary = parent_session.read(cx).binary().clone();
         binary.request_args = request.clone();
-
         cx.spawn_in(window, async move |this, cx| {
             let (session, task) = dap_store_handle.update(cx, |dap_store, cx| {
                 let session =
@@ -452,7 +403,7 @@ impl DebugPanel {
                 });
                 (session, task)
             })?;
-            Self::register_session(this, session, cx).await?;
+            Self::register_session(this, session, false, cx).await?;
             task.await
         })
         .detach_and_log_err(cx);
@@ -910,6 +861,21 @@ impl DebugPanel {
         }
     }
 
+    pub(crate) fn activate_session_by_id(
+        &mut self,
+        session_id: SessionId,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        if let Some(session) = self
+            .sessions
+            .iter()
+            .find(|session| session.read(cx).session_id(cx) == session_id)
+        {
+            self.activate_session(session.clone(), window, cx);
+        }
+    }
+
     pub(crate) fn activate_session(
         &mut self,
         session_item: Entity<DebugSession>,
@@ -923,7 +889,7 @@ impl DebugPanel {
                 this.go_to_selected_stack_frame(window, cx);
             });
         });
-        self.active_session = Some(session_item.clone());
+        self.active_session = Some(session_item);
         cx.notify();
     }
 
@@ -999,6 +965,67 @@ impl DebugPanel {
     }
 }
 
+async fn register_session_inner(
+    this: &WeakEntity<DebugPanel>,
+    session: Entity<Session>,
+    cx: &mut AsyncWindowContext,
+) -> Result<Entity<DebugSession>> {
+    let adapter_name = session.update(cx, |session, _| session.adapter())?;
+    this.update_in(cx, |_, window, cx| {
+        cx.subscribe_in(
+            &session,
+            window,
+            move |this, session, event: &SessionStateEvent, window, cx| match event {
+                SessionStateEvent::Restart => {
+                    this.handle_restart_request(session.clone(), window, cx);
+                }
+                SessionStateEvent::SpawnChildSession { request } => {
+                    this.handle_start_debugging_request(request, session.clone(), window, cx);
+                }
+                _ => {}
+            },
+        )
+        .detach();
+    })
+    .ok();
+    let serialized_layout = persistence::get_serialized_layout(adapter_name).await;
+    let debug_session = this.update_in(cx, |this, window, cx| {
+        this.sessions.retain(|session| {
+            !session
+                .read(cx)
+                .running_state()
+                .read(cx)
+                .session()
+                .read(cx)
+                .is_terminated()
+        });
+
+        let debug_session = DebugSession::running(
+            this.project.clone(),
+            this.workspace.clone(),
+            session,
+            cx.weak_entity(),
+            serialized_layout,
+            this.position(window, cx).axis(),
+            window,
+            cx,
+        );
+
+        // We might want to make this an event subscription and only notify when a new thread is selected
+        // This is used to filter the command menu correctly
+        cx.observe(
+            &debug_session.read(cx).running_state().clone(),
+            |_, _, cx| cx.notify(),
+        )
+        .detach();
+
+        this.sessions.push(debug_session.clone());
+
+        debug_session
+    })?;
+    Ok(debug_session)
+}
+
 impl EventEmitter<PanelEvent> for DebugPanel {}
 impl EventEmitter<DebugPanelEvent> for DebugPanel {}
 

crates/debugger_ui/src/session/running.rs

@@ -585,15 +585,26 @@ impl RunningState {
             cx.subscribe_in(&session, window, |this, _, event, window, cx| {
                 match event {
                     SessionEvent::Stopped(thread_id) => {
-                        this.workspace
+                        let panel = this
+                            .workspace
                             .update(cx, |workspace, cx| {
                                 workspace.open_panel::<crate::DebugPanel>(window, cx);
+                                workspace.panel::<crate::DebugPanel>(cx)
                             })
-                            .log_err();
+                            .log_err()
+                            .flatten();
 
                         if let Some(thread_id) = thread_id {
                             this.select_thread(*thread_id, window, cx);
                         }
+                        if let Some(panel) = panel {
+                            let id = this.session_id;
+                            window.defer(cx, move |window, cx| {
+                                panel.update(cx, |this, cx| {
+                                    this.activate_session_by_id(id, window, cx);
+                                })
+                            })
+                        }
                     }
                     SessionEvent::Threads => {
                         let threads = this.session.update(cx, |this, cx| this.threads(cx));

crates/debugger_ui/src/tests/debugger_panel.rs

@@ -423,6 +423,13 @@ async fn test_handle_start_debugging_request(
         }
     });
 
+    let sessions = workspace
+        .update(cx, |workspace, _window, cx| {
+            let debug_panel = workspace.panel::<DebugPanel>(cx).unwrap();
+            debug_panel.read(cx).sessions()
+        })
+        .unwrap();
+    assert_eq!(sessions.len(), 1);
     client
         .fake_reverse_request::<StartDebugging>(StartDebuggingRequestArguments {
             request: StartDebuggingRequestArgumentsRequest::Launch,
@@ -435,20 +442,42 @@ async fn test_handle_start_debugging_request(
     workspace
         .update(cx, |workspace, _window, cx| {
             let debug_panel = workspace.panel::<DebugPanel>(cx).unwrap();
+
+            // Active session does not change on spawn.
             let active_session = debug_panel
                 .read(cx)
                 .active_session()
                 .unwrap()
                 .read(cx)
                 .session(cx);
-            let parent_session = active_session.read(cx).parent_session().unwrap();
+
+            assert_eq!(active_session, sessions[0].read(cx).session(cx));
+            assert!(active_session.read(cx).parent_session().is_none());
+
+            let current_sessions = debug_panel.read(cx).sessions();
+            assert_eq!(current_sessions.len(), 2);
+            assert_eq!(current_sessions[0], sessions[0]);
+
+            let parent_session = current_sessions[1]
+                .read(cx)
+                .session(cx)
+                .read(cx)
+                .parent_session()
+                .unwrap();
+            assert_eq!(parent_session, &sessions[0].read(cx).session(cx));
+
+            // We should preserve the original binary (params to spawn process etc.) except for launch params
+            // (as they come from reverse spawn request).
             let mut original_binary = parent_session.read(cx).binary().clone();
             original_binary.request_args = StartDebuggingRequestArguments {
                 request: StartDebuggingRequestArgumentsRequest::Launch,
                 configuration: fake_config.clone(),
             };
 
-            assert_eq!(active_session.read(cx).binary(), &original_binary);
+            assert_eq!(
+                current_sessions[1].read(cx).session(cx).read(cx).binary(),
+                &original_binary
+            );
         })
         .unwrap();