"smartStep" skipped frames are not grayed out

[roblourens]

I'm seeing that frames skipped by smartStep are not grayed out, but frames from skipFiles are. This is in 1.30. The DA is sending the right thing and you can see this info in the tooltip.

Here's an example of DA output (not from the screenshot)

			"id": 1065,
			"name": "(anonymous function)",
			"source": {
				"name": "nodeDebugAdapter.js",
				"path": "/Users/roblou/code/vscode-node-debug2/out/src/nodeDebugAdapter.js",
				"origin": "(skipped by 'smartStep')",
				"presentationHint": "deemphasize"
			},
			"line": 10,
			"column": 71
		}, {
			"id": 1066,
			"name": "__awaiter",
			"source": {
				"name": "nodeDebugAdapter.js",
				"path": "/Users/roblou/code/vscode-node-debug2/out/src/nodeDebugAdapter.js",
				"origin": "(skipped by 'smartStep')",
				"presentationHint": "deemphasize"
			},
			"line": 6,
			"column": 12
		}, {
			"id": 1067,
			"name": "terminateSession",
			"source": {
				"name": "nodeDebugAdapter.js",
				"path": "/Users/roblou/code/vscode-node-debug2/out/src/nodeDebugAdapter.js"
			},
			"line": 492,
			"column": 16
		}, {
			"id": 1068,
			"name": "chrome.Runtime.on.params",
			"source": {
				"name": "nodeDebugAdapter.ts",
				"path": "/Users/roblou/code/vscode-node-debug2/src/nodeDebugAdapter.ts"
			},
			"line": 308,
			"column": 22
		}, {
			"id": 1069,
			"name": "emitOne",
			"source": {
				"name": "events.js",
				"path": "<node_internals>/events.js",
				"sourceReference": 1001,
				"origin": "read-only core module (skipped by 'skipFiles')",
				"presentationHint": "deemphasize"
			},
			"line": 116,
			"column": 13
		}, {
			"id": 1070,
			"name": "emit",
			"source": {
				"name": "events.js",
				"path": "<node_internals>/events.js",
				"sourceReference": 1001,
				"origin": "read-only core module (skipped by 'skipFiles')",
				"presentationHint": "deemphasize"
			},
			"line": 211,
			"column": 7
		}, {

[isidorn]

Looking at the code I see no difference in handling these. Since all of them have a "presentationHint": "deemphasize", so they are equel to the vscode frontend.

Can you please provide a reproducable sample so I can investigate further on my machine?
An alternative is that you just put a brekapoint here and check why the disabled class does not get toggled.

[roblourens]

Problem seems to come from here:

source.presentationHint is not read

[isidorn]

Good catch, however I think that is fine.
Please note that we have the presentationHint both on the stackFrame and on the Source which is somehwat confusing.
In this particular case the source object contains the appropriate presentationHint and that is passed inside the stack frame properly. When we are rendering the stackFrame at a code pointer I gave above, we are acutally checking the presentationHint of the Source object inside. And that one should be correct in this case, right?

[roblourens]

At that line in callStackView.ts, stackFrame.source.presentationHint is undefined. So it seems to be correct when the object is created but becomes wrong at some point between that and being rendered.

[isidorn]

@roblourens can you provide some reproducable steps so I try to investigte more?
Thanks

[roblourens]

I see this for all smartStep frames

[isidorn]

@roblourens cool. But do you have an example for which I can easily get into this situation? Asking becuase I do not and I would prefer you give me an example than me setting this up.
Thanks

[roblourens]

Sure, clone this: https://github.com/roblourens/repro65025

Set BPs on lines 6, 11, and 16.

No decoration on the unmapped .js frames when at lines 6 and 11:

But they are decorated correctly at 16:

And in both cases, the DA is returning basically the same thing,

"origin": "(skipped by 'smartStep')",
"presentationHint": "deemphasize"

for all the relevant files.

[isidorn]

@roblourens thanks for this great steps. I can nicely repro.
After investigation the issue is here

So this issue sort of contradicts with #42139

The problem is that we currently do not allow multiple source objects for the same resource. Only one.

As a workaround I suggest that you put the presentation hint of deemphasize on the stack frame when it happens that some parts of the call stack you want to deemphasize and some not - while all of them living in the same file.

Would that work for you?
If not I am open for suggestions on how to fix this

[roblourens]

when it happens that some parts of the call stack you want to deemphasize and some not - while all of them living in the same file.

Is that happening here? I think only the .js is deempasized and the .ts is not.

[isidorn]

Yes. test.js is comming in 3 times, and only the 1 & 2 have presentationHint.

asyncFn1 does not have presentationHint set

Similar situation is for the second breakpoint.

[roblourens]

Ohhh... that is wrong, this actually exposed that my calculations for the stack frame labels are off by one line in the adapter.

[roblourens]

I don't think there's a valid scenario for these DAs where some parts of a file will be skipped and some parts won't.

But to be safe, should I always just set presentationHint on the frame instead of the source?

[isidorn]

For now I would leave it on the source since we actually do not specify in the protocol that a presentationHint on the stack frame can be deemphesized. We might need to change that.

https://github.com/Microsoft/vscode/blob/fd9fbbfd507d7db9eafd86fc746b574e1b86d790/src/vs/workbench/parts/debug/common/debugProtocol.d.ts#L1340