smartStep should only skip unmapped lines in files that have sourcemaps

[digeff]

It would be great if smartstepping would do something smart for JavaScript projects, or JavaScript files in a mixed JS/TS Project

[roblourens]

or JavaScript files in a mixed JS/TS Project

Original assumption for smartStep was that the sourcemapped code is the user code, and everything else should be skipped. But I can imagine someone who only wants to skip unmapped portions of files with sourcemaps, e.g. TS injected async/await code. Especially now that we have skipFiles for people who want to skip node internal files, etc.

A quicker fix for VS's specific issue, can you disable skipFiles when no .ts files are in the project?

[digeff]

I fixed VS by disabling smartstep, so we are not in a hurry. We'd like eventually to be able to use this feature in JS and mixed projects though...

[kkeri]

But I can imagine someone who only wants to skip unmapped portions of files with sourcemaps

I want to do this. I guess source maps become more and more important, so it's a good direction to improve their support, and smartstep is a logical step on this way. But don't make source maps viral.

For example, I can write my tests in pure JS, or I can work on interplaying packages, some of them written in pure JS, others transpiled. In that case I'd like to enjoy the benefit of skipping generated blablah, but I still want to visit my code.

[roblourens]

By default, smartStep should step over unmapped lines in a file that has a sourcemap, but not step through a .js file that doesn't have a sourcemap.

I can't decide whether we should rely on skipFiles for stepping through all other files - someone can add node_modules and node_internals and whatever else they need - or if smartStep should have a mode that gives it the current behavior. We could have "smartStep": "unmappedLines"/"unmappedFiles" or something like that. Thoughts @weinand?

[roblourens]

@digeff I want to make this change in vscode to see what feedback we get

By default, smartStep should step over unmapped lines in a file that has a sourcemap, but not step through a .js file that doesn't have a sourcemap.

Do you enable smartStep for VS? I think that's the behavior you want but I wouldn't want to break anything for you before a release.

[digeff]

Currently smartStep is disabled in VS.
We'd love to start using this functionality in the future, we just need to figure out the best way to do it.

Feel free to change this anyway you need.

[roblourens]

I'd like to fix this for June.

smartStep could become a complex property with multiple modes. A mode for skipping unmapped lines in files with sourcemaps (async/await in TS), a mode for lines in unmapped files (e.g. node internals or node_modules, although this is better served by using skipFiles) and we can leave it open for other modes in the future. Another might be rejected promises that incorrectly break as if they are uncaught exceptions in some cases (we have a checkbox in the breakpoints view for these in the EH debugger).

With this change, smartStep can look like this:

"smartStep": {
  "unmappedLines": true,            // skip unmapped lines in files with sourcemaps (the async/await TS case)
  "filesWithoutSourcemaps": false   // skip lines in files without sourcemaps
}

Which would let me skip async/await code injected by TS or babel, but not skip .js files in node_modules. This is the usecase that is really useful for smartStep currently.

For backcompat with users' existing configs, "smartStep": true would have the same behavior as it does currently. This would be equivalent to

"smartStep": {
  "unmappedLines": true,
  "filesWithoutSourcemaps": true
}

Then after a month or so of stabilization, I'd like to enable this "unmappedLines" mode by default. People expect to step through their source files without randomly seeing generated lines in that same file, and they file issues on this on occasion. Debugging with smart step for these cases is a much nicer experience.

Questions -

• How critical is it to keep the behavior of "smartStep": true the same as it has been?
  • It would be nicer to say that "smartStep": true would only enable the "unmappedLines". But there are probably lots of people with that property set who would see a behavior change. Or, it might not be a big deal.
• Any suggestions for better names for the two smartStep properties?
• Is it confusing/overkill that "filesWithoutSourcemaps" exists, when skipFiles also exists? They can cover different cases but there is a lot of overlap in their scenarios. skipFiles is what we should push people towards (like with node_modules/**/*.js or <node_internals>).

Thoughts @weinand, @auchenberg, @isidorn?