Add support for Function breakpoints

[rkeithhill]

VSCode supports function breakpoints which should map fairly nicely to PowerShell command breakpoints.

[rkeithhill]

Ugh, so I have function breakpoints working as long as you don't use them in conjunction with line breakpoints. If you use only function breakpoints then the setFunctionBreakpoints request comes in before the the launch request. If you use both line and function breakpoints, then the setBreakpoints request comes in before launch but the setFunctionBreakpoints (as well as the setExceptionBreakpoints) request comes in after launch. Then the debugger hangs up in a loop or something (becomes unresponsive from VSCode) as soon as I try to execute any PowerShell command like Set-PSBreakpoint or Remove-PSBreakpoint. Should we be using the configurationDone request:

    /** Event message for "initialized" event type.
        This event indicates that the debug adapter is ready to accept configuration requests (e.g. SetBreakpointsRequest, SetExceptionBreakpointsRequest).
        A debug adapter is expected to send this event when it is ready to accept configuration requests.
        The sequence of events/requests is as follows:
        - adapters sends InitializedEvent (at any time)
        - frontend sends zero or more SetBreakpointsRequest
        - frontend sends one SetExceptionBreakpointsRequest (in the future 'zero or one')
        - frontend sends other configuration requests that are added in the future
        - frontend sends one ConfigurationDoneRequest
    */

Maybe we should wait for configurationDone before actually launching the debugger?

[daviwil]

Yep, definitely seems that we should be waiting for that request now before fully launching. Seems that they've changed the semantics of their protocol a lot since I first wrote this implementation.

[rkeithhill]

OK, I'm capturing the launch parameters (program, arguments) in the launch request and then actually executing the script in the configurationDone request. This seems to be working much better. However, in the process of testing I've found a bug in either my code or VSCode that I'd like to fix (if I can). See this VSCode issue I submitted.

[daviwil]

Cool! Glad to hear that smoothed it out. I'm curious to hear what they have to say about that bug.

[rkeithhill]

I think I'll try to ensure the correct order on our end as an experiment - to see if that is really the issue. Right now I'm optimizing a bit for the common case (no condition) and using a single Set-PSBreakpoint call with multiple lines. Can't do that when there is a condition. Then it has to be one Set-PSBreakpoint invocation per conditional breakpoint.

[rkeithhill]

Update: the breakpoint return order does matter. Interesting.

Well this fixes the issue. I think I'll make one commit with the "optimized" code and then another with it removed so the extension will handle breakpoints correctly. Let me know when it would be a good time to submit the PR against develop.

[rkeithhill]

BTW the change to launch in the configurationDone request results in a change in the unit tests:

        public Task LaunchScript(string scriptFilePath)
        {
            return this.SendRequest(
                LaunchRequest.Type,
                new LaunchRequestArguments
                {
                    Program = scriptFilePath
                });
        }

        public Task ConfigurationDone()
        {
            return this.SendRequest(ConfigurationDoneRequest.Type, null);
        }
...
        public async Task DebugAdapterReceivesOutputEvents()
        {
            OutputReader outputReader = new OutputReader(this.debugAdapterClient);

            await this.LaunchScript(DebugScriptPath);
            await this.ConfigurationDone();

            // Make sure we're getting output from the script
            Assert.Equal("Output 1", await outputReader.ReadLine());

Thoughts? Is there a way to put the this.SendRequest(ConfigurationDoneRequest.Type, null) call in the LaunchScript method?

[daviwil]

Yep! Here's how I'd do it:

        public async Task LaunchScript(string scriptFilePath)
        {
            await this.SendRequest(
                LaunchRequest.Type,
                new LaunchRequestArguments
                {
                    Program = scriptFilePath
                });

            await this.SendRequest(ConfigurationDoneRequest.Type, null);
        }

        public async Task DebugAdapterReceivesOutputEvents()
        {
            OutputReader outputReader = new OutputReader(this.debugAdapterClient);

            await this.LaunchScript(DebugScriptPath);

            // Make sure we're getting output from the script
            Assert.Equal("Output 1", await outputReader.ReadLine());
...

[daviwil]

The difference here is that the async keyword is used in the LaunchScript method so that we can await on both SendRequest calls.