Clear breakpoints action (#27254)

Anthony Eid and Piotr Osiewicz created

This PR adds an action that clears all breakpoints and notifies any
active DAPs.

todo
- [x] Implement clear functionality
- [x] Write an integration test for this

Release Notes:

- N/A *or* Added/Fixed/Improved ...

---------

Co-authored-by: Piotr Osiewicz <peterosiewicz@gmail.com>

Change summary

crates/debugger_ui/src/debugger_panel.rs        |  13 +
crates/debugger_ui/src/tests/debugger_panel.rs  | 133 +++++++++++++++++++
crates/project/src/debugger/breakpoint_store.rs |  16 ++
crates/project/src/debugger/session.rs          |  49 +++++++
crates/workspace/src/workspace.rs               |   3 
5 files changed, 209 insertions(+), 5 deletions(-)

Detailed changes

crates/debugger_ui/src/debugger_panel.rs 🔗

@@ -24,8 +24,8 @@ use ui::prelude::*;
 use util::ResultExt;
 use workspace::{
     dock::{DockPosition, Panel, PanelEvent},
-    pane, Continue, Disconnect, Pane, Pause, Restart, StepBack, StepInto, StepOut, StepOver, Stop,
-    ToggleIgnoreBreakpoints, Workspace,
+    pane, ClearBreakpoints, Continue, Disconnect, Pane, Pause, Restart, StepBack, StepInto,
+    StepOut, StepOver, Stop, ToggleIgnoreBreakpoints, Workspace,
 };
 
 pub enum DebugPanelEvent {
@@ -174,6 +174,15 @@ impl DebugPanel {
             workspace.update_in(cx, |workspace, window, cx| {
                 let debug_panel = DebugPanel::new(workspace, window, cx);
 
+                workspace.register_action(|workspace, _: &ClearBreakpoints, _, cx| {
+                    workspace.project().read(cx).breakpoint_store().update(
+                        cx,
+                        |breakpoint_store, cx| {
+                            breakpoint_store.clear_breakpoints(cx);
+                        },
+                    )
+                });
+
                 cx.observe(&debug_panel, |_, debug_panel, cx| {
                     let (has_active_session, supports_restart, support_step_back) = debug_panel
                         .update(cx, |this, cx| {

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

@@ -1264,6 +1264,139 @@ async fn test_send_breakpoints_when_editor_has_been_saved(
     shutdown_session.await.unwrap();
 }
 
+#[gpui::test]
+async fn test_unsetting_breakpoints_on_clear_breakpoint_action(
+    executor: BackgroundExecutor,
+    cx: &mut TestAppContext,
+) {
+    init_test(cx);
+
+    let fs = FakeFs::new(executor.clone());
+
+    fs.insert_tree(
+        path!("/project"),
+        json!({
+            "main.rs": "First line\nSecond line\nThird line\nFourth line",
+            "second.rs": "First line\nSecond line\nThird line\nFourth line",
+            "no_breakpoints.rs": "Used to ensure that we don't unset breakpoint in files with no breakpoints"
+        }),
+    )
+    .await;
+
+    let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
+    let workspace = init_test_workspace(&project, cx).await;
+    let cx = &mut VisualTestContext::from_window(*workspace, cx);
+    let project_path = Path::new(path!("/project"));
+    let worktree = project
+        .update(cx, |project, cx| project.find_worktree(project_path, cx))
+        .expect("This worktree should exist in project")
+        .0;
+
+    let worktree_id = workspace
+        .update(cx, |_, _, cx| worktree.read(cx).id())
+        .unwrap();
+
+    let first = project
+        .update(cx, |project, cx| {
+            project.open_buffer((worktree_id, "main.rs"), cx)
+        })
+        .await
+        .unwrap();
+
+    let second = project
+        .update(cx, |project, cx| {
+            project.open_buffer((worktree_id, "second.rs"), cx)
+        })
+        .await
+        .unwrap();
+
+    let (first_editor, cx) = cx.add_window_view(|window, cx| {
+        Editor::new(
+            EditorMode::Full,
+            MultiBuffer::build_from_buffer(first, cx),
+            Some(project.clone()),
+            window,
+            cx,
+        )
+    });
+
+    let (second_editor, cx) = cx.add_window_view(|window, cx| {
+        Editor::new(
+            EditorMode::Full,
+            MultiBuffer::build_from_buffer(second, cx),
+            Some(project.clone()),
+            window,
+            cx,
+        )
+    });
+
+    first_editor.update_in(cx, |editor, window, cx| {
+        editor.move_down(&actions::MoveDown, window, cx);
+        editor.toggle_breakpoint(&actions::ToggleBreakpoint, window, cx);
+        editor.move_down(&actions::MoveDown, window, cx);
+        editor.move_down(&actions::MoveDown, window, cx);
+        editor.toggle_breakpoint(&actions::ToggleBreakpoint, window, cx);
+    });
+
+    second_editor.update_in(cx, |editor, window, cx| {
+        editor.toggle_breakpoint(&actions::ToggleBreakpoint, window, cx);
+        editor.move_down(&actions::MoveDown, window, cx);
+        editor.move_down(&actions::MoveDown, window, cx);
+        editor.move_down(&actions::MoveDown, window, cx);
+        editor.toggle_breakpoint(&actions::ToggleBreakpoint, window, cx);
+    });
+
+    let task = project.update(cx, |project, cx| {
+        project.start_debug_session(dap::test_config(DebugRequestType::Launch, None, None), cx)
+    });
+
+    let session = task.await.unwrap();
+    let client = session.update(cx, |session, _| session.adapter_client().unwrap());
+
+    let called_set_breakpoints = Arc::new(AtomicBool::new(false));
+
+    client
+        .on_request::<SetBreakpoints, _>({
+            let called_set_breakpoints = called_set_breakpoints.clone();
+            move |_, args| {
+                assert!(
+                    args.breakpoints.is_none_or(|bps| bps.is_empty()),
+                    "Send empty breakpoint sets to clear them from DAP servers"
+                );
+
+                match args
+                    .source
+                    .path
+                    .expect("We should always send a breakpoint's path")
+                    .as_str()
+                {
+                    "/project/main.rs" | "/project/second.rs" => {}
+                    _ => {
+                        panic!("Unset breakpoints for path that doesn't have any")
+                    }
+                }
+
+                called_set_breakpoints.store(true, Ordering::SeqCst);
+
+                Ok(dap::SetBreakpointsResponse {
+                    breakpoints: Vec::default(),
+                })
+            }
+        })
+        .await;
+
+    cx.dispatch_action(workspace::ClearBreakpoints);
+    cx.run_until_parked();
+
+    let shutdown_session = project.update(cx, |project, cx| {
+        project.dap_store().update(cx, |dap_store, cx| {
+            dap_store.shutdown_session(session.read(cx).session_id(), cx)
+        })
+    });
+
+    shutdown_session.await.unwrap();
+}
+
 #[gpui::test]
 async fn test_debug_session_is_shutdown_when_attach_and_launch_request_fails(
     executor: BackgroundExecutor,

crates/project/src/debugger/breakpoint_store.rs 🔗

@@ -5,7 +5,7 @@ use anyhow::{anyhow, Result};
 use breakpoints_in_file::BreakpointsInFile;
 use collections::BTreeMap;
 use dap::client::SessionId;
-use gpui::{App, AppContext, AsyncApp, Context, Entity, EventEmitter, Task};
+use gpui::{App, AppContext, AsyncApp, Context, Entity, EventEmitter, Subscription, Task};
 use language::{proto::serialize_anchor as serialize_text_anchor, Buffer, BufferSnapshot};
 use rpc::{
     proto::{self},
@@ -31,7 +31,7 @@ mod breakpoints_in_file {
         pub(super) buffer: Entity<Buffer>,
         // TODO: This is.. less than ideal, as it's O(n) and does not return entries in order. We'll have to change TreeMap to support passing in the context for comparisons
         pub(super) breakpoints: Vec<(text::Anchor, Breakpoint)>,
-        _subscription: Arc<gpui::Subscription>,
+        _subscription: Arc<Subscription>,
     }
 
     impl BreakpointsInFile {
@@ -341,6 +341,12 @@ impl BreakpointStore {
         }
     }
 
+    pub fn clear_breakpoints(&mut self, cx: &mut Context<Self>) {
+        let breakpoint_paths = self.breakpoints.keys().cloned().collect();
+        self.breakpoints.clear();
+        cx.emit(BreakpointStoreEvent::BreakpointsCleared(breakpoint_paths));
+    }
+
     pub fn breakpoints<'a>(
         &'a self,
         buffer: &'a Entity<Buffer>,
@@ -498,6 +504,11 @@ impl BreakpointStore {
             Task::ready(Ok(()))
         }
     }
+
+    #[cfg(any(test, feature = "test-support"))]
+    pub(crate) fn breakpoint_paths(&self) -> Vec<Arc<Path>> {
+        self.breakpoints.keys().cloned().collect()
+    }
 }
 
 #[derive(Clone, Copy)]
@@ -509,6 +520,7 @@ pub enum BreakpointUpdatedReason {
 pub enum BreakpointStoreEvent {
     ActiveDebugLineChanged,
     BreakpointsUpdated(Arc<Path>, BreakpointUpdatedReason),
+    BreakpointsCleared(Vec<Arc<Path>>),
 }
 
 impl EventEmitter<BreakpointStoreEvent> for BreakpointStore {}

crates/project/src/debugger/session.rs 🔗

@@ -237,6 +237,19 @@ impl LocalMode {
                     .on_request::<dap::requests::Initialize, _>(move |_, _| Ok(caps.clone()))
                     .await;
 
+                let paths = cx.update(|cx| session.breakpoint_store.read(cx).breakpoint_paths()).expect("Breakpoint store should exist in all tests that start debuggers");
+
+                session.client.on_request::<dap::requests::SetBreakpoints, _>(move |_, args| {
+                    let p = Arc::from(Path::new(&args.source.path.unwrap()));
+                    if !paths.contains(&p) {
+                        panic!("Sent breakpoints for path without any")
+                    }
+
+                    Ok(dap::SetBreakpointsResponse {
+                        breakpoints: Vec::default(),
+                    })
+                }).await;
+
                 match config.request.clone() {
                     dap::DebugRequestType::Launch if fail => {
                         session
@@ -307,6 +320,34 @@ impl LocalMode {
         })
     }
 
+    fn unset_breakpoints_from_paths(&self, paths: &Vec<Arc<Path>>, cx: &mut App) -> Task<()> {
+        let tasks: Vec<_> = paths
+            .into_iter()
+            .map(|path| {
+                self.request(
+                    dap_command::SetBreakpoints {
+                        source: client_source(path),
+                        source_modified: None,
+                        breakpoints: vec![],
+                    },
+                    cx.background_executor().clone(),
+                )
+            })
+            .collect();
+
+        cx.background_spawn(async move {
+            futures::future::join_all(tasks)
+                .await
+                .iter()
+                .for_each(|res| match res {
+                    Ok(_) => {}
+                    Err(err) => {
+                        log::warn!("Set breakpoints request failed: {}", err);
+                    }
+                });
+        })
+    }
+
     fn send_breakpoints_from_path(
         &self,
         abs_path: Arc<Path>,
@@ -752,6 +793,14 @@ impl Session {
                                 .detach();
                         };
                     }
+                    BreakpointStoreEvent::BreakpointsCleared(paths) => {
+                        if let Some(local) = (!this.ignore_breakpoints)
+                            .then(|| this.as_local_mut())
+                            .flatten()
+                        {
+                            local.unset_breakpoints_from_paths(paths, cx).detach();
+                        }
+                    }
                     BreakpointStoreEvent::ActiveDebugLineChanged => {}
                 })
                 .detach();