debugger: Move breakpoint management to the pane strip (#33062)

Closes #ISSUE

Release Notes:

- debugger: Moved "remove breakpoint" button to the top of a breakpoint
list"

crates/debugger_ui/src/debugger_panel.rs

@@ -1472,8 +1472,10 @@ impl Render for DebugPanel {
                                 h_flex().size_full()
                                     .items_start()
 
-                                    .child(v_flex().items_start().min_w_1_3().h_full().p_1()
+                                    .child(v_flex().group("base-breakpoint-list").items_start().min_w_1_3().h_full().p_1()
-                                        .child(h_flex().px_1().child(Label::new("Breakpoints").size(LabelSize::Small)))
+                                        .child(h_flex().pl_1().w_full().justify_between()
+                                            .child(Label::new("Breakpoints").size(LabelSize::Small))
+                                            .child(h_flex().visible_on_hover("base-breakpoint-list").child(self.breakpoint_list.read(cx).render_control_strip())))
                                         .child(Divider::horizontal())
                                         .child(self.breakpoint_list.clone()))
                                     .child(Divider::vertical())

crates/debugger_ui/src/persistence.rs

@@ -265,56 +265,37 @@ pub(crate) fn deserialize_pane_layout(
                         stack_frame_list.focus_handle(cx),
                         stack_frame_list.clone().into(),
                         DebuggerPaneItem::Frames,
-                        None,
                         cx,
                     )),
                     DebuggerPaneItem::Variables => Box::new(SubView::new(
                         variable_list.focus_handle(cx),
                         variable_list.clone().into(),
                         DebuggerPaneItem::Variables,
-                        None,
-                        cx,
-                    )),
-                    DebuggerPaneItem::BreakpointList => Box::new(SubView::new(
-                        breakpoint_list.focus_handle(cx),
-                        breakpoint_list.clone().into(),
-                        DebuggerPaneItem::BreakpointList,
-                        None,
                         cx,
                     )),
+                    DebuggerPaneItem::BreakpointList => {
+                        Box::new(SubView::breakpoint_list(breakpoint_list.clone(), cx))
+                    }
                     DebuggerPaneItem::Modules => Box::new(SubView::new(
                         module_list.focus_handle(cx),
                         module_list.clone().into(),
                         DebuggerPaneItem::Modules,
-                        None,
                         cx,
                     )),
                     DebuggerPaneItem::LoadedSources => Box::new(SubView::new(
                         loaded_sources.focus_handle(cx),
                         loaded_sources.clone().into(),
                         DebuggerPaneItem::LoadedSources,
-                        None,
                         cx,
                     )),
-                    DebuggerPaneItem::Console => Box::new(SubView::new(
+                    DebuggerPaneItem::Console => {
-                        console.focus_handle(cx),
+                        let view = SubView::console(console.clone(), cx);
-                        console.clone().into(),
+                        Box::new(view)
-                        DebuggerPaneItem::Console,
-                        Some(Box::new({
-                            let console = console.clone().downgrade();
-                            move |cx| {
-                                console
-                                    .read_with(cx, |console, cx| console.show_indicator(cx))
-                                    .unwrap_or_default()
                     }
-                        })),
-                        cx,
-                    )),
                     DebuggerPaneItem::Terminal => Box::new(SubView::new(
                         terminal.focus_handle(cx),
                         terminal.clone().into(),
                         DebuggerPaneItem::Terminal,
-                        None,
                         cx,
                     )),
                 })

crates/debugger_ui/src/session/running.rs

@@ -135,6 +135,7 @@ pub(crate) struct SubView {
     item_focus_handle: FocusHandle,
     kind: DebuggerPaneItem,
     show_indicator: Box<dyn Fn(&App) -> bool>,
+    actions: Option<Box<dyn FnMut(&mut Window, &mut App) -> AnyElement>>,
     hovered: bool,
 }
 
@@ -143,21 +144,68 @@ impl SubView {
         item_focus_handle: FocusHandle,
         view: AnyView,
         kind: DebuggerPaneItem,
-        show_indicator: Option<Box<dyn Fn(&App) -> bool>>,
         cx: &mut App,
     ) -> Entity<Self> {
         cx.new(|_| Self {
             kind,
             inner: view,
             item_focus_handle,
-            show_indicator: show_indicator.unwrap_or(Box::new(|_| false)),
+            show_indicator: Box::new(|_| false),
+            actions: None,
             hovered: false,
         })
     }
 
+    pub(crate) fn console(console: Entity<Console>, cx: &mut App) -> Entity<Self> {
+        let weak_console = console.downgrade();
+        let this = Self::new(
+            console.focus_handle(cx),
+            console.into(),
+            DebuggerPaneItem::Console,
+            cx,
+        );
+        this.update(cx, |this, _| {
+            this.with_indicator(Box::new(move |cx| {
+                weak_console
+                    .read_with(cx, |console, cx| console.show_indicator(cx))
+                    .unwrap_or_default()
+            }))
+        });
+        this
+    }
+
+    pub(crate) fn breakpoint_list(list: Entity<BreakpointList>, cx: &mut App) -> Entity<Self> {
+        let weak_list = list.downgrade();
+        let focus_handle = list.focus_handle(cx);
+        let this = Self::new(
+            focus_handle.clone(),
+            list.into(),
+            DebuggerPaneItem::BreakpointList,
+            cx,
+        );
+
+        this.update(cx, |this, _| {
+            this.with_actions(Box::new(move |_, cx| {
+                weak_list
+                    .update(cx, |this, _| this.render_control_strip())
+                    .unwrap_or_else(|_| div().into_any_element())
+            }));
+        });
+        this
+    }
+
     pub(crate) fn view_kind(&self) -> DebuggerPaneItem {
         self.kind
     }
+    pub(crate) fn with_indicator(&mut self, indicator: Box<dyn Fn(&App) -> bool>) {
+        self.show_indicator = indicator;
+    }
+    pub(crate) fn with_actions(
+        &mut self,
+        actions: Box<dyn FnMut(&mut Window, &mut App) -> AnyElement>,
+    ) {
+        self.actions = Some(actions);
+    }
 }
 impl Focusable for SubView {
     fn focus_handle(&self, _: &App) -> FocusHandle {
@@ -359,10 +407,13 @@ pub(crate) fn new_debugger_pane(
                 let active_pane_item = pane.active_item();
                 let pane_group_id: SharedString =
                     format!("pane-zoom-button-hover-{}", cx.entity_id()).into();
-                let is_hovered = active_pane_item.as_ref().map_or(false, |item| {
+                let as_subview = active_pane_item
-                    item.downcast::<SubView>()
+                    .as_ref()
-                        .map_or(false, |this| this.read(cx).hovered)
+                    .and_then(|item| item.downcast::<SubView>());
-                });
+                let is_hovered = as_subview
+                    .as_ref()
+                    .map_or(false, |item| item.read(cx).hovered);
+
                 h_flex()
                     .group(pane_group_id.clone())
                     .justify_between()
@@ -459,9 +510,17 @@ pub(crate) fn new_debugger_pane(
                     )
                     .child({
                         let zoomed = pane.is_zoomed();
-                        div()
+                        h_flex()
                             .visible_on_hover(pane_group_id)
                             .when(is_hovered, |this| this.visible())
+                            .when_some(as_subview.as_ref(), |this, subview| {
+                                subview.update(cx, |view, cx| {
+                                    let Some(additional_actions) = view.actions.as_mut() else {
+                                        return this;
+                                    };
+                                    this.child(additional_actions(window, cx))
+                                })
+                            })
                             .child(
                                 IconButton::new(
                                     SharedString::from(format!(
@@ -1095,61 +1154,38 @@ impl RunningState {
         cx: &mut Context<Self>,
     ) -> Box<dyn ItemHandle> {
         match item_kind {
-            DebuggerPaneItem::Console => {
+            DebuggerPaneItem::Console => Box::new(SubView::console(self.console.clone(), cx)),
-                let weak_console = self.console.clone().downgrade();
-
-                Box::new(SubView::new(
-                    self.console.focus_handle(cx),
-                    self.console.clone().into(),
-                    item_kind,
-                    Some(Box::new(move |cx| {
-                        weak_console
-                            .read_with(cx, |console, cx| console.show_indicator(cx))
-                            .unwrap_or_default()
-                    })),
-                    cx,
-                ))
-            }
             DebuggerPaneItem::Variables => Box::new(SubView::new(
                 self.variable_list.focus_handle(cx),
                 self.variable_list.clone().into(),
                 item_kind,
-                None,
-                cx,
-            )),
-            DebuggerPaneItem::BreakpointList => Box::new(SubView::new(
-                self.breakpoint_list.focus_handle(cx),
-                self.breakpoint_list.clone().into(),
-                item_kind,
-                None,
                 cx,
             )),
+            DebuggerPaneItem::BreakpointList => {
+                Box::new(SubView::breakpoint_list(self.breakpoint_list.clone(), cx))
+            }
             DebuggerPaneItem::Frames => Box::new(SubView::new(
                 self.stack_frame_list.focus_handle(cx),
                 self.stack_frame_list.clone().into(),
                 item_kind,
-                None,
                 cx,
             )),
             DebuggerPaneItem::Modules => Box::new(SubView::new(
                 self.module_list.focus_handle(cx),
                 self.module_list.clone().into(),
                 item_kind,
-                None,
                 cx,
             )),
             DebuggerPaneItem::LoadedSources => Box::new(SubView::new(
                 self.loaded_sources_list.focus_handle(cx),
                 self.loaded_sources_list.clone().into(),
                 item_kind,
-                None,
                 cx,
             )),
             DebuggerPaneItem::Terminal => Box::new(SubView::new(
                 self.debug_terminal.focus_handle(cx),
                 self.debug_terminal.clone().into(),
                 item_kind,
-                None,
                 cx,
             )),
         }
@@ -1558,7 +1594,6 @@ impl RunningState {
                     this.focus_handle(cx),
                     stack_frame_list.clone().into(),
                     DebuggerPaneItem::Frames,
-                    None,
                     cx,
                 )),
                 true,
@@ -1568,13 +1603,7 @@ impl RunningState {
                 cx,
             );
             this.add_item(
-                Box::new(SubView::new(
+                Box::new(SubView::breakpoint_list(breakpoints.clone(), cx)),
-                    breakpoints.focus_handle(cx),
-                    breakpoints.clone().into(),
-                    DebuggerPaneItem::BreakpointList,
-                    None,
-                    cx,
-                )),
                 true,
                 false,
                 None,
@@ -1586,32 +1615,15 @@ impl RunningState {
         let center_pane = new_debugger_pane(workspace.clone(), project.clone(), window, cx);
 
         center_pane.update(cx, |this, cx| {
-            let weak_console = console.downgrade();
+            let view = SubView::console(console.clone(), cx);
-            this.add_item(
+
-                Box::new(SubView::new(
+            this.add_item(Box::new(view), true, false, None, window, cx);
-                    console.focus_handle(cx),
-                    console.clone().into(),
-                    DebuggerPaneItem::Console,
-                    Some(Box::new(move |cx| {
-                        weak_console
-                            .read_with(cx, |console, cx| console.show_indicator(cx))
-                            .unwrap_or_default()
-                    })),
-                    cx,
-                )),
-                true,
-                false,
-                None,
-                window,
-                cx,
-            );
 
             this.add_item(
                 Box::new(SubView::new(
                     variable_list.focus_handle(cx),
                     variable_list.clone().into(),
                     DebuggerPaneItem::Variables,
-                    None,
                     cx,
                 )),
                 true,
@@ -1630,7 +1642,6 @@ impl RunningState {
                     debug_terminal.focus_handle(cx),
                     debug_terminal.clone().into(),
                     DebuggerPaneItem::Terminal,
-                    None,
                     cx,
                 )),
                 false,

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

@@ -8,8 +8,8 @@ use std::{
 use dap::ExceptionBreakpointsFilter;
 use editor::Editor;
 use gpui::{
-    AppContext, Entity, FocusHandle, Focusable, MouseButton, ScrollStrategy, Stateful, Task,
+    Action, AppContext, Entity, FocusHandle, Focusable, MouseButton, ScrollStrategy, Stateful,
-    UniformListScrollHandle, WeakEntity, uniform_list,
+    Task, UniformListScrollHandle, WeakEntity, uniform_list,
 };
 use language::Point;
 use project::{
@@ -21,15 +21,21 @@ use project::{
     worktree_store::WorktreeStore,
 };
 use ui::{
-    App, ButtonCommon, Clickable, Color, Context, Div, FluentBuilder as _, Icon, IconButton,
+    AnyElement, App, ButtonCommon, Clickable, Color, Context, Disableable, Div, FluentBuilder as _,
-    IconName, Indicator, InteractiveElement, IntoElement, Label, LabelCommon, LabelSize, ListItem,
+    Icon, IconButton, IconName, IconSize, Indicator, InteractiveElement, IntoElement, Label,
-    ParentElement, Render, Scrollbar, ScrollbarState, SharedString, StatefulInteractiveElement,
+    LabelCommon, LabelSize, ListItem, ParentElement, Render, Scrollbar, ScrollbarState,
-    Styled, Toggleable, Tooltip, Window, div, h_flex, px, v_flex,
+    SharedString, StatefulInteractiveElement, Styled, Toggleable, Tooltip, Window, div, h_flex, px,
+    v_flex,
 };
 use util::ResultExt;
 use workspace::Workspace;
 use zed_actions::{ToggleEnableBreakpoint, UnsetBreakpoint};
 
+#[derive(Clone, Copy, PartialEq)]
+pub(crate) enum SelectedBreakpointKind {
+    Source,
+    Exception,
+}
 pub(crate) struct BreakpointList {
     workspace: WeakEntity<Workspace>,
     breakpoint_store: Entity<BreakpointStore>,
@@ -127,6 +133,21 @@ impl BreakpointList {
         .detach();
     }
 
+    pub(crate) fn selection_kind(&self) -> Option<(SelectedBreakpointKind, bool)> {
+        self.selected_ix.and_then(|ix| {
+            self.breakpoints.get(ix).map(|bp| match &bp.kind {
+                BreakpointEntryKind::LineBreakpoint(bp) => (
+                    SelectedBreakpointKind::Source,
+                    bp.breakpoint.state
+                        == project::debugger::breakpoint_store::BreakpointState::Enabled,
+                ),
+                BreakpointEntryKind::ExceptionBreakpoint(bp) => {
+                    (SelectedBreakpointKind::Exception, bp.is_enabled)
+                }
+            })
+        })
+    }
+
     fn select_ix(&mut self, ix: Option<usize>, cx: &mut Context<Self>) {
         self.selected_ix = ix;
         if let Some(ix) = ix {
@@ -333,7 +354,93 @@ impl BreakpointList {
                 .children(Scrollbar::vertical(self.scrollbar_state.clone())),
         )
     }
+    pub(crate) fn render_control_strip(&self) -> AnyElement {
+        let selection_kind = self.selection_kind();
+        let focus_handle = self.focus_handle.clone();
+        let remove_breakpoint_tooltip = selection_kind.map(|(kind, _)| match kind {
+            SelectedBreakpointKind::Source => "Remove breakpoint from a breakpoint list",
+            SelectedBreakpointKind::Exception => {
+                "Exception Breakpoints cannot be removed from the breakpoint list"
+            }
+        });
+        let toggle_label = selection_kind.map(|(_, is_enabled)| {
+            if is_enabled {
+                (
+                    "Disable Breakpoint",
+                    "Disable a breakpoint without removing it from the list",
+                )
+            } else {
+                ("Enable Breakpoint", "Re-enable a breakpoint")
+            }
+        });
+
+        h_flex()
+            .gap_2()
+            .child(
+                IconButton::new(
+                    "disable-breakpoint-breakpoint-list",
+                    IconName::DebugDisabledBreakpoint,
+                )
+                .icon_size(IconSize::XSmall)
+                .when_some(toggle_label, |this, (label, meta)| {
+                    this.tooltip({
+                        let focus_handle = focus_handle.clone();
+                        move |window, cx| {
+                            Tooltip::with_meta_in(
+                                label,
+                                Some(&ToggleEnableBreakpoint),
+                                meta,
+                                &focus_handle,
+                                window,
+                                cx,
+                            )
+                        }
+                    })
+                })
+                .disabled(selection_kind.is_none())
+                .on_click({
+                    let focus_handle = focus_handle.clone();
+                    move |_, window, cx| {
+                        focus_handle.focus(window);
+                        window.dispatch_action(ToggleEnableBreakpoint.boxed_clone(), cx)
+                    }
+                }),
+            )
+            .child(
+                IconButton::new("remove-breakpoint-breakpoint-list", IconName::X)
+                    .icon_size(IconSize::XSmall)
+                    .icon_color(ui::Color::Error)
+                    .when_some(remove_breakpoint_tooltip, |this, tooltip| {
+                        this.tooltip({
+                            let focus_handle = focus_handle.clone();
+                            move |window, cx| {
+                                Tooltip::with_meta_in(
+                                    "Remove Breakpoint",
+                                    Some(&UnsetBreakpoint),
+                                    tooltip,
+                                    &focus_handle,
+                                    window,
+                                    cx,
+                                )
+                            }
+                        })
+                    })
+                    .disabled(
+                        selection_kind.map(|kind| kind.0) != Some(SelectedBreakpointKind::Source),
+                    )
+                    .on_click({
+                        let focus_handle = focus_handle.clone();
+                        move |_, window, cx| {
+                            focus_handle.focus(window);
+                            window.dispatch_action(UnsetBreakpoint.boxed_clone(), cx)
+                        }
+                    }),
+            )
+            .mr_2()
+            .into_any_element()
+    }
 }
+
 impl Render for BreakpointList {
     fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl ui::IntoElement {
         // let old_len = self.breakpoints.len();
@@ -505,44 +612,6 @@ impl LineBreakpoint {
         .on_secondary_mouse_down(|_, _, cx| {
             cx.stop_propagation();
         })
-        .end_hover_slot(
-            h_flex()
-                .child(
-                    IconButton::new(
-                        SharedString::from(format!(
-                            "breakpoint-ui-on-click-go-to-line-remove-{:?}/{}:{}",
-                            self.dir, self.name, self.line
-                        )),
-                        IconName::Close,
-                    )
-                    .on_click({
-                        let weak = weak.clone();
-                        let path = path.clone();
-                        move |_, _, cx| {
-                            weak.update(cx, |breakpoint_list, cx| {
-                                breakpoint_list.edit_line_breakpoint(
-                                    path.clone(),
-                                    row,
-                                    BreakpointEditAction::Toggle,
-                                    cx,
-                                );
-                            })
-                            .ok();
-                        }
-                    })
-                    .tooltip(move |window, cx| {
-                        Tooltip::for_action_in(
-                            "Unset Breakpoint",
-                            &UnsetBreakpoint,
-                            &focus_handle,
-                            window,
-                            cx,
-                        )
-                    })
-                    .icon_size(ui::IconSize::XSmall),
-                )
-                .right_4(),
-        )
         .child(
             v_flex()
                 .py_1()