dap: refactor frame handling

5 files changed, 138 insertions, 118 deletions

diff --git a/helix-dap/src/client.rs b/helix-dap/src/client.rs
index b0e43154..72f7e12c 100644
--- a/helix-dap/src/client.rs
+++ b/helix-dap/src/client.rs
@@ -29,10 +29,12 @@ pub struct Client {
     pub caps: Option<DebuggerCapabilities>,
     //
     pub breakpoints: HashMap<PathBuf, Vec<SourceBreakpoint>>,
-    // TODO: multiple threads support
-    pub stack_pointer: Option<StackFrame>,
+    // thread_id -> frames
+    pub stack_frames: HashMap<usize, Vec<StackFrame>>,
     pub thread_id: Option<usize>,
-    pub is_running: bool,
+    /// Currently active frame for the current thread.
+    pub active_frame: Option<usize>,
+    pub is_running: bool, // TODO: track is_running per thread
 }
 
 impl Client {
@@ -76,8 +78,9 @@ impl Client {
             caps: None,
             //
             breakpoints: HashMap::new(),
-            stack_pointer: None,
+            stack_frames: HashMap::new(),
             thread_id: None,
+            active_frame: None,
             is_running: false,
         };
 
diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index 25ed8a0a..1c30c343 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -3,14 +3,7 @@ use helix_dap::Payload;
 use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap};
 use helix_view::{theme, Editor};
 
-use crate::{
-    args::Args,
-    commands::{align_view, Align},
-    compositor::Compositor,
-    config::Config,
-    job::Jobs,
-    ui,
-};
+use crate::{args::Args, compositor::Compositor, config::Config, job::Jobs, ui};
 
 use log::error;
 use std::{
@@ -263,6 +256,7 @@ impl Application {
     }
 
     pub async fn handle_debugger_message(&mut self, payload: helix_dap::Payload) {
+        use crate::commands::dap::select_thread_id;
         use helix_dap::{events, Event};
         let mut debugger = match self.editor.debugger.as_mut() {
             Some(debugger) => debugger,
@@ -281,12 +275,9 @@ impl Application {
                 }) => {
                     debugger.is_running = false;
 
-                    // whichever thread stops is made "current".
-                    debugger.thread_id = thread_id;
-
+                    // whichever thread stops is made "current" (if no previously selected thread).
                     if let Some(thread_id) = thread_id {
-                        let (bt, _) = debugger.stack_trace(thread_id).await.unwrap();
-                        debugger.stack_pointer = bt.get(0).cloned();
+                        select_thread_id(&mut self.editor, thread_id, false).await;
                     }
 
                     let scope = match thread_id {
@@ -305,52 +296,6 @@ impl Application {
                         status.push_str(" (all threads stopped)");
                     }
 
-                    if let Some(helix_dap::StackFrame {
-                        source:
-                            Some(helix_dap::Source {
-                                path: Some(ref path),
-                                ..
-                            }),
-                        line,
-                        column,
-                        end_line,
-                        end_column,
-                        ..
-                    }) = debugger.stack_pointer
-                    {
-                        let path = path.clone();
-                        self.editor
-                            .open(path, helix_view::editor::Action::Replace)
-                            .unwrap(); // TODO: there should be no unwrapping!
-
-                        let (view, doc) = current!(self.editor);
-
-                        fn dap_pos_to_pos(
-                            doc: &helix_core::Rope,
-                            line: usize,
-                            column: usize,
-                        ) -> Option<usize> {
-                            // 1-indexing to 0 indexing
-                            let line = doc.try_line_to_char(line - 1).ok()?;
-                            let pos = line + column;
-                            // TODO: this is probably utf-16 offsets
-                            Some(pos)
-                        }
-
-                        let text_end = doc.text().len_chars().saturating_sub(1);
-                        let start = dap_pos_to_pos(doc.text(), line, column).unwrap_or(0);
-
-                        let selection = if let Some(end_line) = end_line {
-                            let end = dap_pos_to_pos(doc.text(), end_line, end_column.unwrap_or(0))
-                                .unwrap_or(0);
-
-                            Selection::single(start.min(text_end), end.min(text_end))
-                        } else {
-                            Selection::point(start.min(text_end))
-                        };
-                        doc.set_selection(view.id, selection);
-                        align_view(doc, view, Align::Center);
-                    }
                     self.editor.set_status(status);
                 }
                 Event::Output(events::Output {
@@ -373,8 +318,9 @@ impl Application {
                         .set_status("Debugged application started".to_owned());
                 }
                 Event::Continued(_) => {
-                    debugger.stack_pointer = None;
                     debugger.is_running = true;
+                    debugger.active_frame = None;
+                    debugger.thread_id = None;
                 }
                 ev => {
                     log::warn!("Unhandled event {:?}", ev);
diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index ffaaac55..860cacc6 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -1,4 +1,4 @@
-mod dap;
+pub(crate) mod dap;
 
 pub use dap::*;
 
@@ -1955,8 +1955,17 @@ mod cmd {
     ) -> anyhow::Result<()> {
         use helix_lsp::block_on;
         if let Some(debugger) = cx.editor.debugger.as_mut() {
-            let id = debugger.stack_pointer.clone().map(|x| x.id);
-            let response = block_on(debugger.eval(args.join(" "), id))?;
+            let (frame, thread_id) = match (debugger.active_frame, debugger.thread_id) {
+                (Some(frame), Some(thread_id)) => (frame, thread_id),
+                _ => {
+                    bail!("Cannot find current stack frame to access variables")
+                }
+            };
+
+            // TODO: support no frame_id
+
+            let frame_id = debugger.stack_frames[&thread_id][frame].id;
+            let response = block_on(debugger.eval(args.join(" "), Some(frame_id)))?;
             cx.editor.set_status(response.result);
         }
         Ok(())
@@ -1966,7 +1975,7 @@ mod cmd {
         cx: &mut compositor::Context,
         condition: Option<String>,
         log_message: Option<String>,
-    ) {
+    ) -> anyhow::Result<()> {
         use helix_lsp::block_on;
 
         let (view, doc) = current!(cx.editor);
@@ -1981,38 +1990,29 @@ mod cmd {
         let path = match doc.path() {
             Some(path) => path.to_path_buf(),
             None => {
-                cx.editor
-                    .set_error("Can't edit breakpoint: document has no path".to_string());
-                return;
+                bail!("Can't edit breakpoint: document has no path")
             }
         };
         if let Some(debugger) = &mut cx.editor.debugger {
             if breakpoint.condition.is_some()
                 && !debugger
                     .caps
-                    .clone()
+                    .as_ref()
                     .unwrap()
                     .supports_conditional_breakpoints
                     .unwrap_or_default()
             {
-                cx.editor.set_error(
-                    "Can't edit breakpoint: debugger does not support conditional breakpoints"
-                        .to_string(),
-                );
-                return;
+                bail!("Can't edit breakpoint: debugger does not support conditional breakpoints")
             }
             if breakpoint.log_message.is_some()
                 && !debugger
                     .caps
-                    .clone()
+                    .as_ref()
                     .unwrap()
                     .supports_log_points
                     .unwrap_or_default()
             {
-                cx.editor.set_error(
-                    "Can't edit breakpoint: debugger does not support logpoints".to_string(),
-                );
-                return;
+                bail!("Can't edit breakpoint: debugger does not support logpoints")
             }
 
             let breakpoints = debugger.breakpoints.entry(path.clone()).or_default();
@@ -2024,11 +2024,11 @@ mod cmd {
 
                 let request = debugger.set_breakpoints(path, breakpoints);
                 if let Err(e) = block_on(request) {
-                    cx.editor
-                        .set_error(format!("Failed to set breakpoints: {:?}", e));
+                    bail!("Failed to set breakpoints: {:?}", e)
                 }
             }
         }
+        Ok(())
     }
 
     fn debug_start(
@@ -2076,8 +2076,7 @@ mod cmd {
             Some(condition)
         };
 
-        edit_breakpoint_impl(cx, condition, None);
-        Ok(())
+        edit_breakpoint_impl(cx, condition, None)
     }
 
     fn debug_set_logpoint(
@@ -2092,8 +2091,7 @@ mod cmd {
             Some(log_message)
         };
 
-        edit_breakpoint_impl(cx, None, log_message);
-        Ok(())
+        edit_breakpoint_impl(cx, None, log_message)
     }
 
     pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[
diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs
index 66127360..a1558dde 100644
--- a/helix-term/src/commands/dap.rs
+++ b/helix-term/src/commands/dap.rs
@@ -1,5 +1,6 @@
-use super::{Context, Editor};
+use super::{align_view, Align, Context, Editor};
 use crate::ui::Picker;
+use helix_core::Selection;
 use helix_dap::Client;
 use helix_lsp::block_on;
 
@@ -8,6 +9,68 @@ use tokio_stream::wrappers::UnboundedReceiverStream;
 
 use std::collections::HashMap;
 
+// general utils:
+pub fn dap_pos_to_pos(doc: &helix_core::Rope, line: usize, column: usize) -> Option<usize> {
+    // 1-indexing to 0 indexing
+    let line = doc.try_line_to_char(line - 1).ok()?;
+    let pos = line + column;
+    // TODO: this is probably utf-16 offsets
+    Some(pos)
+}
+
+pub async fn select_thread_id(editor: &mut Editor, thread_id: usize, force: bool) {
+    let debugger = match &mut editor.debugger {
+        Some(debugger) => debugger,
+        None => return,
+    };
+
+    if !force && debugger.thread_id.is_some() {
+        return;
+    }
+
+    debugger.thread_id = Some(thread_id);
+
+    // fetch stack trace
+    // TODO: handle requesting more total frames
+    let (frames, _) = debugger.stack_trace(thread_id).await.unwrap();
+    debugger.stack_frames.insert(thread_id, frames);
+    debugger.active_frame = Some(0); // TODO: check how to determine this
+
+    let frame = debugger.stack_frames[&thread_id].get(0).cloned();
+    if let Some(frame) = &frame {
+        jump_to_stack_frame(editor, frame);
+    }
+}
+
+pub fn jump_to_stack_frame(editor: &mut Editor, frame: &helix_dap::StackFrame) {
+    let path = if let Some(helix_dap::Source {
+        path: Some(ref path),
+        ..
+    }) = frame.source
+    {
+        path.clone()
+    } else {
+        return;
+    };
+
+    editor
+        .open(path, helix_view::editor::Action::Replace)
+        .unwrap(); // TODO: there should be no unwrapping!
+
+    let (view, doc) = current!(editor);
+
+    let text_end = doc.text().len_chars().saturating_sub(1);
+    let start = dap_pos_to_pos(doc.text(), frame.line, frame.column).unwrap_or(0);
+    let end = frame
+        .end_line
+        .and_then(|end_line| dap_pos_to_pos(doc.text(), end_line, frame.end_column.unwrap_or(0)))
+        .unwrap_or(start);
+
+    let selection = Selection::single(start.min(text_end), end.min(text_end));
+    doc.set_selection(view.id, selection);
+    align_view(doc, view, Align::Center);
+}
+
 // DAP
 pub fn dap_start_impl(
     editor: &mut Editor,
@@ -222,13 +285,18 @@ pub fn dap_continue(cx: &mut Context) {
             return;
         }
 
-        let request = debugger.continue_thread(debugger.thread_id.unwrap());
-        if let Err(e) = block_on(request) {
-            cx.editor.set_error(format!("Failed to continue: {:?}", e));
-            return;
+        if let Some(thread_id) = debugger.thread_id {
+            let request = debugger.continue_thread(debugger.thread_id.unwrap());
+            if let Err(e) = block_on(request) {
+                cx.editor.set_error(format!("Failed to continue: {:?}", e));
+                return;
+            }
+            debugger.is_running = true;
+            debugger.stack_frames.remove(&thread_id);
+        } else {
+            cx.editor
+                .set_error("Currently active thread is not stopped. Switch the thread.".into());
         }
-        debugger.is_running = true;
-        debugger.stack_pointer = None;
     }
 }
 
@@ -299,13 +367,16 @@ pub fn dap_variables(cx: &mut Context) {
                 .set_status("Cannot access variables while target is running".to_owned());
             return;
         }
-        if debugger.stack_pointer.is_none() {
-            cx.editor
-                .set_status("Cannot find current stack pointer to access variables".to_owned());
-            return;
-        }
+        let (frame, thread_id) = match (debugger.active_frame, debugger.thread_id) {
+            (Some(frame), Some(thread_id)) => (frame, thread_id),
+            _ => {
+                cx.editor
+                    .set_status("Cannot find current stack frame to access variables".to_owned());
+                return;
+            }
+        };
 
-        let frame_id = debugger.stack_pointer.clone().unwrap().id;
+        let frame_id = debugger.stack_frames[&thread_id][frame].id;
         let scopes = match block_on(debugger.scopes(frame_id)) {
             Ok(s) => s,
             Err(e) => {
@@ -366,10 +437,7 @@ pub fn dap_switch_thread(cx: &mut Context) {
             threads,
             |thread| thread.name.clone().into(),
             |editor, thread, _action| {
-                if let Some(debugger) = &mut editor.debugger {
-                    debugger.thread_id = Some(thread.id);
-                    // TODO: probably need to refetch stack frames?
-                }
+                block_on(select_thread_id(editor, thread.id, true));
             },
         );
         cx.push_layer(Box::new(picker))
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index bb183b3a..e13eb323 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -446,12 +446,16 @@ impl EditorView {
             .map(|range| range.cursor_line(text))
             .collect();
 
-        let mut breakpoints: Option<Vec<SourceBreakpoint>> = None;
-        let mut stack_pointer: Option<StackFrame> = None;
+        let mut breakpoints: Option<&Vec<SourceBreakpoint>> = None;
+        let mut stack_frame: Option<&StackFrame> = None;
         if let Some(debugger) = debugger {
             if let Some(path) = doc.path() {
-                breakpoints = debugger.breakpoints.get(path).cloned();
-                stack_pointer = debugger.stack_pointer.clone()
+                breakpoints = debugger.breakpoints.get(path);
+
+                if let (Some(frame), Some(thread_id)) = (debugger.active_frame, debugger.thread_id)
+                {
+                    stack_frame = debugger.stack_frames[&thread_id].get(frame); // TODO: drop the clone..
+                };
             }
         }
 
@@ -477,23 +481,24 @@ impl EditorView {
             if let Some(bps) = breakpoints.as_ref() {
                 if let Some(breakpoint) = bps.iter().find(|breakpoint| breakpoint.line - 1 == line)
                 {
-                    if breakpoint.condition.is_some() {
-                        surface.set_stringn(viewport.x, viewport.y + i as u16, "▲", 1, error);
+                    let style = if breakpoint.condition.is_some() {
+                        error
                     } else if breakpoint.log_message.is_some() {
-                        surface.set_stringn(viewport.x, viewport.y + i as u16, "▲", 1, info);
+                        info
                     } else {
-                        surface.set_stringn(viewport.x, viewport.y + i as u16, "▲", 1, warning);
-                    }
+                        warning
+                    };
+                    surface.set_stringn(viewport.x, viewport.y + i as u16, "▲", 1, style);
                 }
             }
 
-            if let Some(sp) = stack_pointer.as_ref() {
-                if let Some(src) = sp.source.as_ref() {
+            if let Some(frame) = stack_frame {
+                if let Some(src) = &frame.source {
                     if doc
                         .path()
-                        .map(|path| src.path == Some(path.clone()))
+                        .map(|path| src.path.as_ref() == Some(path))
                         .unwrap_or(false)
-                        && sp.line - 1 == line
+                        && frame.line - 1 == line
                     {
                         surface.set_style(
                             Rect::new(viewport.x, viewport.y + i as u16, 6, 1),