From 1399600df390437e7ff8ba21b08f12897d153a98 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Sat, 17 Oct 2020 00:08:42 -0700 Subject: [PATCH 1/2] Ensure debug adapter request/response sequence numbers tally (#14336) --- .../datascience/debugLocationTracker.ts | 101 ++++++++++-------- .../debugLocationTrackerFactory.ts | 4 +- .../datascience/jupyter/debuggerVariables.ts | 29 +++-- src/client/datascience/jupyterDebugService.ts | 13 ++- .../debugLocationTracker.unit.test.ts | 22 +++- 5 files changed, 107 insertions(+), 62 deletions(-) diff --git a/src/client/datascience/debugLocationTracker.ts b/src/client/datascience/debugLocationTracker.ts index 6642b3f44652..4246a9c4c275 100644 --- a/src/client/datascience/debugLocationTracker.ts +++ b/src/client/datascience/debugLocationTracker.ts @@ -1,19 +1,23 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; +import { injectable } from 'inversify'; import { DebugAdapterTracker, Event, EventEmitter } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; import { IDebugLocation } from './types'; // When a python debugging session is active keep track of the current debug location +@injectable() export class DebugLocationTracker implements DebugAdapterTracker { - private waitingForStackTrace: boolean = false; + protected topMostFrameId = 0; + protected sequenceNumbersOfRequestsPendingResponses = new Set(); + private waitingForStackTrace = false; private _debugLocation: IDebugLocation | undefined; private debugLocationUpdatedEvent: EventEmitter = new EventEmitter(); private sessionEndedEmitter: EventEmitter = new EventEmitter(); - constructor(private _sessionId: string) { + constructor(private _sessionId: string | undefined) { this.DebugLocation = undefined; } @@ -33,8 +37,22 @@ export class DebugLocationTracker implements DebugAdapterTracker { return this._debugLocation; } - // tslint:disable-next-line:no-any - public onDidSendMessage(message: DebugProtocol.ProtocolMessage) { + public onDidSendMessage(message: DebugProtocol.Response) { + if (this.isResponseForRequestToFetchAllFrames(message)) { + // This should be the top frame. We need to use this to compute the value of a variable + const topMostFrame = message.body.stackFrames[0]; + this.topMostFrameId = topMostFrame?.id; + this.sequenceNumbersOfRequestsPendingResponses.delete(message.request_seq); + // If we are waiting for a stack trace, check our messages for one + if (this.waitingForStackTrace) { + this.DebugLocation = { + lineNumber: topMostFrame?.line, + fileName: this.normalizeFilePath(topMostFrame?.source?.path), + column: topMostFrame.column + }; + this.waitingForStackTrace = false; + } + } if (this.isStopEvent(message)) { // Some type of stop, wait to see our next stack trace to find our location this.waitingForStackTrace = true; @@ -45,21 +63,23 @@ export class DebugLocationTracker implements DebugAdapterTracker { this.DebugLocation = undefined; this.waitingForStackTrace = false; } - - if (this.waitingForStackTrace) { - // If we are waiting for a stack track, check our messages for one - const debugLoc = this.getStackTrace(message); - if (debugLoc) { - this.DebugLocation = debugLoc; - this.waitingForStackTrace = false; - } - } } public onWillStopSession() { this.sessionEndedEmitter.fire(this); } + public onWillReceiveMessage(message: DebugProtocol.Request) { + if (this.isRequestToFetchAllFrames(message)) { + // VSCode sometimes sends multiple stackTrace requests. The true topmost frame is determined + // based on the response to a stackTrace request where the startFrame is 0 or undefined (i.e. + // this request retrieves all frames). Here, remember the sequence number of the outgoing + // request whose startFrame === 0 or undefined, and update this.topMostFrameId only when we + // receive the response with a matching sequence number. + this.sequenceNumbersOfRequestsPendingResponses.add(message.seq); + } + } + // Set our new location and fire our debug event private set DebugLocation(newLocation: IDebugLocation | undefined) { const oldLocation = this._debugLocation; @@ -70,7 +90,15 @@ export class DebugLocationTracker implements DebugAdapterTracker { } } - // tslint:disable-next-line:no-any + private normalizeFilePath(path: string): string { + // Make the path match the os. Debugger seems to return + // invalid path chars on linux/darwin + if (process.platform !== 'win32') { + return path.replace(/\\/g, '/'); + } + return path; + } + private isStopEvent(message: DebugProtocol.ProtocolMessage) { if (message.type === 'event') { const eventMessage = message as DebugProtocol.Event; @@ -82,34 +110,6 @@ export class DebugLocationTracker implements DebugAdapterTracker { return false; } - // tslint:disable-next-line:no-any - private getStackTrace(message: DebugProtocol.ProtocolMessage): IDebugLocation | undefined { - if (message.type === 'response') { - const responseMessage = message as DebugProtocol.Response; - if (responseMessage.command === 'stackTrace') { - const messageBody = responseMessage.body; - if (messageBody.stackFrames.length > 0) { - const lineNumber = messageBody.stackFrames[0].line; - const fileName = this.normalizeFilePath(messageBody.stackFrames[0].source.path); - const column = messageBody.stackFrames[0].column; - return { lineNumber, fileName, column }; - } - } - } - - return undefined; - } - - private normalizeFilePath(path: string): string { - // Make the path match the os. Debugger seems to return - // invalid path chars on linux/darwin - if (process.platform !== 'win32') { - return path.replace(/\\/g, '/'); - } - return path; - } - - // tslint:disable-next-line:no-any private isContinueEvent(message: DebugProtocol.ProtocolMessage): boolean { if (message.type === 'event') { const eventMessage = message as DebugProtocol.Event; @@ -125,4 +125,21 @@ export class DebugLocationTracker implements DebugAdapterTracker { return false; } + + private isResponseForRequestToFetchAllFrames(message: DebugProtocol.Response) { + return ( + message.type === 'response' && + message.command === 'stackTrace' && + message.body.stackFrames[0] && + this.sequenceNumbersOfRequestsPendingResponses.has(message.request_seq) + ); + } + + private isRequestToFetchAllFrames(message: DebugProtocol.Request) { + return ( + message.type === 'request' && + message.command === 'stackTrace' && + (message.arguments.startFrame === 0 || message.arguments.startFrame === undefined) + ); + } } diff --git a/src/client/datascience/debugLocationTrackerFactory.ts b/src/client/datascience/debugLocationTrackerFactory.ts index d31287410c1a..59bc96d1ab7c 100644 --- a/src/client/datascience/debugLocationTrackerFactory.ts +++ b/src/client/datascience/debugLocationTrackerFactory.ts @@ -50,7 +50,9 @@ export class DebugLocationTrackerFactory implements IDebugLocationTracker, Debug } private onSessionEnd(locationTracker: DebugLocationTracker) { - this.activeTrackers.delete(locationTracker.sessionId); + if (locationTracker.sessionId) { + this.activeTrackers.delete(locationTracker.sessionId); + } } private onLocationUpdated() { diff --git a/src/client/datascience/jupyter/debuggerVariables.ts b/src/client/datascience/jupyter/debuggerVariables.ts index 6b4093774ad1..573aa699b7df 100644 --- a/src/client/datascience/jupyter/debuggerVariables.ts +++ b/src/client/datascience/jupyter/debuggerVariables.ts @@ -10,6 +10,7 @@ import { traceError } from '../../common/logger'; import { IConfigurationService, Resource } from '../../common/types'; import { sendTelemetryEvent } from '../../telemetry'; import { DataFrameLoading, Identifiers, Telemetry } from '../constants'; +import { DebugLocationTracker } from '../debugLocationTracker'; import { IConditionalJupyterVariables, IJupyterDebugService, @@ -23,17 +24,19 @@ const DataViewableTypes: Set = new Set(['DataFrame', 'list', 'di const KnownExcludedVariables = new Set(['In', 'Out', 'exit', 'quit']); @injectable() -export class DebuggerVariables implements IConditionalJupyterVariables, DebugAdapterTracker { +export class DebuggerVariables extends DebugLocationTracker + implements IConditionalJupyterVariables, DebugAdapterTracker { private refreshEventEmitter = new EventEmitter(); private lastKnownVariables: IJupyterVariable[] = []; - private topMostFrameId = 0; private importedIntoKernel = new Set(); private watchedNotebooks = new Map(); private debuggingStarted = false; constructor( @inject(IJupyterDebugService) @named(Identifiers.MULTIPLEXING_DEBUGSERVICE) private debugService: IDebugService, @inject(IConfigurationService) private configService: IConfigurationService - ) {} + ) { + super(undefined); + } public get refreshRequired(): Event { return this.refreshEventEmitter.event; @@ -110,10 +113,12 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda ); // Results should be the updated variable. - return { - ...targetVariable, - ...JSON.parse(results.result.slice(1, -1)) - }; + return results + ? { + ...targetVariable, + ...JSON.parse(results.result.slice(1, -1)) + } + : targetVariable; } public async getDataFrameRows( @@ -166,6 +171,7 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda // tslint:disable-next-line: no-any public onDidSendMessage(message: any) { + super.onDidSendMessage(message); // When the initialize response comes back, indicate we have started. if (message.type === 'response' && message.command === 'initialize') { this.debuggingStarted = true; @@ -174,9 +180,6 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda // tslint:disable-next-line: no-suspicious-comment // TODO: Figure out what resource to use this.updateVariables(undefined, message as DebugProtocol.VariablesResponse); - } else if (message.type === 'response' && message.command === 'stackTrace') { - // This should be the top frame. We need to use this to compute the value of a variable - this.updateStackFrame(message as DebugProtocol.StackTraceResponse); } else if (message.type === 'event' && message.event === 'terminated') { // When the debugger exits, make sure the variables are cleared this.lastKnownVariables = []; @@ -240,12 +243,6 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda } } - private updateStackFrame(stackResponse: DebugProtocol.StackTraceResponse) { - if (stackResponse.body.stackFrames[0]) { - this.topMostFrameId = stackResponse.body.stackFrames[0].id; - } - } - private async getFullVariable(variable: IJupyterVariable, notebook: INotebook): Promise { // See if we imported or not into the kernel our special function await this.importDataFrameScripts(notebook); diff --git a/src/client/datascience/jupyterDebugService.ts b/src/client/datascience/jupyterDebugService.ts index ca774a2c041f..091d13f7a437 100644 --- a/src/client/datascience/jupyterDebugService.ts +++ b/src/client/datascience/jupyterDebugService.ts @@ -260,7 +260,18 @@ export class JupyterDebugService implements IJupyterDebugService, IDisposable { } private sendToTrackers(args: any) { - this.debugAdapterTrackers.forEach((d) => d.onDidSendMessage!(args)); + switch (args.type) { + case 'request': + this.debugAdapterTrackers.forEach((d) => { + if (d.onWillReceiveMessage) { + d.onWillReceiveMessage(args); + } + }); + break; + default: + this.debugAdapterTrackers.forEach((d) => d.onDidSendMessage!(args)); + break; + } } private sendCustomRequest(command: string, args?: any): Promise { diff --git a/src/test/datascience/debugLocationTracker.unit.test.ts b/src/test/datascience/debugLocationTracker.unit.test.ts index f712d9d0f189..aa3b52a3a8d7 100644 --- a/src/test/datascience/debugLocationTracker.unit.test.ts +++ b/src/test/datascience/debugLocationTracker.unit.test.ts @@ -3,6 +3,7 @@ 'use strict'; //tslint:disable:max-func-body-length match-default-export-name no-any no-multiline-string no-trailing-whitespace import { expect } from 'chai'; +import { DebugProtocol } from 'vscode-debugprotocol'; import { DebugLocationTracker } from '../../client/datascience/debugLocationTracker'; import { IDebugLocation } from '../../client/datascience/types'; @@ -21,6 +22,7 @@ suite('Debug Location Tracker', () => { expect(debugTracker.debugLocation).to.be.equal(undefined, 'After stop location is empty'); + debugTracker.onWillReceiveMessage(makeStackTraceRequest()); debugTracker.onDidSendMessage(makeStackTraceMessage()); const testLocation: IDebugLocation = { lineNumber: 1, column: 1, fileName: 'testpath' }; @@ -40,12 +42,28 @@ function makeContinueMessage(): any { return { type: 'event', event: 'continue' }; } -function makeStackTraceMessage(): any { +function makeStackTraceMessage(): DebugProtocol.Response { return { type: 'response', command: 'stackTrace', + request_seq: 42, + success: true, + seq: 43, body: { - stackFrames: [{ line: 1, column: 1, source: { path: 'testpath' } }] + stackFrames: [{ id: 9000, line: 1, column: 1, source: { path: 'testpath' } }] + } + }; +} + +function makeStackTraceRequest(): DebugProtocol.Request { + return { + type: 'request', + command: 'stackTrace', + seq: 42, + arguments: { + levels: 1, + startFrame: 0, + threadId: 1 } }; } From 47054ec2d1f95cb119d5c70c64f8afd8db48ccd8 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Mon, 19 Oct 2020 09:17:58 -0700 Subject: [PATCH 2/2] Make data viewer openable from the variables window while debugging (#14406) --- build/ci/templates/globals.yml | 2 +- news/1 Enhancements/14406.md | 1 + package.json | 17 +++ package.nls.json | 1 + src/client/common/application/commands.ts | 2 + .../datascience/commands/commandRegistry.ts | 38 ++++++ src/client/datascience/constants.ts | 6 +- .../jupyterVariableDataProvider.ts | 18 +-- .../jupyterVariableDataProviderFactory.ts | 2 +- .../editor-integration/hoverProvider.ts | 2 +- .../interactive-common/dataViewerChecker.ts | 45 +++++++ .../intellisense/intellisenseProvider.ts | 2 +- .../interactive-common/interactiveBase.ts | 46 ++----- .../interactiveWindowTypes.ts | 7 +- .../datascience/jupyter/debuggerVariables.ts | 122 +++++++++++------- .../datascience/jupyter/jupyterVariables.ts | 22 ++-- .../datascience/jupyter/kernelVariables.ts | 10 +- .../jupyter/oldJupyterVariables.ts | 10 +- src/client/datascience/types.ts | 14 +- src/client/telemetry/index.ts | 3 + .../commands/commandRegistry.unit.test.ts | 6 + src/test/datascience/variableTestHelpers.ts | 21 +-- types/vscode.proposed.d.ts | 19 +++ 23 files changed, 278 insertions(+), 138 deletions(-) create mode 100644 news/1 Enhancements/14406.md create mode 100644 src/client/datascience/interactive-common/dataViewerChecker.ts diff --git a/build/ci/templates/globals.yml b/build/ci/templates/globals.yml index 03457023e99e..b77fa3605082 100644 --- a/build/ci/templates/globals.yml +++ b/build/ci/templates/globals.yml @@ -9,5 +9,5 @@ variables: VSC_PYTHON_WEBVIEW_LOG_FILE: '$(Build.ArtifactStagingDirectory)/pvsc_webview.log' CI_BRANCH_NAME: ${Build.SourceBranchName} npm_config_cache: $(Pipeline.Workspace)/.npm - vmImageMacOS: 'macOS-10.15' + vmImageMacOS: 'macOS-latest' TS_NODE_FILES: true # Temporarily enabled to allow using types from vscode.proposed.d.ts from ts-node (for tests). diff --git a/news/1 Enhancements/14406.md b/news/1 Enhancements/14406.md new file mode 100644 index 000000000000..2b4e2ab341bd --- /dev/null +++ b/news/1 Enhancements/14406.md @@ -0,0 +1 @@ +Make data viewer openable from the variables window context menu while debugging. diff --git a/package.json b/package.json index 1b6797dc09ff..d215f60ffe45 100644 --- a/package.json +++ b/package.json @@ -927,6 +927,11 @@ "title": "DataScience.latestExtension", "category": "Python" }, + { + "command": "python.datascience.showDataViewer", + "title": "%python.command.python.datascience.showDataViewer.title%", + "category": "Python" + }, { "command": "python.analysis.restartLanguageServer", "title": "%python.command.python.analysis.restartLanguageServer.title%", @@ -1550,6 +1555,11 @@ "category": "Python", "when": "false" }, + { + "command": "python.datascience.showDataViewer", + "category": "Python", + "when": "false" + }, { "command": "python.datascience.export", "title": "%DataScience.notebookExportAs%", @@ -1655,6 +1665,13 @@ "when": "view == python_tests && viewItem == suite && !busyTests", "group": "inline@0" } + ], + "debug/variables/context": [ + { + "command": "python.datascience.showDataViewer", + "group": "1_view", + "when": "debugProtocolVariableMenuContext == 'viewableInDataViewer'" + } ] }, "breakpoints": [ diff --git a/package.nls.json b/package.nls.json index 72b5ae1046cb..337f86570c5d 100644 --- a/package.nls.json +++ b/package.nls.json @@ -120,6 +120,7 @@ "Datascience.currentlySelectedJupyterInterpreterForPlaceholder": "current: {0}", "python.command.python.analysis.clearCache.title": "Clear Module Analysis Cache", "python.command.python.analysis.restartLanguageServer.title": "Restart Language Server", + "python.command.python.datascience.showDataViewer.title": "View Value in Data Viewer", "python.snippet.launch.standard.label": "Python: Current File", "python.snippet.launch.module.label": "Python: Module", "python.snippet.launch.module.default": "enter-your-module-name", diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 0eb6fcf54a8c..de3ff9a36281 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -6,6 +6,7 @@ import { CancellationToken, Position, TextDocument, Uri } from 'vscode'; import { Commands as LSCommands } from '../../activation/commands'; import { Commands as DSCommands } from '../../datascience/constants'; +import { IShowDataViewerFromVariablePanel } from '../../datascience/interactive-common/interactiveWindowTypes'; import { KernelConnectionMetadata } from '../../datascience/jupyter/kernels/types'; import { INotebookModel, ISwitchKernelOptions } from '../../datascience/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -205,4 +206,5 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.TrustNotebook]: [undefined | never | Uri]; [DSCommands.NotebookEditorExpandAllCells]: []; [DSCommands.NotebookEditorCollapseAllCells]: []; + [DSCommands.ShowDataViewer]: [IShowDataViewerFromVariablePanel]; } diff --git a/src/client/datascience/commands/commandRegistry.ts b/src/client/datascience/commands/commandRegistry.ts index ff9c64025dd8..d1778fe59995 100644 --- a/src/client/datascience/commands/commandRegistry.ts +++ b/src/client/datascience/commands/commandRegistry.ts @@ -5,9 +5,11 @@ import { inject, injectable, multiInject, named, optional } from 'inversify'; import { CodeLens, ConfigurationTarget, env, Range, Uri } from 'vscode'; +import { DebugProtocol } from 'vscode-debugprotocol'; import { ICommandNameArgumentTypeMapping } from '../../common/application/commands'; import { IApplicationShell, ICommandManager, IDebugService, IDocumentManager } from '../../common/application/types'; import { Commands as coreCommands } from '../../common/constants'; +import { traceError } from '../../common/logger'; import { IStartPage } from '../../common/startPage/types'; import { IConfigurationService, IDisposable, IOutputChannel } from '../../common/types'; @@ -15,11 +17,16 @@ import { DataScience } from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { Commands, JUPYTER_OUTPUT_CHANNEL, Telemetry } from '../constants'; +import { IDataViewerFactory } from '../data-viewing/types'; +import { DataViewerChecker } from '../interactive-common/dataViewerChecker'; +import { IShowDataViewerFromVariablePanel } from '../interactive-common/interactiveWindowTypes'; +import { convertDebugProtocolVariableToIJupyterVariable } from '../jupyter/debuggerVariables'; import { ICodeWatcher, IDataScienceCodeLensProvider, IDataScienceCommandListener, IDataScienceFileSystem, + IJupyterVariableDataProviderFactory, INotebookEditorProvider } from '../types'; import { JupyterCommandLineSelectorCommand } from './commandLineSelector'; @@ -30,6 +37,7 @@ import { JupyterServerSelectorCommand } from './serverSelector'; @injectable() export class CommandRegistry implements IDisposable { private readonly disposables: IDisposable[] = []; + private dataViewerChecker: DataViewerChecker; constructor( @inject(IDocumentManager) private documentManager: IDocumentManager, @inject(IDataScienceCodeLensProvider) private dataScienceCodeLensProvider: IDataScienceCodeLensProvider, @@ -48,10 +56,14 @@ export class CommandRegistry implements IDisposable { @inject(IOutputChannel) @named(JUPYTER_OUTPUT_CHANNEL) private jupyterOutput: IOutputChannel, @inject(IStartPage) private startPage: IStartPage, @inject(ExportCommands) private readonly exportCommand: ExportCommands, + @inject(IJupyterVariableDataProviderFactory) + private readonly jupyterVariableDataProviderFactory: IJupyterVariableDataProviderFactory, + @inject(IDataViewerFactory) private readonly dataViewerFactory: IDataViewerFactory, @inject(IDataScienceFileSystem) private readonly fs: IDataScienceFileSystem ) { this.disposables.push(this.serverSelectedCommand); this.disposables.push(this.notebookCommands); + this.dataViewerChecker = new DataViewerChecker(configService, appShell); } public register() { this.commandLineCommand.register(); @@ -96,6 +108,7 @@ export class CommandRegistry implements IDisposable { this.registerCommand(Commands.ViewJupyterOutput, this.viewJupyterOutput); this.registerCommand(Commands.GatherQuality, this.reportGatherQuality); this.registerCommand(Commands.LatestExtension, this.openPythonExtensionPage); + this.registerCommand(Commands.ShowDataViewer, this.onVariablePanelShowDataViewerRequest); this.registerCommand( Commands.EnableLoadingWidgetsFrom3rdPartySource, this.enableLoadingWidgetScriptsFromThirdParty @@ -466,4 +479,29 @@ export class CommandRegistry implements IDisposable { private openPythonExtensionPage() { env.openExternal(Uri.parse(`https://marketplace.visualstudio.com/items?itemName=ms-python.python`)); } + + private async onVariablePanelShowDataViewerRequest(request: IShowDataViewerFromVariablePanel) { + sendTelemetryEvent(Telemetry.OpenDataViewerFromVariableWindowRequest); + if (this.debugService.activeDebugSession) { + const jupyterVariable = convertDebugProtocolVariableToIJupyterVariable( + request.variable as DebugProtocol.Variable + ); + try { + const jupyterVariableDataProvider = await this.jupyterVariableDataProviderFactory.create( + jupyterVariable + ); + const dataFrameInfo = await jupyterVariableDataProvider.getDataFrameInfo(); + const columnSize = dataFrameInfo?.columns?.length; + if (columnSize && (await this.dataViewerChecker.isRequestedColumnSizeAllowed(columnSize))) { + const title: string = `${DataScience.dataExplorerTitle()} - ${jupyterVariable.name}`; + await this.dataViewerFactory.create(jupyterVariableDataProvider, title); + sendTelemetryEvent(Telemetry.OpenDataViewerFromVariableWindowSuccess); + } + } catch (e) { + sendTelemetryEvent(Telemetry.OpenDataViewerFromVariableWindowError); + traceError(e); + this.appShell.showErrorMessage(e.toString()); + } + } + } } diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 8e86b92bef28..894d26bcfb9e 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -111,6 +111,7 @@ export namespace Commands { 'python.datascience.enableLoadingWidgetScriptsFromThirdPartySource'; export const NotebookEditorExpandAllCells = 'python.datascience.notebookeditor.expandallcells'; export const NotebookEditorCollapseAllCells = 'python.datascience.notebookeditor.collapseallcells'; + export const ShowDataViewer = 'python.datascience.showDataViewer'; } export namespace CodeLensCommands { @@ -409,7 +410,10 @@ export enum Telemetry { TrustAllNotebooks = 'DATASCIENCE.TRUST_ALL_NOTEBOOKS', TrustNotebook = 'DATASCIENCE.TRUST_NOTEBOOK', DoNotTrustNotebook = 'DATASCIENCE.DO_NOT_TRUST_NOTEBOOK', - NotebookTrustPromptShown = 'DATASCIENCE.NOTEBOOK_TRUST_PROMPT_SHOWN' + NotebookTrustPromptShown = 'DATASCIENCE.NOTEBOOK_TRUST_PROMPT_SHOWN', + OpenDataViewerFromVariableWindowRequest = 'DATASCIENCE.OPEN_DATAVIEWER_FROM_VARIABLE_WINDOW_REQUEST', + OpenDataViewerFromVariableWindowError = 'DATASCIENCE.OPEN_DATAVIEWER_FROM_VARIABLE_WINDOW_ERROR', + OpenDataViewerFromVariableWindowSuccess = 'DATASCIENCE.OPEN_DATAVIEWER_FROM_VARIABLE_WINDOW_SUCCESS' } export enum NativeKeyboardCommandTelemetry { diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts index 58a4982edea9..216c4084d7ba 100644 --- a/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts +++ b/src/client/datascience/data-viewing/jupyterVariableDataProvider.ts @@ -57,7 +57,7 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider return; } - public setDependencies(variable: IJupyterVariable, notebook: INotebook): void { + public setDependencies(variable: IJupyterVariable, notebook?: INotebook): void { this.notebook = notebook; this.variable = variable; } @@ -65,7 +65,7 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider public async getDataFrameInfo(): Promise { let dataFrameInfo: IDataFrameInfo = {}; await this.ensureInitialized(); - if (this.variable && this.notebook) { + if (this.variable) { dataFrameInfo = { columns: this.variable.columns ? JupyterVariableDataProvider.getNormalizedColumns(this.variable.columns) @@ -80,12 +80,12 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider public async getAllRows() { let allRows: IRowsResponse = []; await this.ensureInitialized(); - if (this.variable && this.variable.rowCount && this.notebook) { + if (this.variable && this.variable.rowCount) { const dataFrameRows = await this.variableManager.getDataFrameRows( this.variable, - this.notebook, 0, - this.variable.rowCount + this.variable.rowCount, + this.notebook ); allRows = dataFrameRows && dataFrameRows.data ? (dataFrameRows.data as IRowsResponse) : []; } @@ -95,8 +95,8 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider public async getRows(start: number, end: number) { let rows: IRowsResponse = []; await this.ensureInitialized(); - if (this.variable && this.variable.rowCount && this.notebook) { - const dataFrameRows = await this.variableManager.getDataFrameRows(this.variable, this.notebook, start, end); + if (this.variable && this.variable.rowCount) { + const dataFrameRows = await this.variableManager.getDataFrameRows(this.variable, start, end, this.notebook); rows = dataFrameRows && dataFrameRows.data ? (dataFrameRows.data as IRowsResponse) : []; } return rows; @@ -104,9 +104,9 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider private async ensureInitialized(): Promise { // Postpone pre-req and variable initialization until data is requested. - if (!this.initialized && this.variable && this.notebook) { + if (!this.initialized && this.variable) { this.initialized = true; - await this.dependencyService.checkAndInstallMissingDependencies(this.notebook.getMatchingInterpreter()); + await this.dependencyService.checkAndInstallMissingDependencies(this.notebook?.getMatchingInterpreter()); this.variable = await this.variableManager.getDataFrameInfo(this.variable, this.notebook); } } diff --git a/src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts b/src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts index b8b48b45a4b3..c2361a3faf6a 100644 --- a/src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts +++ b/src/client/datascience/data-viewing/jupyterVariableDataProviderFactory.ts @@ -17,7 +17,7 @@ import { export class JupyterVariableDataProviderFactory implements IJupyterVariableDataProviderFactory { constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {} - public async create(variable: IJupyterVariable, notebook: INotebook): Promise { + public async create(variable: IJupyterVariable, notebook?: INotebook): Promise { const jupyterVariableDataProvider = this.serviceContainer.get( IJupyterVariableDataProvider ); diff --git a/src/client/datascience/editor-integration/hoverProvider.ts b/src/client/datascience/editor-integration/hoverProvider.ts index 23723262de08..62bfab35e5d5 100644 --- a/src/client/datascience/editor-integration/hoverProvider.ts +++ b/src/client/datascience/editor-integration/hoverProvider.ts @@ -109,7 +109,7 @@ export class HoverProvider implements INotebookExecutionLogger, vscode.HoverProv if (notebooks && notebooks.length) { // Just use the first one to reply if more than one. const match = await Promise.race( - notebooks.map((n) => this.variableProvider.getMatchingVariable(n, word, t)) + notebooks.map((n) => this.variableProvider.getMatchingVariable(word, n, t)) ); if (match) { return { diff --git a/src/client/datascience/interactive-common/dataViewerChecker.ts b/src/client/datascience/interactive-common/dataViewerChecker.ts new file mode 100644 index 000000000000..c6fd48f47341 --- /dev/null +++ b/src/client/datascience/interactive-common/dataViewerChecker.ts @@ -0,0 +1,45 @@ +import { ConfigurationTarget } from 'vscode'; +import { IApplicationShell } from '../../common/application/types'; +import { IConfigurationService, Resource } from '../../common/types'; +import * as localize from '../../common/utils/localize'; +import { ColumnWarningSize } from '../data-viewing/types'; + +// This helper class validates requests to show large data in the data viewer and configures related settings. +export class DataViewerChecker { + constructor(private configuration: IConfigurationService, private applicationShell: IApplicationShell) {} + + public async isRequestedColumnSizeAllowed(columnSize: number, owningResource?: Resource): Promise { + if (columnSize > ColumnWarningSize && (await this.shouldAskForLargeData(owningResource))) { + const message = localize.DataScience.tooManyColumnsMessage(); + const yes = localize.DataScience.tooManyColumnsYes(); + const no = localize.DataScience.tooManyColumnsNo(); + const dontAskAgain = localize.DataScience.tooManyColumnsDontAskAgain(); + + const result = await this.applicationShell.showWarningMessage(message, yes, no, dontAskAgain); + if (result === dontAskAgain) { + await this.disableAskForLargeData(); + } + return result === yes; + } + return true; + } + + private async shouldAskForLargeData(owningResource?: Resource): Promise { + const settings = owningResource + ? this.configuration.getSettings(owningResource) + : this.configuration.getSettings(); + return settings && settings.datascience && settings.datascience.askForLargeDataFrames === true; + } + + private async disableAskForLargeData(owningResource?: Resource): Promise { + const settings = owningResource + ? this.configuration.getSettings(owningResource) + : this.configuration.getSettings(); + if (settings && settings.datascience) { + settings.datascience.askForLargeDataFrames = false; + this.configuration + .updateSetting('dataScience.askForLargeDataFrames', false, undefined, ConfigurationTarget.Global) + .ignoreErrors(); + } + } +} diff --git a/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts b/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts index 9124ccd5a842..fa312c62a5cf 100644 --- a/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts +++ b/src/client/datascience/interactive-common/intellisense/intellisenseProvider.ts @@ -938,7 +938,7 @@ export class IntellisenseProvider implements IInteractiveWindowListener { const notebook = await this.getNotebook(token); if (notebook) { try { - const value = await this.variableProvider.getMatchingVariable(notebook, wordAtPosition, token); + const value = await this.variableProvider.getMatchingVariable(wordAtPosition, notebook, token); if (value) { return { contents: [`${wordAtPosition}: ${value.type} = ${value.value}`] diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index f627841b0478..12d7a16ce9df 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -48,7 +48,7 @@ import { generateCellRangesFromDocument } from '../cellFactory'; import { CellMatcher } from '../cellMatcher'; import { addToUriList, translateKernelLanguageToMonaco } from '../common'; import { Commands, Identifiers, Settings, Telemetry } from '../constants'; -import { ColumnWarningSize, IDataViewerFactory } from '../data-viewing/types'; +import { IDataViewerFactory } from '../data-viewing/types'; import { IAddedSysInfo, ICopyCode, @@ -101,6 +101,7 @@ import { WebViewViewChangeEventArgs } from '../types'; import { WebviewPanelHost } from '../webviews/webviewPanelHost'; +import { DataViewerChecker } from './dataViewerChecker'; import { InteractiveWindowMessageListener } from './interactiveWindowMessageListener'; import { serializeLanguageConfiguration } from './serialization'; @@ -124,6 +125,7 @@ export abstract class InteractiveBase extends WebviewPanelHost { - const settings = this.configuration.getSettings(this.owningResource); - return settings && settings.datascience && settings.datascience.askForLargeDataFrames === true; - } - - private async disableAskForLargeData(): Promise { - const settings = this.configuration.getSettings(this.owningResource); - if (settings && settings.datascience) { - settings.datascience.askForLargeDataFrames = false; - this.configuration - .updateSetting('dataScience.askForLargeDataFrames', false, undefined, ConfigurationTarget.Global) - .ignoreErrors(); - } - } - - private async checkColumnSize(columnSize: number): Promise { - if (columnSize > ColumnWarningSize && (await this.shouldAskForLargeData())) { - const message = localize.DataScience.tooManyColumnsMessage(); - const yes = localize.DataScience.tooManyColumnsYes(); - const no = localize.DataScience.tooManyColumnsNo(); - const dontAskAgain = localize.DataScience.tooManyColumnsDontAskAgain(); - - const result = await this.applicationShell.showWarningMessage(message, yes, no, dontAskAgain); - if (result === dontAskAgain) { - await this.disableAskForLargeData(); - } - return result === yes; - } - return true; - } - private async showDataViewer(request: IShowDataViewer): Promise { try { - if (await this.checkColumnSize(request.columnSize)) { + if (await this.dataViewerChecker.isRequestedColumnSizeAllowed(request.columnSize, this.owningResource)) { const jupyterVariableDataProvider = await this.jupyterVariableDataProviderFactory.create( request.variable, this._notebook! ); const title: string = `${localize.DataScience.dataExplorerTitle()} - ${request.variable.name}`; - await this.dataExplorerFactory.create(jupyterVariableDataProvider, title); + await this.dataViewerFactory.create(jupyterVariableDataProvider, title); } } catch (e) { + traceError(e); this.applicationShell.showErrorMessage(e.toString()); } } @@ -1420,7 +1394,7 @@ export abstract class InteractiveBase extends WebviewPanelHost { // Request our new list of variables const response: IJupyterVariablesResponse = this._notebook - ? await this.jupyterVariables.getVariables(this._notebook, args) + ? await this.jupyterVariables.getVariables(args, this._notebook) : { totalCount: 0, pageResponse: [], diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts index 86921d56c919..128dbbc7df37 100644 --- a/src/client/datascience/interactive-common/interactiveWindowTypes.ts +++ b/src/client/datascience/interactive-common/interactiveWindowTypes.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. 'use strict'; import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; -import { Uri } from 'vscode'; +import { DebugProtocolVariable, DebugProtocolVariableContainer, Uri } from 'vscode'; import { DebugState, IServerState } from '../../../datascience-ui/interactive-common/mainState'; import type { KernelMessage } from '@jupyterlab/services'; @@ -334,6 +334,11 @@ export interface IShowDataViewer { columnSize: number; } +export interface IShowDataViewerFromVariablePanel { + container: DebugProtocolVariableContainer | undefined; + variable: DebugProtocolVariable; +} + export interface IRefreshVariablesRequest { newExecutionCount?: number; } diff --git a/src/client/datascience/jupyter/debuggerVariables.ts b/src/client/datascience/jupyter/debuggerVariables.ts index 573aa699b7df..9ed5db8b8aa4 100644 --- a/src/client/datascience/jupyter/debuggerVariables.ts +++ b/src/client/datascience/jupyter/debuggerVariables.ts @@ -48,11 +48,13 @@ export class DebuggerVariables extends DebugLocationTracker // IJupyterVariables implementation public async getVariables( - notebook: INotebook, - request: IJupyterVariablesRequest + request: IJupyterVariablesRequest, + notebook?: INotebook ): Promise { // Listen to notebook events if we haven't already - this.watchNotebook(notebook); + if (notebook) { + this.watchNotebook(notebook); + } const result: IJupyterVariablesResponse = { executionCount: request.executionCount, @@ -71,7 +73,7 @@ export class DebuggerVariables extends DebugLocationTracker for (let i = startPos; i < startPos + chunkSize && i < this.lastKnownVariables.length; i += 1) { const fullVariable = !this.lastKnownVariables[i].truncated ? this.lastKnownVariables[i] - : await this.getFullVariable(this.lastKnownVariables[i], notebook); + : await this.getFullVariable(this.lastKnownVariables[i]); this.lastKnownVariables[i] = fullVariable; result.pageResponse.push(fullVariable); } @@ -81,29 +83,29 @@ export class DebuggerVariables extends DebugLocationTracker return result; } - public async getMatchingVariable(notebook: INotebook, name: string): Promise { + public async getMatchingVariable(name: string, notebook?: INotebook): Promise { if (this.active) { // Note, full variable results isn't necessary for this call. It only really needs the variable value. const result = this.lastKnownVariables.find((v) => v.name === name); - if (result) { - if (notebook.identity.fsPath.endsWith('.ipynb')) { - sendTelemetryEvent(Telemetry.RunByLineVariableHover); - } + if (result && notebook && notebook.identity.fsPath.endsWith('.ipynb')) { + sendTelemetryEvent(Telemetry.RunByLineVariableHover); } return result; } } - public async getDataFrameInfo(targetVariable: IJupyterVariable, notebook: INotebook): Promise { + public async getDataFrameInfo(targetVariable: IJupyterVariable, notebook?: INotebook): Promise { if (!this.active) { // No active server just return the unchanged target variable return targetVariable; } // Listen to notebook events if we haven't already - this.watchNotebook(notebook); + if (notebook) { + this.watchNotebook(notebook); + } // See if we imported or not into the kernel our special function - await this.importDataFrameScripts(notebook); + await this.importDataFrameScripts(); // Then eval calling the main function with our target variable const results = await this.evaluate( @@ -123,9 +125,9 @@ export class DebuggerVariables extends DebugLocationTracker public async getDataFrameRows( targetVariable: IJupyterVariable, - notebook: INotebook, start: number, - end: number + end: number, + notebook?: INotebook ): Promise<{}> { // Run the get dataframe rows script if (!this.debugService.activeDebugSession || targetVariable.columns === undefined) { @@ -133,10 +135,12 @@ export class DebuggerVariables extends DebugLocationTracker return {}; } // Listen to notebook events if we haven't already - this.watchNotebook(notebook); + if (notebook) { + this.watchNotebook(notebook); + } // See if we imported or not into the kernel our special function - await this.importDataFrameScripts(notebook); + await this.importDataFrameScripts(); // Since the debugger splits up long requests, split this based on the number of items. @@ -169,6 +173,7 @@ export class DebuggerVariables extends DebugLocationTracker return output; } + // This special DebugAdapterTracker function listens to messages sent from the debug adapter to VS Code // tslint:disable-next-line: no-any public onDidSendMessage(message: any) { super.onDidSendMessage(message); @@ -186,6 +191,10 @@ export class DebuggerVariables extends DebugLocationTracker this.topMostFrameId = 0; this.debuggingStarted = false; this.refreshEventEmitter.fire(); + const key = this.debugService.activeDebugSession?.id; + if (key) { + this.importedIntoKernel.delete(key); + } } } @@ -228,10 +237,11 @@ export class DebuggerVariables extends DebugLocationTracker throw Error('Debugger is not active, cannot evaluate.'); } - private async importDataFrameScripts(notebook: INotebook): Promise { + private async importDataFrameScripts(): Promise { try { - const key = notebook.identity.toString(); - if (!this.importedIntoKernel.has(key)) { + // Run our dataframe scripts only once per session because they're slow + const key = this.debugService.activeDebugSession?.id; + if (key && !this.importedIntoKernel.has(key)) { await this.evaluate(DataFrameLoading.DataFrameSysImport); await this.evaluate(DataFrameLoading.DataFrameInfoImport); await this.evaluate(DataFrameLoading.DataFrameRowImport); @@ -243,9 +253,9 @@ export class DebuggerVariables extends DebugLocationTracker } } - private async getFullVariable(variable: IJupyterVariable, notebook: INotebook): Promise { + private async getFullVariable(variable: IJupyterVariable): Promise { // See if we imported or not into the kernel our special function - await this.importDataFrameScripts(notebook); + await this.importDataFrameScripts(); // Then eval calling the variable info function with our target variable const results = await this.evaluate( @@ -271,39 +281,51 @@ export class DebuggerVariables extends DebugLocationTracker ? this.configService.getSettings().datascience.variableExplorerExclude?.split(';') : []; - const allowedVariables = variablesResponse.body.variables.filter((v) => { - if (!v.name || !v.type || !v.value) { - return false; - } - if (exclusionList && exclusionList.includes(v.type)) { - return false; - } - if (v.name.startsWith('_')) { - return false; - } - if (KnownExcludedVariables.has(v.name)) { - return false; - } - if (v.type === 'NoneType') { - return false; - } - return true; - }); + const allowedVariables = variablesResponse.body.variables + .filter((v) => { + if (!v.name || !v.type || !v.value) { + return false; + } + if (exclusionList && exclusionList.includes(v.type)) { + return false; + } + if (v.name.startsWith('_')) { + return false; + } + if (KnownExcludedVariables.has(v.name)) { + return false; + } + if (v.type === 'NoneType') { + return false; + } + return true; + }) + .map((v) => { + if (v.type && DataViewableTypes.has(v.type)) { + // tslint:disable-next-line: no-any + (v as any).__vscodeVariableMenuContext = 'viewableInDataViewer'; + } + return v; + }); this.lastKnownVariables = allowedVariables.map((v) => { - return { - name: v.name, - type: v.type!, - count: 0, - shape: '', - size: 0, - supportsDataExplorer: DataViewableTypes.has(v.type || ''), - value: v.value, - truncated: true, - frameId: v.variablesReference - }; + return convertDebugProtocolVariableToIJupyterVariable(v); }); this.refreshEventEmitter.fire(); } } + +export function convertDebugProtocolVariableToIJupyterVariable(variable: DebugProtocol.Variable) { + return { + name: variable.name, + type: variable.type!, + count: 0, + shape: '', + size: 0, + supportsDataExplorer: DataViewableTypes.has(variable.type || ''), + value: variable.value, + truncated: true, + frameId: variable.variablesReference + }; +} diff --git a/src/client/datascience/jupyter/jupyterVariables.ts b/src/client/datascience/jupyter/jupyterVariables.ts index a9cf9958a118..e0d25b5db825 100644 --- a/src/client/datascience/jupyter/jupyterVariables.ts +++ b/src/client/datascience/jupyter/jupyterVariables.ts @@ -48,34 +48,34 @@ export class JupyterVariables implements IJupyterVariables { // IJupyterVariables implementation @captureTelemetry(Telemetry.VariableExplorerFetchTime, undefined, true) public async getVariables( - notebook: INotebook, - request: IJupyterVariablesRequest + request: IJupyterVariablesRequest, + notebook?: INotebook ): Promise { - return this.getVariableHandler(notebook).getVariables(notebook, request); + return this.getVariableHandler(notebook).getVariables(request, notebook); } - public getMatchingVariable(notebook: INotebook, name: string): Promise { - return this.getVariableHandler(notebook).getMatchingVariable(notebook, name); + public getMatchingVariable(name: string, notebook?: INotebook): Promise { + return this.getVariableHandler(notebook).getMatchingVariable(name, notebook); } - public async getDataFrameInfo(targetVariable: IJupyterVariable, notebook: INotebook): Promise { + public async getDataFrameInfo(targetVariable: IJupyterVariable, notebook?: INotebook): Promise { return this.getVariableHandler(notebook).getDataFrameInfo(targetVariable, notebook); } public async getDataFrameRows( targetVariable: IJupyterVariable, - notebook: INotebook, start: number, - end: number + end: number, + notebook?: INotebook ): Promise { - return this.getVariableHandler(notebook).getDataFrameRows(targetVariable, notebook, start, end); + return this.getVariableHandler(notebook).getDataFrameRows(targetVariable, start, end, notebook); } - private getVariableHandler(notebook: INotebook): IJupyterVariables { + private getVariableHandler(notebook?: INotebook): IJupyterVariables { if (!this.experimentsManager.inExperiment(RunByLine.experiment)) { return this.oldVariables; } - if (this.debuggerVariables.active && notebook.status === ServerStatus.Busy) { + if (this.debuggerVariables.active && (!notebook || notebook.status === ServerStatus.Busy)) { return this.debuggerVariables; } diff --git a/src/client/datascience/jupyter/kernelVariables.ts b/src/client/datascience/jupyter/kernelVariables.ts index e0806763c6b5..039421a802dc 100644 --- a/src/client/datascience/jupyter/kernelVariables.ts +++ b/src/client/datascience/jupyter/kernelVariables.ts @@ -56,16 +56,16 @@ export class KernelVariables implements IJupyterVariables { // IJupyterVariables implementation public async getVariables( - notebook: INotebook, - request: IJupyterVariablesRequest + request: IJupyterVariablesRequest, + notebook: INotebook ): Promise { // Run the language appropriate variable fetch return this.getVariablesBasedOnKernel(notebook, request); } public async getMatchingVariable( - notebook: INotebook, name: string, + notebook: INotebook, token?: CancellationToken ): Promise { // See if in the cache @@ -124,9 +124,9 @@ export class KernelVariables implements IJupyterVariables { public async getDataFrameRows( targetVariable: IJupyterVariable, - notebook: INotebook, start: number, - end: number + end: number, + notebook: INotebook ): Promise<{}> { // Import the data frame script directory if we haven't already await this.importDataFrameScripts(notebook); diff --git a/src/client/datascience/jupyter/oldJupyterVariables.ts b/src/client/datascience/jupyter/oldJupyterVariables.ts index 7c1d77de7135..cea3646fc04a 100644 --- a/src/client/datascience/jupyter/oldJupyterVariables.ts +++ b/src/client/datascience/jupyter/oldJupyterVariables.ts @@ -66,14 +66,14 @@ export class OldJupyterVariables implements IJupyterVariables { // IJupyterVariables implementation public async getVariables( - notebook: INotebook, - request: IJupyterVariablesRequest + request: IJupyterVariablesRequest, + notebook: INotebook ): Promise { // Run the language appropriate variable fetch return this.getVariablesBasedOnKernel(notebook, request); } - public async getMatchingVariable(_notebook: INotebook, _name: string): Promise { + public async getMatchingVariable(_name: string, _notebook: INotebook): Promise { // Not supported with old method. return undefined; } @@ -91,9 +91,9 @@ export class OldJupyterVariables implements IJupyterVariables { public async getDataFrameRows( targetVariable: IJupyterVariable, - notebook: INotebook, start: number, - end: number + end: number, + notebook: INotebook ): Promise<{}> { // Run the get dataframe rows script return this.runScript<{}>(notebook, targetVariable, {}, () => this.fetchDataFrameRowsScript, [ diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 7e96cd4bd33e..c3ba89020ae8 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -860,28 +860,28 @@ export interface IJupyterVariable { export const IJupyterVariableDataProvider = Symbol('IJupyterVariableDataProvider'); export interface IJupyterVariableDataProvider extends IDataViewerDataProvider { - setDependencies(variable: IJupyterVariable, notebook: INotebook): void; + setDependencies(variable: IJupyterVariable, notebook?: INotebook): void; } export const IJupyterVariableDataProviderFactory = Symbol('IJupyterVariableDataProviderFactory'); export interface IJupyterVariableDataProviderFactory { - create(variable: IJupyterVariable, notebook: INotebook): Promise; + create(variable: IJupyterVariable, notebook?: INotebook): Promise; } export const IJupyterVariables = Symbol('IJupyterVariables'); export interface IJupyterVariables { readonly refreshRequired: Event; - getVariables(notebook: INotebook, request: IJupyterVariablesRequest): Promise; - getDataFrameInfo(targetVariable: IJupyterVariable, notebook: INotebook): Promise; + getVariables(request: IJupyterVariablesRequest, notebook?: INotebook): Promise; + getDataFrameInfo(targetVariable: IJupyterVariable, notebook?: INotebook): Promise; getDataFrameRows( targetVariable: IJupyterVariable, - notebook: INotebook, start: number, - end: number + end: number, + notebook?: INotebook ): Promise; getMatchingVariable( - notebook: INotebook, name: string, + notebook?: INotebook, cancelToken?: CancellationToken ): Promise; } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 9c71a5ec6fbc..d110fbd10e6c 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1772,6 +1772,9 @@ export interface IEventNamePropertyMapping { [Telemetry.SetJupyterURIToUserSpecified]: never | undefined; [Telemetry.ShiftEnterBannerShown]: never | undefined; [Telemetry.ShowDataViewer]: { rows: number | undefined; columns: number | undefined }; + [Telemetry.OpenDataViewerFromVariableWindowRequest]: never | undefined; + [Telemetry.OpenDataViewerFromVariableWindowError]: never | undefined; + [Telemetry.OpenDataViewerFromVariableWindowSuccess]: never | undefined; [Telemetry.CreateNewInteractive]: never | undefined; [Telemetry.StartJupyter]: never | undefined; [Telemetry.StartJupyterProcess]: never | undefined; diff --git a/src/test/datascience/commands/commandRegistry.unit.test.ts b/src/test/datascience/commands/commandRegistry.unit.test.ts index c6ee1f605ed0..56853077f4da 100644 --- a/src/test/datascience/commands/commandRegistry.unit.test.ts +++ b/src/test/datascience/commands/commandRegistry.unit.test.ts @@ -14,6 +14,8 @@ import { ExportCommands } from '../../../client/datascience/commands/exportComma import { NotebookCommands } from '../../../client/datascience/commands/notebookCommands'; import { JupyterServerSelectorCommand } from '../../../client/datascience/commands/serverSelector'; import { Commands } from '../../../client/datascience/constants'; +import { DataViewerFactory } from '../../../client/datascience/data-viewing/dataViewerFactory'; +import { JupyterVariableDataProviderFactory } from '../../../client/datascience/data-viewing/jupyterVariableDataProviderFactory'; import { DataScienceFileSystem } from '../../../client/datascience/dataScienceFileSystem'; import { DataScienceCodeLensProvider } from '../../../client/datascience/editor-integration/codelensprovider'; import { NativeEditorProvider } from '../../../client/datascience/notebookStorage/nativeEditorProvider'; @@ -40,6 +42,8 @@ suite('DataScience - Commands', () => { const appShell = mock(ApplicationShell); const startPage = mock(StartPage); const exportCommand = mock(ExportCommands); + const jupyterVariableDataProviderFactory = mock(JupyterVariableDataProviderFactory); + const dataViewerFactory = mock(DataViewerFactory); const fileSystem = mock(DataScienceFileSystem); commandRegistry = new CommandRegistry( @@ -57,6 +61,8 @@ suite('DataScience - Commands', () => { new MockOutputChannel('Jupyter'), instance(startPage), instance(exportCommand), + instance(jupyterVariableDataProviderFactory), + instance(dataViewerFactory), instance(fileSystem) ); }); diff --git a/src/test/datascience/variableTestHelpers.ts b/src/test/datascience/variableTestHelpers.ts index a810a5d8ed1b..adb6b884abab 100644 --- a/src/test/datascience/variableTestHelpers.ts +++ b/src/test/datascience/variableTestHelpers.ts @@ -140,21 +140,24 @@ export async function verifyCanFetchData( identity: getDefaultInteractiveIdentity() }); expect(notebook).to.not.be.undefined; - const variableList = await variableFetcher.getVariables(notebook!, { - executionCount, - startIndex: 0, - pageSize: 100, - sortAscending: true, - sortColumn: 'INDEX', - refreshCount: 0 - }); + const variableList = await variableFetcher.getVariables( + { + executionCount, + startIndex: 0, + pageSize: 100, + sortAscending: true, + sortColumn: 'INDEX', + refreshCount: 0 + }, + notebook! + ); expect(variableList.pageResponse.length).to.be.greaterThan(0, 'No variables returned'); const variable = variableList.pageResponse.find((v) => v.name === name); expect(variable).to.not.be.undefined; expect(variable?.supportsDataExplorer).to.eq(true, `Variable ${name} does not support data explorer`); const withInfo = await variableFetcher.getDataFrameInfo(variable!, notebook!); expect(withInfo.count).to.eq(rows.length, 'Wrong number of rows for variable'); - const fetchedRows = await variableFetcher.getDataFrameRows(withInfo!, notebook!, 0, rows.length); + const fetchedRows = await variableFetcher.getDataFrameRows(withInfo!, 0, rows.length, notebook!); expect(fetchedRows.data).to.have.length(rows.length, 'Fetched rows data is not the correct size'); for (let i = 0; i < rows.length; i += 1) { const fetchedRow = (fetchedRows.data as any)[i]; diff --git a/types/vscode.proposed.d.ts b/types/vscode.proposed.d.ts index f693f822bcc1..476e865f53aa 100644 --- a/types/vscode.proposed.d.ts +++ b/types/vscode.proposed.d.ts @@ -716,4 +716,23 @@ declare module 'vscode' { priority?: number ): NotebookCellStatusBarItem; } + + //#region debug + + /** + * A DebugProtocolVariableContainer is an opaque stand-in type for the intersection of the Scope and Variable types defined in the Debug Adapter Protocol. + * See https://microsoft.github.io/debug-adapter-protocol/specification#Types_Scope and https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable. + */ + export interface DebugProtocolVariableContainer { + // Properties: the intersection of DAP's Scope and Variable types. + } + + /** + * A DebugProtocolVariable is an opaque stand-in type for the Variable type defined in the Debug Adapter Protocol. + * See https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable. + */ + export interface DebugProtocolVariable { + // Properties: see details [here](https://microsoft.github.io/debug-adapter-protocol/specification#Base_Protocol_Variable). + } + //#endregion }