debugger: Select first stack frame with valid path (#32724)

Anthony Eid and Remco Smits created 6 months ago

This PR addresses an issue where we could get a stack frame list and
automatically select a stack frame that didn't have a valid path.
Causing a failure on Zed's end to select/update the active debug line.
The fix for this is selecting the first non-subtle stack frame that has
the optional path parameter.

We also made subtle stack frames move into their own collapsable list as
well.

Release Notes:

- debugger: Fix edge case where hitting a breakpoint wouldn't take you
to the active debug line

Co-authored-by: Remco Smits <[e-mail]>

Change summary

crates/debugger_ui/src/session/running/stack_frame_list.rs | 24 +++++--
crates/debugger_ui/src/tests/stack_frame_list.rs           |  4 
2 files changed, 18 insertions(+), 10 deletions(-)

Detailed changes

crates/debugger_ui/src/session/running/stack_frame_list.rs

@@ -184,7 +184,7 @@ impl StackFrameList {
         let mut entries = Vec::new();
         let mut collapsed_entries = Vec::new();
         let mut first_stack_frame = None;
-        let mut first_not_subtle_frame = None;
+        let mut first_stack_frame_with_path = None;
 
         let stack_frames = match self.stack_frames(cx) {
             Ok(stack_frames) => stack_frames,
@@ -200,7 +200,8 @@ impl StackFrameList {
         };
         for stack_frame in &stack_frames {
             match stack_frame.dap.presentation_hint {
-                Some(dap::StackFramePresentationHint::Deemphasize) => {
+                Some(dap::StackFramePresentationHint::Deemphasize)
+                | Some(dap::StackFramePresentationHint::Subtle) => {
                     collapsed_entries.push(stack_frame.dap.clone());
                 }
                 Some(dap::StackFramePresentationHint::Label) => {
@@ -213,10 +214,14 @@ impl StackFrameList {
                     }
 
                     first_stack_frame.get_or_insert(entries.len());
-                    if stack_frame.dap.presentation_hint
-                        != Some(dap::StackFramePresentationHint::Subtle)
+
+                    if stack_frame
+                        .dap
+                        .source
+                        .as_ref()
+                        .is_some_and(|source| source.path.is_some())
                     {
-                        first_not_subtle_frame.get_or_insert(entries.len());
+                        first_stack_frame_with_path.get_or_insert(entries.len());
                     }
                     entries.push(StackFrameEntry::Normal(stack_frame.dap.clone()));
                 }
@@ -229,7 +234,7 @@ impl StackFrameList {
         }
         self.entries = entries;
 
-        if let Some(ix) = first_not_subtle_frame
+        if let Some(ix) = first_stack_frame_with_path
             .or(first_stack_frame)
             .filter(|_| open_first_stack_frame)
         {
@@ -513,7 +518,7 @@ impl StackFrameList {
             .into_any()
     }
 
-    pub(crate) fn expand_collapsed_entry(&mut self, ix: usize) {
+    pub(crate) fn expand_collapsed_entry(&mut self, ix: usize, cx: &mut Context<Self>) {
         let Some(StackFrameEntry::Collapsed(stack_frames)) = self.entries.get_mut(ix) else {
             return;
         };
@@ -522,6 +527,9 @@ impl StackFrameList {
             .map(StackFrameEntry::Normal);
         self.entries.splice(ix..ix + 1, entries);
         self.selected_ix = Some(ix);
+        self.list_state.reset(self.entries.len());
+        cx.emit(StackFrameListEvent::BuiltEntries);
+        cx.notify();
     }
 
     fn render_collapsed_entry(
@@ -691,7 +699,7 @@ impl StackFrameList {
             StackFrameEntry::Label(_) => {
                 debug_panic!("You should not be able to select a label stack frame")
             }
-            StackFrameEntry::Collapsed(_) => self.expand_collapsed_entry(ix),
+            StackFrameEntry::Collapsed(_) => self.expand_collapsed_entry(ix, cx),
         }
         cx.notify();
     }

crates/debugger_ui/src/tests/stack_frame_list.rs

@@ -718,7 +718,7 @@ async fn test_collapsed_entries(executor: BackgroundExecutor, cx: &mut TestAppCo
                 stack_frame_list.entries()
             );
 
-            stack_frame_list.expand_collapsed_entry(1);
+            stack_frame_list.expand_collapsed_entry(1, cx);
 
             assert_eq!(
                 &vec![
@@ -735,7 +735,7 @@ async fn test_collapsed_entries(executor: BackgroundExecutor, cx: &mut TestAppCo
                 stack_frame_list.entries()
             );
 
-            stack_frame_list.expand_collapsed_entry(4);
+            stack_frame_list.expand_collapsed_entry(4, cx);
 
             assert_eq!(
                 &vec![