Debugger: Fix breakpoint serialization (#27825)

This PR fixes two bugs that cause unexpected behavior with breakpoints.

The first bug made it impossible to delete the last breakpoint in a file
in the workspace's database. This caused deleted breakpoints to remain
in the database and added to new projects.

The second bug was an edge case in the breakpoint context menu where
disabling/enabling a breakpoint would sometimes set a new breakpoint on
top of the old breakpoint.


Release Notes:

- N/A

crates/editor/src/editor.rs

@@ -6249,7 +6249,7 @@ impl Editor {
     /// It's also used to set the color of line numbers with breakpoints to the breakpoint color.
     /// TODO debugger: Use this function to color toggle symbols that house nested breakpoints
     fn active_breakpoints(
-        &mut self,
+        &self,
         range: Range<DisplayRow>,
         window: &mut Window,
         cx: &mut Context<Self>,
@@ -6320,24 +6320,26 @@ impl Editor {
 
         let breakpoint = self
             .breakpoint_at_row(row, window, cx)
-            .map(|(_, bp)| Arc::from(bp));
+            .map(|(anchor, bp)| (anchor, Arc::from(bp)));
 
-        let log_breakpoint_msg = if breakpoint.as_ref().is_some_and(|bp| bp.message.is_some()) {
+        let log_breakpoint_msg = if breakpoint.as_ref().is_some_and(|bp| bp.1.message.is_some()) {
             "Edit Log Breakpoint"
         } else {
             "Set Log Breakpoint"
         };
 
-        let condition_breakpoint_msg =
-            if breakpoint.as_ref().is_some_and(|bp| bp.condition.is_some()) {
-                "Edit Condition Breakpoint"
-            } else {
-                "Set Condition Breakpoint"
-            };
+        let condition_breakpoint_msg = if breakpoint
+            .as_ref()
+            .is_some_and(|bp| bp.1.condition.is_some())
+        {
+            "Edit Condition Breakpoint"
+        } else {
+            "Set Condition Breakpoint"
+        };
 
         let hit_condition_breakpoint_msg = if breakpoint
             .as_ref()
-            .is_some_and(|bp| bp.hit_condition.is_some())
+            .is_some_and(|bp| bp.1.hit_condition.is_some())
         {
             "Edit Hit Condition Breakpoint"
         } else {
@@ -6350,12 +6352,13 @@ impl Editor {
             "Set Breakpoint"
         };
 
-        let toggle_state_msg = breakpoint.as_ref().map_or(None, |bp| match bp.state {
+        let toggle_state_msg = breakpoint.as_ref().map_or(None, |bp| match bp.1.state {
             BreakpointState::Enabled => Some("Disable"),
             BreakpointState::Disabled => Some("Enable"),
         });
 
-        let breakpoint = breakpoint.unwrap_or_else(|| Arc::new(Breakpoint::new_standard()));
+        let (anchor, breakpoint) =
+            breakpoint.unwrap_or_else(|| (anchor, Arc::new(Breakpoint::new_standard())));
 
         ui::ContextMenu::build(window, cx, |menu, _, _cx| {
             menu.on_blur_subscription(Subscription::new(|| {}))

crates/project/src/debugger/breakpoint_store.rs

@@ -566,7 +566,20 @@ impl BreakpointStore {
                     new_breakpoints.insert(path, breakpoints_for_file);
                 }
                 this.update(cx, |this, cx| {
+                    log::info!("Finish deserializing breakpoints & initializing breakpoint store");
+                    for (path, count) in new_breakpoints.iter().map(|(path, bp_in_file)| {
+                        (path.to_string_lossy(), bp_in_file.breakpoints.len())
+                    }) {
+                        let breakpoint_str = if count > 1 {
+                            "breakpoints"
+                        } else {
+                            "breakpoint"
+                        };
+                        log::info!("Deserialized {count} {breakpoint_str} at path: {path}");
+                    }
+
                     this.breakpoints = new_breakpoints;
+
                     cx.notify();
                 })?;
 

crates/workspace/src/persistence.rs

@@ -720,8 +720,8 @@ impl WorkspaceDb {
                 }
 
                 for (path, bps) in map.iter() {
-                    log::debug!(
-                        "Got {} breakpoints from path: {}",
+                    log::info!(
+                        "Got {} breakpoints from database at path: {}",
                         bps.len(),
                         path.to_string_lossy()
                     );
@@ -746,9 +746,10 @@ impl WorkspaceDb {
                     DELETE FROM pane_groups WHERE workspace_id = ?1;
                     DELETE FROM panes WHERE workspace_id = ?1;))?(workspace.id)
                 .context("Clearing old panes")?;
+
+                conn.exec_bound(sql!(DELETE FROM breakpoints WHERE workspace_id = ?1))?(workspace.id).context("Clearing old breakpoints")?;
+
                 for (path, breakpoints) in workspace.breakpoints {
-                    conn.exec_bound(sql!(DELETE FROM breakpoints WHERE workspace_id = ?1 AND path = ?2))?((workspace.id, path.as_ref()))
-                    .context("Clearing old breakpoints")?;
                     for bp in breakpoints {
                         let state = BreakpointStateWrapper::from(bp.state);
                         match conn.exec_bound(sql!(
@@ -1607,6 +1608,93 @@ mod tests {
         assert_eq!(loaded_breakpoints[4].path, Arc::from(path));
     }
 
+    #[gpui::test]
+    async fn test_remove_last_breakpoint() {
+        env_logger::try_init().ok();
+
+        let db = WorkspaceDb(open_test_db("test_remove_last_breakpoint").await);
+        let id = db.next_id().await.unwrap();
+
+        let singular_path = Path::new("/tmp/test_remove_last_breakpoint.rs");
+
+        let breakpoint_to_remove = Breakpoint {
+            position: 100,
+            message: None,
+            state: BreakpointState::Enabled,
+            condition: None,
+            hit_condition: None,
+        };
+
+        let workspace = SerializedWorkspace {
+            id,
+            location: SerializedWorkspaceLocation::from_local_paths(["/tmp"]),
+            center_group: Default::default(),
+            window_bounds: Default::default(),
+            display: Default::default(),
+            docks: Default::default(),
+            centered_layout: false,
+            breakpoints: {
+                let mut map = collections::BTreeMap::default();
+                map.insert(
+                    Arc::from(singular_path),
+                    vec![SourceBreakpoint {
+                        row: breakpoint_to_remove.position,
+                        path: Arc::from(singular_path),
+                        message: None,
+                        state: BreakpointState::Enabled,
+                        condition: None,
+                        hit_condition: None,
+                    }],
+                );
+                map
+            },
+            session_id: None,
+            window_id: None,
+        };
+
+        db.save_workspace(workspace.clone()).await;
+
+        let loaded = db.workspace_for_roots(&["/tmp"]).unwrap();
+        let loaded_breakpoints = loaded.breakpoints.get(&Arc::from(singular_path)).unwrap();
+
+        assert_eq!(loaded_breakpoints.len(), 1);
+        assert_eq!(loaded_breakpoints[0].row, breakpoint_to_remove.position);
+        assert_eq!(loaded_breakpoints[0].message, breakpoint_to_remove.message);
+        assert_eq!(
+            loaded_breakpoints[0].condition,
+            breakpoint_to_remove.condition
+        );
+        assert_eq!(
+            loaded_breakpoints[0].hit_condition,
+            breakpoint_to_remove.hit_condition
+        );
+        assert_eq!(loaded_breakpoints[0].state, breakpoint_to_remove.state);
+        assert_eq!(loaded_breakpoints[0].path, Arc::from(singular_path));
+
+        let workspace_without_breakpoint = SerializedWorkspace {
+            id,
+            location: SerializedWorkspaceLocation::from_local_paths(["/tmp"]),
+            center_group: Default::default(),
+            window_bounds: Default::default(),
+            display: Default::default(),
+            docks: Default::default(),
+            centered_layout: false,
+            breakpoints: collections::BTreeMap::default(),
+            session_id: None,
+            window_id: None,
+        };
+
+        db.save_workspace(workspace_without_breakpoint.clone())
+            .await;
+
+        let loaded_after_remove = db.workspace_for_roots(&["/tmp"]).unwrap();
+        let empty_breakpoints = loaded_after_remove
+            .breakpoints
+            .get(&Arc::from(singular_path));
+
+        assert!(empty_breakpoints.is_none());
+    }
+
     #[gpui::test]
     async fn test_next_id_stability() {
         env_logger::try_init().ok();

crates/workspace/src/workspace.rs

@@ -988,10 +988,12 @@ impl Workspace {
         cx.subscribe_in(
             &project.read(cx).breakpoint_store(),
             window,
-            |workspace, _, evt, window, cx| {
-                if let BreakpointStoreEvent::BreakpointsUpdated(_, _) = evt {
+            |workspace, _, event, window, cx| match event {
+                BreakpointStoreEvent::BreakpointsUpdated(_, _)
+                | BreakpointStoreEvent::BreakpointsCleared(_) => {
                     workspace.serialize_workspace(window, cx);
                 }
+                BreakpointStoreEvent::ActiveDebugLineChanged => {}
             },
         )
         .detach();