-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Implement Disassembly View and InstructionBreakpoints #11186
Implement Disassembly View and InstructionBreakpoints #11186
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.
Looks mostly good. I noticed some unexpected behavior in regards to disassembly breakpoints:
As far as I can tell, debugging directly in the disassembly view is not supported, right (that's what the icon indicates)? Anyway, these breakpoints cannot be removed, and clicking on the breakpoint sidebar in the disassembly view only adds more breakpoints, instead of removing the existing ones.
packages/debug/src/browser/disassembly-view/disassembly-view-accessibility-provider.ts
Outdated
Show resolved
Hide resolved
packages/debug/src/browser/disassembly-view/disassembly-view-accessibility-provider.ts
Outdated
Show resolved
Hide resolved
packages/debug/src/browser/disassembly-view/disassembly-view-accessibility-provider.ts
Outdated
Show resolved
Hide resolved
packages/debug/src/browser/disassembly-view/disassembly-view-contribution.ts
Outdated
Show resolved
Hide resolved
packages/debug/src/browser/disassembly-view/disassembly-view-widget.ts
Outdated
Show resolved
Hide resolved
packages/debug/src/browser/disassembly-view/disassembly-view-widget.ts
Outdated
Show resolved
Hide resolved
36d80d7
to
9227c72
Compare
46cd5e1
to
2bd892a
Compare
Don't allow breakpoints if they're not supported
@JonasHelming, would you be willing to push a bit to get the CQ through? |
The CQ has been approved, thank you @JonasHelming for helping it get across the line 👍 |
@msujew, I pushed a change (and meant to mention it - sorry) that addresses your comments. I have modified the enablement of the commands that manipulate all breakpoints to be available when only instruction breakpoints are present. I have also (and this makes it harder to test my last claim ;-)) added code that prevents the creation of instruction breakpoints for sessions that don't support them - an improvement, I believe, over VSCode's code. |
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.
Great, looks good to me!
- Hitting a breakpoint in the C code lets us open the disassembly view via a context menu / command.
- Scrolling up/down in the disassembly view loads more instructions and (relatively accurate) changes the position within the list view.
- Breakpoints within the disassembly view are disabled for
cdt-gdb
, definitely an improvements compared to vscode.
We can't really test the functionality of breakpoints within the disassembly view, but I'm fine with revisiting this if there are any problems with it and some extension has mature support for it.
What it does
Addresses parts of #8136 by implementing InstructionBreakpoints and a disassembly view to and the return of the disassemble request.
This PR copies and adapts VSCode's implementation of the Disassembly View for Theia.
How to test
a.cpp
and start debugging.CQ
Review checklist
Reminder for reviewers