When using Run to Cursor, then there is no inline instruction pointer decoration

[egamma]

Testing #20793

I would have expected to see an inline instruction pointer decoration when I use the 'Run to Cursor' command.

[isidorn]

This is not related to run to cursor command but with our heuristic of when to display the inline instruction pointer.
We only show the inline instruction pointer if the previous stop was on the same line.

We do not show the inline instruction pointer when the stop event has a column. Since then we would display the inline decoration all the time (all node stop events have column specified) and that is too much noise.

Thus closing as designed. Let me know if you have ideas on how to improve this and I can reopen

[egamma]

The explanations makes sense from an implementation perspective. It isn't obvious from the user's perspective.

We do not show the inline instruction pointer when the stop event has a column.

Can you distinguish between a stop event on the first character on the line and a stop event in the middle of the line? If you can, then you only show the inline instruction pointer when the column is inside the line.

Showing the inline instruction pointer also for run to cursor would be a nice feature. I'll reopen the issue and label it as a feature request.

[isidorn]

Can you distinguish between a stop event on the first character on the line and a stop event in the middle of the line?

Yes I can, but the issue is that almost every stop event has a column which is not on the first character on the line.

Agree, that from the user perspective this is not so obvious so let's keep it open as a feature request.

[weinand]

@isidorn we have to fix this (the heuristic).
I've experienced the broken heuristic in mono-debug with multi threaded programs.
Let's use this issue as the umbrella for now.

[weinand]

The problem is basically this:

In the DAP it is specified that a source location has a mandatory line and an optional column attribute.

VS Code could use this in the following way: if no column attribute is returned from the DA, VS Code shows only the line indicator (in the editor margin), and if a column attribute is provided VS Code shows a column indicator (in addition to the line indicator).

This naïve approach results in the problem, that a column indicator is shown even if there is only a single statement in a line (and there is no real value in showing the column indicator).

To avoid this VS Code uses a heuristic: the column indicator is only shown if the same line is hit more than once.

This heuristic works most of the time but we have seen situations where the column indicator shows up even when it shouldn't (e.g. multiple threads hitting the same line).

To fix these issues we tried to remove the heuristic from VS Code and delegate the problem to the backend (DA). So it would be the responsibility of the DA to only provide the column location if there are multiple statements in a line.

When trying to implement this logic in the node (legacy) DA, it became clear that we would have to implement another heuristic (now in the backend) because node would just not provide the information whether a line has one or more statement locations (and needs to return a column attribute or not).

@roblourens what is the situation in the new (inspector) protocol? Could the node (inspector) DA more easily return the column attribute only when it is needed?

/cc @isidorn

[roblourens]

No, I don't think the inspector protocol has anything to help here. It always returns the column, and chrome devtools always shows the column. I don't even have an easy way to get the text of the line.

One possibility is using the same API they have to implement column BPs. It's called getPossibleBreakpoints and returns a list of places where a breakpoint can be set in a line, which, I think, would be the same locations that you will step over. We could show the indicator if we are not at the first location. But this API isn't in stable Chrome or Node yet, and I don't know if it will be fast enough to call on every step.

[weinand]

Yes, getPossibleBreakpoints seems to be what I was thinking of (and there is another DAP request that could profit from that: GotoTargetsRequest).

We should revisit this when getPossibleBreakpoints has found its way into Chrome and/or Node stable.

For March we've tried to improve the heuristic in the VS Code debugger UI.

[roblourens]

Actually it's in Chrome 57, which is stable as of this week. Not in any Node version, though. Reminds me that I should do column BPs next month.

[isidorn]

Makes sense and thanks for the details.
Unassigning myself since I have improved the heuristic on the vscode side and assigning to @roblourens and @weinand to look into improving this once the mentioned API lands

[roblourens]

Sorry for not following up, I didn't realize this was waiting on me. The Node 8 release is pushed out until the end of May so it can wait.

I'm using the new API to implement column BPs for Node now. Would it be fast enough to call on every step? It's very fast most of the time. Typical line of code - returns in less than 10ms. But for a very long line of code, like if you're stepping through minified code, it can take 1s or more to return. But it's easy enough to cache the result. So yeah, I think it would be totally possible to use this to decide when to show the indicator.

If we want to keep the heuristic for other adapters, we will probably need a switch to opt-in to this behavior.

If you have time to do this in April, I could enable it for chrome-debug so we can try it out.

[roblourens]

Do we still want to revisit this using getPossibleBreakpoints? Seems like a nice-to-have but I'm not sure it's worth extending the DAP for it, unless we also use it to show decorations for column bp options like chrome devtools does, and I think the issue for that was closed.

[roblourens]

@isidorn @weinand Is there anything else to do here now that we have breakpointLocations?

[weinand]

@isidorn Since DAP's "getPossibleBreakpoints" is now available, we should reconsider this feature request.