Add support for conditional breakpoints

[daviwil]

The VS Code debug adapter protocol just gained support for conditional breakpoints. We should adopt the protocol changes and add this support in our debug service. See the commit related to this issue for the protocol changes:

microsoft/vscode-debugadapter-node#6

[rkeithhill]

Ooh, that would be nice. So the expression string would be passed back to the extension to evaluate and the extension would return either true or false?

BTW this doesn't help with management of data and command breakpoints which can be set by the user in the Debug Console (fortunately). Although it is a bit of a pain e.g.:

Set-PSBreakpoint -Script C:\Users\Keith\.vscode\extensions\ms-vscode.PowerShell-0.3.1\examples\DebugTest.ps1 -Command Write-Output

It would be nice if I could use some variable in the Debug Console would recognize as the path to the workspaceRoot and the current file. I tried $file, $MyInvocation.ScriptName (and InvocationName) and these all came back empty.

Or perhaps we inject some sort of DTE like object in the Debug Console session to allow the user to access things like the path to workspaceRoot, the full path of the currently open (debugged) file, the collection of Watched variables, etc.

[daviwil]

Yep, we'd basically just take the expression and register it with Set-PSBreakpoint so that the PowerShell debugger will stop when it evaluates to true.

I think it would be good to propose command breakpoints as a possibility to the VS Code team. Not sure if other languages have that feature though?

The DTE idea is interesting. I think we're going to need a broader strategy for a $psise-like object, so your ideas here could fit in well with that. This is something that I'll want to start building along with the proposed extensibility model, definitely interested to hear your ideas on what else would be useful there!

[daviwil]

I'm going to push this out to 0.5.0 because I'm not sure how it will work just yet.

[rkeithhill]

OK I have this "prototyped" and it basically works:

Now to discuss a few issues. The way the Set-PSBreakpoint -Action {} parameter works is that it defaults to an action breakpoint. That is, it does something and execution continues. To get it to behave like a conditional breakpoint, you have to conditionally execute break. So for the "simple" expression I entered into VSCode, I had to convert that to if ($i -eq 3) {break}.

I would like to allow action style breakpoints as well. For instance, if you entered Write-Host "i is $i"; continue, I would note there is a continue and that you want an action breakpoint and pass the script through "as-is" to the Action parameter. This currently works BTW. It spits out the value of $i to the Debug Console.

I guess the basic question is how cute do we want to get. I like the idea of something like $i -eq 3 "just working". I think that is what folks will expect. If we tell them that even though VSCode says this:

And then tell them that for PowerShell, they have to type in if (<expression>) {break} to get a conditional breakpoint to work ... well that would suck IMO.

I also think that we should ask the VSCode team for "action" breakpoints as well. If we had support in VSCode for action breakpoints, then I would pass those through as-is.

[KirkMunro]

A few thoughts:

1. I think basic conditional breakpoints, whether they are line, command or variable breakpoints, should work just like you described, where you enter nothing other than the condition and internally VSCode is smart enough to wrap your condition in an if ($condition) {break} breakpoint.
2. When entering a condition for a breakpoint, I think there should be an advanced mode, where I can enter the actual script; this allows me to provide weird custom conditional logic that may include continue statements. Being able to switch from basic to advanced mode would always work, showing the full logic; being able to switch from advanced to basic may work, if the logic is a simple condition with a break statement inside (should be determinable via AST). Regardless, in advanced mode I think that the editor should absolutely require a break statement before accepting the script block that was provided. Otherwise it's not really a breakpoint at all. Most of the time, it would be a user error to create a custom conditional breakpoint without the break statement, and I think modern editors should not allow that to happen.
3. Action-style "breakpoints" are not breakpoints at all. They are debugger hooks, allowing custom script to be invoked every time a command is about to be invoked, a line is reached in the debugger, or a variable is about to be read from or written to. From an end-user perspective, I think the UX would be better if action-style breakpoints are called what they really are: debugger hooks. I also think the color marker for these hooks should not look like a breakpoint, because they aren't breakpoints at all. A different color would do, perhaps with a lightning bolt or a hook in the circle? Similar to conditional breakpoints, a debugger hook should not allow the break statement to be used in the outer scope because otherwise it would be a breakpoint.
4. Since technically both debugger hooks and conditional breakpoints are breakpoints behind the scenes, you could allow conversion from one to the other (if you use break in the outer scope of a debugger hook, ask the user if they want to convert that to a breakpoint, or if you create a custom conditional breakpoint without the break keyword in the outer scope, ask the user if they want to convert that to a debugger hook). I'm not so sure that's necessary though, because creating breakpoints is conceptually different from creating debugger hooks IMHO, and for the PowerShell scripter I think having these two tasks as separate might be helpful for them to actually use these features (based on questions I have asked at conferences and events, very, very few PowerShell scripters use breakpoints/debug code -- most just use a run script, modify script, run script cycle).

