Fixes #173 - implementation for function breakpoints

[rkeithhill]

This PR introduces support for the configurationDone request. In fact, due to the order in which set*Breakpoints requests and the launch request come in, we now defer the execution of the script until we receive the configurationDone request. When we executed from the launch request, the debug host would appear to hang. It did not like executing Set/Remove-PSBreakpoint commands when the host debugger was running (not in a stopped state) AFAICT.

The first commit has a known issue with VSCode mixing up which breakpoint has a condition associated with it when you add/remove a breakpoint (see microsoft/vscode#3558). The second commit fixes this issue.

[daviwil]

Can a breakpoint be both on a function and conditional?

[daviwil]

Maybe I'm asking the wrong question, just confused by seeing this in the SetFunctionBreakpoints method. Maybe the function name is wrong?

[rkeithhill]

No, the method name is correct. The comment is misleading. I just committed a fix for the comment. As I mentioned below, function breakpoints in VSCode can have conditions (at the debug protocol level) and PowerShell supports it as well.

[daviwil]

That's pretty cool!

[daviwil]

This is really awesome! We're definitely going to need some instructive documentation on how to use the debugger for this release. It can do so much cool stuff now that it's a shame people don't use it more. I'll see what we need to do for getting a doc tool setup going in a couple weeks.

[rkeithhill]

Yes, PowerShell supports it:

    Command: Microsoft.PowerShell.Utility/Set-PSBreakpoint
    Set:    Command


Name                   Aliases      Position Mandatory Pipeline ByName Provider        Type
----                   -------      -------- --------- -------- ------ --------        ----
Action                 {A*}         Named    False     False    False  All             ScriptBlock
Command                {C}          Named    True      False    False  All             String[]
Script                 {S*}         0        False     False    False  All             String[]

And the debug protocol supports. What I haven't seen yet is UI in VSCode that supports. But once they add that, we should be able to support it.

[rkeithhill]

We're definitely going to need some instructive documentation

And I'll definitely have a few blog posts queued up. One for breakpoint enhancements and another for Pester integration. What's your target release date BTW?

[daviwil]

For now my mental stake in the ground is March 25 or 28th. Could be sooner depending on how fast I'm able to land certain things. I guess it'd be a little unfortunate if VS Code ships their update next week and we aren't up to date for another couple weeks. What do you think?

[rkeithhill]

At this point, I'd be ready except for anything new in the next release of VSCode that we would want to take advantage of. That would take a day or two after we find out what is new. The drag is that even though I have access to the alpha channel, I won't have a good idea on what's "new" until I see some release notes. Or I guess, look at the VSCode commit history. :-)

[daviwil]

They've got release notes for 0.10.10 already with the Insiders release that went out today:

https://github.com/Microsoft/vscode-docs/blob/vnext/release-notes/latest.md

I don't know if they'll do a "March release" this month since they're already running late on their release schedule ;)

[rkeithhill]

I have another comment cleanup commit for this PR (sigh). Anyway, before I check this in I have a question about naming. Should the method names in the debug service be PowerShell oriented or VSCode oriented? I suspect it should be the former and if so, I should probably change the name of the method SetFunctionBreakpoints to SetCommandBreakpoints. Thoughts?

Also, this is for a later API cleanup, but shouldn't all the async methods be suffixed with Async as in SetCommandBreakpointsAsync()?

[daviwil]

Yeah, the core Editor Services library should be using PowerShell terms. As far as the -Async is concerned, I don't know. I guess that has been the standard but I don't really like having an extra word for it. Seems like a failure of tooling rather than a naming requirement. We should definitely have that discussion again for 1.0 so I've filed an issue for it:

#177

[rkeithhill]

@daviwil Let me know when you've had enough time to look this over. It is big and got bigger with subsequent commits (sorry about that).

[daviwil]

I checked it out already, looks good! Merge when ready.