debugger: Touchups to log breakpoints (#27675)

Piotr Osiewicz created

This is a slight refactor that flattens Breakpoint struct in
anticipation of condition/hit breakpoints. It also adds a slight delay
before breakpoints are shown on gutter hover to make breakpoints less
attention grabbing.
Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs                     |  34 ++--
crates/editor/src/editor_tests.rs               |  16 +-
crates/editor/src/element.rs                    |  40 +++++
crates/project/src/debugger/breakpoint_store.rs | 119 ++++------------
crates/project/src/debugger/dap_store.rs        |  60 --------
crates/proto/proto/zed.proto                    |   8 -
crates/workspace/src/persistence.rs             | 131 +++++-------------
crates/workspace/src/persistence/model.rs       |   4 
8 files changed, 136 insertions(+), 276 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -147,7 +147,7 @@ use multi_buffer::{
 };
 use parking_lot::Mutex;
 use project::{
-    debugger::breakpoint_store::{Breakpoint, BreakpointKind},
+    debugger::breakpoint_store::Breakpoint,
     lsp_store::{CompletionDocumentation, FormatTrigger, LspFormatTarget, OpenLspBufferHandle},
     project_settings::{GitGutterSetting, ProjectSettings},
     CodeAction, Completion, CompletionIntent, CompletionSource, DocumentHighlight, InlayHint,
@@ -785,11 +785,11 @@ pub struct Editor {
     expect_bounds_change: Option<Bounds<Pixels>>,
     tasks: BTreeMap<(BufferId, BufferRow), RunnableTasks>,
     tasks_update_task: Option<Task<()>>,
-    pub breakpoint_store: Option<Entity<BreakpointStore>>,
+    breakpoint_store: Option<Entity<BreakpointStore>>,
     /// Allow's a user to create a breakpoint by selecting this indicator
     /// It should be None while a user is not hovering over the gutter
     /// Otherwise it represents the point that the breakpoint will be shown
-    pub gutter_breakpoint_indicator: Option<DisplayPoint>,
+    gutter_breakpoint_indicator: (Option<(DisplayPoint, bool)>, Option<Task<()>>),
     in_project_search: bool,
     previous_search_ranges: Option<Arc<[Range<Anchor>]>>,
     breadcrumb_header: Option<String>,
@@ -1549,7 +1549,7 @@ impl Editor {
             tasks: Default::default(),
 
             breakpoint_store,
-            gutter_breakpoint_indicator: None,
+            gutter_breakpoint_indicator: (None, None),
             _subscriptions: vec![
                 cx.observe(&buffer, Self::on_buffer_changed),
                 cx.subscribe_in(&buffer, window, Self::on_buffer_event),
@@ -6226,10 +6226,7 @@ impl Editor {
             .breakpoint_at_row(row, window, cx)
             .map(|(_, bp)| Arc::from(bp));
 
-        let log_breakpoint_msg = if breakpoint
-            .as_ref()
-            .is_some_and(|bp| bp.kind.log_message().is_some())
-        {
+        let log_breakpoint_msg = if breakpoint.as_ref().is_some_and(|bp| bp.message.is_some()) {
             "Edit Log Breakpoint"
         } else {
             "Set Log Breakpoint"
@@ -6249,7 +6246,7 @@ impl Editor {
         let breakpoint = breakpoint.unwrap_or_else(|| {
             Arc::new(Breakpoint {
                 state: BreakpointState::Enabled,
-                kind: BreakpointKind::Standard,
+                message: None,
             })
         });
 
@@ -6308,16 +6305,17 @@ impl Editor {
         cx: &mut Context<Self>,
     ) -> IconButton {
         let (color, icon) = {
-            let icon = match (&breakpoint.kind, breakpoint.is_disabled()) {
-                (BreakpointKind::Standard, false) => ui::IconName::DebugBreakpoint,
-                (BreakpointKind::Log(_), false) => ui::IconName::DebugLogBreakpoint,
-                (BreakpointKind::Standard, true) => ui::IconName::DebugDisabledBreakpoint,
-                (BreakpointKind::Log(_), true) => ui::IconName::DebugDisabledLogBreakpoint,
+            let icon = match (&breakpoint.message.is_some(), breakpoint.is_disabled()) {
+                (false, false) => ui::IconName::DebugBreakpoint,
+                (true, false) => ui::IconName::DebugLogBreakpoint,
+                (false, true) => ui::IconName::DebugDisabledBreakpoint,
+                (true, true) => ui::IconName::DebugDisabledLogBreakpoint,
             };
 
             let color = if self
                 .gutter_breakpoint_indicator
-                .is_some_and(|point| point.row() == row)
+                .0
+                .is_some_and(|(point, is_visible)| is_visible && point.row() == row)
             {
                 Color::Hint
             } else {
@@ -8654,7 +8652,7 @@ impl Editor {
                 (
                     breakpoint_position,
                     Breakpoint {
-                        kind: BreakpointKind::Standard,
+                        message: None,
                         state: BreakpointState::Enabled,
                     },
                 )
@@ -19758,8 +19756,8 @@ impl BreakpointPromptEditor {
         let buffer = cx.new(|cx| {
             Buffer::local(
                 breakpoint
-                    .kind
-                    .log_message()
+                    .message
+                    .as_ref()
                     .map(|msg| msg.to_string())
                     .unwrap_or_default(),
                 cx,

crates/editor/src/editor_tests.rs 🔗

@@ -32,7 +32,7 @@ use multi_buffer::{IndentGuide, PathKey};
 use parking_lot::Mutex;
 use pretty_assertions::{assert_eq, assert_ne};
 use project::{
-    debugger::breakpoint_store::{BreakpointKind, BreakpointState, SerializedBreakpoint},
+    debugger::breakpoint_store::{BreakpointState, SourceBreakpoint},
     project_settings::{LspSettings, ProjectSettings},
     FakeFs,
 };
@@ -17390,12 +17390,12 @@ async fn assert_highlighted_edits(
 
 #[track_caller]
 fn assert_breakpoint(
-    breakpoints: &BTreeMap<Arc<Path>, Vec<SerializedBreakpoint>>,
+    breakpoints: &BTreeMap<Arc<Path>, Vec<SourceBreakpoint>>,
     path: &Arc<Path>,
     expected: Vec<(u32, Breakpoint)>,
 ) {
     if expected.len() == 0usize {
-        assert!(!breakpoints.contains_key(path));
+        assert!(!breakpoints.contains_key(path), "{}", path.display());
     } else {
         let mut breakpoint = breakpoints
             .get(path)
@@ -17403,9 +17403,9 @@ fn assert_breakpoint(
             .into_iter()
             .map(|breakpoint| {
                 (
-                    breakpoint.position,
+                    breakpoint.row,
                     Breakpoint {
-                        kind: breakpoint.kind.clone(),
+                        message: breakpoint.message.clone(),
                         state: breakpoint.state,
                     },
                 )
@@ -17435,12 +17435,10 @@ fn add_log_breakpoint_at_cursor(
                 .buffer_snapshot
                 .anchor_before(Point::new(cursor_position.row, 0));
 
-            let kind = BreakpointKind::Log(Arc::from(log_message));
-
             (
                 breakpoint_position,
                 Breakpoint {
-                    kind,
+                    message: Some(Arc::from(log_message)),
                     state: BreakpointState::Enabled,
                 },
             )
@@ -17738,7 +17736,7 @@ async fn test_log_breakpoint_editing(cx: &mut TestAppContext) {
         &abs_path,
         vec![
             (0, Breakpoint::new_standard()),
-            (3, Breakpoint::new_log("hello Earth !!")),
+            (3, Breakpoint::new_log("hello Earth!!")),
         ],
     );
 }

crates/editor/src/element.rs 🔗

@@ -71,6 +71,7 @@ use std::{
     ops::{Deref, Range},
     rc::Rc,
     sync::Arc,
+    time::Duration,
 };
 use sum_tree::Bias;
 use text::BufferId;
@@ -907,7 +908,6 @@ impl EditorElement {
         let modifiers = event.modifiers;
         let gutter_hovered = gutter_hitbox.is_hovered(window);
         editor.set_gutter_hovered(gutter_hovered, cx);
-        editor.gutter_breakpoint_indicator = None;
         editor.mouse_cursor_hidden = false;
 
         if gutter_hovered {
@@ -924,8 +924,38 @@ impl EditorElement {
                 .buffer_for_excerpt(buffer_anchor.excerpt_id)
                 .is_some_and(|buffer| buffer.file().is_some())
             {
-                editor.gutter_breakpoint_indicator = Some(new_point);
+                let was_hovered = editor.gutter_breakpoint_indicator.0.is_some();
+                let is_visible = editor
+                    .gutter_breakpoint_indicator
+                    .0
+                    .map_or(false, |(_, is_active)| is_active);
+                editor.gutter_breakpoint_indicator.0 = Some((new_point, is_visible));
+
+                editor.gutter_breakpoint_indicator.1.get_or_insert_with(|| {
+                    cx.spawn(async move |this, cx| {
+                        if !was_hovered {
+                            cx.background_executor()
+                                .timer(Duration::from_millis(200))
+                                .await;
+                        }
+
+                        this.update(cx, |this, cx| {
+                            if let Some((_, is_active)) =
+                                this.gutter_breakpoint_indicator.0.as_mut()
+                            {
+                                *is_active = true;
+                            }
+
+                            cx.notify();
+                        })
+                        .ok();
+                    })
+                });
+            } else {
+                editor.gutter_breakpoint_indicator = (None, None);
             }
+        } else {
+            editor.gutter_breakpoint_indicator = (None, None);
         }
 
         cx.notify();
@@ -6851,8 +6881,10 @@ impl Element for EditorElement {
                     // has their mouse over that line when a breakpoint isn't there
                     if cx.has_flag::<Debugger>() {
                         let gutter_breakpoint_indicator =
-                            self.editor.read(cx).gutter_breakpoint_indicator;
-                        if let Some(gutter_breakpoint_point) = gutter_breakpoint_indicator {
+                            self.editor.read(cx).gutter_breakpoint_indicator.0;
+                        if let Some((gutter_breakpoint_point, _)) =
+                            gutter_breakpoint_indicator.filter(|(_, is_active)| *is_active)
+                        {
                             breakpoint_rows
                                 .entry(gutter_breakpoint_point.row())
                                 .or_insert_with(|| {

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

@@ -11,12 +11,7 @@ use rpc::{
     proto::{self},
     AnyProtoClient, TypedEnvelope,
 };
-use std::{
-    hash::{Hash, Hasher},
-    ops::Range,
-    path::Path,
-    sync::Arc,
-};
+use std::{hash::Hash, ops::Range, path::Path, sync::Arc};
 use text::{Point, PointUtf16};
 
 use crate::{buffer_store::BufferStore, worktree_store::WorktreeStore, Project, ProjectPath};
@@ -274,8 +269,6 @@ impl BreakpointStore {
             }
             BreakpointEditAction::EditLogMessage(log_message) => {
                 if !log_message.is_empty() {
-                    breakpoint.1.kind = BreakpointKind::Log(log_message.clone());
-
                     let found_bp =
                         breakpoint_set
                             .breakpoints
@@ -289,18 +282,16 @@ impl BreakpointStore {
                             });
 
                     if let Some(found_bp) = found_bp {
-                        found_bp.kind = BreakpointKind::Log(log_message.clone());
+                        found_bp.message = Some(log_message.clone());
                     } else {
+                        breakpoint.1.message = Some(log_message.clone());
                         // We did not remove any breakpoint, hence let's toggle one.
                         breakpoint_set.breakpoints.push(breakpoint.clone());
                     }
-                } else if matches!(&breakpoint.1.kind, BreakpointKind::Log(_)) {
-                    breakpoint_set
-                        .breakpoints
-                        .retain(|(other_pos, other_kind)| {
-                            &breakpoint.0 != other_pos
-                                && matches!(other_kind.kind, BreakpointKind::Standard)
-                        });
+                } else if breakpoint.1.message.is_some() {
+                    breakpoint_set.breakpoints.retain(|(other_pos, other)| {
+                        &breakpoint.0 != other_pos && other.message.is_none()
+                    })
                 }
             }
         }
@@ -419,7 +410,7 @@ impl BreakpointStore {
         cx.notify();
     }
 
-    pub fn breakpoints_from_path(&self, path: &Arc<Path>, cx: &App) -> Vec<SerializedBreakpoint> {
+    pub fn breakpoints_from_path(&self, path: &Arc<Path>, cx: &App) -> Vec<SourceBreakpoint> {
         self.breakpoints
             .get(path)
             .map(|bp| {
@@ -428,11 +419,11 @@ impl BreakpointStore {
                     .iter()
                     .map(|(position, breakpoint)| {
                         let position = snapshot.summary_for_anchor::<PointUtf16>(position).row;
-                        SerializedBreakpoint {
-                            position,
+                        SourceBreakpoint {
+                            row: position,
                             path: path.clone(),
-                            kind: breakpoint.kind.clone(),
                             state: breakpoint.state,
+                            message: breakpoint.message.clone(),
                         }
                     })
                     .collect()
@@ -440,7 +431,7 @@ impl BreakpointStore {
             .unwrap_or_default()
     }
 
-    pub fn all_breakpoints(&self, cx: &App) -> BTreeMap<Arc<Path>, Vec<SerializedBreakpoint>> {
+    pub fn all_breakpoints(&self, cx: &App) -> BTreeMap<Arc<Path>, Vec<SourceBreakpoint>> {
         self.breakpoints
             .iter()
             .map(|(path, bp)| {
@@ -451,10 +442,10 @@ impl BreakpointStore {
                         .iter()
                         .map(|(position, breakpoint)| {
                             let position = snapshot.summary_for_anchor::<PointUtf16>(position).row;
-                            SerializedBreakpoint {
-                                position,
+                            SourceBreakpoint {
+                                row: position,
                                 path: path.clone(),
-                                kind: breakpoint.kind.clone(),
+                                message: breakpoint.message.clone(),
                                 state: breakpoint.state,
                             }
                         })
@@ -466,7 +457,7 @@ impl BreakpointStore {
 
     pub fn with_serialized_breakpoints(
         &self,
-        breakpoints: BTreeMap<Arc<Path>, Vec<SerializedBreakpoint>>,
+        breakpoints: BTreeMap<Arc<Path>, Vec<SourceBreakpoint>>,
         cx: &mut Context<BreakpointStore>,
     ) -> Task<Result<()>> {
         if let BreakpointStoreMode::Local(mode) = &self.mode {
@@ -503,11 +494,11 @@ impl BreakpointStore {
                         this.update(cx, |_, cx| BreakpointsInFile::new(buffer, cx))?;
 
                     for bp in bps {
-                        let position = snapshot.anchor_after(Point::new(bp.position, 0));
+                        let position = snapshot.anchor_after(Point::new(bp.row, 0));
                         breakpoints_for_file.breakpoints.push((
                             position,
                             Breakpoint {
-                                kind: bp.kind,
+                                message: bp.message,
                                 state: bp.state,
                             },
                         ))
@@ -555,42 +546,6 @@ pub enum BreakpointEditAction {
     EditLogMessage(LogMessage),
 }
 
-#[derive(Clone, Debug)]
-pub enum BreakpointKind {
-    Standard,
-    Log(LogMessage),
-}
-
-impl BreakpointKind {
-    pub fn to_int(&self) -> i32 {
-        match self {
-            BreakpointKind::Standard => 0,
-            BreakpointKind::Log(_) => 1,
-        }
-    }
-
-    pub fn log_message(&self) -> Option<LogMessage> {
-        match self {
-            BreakpointKind::Standard => None,
-            BreakpointKind::Log(message) => Some(message.clone()),
-        }
-    }
-}
-
-impl PartialEq for BreakpointKind {
-    fn eq(&self, other: &Self) -> bool {
-        std::mem::discriminant(self) == std::mem::discriminant(other)
-    }
-}
-
-impl Eq for BreakpointKind {}
-
-impl Hash for BreakpointKind {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        std::mem::discriminant(self).hash(state);
-    }
-}
-
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
 pub enum BreakpointState {
     Enabled,
@@ -619,22 +574,22 @@ impl BreakpointState {
 
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct Breakpoint {
-    pub kind: BreakpointKind,
+    pub message: Option<Arc<str>>,
     pub state: BreakpointState,
 }
 
 impl Breakpoint {
     pub fn new_standard() -> Self {
         Self {
-            kind: BreakpointKind::Standard,
             state: BreakpointState::Enabled,
+            message: None,
         }
     }
 
     pub fn new_log(log_message: &str) -> Self {
         Self {
-            kind: BreakpointKind::Log(log_message.to_owned().into()),
             state: BreakpointState::Enabled,
+            message: Some(log_message.into()),
         }
     }
 
@@ -645,30 +600,17 @@ impl Breakpoint {
                 BreakpointState::Enabled => proto::BreakpointState::Enabled.into(),
                 BreakpointState::Disabled => proto::BreakpointState::Disabled.into(),
             },
-            kind: match self.kind {
-                BreakpointKind::Standard => proto::BreakpointKind::Standard.into(),
-                BreakpointKind::Log(_) => proto::BreakpointKind::Log.into(),
-            },
-            message: if let BreakpointKind::Log(message) = &self.kind {
-                Some(message.to_string())
-            } else {
-                None
-            },
+            message: self.message.as_ref().map(|s| String::from(s.as_ref())),
         })
     }
 
     fn from_proto(breakpoint: client::proto::Breakpoint) -> Option<Self> {
         Some(Self {
-            kind: match proto::BreakpointKind::from_i32(breakpoint.kind) {
-                Some(proto::BreakpointKind::Log) => {
-                    BreakpointKind::Log(breakpoint.message.clone().unwrap_or_default().into())
-                }
-                None | Some(proto::BreakpointKind::Standard) => BreakpointKind::Standard,
-            },
             state: match proto::BreakpointState::from_i32(breakpoint.state) {
                 Some(proto::BreakpointState::Disabled) => BreakpointState::Disabled,
                 None | Some(proto::BreakpointState::Enabled) => BreakpointState::Enabled,
             },
+            message: breakpoint.message.map(|message| message.into()),
         })
     }
 
@@ -683,22 +625,23 @@ impl Breakpoint {
     }
 }
 
+/// Breakpoint for location within source code.
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
-pub struct SerializedBreakpoint {
-    pub position: u32,
+pub struct SourceBreakpoint {
+    pub row: u32,
     pub path: Arc<Path>,
-    pub kind: BreakpointKind,
+    pub message: Option<Arc<str>>,
     pub state: BreakpointState,
 }
 
-impl From<SerializedBreakpoint> for dap::SourceBreakpoint {
-    fn from(bp: SerializedBreakpoint) -> Self {
+impl From<SourceBreakpoint> for dap::SourceBreakpoint {
+    fn from(bp: SourceBreakpoint) -> Self {
         Self {
-            line: bp.position as u64 + 1,
+            line: bp.row as u64 + 1,
             column: None,
             condition: None,
             hit_condition: None,
-            log_message: bp.kind.log_message().as_deref().map(Into::into),
+            log_message: bp.message.map(|message| String::from(message.as_ref())),
             mode: None,
         }
     }

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

@@ -16,13 +16,10 @@ use dap::{
     adapters::{DapStatus, DebugAdapterName},
     client::SessionId,
     messages::Message,
-    requests::{
-        Completions, Evaluate, Request as _, RunInTerminal, SetExpression, SetVariable,
-        StartDebugging,
-    },
+    requests::{Completions, Evaluate, Request as _, RunInTerminal, StartDebugging},
     Capabilities, CompletionItem, CompletionsArguments, DapRegistry, ErrorResponse,
     EvaluateArguments, EvaluateArgumentsContext, EvaluateResponse, RunInTerminalRequestArguments,
-    SetExpressionArguments, SetVariableArguments, Source, StartDebuggingRequestArguments,
+    Source, StartDebuggingRequestArguments,
 };
 use fs::Fs;
 use futures::{
@@ -710,59 +707,6 @@ impl DapStore {
         })
     }
 
-    #[allow(clippy::too_many_arguments)]
-    pub fn set_variable_value(
-        &self,
-        session_id: &SessionId,
-        stack_frame_id: u64,
-        variables_reference: u64,
-        name: String,
-        value: String,
-        evaluate_name: Option<String>,
-        cx: &mut Context<Self>,
-    ) -> Task<Result<()>> {
-        let Some(client) = self
-            .session_by_id(session_id)
-            .and_then(|client| client.read(cx).adapter_client())
-        else {
-            return Task::ready(Err(anyhow!("Could not find client: {:?}", session_id)));
-        };
-
-        let supports_set_expression = self
-            .capabilities_by_id(session_id, cx)
-            .and_then(|caps| caps.supports_set_expression)
-            .unwrap_or_default();
-
-        cx.background_executor().spawn(async move {
-            if let Some(evaluate_name) = supports_set_expression.then(|| evaluate_name).flatten() {
-                client
-                    .request::<SetExpression>(SetExpressionArguments {
-                        expression: evaluate_name,
-                        value,
-                        frame_id: Some(stack_frame_id),
-                        format: None,
-                    })
-                    .await?;
-            } else {
-                client
-                    .request::<SetVariable>(SetVariableArguments {
-                        variables_reference,
-                        name,
-                        value,
-                        format: None,
-                    })
-                    .await?;
-            }
-
-            Ok(())
-        })
-    }
-
-    // .. get the client and what not
-    // let _ = client.modules(); // This can fire a request to a dap adapter or be a cheap getter.
-    // client.wait_for_request(request::Modules); // This ensures that the request that we've fired off runs to completions
-    // let returned_value = client.modules(); // this is a cheap getter.
-
     pub fn shutdown_sessions(&mut self, cx: &mut Context<Self>) -> Task<()> {
         let mut tasks = vec![];
         for session_id in self.sessions.keys().cloned().collect::<Vec<_>>() {

crates/proto/proto/zed.proto 🔗

@@ -2660,21 +2660,15 @@ message RefreshLlmToken {}
 
 // Remote debugging
 
-enum BreakpointKind {
-    Standard = 0;
-    Log = 1;
-}
-
 enum BreakpointState {
     Enabled = 0;
     Disabled = 1;
 }
 
-
 message Breakpoint {
     Anchor position = 1;
     BreakpointState state = 2;
-    BreakpointKind kind = 3;
+    reserved 3;
     optional string message = 4;
 }
 

crates/workspace/src/persistence.rs 🔗

@@ -13,7 +13,7 @@ use client::DevServerProjectId;
 use db::{define_connection, query, sqlez::connection::Connection, sqlez_macros::sql};
 use gpui::{point, size, Axis, Bounds, WindowBounds, WindowId};
 use itertools::Itertools;
-use project::debugger::breakpoint_store::{BreakpointKind, BreakpointState, SerializedBreakpoint};
+use project::debugger::breakpoint_store::{BreakpointState, SourceBreakpoint};
 
 use language::{LanguageName, Toolchain};
 use project::WorktreeId;
@@ -147,7 +147,7 @@ impl Column for SerializedWindowBounds {
 #[derive(Debug)]
 pub struct Breakpoint {
     pub position: u32,
-    pub kind: BreakpointKind,
+    pub message: Option<Arc<str>>,
     pub state: BreakpointState,
 }
 
@@ -183,50 +183,6 @@ impl Column for BreakpointStateWrapper<'_> {
     }
 }
 
-/// Wrapper for DB type of a breakpoint
-#[derive(Debug)]
-struct BreakpointKindWrapper<'a>(Cow<'a, BreakpointKind>);
-
-impl From<BreakpointKind> for BreakpointKindWrapper<'static> {
-    fn from(kind: BreakpointKind) -> Self {
-        BreakpointKindWrapper(Cow::Owned(kind))
-    }
-}
-impl StaticColumnCount for BreakpointKindWrapper<'_> {
-    fn column_count() -> usize {
-        1
-    }
-}
-
-impl Bind for BreakpointKindWrapper<'_> {
-    fn bind(&self, statement: &Statement, start_index: i32) -> anyhow::Result<i32> {
-        let next_index = statement.bind(&self.0.to_int(), start_index)?;
-
-        match self.0.as_ref() {
-            BreakpointKind::Standard => {
-                statement.bind_null(next_index)?;
-                Ok(next_index + 1)
-            }
-            BreakpointKind::Log(message) => statement.bind(&message.as_ref(), next_index),
-        }
-    }
-}
-
-impl Column for BreakpointKindWrapper<'_> {
-    fn column(statement: &mut Statement, start_index: i32) -> anyhow::Result<(Self, i32)> {
-        let kind = statement.column_int(start_index)?;
-
-        match kind {
-            0 => Ok((BreakpointKind::Standard.into(), start_index + 2)),
-            1 => {
-                let message = statement.column_text(start_index)?.to_string();
-                Ok((BreakpointKind::Log(message.into()).into(), start_index + 1))
-            }
-            _ => Err(anyhow::anyhow!("Invalid BreakpointKind discriminant")),
-        }
-    }
-}
-
 /// This struct is used to implement traits on Vec<breakpoint>
 #[derive(Debug)]
 #[allow(dead_code)]
@@ -234,7 +190,7 @@ struct Breakpoints(Vec<Breakpoint>);
 
 impl sqlez::bindable::StaticColumnCount for Breakpoint {
     fn column_count() -> usize {
-        1 + BreakpointKindWrapper::column_count() + BreakpointStateWrapper::column_count()
+        2 + BreakpointStateWrapper::column_count()
     }
 }
 
@@ -245,10 +201,7 @@ impl sqlez::bindable::Bind for Breakpoint {
         start_index: i32,
     ) -> anyhow::Result<i32> {
         let next_index = statement.bind(&self.position, start_index)?;
-        let next_index = statement.bind(
-            &BreakpointKindWrapper(Cow::Borrowed(&self.kind)),
-            next_index,
-        )?;
+        let next_index = statement.bind(&self.message, next_index)?;
         statement.bind(
             &BreakpointStateWrapper(Cow::Borrowed(&self.state)),
             next_index,
@@ -262,13 +215,13 @@ impl Column for Breakpoint {
             .column_int(start_index)
             .with_context(|| format!("Failed to read BreakPoint at index {start_index}"))?
             as u32;
-        let (kind, next_index) = BreakpointKindWrapper::column(statement, start_index + 1)?;
+        let (message, next_index) = Option::<String>::column(statement, start_index + 1)?;
         let (state, next_index) = BreakpointStateWrapper::column(statement, next_index)?;
 
         Ok((
             Breakpoint {
                 position,
-                kind: kind.0.into_owned(),
+                message: message.map(Arc::from),
                 state: state.0.into_owned(),
             },
             next_index,
@@ -570,6 +523,9 @@ define_connection! {
     ),
     sql!(
         ALTER TABLE breakpoints ADD COLUMN state INTEGER DEFAULT(0) NOT NULL
+    ),
+    sql!(
+        ALTER TABLE breakpoints DROP COLUMN kind
     )
     ];
 }
@@ -720,13 +676,10 @@ impl WorkspaceDb {
         })
     }
 
-    fn breakpoints(
-        &self,
-        workspace_id: WorkspaceId,
-    ) -> BTreeMap<Arc<Path>, Vec<SerializedBreakpoint>> {
+    fn breakpoints(&self, workspace_id: WorkspaceId) -> BTreeMap<Arc<Path>, Vec<SourceBreakpoint>> {
         let breakpoints: Result<Vec<(PathBuf, Breakpoint)>> = self
             .select_bound(sql! {
-                SELECT path, breakpoint_location, kind, log_message, state
+                SELECT path, breakpoint_location, log_message, state
                 FROM breakpoints
                 WHERE workspace_id = ?
             })
@@ -738,18 +691,16 @@ impl WorkspaceDb {
                     log::debug!("Breakpoints are empty after querying database for them");
                 }
 
-                let mut map: BTreeMap<Arc<Path>, Vec<SerializedBreakpoint>> = Default::default();
+                let mut map: BTreeMap<Arc<Path>, Vec<SourceBreakpoint>> = Default::default();
 
                 for (path, breakpoint) in bp {
                     let path: Arc<Path> = path.into();
-                    map.entry(path.clone())
-                        .or_default()
-                        .push(SerializedBreakpoint {
-                            position: breakpoint.position,
-                            path,
-                            kind: breakpoint.kind,
-                            state: breakpoint.state,
-                        });
+                    map.entry(path.clone()).or_default().push(SourceBreakpoint {
+                        row: breakpoint.position,
+                        path,
+                        message: breakpoint.message,
+                        state: breakpoint.state,
+                    });
                 }
 
                 map
@@ -775,17 +726,17 @@ impl WorkspaceDb {
                     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 kind = BreakpointKindWrapper::from(bp.kind);
+                        let message = bp.message;
                         let state = BreakpointStateWrapper::from(bp.state);
                         match conn.exec_bound(sql!(
-                            INSERT INTO breakpoints (workspace_id, path, breakpoint_location, kind, log_message, state)
-                            VALUES (?1, ?2, ?3, ?4, ?5, ?6);))?
+                            INSERT INTO breakpoints (workspace_id, path, breakpoint_location,  log_message, state)
+                            VALUES (?1, ?2, ?3, ?4, ?5);))?
 
                         ((
                             workspace.id,
                             path.as_ref(),
-                            bp.position,
-                            kind,
+                            bp.row,
+                            message,
                             state,
                         )) {
                             Ok(_) => {}
@@ -1453,19 +1404,19 @@ mod tests {
 
         let breakpoint = Breakpoint {
             position: 123,
-            kind: BreakpointKind::Standard,
+            message: None,
             state: BreakpointState::Enabled,
         };
 
         let log_breakpoint = Breakpoint {
             position: 456,
-            kind: BreakpointKind::Log("Test log message".into()),
+            message: Some("Test log message".into()),
             state: BreakpointState::Enabled,
         };
 
         let disable_breakpoint = Breakpoint {
             position: 578,
-            kind: BreakpointKind::Standard,
+            message: None,
             state: BreakpointState::Disabled,
         };
 
@@ -1482,22 +1433,22 @@ mod tests {
                 map.insert(
                     Arc::from(path),
                     vec![
-                        SerializedBreakpoint {
-                            position: breakpoint.position,
+                        SourceBreakpoint {
+                            row: breakpoint.position,
                             path: Arc::from(path),
-                            kind: breakpoint.kind.clone(),
+                            message: breakpoint.message.clone(),
                             state: breakpoint.state,
                         },
-                        SerializedBreakpoint {
-                            position: log_breakpoint.position,
+                        SourceBreakpoint {
+                            row: log_breakpoint.position,
                             path: Arc::from(path),
-                            kind: log_breakpoint.kind.clone(),
+                            message: log_breakpoint.message.clone(),
                             state: log_breakpoint.state,
                         },
-                        SerializedBreakpoint {
-                            position: disable_breakpoint.position,
+                        SourceBreakpoint {
+                            row: disable_breakpoint.position,
                             path: Arc::from(path),
-                            kind: disable_breakpoint.kind.clone(),
+                            message: disable_breakpoint.message.clone(),
                             state: disable_breakpoint.state,
                         },
                     ],
@@ -1515,18 +1466,18 @@ mod tests {
 
         assert_eq!(loaded_breakpoints.len(), 3);
 
-        assert_eq!(loaded_breakpoints[0].position, breakpoint.position);
-        assert_eq!(loaded_breakpoints[0].kind, breakpoint.kind);
+        assert_eq!(loaded_breakpoints[0].row, breakpoint.position);
+        assert_eq!(loaded_breakpoints[0].message, breakpoint.message);
         assert_eq!(loaded_breakpoints[0].state, breakpoint.state);
         assert_eq!(loaded_breakpoints[0].path, Arc::from(path));
 
-        assert_eq!(loaded_breakpoints[1].position, log_breakpoint.position);
-        assert_eq!(loaded_breakpoints[1].kind, log_breakpoint.kind);
+        assert_eq!(loaded_breakpoints[1].row, log_breakpoint.position);
+        assert_eq!(loaded_breakpoints[1].message, log_breakpoint.message);
         assert_eq!(loaded_breakpoints[1].state, log_breakpoint.state);
         assert_eq!(loaded_breakpoints[1].path, Arc::from(path));
 
-        assert_eq!(loaded_breakpoints[2].position, disable_breakpoint.position);
-        assert_eq!(loaded_breakpoints[2].kind, disable_breakpoint.kind);
+        assert_eq!(loaded_breakpoints[2].row, disable_breakpoint.position);
+        assert_eq!(loaded_breakpoints[2].message, disable_breakpoint.message);
         assert_eq!(loaded_breakpoints[2].state, disable_breakpoint.state);
         assert_eq!(loaded_breakpoints[2].path, Arc::from(path));
     }

crates/workspace/src/persistence/model.rs 🔗

@@ -10,7 +10,7 @@ use db::sqlez::{
 };
 use gpui::{AsyncWindowContext, Entity, WeakEntity};
 use itertools::Itertools as _;
-use project::{debugger::breakpoint_store::SerializedBreakpoint, Project};
+use project::{debugger::breakpoint_store::SourceBreakpoint, Project};
 use remote::ssh_session::SshProjectId;
 use serde::{Deserialize, Serialize};
 use std::{
@@ -264,7 +264,7 @@ pub(crate) struct SerializedWorkspace {
     pub(crate) display: Option<Uuid>,
     pub(crate) docks: DockStructure,
     pub(crate) session_id: Option<String>,
-    pub(crate) breakpoints: BTreeMap<Arc<Path>, Vec<SerializedBreakpoint>>,
+    pub(crate) breakpoints: BTreeMap<Arc<Path>, Vec<SourceBreakpoint>>,
     pub(crate) window_id: Option<u64>,
 }