debugger: Make the stack frame list and module list keyboard-navigable (#30682)

Cole Miller created

- Switch stack frame list and module list to `UniformList` to access
scrolling behavior
- Implement `menu::` navigation actions

Release Notes:

- Debugger Beta: Added support for menu navigation actions (`ctrl-n`,
`ctrl-p`, etc.) in the stack frame list and module list.

Change summary

crates/debugger_ui/src/session/running.rs                  |  12 
crates/debugger_ui/src/session/running/console.rs          |   4 
crates/debugger_ui/src/session/running/module_list.rs      | 186 +++-
crates/debugger_ui/src/session/running/stack_frame_list.rs | 321 ++++---
crates/debugger_ui/src/stack_trace_view.rs                 |   4 
crates/debugger_ui/src/tests/stack_frame_list.rs           |  20 
crates/debugger_ui/src/tests/variable_list.rs              |  14 
7 files changed, 360 insertions(+), 201 deletions(-)

Detailed changes

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

@@ -1219,10 +1219,16 @@ impl RunningState {
         }
     }
 
-    pub(crate) fn go_to_selected_stack_frame(&self, window: &Window, cx: &mut Context<Self>) {
+    pub(crate) fn go_to_selected_stack_frame(&self, window: &mut Window, cx: &mut Context<Self>) {
         if self.thread_id.is_some() {
             self.stack_frame_list
-                .update(cx, |list, cx| list.go_to_selected_stack_frame(window, cx));
+                .update(cx, |list, cx| {
+                    let Some(stack_frame_id) = list.opened_stack_frame_id() else {
+                        return Task::ready(Ok(()));
+                    };
+                    list.go_to_stack_frame(stack_frame_id, window, cx)
+                })
+                .detach();
         }
     }
 
@@ -1239,7 +1245,7 @@ impl RunningState {
     }
 
     pub(crate) fn selected_stack_frame_id(&self, cx: &App) -> Option<dap::StackFrameId> {
-        self.stack_frame_list.read(cx).selected_stack_frame_id()
+        self.stack_frame_list.read(cx).opened_stack_frame_id()
     }
 
     pub(crate) fn stack_frame_list(&self) -> &Entity<StackFrameList> {

crates/debugger_ui/src/session/running/console.rs 🔗

@@ -162,7 +162,7 @@ impl Console {
                 .evaluate(
                     expression,
                     Some(dap::EvaluateArgumentsContext::Repl),
-                    self.stack_frame_list.read(cx).selected_stack_frame_id(),
+                    self.stack_frame_list.read(cx).opened_stack_frame_id(),
                     None,
                     cx,
                 )
@@ -389,7 +389,7 @@ impl ConsoleQueryBarCompletionProvider {
     ) -> Task<Result<Option<Vec<Completion>>>> {
         let completion_task = console.update(cx, |console, cx| {
             console.session.update(cx, |state, cx| {
-                let frame_id = console.stack_frame_list.read(cx).selected_stack_frame_id();
+                let frame_id = console.stack_frame_list.read(cx).opened_stack_frame_id();
 
                 state.completions(
                     CompletionsQuery::new(buffer.read(cx), buffer_position, frame_id),

crates/debugger_ui/src/session/running/module_list.rs 🔗

@@ -1,7 +1,8 @@
 use anyhow::anyhow;
+use dap::Module;
 use gpui::{
-    AnyElement, Empty, Entity, FocusHandle, Focusable, ListState, MouseButton, Stateful,
-    Subscription, WeakEntity, list,
+    AnyElement, Entity, FocusHandle, Focusable, MouseButton, ScrollStrategy, Stateful,
+    Subscription, Task, UniformListScrollHandle, WeakEntity, uniform_list,
 };
 use project::{
     ProjectItem as _, ProjectPath,
@@ -9,16 +10,17 @@ use project::{
 };
 use std::{path::Path, sync::Arc};
 use ui::{Scrollbar, ScrollbarState, prelude::*};
-use util::maybe;
 use workspace::Workspace;
 
 pub struct ModuleList {
-    list: ListState,
-    invalidate: bool,
+    scroll_handle: UniformListScrollHandle,
+    selected_ix: Option<usize>,
     session: Entity<Session>,
     workspace: WeakEntity<Workspace>,
     focus_handle: FocusHandle,
     scrollbar_state: ScrollbarState,
+    entries: Vec<Module>,
+    _rebuild_task: Task<()>,
     _subscription: Subscription,
 }
 
@@ -28,38 +30,43 @@ impl ModuleList {
         workspace: WeakEntity<Workspace>,
         cx: &mut Context<Self>,
     ) -> Self {
-        let weak_entity = cx.weak_entity();
         let focus_handle = cx.focus_handle();
 
-        let list = ListState::new(
-            0,
-            gpui::ListAlignment::Top,
-            px(1000.),
-            move |ix, _window, cx| {
-                weak_entity
-                    .upgrade()
-                    .map(|module_list| module_list.update(cx, |this, cx| this.render_entry(ix, cx)))
-                    .unwrap_or(div().into_any())
-            },
-        );
-
         let _subscription = cx.subscribe(&session, |this, _, event, cx| match event {
             SessionEvent::Stopped(_) | SessionEvent::Modules => {
-                this.invalidate = true;
-                cx.notify();
+                this.schedule_rebuild(cx);
             }
             _ => {}
         });
 
-        Self {
-            scrollbar_state: ScrollbarState::new(list.clone()),
-            list,
+        let scroll_handle = UniformListScrollHandle::new();
+
+        let mut this = Self {
+            scrollbar_state: ScrollbarState::new(scroll_handle.clone()),
+            scroll_handle,
             session,
             workspace,
             focus_handle,
+            entries: Vec::new(),
+            selected_ix: None,
             _subscription,
-            invalidate: true,
-        }
+            _rebuild_task: Task::ready(()),
+        };
+        this.schedule_rebuild(cx);
+        this
+    }
+
+    fn schedule_rebuild(&mut self, cx: &mut Context<Self>) {
+        self._rebuild_task = cx.spawn(async move |this, cx| {
+            this.update(cx, |this, cx| {
+                let modules = this
+                    .session
+                    .update(cx, |session, cx| session.modules(cx).to_owned());
+                this.entries = modules;
+                cx.notify();
+            })
+            .ok();
+        });
     }
 
     fn open_module(&mut self, path: Arc<Path>, window: &mut Window, cx: &mut Context<Self>) {
@@ -111,16 +118,11 @@ impl ModuleList {
 
             anyhow::Ok(())
         })
-        .detach_and_log_err(cx);
+        .detach();
     }
 
     fn render_entry(&mut self, ix: usize, cx: &mut Context<Self>) -> AnyElement {
-        let Some(module) = maybe!({
-            self.session
-                .update(cx, |state, cx| state.modules(cx).get(ix).cloned())
-        }) else {
-            return Empty.into_any();
-        };
+        let module = self.entries[ix].clone();
 
         v_flex()
             .rounded_md()
@@ -129,18 +131,24 @@ impl ModuleList {
             .id(("module-list", ix))
             .when(module.path.is_some(), |this| {
                 this.on_click({
-                    let path = module.path.as_deref().map(|path| Arc::<Path>::from(Path::new(path)));
+                    let path = module
+                        .path
+                        .as_deref()
+                        .map(|path| Arc::<Path>::from(Path::new(path)));
                     cx.listener(move |this, _, window, cx| {
+                        this.selected_ix = Some(ix);
                         if let Some(path) = path.as_ref() {
                             this.open_module(path.clone(), window, cx);
-                        } else {
-                            log::error!("Wasn't able to find module path, but was still able to click on module list entry");
                         }
+                        cx.notify();
                     })
                 })
             })
             .p_1()
             .hover(|s| s.bg(cx.theme().colors().element_hover))
+            .when(Some(ix) == self.selected_ix, |s| {
+                s.bg(cx.theme().colors().element_hover)
+            })
             .child(h_flex().gap_0p5().text_ui_sm(cx).child(module.name.clone()))
             .child(
                 h_flex()
@@ -188,6 +196,96 @@ impl ModuleList {
             .cursor_default()
             .children(Scrollbar::vertical(self.scrollbar_state.clone()))
     }
+
+    fn confirm(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context<Self>) {
+        let Some(ix) = self.selected_ix else { return };
+        let Some(entry) = self.entries.get(ix) else {
+            return;
+        };
+        let Some(path) = entry.path.as_deref() else {
+            return;
+        };
+        let path = Arc::from(Path::new(path));
+        self.open_module(path, window, cx);
+    }
+
+    fn select_ix(&mut self, ix: Option<usize>, cx: &mut Context<Self>) {
+        self.selected_ix = ix;
+        if let Some(ix) = ix {
+            self.scroll_handle
+                .scroll_to_item(ix, ScrollStrategy::Center);
+        }
+        cx.notify();
+    }
+
+    fn select_next(&mut self, _: &menu::SelectNext, _window: &mut Window, cx: &mut Context<Self>) {
+        let ix = match self.selected_ix {
+            _ if self.entries.len() == 0 => None,
+            None => Some(0),
+            Some(ix) => {
+                if ix == self.entries.len() - 1 {
+                    Some(0)
+                } else {
+                    Some(ix + 1)
+                }
+            }
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn select_previous(
+        &mut self,
+        _: &menu::SelectPrevious,
+        _window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        let ix = match self.selected_ix {
+            _ if self.entries.len() == 0 => None,
+            None => Some(self.entries.len() - 1),
+            Some(ix) => {
+                if ix == 0 {
+                    Some(self.entries.len() - 1)
+                } else {
+                    Some(ix - 1)
+                }
+            }
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn select_first(
+        &mut self,
+        _: &menu::SelectFirst,
+        _window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        let ix = if self.entries.len() > 0 {
+            Some(0)
+        } else {
+            None
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn select_last(&mut self, _: &menu::SelectLast, _window: &mut Window, cx: &mut Context<Self>) {
+        let ix = if self.entries.len() > 0 {
+            Some(self.entries.len() - 1)
+        } else {
+            None
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn render_list(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        uniform_list(
+            cx.entity(),
+            "module-list",
+            self.entries.len(),
+            |this, range, _window, cx| range.map(|ix| this.render_entry(ix, cx)).collect(),
+        )
+        .track_scroll(self.scroll_handle.clone())
+        .size_full()
+    }
 }
 
 impl Focusable for ModuleList {
@@ -197,21 +295,17 @@ impl Focusable for ModuleList {
 }
 
 impl Render for ModuleList {
-    fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
-        if self.invalidate {
-            let len = self
-                .session
-                .update(cx, |session, cx| session.modules(cx).len());
-            self.list.reset(len);
-            self.invalidate = false;
-            cx.notify();
-        }
-
+    fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         div()
             .track_focus(&self.focus_handle)
+            .on_action(cx.listener(Self::select_last))
+            .on_action(cx.listener(Self::select_first))
+            .on_action(cx.listener(Self::select_next))
+            .on_action(cx.listener(Self::select_previous))
+            .on_action(cx.listener(Self::confirm))
             .size_full()
             .p_1()
-            .child(list(self.list.clone()).size_full())
+            .child(self.render_list(window, cx))
             .child(self.render_vertical_scrollbar(cx))
     }
 }

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

@@ -5,20 +5,18 @@ use std::time::Duration;
 use anyhow::{Result, anyhow};
 use dap::StackFrameId;
 use gpui::{
-    AnyElement, Entity, EventEmitter, FocusHandle, Focusable, ListState, MouseButton, Stateful,
-    Subscription, Task, WeakEntity, list,
+    AnyElement, Entity, EventEmitter, FocusHandle, Focusable, MouseButton, ScrollStrategy,
+    Stateful, Subscription, Task, UniformListScrollHandle, WeakEntity, uniform_list,
 };
 
+use crate::StackTraceView;
 use language::PointUtf16;
 use project::debugger::breakpoint_store::ActiveStackFrame;
 use project::debugger::session::{Session, SessionEvent, StackFrame};
 use project::{ProjectItem, ProjectPath};
 use ui::{Scrollbar, ScrollbarState, Tooltip, prelude::*};
-use util::ResultExt;
 use workspace::{ItemHandle, Workspace};
 
-use crate::StackTraceView;
-
 use super::RunningState;
 
 #[derive(Debug)]
@@ -28,15 +26,16 @@ pub enum StackFrameListEvent {
 }
 
 pub struct StackFrameList {
-    list: ListState,
     focus_handle: FocusHandle,
     _subscription: Subscription,
     session: Entity<Session>,
     state: WeakEntity<RunningState>,
     entries: Vec<StackFrameEntry>,
     workspace: WeakEntity<Workspace>,
-    selected_stack_frame_id: Option<StackFrameId>,
+    selected_ix: Option<usize>,
+    opened_stack_frame_id: Option<StackFrameId>,
     scrollbar_state: ScrollbarState,
+    scroll_handle: UniformListScrollHandle,
     _refresh_task: Task<()>,
 }
 
@@ -55,22 +54,8 @@ impl StackFrameList {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Self {
-        let weak_entity = cx.weak_entity();
         let focus_handle = cx.focus_handle();
-
-        let list = ListState::new(
-            0,
-            gpui::ListAlignment::Top,
-            px(1000.),
-            move |ix, _window, cx| {
-                weak_entity
-                    .upgrade()
-                    .map(|stack_frame_list| {
-                        stack_frame_list.update(cx, |this, cx| this.render_entry(ix, cx))
-                    })
-                    .unwrap_or(div().into_any())
-            },
-        );
+        let scroll_handle = UniformListScrollHandle::new();
 
         let _subscription =
             cx.subscribe_in(&session, window, |this, _, event, window, cx| match event {
@@ -84,15 +69,16 @@ impl StackFrameList {
             });
 
         let mut this = Self {
-            scrollbar_state: ScrollbarState::new(list.clone()),
-            list,
+            scrollbar_state: ScrollbarState::new(scroll_handle.clone()),
             session,
             workspace,
             focus_handle,
             state,
             _subscription,
             entries: Default::default(),
-            selected_stack_frame_id: None,
+            selected_ix: None,
+            opened_stack_frame_id: None,
+            scroll_handle,
             _refresh_task: Task::ready(()),
         };
         this.schedule_refresh(true, window, cx);
@@ -123,7 +109,7 @@ impl StackFrameList {
     fn stack_frames(&self, cx: &mut App) -> Vec<StackFrame> {
         self.state
             .read_with(cx, |state, _| state.thread_id)
-            .log_err()
+            .ok()
             .flatten()
             .map(|thread_id| {
                 self.session
@@ -140,27 +126,8 @@ impl StackFrameList {
             .collect()
     }
 
-    pub fn selected_stack_frame_id(&self) -> Option<StackFrameId> {
-        self.selected_stack_frame_id
-    }
-
-    pub(crate) fn select_stack_frame_id(
-        &mut self,
-        id: StackFrameId,
-        window: &Window,
-        cx: &mut Context<Self>,
-    ) {
-        if !self.entries.iter().any(|entry| match entry {
-            StackFrameEntry::Normal(entry) => entry.id == id,
-            StackFrameEntry::Collapsed(stack_frames) => {
-                stack_frames.iter().any(|frame| frame.id == id)
-            }
-        }) {
-            return;
-        }
-
-        self.selected_stack_frame_id = Some(id);
-        self.go_to_selected_stack_frame(window, cx);
+    pub fn opened_stack_frame_id(&self) -> Option<StackFrameId> {
+        self.opened_stack_frame_id
     }
 
     pub(super) fn schedule_refresh(
@@ -193,13 +160,22 @@ impl StackFrameList {
 
     pub fn build_entries(
         &mut self,
-        select_first_stack_frame: bool,
+        open_first_stack_frame: bool,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
+        let old_selected_frame_id = self
+            .selected_ix
+            .and_then(|ix| self.entries.get(ix))
+            .and_then(|entry| match entry {
+                StackFrameEntry::Normal(stack_frame) => Some(stack_frame.id),
+                StackFrameEntry::Collapsed(stack_frames) => {
+                    stack_frames.first().map(|stack_frame| stack_frame.id)
+                }
+            });
         let mut entries = Vec::new();
         let mut collapsed_entries = Vec::new();
-        let mut current_stack_frame = None;
+        let mut first_stack_frame = None;
 
         let stack_frames = self.stack_frames(cx);
         for stack_frame in &stack_frames {
@@ -213,7 +189,7 @@ impl StackFrameList {
                         entries.push(StackFrameEntry::Collapsed(collapsed_entries.clone()));
                     }
 
-                    current_stack_frame.get_or_insert(&stack_frame.dap);
+                    first_stack_frame.get_or_insert(entries.len());
                     entries.push(StackFrameEntry::Normal(stack_frame.dap.clone()));
                 }
             }
@@ -225,69 +201,60 @@ impl StackFrameList {
         }
 
         std::mem::swap(&mut self.entries, &mut entries);
-        self.list.reset(self.entries.len());
 
-        if let Some(current_stack_frame) = current_stack_frame.filter(|_| select_first_stack_frame)
-        {
-            self.select_stack_frame(current_stack_frame, true, window, cx)
-                .detach_and_log_err(cx);
+        if let Some(ix) = first_stack_frame.filter(|_| open_first_stack_frame) {
+            self.select_ix(Some(ix), cx);
+            self.activate_selected_entry(window, cx);
+        } else if let Some(old_selected_frame_id) = old_selected_frame_id {
+            let ix = self.entries.iter().position(|entry| match entry {
+                StackFrameEntry::Normal(frame) => frame.id == old_selected_frame_id,
+                StackFrameEntry::Collapsed(frames) => {
+                    frames.iter().any(|frame| frame.id == old_selected_frame_id)
+                }
+            });
+            self.selected_ix = ix;
         }
 
         cx.emit(StackFrameListEvent::BuiltEntries);
         cx.notify();
     }
 
-    pub fn go_to_selected_stack_frame(&mut self, window: &Window, cx: &mut Context<Self>) {
-        if let Some(selected_stack_frame_id) = self.selected_stack_frame_id {
-            let frame = self
-                .entries
-                .iter()
-                .find_map(|entry| match entry {
-                    StackFrameEntry::Normal(dap) => {
-                        if dap.id == selected_stack_frame_id {
-                            Some(dap)
-                        } else {
-                            None
-                        }
-                    }
-                    StackFrameEntry::Collapsed(daps) => {
-                        daps.iter().find(|dap| dap.id == selected_stack_frame_id)
-                    }
-                })
-                .cloned();
-
-            if let Some(frame) = frame.as_ref() {
-                self.select_stack_frame(frame, true, window, cx)
-                    .detach_and_log_err(cx);
-            }
-        }
-    }
-
-    pub fn select_stack_frame(
+    pub fn go_to_stack_frame(
         &mut self,
-        stack_frame: &dap::StackFrame,
-        go_to_stack_frame: bool,
-        window: &Window,
+        stack_frame_id: StackFrameId,
+        window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Result<()>> {
-        self.selected_stack_frame_id = Some(stack_frame.id);
-
-        cx.emit(StackFrameListEvent::SelectedStackFrameChanged(
-            stack_frame.id,
-        ));
-        cx.notify();
-
-        if !go_to_stack_frame {
-            return Task::ready(Ok(()));
+        let Some(stack_frame) = self
+            .entries
+            .iter()
+            .flat_map(|entry| match entry {
+                StackFrameEntry::Normal(stack_frame) => std::slice::from_ref(stack_frame),
+                StackFrameEntry::Collapsed(stack_frames) => stack_frames.as_slice(),
+            })
+            .find(|stack_frame| stack_frame.id == stack_frame_id)
+            .cloned()
+        else {
+            return Task::ready(Err(anyhow!("No stack frame for ID")));
         };
+        self.go_to_stack_frame_inner(stack_frame, window, cx)
+    }
 
-        let row = (stack_frame.line.saturating_sub(1)) as u32;
-
+    fn go_to_stack_frame_inner(
+        &mut self,
+        stack_frame: dap::StackFrame,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) -> Task<Result<()>> {
+        let stack_frame_id = stack_frame.id;
+        self.opened_stack_frame_id = Some(stack_frame_id);
         let Some(abs_path) = Self::abs_path_from_stack_frame(&stack_frame) else {
             return Task::ready(Err(anyhow!("Project path not found")));
         };
-
-        let stack_frame_id = stack_frame.id;
+        let row = stack_frame.line.saturating_sub(1) as u32;
+        cx.emit(StackFrameListEvent::SelectedStackFrameChanged(
+            stack_frame_id,
+        ));
         cx.spawn_in(window, async move |this, cx| {
             let (worktree, relative_path) = this
                 .update(cx, |this, cx| {
@@ -386,11 +353,12 @@ impl StackFrameList {
 
     fn render_normal_entry(
         &self,
+        ix: usize,
         stack_frame: &dap::StackFrame,
         cx: &mut Context<Self>,
     ) -> AnyElement {
         let source = stack_frame.source.clone();
-        let is_selected_frame = Some(stack_frame.id) == self.selected_stack_frame_id;
+        let is_selected_frame = Some(ix) == self.selected_ix;
 
         let path = source.clone().and_then(|s| s.path.or(s.name));
         let formatted_path = path.map(|path| format!("{}:{}", path, stack_frame.line,));
@@ -426,12 +394,9 @@ impl StackFrameList {
             .when(is_selected_frame, |this| {
                 this.bg(cx.theme().colors().element_hover)
             })
-            .on_click(cx.listener({
-                let stack_frame = stack_frame.clone();
-                move |this, _, window, cx| {
-                    this.select_stack_frame(&stack_frame, true, window, cx)
-                        .detach_and_log_err(cx);
-                }
+            .on_click(cx.listener(move |this, _, window, cx| {
+                this.selected_ix = Some(ix);
+                this.activate_selected_entry(window, cx);
             }))
             .hover(|style| style.bg(cx.theme().colors().element_hover).cursor_pointer())
             .child(
@@ -486,20 +451,15 @@ impl StackFrameList {
             .into_any()
     }
 
-    pub fn expand_collapsed_entry(
-        &mut self,
-        ix: usize,
-        stack_frames: &Vec<dap::StackFrame>,
-        cx: &mut Context<Self>,
-    ) {
-        self.entries.splice(
-            ix..ix + 1,
-            stack_frames
-                .iter()
-                .map(|frame| StackFrameEntry::Normal(frame.clone())),
-        );
-        self.list.reset(self.entries.len());
-        cx.notify();
+    pub(crate) fn expand_collapsed_entry(&mut self, ix: usize) {
+        let Some(StackFrameEntry::Collapsed(stack_frames)) = self.entries.get_mut(ix) else {
+            return;
+        };
+        let entries = std::mem::take(stack_frames)
+            .into_iter()
+            .map(StackFrameEntry::Normal);
+        self.entries.splice(ix..ix + 1, entries);
+        self.selected_ix = Some(ix);
     }
 
     fn render_collapsed_entry(
@@ -509,6 +469,7 @@ impl StackFrameList {
         cx: &mut Context<Self>,
     ) -> AnyElement {
         let first_stack_frame = &stack_frames[0];
+        let is_selected = Some(ix) == self.selected_ix;
 
         h_flex()
             .rounded_md()
@@ -517,11 +478,12 @@ impl StackFrameList {
             .group("")
             .id(("stack-frame", first_stack_frame.id))
             .p_1()
-            .on_click(cx.listener({
-                let stack_frames = stack_frames.clone();
-                move |this, _, _window, cx| {
-                    this.expand_collapsed_entry(ix, &stack_frames, cx);
-                }
+            .when(is_selected, |this| {
+                this.bg(cx.theme().colors().element_hover)
+            })
+            .on_click(cx.listener(move |this, _, window, cx| {
+                this.selected_ix = Some(ix);
+                this.activate_selected_entry(window, cx);
             }))
             .hover(|style| style.bg(cx.theme().colors().element_hover).cursor_pointer())
             .child(
@@ -544,7 +506,7 @@ impl StackFrameList {
 
     fn render_entry(&self, ix: usize, cx: &mut Context<Self>) -> AnyElement {
         match &self.entries[ix] {
-            StackFrameEntry::Normal(stack_frame) => self.render_normal_entry(stack_frame, cx),
+            StackFrameEntry::Normal(stack_frame) => self.render_normal_entry(ix, stack_frame, cx),
             StackFrameEntry::Collapsed(stack_frames) => {
                 self.render_collapsed_entry(ix, stack_frames, cx)
             }
@@ -583,15 +545,120 @@ impl StackFrameList {
             .cursor_default()
             .children(Scrollbar::vertical(self.scrollbar_state.clone()))
     }
+
+    fn select_ix(&mut self, ix: Option<usize>, cx: &mut Context<Self>) {
+        self.selected_ix = ix;
+        if let Some(ix) = self.selected_ix {
+            self.scroll_handle
+                .scroll_to_item(ix, ScrollStrategy::Center);
+        }
+        cx.notify();
+    }
+
+    fn select_next(&mut self, _: &menu::SelectNext, _window: &mut Window, cx: &mut Context<Self>) {
+        let ix = match self.selected_ix {
+            _ if self.entries.len() == 0 => None,
+            None => Some(0),
+            Some(ix) => {
+                if ix == self.entries.len() - 1 {
+                    Some(0)
+                } else {
+                    Some(ix + 1)
+                }
+            }
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn select_previous(
+        &mut self,
+        _: &menu::SelectPrevious,
+        _window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        let ix = match self.selected_ix {
+            _ if self.entries.len() == 0 => None,
+            None => Some(self.entries.len() - 1),
+            Some(ix) => {
+                if ix == 0 {
+                    Some(self.entries.len() - 1)
+                } else {
+                    Some(ix - 1)
+                }
+            }
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn select_first(
+        &mut self,
+        _: &menu::SelectFirst,
+        _window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        let ix = if self.entries.len() > 0 {
+            Some(0)
+        } else {
+            None
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn select_last(&mut self, _: &menu::SelectLast, _window: &mut Window, cx: &mut Context<Self>) {
+        let ix = if self.entries.len() > 0 {
+            Some(self.entries.len() - 1)
+        } else {
+            None
+        };
+        self.select_ix(ix, cx);
+    }
+
+    fn activate_selected_entry(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+        let Some(ix) = self.selected_ix else {
+            return;
+        };
+        let Some(entry) = self.entries.get_mut(ix) else {
+            return;
+        };
+        match entry {
+            StackFrameEntry::Normal(stack_frame) => {
+                let stack_frame = stack_frame.clone();
+                self.go_to_stack_frame_inner(stack_frame, window, cx)
+                    .detach_and_log_err(cx)
+            }
+            StackFrameEntry::Collapsed(_) => self.expand_collapsed_entry(ix),
+        }
+        cx.notify();
+    }
+
+    fn confirm(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context<Self>) {
+        self.activate_selected_entry(window, cx);
+    }
+
+    fn render_list(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        uniform_list(
+            cx.entity(),
+            "stack-frame-list",
+            self.entries.len(),
+            |this, range, _window, cx| range.map(|ix| this.render_entry(ix, cx)).collect(),
+        )
+        .track_scroll(self.scroll_handle.clone())
+        .size_full()
+    }
 }
 
 impl Render for StackFrameList {
-    fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+    fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         div()
             .track_focus(&self.focus_handle)
             .size_full()
             .p_1()
-            .child(list(self.list.clone()).size_full())
+            .on_action(cx.listener(Self::select_next))
+            .on_action(cx.listener(Self::select_previous))
+            .on_action(cx.listener(Self::select_first))
+            .on_action(cx.listener(Self::select_last))
+            .on_action(cx.listener(Self::confirm))
+            .child(self.render_list(window, cx))
             .child(self.render_vertical_scrollbar(cx))
     }
 }

crates/debugger_ui/src/stack_trace_view.rs 🔗

@@ -69,7 +69,7 @@ impl StackTraceView {
                     .filter(|id| Some(**id) != this.selected_stack_frame_id)
                 {
                     this.stack_frame_list.update(cx, |list, cx| {
-                        list.select_stack_frame_id(*stack_frame_id, window, cx);
+                        list.go_to_stack_frame(*stack_frame_id, window, cx).detach();
                     });
                 }
             }
@@ -82,7 +82,7 @@ impl StackTraceView {
             |this, stack_frame_list, event, window, cx| match event {
                 StackFrameListEvent::BuiltEntries => {
                     this.selected_stack_frame_id =
-                        stack_frame_list.read(cx).selected_stack_frame_id();
+                        stack_frame_list.read(cx).opened_stack_frame_id();
                     this.update_excerpts(window, cx);
                 }
                 StackFrameListEvent::SelectedStackFrameChanged(selected_frame_id) => {

crates/debugger_ui/src/tests/stack_frame_list.rs 🔗

@@ -168,7 +168,7 @@ async fn test_fetch_initial_stack_frames_and_go_to_stack_frame(
             .update(cx, |state, _| state.stack_frame_list().clone());
 
         stack_frame_list.update(cx, |stack_frame_list, cx| {
-            assert_eq!(Some(1), stack_frame_list.selected_stack_frame_id());
+            assert_eq!(Some(1), stack_frame_list.opened_stack_frame_id());
             assert_eq!(stack_frames, stack_frame_list.dap_stack_frames(cx));
         });
     });
@@ -373,14 +373,14 @@ async fn test_select_stack_frame(executor: BackgroundExecutor, cx: &mut TestAppC
         .unwrap();
 
     stack_frame_list.update(cx, |stack_frame_list, cx| {
-        assert_eq!(Some(1), stack_frame_list.selected_stack_frame_id());
+        assert_eq!(Some(1), stack_frame_list.opened_stack_frame_id());
         assert_eq!(stack_frames, stack_frame_list.dap_stack_frames(cx));
     });
 
     // select second stack frame
     stack_frame_list
         .update_in(cx, |stack_frame_list, window, cx| {
-            stack_frame_list.select_stack_frame(&stack_frames[1], true, window, cx)
+            stack_frame_list.go_to_stack_frame(stack_frames[1].id, window, cx)
         })
         .await
         .unwrap();
@@ -388,7 +388,7 @@ async fn test_select_stack_frame(executor: BackgroundExecutor, cx: &mut TestAppC
     cx.run_until_parked();
 
     stack_frame_list.update(cx, |stack_frame_list, cx| {
-        assert_eq!(Some(2), stack_frame_list.selected_stack_frame_id());
+        assert_eq!(Some(2), stack_frame_list.opened_stack_frame_id());
         assert_eq!(stack_frames, stack_frame_list.dap_stack_frames(cx));
     });
 
@@ -718,11 +718,7 @@ async fn test_collapsed_entries(executor: BackgroundExecutor, cx: &mut TestAppCo
                 stack_frame_list.entries()
             );
 
-            stack_frame_list.expand_collapsed_entry(
-                1,
-                &vec![stack_frames[1].clone(), stack_frames[2].clone()],
-                cx,
-            );
+            stack_frame_list.expand_collapsed_entry(1);
 
             assert_eq!(
                 &vec![
@@ -739,11 +735,7 @@ async fn test_collapsed_entries(executor: BackgroundExecutor, cx: &mut TestAppCo
                 stack_frame_list.entries()
             );
 
-            stack_frame_list.expand_collapsed_entry(
-                4,
-                &vec![stack_frames[4].clone(), stack_frames[5].clone()],
-                cx,
-            );
+            stack_frame_list.expand_collapsed_entry(4);
 
             assert_eq!(
                 &vec![

crates/debugger_ui/src/tests/variable_list.rs 🔗

@@ -190,7 +190,7 @@ async fn test_basic_fetch_initial_scope_and_variables(
     running_state.update(cx, |running_state, cx| {
         let (stack_frame_list, stack_frame_id) =
             running_state.stack_frame_list().update(cx, |list, _| {
-                (list.flatten_entries(true), list.selected_stack_frame_id())
+                (list.flatten_entries(true), list.opened_stack_frame_id())
             });
 
         assert_eq!(stack_frames, stack_frame_list);
@@ -431,7 +431,7 @@ async fn test_fetch_variables_for_multiple_scopes(
     running_state.update(cx, |running_state, cx| {
         let (stack_frame_list, stack_frame_id) =
             running_state.stack_frame_list().update(cx, |list, _| {
-                (list.flatten_entries(true), list.selected_stack_frame_id())
+                (list.flatten_entries(true), list.opened_stack_frame_id())
             });
 
         assert_eq!(Some(1), stack_frame_id);
@@ -1452,7 +1452,7 @@ async fn test_variable_list_only_sends_requests_when_rendering(
     running_state.update(cx, |running_state, cx| {
         let (stack_frame_list, stack_frame_id) =
             running_state.stack_frame_list().update(cx, |list, _| {
-                (list.flatten_entries(true), list.selected_stack_frame_id())
+                (list.flatten_entries(true), list.opened_stack_frame_id())
             });
 
         assert_eq!(Some(1), stack_frame_id);
@@ -1734,7 +1734,7 @@ async fn test_it_fetches_scopes_variables_when_you_select_a_stack_frame(
     running_state.update(cx, |running_state, cx| {
         let (stack_frame_list, stack_frame_id) =
             running_state.stack_frame_list().update(cx, |list, _| {
-                (list.flatten_entries(true), list.selected_stack_frame_id())
+                (list.flatten_entries(true), list.opened_stack_frame_id())
             });
 
         let variable_list = running_state.variable_list().read(cx);
@@ -1745,7 +1745,7 @@ async fn test_it_fetches_scopes_variables_when_you_select_a_stack_frame(
             running_state
                 .stack_frame_list()
                 .read(cx)
-                .selected_stack_frame_id(),
+                .opened_stack_frame_id(),
             Some(1)
         );
 
@@ -1778,7 +1778,7 @@ async fn test_it_fetches_scopes_variables_when_you_select_a_stack_frame(
             running_state
                 .stack_frame_list()
                 .update(cx, |stack_frame_list, cx| {
-                    stack_frame_list.select_stack_frame(&stack_frames[1], true, window, cx)
+                    stack_frame_list.go_to_stack_frame(stack_frames[1].id, window, cx)
                 })
         })
         .await
@@ -1789,7 +1789,7 @@ async fn test_it_fetches_scopes_variables_when_you_select_a_stack_frame(
     running_state.update(cx, |running_state, cx| {
         let (stack_frame_list, stack_frame_id) =
             running_state.stack_frame_list().update(cx, |list, _| {
-                (list.flatten_entries(true), list.selected_stack_frame_id())
+                (list.flatten_entries(true), list.opened_stack_frame_id())
             });
 
         let variable_list = running_state.variable_list().read(cx);