Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debugging - UI/protocol support for "blackbox script"/"Just my code" #14728

Closed
roblourens opened this issue Oct 31, 2016 · 11 comments
Closed

Debugging - UI/protocol support for "blackbox script"/"Just my code" #14728

roblourens opened this issue Oct 31, 2016 · 11 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Milestone

Comments

@roblourens
Copy link
Member

roblourens commented Oct 31, 2016

Chrome devtools calls this "Blackboxed scripts" and VS/F12 use "Library code" and "Just my code".

This feature has been requested several times - examples,

It could be implemented entirely in the debug adapter, with a new option in the launch config, and I hope to do that to try it out. But when I've used it, I've found it most useful when I can mark a script as "library code" on the fly. For this, we would need a context menu option on the callstack which can mark/unmark a particular script as library code, debug protocol support, and a toggle button to globally enable/disable the feature. (For the last item, we could use the 'exceptionBreakpointFilters' - is there anything wrong with reusing that for something besides exception breakpoints?) We also need to persist these.

This is implemented by the chrome debugging protocol. For reference it looks like this - https://chromedevtools.github.io/debugger-protocol-viewer/v8/Debugger/#method-setBlackboxPatterns - but we really just need a message that includes a DebugProtocol.Source and the library code state. I think this is of general interest since VS implements it for Javascript, C++, C# and other languages.

Thoughts/comments?

@roblourens roblourens added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues labels Oct 31, 2016
@octref
Copy link
Contributor

octref commented Oct 31, 2016

My use case is mostly black-boxing everything required from node_modules.

@roblourens roblourens changed the title Debugging - Allow "blackbox script"/"Just my code" Debugging - UI/protocol support for "blackbox script"/"Just my code" Oct 31, 2016
@weinand weinand added this to the November 2016 milestone Oct 31, 2016
@weinand
Copy link
Contributor

weinand commented Oct 31, 2016

@roblourens makes sense, I'll look into this. Protocol change should be possible for November.

@roblourens
Copy link
Member Author

Cool. I can write something more comprehensive if needed. Another optional UI feature that I didn't mention is that VS and F12 collapse library code frames into one, so that you see

myCode.js
[library code]
myCode.js

instead of

myCode.js
react.js
react.js
react.js
myCode.js

but you can double-click to expand

@weinand
Copy link
Contributor

weinand commented Nov 8, 2016

@roblourens as you've pointed out, there are two 'UIs' possible for this feature request:

  • a static launch.json based configuration option (e.g. glob patterns) and
  • an interactive UI based on stack frames.

All feature requests from above ask only for the static mechanism ('black box everything').
And my own experience confirms this: I would blacklist 'node_modules' and other folders with low level code. But on top of that I would like to exclude additional files and/or functions on a case by case basis.

Having only the interactive UI available would require to exclude each and every file manually (which would be a pain).
Having only the launch.json based configuration option available, would result in the same functionality as the interactive approach, but adding individual files to the launch.json would be laborious.

I propose that we first implement the static launch config based approach for node-debug and node-debug2 (Nov) and then add the interactive UI (Dec/Jan) after we have resolved the following questions/issues:

  • Are the blackbox items persisted across sessions? If yes:
    • where and how?
    • how is the persisted list cleared?
  • What is the granularity of the items added to the blackbox list? Files or functions?

The CDP API for Blackbox patterns is marked as experimental. So node-debug2 has to deal with the situation that the API might disappear in newer versions of node.

@roblourens
Copy link
Member Author

roblourens commented Nov 8, 2016

I think it's unlikely that they'd remove the API entirely, but if they did, I think it would be easy to implement the same as smartStep. I think the remaining work on the debug adapter (without adding anything to the protocol) is-

  • Resolve patterns for sourcemapped files to a region inside a script
  • Use glob patterns instead of regexes

And another UI thing to think about is that if we blackblox a script via the call stack, how do we unmark it? In other tools you do this by finding it in the file explorer, but that doesn't feel like the right model for us. Plus there are scripts that don't show up in our file explorer.

Another option would be to add another section to the debug viewlet (like call stack, breakpoints, etc) where patterns could be added and removed.

@roblourens
Copy link
Member Author

Protocol issue here - microsoft/vscode-debugadapter-node#87

Let's use this issue for the context menu item on the call stack.

It could say "Library code" with a checkmark, or "Mark as library code"/"Unmark ...". Or something else. I like the "library code" terminology over the "blackbox script" terminology.

@isidorn
Copy link
Contributor

isidorn commented Nov 10, 2016

'Mark as library code' might be a bit cryptic to the user as to how that changes the debugging experience for him. 'Blackbox' is more intuitive.
If we go into the 'Library' wording we should gray out the stack frames and on hover provide an explenation to the user as to what is going on

@roblourens
Copy link
Member Author

'Library' is what's used by VS and F12 - it might be useful to match them. Some web developers will be more familiar with 'blackbox', but if we go that direction, we shouldn't use 'scripts'.

@weinand
Copy link
Contributor

weinand commented Nov 17, 2016

@roblourens since the UI for this feature will be mostly based on context menu actions, I suggest that we try to build this first as a pure extension (and not as a generic VS Code debugger UI).
This approach helps us in two ways: first we are forced to open our extension API to support this and second we can experiment with the UI for one debug extension before we are adding more generic functionality to VS Code.
As a consequence of this approach we do not have to extend the debug protocol officially but we can just start with some private protocol that is only known to the corresponding extension.

However, before we can do this, the following problems must be solved:

What do you think?

/cc @isidorn

@roblourens
Copy link
Member Author

Good idea, sounds like a good time to solve those problems.

@weinand
Copy link
Contributor

weinand commented Nov 22, 2016

This feature request is for January 2017 and depends on #15657 and #15656.

Let's use #3215 for the static "blackbox script" feature which relies on an option in the launch configuration. This will be the item to put on the November 2016 plan.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants