From 2c7b75475f9534ca330cccea4778a3878fed6b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Mon, 30 Aug 2021 10:58:15 +0900 Subject: [PATCH] dap: refactor frame handling --- helix-dap/src/client.rs | 11 ++-- helix-term/src/application.rs | 66 ++------------------- helix-term/src/commands.rs | 46 +++++++-------- helix-term/src/commands/dap.rs | 102 +++++++++++++++++++++++++++------ helix-term/src/ui/editor.rs | 31 +++++----- 5 files changed, 138 insertions(+), 118 deletions(-) diff --git a/helix-dap/src/client.rs b/helix-dap/src/client.rs index b0e43154d..72f7e12c8 100644 --- a/helix-dap/src/client.rs +++ b/helix-dap/src/client.rs @@ -29,10 +29,12 @@ pub struct Client { pub caps: Option, // pub breakpoints: HashMap>, - // TODO: multiple threads support - pub stack_pointer: Option, + // thread_id -> frames + pub stack_frames: HashMap>, pub thread_id: Option, - pub is_running: bool, + /// Currently active frame for the current thread. + pub active_frame: Option, + 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 25ed8a0aa..1c30c343f 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 { - // 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 ffaaac55e..860cacc6e 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, log_message: Option, - ) { + ) -> 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 66127360b..a1558dde6 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 { + // 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 bb183b3a7..e13eb3231 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> = None; - let mut stack_pointer: Option = None; + let mut breakpoints: Option<&Vec> = 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),