Improving the UI for "step in targets"

[connor4312]

On my plate is @hediet's Debugger: Smart Step Into issue. We already support this in DAP via the step in targets, but the UI in VS Code could use some polish.

Today, step in targets is only surfaced through a context menu actions that are available when paused and supportsStepInTargetsRequest is true:

The result of this is a context menu, which allows the user to select one of the targets by their label

---

I suggest the following:

1. Add the command to the command palette (added in debug: improve step in target UI #152131) and Debug toolbar. This is our "step in" icon
   

   Maybe an icon like this would work:

   

   Alternatively we could use a dropdown button, but I'm not the biggest fan of throwing those around everywhere, especially for actions the user may want to repeat frequently.

2. Allow for the step in range to be correlated with a position on the line. Currently the StepInTarget only has a label. We could match that as text to contents of the line, but this wouldn't work if there's repetitive content like find in find(arr1, a => find(arr2, a => a == b)

   Therefore, I suggest (and will if we agree on a direction!) making a proposal to add an option line, column, endLine, and endColumn options to the StepInTarget akin to the GoToTarget. Proposed in Add additional location information to StepInTarget debug-adapter-protocol#274

3. With that change, we can now have commands to step into the target in the current cursor position. When the "step in targets" command/button is executed, it can open quickpick. As items in the quickpick are focused, the editor reveals and selects the document range that the target refers to. (added in debug: improve step in target UI #152131)

[roblourens]

Makes sense to me 👍

[roblourens]

I see there is some discussion about this in the other issue but did you work out how to come up with targets for js-debug? Ignoring getters doesn't seem great to me but there's probably no way around that

[weinand]

@connor4312 yes, I was thinking of your option 2 when working on the DAP proposal initially. Ideally the cursor position would determine the target for the "step in". May be it is not even necessary to have a new "step in targets" action in the toolbar: if the cursor is in a valid "step-in-target" range, then VS Code will automatically use "step in target" instead of "step in".

Since I didn't want to add additional "position" properties to the DAP request, I decided to start simple and add more properties when needed.

[roblourens]

May be it is not even necessary to have a new "step in targets" action in the toolbar: if the cursor is in a valid "step-in-target" range, then VS Code will automatically use "step in target" instead of "step in".

I wouldn't want to trigger it on accident. Especially tricky since we move the cursor to the paused location as you step.

[connor4312]

May be it is not even necessary to have a new "step in targets" action in the toolbar: if the cursor is in a valid "step-in-target" range, then VS Code will automatically use "step in target" instead of "step in".
...
I wouldn't want to trigger it on accident.

I agree that this isn't very obvious to me. If I didn't know about step in targets I would probably be quite confused if this happened.

We could always have the button in the debug toolbar be under an alt/option key hold, like we used to have with stop/disconnect. We could also allow alt+clicking in the line to step into a target at the given position. I like the similarity of having the same key being used for both of the mechanisms. However neither are very discoverable.

[weinand]

@roblourens @connor4312 you both have convinced me that a "step in targets" only based on caret position is too much magic and might be confusing.
So let's go with the separate command first and explore shortcuts like alt-clicking later.

[alexr00]

Verified using mock debug. I didn't see a new button in the debug toolbar, but I verified from the context menu and from the command palette.

[hediet]

Is this supported for JS already?

I would have expected to see "inner" there:

[connor4312]

Yes, it's supported. We exclude the location you're currently paused at, though we could special case that to just do an ordinary "step in". I'll make an issue for it.

[hediet]

We exclude the location you're currently paused at, though we could special case that to just do an ordinary "step in". I'll make an issue for it.

Not sure I follow. Step in here would step into "bar", no? What I actually want and cannot do with the normal "Step in", is to step into "inner" (which I should have called "outer" btw).

Nvm, it works here:

function main() {
    document.writeln('test1');
    bar();
    outer(bar(1), bar(2));
    document.writeln('test2');
}

function outer() {

}

function bar(val) {
    document.writeln('bar1' + val);
    document.writeln('bar2');
    document.writeln('bar3');
}

main();

Very cool. However, a modified+click solution would be the icing on the cake.

[connor4312]

Agreed 🙂 Some polish next iteration I think