-
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
Add support for conditional exception breakpoints #12445
Conversation
The PR is ready for review. |
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.
Some suggestions
@@ -692,13 +692,26 @@ export class DebugSession implements CompositeTreeElement { | |||
} | |||
|
|||
protected async sendExceptionBreakpoints(): Promise<void> { | |||
const filters = []; | |||
const args: DebugProtocol.SetExceptionBreakpointsArguments = { |
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.
Wouldn't this be simpler:
const filters= [];
const filterOptions= this.capabilities.supporteExceptionFilterOptions ? [] : undefined;
for (....) {
...
}
await this.sendRequest('setExceptionBreakpoints', { filters, filterOptions});
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.
Sure. Thanks for the suggestion!
@@ -31,13 +34,35 @@ export class DebugExceptionBreakpoint implements TreeElement { | |||
} | |||
|
|||
render(): React.ReactNode { | |||
return <div title={this.data.raw.label} className='theia-source-breakpoint'> | |||
return <div title={this.data.raw.description || this.data.raw.label} className='theia-source-breakpoint'> |
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.
Drive-by fix?
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.
That's right.
The behaviour seems fine. Would you mind fixing the conflicts? |
Done. Thank you for review. |
What it does
Closes #12444 by adding support for conditional exception breakpoints to Theia:
How to test
Try to debug a simple Node.js program like this:
Enable both
Caught Exceptions
andUncaught Exceptions
inBreakpoints
view. Try setting a condition likeerror.name == "MyError"
using the context menu itemEdit Condition...
for either of the exception breakpoints. Verify that exception filtering works as expected while debugging. Also, verify that exception breakpoints' condition settings are properly restored upon workbench restart. Try removing the condition by editing it again and blanking out the input field.Review checklist
Reminder for reviewers