dap: Rewrite breakpoints so that there's a single set maintained

4 files changed, 148 insertions, 177 deletions

diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index 69a51a21..3d6d00fe 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -1,7 +1,7 @@
 use helix_core::{merge_toml_values, syntax};
 use helix_dap::Payload;
 use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap};
-use helix_view::{theme, Editor};
+use helix_view::{editor::Breakpoint, theme, Editor};
 
 use crate::{
     args::Args, commands::fetch_stack_trace, compositor::Compositor, config::Config, job::Jobs, ui,
@@ -319,7 +319,7 @@ impl Application {
     }
 
     pub async fn handle_debugger_message(&mut self, payload: helix_dap::Payload) {
-        use crate::commands::dap::{resume_application, select_thread_id};
+        use crate::commands::dap::{breakpoints_changed, resume_application, select_thread_id};
         use helix_dap::{events, Event};
         let debugger = match self.editor.debugger.as_mut() {
             Some(debugger) => debugger,
@@ -388,46 +388,35 @@ impl Application {
                 }
                 Event::Breakpoint(events::Breakpoint { reason, breakpoint }) => match &reason[..] {
                     "new" => {
-                        debugger.breakpoints.push(breakpoint);
+                        self.editor
+                            .breakpoints
+                            .entry(breakpoint.source.unwrap().path.unwrap()) // TODO: no unwraps
+                            .or_default()
+                            .push(Breakpoint {
+                                id: breakpoint.id,
+                                verified: breakpoint.verified,
+                                message: breakpoint.message,
+                                line: breakpoint.line.unwrap().saturating_sub(1), // TODO: no unwrap
+                                column: breakpoint.column,
+                                ..Default::default()
+                            });
                     }
                     "changed" => {
-                        match debugger
-                            .breakpoints
-                            .iter()
-                            .position(|b| b.id == breakpoint.id)
-                        {
-                            Some(i) => {
-                                debugger.breakpoints[i] = breakpoint;
-                                // let item = debugger.breakpoints.get_mut(i).unwrap();
-                                // item.verified = breakpoint.verified;
-                                // // TODO: wasteful clones
-                                // item.message = breakpoint.message.or_else(|| item.message.clone());
-                                // item.source = breakpoint.source.or_else(|| item.source.clone());
-                                // item.line = breakpoint.line.or(item.line);
-                                // item.column = breakpoint.column.or(item.column);
-                                // item.end_line = breakpoint.end_line.or(item.end_line);
-                                // item.end_column = breakpoint.end_column.or(item.end_column);
-                                // item.instruction_reference = breakpoint
-                                //     .instruction_reference
-                                //     .or_else(|| item.instruction_reference.clone());
-                                // item.offset = breakpoint.offset.or(item.offset);
-                            }
-                            None => {
-                                warn!("Changed breakpoint with id {:?} not found", breakpoint.id);
+                        for breakpoints in self.editor.breakpoints.values_mut() {
+                            if let Some(i) = breakpoints.iter().position(|b| b.id == breakpoint.id)
+                            {
+                                breakpoints[i].verified = breakpoint.verified;
+                                breakpoints[i].message = breakpoint.message.clone();
+                                breakpoints[i].line = breakpoint.line.unwrap().saturating_sub(1); // TODO: no unwrap
+                                breakpoints[i].column = breakpoint.column;
                             }
                         }
                     }
                     "removed" => {
-                        match debugger
-                            .breakpoints
-                            .iter()
-                            .position(|b| b.id == breakpoint.id)
-                        {
-                            Some(i) => {
-                                debugger.breakpoints.remove(i);
-                            }
-                            None => {
-                                warn!("Removed breakpoint with id {:?} not found", breakpoint.id);
+                        for breakpoints in self.editor.breakpoints.values_mut() {
+                            if let Some(i) = breakpoints.iter().position(|b| b.id == breakpoint.id)
+                            {
+                                breakpoints.remove(i);
                             }
                         }
                     }
@@ -453,13 +442,9 @@ impl Application {
                 }
                 Event::Initialized => {
                     // send existing breakpoints
-                    for (path, breakpoints) in &self.editor.breakpoints {
+                    for (path, breakpoints) in &mut self.editor.breakpoints {
                         // TODO: call futures in parallel, await all
-                        debugger.breakpoints = debugger
-                            .set_breakpoints(path.clone(), breakpoints.clone())
-                            .await
-                            .unwrap()
-                            .unwrap();
+                        let _ = breakpoints_changed(debugger, path.clone(), breakpoints);
                     }
                     // TODO: fetch breakpoints (in case we're attaching)
 
diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index 084479cc..3d500405 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -1897,8 +1897,8 @@ fn append_mode(cx: &mut Context) {
 mod cmd {
     use super::*;
 
-    use helix_dap::SourceBreakpoint;
     use helix_view::editor::Action;
+    use helix_view::editor::Breakpoint;
     use ui::completers::{self, Completer};
 
     #[derive(Clone)]
@@ -2563,9 +2563,7 @@ mod cmd {
         Ok(())
     }
 
-    pub fn get_breakpoint_at_current_line(
-        editor: &mut Editor,
-    ) -> Option<(usize, SourceBreakpoint)> {
+    pub fn get_breakpoint_at_current_line(editor: &mut Editor) -> Option<(usize, Breakpoint)> {
         let (view, doc) = current!(editor);
         let text = doc.text().slice(..);
 
diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs
index 2a3d08a2..eee42244 100644
--- a/helix-term/src/commands/dap.rs
+++ b/helix-term/src/commands/dap.rs
@@ -11,11 +11,13 @@ use helix_core::{
 };
 use helix_dap::{self as dap, Client, ThreadId};
 use helix_lsp::block_on;
+use helix_view::editor::Breakpoint;
 
 use serde_json::{to_value, Value};
 use tokio_stream::wrappers::UnboundedReceiverStream;
 
 use std::collections::HashMap;
+use std::path::PathBuf;
 
 // general utils:
 pub fn dap_pos_to_pos(doc: &helix_core::Rope, line: usize, column: usize) -> Option<usize> {
@@ -406,40 +408,90 @@ pub fn dap_toggle_breakpoint(cx: &mut Context) {
     dap_toggle_breakpoint_impl(cx, path, line);
 }
 
-pub fn dap_toggle_breakpoint_impl(cx: &mut Context, path: std::path::PathBuf, line: usize) {
-    let breakpoint = helix_dap::SourceBreakpoint {
-        line: line + 1, // convert from 0-indexing to 1-indexing (TODO: could set debugger to 0-indexing on init)
-        ..Default::default()
+pub fn breakpoints_changed(
+    debugger: &mut dap::Client,
+    path: PathBuf,
+    breakpoints: &mut [Breakpoint],
+) -> Result<(), anyhow::Error> {
+    // TODO: handle capabilities correctly again, by filterin breakpoints when emitting
+    // if breakpoint.condition.is_some()
+    //     && !debugger
+    //         .caps
+    //         .as_ref()
+    //         .unwrap()
+    //         .supports_conditional_breakpoints
+    //         .unwrap_or_default()
+    // {
+    //     bail!(
+    //         "Can't edit breakpoint: debugger does not support conditional breakpoints"
+    //     )
+    // }
+    // if breakpoint.log_message.is_some()
+    //     && !debugger
+    //         .caps
+    //         .as_ref()
+    //         .unwrap()
+    //         .supports_log_points
+    //         .unwrap_or_default()
+    // {
+    //     bail!("Can't edit breakpoint: debugger does not support logpoints")
+    // }
+    let source_breakpoints = breakpoints
+        .iter()
+        .map(|breakpoint| helix_dap::SourceBreakpoint {
+            line: breakpoint.line + 1, // convert from 0-indexing to 1-indexing (TODO: could set debugger to 0-indexing on init)
+            ..Default::default()
+        })
+        .collect::<Vec<_>>();
+
+    let request = debugger.set_breakpoints(path, source_breakpoints);
+    match block_on(request) {
+        Ok(Some(dap_breakpoints)) => {
+            for (breakpoint, dap_breakpoint) in breakpoints.iter_mut().zip(dap_breakpoints) {
+                breakpoint.id = dap_breakpoint.id;
+                breakpoint.verified = dap_breakpoint.verified;
+                breakpoint.message = dap_breakpoint.message;
+                // TODO: handle breakpoint.message
+                // TODO: verify source matches
+                breakpoint.line = dap_breakpoint.line.unwrap_or(0).saturating_sub(1); // convert to 0-indexing
+                                                                                      // TODO: no unwrap
+                breakpoint.column = dap_breakpoint.column;
+                // TODO: verify end_linef/col instruction reference, offset
+            }
+        }
+        Err(e) => anyhow::bail!("Failed to set breakpoints: {}", e),
+        _ => {}
     };
+    Ok(())
+}
 
+pub fn dap_toggle_breakpoint_impl(cx: &mut Context, path: PathBuf, line: 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
-    if let Some(pos) = breakpoints.iter().position(|b| b.line == breakpoint.line) {
+    // TODO: always keep breakpoints sorted and use binary search to determine insertion point
+    if let Some(pos) = breakpoints
+        .iter()
+        .position(|breakpoint| breakpoint.line == line)
+    {
         breakpoints.remove(pos);
     } else {
-        breakpoints.push(breakpoint);
+        breakpoints.push(Breakpoint {
+            line,
+            ..Default::default()
+        });
     }
 
-    let breakpoints = breakpoints.clone();
-
     let debugger = match &mut cx.editor.debugger {
         Some(debugger) => debugger,
         None => return,
     };
-    let request = debugger.set_breakpoints(path, breakpoints);
-    match block_on(request) {
-        Ok(Some(breakpoints)) => {
-            // TODO: handle breakpoint.message
-            debugger.breakpoints = breakpoints;
-        }
-        Err(e) => cx
-            .editor
-            .set_error(format!("Failed to set breakpoints: {}", e)),
-        _ => {}
-    };
+
+    if let Err(e) = breakpoints_changed(debugger, path, breakpoints) {
+        cx.editor
+            .set_error(format!("Failed to set breakpoints: {}", e));
+    }
 }
 
 pub fn dap_continue(cx: &mut Context) {
@@ -651,36 +703,15 @@ pub fn dap_edit_condition(cx: &mut Context) {
                                 input => Some(input.to_owned()),
                             };
 
-                            if let Some(debugger) = &mut cx.editor.debugger {
-                                // TODO: handle capabilities correctly again, by filterin breakpoints when emitting
-                                // if breakpoint.condition.is_some()
-                                //     && !debugger
-                                //         .caps
-                                //         .as_ref()
-                                //         .unwrap()
-                                //         .supports_conditional_breakpoints
-                                //         .unwrap_or_default()
-                                // {
-                                //     bail!(
-                                //         "Can't edit breakpoint: debugger does not support conditional breakpoints"
-                                //     )
-                                // }
-                                // if breakpoint.log_message.is_some()
-                                //     && !debugger
-                                //         .caps
-                                //         .as_ref()
-                                //         .unwrap()
-                                //         .supports_log_points
-                                //         .unwrap_or_default()
-                                // {
-                                //     bail!("Can't edit breakpoint: debugger does not support logpoints")
-                                // }
-                                let request =
-                                    debugger.set_breakpoints(path.clone(), breakpoints.clone());
-                                if let Err(e) = block_on(request) {
-                                    cx.editor
-                                        .set_status(format!("Failed to set breakpoints: {}", e))
-                                }
+                            let debugger = match &mut cx.editor.debugger {
+                                Some(debugger) => debugger,
+                                None => return,
+                            };
+
+                            if let Err(e) = breakpoints_changed(debugger, path.clone(), breakpoints)
+                            {
+                                cx.editor
+                                    .set_error(format!("Failed to set breakpoints: {}", e));
                             }
                         },
                     );
@@ -719,36 +750,14 @@ pub fn dap_edit_log(cx: &mut Context) {
                                 input => Some(input.to_owned()),
                             };
 
-                            if let Some(debugger) = &mut cx.editor.debugger {
-                                // TODO: handle capabilities correctly again, by filterin breakpoints when emitting
-                                // if breakpoint.condition.is_some()
-                                //     && !debugger
-                                //         .caps
-                                //         .as_ref()
-                                //         .unwrap()
-                                //         .supports_conditional_breakpoints
-                                //         .unwrap_or_default()
-                                // {
-                                //     bail!(
-                                //         "Can't edit breakpoint: debugger does not support conditional breakpoints"
-                                //     )
-                                // }
-                                // if breakpoint.log_message.is_some()
-                                //     && !debugger
-                                //         .caps
-                                //         .as_ref()
-                                //         .unwrap()
-                                //         .supports_log_points
-                                //         .unwrap_or_default()
-                                // {
-                                //     bail!("Can't edit breakpoint: debugger does not support logpoints")
-                                // }
-                                let request =
-                                    debugger.set_breakpoints(path.clone(), breakpoints.clone());
-                                if let Err(e) = block_on(request) {
-                                    cx.editor
-                                        .set_status(format!("Failed to set breakpoints: {}", e))
-                                }
+                            let debugger = match &mut cx.editor.debugger {
+                                Some(debugger) => debugger,
+                                None => return,
+                            };
+                            if let Err(e) = breakpoints_changed(debugger, path.clone(), breakpoints)
+                            {
+                                cx.editor
+                                    .set_error(format!("Failed to set breakpoints: {}", e));
                             }
                         },
                     );
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index bae1edfb..a3421991 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -15,7 +15,6 @@ use helix_core::{
     unicode::width::UnicodeWidthStr,
     LineEnding, Position, Range, Selection,
 };
-use helix_dap::{Breakpoint, SourceBreakpoint, StackFrame};
 use helix_view::{
     document::{Mode, SCRATCH_BUFFER_NAME},
     graphics::{Color, CursorKind, Modifier, Rect, Style},
@@ -24,7 +23,7 @@ use helix_view::{
     keyboard::{KeyCode, KeyModifiers},
     Document, Editor, Theme, View,
 };
-use std::{borrow::Cow, collections::HashMap, path::PathBuf};
+use std::borrow::Cow;
 
 use crossterm::event::{Event, MouseButton, MouseEvent, MouseEventKind};
 use tui::buffer::Buffer as Surface;
@@ -74,7 +73,6 @@ impl EditorView {
         let inner = view.inner_area();
         let area = view.area;
         let theme = &editor.theme;
-        let all_breakpoints = HashMap::new();
 
         let highlights = Self::doc_syntax_highlights(doc, view.offset, inner.height, theme, loader);
         let highlights = syntax::merge(highlights, Self::doc_diagnostics_highlights(doc, theme));
@@ -107,7 +105,7 @@ impl EditorView {
             }
         }
 
-        self.render_diagnostics(doc, view, inner, surface, theme, &all_breakpoints);
+        self.render_diagnostics(doc, view, inner, surface, theme);
 
         let statusline_area = view
             .area
@@ -436,39 +434,21 @@ impl EditorView {
             let error = theme.get("error");
             let info = theme.get("info");
 
-            let all_breakpoints = &editor.breakpoints;
-
-            // TODO: debugger breakpoints should keep editor level breakpoints in sync
-
             let breakpoints = doc
                 .path()
-                .and_then(|path| all_breakpoints.get(path))
+                .and_then(|path| editor.breakpoints.get(path))
                 .unwrap();
 
             Box::new(move |line: usize, _selected: bool, out: &mut String| {
-                // TODO: debugger should translate received breakpoints to 0-indexing
-                let breakpoint = breakpoints.iter().find(|breakpoint| {
-                    false
-                    // if breakpoint.source == doc.path() {
-                    //     match (breakpoint.line, breakpoint.end_line) {
-                    //         #[allow(clippy::int_plus_one)]
-                    //         (Some(l), Some(el)) => l - 1 <= line && line <= el - 1,
-                    //         (Some(l), None) => l - 1 == line,
-                    //         _ => false,
-                    //     }
-                    // } else {
-                    //     false
-                    // }
-                });
+                let breakpoint = breakpoints
+                    .iter()
+                    .find(|breakpoint| breakpoint.line == line);
 
                 let breakpoint = match breakpoint {
                     Some(b) => b,
                     None => return None,
                 };
 
-                // let verified = breakpoint.verified;
-                let verified = false;
-
                 let mut style =
                     if breakpoint.condition.is_some() && breakpoint.log_message.is_some() {
                         error.add_modifier(Modifier::UNDERLINED)
@@ -480,7 +460,7 @@ impl EditorView {
                         warning
                     };
 
-                if !verified {
+                if !breakpoint.verified {
                     // Faded colors
                     style = if let Some(Color::Rgb(r, g, b)) = style.fg {
                         style.fg(Color::Rgb(
@@ -495,7 +475,7 @@ impl EditorView {
 
                 // TODO: also handle breakpoints only present in the user struct
                 use std::fmt::Write;
-                let sym = if verified { "▲" } else { "⊚" };
+                let sym = if breakpoint.verified { "▲" } else { "⊚" };
                 write!(out, "{}", sym).unwrap();
                 Some(style)
             })
@@ -567,7 +547,6 @@ impl EditorView {
         viewport: Rect,
         surface: &mut Surface,
         theme: &Theme,
-        all_breakpoints: &HashMap<PathBuf, Vec<SourceBreakpoint>>,
     ) {
         use helix_core::diagnostic::Severity;
         use tui::{
@@ -605,28 +584,28 @@ impl EditorView {
             lines.extend(text.lines);
         }
 
-        if let Some(path) = doc.path() {
-            let line = doc.text().char_to_line(cursor);
-            if let Some(breakpoints) = all_breakpoints.get(path) {
-                if let Some(breakpoint) = breakpoints
-                    .iter()
-                    .find(|breakpoint| breakpoint.line - 1 == line)
-                {
-                    if let Some(condition) = &breakpoint.condition {
-                        lines.extend(
-                            Text::styled(condition, warning.add_modifier(Modifier::UNDERLINED))
-                                .lines,
-                        );
-                    }
-                    if let Some(log_message) = &breakpoint.log_message {
-                        lines.extend(
-                            Text::styled(log_message, info.add_modifier(Modifier::UNDERLINED))
-                                .lines,
-                        );
-                    }
-                }
-            }
-        }
+        // let line = doc.text().char_to_line(cursor);
+        // let breakpoint = doc
+        //     .path()
+        //     .and_then(|path| all_breakpoints.get(path))
+        //     .and_then(|breakpoints| {
+        //         breakpoints
+        //             .iter()
+        //             .find(|breakpoint| breakpoint.line == line)
+        //     });
+
+        // if let Some(breakpoint) = breakpoint {
+        //     if let Some(condition) = &breakpoint.condition {
+        //         lines.extend(
+        //             Text::styled(condition, warning.add_modifier(Modifier::UNDERLINED)).lines,
+        //         );
+        //     }
+        //     if let Some(log_message) = &breakpoint.log_message {
+        //         lines.extend(
+        //             Text::styled(log_message, info.add_modifier(Modifier::UNDERLINED)).lines,
+        //         );
+        //     }
+        // }
 
         let paragraph = Paragraph::new(lines)
             .alignment(Alignment::Right)