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

This commit is contained in:
Blaž Hrastnik 2021-11-30 17:52:39 +09:00
parent 3633f85b38
commit 8ffafb826f
6 changed files with 163 additions and 181 deletions

View file

@ -35,7 +35,6 @@ pub struct Client {
pub thread_id: Option<ThreadId>,
/// Currently active frame for the current thread.
pub active_frame: Option<usize>,
pub breakpoints: Vec<Breakpoint>,
pub quirks: DebuggerQuirks,
}
@ -81,7 +80,6 @@ impl Client {
thread_states: HashMap::new(),
thread_id: None,
active_frame: None,
breakpoints: vec![],
quirks: DebuggerQuirks::default(),
};

View file

@ -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)

View file

@ -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(..);

View file

@ -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));
}
},
);

View file

@ -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)

View file

@ -156,6 +156,19 @@ impl std::fmt::Debug for Motion {
}
}
#[derive(Debug, Clone, Default)]
pub struct Breakpoint {
pub id: Option<usize>,
pub verified: bool,
pub message: Option<String>,
pub line: usize,
pub column: Option<usize>,
pub condition: Option<String>,
pub hit_condition: Option<String>,
pub log_message: Option<String>,
}
#[derive(Debug)]
pub struct Editor {
pub tree: Tree,
@ -169,7 +182,7 @@ pub struct Editor {
pub debugger: Option<dap::Client>,
pub debugger_events: SelectAll<UnboundedReceiverStream<dap::Payload>>,
pub breakpoints: HashMap<PathBuf, Vec<dap::SourceBreakpoint>>,
pub breakpoints: HashMap<PathBuf, Vec<Breakpoint>>,
pub clipboard_provider: Box<dyn ClipboardProvider>,