search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.35k stars 28.91k forks source link

When processing setBreakpoints response treat undefined source.path the same as undefined source #117554

Open polinasok opened 3 years ago

polinasok commented 3 years ago

This is inspired by https://github.com/golang/vscode-go/issues/1212. See this update for details: https://github.com/golang/vscode-go/issues/1212#issuecomment-784917942

It would be great if the following were treated the same: "body":{"breakpoints":[{"verified":true,"line":4}]} OR "body":{"breakpoints":[{"verified":true,"source":{},"line":4}]}

As far as I can tell, upon receiving the response the breakpoint gets redisplayed here: https://github.com/microsoft/vscode/blob/febd96f0e35811198f0ad05414bf6a6f861e1074/src/vs/workbench/contrib/debug/browser/breakpointsView.ts#L393-L399

I tried adding a check for undefined path here (&& this.data.source.path): https://github.com/microsoft/vscode/blob/a699ffaee62010c4634d301da2bbdb7646b8d1da/src/vs/workbench/contrib/debug/common/debugModel.ts#L704 but that was not enough because somewhere along the way the default also lost the path and had this._uri._fsPath as null.

weinand commented 3 years ago

@isidorn I think it is a fair request to treat an empty "source" and "no source" the same.

Since all attributes of source are optional an "empty" source is perfectly legal. And since the spec does not define semantics for an "empty" source, we should treat it as "no source".

isidorn commented 3 years ago

Assigning to March to investigate in debt week

isidorn commented 3 years ago

@polinasok I think you got the right location with the debugModel on line 704. That would be my attempt to fix. However if you say that was not enough then it must be something else. However please note that the breakpoint._uri does not change, it is readonly (even though I forgot to put that modifer). So probably when the breakpoint got created the uri was bogus. I suggest to put a breakpoint here

@polinasok if you would like me to investigate further it would be great if you could fork the mock debug adapter and make changes to it such that this issue is reproducible. Then I can clone your fork and see the issue on my machine. Thanks