Proposal: Isomorphic pythonVariableRequester and debuggerVariables.ts (…

…#10142)

* Proposal: Isomorphic pythonVariableRequester

* not needed

* scriptConstants: No more SysPath, and ScriptPath is now getScriptPath

* debuggerVariables.node.ts -> debuggerVariables.ts

* lint fix

* using uriPath.extname

* better extension match

src/kernels/installer/poetry.node.ts

@@ -18,7 +18,7 @@ import {
     isVirtualenvEnvironment,
     pathExists
 } from '../../platform/common/platform/fileUtils.node';
-import { isTestExecution } from '../../platform/common/constants.node';
+import { isTestExecution } from '../../platform/common/constants';
 
 /**
  * Global virtual env dir for a project is named as:

src/kernels/kernel.node.ts

@@ -21,7 +21,7 @@ import {
     KernelActionSource,
     KernelConnectionMetadata
 } from './types';
-import { AddRunCellHook } from '../platform/common/constants.node';
+import { AddRunCellHook } from '../platform/common/scriptConstants';
 import { IStatusProvider } from '../platform/progress/types';
 import { getAssociatedNotebookDocument } from '../notebooks/controllers/kernelSelector';
 import { sendTelemetryForPythonKernelExecutable } from './helpers.node';
@@ -44,7 +44,7 @@ export class Kernel extends BaseKernel {
         private readonly pythonExecutionFactory: IPythonExecutionFactory,
         statusProvider: IStatusProvider,
         creator: KernelActionSource,
-        context: IExtensionContext,
+        private readonly context: IExtensionContext,
         formatters: ITracebackFormatter[]
     ) {
         super(
@@ -79,11 +79,12 @@ export class Kernel extends BaseKernel {
         if (getAssociatedNotebookDocument(this)?.notebookType === InteractiveWindowView) {
             // If using ipykernel 6, we need to set the IPYKERNEL_CELL_NAME so that
             // debugging can work. However this code is harmless for IPYKERNEL 5 so just always do it
-            if (await this.fs.localFileExists(AddRunCellHook.ScriptPath)) {
-                const fileContents = await this.fs.readLocalFile(AddRunCellHook.ScriptPath);
+            const scriptPath = AddRunCellHook.getScriptPath(this.context);
+            if (await this.fs.localFileExists(scriptPath.fsPath)) {
+                const fileContents = await this.fs.readLocalFile(scriptPath.fsPath);
                 return fileContents.splitLines({ trim: false });
             }
-            traceError(`Cannot run non-existent script file: ${AddRunCellHook.ScriptPath}`);
+            traceError(`Cannot run non-existent script file: ${scriptPath}`);
         }
         return [];
     }

src/kernels/raw/launcher/kernelLauncher.node.ts

@@ -32,7 +32,7 @@ import { IKernelLauncher, IKernelProcess, IKernelConnection } from '../types';
 import { KernelEnvironmentVariablesService } from './kernelEnvVarsService.node';
 import { KernelProcess } from './kernelProcess.node';
 import { JupyterPaths } from '../finder/jupyterPaths.node';
-import { isTestExecution } from '../../../platform/common/constants.node';
+import { isTestExecution } from '../../../platform/common/constants';
 import { getDisplayPathFromLocalFile } from '../../../platform/common/platform/fs-paths.node';
 import { noop } from '../../../platform/common/utils/misc';
 

src/kernels/serviceRegistry.node.ts

@@ -28,11 +28,11 @@ import { HostRawNotebookProvider } from './raw/session/hostRawNotebookProvider.n
 import { RawNotebookSupportedService } from './raw/session/rawNotebookSupportedService.node';
 import { IKernelLauncher, ILocalKernelFinder, IRawNotebookProvider, IRawNotebookSupportedService } from './raw/types';
 import { DebuggerVariableRegistration } from './variables/debuggerVariableRegistration.node';
-import { DebuggerVariables } from './variables/debuggerVariables.node';
+import { DebuggerVariables } from './variables/debuggerVariables';
 import { JupyterVariables } from './variables/jupyterVariables';
 import { KernelVariables } from './variables/kernelVariables';
 import { PreWarmActivatedJupyterEnvironmentVariables } from './variables/preWarmVariables.node';
-import { PythonVariablesRequester } from './variables/pythonVariableRequester.node';
+import { PythonVariablesRequester } from './variables/pythonVariableRequester';
 import { IInteractiveWindowDebugger } from '../interactive-window/types';
 import { MultiplexingDebugService } from './debugger/multiplexingDebugService';
 import { JupyterVariableDataProvider } from '../webviews/extension-side/dataviewer/jupyterVariableDataProvider';

src/kernels/variables/debuggerVariables.node.ts → src/kernels/variables/debuggerVariables.ts

@@ -3,27 +3,29 @@
 'use strict';
 import { inject, injectable, named } from 'inversify';
 import * as path from '../../platform/vscode-path/path';
+import * as uriPath from '../../platform/vscode-path/resources';
 
 import { DebugAdapterTracker, Disposable, Event, EventEmitter } from 'vscode';
 import { DebugProtocol } from 'vscode-debugprotocol';
 import { getAssociatedNotebookDocument } from '../../notebooks/controllers/kernelSelector';
 import { IDebugService, IVSCodeNotebook } from '../../platform/common/application/types';
-import { DataFrameLoading, GetVariableInfo } from '../../platform/common/constants.node';
+import { DataFrameLoading, GetVariableInfo } from '../../platform/common/scriptConstants';
 import { traceError } from '../../platform/logging';
-import { IConfigurationService, Resource } from '../../platform/common/types';
+import { IConfigurationService, IExtensionContext, Resource } from '../../platform/common/types';
 import { DebugLocationTracker } from '../debugger/debugLocationTracker';
 import { sendTelemetryEvent } from '../../telemetry';
 import { Identifiers, Telemetry } from '../../webviews/webview-side/common/constants';
 import { IDebuggingManager, IJupyterDebugService, KernelDebugMode } from '../debugger/types';
 import { IKernel } from '../types';
-import { parseDataFrame } from './pythonVariableRequester.node';
+import { parseDataFrame } from './pythonVariableRequester';
 import {
     IConditionalJupyterVariables,
     IJupyterVariable,
     IJupyterVariablesRequest,
     IJupyterVariablesResponse
 } from './types';
 import { convertDebugProtocolVariableToIJupyterVariable, DataViewableTypes } from './helpers';
+import { IFileSystem } from '../../platform/common/platform/types';
 
 const KnownExcludedVariables = new Set<string>(['In', 'Out', 'exit', 'quit']);
 const MaximumRowChunkSizeForDebugger = 100;
@@ -46,7 +48,9 @@ export class DebuggerVariables
         @inject(IJupyterDebugService) @named(Identifiers.MULTIPLEXING_DEBUGSERVICE) private debugService: IDebugService,
         @inject(IDebuggingManager) private readonly debuggingManager: IDebuggingManager,
         @inject(IConfigurationService) private configService: IConfigurationService,
-        @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook
+        @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook,
+        @inject(IFileSystem) private readonly fs: IFileSystem,
+        @inject(IExtensionContext) private readonly context: IExtensionContext
     ) {
         super(undefined);
         this.debuggingManager.onDoneDebugging(() => this.refreshEventEmitter.fire(), this);
@@ -116,7 +120,7 @@ export class DebuggerVariables
         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 && kernel?.resourceUri?.fsPath.endsWith('.ipynb')) {
+            if (result && kernel?.resourceUri && uriPath.extname(kernel?.resourceUri).toLowerCase() === '.ipynb') {
                 sendTelemetryEvent(Telemetry.RunByLineVariableHover);
             }
             return result;
@@ -157,7 +161,7 @@ export class DebuggerVariables
         );
 
         const notebook = getAssociatedNotebookDocument(kernel);
-        let fileName = notebook ? path.basename(notebook.uri.fsPath) : '';
+        let fileName = notebook ? path.basename(notebook.uri.path) : '';
         if (!fileName && this.debugLocation?.fileName) {
             fileName = path.basename(this.debugLocation.fileName);
         }
@@ -316,7 +320,9 @@ export class DebuggerVariables
             // Run our dataframe scripts only once per session because they're slow
             const key = this.debugService.activeDebugSession?.id;
             if (key && !this.importedDataFrameScriptsIntoKernel.has(key)) {
-                await this.evaluate(DataFrameLoading.DataFrameSysImport);
+                const scriptPath = DataFrameLoading.getScriptPath(this.context);
+                const contents = await this.fs.readFile(scriptPath);
+                await this.evaluate(contents);
                 this.importedDataFrameScriptsIntoKernel.add(key);
             }
         } catch (exc) {
@@ -329,7 +335,9 @@ export class DebuggerVariables
             // Run our variable info scripts only once per session because they're slow
             const key = this.debugService.activeDebugSession?.id;
             if (key && !this.importedGetVariableInfoScriptsIntoKernel.has(key)) {
-                await this.evaluate(GetVariableInfo.GetVariableInfoSysImport);
+                const scriptPath = DataFrameLoading.getScriptPath(this.context);
+                const contents = await this.fs.readFile(scriptPath);
+                await this.evaluate(contents);
                 this.importedGetVariableInfoScriptsIntoKernel.add(key);
             }
         } catch (exc) {

src/kernels/variables/pythonVariableRequester.node.ts → src/kernels/variables/pythonVariableRequester.ts

@@ -1,10 +1,9 @@
 import { IDisposable } from '@fluentui/react';
 import type * as nbformat from '@jupyterlab/nbformat';
 import { inject, injectable } from 'inversify';
-import * as path from '../../platform/vscode-path/path';
-import { CancellationToken, NotebookDocument } from 'vscode';
+import { CancellationToken, NotebookDocument, Uri } from 'vscode';
 import { traceError } from '../../platform/logging';
-import { IFileSystemNode } from '../../platform/common/platform/types.node';
+import { IFileSystem } from '../../platform/common/platform/types';
 import { DataScience } from '../../platform/common/utils/localize';
 import { stripAnsi } from '../../platform/common/utils/regexp';
 import { JupyterDataRateLimitError } from '../../platform/errors/jupyterDataRateLimitError';
@@ -13,7 +12,8 @@ import { executeSilently } from '../helpers';
 import { IKernel } from '../types';
 import { IKernelVariableRequester, IJupyterVariable } from './types';
 import { getAssociatedNotebookDocument } from '../../notebooks/controllers/kernelSelector';
-import { DataFrameLoading, GetVariableInfo } from '../../platform/common/constants.node';
+import { DataFrameLoading, GetVariableInfo } from '../../platform/common/scriptConstants';
+import { IExtensionContext } from '../../platform/common/types';
 
 type DataFrameSplitFormat = {
     index: (number | string)[];
@@ -42,7 +42,10 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
     private importedDataFrameScripts = new WeakMap<NotebookDocument, boolean>();
     private importedGetVariableInfoScripts = new WeakMap<NotebookDocument, boolean>();
 
-    constructor(@inject(IFileSystemNode) private fs: IFileSystemNode) {}
+    constructor(
+        @inject(IFileSystem) private fs: IFileSystem,
+        @inject(IExtensionContext) private readonly context: IExtensionContext
+    ) {}
 
     public async getDataFrameInfo(
         targetVariable: IJupyterVariable,
@@ -65,9 +68,7 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
               )
             : [];
 
-        const fileName = path.basename(
-            getAssociatedNotebookDocument(kernel)?.uri.fsPath || kernel.resourceUri?.fsPath || kernel.id.path
-        );
+        const fileName = getAssociatedNotebookDocument(kernel)?.uri || kernel.resourceUri || kernel.id;
 
         // Combine with the original result (the call only returns the new fields)
         return {
@@ -225,7 +226,7 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
             disposables.push(kernel.onRestarted(handler));
 
             // First put the code from our helper files into the notebook
-            await this.runScriptFile(kernel, DataFrameLoading.ScriptPath);
+            await this.runScriptFile(kernel, DataFrameLoading.getScriptPath(this.context));
 
             this.importedDataFrameScripts.set(key, true);
         }
@@ -243,19 +244,19 @@ export class PythonVariablesRequester implements IKernelVariableRequester {
             disposables.push(kernel.onDisposed(handler));
             disposables.push(kernel.onRestarted(handler));
 
-            await this.runScriptFile(kernel, GetVariableInfo.ScriptPath);
+            await this.runScriptFile(kernel, GetVariableInfo.getScriptPath(this.context));
 
             this.importedGetVariableInfoScripts.set(key, true);
         }
     }
 
     // Read in a .py file and execute it silently in the given notebook
-    private async runScriptFile(kernel: IKernel, scriptFile: string) {
-        if (await this.fs.localFileExists(scriptFile)) {
-            const fileContents = await this.fs.readLocalFile(scriptFile);
+    private async runScriptFile(kernel: IKernel, scriptFile: Uri) {
+        if (await this.fs.exists(scriptFile)) {
+            const fileContents = await this.fs.readFile(scriptFile);
             return kernel.session ? executeSilently(kernel.session, fileContents) : [];
         }
-        traceError('Cannot run non-existant script file');
+        traceError('Cannot run non-existent script file');
     }
 
     private extractJupyterResultText(outputs: nbformat.IOutput[]): string {

src/kernels/variables/types.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 'use strict';
 
-import { CancellationToken, Event } from 'vscode';
+import { CancellationToken, Event, Uri } from 'vscode';
 import { IKernel } from '../types';
 import type { JSONObject } from '@lumino/coreutils';
 
@@ -24,7 +24,7 @@ export interface IJupyterVariable {
     rowCount?: number;
     indexColumn?: string;
     maximumRowChunkSize?: number;
-    fileName?: string;
+    fileName?: Uri;
 }
 
 export const IJupyterVariables = Symbol('IJupyterVariables');

src/platform/common/constants.node.ts → src/platform/common/scriptConstants.ts

@@ -1,12 +1,16 @@
-import { EXTENSION_ROOT_DIR } from '../constants.node';
-import * as path from '../../platform/vscode-path/path';
-
-export * from './constants';
+import { joinPath } from '../vscode-path/resources';
+import { IExtensionContext } from './types';
 
 export namespace DataFrameLoading {
-    export const SysPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'vscode_datascience_helpers', 'dataframes');
-    export const DataFrameSysImport = `import sys\nsys.path.append("${SysPath.replace(/\\/g, '\\\\')}")`;
-    export const ScriptPath = path.join(SysPath, 'vscodeDataFrame.py');
+    export function getScriptPath(context: IExtensionContext) {
+        return joinPath(
+            context.extensionUri,
+            'pythonFiles',
+            'vscode_datascience_helpers',
+            'dataframes',
+            'vscodeDataFrame.py'
+        );
+    }
 
     export const DataFrameInfoFunc = '_VSCODE_getDataFrameInfo';
     export const DataFrameRowFunc = '_VSCODE_getDataFrameRows';
@@ -18,14 +22,16 @@ export namespace DataFrameLoading {
 }
 
 export namespace GetVariableInfo {
-    export const SysPath = path.join(
-        EXTENSION_ROOT_DIR,
-        'pythonFiles',
-        'vscode_datascience_helpers',
-        'getVariableInfo'
-    );
-    export const GetVariableInfoSysImport = `import sys\nsys.path.append("${SysPath.replace(/\\/g, '\\\\')}")`;
-    export const ScriptPath = path.join(SysPath, 'vscodeGetVariableInfo.py');
+    export function getScriptPath(context: IExtensionContext) {
+        return joinPath(
+            context.extensionUri,
+            'pythonFiles',
+            'vscode_datascience_helpers',
+            'getVariableInfo',
+            'vscodeGetVariableInfo.py'
+        );
+    }
+
     export const VariableInfoFunc = '_VSCODE_getVariableInfo';
     export const VariablePropertiesFunc = '_VSCODE_getVariableProperties';
     export const VariableTypesFunc = '_VSCODE_getVariableTypes';
@@ -36,6 +42,13 @@ export namespace GetVariableInfo {
 }
 
 export namespace AddRunCellHook {
-    export const SysPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'vscode_datascience_helpers', 'kernel');
-    export const ScriptPath = path.join(SysPath, 'addRunCellHook.py');
+    export function getScriptPath(context: IExtensionContext) {
+        return joinPath(
+            context.extensionUri,
+            'pythonFiles',
+            'vscode_datascience_helpers',
+            'kernel',
+            'addRunCellHook.py'
+        );
+    }
 }

src/webviews/extension-side/dataviewer/jupyterVariableDataProvider.ts

@@ -16,6 +16,7 @@ import { IKernel } from '../../../kernels/types';
 import { IJupyterVariable, IJupyterVariables } from '../../../kernels/variables/types';
 import { traceError } from '../../../platform/logging';
 import { Identifiers } from '../../webview-side/common/constants';
+import { getFilePath } from '../../../platform/common/platform/fs-paths';
 
 @injectable()
 export class JupyterVariableDataProvider implements IJupyterVariableDataProvider {
@@ -115,7 +116,7 @@ export class JupyterVariableDataProvider implements IJupyterVariableDataProvider
                 type: variable.type,
                 maximumRowChunkSize: variable.maximumRowChunkSize,
                 name: variable.name,
-                fileName: variable.fileName
+                fileName: getFilePath(variable.fileName)
             };
         }
         if (isRefresh) {