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

Enable Loaded Scripts for VS Live Share #180

Merged
merged 3 commits into from Jun 8, 2018

Conversation

Projects
None yet
4 participants
@jramsay
Contributor

jramsay commented May 8, 2018

Enable Loaded Scripts for VS Live Share

Issue: When VS LiveShare is installed the Loaded Scripts pane no longer shows up when the host is debugging, irrespective of if they are sharing or not. This is because the debugType is 'vslsShare' instead of one of the known supported types.

Fix: When the debug type is 'vslsShare' attempt to retrieve the real debug type from the custom debugSessionInfo's configurationProperties.

@jramsay

This comment has been minimized.

Contributor

jramsay commented May 8, 2018

@roblourens, @weinand, @rodrigovaras: can you please take a look at this?

@weinand weinand self-assigned this May 8, 2018

@weinand weinand added this to the May 2018 milestone May 8, 2018

private async isSupportedDebugType(debugType: string | undefined, session: vscode.DebugSession): Promise<boolean> {
if (debugType === 'vslsShare') {
const debugSessionInfo = session ? await session.customRequest('debugSessionInfo', {}) : undefined;

This comment has been minimized.

@jramsay

jramsay May 8, 2018

Contributor

As per conversation with Rodrigo will add a Try-Catch around the customRequest in case this changes in future.

@weinand

This comment has been minimized.

Member

weinand commented May 31, 2018

I was on vacation in May, I will merge this next week.

Optimization: Retrieve the debugType directly using a customRequest i…
…nstead of from the configurationProperties
@weinand

This comment has been minimized.

Member

weinand commented Jun 8, 2018

This relies on the not properly documented 'setContext' API:
Microsoft/vscode#10471

@weinand weinand merged commit 52723ce into Microsoft:master Jun 8, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
@weinand

This comment has been minimized.

Member

weinand commented Jun 8, 2018

Thanks for the PR.
It will become available on the next Insider release (Monday).

@@ -89,6 +86,14 @@ export class LoadedScriptsProvider implements TreeDataProvider<BaseTreeItem> {
getTreeItem(node: BaseTreeItem): TreeItem {
return node;
}
private async isSupportedDebugType(debugType: string | undefined, session: vscode.DebugSession): Promise<boolean> {
if (debugType === 'vslsShare') {

This comment has been minimized.

@auchenberg

auchenberg Jun 8, 2018

Do we really want to have Live Share specific logic in Debug Adaptors? This feels like a slippery slope...

This comment has been minimized.

@weinand

weinand Jun 8, 2018

Member

This is a temporary measure while the LoadedScript explorer still lives in node-debug and provides a service to other well-known DAs.
Moving the LoadedScript explorer into VS Code core is on our 6 month roadmap and will make this code unnecessary.

The previous code from node-debug suffers from a similar problem:

{
	"id": "extension.node-debug.loadedScriptsExplorer",
	"name": "%loaded.scripts.view.name%",
	"when": "inDebugMode && debugType =~ /^(node|node2|extensionHost|chrome)$/"
}

It refers to the Chrome Debugger extension (which is not built-in).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment