Skip to content

Handled case with inline disabled breakpoint gets reactivated#155403

Merged
roblourens merged 2 commits intomicrosoft:mainfrom
Balastrong:feature/154998-add-disabled-inline-breakpoint
Sep 7, 2022
Merged

Handled case with inline disabled breakpoint gets reactivated#155403
roblourens merged 2 commits intomicrosoft:mainfrom
Balastrong:feature/154998-add-disabled-inline-breakpoint

Conversation

@Balastrong
Copy link
Contributor

@Balastrong Balastrong commented Jul 16, 2022 •
edited
Loading

This PR fixes #154998.

I think these are the lines where it fails.

vscode/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts

Lines 682 to 688 in 7839742

this.toDispose.push(dom.addDisposableListener(this.domNode, dom.EventType.CLICK, async e => {
if (this.breakpoint) {
await this.debugService.removeBreakpoints(this.breakpoint.getId());
} else {
await this.debugService.addBreakpoints(this.editor.getModel().uri, [{ lineNumber: this.range!.startLineNumber, column: this.range!.startColumn }]);
}
}));

Looks like we have three cases here when we click on an InlineBreakpointWidget, but right now we handle only two.

If this.breakpoint is undefined, a new breakpoint is added.
If this.breakpoint is defined, the current implementation calls addBreakpoints which apparently fails because the breakpoint already exists, it's just disabled.

By explicitly handling when this.breakpoint is defined but disabled, as shown in this PR, looks like the issue is gone.

@roblourens roblourens added this to the August 2022 milestone Jul 19, 2022
@roblourens
Copy link
Member

roblourens commented Jul 19, 2022

Thanks for the PR. I am heading out of town and will review for our next release.

roblourens
roblourens approved these changes Sep 2, 2022
View reviewed changes
Copy link
Member

@roblourens roblourens left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect thanks! Not sure why the build originally failed, I will update it to try again

@roblourens roblourens merged commit e6b2d88 into microsoft:main Sep 7, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@roblourens roblourens roblourens approved these changes

@bpasero bpasero bpasero approved these changes

Assignees

@roblourens roblourens

Labels

None yet

Projects

None yet

Milestone

September 2022

Development

Successfully merging this pull request may close these issues.

Nested breakpoint is gone on click action

3 participants

@Balastrong @roblourens @bpasero