-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cider-dap changes #1768
Cider-dap changes #1768
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good on the whole! I left some minor notes. You'll need to run cargo fmt on the files (cargo fmt --all
should do the trick)
cider-dap/src/adapter.rs
Outdated
use std::fs::File; | ||
pub struct MyAdapter { | ||
#[allow(dead_code)] | ||
file: File, | ||
// Other fields of the struct | ||
breakpoints: Vec<(Source, i64)>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment for breakpoints
and threads
to mark that it is a placeholder?
cider-dap/src/adapter.rs
Outdated
let breakpoint = Breakpoint { | ||
id: self.break_count.increment().into(), | ||
verified: true, | ||
message: None, | ||
source: Some(path.clone()), | ||
line: None, | ||
column: None, | ||
end_line: None, | ||
end_column: None, | ||
instruction_reference: None, | ||
offset: None, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be useful to abstract this into our own constructor. Something like make_breakpoint
which takes the relevant args that we use.
cider-dap/src/adapter.rs
Outdated
} | ||
|
||
//Return threads | ||
pub fn get_threads(&self) -> Vec<Thread> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename this something like clone_threads
since I would normally expect a get method to return a reference, but we need the owned vec for the return message (which is silly but hey that's a problem for another day)
cider-dap/src/adapter.rs
Outdated
pub fn new() -> Self { | ||
Counter { value: 0 } | ||
} | ||
//Inc the counter, return the OLD value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this a doc-string (i.e. ///)
cider-dap/src/error.rs
Outdated
|
||
#[allow(dead_code)] // remove this later | ||
#[derive(thiserror::Error, Debug)] | ||
pub enum MyAdapterError { | ||
pub enum MyAdapterError{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is gonna make the formatter angry
* Cleaned up UnhandledCommandError * Added the SetBreakPoints command to the debugger * Added SetExceptionBreakpoints * Cleaned up code and added threads command, debugger now runs * PR revisions --------- Co-authored-by: root <root@EliasSP>
main.rs
adapter.rs
The debugger now runs!