Support condition and hit count on data breakpoints (fix #188721)#195710
Support condition and hit count on data breakpoints (fix #188721)#195710connor4312 merged 5 commits intomicrosoft:mainfrom
Conversation
|
Thanks @gjsjohnmurray! A couple questions Other exception rows have the pencil button to edit the condition, should we add the toolbar for the data breakpoints too? And normal breakpoints have the X button, I'm not sure why that wasn't adopted here. And exception breakpoints show the condition inline, but there isn't any way to see that the data breakpoint has a condition, do you think we should try to add the condition to the description? |
|
@roblourens thanks for the feedback. I have pushed the improvements:
|
|
@roblourens I think this is ready for merging to catch its November milestone's endgame. |
|
Seeing some unit test failures that look relevant |
|
Fixed, and tests now passed. |
|
|
||
| const primary: IAction[] = []; | ||
| this.breakpointSupportsCondition.set(!session || !!session.capabilities.supportsConditionalBreakpoints); | ||
| this.breakpointItemType.set('dataBreakpoint'); |
There was a problem hiding this comment.
The usual way to do this would be with an overlaid context key service, which avoids having to re-evaluate context keys globally in vs code. Example:
vscode/src/vs/workbench/contrib/testing/browser/testingExplorerView.ts
Lines 1486 to 1487 in 61a5457
There was a problem hiding this comment.
Yeah, but I think it's been wrong for a long time for other breakpoint types
There was a problem hiding this comment.
Thanks for the feedback. This code currently replicates existing code for function breakpoints.
| group: 'navigation', | ||
| order: 20, | ||
| when: CONTEXT_BREAKPOINT_ITEM_TYPE.isEqualTo('functionBreakpoint') | ||
| when: ContextKeyExpr.or(CONTEXT_BREAKPOINT_ITEM_TYPE.isEqualTo('functionBreakpoint'), CONTEXT_BREAKPOINT_ITEM_TYPE.isEqualTo('dataBreakpoint')) |
There was a problem hiding this comment.
I noticed when looking at the precondition that this is enabled when supportsConditionalBreakpoints, not when supportsHitConditionalBreakpoints. Not an issue in this PR, but something we should fix 🤔
3 participants
This PR fixes #188721 by making a data breakpoint's "Edit Condition..." context menu entry produce the same inline editor as already happens for function breakpoints. Also did the same for Hit Count (not previously in the context menu for data breakpoints).
Repro steps are available on #195695