[daviwil]

Kirk, I agree, it'd be nice if we could make some distinction there so that we could enable both kinds of breakpoints. I think we'll have some more leverage to ask them about more breakpoint types in a couple months. If they can't do it then we could at least enable it in Editor Services so that another editor (like the ISE) could have that capability.

Keith, I agree with going with the simplified predicate expression approach for now since that's what VS Code's UI communicates. Let's still ask them about the possibility of adding action breakpoints, but maybe after some of our current requests have settled.

Also, I spy code folding icons ;) Does it work with #region/#endregion?

[rkeithhill]

ask them about more breakpoint types in a couple months

Not only could PowerShell support "action" breakpoints (as they're called in Visual Studio) but also variable breakpoints. The latter is similar to the data breakpoints you get in C++ IIRC.

Also, I spy code folding icons ;) Does it work with #region/#endregion?

No. But perhaps that would require that the language declares support for such directives?

@KirkMunro
Thanks for the great feedback. WRT:

(if you use break in the outer scope of a debugger hook, ask the user if they want to convert that to a breakpoint, or if you create a custom conditional breakpoint without the break keyword in the outer scope, ask the user if they want to convert that to a debugger hook

As @daviwil alludes to, with VSCode we don't have that sort of control over the end user UI experience. We get a JSON message from VSCode saying put a breakpoint on line X, optionally on column Y and optionally with a condition string. We try to do that and then let VSCode know whether we succeeded and if not, we can provide a message indicating why.

Also RE this:

... and I think modern editors should not allow that to happen.

Don't you mean "modern debuggers"? :-) BTW I consider Visual Studio a modern debugger and it allows just that. Note the Continue execution checkbox below that is checked by default for an Action breakpoint. VS has had this feature for quite a while.

So I think I want to A) use the ScriptBlock AST to search for either a BreakStatementAst or ContinueStatementAst. If I find either of those, the user is using "advanced" syntax and I pass the scriptblock as-is to the Action parameter. If I don't find either of those two, I stick the condition string inside this template if (<expression>) {break} and use that. That said, I could be convinced to completely punt on the advanced syntax support.

[daviwil]

If it turns out to be fairly straightforward to do the advanced syntax support, I'd say go for it. We've got plenty of time until 0.5.0 gets released. We don't even have to advertise that it works until VS Code expands their UI to include it as a possibility.

[KirkMunro]

@rkeithhill Yeah, I meant modern debuggers, not modern editors. When considering features for PowerShell editors/debuggers, while I draw some inspiration from Visual Studio and other IDEs (and I know, this is Visual Studio Code), I try to tweak it for the target audience. I did this when working on PowerGUI and PowerSE as well. It might make sense to diverge from how Visual Studio does things when the majority of the PowerShell community doesn't do debugging. But then again, maybe not, since this is a Visual Studio environment. Still thinking about it. For me though, regardless of the debugger, my brain still wants to pause when it hears the word breakpoint in the context of something that doesn't break at all. I get it, and I use it, but the presentation doesn't feel right to me.