8ffafb8 · helix · git.iko.soy

git.iko.soy/helix

on master help sign in

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

8ffafb8Blaž Hrastnik5 years ago

helix-dap/src/client.rs−2

compare with master

unifiedsplit

@@ -35,7 +35,6 @@ pub struct Client {

35 pub thread_id: Option<ThreadId>,

36 /// Currently active frame for the current thread.

37 pub active_frame: Option<usize>,

38− pub breakpoints: Vec<Breakpoint>,

39 pub quirks: DebuggerQuirks,

38 pub quirks: DebuggerQuirks,

40}

39}

@@ -81,7 +80,6 @@ impl Client {

81 thread_states: HashMap::new(),

80 thread_states: HashMap::new(),

82 thread_id: None,

81 thread_id: None,

83 active_frame: None,

82 active_frame: None,

84− breakpoints: vec![],

85 quirks: DebuggerQuirks::default(),

83 quirks: DebuggerQuirks::default(),

86 };

84 };

helix-term/src/application.rs+27−42

@@ -1,7 +1,7 @@

1use helix_core::{merge_toml_values, syntax};

2use helix_dap::Payload;

3use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap};

4−use helix_view::{theme, Editor};

4+use helix_view::{editor::Breakpoint, theme, Editor};

6use crate::{

7 args::Args, commands::fetch_stack_trace, compositor::Compositor, config::Config, job::Jobs, ui,

@@ -319,7 +319,7 @@ impl Application {

319 }

320

321 pub async fn handle_debugger_message(&mut self, payload: helix_dap::Payload) {

322− use crate::commands::dap::{resume_application, select_thread_id};

322+ use crate::commands::dap::{breakpoints_changed, resume_application, select_thread_id};

323 use helix_dap::{events, Event};

324 let debugger = match self.editor.debugger.as_mut() {

325 Some(debugger) => debugger,

@@ -388,46 +388,35 @@ impl Application {

388 }

389 Event::Breakpoint(events::Breakpoint { reason, breakpoint }) => match &reason[..] {

390 "new" => {

391+ self.editor

391− debugger.breakpoints.push(breakpoint);

392+ .breakpoints

393+ .entry(breakpoint.source.unwrap().path.unwrap()) // TODO: no unwraps

394+ .or_default()

395+ .push(Breakpoint {

396+ id: breakpoint.id,

397+ verified: breakpoint.verified,

398+ message: breakpoint.message,

399+ line: breakpoint.line.unwrap().saturating_sub(1), // TODO: no unwrap

400+ column: breakpoint.column,

401+ ..Default::default()

402+ });

392 }

403 }

393 "changed" => {

404 "changed" => {

394− match debugger

405+ for breakpoints in self.editor.breakpoints.values_mut() {

395− .breakpoints

396− .iter()

397− .position(|b| b.id == breakpoint.id)

406+ if let Some(i) = breakpoints.iter().position(|b| b.id == breakpoint.id)

398− {

407+ {

399− Some(i) => {

400− debugger.breakpoints[i] = breakpoint;

408+ breakpoints[i].verified = breakpoint.verified;

401− // let item = debugger.breakpoints.get_mut(i).unwrap();

409+ breakpoints[i].message = breakpoint.message.clone();

402− // item.verified = breakpoint.verified;

410+ breakpoints[i].line = breakpoint.line.unwrap().saturating_sub(1); // TODO: no unwrap

403− // // TODO: wasteful clones

404− // item.message = breakpoint.message.or_else(|| item.message.clone());

405− // item.source = breakpoint.source.or_else(|| item.source.clone());

406− // item.line = breakpoint.line.or(item.line);

407− // item.column = breakpoint.column.or(item.column);

411+ breakpoints[i].column = breakpoint.column;

408− // item.end_line = breakpoint.end_line.or(item.end_line);

409− // item.end_column = breakpoint.end_column.or(item.end_column);

410− // item.instruction_reference = breakpoint

411− // .instruction_reference

412− // .or_else(|| item.instruction_reference.clone());

413− // item.offset = breakpoint.offset.or(item.offset);

414− }

415− None => {

416− warn!("Changed breakpoint with id {:?} not found", breakpoint.id);

417 }

412 }

418 }

413 }

419 }

414 }

420 "removed" => {

415 "removed" => {

421− match debugger

416+ for breakpoints in self.editor.breakpoints.values_mut() {

422− .breakpoints

423− .iter()

424− .position(|b| b.id == breakpoint.id)

417+ if let Some(i) = breakpoints.iter().position(|b| b.id == breakpoint.id)

425− {

418+ {

426− Some(i) => {

427− debugger.breakpoints.remove(i);

419+ breakpoints.remove(i);

428− }

429− None => {

430− warn!("Removed breakpoint with id {:?} not found", breakpoint.id);

431 }

420 }

432 }

421 }

433 }

422 }

@@ -453,13 +442,9 @@ impl Application {

453 }

442 }

454 Event::Initialized => {

443 Event::Initialized => {

455 // send existing breakpoints

444 // send existing breakpoints

456− for (path, breakpoints) in &self.editor.breakpoints {

445+ for (path, breakpoints) in &mut self.editor.breakpoints {

457 // TODO: call futures in parallel, await all

446 // TODO: call futures in parallel, await all

458− debugger.breakpoints = debugger

459− .set_breakpoints(path.clone(), breakpoints.clone())

447+ let _ = breakpoints_changed(debugger, path.clone(), breakpoints);

460− .await

461− .unwrap()

462− .unwrap();

463 }

448 }

464 // TODO: fetch breakpoints (in case we're attaching)

449 // TODO: fetch breakpoints (in case we're attaching)

465

450

helix-term/src/commands.rs+2−4

@@ -1897,8 +1897,8 @@ fn append_mode(cx: &mut Context) {

1897mod cmd {

1898 use super::*;

1899

1900− use helix_dap::SourceBreakpoint;

1901 use helix_view::editor::Action;

1900 use helix_view::editor::Action;

1901+ use helix_view::editor::Breakpoint;

1902 use ui::completers::{self, Completer};

1903

1904 #[derive(Clone)]

@@ -2563,9 +2563,7 @@ mod cmd {

2563 Ok(())

2564 }

2565

2566− pub fn get_breakpoint_at_current_line(

2566+ pub fn get_breakpoint_at_current_line(editor: &mut Editor) -> Option<(usize, Breakpoint)> {

2567− editor: &mut Editor,

2568− ) -> Option<(usize, SourceBreakpoint)> {

2569 let (view, doc) = current!(editor);

2567 let (view, doc) = current!(editor);

2570 let text = doc.text().slice(..);

2568 let text = doc.text().slice(..);

2571

2569

helix-term/src/commands/dap.rs+89−80

@@ -11,11 +11,13 @@ use helix_core::{

11};

12use helix_dap::{self as dap, Client, ThreadId};

13use helix_lsp::block_on;

14+use helix_view::editor::Breakpoint;

15use serde_json::{to_value, Value};

16use serde_json::{to_value, Value};

16use tokio_stream::wrappers::UnboundedReceiverStream;

17use tokio_stream::wrappers::UnboundedReceiverStream;

18use std::collections::HashMap;

19use std::collections::HashMap;

20+use std::path::PathBuf;

20// general utils:

22// general utils:

21pub fn dap_pos_to_pos(doc: &helix_core::Rope, line: usize, column: usize) -> Option<usize> {

23pub 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) {

406 dap_toggle_breakpoint_impl(cx, path, line);

408 dap_toggle_breakpoint_impl(cx, path, line);

407}

409}

408

410

409−pub fn dap_toggle_breakpoint_impl(cx: &mut Context, path: std::path::PathBuf, line: usize) {

411+pub fn breakpoints_changed(

412+ debugger: &mut dap::Client,

413+ path: PathBuf,

414+ breakpoints: &mut [Breakpoint],

415+) -> Result<(), anyhow::Error> {

416+ // TODO: handle capabilities correctly again, by filterin breakpoints when emitting

417+ // if breakpoint.condition.is_some()

418+ // && !debugger

419+ // .caps

420+ // .as_ref()

421+ // .unwrap()

422+ // .supports_conditional_breakpoints

423+ // .unwrap_or_default()

424+ // {

425+ // bail!(

426+ // "Can't edit breakpoint: debugger does not support conditional breakpoints"

427+ // )

428+ // }

429+ // if breakpoint.log_message.is_some()

430+ // && !debugger

431+ // .caps

432+ // .as_ref()

433+ // .unwrap()

434+ // .supports_log_points

435+ // .unwrap_or_default()

436+ // {

437+ // bail!("Can't edit breakpoint: debugger does not support logpoints")

438+ // }

439+ let source_breakpoints = breakpoints

440+ .iter()

410− let breakpoint = helix_dap::SourceBreakpoint {

441+ .map(|breakpoint| helix_dap::SourceBreakpoint {

411− line: line + 1, // convert from 0-indexing to 1-indexing (TODO: could set debugger to 0-indexing on init)

442+ line: breakpoint.line + 1, // convert from 0-indexing to 1-indexing (TODO: could set debugger to 0-indexing on init)

412− ..Default::default()

443+ ..Default::default()

444+ })

445+ .collect::<Vec<_>>();

446+

447+ let request = debugger.set_breakpoints(path, source_breakpoints);

448+ match block_on(request) {

449+ Ok(Some(dap_breakpoints)) => {

450+ for (breakpoint, dap_breakpoint) in breakpoints.iter_mut().zip(dap_breakpoints) {

451+ breakpoint.id = dap_breakpoint.id;

452+ breakpoint.verified = dap_breakpoint.verified;

453+ breakpoint.message = dap_breakpoint.message;

454+ // TODO: handle breakpoint.message

455+ // TODO: verify source matches

456+ breakpoint.line = dap_breakpoint.line.unwrap_or(0).saturating_sub(1); // convert to 0-indexing

457+ // TODO: no unwrap

458+ breakpoint.column = dap_breakpoint.column;

459+ // TODO: verify end_linef/col instruction reference, offset

460+ }

461+ }

462+ Err(e) => anyhow::bail!("Failed to set breakpoints: {}", e),

463+ _ => {}

413 };

464 };

465+ Ok(())

466+}

414

467

468+pub fn dap_toggle_breakpoint_impl(cx: &mut Context, path: PathBuf, line: usize) {

415 // TODO: need to map breakpoints over edits and update them?

469 // TODO: need to map breakpoints over edits and update them?

416 // we shouldn't really allow editing while debug is running though

470 // we shouldn't really allow editing while debug is running though

417

471

418 let breakpoints = cx.editor.breakpoints.entry(path.clone()).or_default();

472 let breakpoints = cx.editor.breakpoints.entry(path.clone()).or_default();

419− // TODO: always keep breakpoints sorted and use binary search

473+ // TODO: always keep breakpoints sorted and use binary search to determine insertion point

420− if let Some(pos) = breakpoints.iter().position(|b| b.line == breakpoint.line) {

474+ if let Some(pos) = breakpoints

475+ .iter()

476+ .position(|breakpoint| breakpoint.line == line)

477+ {

421 breakpoints.remove(pos);

478 breakpoints.remove(pos);

422 } else {

479 } else {

423− breakpoints.push(breakpoint);

480+ breakpoints.push(Breakpoint {

481+ line,

482+ ..Default::default()

483+ });

424 }

484 }

425

485

426− let breakpoints = breakpoints.clone();

427−

428 let debugger = match &mut cx.editor.debugger {

486 let debugger = match &mut cx.editor.debugger {

429 Some(debugger) => debugger,

487 Some(debugger) => debugger,

430 None => return,

488 None => return,

431 };

489 };

432− let request = debugger.set_breakpoints(path, breakpoints);

490+

433− match block_on(request) {

491+ if let Err(e) = breakpoints_changed(debugger, path, breakpoints) {

434− Ok(Some(breakpoints)) => {

435− // TODO: handle breakpoint.message

436− debugger.breakpoints = breakpoints;

437− }

438− Err(e) => cx

439− .editor

492+ cx.editor

440− .set_error(format!("Failed to set breakpoints: {}", e)),

493+ .set_error(format!("Failed to set breakpoints: {}", e));

441− _ => {}

442− };

494+ }

443}

495}

444

496

445pub fn dap_continue(cx: &mut Context) {

497pub fn dap_continue(cx: &mut Context) {

@@ -651,36 +703,15 @@ pub fn dap_edit_condition(cx: &mut Context) {

651 input => Some(input.to_owned()),

703 input => Some(input.to_owned()),

652 };

704 };

653

705

654− if let Some(debugger) = &mut cx.editor.debugger {

706+ let debugger = match &mut cx.editor.debugger {

655− // TODO: handle capabilities correctly again, by filterin breakpoints when emitting

656− // if breakpoint.condition.is_some()

657− // && !debugger

707+ Some(debugger) => debugger,

658− // .caps

708+ None => return,

659− // .as_ref()

660− // .unwrap()

661− // .supports_conditional_breakpoints

662− // .unwrap_or_default()

663− // {

664− // bail!(

665− // "Can't edit breakpoint: debugger does not support conditional breakpoints"

666− // )

667− // }

668− // if breakpoint.log_message.is_some()

669− // && !debugger

670− // .caps

671− // .as_ref()

672− // .unwrap()

673− // .supports_log_points

674− // .unwrap_or_default()

675− // {

676− // bail!("Can't edit breakpoint: debugger does not support logpoints")

677− // }

709+ };

678− let request =

710+

679− debugger.set_breakpoints(path.clone(), breakpoints.clone());

711+ if let Err(e) = breakpoints_changed(debugger, path.clone(), breakpoints)

680− if let Err(e) = block_on(request) {

712+ {

681− cx.editor

713+ cx.editor

682− .set_status(format!("Failed to set breakpoints: {}", e))

714+ .set_error(format!("Failed to set breakpoints: {}", e));

683− }

684 }

715 }

685 },

716 },

686 );

717 );

@@ -719,36 +750,14 @@ pub fn dap_edit_log(cx: &mut Context) {

719 input => Some(input.to_owned()),

750 input => Some(input.to_owned()),

720 };

751 };

721

752

722− if let Some(debugger) = &mut cx.editor.debugger {

753+ let debugger = match &mut cx.editor.debugger {

723− // TODO: handle capabilities correctly again, by filterin breakpoints when emitting

724− // if breakpoint.condition.is_some()

725− // && !debugger

726− // .caps

727− // .as_ref()

728− // .unwrap()

729− // .supports_conditional_breakpoints

730− // .unwrap_or_default()

731− // {

732− // bail!(

733− // "Can't edit breakpoint: debugger does not support conditional breakpoints"

734− // )

735− // }

736− // if breakpoint.log_message.is_some()

737− // && !debugger

754+ Some(debugger) => debugger,

738− // .caps

755+ None => return,

739− // .as_ref()

740− // .unwrap()

741− // .supports_log_points

742− // .unwrap_or_default()

743− // {

744− // bail!("Can't edit breakpoint: debugger does not support logpoints")

745− // }

756+ };

746− let request =

757+ if let Err(e) = breakpoints_changed(debugger, path.clone(), breakpoints)

747− debugger.set_breakpoints(path.clone(), breakpoints.clone());

758+ {

748− if let Err(e) = block_on(request) {

749− cx.editor

759+ cx.editor

750− .set_status(format!("Failed to set breakpoints: {}", e))

760+ .set_error(format!("Failed to set breakpoints: {}", e));

751− }

752 }

761 }

753 },

762 },

754 );

