Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Emit "scriptLoaded" event #118

Closed
weinand opened this issue Jul 12, 2017 · 9 comments
Closed

Emit "scriptLoaded" event #118

weinand opened this issue Jul 12, 2017 · 9 comments
Assignees
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Jul 12, 2017

To finish the "Loaded Scripts Explorer" I'd need a custom scriptLoaded event from "node-debug2":

Here is the code from node-debug:

this._node.on('afterCompile', (event: NodeV8Event) => {
    this.sendEvent(new Event('scriptLoaded', { path: this._scriptToPath(event.body.script) }));
});

The "path" attribute should use the same form as the paths used in the getLoadedScripts request.

Tomorrows insider should have the "Loaded Scripts Explorer" (and if not VS Code will ignore the event).

(Since I've noticed that the result format of the getLoadedScripts request is utterly redundant, I'm planning to simplify this today. But I will create a separate issue for this and I will continue to support the old and new format in parallel..).

@roblourens
Copy link
Member

I'm already sending an event of type 'script' for VS, I guess these are both private/experimental extensions to the protocol. Hopefully we can consolidate them in the future, per microsoft/vscode-debugadapter-node#108

@roblourens
Copy link
Member

roblourens commented Jul 26, 2017

I can't get this to work. I'm sending scriptLoaded events but usually only the <node_internals> scripts show up. Sometimes everything shows up, but only about 1/10 times. It seems like it's also calling getLoadedScripts initially, and those scripts show up, but that's called before my .../app.js scripts are loaded.

I debugged the extension host side for a bit, and it seems to be successfully adding the items to the tree, so I'm not sure what the problem is.

And it seems more likely to repro when I'm debugging the event handler for awhile, so maybe there's a timing issue.

@weinand
Copy link
Contributor Author

weinand commented Jul 26, 2017

@roblourens you are probably running into Sandeep's bad fix for microsoft/vscode#31191
Please try again with today's Insiders.

@weinand
Copy link
Contributor Author

weinand commented Jul 26, 2017

With a good fix for microsoft/vscode#31191 and some node2 specific glue, the loaded scripts show and update correctly for the "inspector" protocol.

You might want to consider to use the same path format in the event as in the 'getLoadedScripts' response.

@roblourens
Copy link
Member

I'll send a scriptLoaded event so I can leave the VS event alone, and send the right path for vscode. It's easy and I have the change ready anyway :)

@roblourens roblourens reopened this Jul 26, 2017
@weinand
Copy link
Contributor Author

weinand commented Jul 26, 2017

even better!
BTW, you can check the InitializeRequest.clientId if you only want to emit custom events if the corresponding client listens:
see https://github.com/Microsoft/vscode-debugadapter-node/wiki/Client-IDs

@roblourens
Copy link
Member

There can be lots of eval scripts:

image

What if vscode-node-debug puts them under a root item called <eval>, or "Dynamic scripts"? Or, should the adapter change the name of these and add <eval>/?

@weinand
Copy link
Contributor Author

weinand commented Jul 26, 2017

"legacy" puts the unnamed scripts under <node_internals> so they don't clutter the top level:

2017-07-26_17-41-40

But we can consider to introduce another "well known" category: I like the <eval>.

If you like to try this, you can do this already in node-debug2 without touching "loadedScripts.ts". Just return the paths for eval scripts with the <eval> prefix and the tree will show them correctly (only sorting will be off because "loadedScripts.ts" treats <node_internals> in a special way, but we could generalise this to sort all <*> at the end).
And you'll have to recognise all incoming paths starting with <eval> as well...

@roblourens
Copy link
Member

@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants