Show unverified function breakpoints in debug viewlet

[alexdima]

Testing #2832

I have a breakpoint on if (_key === key) { and I've added a function breakpoint for console.log. The breakpoint for if (_key === key) { is hit, but the console.log near the end is not hit.

I tried clicking Reapply all breakpoints which did not help in this case.

function Safe(_key) {
    var _locked = true;
    var _value = null;

    this.unlock = function(key) {
        if (_key === key) {
            _locked = false;
        }
        return;
    };

    this.lock = function() {
        _locked = true;
    };

    Object.defineProperty(this, 'value', {
        get: function() {
            if (_locked) {
                console.log('thief detected!');
                process.exit(0);
            }
            return _value;
        },

        set: function(value) {
            if (_locked) {
                console.log('thief detected!');
                process.exit(0);
            }
            _value = value;
        }
    });
}

var safe = new Safe('mySecretPassword');

safe.unlock('mySecretPassword');
safe.value = 3;
console.log(safe.value);
safe.lock();

[weinand]

@alexandrudima console.log is not a global function, so it is not supported by node. Global functions are something like "setTimeout" or this:

// global function
foo = function(n) {
    return n;
}

The problem is, that the debugger UI does not seem to show errors for function breakpoint.

[alexdima]

ok, sorry my bad, would be nice to get a warning or something

[weinand]

@alexandrudima I just debugged this and the problem with "console.log" is that it is a native function. The debug adapter returns this error nicely, but it is not shown in the UI:

@isidorn we need to find a way to show these errors in the breakpoint viewlet.

[alexdima]

But now, with the previous comment, I am confused. So I can set a function breakpoint for setTimeout (which is a native function), but not for console.log which is also a native function? I would expect in case of native functions the program to stop at the call site.

[weinand]

@alexandrudima setTimeout is not a a native function:

It lives in timers.js.

Node behaves like this (even if this does not match our expectation).
Node's function breakpoints are weak, but since we have no other debug adapter that implements that part of the protocol, we can only test it with node-debug.

[alexdima]

oh, ok, sorry my bad again.

[isidorn]

Assigning to @bgashler1 so he can work on some initial designs for this

[bgashler1]

Here are some options. Please let me know your thoughts.

I also took a stab at making it more intuitive to deactivate all breakpoints and to visually make it more clear how that's different from enable or disable breakpoints.

1) Icon for breakpoint type; keep current UX

A) Show all breakpoints

If we choose Approach # 1, then this is my recommendation. In this case, adding icons for all breakpoints will visually separate exception settings from the list of individual breakpoints.

B) show only non-standard breakpoints

This is visually less noisy, but if we're going to do this I would recommend applying it to Option 2 or 3 below, because otherwise the breakpoints in the list can blend too much with the exception settings.

2) Add a parent node to deactivate all breakpoints

(animated GIF)

3) Divide breakpoints from exceptions; visual toggle to deactivate

(animated GIF)

[isidorn]

@bgashler1 Great work! For 3) we already have a visual toggle to deactivate, the only new thing here is the seperator for brakpoints - exception breakpoints and that you put breakpoitns on top. Right?

I personally like 1A the best and would like to try it out. The only thing that worries me is that all that red might draw the unnecessery attention of a user. Maybe we show this decorations only when a debug session is active - otherwise not show anything (same as we do now).

2. Not a big fan of this idea, I do not think the added functionality gain is worth of using a more complex ui representation

3. Like it, brings a nice visual separation

@weinand @bpasero opinions?

[bpasero]

Cool stuff. I think it would be nice to visually separate exception breakpoints from the rest! I also think if we chose to show the breakpoint logo we should do it for all breakpoints, but I would have to see it in action to tell for sure. I also wonder if the checkbox should come first and then the icon?

[isidorn]

We now show the state of breakpoins via icons in the viewlet.
I have opened an additional item for the function breakpoint icon #43763