763 );

helix-term/src/ui/editor.rs+30−51

@@ -15,7 +15,6 @@ use helix_core::{

15 unicode::width::UnicodeWidthStr,

16 LineEnding, Position, Range, Selection,

17};

18−use helix_dap::{Breakpoint, SourceBreakpoint, StackFrame};

19use helix_view::{

18use helix_view::{

20 document::{Mode, SCRATCH_BUFFER_NAME},

19 document::{Mode, SCRATCH_BUFFER_NAME},

21 graphics::{Color, CursorKind, Modifier, Rect, Style},

20 graphics::{Color, CursorKind, Modifier, Rect, Style},

@@ -24,7 +23,7 @@ use helix_view::{

24 keyboard::{KeyCode, KeyModifiers},

23 keyboard::{KeyCode, KeyModifiers},

25 Document, Editor, Theme, View,

24 Document, Editor, Theme, View,

26};

25};

27−use std::{borrow::Cow, collections::HashMap, path::PathBuf};

26+use std::borrow::Cow;

29use crossterm::event::{Event, MouseButton, MouseEvent, MouseEventKind};

28use crossterm::event::{Event, MouseButton, MouseEvent, MouseEventKind};

30use tui::buffer::Buffer as Surface;

29use tui::buffer::Buffer as Surface;

@@ -74,7 +73,6 @@ impl EditorView {

74 let inner = view.inner_area();

73 let inner = view.inner_area();

75 let area = view.area;

74 let area = view.area;

76 let theme = &editor.theme;

75 let theme = &editor.theme;

77− let all_breakpoints = HashMap::new();

79 let highlights = Self::doc_syntax_highlights(doc, view.offset, inner.height, theme, loader);

77 let highlights = Self::doc_syntax_highlights(doc, view.offset, inner.height, theme, loader);

80 let highlights = syntax::merge(highlights, Self::doc_diagnostics_highlights(doc, theme));

78 let highlights = syntax::merge(highlights, Self::doc_diagnostics_highlights(doc, theme));

@@ -107,7 +105,7 @@ impl EditorView {

107 }

105 }

108 }

106 }

109

107

110− self.render_diagnostics(doc, view, inner, surface, theme, &all_breakpoints);

108+ self.render_diagnostics(doc, view, inner, surface, theme);

111

109

112 let statusline_area = view

110 let statusline_area = view

113 .area

111 .area

@@ -436,39 +434,21 @@ impl EditorView {

436 let error = theme.get("error");

434 let error = theme.get("error");

437 let info = theme.get("info");

435 let info = theme.get("info");

438

436

439− let all_breakpoints = &editor.breakpoints;

440−

441− // TODO: debugger breakpoints should keep editor level breakpoints in sync

442−

443 let breakpoints = doc

437 let breakpoints = doc

444 .path()

438 .path()

445− .and_then(|path| all_breakpoints.get(path))

439+ .and_then(|path| editor.breakpoints.get(path))

446 .unwrap();

440 .unwrap();

447

441

448 Box::new(move |line: usize, _selected: bool, out: &mut String| {

442 Box::new(move |line: usize, _selected: bool, out: &mut String| {

449− // TODO: debugger should translate received breakpoints to 0-indexing

450− let breakpoint = breakpoints.iter().find(|breakpoint| {

443+ let breakpoint = breakpoints

451− false

444+ .iter()

452− // if breakpoint.source == doc.path() {

453− // match (breakpoint.line, breakpoint.end_line) {

445+ .find(|breakpoint| breakpoint.line == line);

454− // #[allow(clippy::int_plus_one)]

455− // (Some(l), Some(el)) => l - 1 <= line && line <= el - 1,

456− // (Some(l), None) => l - 1 == line,

457− // _ => false,

458− // }

459− // } else {

460− // false

461− // }

462− });

463

446

464 let breakpoint = match breakpoint {

447 let breakpoint = match breakpoint {

465 Some(b) => b,

448 Some(b) => b,

466 None => return None,

449 None => return None,

467 };

450 };

468

451

469− // let verified = breakpoint.verified;

470− let verified = false;

471−

472 let mut style =

452 let mut style =

473 if breakpoint.condition.is_some() && breakpoint.log_message.is_some() {

453 if breakpoint.condition.is_some() && breakpoint.log_message.is_some() {

474 error.add_modifier(Modifier::UNDERLINED)

454 error.add_modifier(Modifier::UNDERLINED)

@@ -480,7 +460,7 @@ impl EditorView {

480 warning

460 warning

481 };

461 };

482

462

483− if !verified {

463+ if !breakpoint.verified {

484 // Faded colors

464 // Faded colors

485 style = if let Some(Color::Rgb(r, g, b)) = style.fg {

465 style = if let Some(Color::Rgb(r, g, b)) = style.fg {

486 style.fg(Color::Rgb(

466 style.fg(Color::Rgb(

@@ -495,7 +475,7 @@ impl EditorView {

495

475

496 // TODO: also handle breakpoints only present in the user struct

476 // TODO: also handle breakpoints only present in the user struct

497 use std::fmt::Write;

477 use std::fmt::Write;

498− let sym = if verified { "▲" } else { "⊚" };

478+ let sym = if breakpoint.verified { "▲" } else { "⊚" };

499 write!(out, "{}", sym).unwrap();

479 write!(out, "{}", sym).unwrap();

500 Some(style)

480 Some(style)

501 })

481 })

@@ -567,7 +547,6 @@ impl EditorView {

567 viewport: Rect,

547 viewport: Rect,

568 surface: &mut Surface,

548 surface: &mut Surface,

569 theme: &Theme,

549 theme: &Theme,

570− all_breakpoints: &HashMap<PathBuf, Vec<SourceBreakpoint>>,

571 ) {

550 ) {

572 use helix_core::diagnostic::Severity;

551 use helix_core::diagnostic::Severity;

573 use tui::{

552 use tui::{

@@ -605,28 +584,28 @@ impl EditorView {

605 lines.extend(text.lines);

584 lines.extend(text.lines);

606 }

585 }

607

586

608− if let Some(path) = doc.path() {

609− let line = doc.text().char_to_line(cursor);

587+ // let line = doc.text().char_to_line(cursor);

588+ // let breakpoint = doc

589+ // .path()

610− if let Some(breakpoints) = all_breakpoints.get(path) {

590+ // .and_then(|path| all_breakpoints.get(path))

591+ // .and_then(|breakpoints| {

611− if let Some(breakpoint) = breakpoints

592+ // breakpoints

612− .iter()

593+ // .iter()

613− .find(|breakpoint| breakpoint.line - 1 == line)

594+ // .find(|breakpoint| breakpoint.line == line)

614− {

595+ // });

596+

597+ // if let Some(breakpoint) = breakpoint {

615− if let Some(condition) = &breakpoint.condition {

598+ // if let Some(condition) = &breakpoint.condition {

616− lines.extend(

599+ // lines.extend(

617− Text::styled(condition, warning.add_modifier(Modifier::UNDERLINED))

600+ // Text::styled(condition, warning.add_modifier(Modifier::UNDERLINED)).lines,

618− .lines,

619− );

601+ // );

620− }

602+ // }

621− if let Some(log_message) = &breakpoint.log_message {

603+ // if let Some(log_message) = &breakpoint.log_message {

622− lines.extend(

604+ // lines.extend(

623− Text::styled(log_message, info.add_modifier(Modifier::UNDERLINED))

605+ // Text::styled(log_message, info.add_modifier(Modifier::UNDERLINED)).lines,

624− .lines,

625− );

606+ // );

626− }

607+ // }

627− }

628− }

629− }

608+ // }

630

609

631 let paragraph = Paragraph::new(lines)

610 let paragraph = Paragraph::new(lines)

632 .alignment(Alignment::Right)

611 .alignment(Alignment::Right)

helix-view/src/editor.rs+14−1

@@ -156,6 +156,19 @@ impl std::fmt::Debug for Motion {

156 }

157}

158

159+#[derive(Debug, Clone, Default)]

160+pub struct Breakpoint {

161+ pub id: Option<usize>,

162+ pub verified: bool,

163+ pub message: Option<String>,

164+

165+ pub line: usize,

166+ pub column: Option<usize>,

167+ pub condition: Option<String>,

168+ pub hit_condition: Option<String>,

169+ pub log_message: Option<String>,

170+}

171+

159#[derive(Debug)]

172#[derive(Debug)]

160pub struct Editor {

173pub struct Editor {

161 pub tree: Tree,

174 pub tree: Tree,

@@ -169,7 +182,7 @@ pub struct Editor {

169

182

170 pub debugger: Option<dap::Client>,

183 pub debugger: Option<dap::Client>,

171 pub debugger_events: SelectAll<UnboundedReceiverStream<dap::Payload>>,

184 pub debugger_events: SelectAll<UnboundedReceiverStream<dap::Payload>>,

172− pub breakpoints: HashMap<PathBuf, Vec<dap::SourceBreakpoint>>,

185+ pub breakpoints: HashMap<PathBuf, Vec<Breakpoint>>,

173

186

174 pub clipboard_provider: Box<dyn ClipboardProvider>,

187 pub clipboard_provider: Box<dyn ClipboardProvider>,

175

188