-
Notifications
You must be signed in to change notification settings - Fork 235
Debug adapter crashes when adding breakpoints for files that don't exist #195
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
Comments
This is an artifact of VSCode keeping track of the breakpoints by filename/line and not being savvy about the associated file being renamed, right? If the file was renamed "within" and "by" VSCode then this seems like a VSCode bug. It should know what breakpoints are set in the file and update the filename appropriately when it is renamed. I wonder if an issue has been submitted to VSCode on this? That said, we can still fix the extension to not crash. :-) |
I think the guy renamed the file outside of VS Code, or at least that's the impression I got. I'm guessing they aren't watching the filesystem for renames when a file gets renamed out of band. |
I verified that VSCode moves the breakpoint accordingly if you rename the file within VSCode. |
That's good at least, means this probably hasn't been a major issue for users. |
OK I have a fix for this but I'm a bit disappointed that when I return this message (note: verified = false):
VSCode doesn't do anything with it AFAICT. I see no tooltip over the breakpoint. No popup. Nothing. What's your policy on using the Debug Console? Should this only be for "script output" or can we output certain debug events (like VS does in its Output window)? Can/should we output something to the debug console indicating that the breakpoint was ignored? Or should we just rely on VSCode eventually getting around to doing something with the message we return? Also, any input on the wording for that message? |
I don't have a problem with printing output, it'd be helpful in the short term if VS Code doesn't provide an integrated way to see the message. It looks like Set-PSBreakpoint actually prints an error if the file doesn't exist:
I suppose we could just let it write its error to the host? |
I'm also OK with having a less noisy output that is more appropriate for the console. |
This is actually a good conversation to have as I'm catching the non-existing file at the protocol layer so I can respond with what I "believe" is the appropriate response to the client (even if they seem to ignore it for now). That said, I short-circuit the call down to the editor services layer. Perhaps I should catch the FileNotFoundException at the protocol layer but still call down to the editor services layer?? But then if VSCode ever does start displaying info to the user, the debug console output becomes a bit redundant. Hmmm. |
Yeah, let's just show our own message for now. Once we start plugging in to other editors we may have to think a bit more about the right way to do this, but I think doing it at the protocol layer is a fine solution for now. |
Hmm, one issue. At the protocol layer we try to look up a ScriptFile to pass down to the editor services method: public async Task<BreakpointDetails[]> SetLineBreakpoints(
ScriptFile scriptFile,
BreakpointDetails[] breakpoints,
bool clearExisting = true) But since the file doesn't exist, how do I pass it to this method other than to pass I guess what we could do is add an overload that takes just a string path (which may not exist) and have the current method resolve ScriptFile to a string path and call the new overload. Thoughts? |
Sorry for the delay, have been busy with stuff at the Summit. New changes look good to me! |
…onexisting-files Fixes #195 - set breakpoint in non-existing file doesn't crash
This can happen when VS Code keeps old breakpoints for files that have been renamed or deleted. See this bug report: PowerShell/vscode-powershell#121
The optimal behavior here is to send back information to VS Code saying that the breakpoint is unverified. Maybe there's a way to send an error saying that the breakpoint is completely invalid and should be removed?
Stack trace:
The text was updated successfully, but these errors were encountered: