Add support for function breakpoints

[thegecko]

Signed-off-by: Rob Moran [email protected]

Introduced support for adding function breakpoints, see:

https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_function-breakpoints

[jonahgraham]

This kind of change needs some kind of basic test please.

[thegecko]

Tests would be a lot easier to write if they worked on MacOS or ran on PRs (with the output shown)

[jonahgraham]

Tests would be a lot easier to write if they worked on MacOS or ran on PRs (with the output shown)

Please add details on Issue #12 - or raise a new issue if needed.

[jonahgraham]

In addition to the missing tests, this doesn't quite work. See spec:

↩️ SetFunctionBreakpoints Request
Replaces all existing function breakpoints with new function breakpoints.

The current implementation in this commit just keeps adding to the installed breakpoints. This probably wants to be merged with setBreakPointsRequest as that (sort of) handles the case.

As for a minimal test, something like this would be a start.

    it('hits a function breakpoint', async () => {
        await dc.setFunctionBreakpointsRequest({
            breakpoints: [
                {
                    name: 'main',
                },
            ],
        });
        await dc.configurationDoneRequest();
        const scope = await getScopes(dc);
        const vr = scope.scopes.body.scopes[0].variablesReference;
        const vars = await dc.variablesRequest({ variablesReference: vr });
        verifyVariable(vars.body.variables[0], 'count', 'int', '0');
    });

Although it would probably be better to add to count.c to add an additional function.

[jonahgraham]

Thanks for the test. I think this still doesn't meet the DAP spec: #136 (review)

[thegecko]

I think this still doesn't meet the DAP spec

Still WiP

[thegecko]

Rebased and added tests, still need to resolve the issue with not removing them...

[jonahgraham]

@thegecko pushed 1 commit.

88fcd1f Support removing function breakpoints

@thegecko does this mean it is ready for review? Let me know when/if it is and I will try to get it in.

[thegecko]

@jonahgraham This should be good to go. I've got a change which de-duplicates the breakpoint resolving in setBreakpoints and setFunctionBreakpoints, but felt that should be a PR after this one. Let me know if you'd prefer to see it added here, however.

[jonahgraham]

Let me know if you'd prefer to see it added here, however.

Lets avoid the duplication in the first place - if you are ready to add it now, please do that. It will save me having to compare the setBreakpoints and setFunctionBreakpoints versions.

[thegecko]

@jonahgraham, I've introduced a common resolveBreakpoints() function for determining the breakpoints to add and delete across setBreakpoints() and setFunctionBreakpoints().

I've introduced a few more tests to exercise this new common function and I've tested it all locally.

It's probably worth focussing your checks on the breakpoint handling overall as there is some churn in that area.

I've also added a few minor bug-fixes and comments.

[jonahgraham]

Looks good. I am going to do a touch of code cleanup as commented below and push it.

[jonahgraham]

Cool, I didn't know you could do this. I am fixing up some other code because of this - PR coming soon.

[jonahgraham]

hitCondition and condition - VSCode does not yet have UI for this so it seems acceptable to not implement support for now, but we are at some point going to need our own fork of the debug protocol spec to document our differences and extensions and this is one of them.

[jonahgraham]

There is already a helper that does this:

        dc.assertStoppedLocation('function breakpoint', {line: 2});