feat(debug): scope-precise breakpoints

Continuing work done in helix-editor#5957, these changes add column precision
to breakpoints for those set using the keymap command. For those set
with the mouse, preserve the current behaviour, concretely, add a line
breakpoint. This allows for more fine grained control, allowing, for
debuggers which support it, to add breakpoints to scopes such as lambda
functions, callbacks and more.

Change how the stack frame line is highlighted, using a full line for
breakpoints which apply to a line and highlight only the scope for those
that operate only on those scopes.

Closes: helix-editor#6238
Signed-off-by: Filip Dutescu <[email protected]>

helix-dap/src/types.rs

@@ -1,6 +1,7 @@
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use std::collections::HashMap;
+use std::convert::TryFrom;
 use std::path::PathBuf;
 
 #[derive(
@@ -22,6 +23,64 @@ pub trait Request {
     const COMMAND: &'static str;
 }
 
+#[derive(Copy, Debug, Default, PartialEq, Eq, Clone, Deserialize, Serialize)]
+pub struct Line(u32);
+
+impl TryFrom<Line> for usize {
+    type Error = ();
+
+    fn try_from(value: Line) -> Result<Self, Self::Error> {
+        (value.0 as usize).checked_sub(1).ok_or(())
+    }
+}
+
+impl TryFrom<usize> for Line {
+    type Error = ();
+
+    fn try_from(value: usize) -> Result<Self, Self::Error> {
+        if let Some(value) = (value as u32).checked_add(1) {
+            Ok(Line(value))
+        } else {
+            Err(())
+        }
+    }
+}
+
+impl TryFrom<Line> for u32 {
+    type Error = ();
+
+    fn try_from(value: Line) -> Result<Self, Self::Error> {
+        value.0.checked_sub(1).ok_or(())
+    }
+}
+
+#[derive(Copy, Debug, Default, PartialEq, Eq, Clone, Deserialize, Serialize)]
+pub struct Column(u32);
+
+impl Column {
+    pub fn saturating_sub(self, rhs: u32) -> Self {
+        Self(self.0.saturating_sub(rhs))
+    }
+}
+
+impl From<usize> for Column {
+    fn from(column: usize) -> Self {
+        Column(column as u32)
+    }
+}
+
+impl From<Column> for usize {
+    fn from(column: Column) -> Self {
+        column.0 as usize
+    }
+}
+
+impl From<Column> for u32 {
+    fn from(column: Column) -> Self {
+        column.0
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ColumnDescriptor {
@@ -162,9 +221,9 @@ pub struct Source {
 #[derive(Debug, Default, PartialEq, Eq, Clone, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct SourceBreakpoint {
-    pub line: usize,
+    pub line: Line,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub column: Option<usize>,
+    pub column: Option<Column>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub condition: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -184,19 +243,31 @@ pub struct Breakpoint {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub source: Option<Source>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub line: Option<usize>,
+    pub line: Option<Line>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub column: Option<usize>,
+    pub column: Option<Column>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub end_line: Option<usize>,
+    pub end_line: Option<Line>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub end_column: Option<usize>,
+    pub end_column: Option<Column>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub instruction_reference: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub offset: Option<usize>,
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct BreakpointLocation {
+    pub line: Line,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub column: Option<Column>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub end_line: Option<Line>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub end_column: Option<Column>,
+}
+
 #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct StackFrameFormat {
@@ -223,12 +294,12 @@ pub struct StackFrame {
     pub name: String,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub source: Option<Source>,
-    pub line: usize,
-    pub column: usize,
+    pub line: Line,
+    pub column: Column,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub end_line: Option<usize>,
+    pub end_line: Option<Line>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub end_column: Option<usize>,
+    pub end_column: Option<Column>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub can_restart: Option<bool>,
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -261,13 +332,13 @@ pub struct Scope {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub source: Option<Source>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub line: Option<usize>,
+    pub line: Option<Line>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub column: Option<usize>,
+    pub column: Option<Column>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub end_line: Option<usize>,
+    pub end_line: Option<Line>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub end_column: Option<usize>,
+    pub end_column: Option<Column>,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
@@ -820,9 +891,9 @@ pub mod events {
         #[serde(skip_serializing_if = "Option::is_none")]
         pub group: Option<String>,
         #[serde(skip_serializing_if = "Option::is_none")]
-        pub line: Option<usize>,
+        pub line: Option<Line>,
         #[serde(skip_serializing_if = "Option::is_none")]
-        pub column: Option<usize>,
+        pub column: Option<Column>,
         #[serde(skip_serializing_if = "Option::is_none")]
         pub variables_reference: Option<usize>,
         #[serde(skip_serializing_if = "Option::is_none")]

helix-term/src/application.rs

@@ -99,7 +99,7 @@ fn setup_integration_logging() {
             ))
         })
         .level(level)
-        .chain(std::io::stdout())
+        .chain(stdout())
         .apply();
 }
 

helix-term/src/commands/dap.rs

@@ -8,7 +8,7 @@ use dap::{StackFrame, Thread, ThreadStates};
 use helix_core::syntax::{DebugArgumentValue, DebugConfigCompletion, DebugTemplate};
 use helix_dap::{self as dap, Client};
 use helix_lsp::block_on;
-use helix_view::editor::Breakpoint;
+use helix_view::{editor::Breakpoint, handlers::dap::pos_to_dap_pos};
 
 use serde_json::{to_value, Value};
 use tokio_stream::wrappers::UnboundedReceiverStream;
@@ -82,8 +82,8 @@ fn thread_picker(
                     let frame = frames.get(0)?;
                     let path = frame.source.as_ref()?.path.clone()?;
                     let pos = Some((
-                        frame.line.saturating_sub(1),
-                        frame.end_line.unwrap_or(frame.line).saturating_sub(1),
+                        frame.line.try_into().unwrap_or(0),
+                        frame.end_line.unwrap_or(frame.line).try_into().unwrap_or(0),
                     ));
                     Some((path.into(), pos))
                 },
@@ -100,7 +100,9 @@ fn get_breakpoint_at_current_line(editor: &mut Editor) -> Option<(usize, Breakpo
     let line = doc.selection(view.id).primary().cursor_line(text);
     let path = doc.path()?;
     editor.breakpoints.get(path).and_then(|breakpoints| {
-        let i = breakpoints.iter().position(|b| b.line == line);
+        let i = breakpoints
+            .iter()
+            .position(|b| b.line.try_into().unwrap_or(0) == line);
         i.map(|i| (i, breakpoints[i].clone()))
     })
 }
@@ -396,25 +398,37 @@ pub fn dap_toggle_breakpoint(cx: &mut Context) {
             return;
         }
     };
-    let text = doc.text().slice(..);
-    let line = doc.selection(view.id).primary().cursor_line(text);
-    dap_toggle_breakpoint_impl(cx, path, line);
+    let dap_pos = pos_to_dap_pos(doc.text(), doc.selection(view.id).primary().head);
+    dap_toggle_breakpoint_impl(
+        cx,
+        path,
+        dap_pos.line as usize,
+        Some(dap_pos.character as usize),
+    );
 }
 
-pub fn dap_toggle_breakpoint_impl(cx: &mut Context, path: PathBuf, line: usize) {
+pub fn dap_toggle_breakpoint_impl(
+    cx: &mut Context,
+    path: PathBuf,
+    line: usize,
+    column: Option<usize>,
+) {
     // TODO: need to map breakpoints over edits and update them?
     // we shouldn't really allow editing while debug is running though
 
     let breakpoints = cx.editor.breakpoints.entry(path.clone()).or_default();
     // TODO: always keep breakpoints sorted and use binary search to determine insertion point
-    if let Some(pos) = breakpoints
-        .iter()
-        .position(|breakpoint| breakpoint.line == line)
-    {
+    if let Some(pos) = breakpoints.iter().position(|breakpoint| {
+        breakpoint.line.try_into().unwrap_or(0) == line
+            && breakpoint.column.map(|column| column.into()) == column
+    }) {
         breakpoints.remove(pos);
     } else {
+        log::error!("line is {line:?}");
+        log::error!("column is {column:?}");
         breakpoints.push(Breakpoint {
-            line,
+            line: line.try_into().unwrap_or_default(),
+            column: column.map(|column| column.into()),
             ..Default::default()
         });
     }
@@ -755,8 +769,12 @@ pub fn dap_switch_stack_frame(cx: &mut Context) {
                     (
                         path.into(),
                         Some((
-                            frame.line.saturating_sub(1),
-                            frame.end_line.unwrap_or(frame.line).saturating_sub(1),
+                            frame.line.try_into().unwrap_or_default(),
+                            frame
+                                .end_line
+                                .unwrap_or(frame.line)
+                                .try_into()
+                                .unwrap_or_default(),
                         )),
                     )
                 })

helix-term/src/ui/editor.rs

@@ -24,6 +24,7 @@ use helix_view::{
     document::{Mode, SavePoint, SCRATCH_BUFFER_NAME},
     editor::{CompleteAction, CursorShapeConfig},
     graphics::{Color, CursorKind, Modifier, Rect, Style},
+    handlers::dap::dap_pos_to_pos,
     input::{KeyEvent, MouseButton, MouseEvent, MouseEventKind},
     keyboard::{KeyCode, KeyModifiers},
     Document, Editor, Theme, View,
@@ -138,6 +139,34 @@ impl EditorView {
             highlights = Box::new(syntax::merge(highlights, diagnostic));
         }
 
+        // Set DAP highlights, with possibility to overwrite syntax highlights, if fg set.
+        let highlights: Box<dyn Iterator<Item = HighlightEvent>> =
+            if let Some(frame) = editor.current_stack_frame() {
+                log::error!("Frame: {frame:?}");
+                let dap_line_start = frame.line;
+                let dap_line_end = frame.end_line.unwrap_or(dap_line_start);
+                let dap_col_start = frame.column;
+                let dap_col_end = frame.end_column.unwrap_or_default();
+
+                let dap_start = dap_pos_to_pos(doc.text(), dap_line_start, dap_col_start);
+                let dap_end = dap_pos_to_pos(doc.text(), dap_line_end, dap_col_end);
+                log::error!("Dap: {dap_start:?}->{dap_end:?}");
+                let dap_start =
+                    dap_start.unwrap_or_else(|| usize::try_from(dap_line_start).unwrap_or(0));
+                let dap_end = dap_end.unwrap_or_else(|| usize::try_from(dap_line_end).unwrap_or(0));
+
+                if let Some(dap_current_index) = theme.find_scope_index("ui.highlight.frameline") {
+                    Box::new(syntax::merge(
+                        highlights,
+                        vec![(dap_current_index, dap_start..dap_end)],
+                    ))
+                } else {
+                    highlights
+                }
+            } else {
+                highlights
+            };
+
         let highlights: Box<dyn Iterator<Item = HighlightEvent>> = if is_focused {
             let highlights = syntax::merge(
                 highlights,
@@ -411,20 +440,10 @@ impl EditorView {
         let base_primary_cursor_scope = theme
             .find_scope_index("ui.cursor.primary")
             .unwrap_or(base_cursor_scope);
-
-        let cursor_scope = match mode {
-            Mode::Insert => theme.find_scope_index_exact("ui.cursor.insert"),
-            Mode::Select => theme.find_scope_index_exact("ui.cursor.select"),
-            Mode::Normal => theme.find_scope_index_exact("ui.cursor.normal"),
-        }
-        .unwrap_or(base_cursor_scope);
-
-        let primary_cursor_scope = match mode {
-            Mode::Insert => theme.find_scope_index_exact("ui.cursor.primary.insert"),
-            Mode::Select => theme.find_scope_index_exact("ui.cursor.primary.select"),
-            Mode::Normal => theme.find_scope_index_exact("ui.cursor.primary.normal"),
-        }
-        .unwrap_or(base_primary_cursor_scope);
+        let cursor_scope = mode.cursor_scope(theme).unwrap_or(base_cursor_scope);
+        let primary_cursor_scope = mode
+            .primary_cursor_scope(theme)
+            .unwrap_or(base_primary_cursor_scope);
 
         let mut spans: Vec<(usize, std::ops::Range<usize>)> = Vec::new();
         for (i, range) in selection.iter().enumerate() {
@@ -459,16 +478,14 @@ impl EditorView {
                     } else {
                         cursor_start
                     };
+
                 spans.push((selection_scope, range.anchor..selection_end));
                 if !selection_is_primary || cursor_is_block {
                     spans.push((cursor_scope, cursor_start..range.head));
                 }
             } else {
                 // Reverse case.
                 let cursor_end = next_grapheme_boundary(text, range.head);
-                if !selection_is_primary || cursor_is_block {
-                    spans.push((cursor_scope, range.head..cursor_end));
-                }
                 // non block cursors look like they exclude the cursor
                 let selection_start = if selection_is_primary
                     && !cursor_is_block
@@ -478,6 +495,10 @@ impl EditorView {
                 } else {
                     cursor_end
                 };
+
+                if !selection_is_primary || cursor_is_block {
+                    spans.push((cursor_scope, range.head..cursor_end));
+                }
                 spans.push((selection_scope, selection_start..range.anchor));
             }
         }
@@ -1055,7 +1076,7 @@ impl EditorView {
                         view.pos_at_visual_coords(doc, coords.row as u16, coords.col as u16, true)
                     {
                         let line = doc.text().char_to_line(char_idx);
-                        commands::dap_toggle_breakpoint_impl(cxt, path, line);
+                        commands::dap_toggle_breakpoint_impl(cxt, path, line, None);
                         return EventResult::Consumed(None);
                     }
                 }