Several debugger improvements #20
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jovobe
requested changes
Aug 25, 2018
desktop/src/args.rs
Outdated
| parse(try_from_str = "parse_breakpoint"), | ||
| requires = "debug", | ||
| help = "Breakpoint that is added to the debugger at the very beginning. Breakpoints are \ | ||
| specified in hexadecimal.", |
There was a problem hiding this comment.
hexadecimal
An example for the exact format would be great! (e.g. 0x42 or 0x0042 or 42h or $42, etc.)
| long = "--breakpoints", | ||
| parse(try_from_str = "parse_breakpoint"), | ||
| requires = "debug", | ||
| help = "Breakpoint that is added to the debugger at the very beginning. Breakpoints are \ |
There was a problem hiding this comment.
It is not clear, if the user can pass one or many breakpoints (and if many, how they have to be separated (e.g. 0x42, 0x21 or 0x42;0x21 or 0x42 0x21 etc.)).
desktop/src/args.rs
Outdated
| fn parse_breakpoint(src: &str) -> Result<Word, String> { | ||
| u16::from_str_radix(src, 16) | ||
| .map(Word::new) | ||
| .map_err(|e| format!("failed to parse breakpoint: {}", e)) |
There was a problem hiding this comment.
Is the --help message triggered here? If not, information about the proper format would be helpful.
LukasKalbertodt
force-pushed
the
cli-breakpoints
branch
from
9c9005c to
dc59c5e
Compare
jovobe
approved these changes
Aug 26, 2018
|
Wait with this merge for #21. |
LukasKalbertodt
force-pushed
the
cli-breakpoints
branch
from
dc59c5e to
b256fe8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CC #18
See commit messages. Regarding review: again, debugger code isn't that beautiful. And it's not that critical, so I'd say a proper review is not necessary.