From cfa19b01ee31290b7623f68b56aa52aab92b615f Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sat, 21 Sep 2024 02:18:38 -0700 Subject: [PATCH 01/25] Handle reloading REPL Window --- src/client/repl/nativeRepl.ts | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 413c795e80d6..f2bc5461c6bc 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -1,11 +1,13 @@ // Native Repl class that holds instance of pythonServer and replController import { + ExtensionContext, NotebookController, NotebookControllerAffinity, NotebookDocument, QuickPickItem, TextEditor, + Uri, workspace, WorkspaceFolder, } from 'vscode'; @@ -22,6 +24,7 @@ import { sendTelemetryEvent } from '../telemetry'; import { VariablesProvider } from './variables/variablesProvider'; import { VariableRequester } from './variables/variableRequester'; +const NATIVE_REPL_URI_MEMENTO = 'nativeReplUri'; let nativeRepl: NativeRepl | undefined; // In multi REPL scenario, hashmap of URI to Repl. export class NativeRepl implements Disposable { // Adding ! since it will get initialized in create method, not the constructor. @@ -39,14 +42,19 @@ export class NativeRepl implements Disposable { public newReplSession: boolean | undefined = true; + private replUri: Uri | undefined; + + private context: ExtensionContext; + // TODO: In the future, could also have attribute of URI for file specific REPL. - private constructor() { + private constructor(context: ExtensionContext) { this.watchNotebookClosed(); + this.context = context; } // Static async factory method to handle asynchronous initialization - public static async create(interpreter: PythonEnvironment): Promise { - const nativeRepl = new NativeRepl(); + public static async create(interpreter: PythonEnvironment, context: ExtensionContext): Promise { + const nativeRepl = new NativeRepl(context); nativeRepl.interpreter = interpreter; await nativeRepl.setReplDirectory(); nativeRepl.pythonServer = createPythonServer([interpreter.path as string], nativeRepl.cwd); @@ -65,10 +73,12 @@ export class NativeRepl implements Disposable { */ private watchNotebookClosed(): void { this.disposables.push( - workspace.onDidCloseNotebookDocument((nb) => { + workspace.onDidCloseNotebookDocument(async (nb) => { if (this.notebookDocument && nb.uri.toString() === this.notebookDocument.uri.toString()) { this.notebookDocument = undefined; this.newReplSession = true; + this.replUri = undefined; + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); } }), ); @@ -152,6 +162,8 @@ export class NativeRepl implements Disposable { public async sendToNativeRepl(code?: string): Promise { const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument); this.notebookDocument = notebookEditor.notebook; + this.replUri = this.notebookDocument.uri; + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri); if (this.notebookDocument) { this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default); @@ -168,9 +180,13 @@ export class NativeRepl implements Disposable { * @param interpreter * @returns Native REPL instance */ -export async function getNativeRepl(interpreter: PythonEnvironment, disposables: Disposable[]): Promise { +export async function getNativeRepl( + interpreter: PythonEnvironment, + disposables: Disposable[], + context: ExtensionContext, +): Promise { if (!nativeRepl) { - nativeRepl = await NativeRepl.create(interpreter); + nativeRepl = await NativeRepl.create(interpreter, context); disposables.push(nativeRepl); } if (nativeRepl && nativeRepl.newReplSession) { From 139f374c0209bf0d8c6e8b5aaaf412c42b94f613 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 22 Sep 2024 00:25:16 -0700 Subject: [PATCH 02/25] progress, still draft mode - exploring --- src/client/extensionActivation.ts | 6 +- src/client/repl/nativeRepl.ts | 3 +- src/client/repl/replCommandHandler.ts | 10 +- src/client/repl/replCommands.ts | 16 +- src/test/repl/nativeRepl.test.ts | 112 +++---- src/test/repl/replCommand.test.ts | 408 +++++++++++++------------- 6 files changed, 282 insertions(+), 273 deletions(-) diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 429004e951cb..2d22bc84c76e 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -111,9 +111,9 @@ export function activateFeatures(ext: ExtensionState, _components: Components): const executionHelper = ext.legacyIOC.serviceContainer.get(ICodeExecutionHelper); const commandManager = ext.legacyIOC.serviceContainer.get(ICommandManager); registerTriggerForTerminalREPL(ext.disposables); - registerStartNativeReplCommand(ext.disposables, interpreterService); - registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager); - registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager); + registerStartNativeReplCommand(ext.disposables, interpreterService, ext.context); + registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager, ext.context); + registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager, ext.context); } /// ////////////////////////// diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index f2bc5461c6bc..a94d61a1b3ca 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -160,7 +160,8 @@ export class NativeRepl implements Disposable { * @param code */ public async sendToNativeRepl(code?: string): Promise { - const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument); + const mementoValue = this.context.globalState.get(NATIVE_REPL_URI_MEMENTO) as Uri | undefined; + const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoValue); this.notebookDocument = notebookEditor.notebook; this.replUri = this.notebookDocument.uri; await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri); diff --git a/src/client/repl/replCommandHandler.ts b/src/client/repl/replCommandHandler.ts index b8fe579647a1..9cba7edc63b6 100644 --- a/src/client/repl/replCommandHandler.ts +++ b/src/client/repl/replCommandHandler.ts @@ -10,6 +10,7 @@ import { NotebookEdit, WorkspaceEdit, workspace, + Uri, } from 'vscode'; import { getExistingReplViewColumn } from './replUtils'; import { PVSC_EXTENSION_ID } from '../common/constants'; @@ -23,11 +24,14 @@ import { PVSC_EXTENSION_ID } from '../common/constants'; export async function openInteractiveREPL( notebookController: NotebookController, notebookDocument: NotebookDocument | undefined, + mementoValue: Uri | undefined, ): Promise { let viewColumn = ViewColumn.Beside; - - // Case where NotebookDocument (REPL document already exists in the tab) - if (notebookDocument) { + if (mementoValue) { + // Cachhed NotebookDocument exists. + notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri); + } else if (notebookDocument) { + // Case where NotebookDocument (REPL document already exists in the tab) const existingReplViewColumn = getExistingReplViewColumn(notebookDocument); viewColumn = existingReplViewColumn ?? viewColumn; } else if (!notebookDocument) { diff --git a/src/client/repl/replCommands.ts b/src/client/repl/replCommands.ts index 82b4aae4e5ee..ba725050b740 100644 --- a/src/client/repl/replCommands.ts +++ b/src/client/repl/replCommands.ts @@ -1,4 +1,4 @@ -import { commands, Uri, window } from 'vscode'; +import { commands, ExtensionContext, Uri, window } from 'vscode'; import { Disposable } from 'vscode-jsonrpc'; import { ICommandManager } from '../common/application/types'; import { Commands } from '../common/constants'; @@ -29,6 +29,7 @@ import { EventName } from '../telemetry/constants'; export async function registerStartNativeReplCommand( disposables: Disposable[], interpreterService: IInterpreterService, + context: ExtensionContext, ): Promise { disposables.push( registerCommand(Commands.Start_Native_REPL, async (uri: Uri) => { @@ -36,7 +37,7 @@ export async function registerStartNativeReplCommand( const interpreter = await getActiveInterpreter(uri, interpreterService); if (interpreter) { if (interpreter) { - const nativeRepl = await getNativeRepl(interpreter, disposables); + const nativeRepl = await getNativeRepl(interpreter, disposables, context); await nativeRepl.sendToNativeRepl(); } } @@ -55,6 +56,7 @@ export async function registerReplCommands( interpreterService: IInterpreterService, executionHelper: ICodeExecutionHelper, commandManager: ICommandManager, + context: ExtensionContext, ): Promise { disposables.push( commandManager.registerCommand(Commands.Exec_In_REPL, async (uri: Uri) => { @@ -67,7 +69,7 @@ export async function registerReplCommands( const interpreter = await getActiveInterpreter(uri, interpreterService); if (interpreter) { - const nativeRepl = await getNativeRepl(interpreter, disposables); + const nativeRepl = await getNativeRepl(interpreter, disposables, context); const activeEditor = window.activeTextEditor; if (activeEditor) { const code = await getSelectedTextToExecute(activeEditor); @@ -95,15 +97,16 @@ export async function registerReplExecuteOnEnter( disposables: Disposable[], interpreterService: IInterpreterService, commandManager: ICommandManager, + context: ExtensionContext, ): Promise { disposables.push( commandManager.registerCommand(Commands.Exec_In_REPL_Enter, async (uri: Uri) => { - await onInputEnter(uri, 'repl.execute', interpreterService, disposables); + await onInputEnter(uri, 'repl.execute', interpreterService, disposables, context); }), ); disposables.push( commandManager.registerCommand(Commands.Exec_In_IW_Enter, async (uri: Uri) => { - await onInputEnter(uri, 'interactive.execute', interpreterService, disposables); + await onInputEnter(uri, 'interactive.execute', interpreterService, disposables, context); }), ); } @@ -113,6 +116,7 @@ async function onInputEnter( commandName: string, interpreterService: IInterpreterService, disposables: Disposable[], + context: ExtensionContext, ): Promise { const interpreter = await interpreterService.getActiveInterpreter(uri); if (!interpreter) { @@ -120,7 +124,7 @@ async function onInputEnter( return; } - const nativeRepl = await getNativeRepl(interpreter, disposables); + const nativeRepl = await getNativeRepl(interpreter, disposables, context); const completeCode = await nativeRepl?.checkUserInputCompleteCode(window.activeTextEditor); const editor = window.activeTextEditor; diff --git a/src/test/repl/nativeRepl.test.ts b/src/test/repl/nativeRepl.test.ts index 0fc55abe1a64..c2f6d7edc063 100644 --- a/src/test/repl/nativeRepl.test.ts +++ b/src/test/repl/nativeRepl.test.ts @@ -1,70 +1,70 @@ -/* eslint-disable no-unused-expressions */ -/* eslint-disable @typescript-eslint/no-explicit-any */ -import * as TypeMoq from 'typemoq'; -import * as sinon from 'sinon'; -import { Disposable } from 'vscode'; -import { expect } from 'chai'; +// /* eslint-disable no-unused-expressions */ +// /* eslint-disable @typescript-eslint/no-explicit-any */ +// import * as TypeMoq from 'typemoq'; +// import * as sinon from 'sinon'; +// import { Disposable } from 'vscode'; +// import { expect } from 'chai'; -import { IInterpreterService } from '../../client/interpreter/contracts'; -import { PythonEnvironment } from '../../client/pythonEnvironments/info'; -import { getNativeRepl, NativeRepl } from '../../client/repl/nativeRepl'; +// import { IInterpreterService } from '../../client/interpreter/contracts'; +// import { PythonEnvironment } from '../../client/pythonEnvironments/info'; +// import { getNativeRepl, NativeRepl } from '../../client/repl/nativeRepl'; -suite('REPL - Native REPL', () => { - let interpreterService: TypeMoq.IMock; +// suite('REPL - Native REPL', () => { +// let interpreterService: TypeMoq.IMock; - let disposable: TypeMoq.IMock; - let disposableArray: Disposable[] = []; +// let disposable: TypeMoq.IMock; +// let disposableArray: Disposable[] = []; - let setReplDirectoryStub: sinon.SinonStub; - let setReplControllerSpy: sinon.SinonSpy; +// let setReplDirectoryStub: sinon.SinonStub; +// let setReplControllerSpy: sinon.SinonSpy; - setup(() => { - interpreterService = TypeMoq.Mock.ofType(); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - disposable = TypeMoq.Mock.ofType(); - disposableArray = [disposable.object]; +// setup(() => { +// interpreterService = TypeMoq.Mock.ofType(); +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); +// disposable = TypeMoq.Mock.ofType(); +// disposableArray = [disposable.object]; - setReplDirectoryStub = sinon.stub(NativeRepl.prototype as any, 'setReplDirectory').resolves(); // Stubbing private method - // Use a spy instead of a stub for setReplController - setReplControllerSpy = sinon.spy(NativeRepl.prototype, 'setReplController'); - }); +// setReplDirectoryStub = sinon.stub(NativeRepl.prototype as any, 'setReplDirectory').resolves(); // Stubbing private method +// // Use a spy instead of a stub for setReplController +// setReplControllerSpy = sinon.spy(NativeRepl.prototype, 'setReplController'); +// }); - teardown(() => { - disposableArray.forEach((d) => { - if (d) { - d.dispose(); - } - }); +// teardown(() => { +// disposableArray.forEach((d) => { +// if (d) { +// d.dispose(); +// } +// }); - disposableArray = []; - sinon.restore(); - }); +// disposableArray = []; +// sinon.restore(); +// }); - test('getNativeRepl should call create constructor', async () => { - const createMethodStub = sinon.stub(NativeRepl, 'create'); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - const interpreter = await interpreterService.object.getActiveInterpreter(); - await getNativeRepl(interpreter as PythonEnvironment, disposableArray); +// test('getNativeRepl should call create constructor', async () => { +// const createMethodStub = sinon.stub(NativeRepl, 'create'); +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); +// const interpreter = await interpreterService.object.getActiveInterpreter(); +// await getNativeRepl(interpreter as PythonEnvironment, disposableArray); - expect(createMethodStub.calledOnce).to.be.true; - }); +// expect(createMethodStub.calledOnce).to.be.true; +// }); - test('create should call setReplDirectory, setReplController', async () => { - const interpreter = await interpreterService.object.getActiveInterpreter(); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); +// test('create should call setReplDirectory, setReplController', async () => { +// const interpreter = await interpreterService.object.getActiveInterpreter(); +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - await NativeRepl.create(interpreter as PythonEnvironment); +// await NativeRepl.create(interpreter as PythonEnvironment); - expect(setReplDirectoryStub.calledOnce).to.be.true; - expect(setReplControllerSpy.calledOnce).to.be.true; +// expect(setReplDirectoryStub.calledOnce).to.be.true; +// expect(setReplControllerSpy.calledOnce).to.be.true; - setReplDirectoryStub.restore(); - setReplControllerSpy.restore(); - }); -}); +// setReplDirectoryStub.restore(); +// setReplControllerSpy.restore(); +// }); +// }); diff --git a/src/test/repl/replCommand.test.ts b/src/test/repl/replCommand.test.ts index 444b8e5f16b9..b4b8e347e040 100644 --- a/src/test/repl/replCommand.test.ts +++ b/src/test/repl/replCommand.test.ts @@ -1,204 +1,204 @@ -// Create test suite and test cases for the `replUtils` module -import * as TypeMoq from 'typemoq'; -import { Disposable } from 'vscode'; -import * as sinon from 'sinon'; -import { expect } from 'chai'; -import { IInterpreterService } from '../../client/interpreter/contracts'; -import { ICommandManager } from '../../client/common/application/types'; -import { ICodeExecutionHelper } from '../../client/terminals/types'; -import * as replCommands from '../../client/repl/replCommands'; -import * as replUtils from '../../client/repl/replUtils'; -import * as nativeRepl from '../../client/repl/nativeRepl'; -import { Commands } from '../../client/common/constants'; -import { PythonEnvironment } from '../../client/pythonEnvironments/info'; - -suite('REPL - register native repl command', () => { - let interpreterService: TypeMoq.IMock; - let commandManager: TypeMoq.IMock; - let executionHelper: TypeMoq.IMock; - let getSendToNativeREPLSettingStub: sinon.SinonStub; - // @ts-ignore: TS6133 - // eslint-disable-next-line @typescript-eslint/no-unused-vars - let registerCommandSpy: sinon.SinonSpy; - let executeInTerminalStub: sinon.SinonStub; - let getNativeReplStub: sinon.SinonStub; - let disposable: TypeMoq.IMock; - let disposableArray: Disposable[] = []; - setup(() => { - interpreterService = TypeMoq.Mock.ofType(); - commandManager = TypeMoq.Mock.ofType(); - executionHelper = TypeMoq.Mock.ofType(); - commandManager - .setup((cm) => cm.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => TypeMoq.Mock.ofType().object); - - getSendToNativeREPLSettingStub = sinon.stub(replUtils, 'getSendToNativeREPLSetting'); - getSendToNativeREPLSettingStub.returns(false); - executeInTerminalStub = sinon.stub(replUtils, 'executeInTerminal'); - executeInTerminalStub.returns(Promise.resolve()); - registerCommandSpy = sinon.spy(commandManager.object, 'registerCommand'); - disposable = TypeMoq.Mock.ofType(); - disposableArray = [disposable.object]; - }); - - teardown(() => { - sinon.restore(); - disposableArray.forEach((d) => { - if (d) { - d.dispose(); - } - }); - - disposableArray = []; - }); - - test('Ensure repl command is registered', async () => { - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - - await replCommands.registerReplCommands( - disposableArray, - interpreterService.object, - executionHelper.object, - commandManager.object, - ); - - commandManager.verify( - (c) => c.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny()), - TypeMoq.Times.atLeastOnce(), - ); - }); - - test('Ensure getSendToNativeREPLSetting is called', async () => { - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - - let commandHandler: undefined | (() => Promise); - commandManager - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .setup((c) => c.registerCommand as any) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { - if (command === Commands.Exec_In_REPL) { - commandHandler = callback; - } - // eslint-disable-next-line no-void - return { dispose: () => void 0 }; - }); - replCommands.registerReplCommands( - disposableArray, - interpreterService.object, - executionHelper.object, - commandManager.object, - ); - - expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - - await commandHandler!(); - - sinon.assert.calledOnce(getSendToNativeREPLSettingStub); - }); - - test('Ensure executeInTerminal is called when getSendToNativeREPLSetting returns false', async () => { - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - getSendToNativeREPLSettingStub.returns(false); - - let commandHandler: undefined | (() => Promise); - commandManager - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .setup((c) => c.registerCommand as any) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { - if (command === Commands.Exec_In_REPL) { - commandHandler = callback; - } - // eslint-disable-next-line no-void - return { dispose: () => void 0 }; - }); - replCommands.registerReplCommands( - disposableArray, - interpreterService.object, - executionHelper.object, - commandManager.object, - ); - - expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - - await commandHandler!(); - - sinon.assert.calledOnce(executeInTerminalStub); - }); - - test('Ensure we call getNativeREPL() when interpreter exist', async () => { - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - getSendToNativeREPLSettingStub.returns(true); - getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); - - let commandHandler: undefined | ((uri: string) => Promise); - commandManager - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .setup((c) => c.registerCommand as any) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { - if (command === Commands.Exec_In_REPL) { - commandHandler = callback; - } - // eslint-disable-next-line no-void - return { dispose: () => void 0 }; - }); - replCommands.registerReplCommands( - disposableArray, - interpreterService.object, - executionHelper.object, - commandManager.object, - ); - - expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - - await commandHandler!('uri'); - sinon.assert.calledOnce(getNativeReplStub); - }); - - test('Ensure we do not call getNativeREPL() when interpreter does not exist', async () => { - getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); - getSendToNativeREPLSettingStub.returns(true); - - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(undefined)); - - let commandHandler: undefined | ((uri: string) => Promise); - commandManager - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .setup((c) => c.registerCommand as any) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { - if (command === Commands.Exec_In_REPL) { - commandHandler = callback; - } - // eslint-disable-next-line no-void - return { dispose: () => void 0 }; - }); - interpreterService - .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(undefined)); - - replCommands.registerReplCommands( - disposableArray, - interpreterService.object, - executionHelper.object, - commandManager.object, - ); - - expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - - await commandHandler!('uri'); - sinon.assert.notCalled(getNativeReplStub); - }); -}); +// // Create test suite and test cases for the `replUtils` module +// import * as TypeMoq from 'typemoq'; +// import { Disposable } from 'vscode'; +// import * as sinon from 'sinon'; +// import { expect } from 'chai'; +// import { IInterpreterService } from '../../client/interpreter/contracts'; +// import { ICommandManager } from '../../client/common/application/types'; +// import { ICodeExecutionHelper } from '../../client/terminals/types'; +// import * as replCommands from '../../client/repl/replCommands'; +// import * as replUtils from '../../client/repl/replUtils'; +// import * as nativeRepl from '../../client/repl/nativeRepl'; +// import { Commands } from '../../client/common/constants'; +// import { PythonEnvironment } from '../../client/pythonEnvironments/info'; + +// suite('REPL - register native repl command', () => { +// let interpreterService: TypeMoq.IMock; +// let commandManager: TypeMoq.IMock; +// let executionHelper: TypeMoq.IMock; +// let getSendToNativeREPLSettingStub: sinon.SinonStub; +// // @ts-ignore: TS6133 +// // eslint-disable-next-line @typescript-eslint/no-unused-vars +// let registerCommandSpy: sinon.SinonSpy; +// let executeInTerminalStub: sinon.SinonStub; +// let getNativeReplStub: sinon.SinonStub; +// let disposable: TypeMoq.IMock; +// let disposableArray: Disposable[] = []; +// setup(() => { +// interpreterService = TypeMoq.Mock.ofType(); +// commandManager = TypeMoq.Mock.ofType(); +// executionHelper = TypeMoq.Mock.ofType(); +// commandManager +// .setup((cm) => cm.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) +// .returns(() => TypeMoq.Mock.ofType().object); + +// getSendToNativeREPLSettingStub = sinon.stub(replUtils, 'getSendToNativeREPLSetting'); +// getSendToNativeREPLSettingStub.returns(false); +// executeInTerminalStub = sinon.stub(replUtils, 'executeInTerminal'); +// executeInTerminalStub.returns(Promise.resolve()); +// registerCommandSpy = sinon.spy(commandManager.object, 'registerCommand'); +// disposable = TypeMoq.Mock.ofType(); +// disposableArray = [disposable.object]; +// }); + +// teardown(() => { +// sinon.restore(); +// disposableArray.forEach((d) => { +// if (d) { +// d.dispose(); +// } +// }); + +// disposableArray = []; +// }); + +// test('Ensure repl command is registered', async () => { +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + +// await replCommands.registerReplCommands( +// disposableArray, +// interpreterService.object, +// executionHelper.object, +// commandManager.object, +// ); + +// commandManager.verify( +// (c) => c.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny()), +// TypeMoq.Times.atLeastOnce(), +// ); +// }); + +// test('Ensure getSendToNativeREPLSetting is called', async () => { +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + +// let commandHandler: undefined | (() => Promise); +// commandManager +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .setup((c) => c.registerCommand as any) +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { +// if (command === Commands.Exec_In_REPL) { +// commandHandler = callback; +// } +// // eslint-disable-next-line no-void +// return { dispose: () => void 0 }; +// }); +// replCommands.registerReplCommands( +// disposableArray, +// interpreterService.object, +// executionHelper.object, +// commandManager.object, +// ); + +// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + +// await commandHandler!(); + +// sinon.assert.calledOnce(getSendToNativeREPLSettingStub); +// }); + +// test('Ensure executeInTerminal is called when getSendToNativeREPLSetting returns false', async () => { +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); +// getSendToNativeREPLSettingStub.returns(false); + +// let commandHandler: undefined | (() => Promise); +// commandManager +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .setup((c) => c.registerCommand as any) +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { +// if (command === Commands.Exec_In_REPL) { +// commandHandler = callback; +// } +// // eslint-disable-next-line no-void +// return { dispose: () => void 0 }; +// }); +// replCommands.registerReplCommands( +// disposableArray, +// interpreterService.object, +// executionHelper.object, +// commandManager.object, +// ); + +// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + +// await commandHandler!(); + +// sinon.assert.calledOnce(executeInTerminalStub); +// }); + +// test('Ensure we call getNativeREPL() when interpreter exist', async () => { +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); +// getSendToNativeREPLSettingStub.returns(true); +// getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); + +// let commandHandler: undefined | ((uri: string) => Promise); +// commandManager +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .setup((c) => c.registerCommand as any) +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { +// if (command === Commands.Exec_In_REPL) { +// commandHandler = callback; +// } +// // eslint-disable-next-line no-void +// return { dispose: () => void 0 }; +// }); +// replCommands.registerReplCommands( +// disposableArray, +// interpreterService.object, +// executionHelper.object, +// commandManager.object, +// ); + +// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + +// await commandHandler!('uri'); +// sinon.assert.calledOnce(getNativeReplStub); +// }); + +// test('Ensure we do not call getNativeREPL() when interpreter does not exist', async () => { +// getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); +// getSendToNativeREPLSettingStub.returns(true); + +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(undefined)); + +// let commandHandler: undefined | ((uri: string) => Promise); +// commandManager +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .setup((c) => c.registerCommand as any) +// // eslint-disable-next-line @typescript-eslint/no-explicit-any +// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { +// if (command === Commands.Exec_In_REPL) { +// commandHandler = callback; +// } +// // eslint-disable-next-line no-void +// return { dispose: () => void 0 }; +// }); +// interpreterService +// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve(undefined)); + +// replCommands.registerReplCommands( +// disposableArray, +// interpreterService.object, +// executionHelper.object, +// commandManager.object, +// ); + +// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + +// await commandHandler!('uri'); +// sinon.assert.notCalled(getNativeReplStub); +// }); +// }); From 112c0f490778405060166861de004ba53c06168d Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 13 Nov 2024 11:25:06 -0800 Subject: [PATCH 03/25] bunch of TODOs --- src/client/extensionActivation.ts | 4 ++++ src/client/repl/nativeRepl.ts | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 6be9c5d69495..29468ce98ad2 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -115,6 +115,10 @@ export function activateFeatures(ext: ExtensionState, _components: Components): registerStartNativeReplCommand(ext.disposables, interpreterService, ext.context); registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager, ext.context); registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager, ext.context); + + // TODO check tabs and if they match with memto it knows about. + // create repl, reload repl, close tab, then open notebook, and then we fall into trap where we think notebook is repl. + // check label of editor name - if its python repl or not. } /// ////////////////////////// diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 3b2c25c7aba0..7fc23e29a8a3 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -156,7 +156,15 @@ export class NativeRepl implements Disposable { * Function that opens interactive repl, selects kernel, and send/execute code to the native repl. */ public async sendToNativeRepl(code?: string): Promise { + // TODO need to check if that memento URI exist in my tab + // plain untitled notebook same uri as REPL. + // editor option check const mementoValue = this.context.globalState.get(NATIVE_REPL_URI_MEMENTO) as Uri | undefined; + // mementoValue = undefined; + + // try to restore notebook doc based on memento value. + // if I cant, then clear momento and openInteractiveREPL. + const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoValue); this.notebookDocument = notebookEditor.notebook; this.replUri = this.notebookDocument.uri; From 6b469459cba30e98c43557081ffa7c3f8c9ddd43 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 13 Nov 2024 12:18:07 -0800 Subject: [PATCH 04/25] WIP, workspace.textDocuments.map returns notebookcell document => invalid argument --- src/client/repl/nativeRepl.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 7fc23e29a8a3..31ed6e2348a1 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -156,19 +156,30 @@ export class NativeRepl implements Disposable { * Function that opens interactive repl, selects kernel, and send/execute code to the native repl. */ public async sendToNativeRepl(code?: string): Promise { + const mementoValue = (await this.context.globalState.get(NATIVE_REPL_URI_MEMENTO)) as string | undefined; + let mementoUri = mementoValue ? Uri.parse(mementoValue) : undefined; + + // const mementoFsPath = mementoUri?.fsPath; + // const openEditorsFsPath = workspace.textDocuments.map((doc) => doc.uri.fsPath); + const openEditors = workspace.textDocuments.map((doc) => doc.uri); // TODO need to check if that memento URI exist in my tab // plain untitled notebook same uri as REPL. // editor option check - const mementoValue = this.context.globalState.get(NATIVE_REPL_URI_MEMENTO) as Uri | undefined; - // mementoValue = undefined; + if (mementoUri && openEditors.includes(mementoUri)) { + this.replUri = mementoUri; + } else { + this.replUri = undefined; + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + mementoUri = undefined; + } // try to restore notebook doc based on memento value. // if I cant, then clear momento and openInteractiveREPL. - const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoValue); + const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoUri); this.notebookDocument = notebookEditor.notebook; this.replUri = this.notebookDocument.uri; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri); + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); // TODO store Uri as string and then parse this on recovery. if (this.notebookDocument) { this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default); From 74a31ef4081bc0ce3b1d8bc2a4c3a3a57ea7b076 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 13 Nov 2024 13:03:37 -0800 Subject: [PATCH 05/25] properly reload via fsPath. TODO: could use workspace.notebookDocuments instead. --- src/client/repl/nativeRepl.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 31ed6e2348a1..26696653167f 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -160,13 +160,21 @@ export class NativeRepl implements Disposable { let mementoUri = mementoValue ? Uri.parse(mementoValue) : undefined; // const mementoFsPath = mementoUri?.fsPath; - // const openEditorsFsPath = workspace.textDocuments.map((doc) => doc.uri.fsPath); + const openEditors = workspace.textDocuments.map((doc) => doc.uri); + const openEditorsFsPath = workspace.textDocuments.map((doc) => doc.uri.fsPath); // TODO need to check if that memento URI exist in my tab // plain untitled notebook same uri as REPL. // editor option check - if (mementoUri && openEditors.includes(mementoUri)) { - this.replUri = mementoUri; + if (mementoUri?.fsPath) { + const matchingEditor = openEditors.find((uri) => uri.fsPath === mementoUri?.fsPath); + if (matchingEditor) { + this.replUri = matchingEditor; + this.notebookDocument = workspace.notebookDocuments.find( + (doc) => doc.uri.fsPath === matchingEditor.fsPath, + ); + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); + } } else { this.replUri = undefined; await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); From 640e976ca8c5fdcd2aa6d261a0c666ffa81561fc Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 13 Nov 2024 16:14:08 -0800 Subject: [PATCH 06/25] use watching of notebookDocument instead of textEditor --- src/client/extensionActivation.ts | 3 +-- src/client/repl/nativeRepl.ts | 26 +++++++++----------------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 29468ce98ad2..a36cd93b0eaf 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -116,8 +116,7 @@ export function activateFeatures(ext: ExtensionState, _components: Components): registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager, ext.context); registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager, ext.context); - // TODO check tabs and if they match with memto it knows about. - // create repl, reload repl, close tab, then open notebook, and then we fall into trap where we think notebook is repl. + // TODO cover edge case: create repl, reload repl, close tab, then open notebook, and then we fall into trap where we think notebook is repl. // check label of editor name - if its python repl or not. } diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 26696653167f..2a4f86d52707 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -158,36 +158,28 @@ export class NativeRepl implements Disposable { public async sendToNativeRepl(code?: string): Promise { const mementoValue = (await this.context.globalState.get(NATIVE_REPL_URI_MEMENTO)) as string | undefined; let mementoUri = mementoValue ? Uri.parse(mementoValue) : undefined; + const openNotebookDocuments = workspace.notebookDocuments.map((doc) => doc.uri); - // const mementoFsPath = mementoUri?.fsPath; - - const openEditors = workspace.textDocuments.map((doc) => doc.uri); - const openEditorsFsPath = workspace.textDocuments.map((doc) => doc.uri.fsPath); - // TODO need to check if that memento URI exist in my tab - // plain untitled notebook same uri as REPL. - // editor option check - if (mementoUri?.fsPath) { - const matchingEditor = openEditors.find((uri) => uri.fsPath === mementoUri?.fsPath); - if (matchingEditor) { - this.replUri = matchingEditor; + if (mementoUri) { + const replTabBeforeReload = openNotebookDocuments.find((uri) => uri.fsPath === mementoUri?.fsPath); + if (replTabBeforeReload) { + this.replUri = replTabBeforeReload; this.notebookDocument = workspace.notebookDocuments.find( - (doc) => doc.uri.fsPath === matchingEditor.fsPath, + (doc) => doc.uri.fsPath === replTabBeforeReload.fsPath, ); await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); } } else { this.replUri = undefined; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); mementoUri = undefined; + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); } - // try to restore notebook doc based on memento value. - // if I cant, then clear momento and openInteractiveREPL. - const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoUri); + this.notebookDocument = notebookEditor.notebook; this.replUri = this.notebookDocument.uri; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); // TODO store Uri as string and then parse this on recovery. + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); if (this.notebookDocument) { this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default); From 48c86860a1ae0fc018577f19e0ed01c2a1ac7de8 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 13 Nov 2024 16:36:47 -0800 Subject: [PATCH 07/25] remove unncessary todo --- src/client/extensionActivation.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index a36cd93b0eaf..6be9c5d69495 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -115,9 +115,6 @@ export function activateFeatures(ext: ExtensionState, _components: Components): registerStartNativeReplCommand(ext.disposables, interpreterService, ext.context); registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager, ext.context); registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager, ext.context); - - // TODO cover edge case: create repl, reload repl, close tab, then open notebook, and then we fall into trap where we think notebook is repl. - // check label of editor name - if its python repl or not. } /// ////////////////////////// From 4a278e7af1f1ad18cc054c16311040cbe77c34cd Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 13 Nov 2024 21:08:38 -0800 Subject: [PATCH 08/25] tests --- src/test/repl/nativeRepl.test.ts | 116 +++++---- src/test/repl/replCommand.test.ts | 416 +++++++++++++++--------------- 2 files changed, 271 insertions(+), 261 deletions(-) diff --git a/src/test/repl/nativeRepl.test.ts b/src/test/repl/nativeRepl.test.ts index c2f6d7edc063..0f1e451e25d6 100644 --- a/src/test/repl/nativeRepl.test.ts +++ b/src/test/repl/nativeRepl.test.ts @@ -1,70 +1,72 @@ -// /* eslint-disable no-unused-expressions */ -// /* eslint-disable @typescript-eslint/no-explicit-any */ -// import * as TypeMoq from 'typemoq'; -// import * as sinon from 'sinon'; -// import { Disposable } from 'vscode'; -// import { expect } from 'chai'; +/* eslint-disable no-unused-expressions */ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import * as TypeMoq from 'typemoq'; +import * as sinon from 'sinon'; +import { Disposable } from 'vscode'; +import { expect } from 'chai'; -// import { IInterpreterService } from '../../client/interpreter/contracts'; -// import { PythonEnvironment } from '../../client/pythonEnvironments/info'; -// import { getNativeRepl, NativeRepl } from '../../client/repl/nativeRepl'; +import { IInterpreterService } from '../../client/interpreter/contracts'; +import { PythonEnvironment } from '../../client/pythonEnvironments/info'; +import { getNativeRepl, NativeRepl } from '../../client/repl/nativeRepl'; +import { IExtensionContext } from '../../client/common/types'; -// suite('REPL - Native REPL', () => { -// let interpreterService: TypeMoq.IMock; +suite('REPL - Native REPL', () => { + let interpreterService: TypeMoq.IMock; + let extensionContext: TypeMoq.IMock; + let disposable: TypeMoq.IMock; + let disposableArray: Disposable[] = []; -// let disposable: TypeMoq.IMock; -// let disposableArray: Disposable[] = []; + let setReplDirectoryStub: sinon.SinonStub; + let setReplControllerSpy: sinon.SinonSpy; -// let setReplDirectoryStub: sinon.SinonStub; -// let setReplControllerSpy: sinon.SinonSpy; + setup(() => { + interpreterService = TypeMoq.Mock.ofType(); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + disposable = TypeMoq.Mock.ofType(); + disposableArray = [disposable.object]; -// setup(() => { -// interpreterService = TypeMoq.Mock.ofType(); -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); -// disposable = TypeMoq.Mock.ofType(); -// disposableArray = [disposable.object]; + setReplDirectoryStub = sinon.stub(NativeRepl.prototype as any, 'setReplDirectory').resolves(); // Stubbing private method + // Use a spy instead of a stub for setReplController + setReplControllerSpy = sinon.spy(NativeRepl.prototype, 'setReplController'); + extensionContext = TypeMoq.Mock.ofType(); + }); -// setReplDirectoryStub = sinon.stub(NativeRepl.prototype as any, 'setReplDirectory').resolves(); // Stubbing private method -// // Use a spy instead of a stub for setReplController -// setReplControllerSpy = sinon.spy(NativeRepl.prototype, 'setReplController'); -// }); + teardown(() => { + disposableArray.forEach((d) => { + if (d) { + d.dispose(); + } + }); -// teardown(() => { -// disposableArray.forEach((d) => { -// if (d) { -// d.dispose(); -// } -// }); + disposableArray = []; + sinon.restore(); + }); -// disposableArray = []; -// sinon.restore(); -// }); + test('getNativeRepl should call create constructor', async () => { + const createMethodStub = sinon.stub(NativeRepl, 'create'); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + const interpreter = await interpreterService.object.getActiveInterpreter(); + await getNativeRepl(interpreter as PythonEnvironment, disposableArray, extensionContext.object); -// test('getNativeRepl should call create constructor', async () => { -// const createMethodStub = sinon.stub(NativeRepl, 'create'); -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); -// const interpreter = await interpreterService.object.getActiveInterpreter(); -// await getNativeRepl(interpreter as PythonEnvironment, disposableArray); + expect(createMethodStub.calledOnce).to.be.true; + }); -// expect(createMethodStub.calledOnce).to.be.true; -// }); + test('create should call setReplDirectory, setReplController', async () => { + const interpreter = await interpreterService.object.getActiveInterpreter(); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); -// test('create should call setReplDirectory, setReplController', async () => { -// const interpreter = await interpreterService.object.getActiveInterpreter(); -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + await NativeRepl.create(interpreter as PythonEnvironment, extensionContext.object); -// await NativeRepl.create(interpreter as PythonEnvironment); + expect(setReplDirectoryStub.calledOnce).to.be.true; + expect(setReplControllerSpy.calledOnce).to.be.true; -// expect(setReplDirectoryStub.calledOnce).to.be.true; -// expect(setReplControllerSpy.calledOnce).to.be.true; - -// setReplDirectoryStub.restore(); -// setReplControllerSpy.restore(); -// }); -// }); + setReplDirectoryStub.restore(); + setReplControllerSpy.restore(); + }); +}); diff --git a/src/test/repl/replCommand.test.ts b/src/test/repl/replCommand.test.ts index b4b8e347e040..02bec7c6a581 100644 --- a/src/test/repl/replCommand.test.ts +++ b/src/test/repl/replCommand.test.ts @@ -1,204 +1,212 @@ -// // Create test suite and test cases for the `replUtils` module -// import * as TypeMoq from 'typemoq'; -// import { Disposable } from 'vscode'; -// import * as sinon from 'sinon'; -// import { expect } from 'chai'; -// import { IInterpreterService } from '../../client/interpreter/contracts'; -// import { ICommandManager } from '../../client/common/application/types'; -// import { ICodeExecutionHelper } from '../../client/terminals/types'; -// import * as replCommands from '../../client/repl/replCommands'; -// import * as replUtils from '../../client/repl/replUtils'; -// import * as nativeRepl from '../../client/repl/nativeRepl'; -// import { Commands } from '../../client/common/constants'; -// import { PythonEnvironment } from '../../client/pythonEnvironments/info'; - -// suite('REPL - register native repl command', () => { -// let interpreterService: TypeMoq.IMock; -// let commandManager: TypeMoq.IMock; -// let executionHelper: TypeMoq.IMock; -// let getSendToNativeREPLSettingStub: sinon.SinonStub; -// // @ts-ignore: TS6133 -// // eslint-disable-next-line @typescript-eslint/no-unused-vars -// let registerCommandSpy: sinon.SinonSpy; -// let executeInTerminalStub: sinon.SinonStub; -// let getNativeReplStub: sinon.SinonStub; -// let disposable: TypeMoq.IMock; -// let disposableArray: Disposable[] = []; -// setup(() => { -// interpreterService = TypeMoq.Mock.ofType(); -// commandManager = TypeMoq.Mock.ofType(); -// executionHelper = TypeMoq.Mock.ofType(); -// commandManager -// .setup((cm) => cm.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) -// .returns(() => TypeMoq.Mock.ofType().object); - -// getSendToNativeREPLSettingStub = sinon.stub(replUtils, 'getSendToNativeREPLSetting'); -// getSendToNativeREPLSettingStub.returns(false); -// executeInTerminalStub = sinon.stub(replUtils, 'executeInTerminal'); -// executeInTerminalStub.returns(Promise.resolve()); -// registerCommandSpy = sinon.spy(commandManager.object, 'registerCommand'); -// disposable = TypeMoq.Mock.ofType(); -// disposableArray = [disposable.object]; -// }); - -// teardown(() => { -// sinon.restore(); -// disposableArray.forEach((d) => { -// if (d) { -// d.dispose(); -// } -// }); - -// disposableArray = []; -// }); - -// test('Ensure repl command is registered', async () => { -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - -// await replCommands.registerReplCommands( -// disposableArray, -// interpreterService.object, -// executionHelper.object, -// commandManager.object, -// ); - -// commandManager.verify( -// (c) => c.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny()), -// TypeMoq.Times.atLeastOnce(), -// ); -// }); - -// test('Ensure getSendToNativeREPLSetting is called', async () => { -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - -// let commandHandler: undefined | (() => Promise); -// commandManager -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .setup((c) => c.registerCommand as any) -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { -// if (command === Commands.Exec_In_REPL) { -// commandHandler = callback; -// } -// // eslint-disable-next-line no-void -// return { dispose: () => void 0 }; -// }); -// replCommands.registerReplCommands( -// disposableArray, -// interpreterService.object, -// executionHelper.object, -// commandManager.object, -// ); - -// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - -// await commandHandler!(); - -// sinon.assert.calledOnce(getSendToNativeREPLSettingStub); -// }); - -// test('Ensure executeInTerminal is called when getSendToNativeREPLSetting returns false', async () => { -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); -// getSendToNativeREPLSettingStub.returns(false); - -// let commandHandler: undefined | (() => Promise); -// commandManager -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .setup((c) => c.registerCommand as any) -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { -// if (command === Commands.Exec_In_REPL) { -// commandHandler = callback; -// } -// // eslint-disable-next-line no-void -// return { dispose: () => void 0 }; -// }); -// replCommands.registerReplCommands( -// disposableArray, -// interpreterService.object, -// executionHelper.object, -// commandManager.object, -// ); - -// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - -// await commandHandler!(); - -// sinon.assert.calledOnce(executeInTerminalStub); -// }); - -// test('Ensure we call getNativeREPL() when interpreter exist', async () => { -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); -// getSendToNativeREPLSettingStub.returns(true); -// getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); - -// let commandHandler: undefined | ((uri: string) => Promise); -// commandManager -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .setup((c) => c.registerCommand as any) -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { -// if (command === Commands.Exec_In_REPL) { -// commandHandler = callback; -// } -// // eslint-disable-next-line no-void -// return { dispose: () => void 0 }; -// }); -// replCommands.registerReplCommands( -// disposableArray, -// interpreterService.object, -// executionHelper.object, -// commandManager.object, -// ); - -// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - -// await commandHandler!('uri'); -// sinon.assert.calledOnce(getNativeReplStub); -// }); - -// test('Ensure we do not call getNativeREPL() when interpreter does not exist', async () => { -// getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); -// getSendToNativeREPLSettingStub.returns(true); - -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(undefined)); - -// let commandHandler: undefined | ((uri: string) => Promise); -// commandManager -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .setup((c) => c.registerCommand as any) -// // eslint-disable-next-line @typescript-eslint/no-explicit-any -// .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { -// if (command === Commands.Exec_In_REPL) { -// commandHandler = callback; -// } -// // eslint-disable-next-line no-void -// return { dispose: () => void 0 }; -// }); -// interpreterService -// .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve(undefined)); - -// replCommands.registerReplCommands( -// disposableArray, -// interpreterService.object, -// executionHelper.object, -// commandManager.object, -// ); - -// expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); - -// await commandHandler!('uri'); -// sinon.assert.notCalled(getNativeReplStub); -// }); -// }); +// Create test suite and test cases for the `replUtils` module +import * as TypeMoq from 'typemoq'; +import { Disposable } from 'vscode'; +import * as sinon from 'sinon'; +import { expect } from 'chai'; +import { IInterpreterService } from '../../client/interpreter/contracts'; +import { ICommandManager } from '../../client/common/application/types'; +import { ICodeExecutionHelper } from '../../client/terminals/types'; +import * as replCommands from '../../client/repl/replCommands'; +import * as replUtils from '../../client/repl/replUtils'; +import * as nativeRepl from '../../client/repl/nativeRepl'; +import { Commands } from '../../client/common/constants'; +import { PythonEnvironment } from '../../client/pythonEnvironments/info'; +import { IExtensionContext } from '../../client/common/types'; + +suite('REPL - register native repl command', () => { + let interpreterService: TypeMoq.IMock; + let commandManager: TypeMoq.IMock; + let executionHelper: TypeMoq.IMock; + let getSendToNativeREPLSettingStub: sinon.SinonStub; + // @ts-ignore: TS6133 + // eslint-disable-next-line @typescript-eslint/no-unused-vars + let registerCommandSpy: sinon.SinonSpy; + let executeInTerminalStub: sinon.SinonStub; + let getNativeReplStub: sinon.SinonStub; + let disposable: TypeMoq.IMock; + let disposableArray: Disposable[] = []; + let extensionContext: TypeMoq.IMock; + setup(() => { + interpreterService = TypeMoq.Mock.ofType(); + commandManager = TypeMoq.Mock.ofType(); + executionHelper = TypeMoq.Mock.ofType(); + commandManager + .setup((cm) => cm.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => TypeMoq.Mock.ofType().object); + + getSendToNativeREPLSettingStub = sinon.stub(replUtils, 'getSendToNativeREPLSetting'); + getSendToNativeREPLSettingStub.returns(false); + executeInTerminalStub = sinon.stub(replUtils, 'executeInTerminal'); + executeInTerminalStub.returns(Promise.resolve()); + registerCommandSpy = sinon.spy(commandManager.object, 'registerCommand'); + disposable = TypeMoq.Mock.ofType(); + disposableArray = [disposable.object]; + extensionContext = TypeMoq.Mock.ofType(); + }); + + teardown(() => { + sinon.restore(); + disposableArray.forEach((d) => { + if (d) { + d.dispose(); + } + }); + + disposableArray = []; + }); + + test('Ensure repl command is registered', async () => { + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + + await replCommands.registerReplCommands( + disposableArray, + interpreterService.object, + executionHelper.object, + commandManager.object, + extensionContext.object, + ); + + commandManager.verify( + (c) => c.registerCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + TypeMoq.Times.atLeastOnce(), + ); + }); + + test('Ensure getSendToNativeREPLSetting is called', async () => { + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + + let commandHandler: undefined | (() => Promise); + commandManager + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .setup((c) => c.registerCommand as any) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { + if (command === Commands.Exec_In_REPL) { + commandHandler = callback; + } + // eslint-disable-next-line no-void + return { dispose: () => void 0 }; + }); + replCommands.registerReplCommands( + disposableArray, + interpreterService.object, + executionHelper.object, + commandManager.object, + extensionContext.object, + ); + + expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + + await commandHandler!(); + + sinon.assert.calledOnce(getSendToNativeREPLSettingStub); + }); + + test('Ensure executeInTerminal is called when getSendToNativeREPLSetting returns false', async () => { + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + getSendToNativeREPLSettingStub.returns(false); + + let commandHandler: undefined | (() => Promise); + commandManager + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .setup((c) => c.registerCommand as any) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { + if (command === Commands.Exec_In_REPL) { + commandHandler = callback; + } + // eslint-disable-next-line no-void + return { dispose: () => void 0 }; + }); + replCommands.registerReplCommands( + disposableArray, + interpreterService.object, + executionHelper.object, + commandManager.object, + extensionContext.object, + ); + + expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + + await commandHandler!(); + + sinon.assert.calledOnce(executeInTerminalStub); + }); + + test('Ensure we call getNativeREPL() when interpreter exist', async () => { + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + getSendToNativeREPLSettingStub.returns(true); + getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); + + let commandHandler: undefined | ((uri: string) => Promise); + commandManager + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .setup((c) => c.registerCommand as any) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { + if (command === Commands.Exec_In_REPL) { + commandHandler = callback; + } + // eslint-disable-next-line no-void + return { dispose: () => void 0 }; + }); + replCommands.registerReplCommands( + disposableArray, + interpreterService.object, + executionHelper.object, + commandManager.object, + extensionContext.object, + ); + + expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + + await commandHandler!('uri'); + sinon.assert.calledOnce(getNativeReplStub); + }); + + test('Ensure we do not call getNativeREPL() when interpreter does not exist', async () => { + getNativeReplStub = sinon.stub(nativeRepl, 'getNativeRepl'); + getSendToNativeREPLSettingStub.returns(true); + + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(undefined)); + + let commandHandler: undefined | ((uri: string) => Promise); + commandManager + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .setup((c) => c.registerCommand as any) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .returns(() => (command: string, callback: (...args: any[]) => any, _thisArg?: any) => { + if (command === Commands.Exec_In_REPL) { + commandHandler = callback; + } + // eslint-disable-next-line no-void + return { dispose: () => void 0 }; + }); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(undefined)); + + replCommands.registerReplCommands( + disposableArray, + interpreterService.object, + executionHelper.object, + commandManager.object, + extensionContext.object, + ); + + expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); + + await commandHandler!('uri'); + sinon.assert.notCalled(getNativeReplStub); + }); +}); From 3a0c4fd1685d1b25050324d78f4702c13b62dea3 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 14 Nov 2024 23:50:33 -0800 Subject: [PATCH 09/25] tab groups are returning same untitled-1-ipynb for notebook and REPL --- src/client/repl/nativeRepl.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 2a4f86d52707..93c8f39c944d 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -6,12 +6,17 @@ import { NotebookControllerAffinity, NotebookDocument, QuickPickItem, + TabInputText, TextEditor, Uri, workspace, WorkspaceFolder, + window, + TabInputTextDiff, + TabInputNotebook, } from 'vscode'; import { Disposable } from 'vscode-jsonrpc'; +import * as path from 'path'; import { PVSC_EXTENSION_ID } from '../common/constants'; import { showQuickPick } from '../common/vscodeApis/windowApis'; import { getWorkspaceFolders } from '../common/vscodeApis/workspaceApis'; @@ -168,6 +173,9 @@ export class NativeRepl implements Disposable { (doc) => doc.uri.fsPath === replTabBeforeReload.fsPath, ); await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); + const myFileName = path.basename(this.replUri.fsPath); + const whatever = 'hi'; + const tabNames = getOpenTabNames(); } } else { this.replUri = undefined; @@ -191,6 +199,28 @@ export class NativeRepl implements Disposable { } } +function getOpenTabNames(): string[] { + const tabNames: string[] = []; + const tabGroups = window.tabGroups.all; + + for (const tabGroup of tabGroups) { + for (const tab of tabGroup.tabs) { + if (tab.input instanceof TabInputText) { + tabNames.push(tab.input.uri.fsPath); + } else if (tab.input instanceof TabInputTextDiff) { + if (tab.input.modified instanceof Uri) { + tabNames.push(tab.input.modified.fsPath); + } + } else if (tab.input instanceof TabInputNotebook) { + tabNames.push(tab.input.uri.fsPath); + } + } + } + const tabGroupTemp = tabGroups; + + return tabNames; +} + /** * Get Singleton Native REPL Instance * @param interpreter From f54ef3a0b4dc3de27139ceb1c1a0cf156638b575 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 15 Nov 2024 11:19:03 -0800 Subject: [PATCH 10/25] use tab.label to differentiate untitled notebook vs. native repl as suggested --- src/client/repl/nativeRepl.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 93c8f39c944d..b3d74b1137f1 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -205,18 +205,9 @@ function getOpenTabNames(): string[] { for (const tabGroup of tabGroups) { for (const tab of tabGroup.tabs) { - if (tab.input instanceof TabInputText) { - tabNames.push(tab.input.uri.fsPath); - } else if (tab.input instanceof TabInputTextDiff) { - if (tab.input.modified instanceof Uri) { - tabNames.push(tab.input.modified.fsPath); - } - } else if (tab.input instanceof TabInputNotebook) { - tabNames.push(tab.input.uri.fsPath); - } + tabNames.push(tab.label); } } - const tabGroupTemp = tabGroups; return tabNames; } From afa89c997bb2a4c4d6739e6c5a3d076c689352fc Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 15 Nov 2024 20:32:22 -0800 Subject: [PATCH 11/25] now handling edge case but need a huge clean up --- src/client/repl/nativeRepl.ts | 46 ++++++++++++++++++++++++--- src/client/repl/replCommandHandler.ts | 1 + 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index b3d74b1137f1..9f65df9567fb 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -6,14 +6,14 @@ import { NotebookControllerAffinity, NotebookDocument, QuickPickItem, - TabInputText, TextEditor, Uri, workspace, WorkspaceFolder, window, - TabInputTextDiff, TabInputNotebook, + TabInputTextDiff, + TabInputText, } from 'vscode'; import { Disposable } from 'vscode-jsonrpc'; import * as path from 'path'; @@ -174,13 +174,25 @@ export class NativeRepl implements Disposable { ); await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); const myFileName = path.basename(this.replUri.fsPath); - const whatever = 'hi'; - const tabNames = getOpenTabNames(); + + // const tabNames = getOpenTabNames(); + const tabLabel = getTabNameForUri(this.replUri); + if (tabLabel !== 'Python REPL') { + const regex = /^Untitled-\d+\.ipynb$/; + const isUntitled = regex.test(myFileName); + + this.replUri = undefined; + mementoUri = undefined; + + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + this.notebookDocument = undefined; + } } } else { this.replUri = undefined; mementoUri = undefined; await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + this.notebookDocument = undefined; } const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoUri); @@ -208,10 +220,34 @@ function getOpenTabNames(): string[] { tabNames.push(tab.label); } } - + // TODO if tabName includes 'Python REPL' and there are other 'Untitled-*.ipynb' then, we need to re-create REPL instance with new URI. return tabNames; } +function getTabNameForUri(uri: Uri): string | undefined { + const tabGroups = window.tabGroups.all; + + for (const tabGroup of tabGroups) { + for (const tab of tabGroup.tabs) { + if (tab.input instanceof TabInputText && tab.input.uri.toString() === uri.toString()) { + return tab.label; + } + if (tab.input instanceof TabInputTextDiff) { + if ( + tab.input.original.toString() === uri.toString() || + tab.input.modified.toString() === uri.toString() + ) { + return tab.label; + } + } else if (tab.input instanceof TabInputNotebook && tab.input.uri.toString() === uri.toString()) { + return tab.label; + } + } + } + + return undefined; +} + /** * Get Singleton Native REPL Instance * @param interpreter diff --git a/src/client/repl/replCommandHandler.ts b/src/client/repl/replCommandHandler.ts index 03bcd6cc46e2..6bfd4d87bc8e 100644 --- a/src/client/repl/replCommandHandler.ts +++ b/src/client/repl/replCommandHandler.ts @@ -25,6 +25,7 @@ export async function openInteractiveREPL( ): Promise { let viewColumn = ViewColumn.Beside; if (mementoValue) { + // also check if memento value URI tab has file name of Python REPL // Cachhed NotebookDocument exists. notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri); } else if (notebookDocument) { From 44bd14d3bf4351bd8783787f5743cd5e0ac4e4c4 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sat, 16 Nov 2024 14:31:26 -0800 Subject: [PATCH 12/25] remove this.replUri to make my life easier --- src/client/repl/nativeRepl.ts | 65 +++------------------------ src/client/repl/replCommandHandler.ts | 5 ++- src/client/repl/replUtils.ts | 18 ++++++++ 3 files changed, 27 insertions(+), 61 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 9f65df9567fb..e69678d567bc 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -10,13 +10,8 @@ import { Uri, workspace, WorkspaceFolder, - window, - TabInputNotebook, - TabInputTextDiff, - TabInputText, } from 'vscode'; import { Disposable } from 'vscode-jsonrpc'; -import * as path from 'path'; import { PVSC_EXTENSION_ID } from '../common/constants'; import { showQuickPick } from '../common/vscodeApis/windowApis'; import { getWorkspaceFolders } from '../common/vscodeApis/workspaceApis'; @@ -28,6 +23,7 @@ import { EventName } from '../telemetry/constants'; import { sendTelemetryEvent } from '../telemetry'; import { VariablesProvider } from './variables/variablesProvider'; import { VariableRequester } from './variables/variableRequester'; +import { getTabNameForUri } from './replUtils'; const NATIVE_REPL_URI_MEMENTO = 'nativeReplUri'; let nativeRepl: NativeRepl | undefined; // In multi REPL scenario, hashmap of URI to Repl. @@ -47,8 +43,6 @@ export class NativeRepl implements Disposable { public newReplSession: boolean | undefined = true; - private replUri: Uri | undefined; - private context: ExtensionContext; // TODO: In the future, could also have attribute of URI for file specific REPL. @@ -82,7 +76,6 @@ export class NativeRepl implements Disposable { if (this.notebookDocument && nb.uri.toString() === this.notebookDocument.uri.toString()) { this.notebookDocument = undefined; this.newReplSession = true; - this.replUri = undefined; await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); } }), @@ -168,28 +161,20 @@ export class NativeRepl implements Disposable { if (mementoUri) { const replTabBeforeReload = openNotebookDocuments.find((uri) => uri.fsPath === mementoUri?.fsPath); if (replTabBeforeReload) { - this.replUri = replTabBeforeReload; this.notebookDocument = workspace.notebookDocuments.find( (doc) => doc.uri.fsPath === replTabBeforeReload.fsPath, ); - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); - const myFileName = path.basename(this.replUri.fsPath); - - // const tabNames = getOpenTabNames(); - const tabLabel = getTabNameForUri(this.replUri); - if (tabLabel !== 'Python REPL') { - const regex = /^Untitled-\d+\.ipynb$/; - const isUntitled = regex.test(myFileName); + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, replTabBeforeReload.toString()); - this.replUri = undefined; + // If repl URI does not have tabLabel 'Python REPL', something has changed: + // e.g. creation of untitled notebook without Python extension knowing. + if (getTabNameForUri(replTabBeforeReload) !== 'Python REPL') { mementoUri = undefined; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); this.notebookDocument = undefined; } } } else { - this.replUri = undefined; mementoUri = undefined; await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); this.notebookDocument = undefined; @@ -198,8 +183,7 @@ export class NativeRepl implements Disposable { const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoUri); this.notebookDocument = notebookEditor.notebook; - this.replUri = this.notebookDocument.uri; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.replUri.toString()); + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); if (this.notebookDocument) { this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default); @@ -211,43 +195,6 @@ export class NativeRepl implements Disposable { } } -function getOpenTabNames(): string[] { - const tabNames: string[] = []; - const tabGroups = window.tabGroups.all; - - for (const tabGroup of tabGroups) { - for (const tab of tabGroup.tabs) { - tabNames.push(tab.label); - } - } - // TODO if tabName includes 'Python REPL' and there are other 'Untitled-*.ipynb' then, we need to re-create REPL instance with new URI. - return tabNames; -} - -function getTabNameForUri(uri: Uri): string | undefined { - const tabGroups = window.tabGroups.all; - - for (const tabGroup of tabGroups) { - for (const tab of tabGroup.tabs) { - if (tab.input instanceof TabInputText && tab.input.uri.toString() === uri.toString()) { - return tab.label; - } - if (tab.input instanceof TabInputTextDiff) { - if ( - tab.input.original.toString() === uri.toString() || - tab.input.modified.toString() === uri.toString() - ) { - return tab.label; - } - } else if (tab.input instanceof TabInputNotebook && tab.input.uri.toString() === uri.toString()) { - return tab.label; - } - } - } - - return undefined; -} - /** * Get Singleton Native REPL Instance * @param interpreter diff --git a/src/client/repl/replCommandHandler.ts b/src/client/repl/replCommandHandler.ts index 6bfd4d87bc8e..97c4364e777d 100644 --- a/src/client/repl/replCommandHandler.ts +++ b/src/client/repl/replCommandHandler.ts @@ -26,14 +26,15 @@ export async function openInteractiveREPL( let viewColumn = ViewColumn.Beside; if (mementoValue) { // also check if memento value URI tab has file name of Python REPL - // Cachhed NotebookDocument exists. + // Cached NotebookDocument exists. notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri); } else if (notebookDocument) { // Case where NotebookDocument (REPL document already exists in the tab) const existingReplViewColumn = getExistingReplViewColumn(notebookDocument); viewColumn = existingReplViewColumn ?? viewColumn; } else if (!notebookDocument) { - // Case where NotebookDocument doesnt exist, create a blank one. + // Case where NotebookDocument doesnt exist, or + // became outdated (untitled.ipynb created without Python extension knowing, effectively taking over original Python REPL's URI) notebookDocument = await workspace.openNotebookDocument('jupyter-notebook'); } const editor = window.showNotebookDocument(notebookDocument!, { diff --git a/src/client/repl/replUtils.ts b/src/client/repl/replUtils.ts index 5f58f461155b..6f0fd8bc1dee 100644 --- a/src/client/repl/replUtils.ts +++ b/src/client/repl/replUtils.ts @@ -99,3 +99,21 @@ export function getExistingReplViewColumn(notebookDocument: NotebookDocument): V } return undefined; } +/** + * Function that will return tab name for before reloading VS Code + * This is so we can make sure tab name is still 'Python REPL' after reloading VS Code, + * and make sure Python REPL does not get 'merged' into unaware untitled.ipynb tab. + */ +export function getTabNameForUri(uri: Uri): string | undefined { + const tabGroups = window.tabGroups.all; + + for (const tabGroup of tabGroups) { + for (const tab of tabGroup.tabs) { + if (tab.input instanceof TabInputNotebook && tab.input.uri.toString() === uri.toString()) { + return tab.label; + } + } + } + + return undefined; +} From fc966f53c1d09e7a0d89e8f11849c73aaba5e317 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sat, 16 Nov 2024 15:01:20 -0800 Subject: [PATCH 13/25] more refactoring to make my life easier --- src/client/repl/nativeRepl.ts | 16 +++++++++++----- src/client/repl/replUtils.ts | 1 + 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index e69678d567bc..d1c3e47d0fb4 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -170,14 +170,11 @@ export class NativeRepl implements Disposable { // e.g. creation of untitled notebook without Python extension knowing. if (getTabNameForUri(replTabBeforeReload) !== 'Python REPL') { mementoUri = undefined; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); - this.notebookDocument = undefined; + await this.cleanRepl(); } } } else { - mementoUri = undefined; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); - this.notebookDocument = undefined; + await this.cleanRepl(); } const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoUri); @@ -193,6 +190,15 @@ export class NativeRepl implements Disposable { } } } + + /** + * Properly clean up notebook document stored inside Native REPL. + * Also remove the Native REPL URI from memento to prepare for brand new REPL creation. + */ + private async cleanRepl(): Promise { + this.notebookDocument = undefined; + await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + } } /** diff --git a/src/client/repl/replUtils.ts b/src/client/repl/replUtils.ts index 6f0fd8bc1dee..0c2c4ba0d84e 100644 --- a/src/client/repl/replUtils.ts +++ b/src/client/repl/replUtils.ts @@ -99,6 +99,7 @@ export function getExistingReplViewColumn(notebookDocument: NotebookDocument): V } return undefined; } + /** * Function that will return tab name for before reloading VS Code * This is so we can make sure tab name is still 'Python REPL' after reloading VS Code, From 79a14780ba7def542df0af808dc9f608b017a119 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sat, 16 Nov 2024 15:06:37 -0800 Subject: [PATCH 14/25] remove unused --- src/client/repl/nativeRepl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index d1c3e47d0fb4..e58b8b3796ff 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -26,7 +26,7 @@ import { VariableRequester } from './variables/variableRequester'; import { getTabNameForUri } from './replUtils'; const NATIVE_REPL_URI_MEMENTO = 'nativeReplUri'; -let nativeRepl: NativeRepl | undefined; // In multi REPL scenario, hashmap of URI to Repl. +let nativeRepl: NativeRepl | undefined; export class NativeRepl implements Disposable { // Adding ! since it will get initialized in create method, not the constructor. private pythonServer!: PythonServer; From 140771b18b98ab29e5d1129ca64fc7f42f8f583f Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 17 Nov 2024 22:10:52 -0800 Subject: [PATCH 15/25] co-authored from @amunger via #24451 --- src/client/repl/nativeRepl.ts | 9 +++++++-- src/client/repl/replCommandHandler.ts | 3 ++- src/client/repl/replCommands.ts | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index e58b8b3796ff..ff85bbb8b4d4 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -153,7 +153,7 @@ export class NativeRepl implements Disposable { /** * Function that opens interactive repl, selects kernel, and send/execute code to the native repl. */ - public async sendToNativeRepl(code?: string): Promise { + public async sendToNativeRepl(code?: string | undefined, preserveFocus: boolean = true): Promise { const mementoValue = (await this.context.globalState.get(NATIVE_REPL_URI_MEMENTO)) as string | undefined; let mementoUri = mementoValue ? Uri.parse(mementoValue) : undefined; const openNotebookDocuments = workspace.notebookDocuments.map((doc) => doc.uri); @@ -177,7 +177,12 @@ export class NativeRepl implements Disposable { await this.cleanRepl(); } - const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument, mementoUri); + const notebookEditor = await openInteractiveREPL( + this.replController, + this.notebookDocument, + mementoUri, + preserveFocus, + ); this.notebookDocument = notebookEditor.notebook; await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); diff --git a/src/client/repl/replCommandHandler.ts b/src/client/repl/replCommandHandler.ts index 97c4364e777d..3385c91a355c 100644 --- a/src/client/repl/replCommandHandler.ts +++ b/src/client/repl/replCommandHandler.ts @@ -22,6 +22,7 @@ export async function openInteractiveREPL( notebookController: NotebookController, notebookDocument: NotebookDocument | undefined, mementoValue: Uri | undefined, + preserveFocus: boolean = true, ): Promise { let viewColumn = ViewColumn.Beside; if (mementoValue) { @@ -40,7 +41,7 @@ export async function openInteractiveREPL( const editor = window.showNotebookDocument(notebookDocument!, { viewColumn, asRepl: 'Python REPL', - preserveFocus: true, + preserveFocus, }); await commands.executeCommand('notebook.selectKernel', { editor, diff --git a/src/client/repl/replCommands.ts b/src/client/repl/replCommands.ts index da3356d0c95b..ab81f07d6482 100644 --- a/src/client/repl/replCommands.ts +++ b/src/client/repl/replCommands.ts @@ -34,7 +34,7 @@ export async function registerStartNativeReplCommand( if (interpreter) { if (interpreter) { const nativeRepl = await getNativeRepl(interpreter, disposables, context); - await nativeRepl.sendToNativeRepl(); + await nativeRepl.sendToNativeRepl(undefined, false); } } }), From 418c1a31ff85e6b5f86f82899f2c91eb7d835026 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 18 Nov 2024 00:05:23 -0800 Subject: [PATCH 16/25] test --- src/client/repl/nativeRepl.ts | 2 +- src/test/repl/nativeRepl.test.ts | 35 ++++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index ff85bbb8b4d4..8c78cd124b0c 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -25,7 +25,7 @@ import { VariablesProvider } from './variables/variablesProvider'; import { VariableRequester } from './variables/variableRequester'; import { getTabNameForUri } from './replUtils'; -const NATIVE_REPL_URI_MEMENTO = 'nativeReplUri'; +export const NATIVE_REPL_URI_MEMENTO = 'nativeReplUri'; let nativeRepl: NativeRepl | undefined; export class NativeRepl implements Disposable { // Adding ! since it will get initialized in create method, not the constructor. diff --git a/src/test/repl/nativeRepl.test.ts b/src/test/repl/nativeRepl.test.ts index 0f1e451e25d6..fa582070284a 100644 --- a/src/test/repl/nativeRepl.test.ts +++ b/src/test/repl/nativeRepl.test.ts @@ -2,23 +2,24 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import * as TypeMoq from 'typemoq'; import * as sinon from 'sinon'; -import { Disposable } from 'vscode'; +import { Disposable, ExtensionContext } from 'vscode'; import { expect } from 'chai'; import { IInterpreterService } from '../../client/interpreter/contracts'; import { PythonEnvironment } from '../../client/pythonEnvironments/info'; -import { getNativeRepl, NativeRepl } from '../../client/repl/nativeRepl'; +import { getNativeRepl, NATIVE_REPL_URI_MEMENTO, NativeRepl } from '../../client/repl/nativeRepl'; import { IExtensionContext } from '../../client/common/types'; +import * as replUtils from '../../client/repl/replUtils'; suite('REPL - Native REPL', () => { let interpreterService: TypeMoq.IMock; let extensionContext: TypeMoq.IMock; let disposable: TypeMoq.IMock; let disposableArray: Disposable[] = []; - let setReplDirectoryStub: sinon.SinonStub; let setReplControllerSpy: sinon.SinonSpy; - + let memento: TypeMoq.IMock; + let getTabNameForUriStub: sinon.SinonStub; setup(() => { interpreterService = TypeMoq.Mock.ofType(); interpreterService @@ -26,11 +27,14 @@ suite('REPL - Native REPL', () => { .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); disposable = TypeMoq.Mock.ofType(); disposableArray = [disposable.object]; - + memento = TypeMoq.Mock.ofType(); setReplDirectoryStub = sinon.stub(NativeRepl.prototype as any, 'setReplDirectory').resolves(); // Stubbing private method // Use a spy instead of a stub for setReplController + getTabNameForUriStub = sinon.stub(replUtils, 'getTabNameForUri').returns('tabName'); setReplControllerSpy = sinon.spy(NativeRepl.prototype, 'setReplController'); extensionContext = TypeMoq.Mock.ofType(); + extensionContext.setup((c) => c.globalState).returns(() => memento.object); + memento.setup((m) => m.get(NATIVE_REPL_URI_MEMENTO)).returns(() => undefined); }); teardown(() => { @@ -39,9 +43,10 @@ suite('REPL - Native REPL', () => { d.dispose(); } }); - disposableArray = []; sinon.restore(); + extensionContext?.reset(); + memento?.reset(); }); test('getNativeRepl should call create constructor', async () => { @@ -55,6 +60,24 @@ suite('REPL - Native REPL', () => { expect(createMethodStub.calledOnce).to.be.true; }); + test('sendToNativeRepl with undefined URI should not try to reload', async () => { + memento.setup((m) => m.get(NATIVE_REPL_URI_MEMENTO)).returns(() => undefined); + + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + const interpreter = await interpreterService.object.getActiveInterpreter(); + const nativeRepl = await getNativeRepl( + interpreter as PythonEnvironment, + disposableArray, + extensionContext.object, + ); + + nativeRepl.sendToNativeRepl(undefined, false); + + expect(getTabNameForUriStub.notCalled).to.be.true; + }); + test('create should call setReplDirectory, setReplController', async () => { const interpreter = await interpreterService.object.getActiveInterpreter(); interpreterService From 5698f49ded82293d0959e2715744dcb4c761f9f9 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 18 Nov 2024 12:11:15 -0800 Subject: [PATCH 17/25] remove context, unncessary openNB check, use workspace memento --- src/client/extensionActivation.ts | 6 +-- src/client/repl/nativeRepl.ts | 60 ++++++++++----------------- src/client/repl/replCommandHandler.ts | 8 ++-- src/client/repl/replCommands.ts | 16 +++---- src/test/repl/nativeRepl.test.ts | 10 ++--- src/test/repl/replCommand.test.ts | 9 +--- 6 files changed, 41 insertions(+), 68 deletions(-) diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 6be9c5d69495..38f2d6a56277 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -112,9 +112,9 @@ export function activateFeatures(ext: ExtensionState, _components: Components): const executionHelper = ext.legacyIOC.serviceContainer.get(ICodeExecutionHelper); const commandManager = ext.legacyIOC.serviceContainer.get(ICommandManager); registerTriggerForTerminalREPL(ext.disposables); - registerStartNativeReplCommand(ext.disposables, interpreterService, ext.context); - registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager, ext.context); - registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager, ext.context); + registerStartNativeReplCommand(ext.disposables, interpreterService); + registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager); + registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager); } /// ////////////////////////// diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 8c78cd124b0c..7bda5b84c8fd 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -1,7 +1,6 @@ // Native Repl class that holds instance of pythonServer and replController import { - ExtensionContext, NotebookController, NotebookControllerAffinity, NotebookDocument, @@ -24,6 +23,7 @@ import { sendTelemetryEvent } from '../telemetry'; import { VariablesProvider } from './variables/variablesProvider'; import { VariableRequester } from './variables/variableRequester'; import { getTabNameForUri } from './replUtils'; +import { getWorkspaceStateValue, updateWorkspaceStateValue } from '../common/persistentState'; export const NATIVE_REPL_URI_MEMENTO = 'nativeReplUri'; let nativeRepl: NativeRepl | undefined; @@ -43,17 +43,14 @@ export class NativeRepl implements Disposable { public newReplSession: boolean | undefined = true; - private context: ExtensionContext; - // TODO: In the future, could also have attribute of URI for file specific REPL. - private constructor(context: ExtensionContext) { + private constructor() { this.watchNotebookClosed(); - this.context = context; } // Static async factory method to handle asynchronous initialization - public static async create(interpreter: PythonEnvironment, context: ExtensionContext): Promise { - const nativeRepl = new NativeRepl(context); + public static async create(interpreter: PythonEnvironment): Promise { + const nativeRepl = new NativeRepl(); nativeRepl.interpreter = interpreter; await nativeRepl.setReplDirectory(); nativeRepl.pythonServer = createPythonServer([interpreter.path as string], nativeRepl.cwd); @@ -76,7 +73,8 @@ export class NativeRepl implements Disposable { if (this.notebookDocument && nb.uri.toString() === this.notebookDocument.uri.toString()) { this.notebookDocument = undefined; this.newReplSession = true; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + // await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); } }), ); @@ -154,38 +152,29 @@ export class NativeRepl implements Disposable { * Function that opens interactive repl, selects kernel, and send/execute code to the native repl. */ public async sendToNativeRepl(code?: string | undefined, preserveFocus: boolean = true): Promise { - const mementoValue = (await this.context.globalState.get(NATIVE_REPL_URI_MEMENTO)) as string | undefined; - let mementoUri = mementoValue ? Uri.parse(mementoValue) : undefined; - const openNotebookDocuments = workspace.notebookDocuments.map((doc) => doc.uri); - - if (mementoUri) { - const replTabBeforeReload = openNotebookDocuments.find((uri) => uri.fsPath === mementoUri?.fsPath); - if (replTabBeforeReload) { - this.notebookDocument = workspace.notebookDocuments.find( - (doc) => doc.uri.fsPath === replTabBeforeReload.fsPath, - ); - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, replTabBeforeReload.toString()); - - // If repl URI does not have tabLabel 'Python REPL', something has changed: - // e.g. creation of untitled notebook without Python extension knowing. - if (getTabNameForUri(replTabBeforeReload) !== 'Python REPL') { - mementoUri = undefined; - await this.cleanRepl(); - } + let wsMementoUri: Uri | undefined; + + if (!this.notebookDocument) { + const wsMemento = getWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO); + wsMementoUri = wsMemento ? Uri.parse(wsMemento) : undefined; + + if (!wsMementoUri || getTabNameForUri(wsMementoUri) !== 'Python REPL') { + await this.cleanRepl(); + wsMementoUri = undefined; + this.notebookDocument = undefined; } - } else { - await this.cleanRepl(); } const notebookEditor = await openInteractiveREPL( this.replController, this.notebookDocument, - mementoUri, + wsMementoUri, preserveFocus, ); this.notebookDocument = notebookEditor.notebook; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); + // await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); + updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); if (this.notebookDocument) { this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default); @@ -202,7 +191,8 @@ export class NativeRepl implements Disposable { */ private async cleanRepl(): Promise { this.notebookDocument = undefined; - await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + // await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); + updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); } } @@ -211,13 +201,9 @@ export class NativeRepl implements Disposable { * @param interpreter * @returns Native REPL instance */ -export async function getNativeRepl( - interpreter: PythonEnvironment, - disposables: Disposable[], - context: ExtensionContext, -): Promise { +export async function getNativeRepl(interpreter: PythonEnvironment, disposables: Disposable[]): Promise { if (!nativeRepl) { - nativeRepl = await NativeRepl.create(interpreter, context); + nativeRepl = await NativeRepl.create(interpreter); disposables.push(nativeRepl); } if (nativeRepl && nativeRepl.newReplSession) { diff --git a/src/client/repl/replCommandHandler.ts b/src/client/repl/replCommandHandler.ts index 3385c91a355c..f695642cdcc5 100644 --- a/src/client/repl/replCommandHandler.ts +++ b/src/client/repl/replCommandHandler.ts @@ -26,9 +26,9 @@ export async function openInteractiveREPL( ): Promise { let viewColumn = ViewColumn.Beside; if (mementoValue) { - // also check if memento value URI tab has file name of Python REPL - // Cached NotebookDocument exists. - notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri); + if (!notebookDocument) { + notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri); + } } else if (notebookDocument) { // Case where NotebookDocument (REPL document already exists in the tab) const existingReplViewColumn = getExistingReplViewColumn(notebookDocument); @@ -43,6 +43,8 @@ export async function openInteractiveREPL( asRepl: 'Python REPL', preserveFocus, }); + // sanity check that we opened a Native REPL from showNotebookDocument. + // if not true, set notebook = undefined. await commands.executeCommand('notebook.selectKernel', { editor, id: notebookController.id, diff --git a/src/client/repl/replCommands.ts b/src/client/repl/replCommands.ts index ab81f07d6482..f260185988a8 100644 --- a/src/client/repl/replCommands.ts +++ b/src/client/repl/replCommands.ts @@ -1,4 +1,4 @@ -import { commands, ExtensionContext, Uri, window } from 'vscode'; +import { commands, Uri, window } from 'vscode'; import { Disposable } from 'vscode-jsonrpc'; import { ICommandManager } from '../common/application/types'; import { Commands } from '../common/constants'; @@ -25,7 +25,6 @@ import { ReplType } from './types'; export async function registerStartNativeReplCommand( disposables: Disposable[], interpreterService: IInterpreterService, - context: ExtensionContext, ): Promise { disposables.push( registerCommand(Commands.Start_Native_REPL, async (uri: Uri) => { @@ -33,7 +32,7 @@ export async function registerStartNativeReplCommand( const interpreter = await getActiveInterpreter(uri, interpreterService); if (interpreter) { if (interpreter) { - const nativeRepl = await getNativeRepl(interpreter, disposables, context); + const nativeRepl = await getNativeRepl(interpreter, disposables); await nativeRepl.sendToNativeRepl(undefined, false); } } @@ -49,7 +48,6 @@ export async function registerReplCommands( interpreterService: IInterpreterService, executionHelper: ICodeExecutionHelper, commandManager: ICommandManager, - context: ExtensionContext, ): Promise { disposables.push( commandManager.registerCommand(Commands.Exec_In_REPL, async (uri: Uri) => { @@ -62,7 +60,7 @@ export async function registerReplCommands( const interpreter = await getActiveInterpreter(uri, interpreterService); if (interpreter) { - const nativeRepl = await getNativeRepl(interpreter, disposables, context); + const nativeRepl = await getNativeRepl(interpreter, disposables); const activeEditor = window.activeTextEditor; if (activeEditor) { const code = await getSelectedTextToExecute(activeEditor); @@ -92,16 +90,15 @@ export async function registerReplExecuteOnEnter( disposables: Disposable[], interpreterService: IInterpreterService, commandManager: ICommandManager, - context: ExtensionContext, ): Promise { disposables.push( commandManager.registerCommand(Commands.Exec_In_REPL_Enter, async (uri: Uri) => { - await onInputEnter(uri, 'repl.execute', interpreterService, disposables, context); + await onInputEnter(uri, 'repl.execute', interpreterService, disposables); }), ); disposables.push( commandManager.registerCommand(Commands.Exec_In_IW_Enter, async (uri: Uri) => { - await onInputEnter(uri, 'interactive.execute', interpreterService, disposables, context); + await onInputEnter(uri, 'interactive.execute', interpreterService, disposables); }), ); } @@ -111,7 +108,6 @@ async function onInputEnter( commandName: string, interpreterService: IInterpreterService, disposables: Disposable[], - context: ExtensionContext, ): Promise { const interpreter = await interpreterService.getActiveInterpreter(uri); if (!interpreter) { @@ -119,7 +115,7 @@ async function onInputEnter( return; } - const nativeRepl = await getNativeRepl(interpreter, disposables, context); + const nativeRepl = await getNativeRepl(interpreter, disposables); const completeCode = await nativeRepl?.checkUserInputCompleteCode(window.activeTextEditor); const editor = window.activeTextEditor; diff --git a/src/test/repl/nativeRepl.test.ts b/src/test/repl/nativeRepl.test.ts index fa582070284a..fb3213d95e8a 100644 --- a/src/test/repl/nativeRepl.test.ts +++ b/src/test/repl/nativeRepl.test.ts @@ -55,7 +55,7 @@ suite('REPL - Native REPL', () => { .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); const interpreter = await interpreterService.object.getActiveInterpreter(); - await getNativeRepl(interpreter as PythonEnvironment, disposableArray, extensionContext.object); + await getNativeRepl(interpreter as PythonEnvironment, disposableArray); expect(createMethodStub.calledOnce).to.be.true; }); @@ -67,11 +67,7 @@ suite('REPL - Native REPL', () => { .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); const interpreter = await interpreterService.object.getActiveInterpreter(); - const nativeRepl = await getNativeRepl( - interpreter as PythonEnvironment, - disposableArray, - extensionContext.object, - ); + const nativeRepl = await getNativeRepl(interpreter as PythonEnvironment, disposableArray); nativeRepl.sendToNativeRepl(undefined, false); @@ -84,7 +80,7 @@ suite('REPL - Native REPL', () => { .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); - await NativeRepl.create(interpreter as PythonEnvironment, extensionContext.object); + await NativeRepl.create(interpreter as PythonEnvironment); expect(setReplDirectoryStub.calledOnce).to.be.true; expect(setReplControllerSpy.calledOnce).to.be.true; diff --git a/src/test/repl/replCommand.test.ts b/src/test/repl/replCommand.test.ts index 02bec7c6a581..7c26ebd69c80 100644 --- a/src/test/repl/replCommand.test.ts +++ b/src/test/repl/replCommand.test.ts @@ -11,7 +11,6 @@ import * as replUtils from '../../client/repl/replUtils'; import * as nativeRepl from '../../client/repl/nativeRepl'; import { Commands } from '../../client/common/constants'; import { PythonEnvironment } from '../../client/pythonEnvironments/info'; -import { IExtensionContext } from '../../client/common/types'; suite('REPL - register native repl command', () => { let interpreterService: TypeMoq.IMock; @@ -25,7 +24,7 @@ suite('REPL - register native repl command', () => { let getNativeReplStub: sinon.SinonStub; let disposable: TypeMoq.IMock; let disposableArray: Disposable[] = []; - let extensionContext: TypeMoq.IMock; + setup(() => { interpreterService = TypeMoq.Mock.ofType(); commandManager = TypeMoq.Mock.ofType(); @@ -41,7 +40,6 @@ suite('REPL - register native repl command', () => { registerCommandSpy = sinon.spy(commandManager.object, 'registerCommand'); disposable = TypeMoq.Mock.ofType(); disposableArray = [disposable.object]; - extensionContext = TypeMoq.Mock.ofType(); }); teardown(() => { @@ -65,7 +63,6 @@ suite('REPL - register native repl command', () => { interpreterService.object, executionHelper.object, commandManager.object, - extensionContext.object, ); commandManager.verify( @@ -96,7 +93,6 @@ suite('REPL - register native repl command', () => { interpreterService.object, executionHelper.object, commandManager.object, - extensionContext.object, ); expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); @@ -129,7 +125,6 @@ suite('REPL - register native repl command', () => { interpreterService.object, executionHelper.object, commandManager.object, - extensionContext.object, ); expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); @@ -163,7 +158,6 @@ suite('REPL - register native repl command', () => { interpreterService.object, executionHelper.object, commandManager.object, - extensionContext.object, ); expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); @@ -201,7 +195,6 @@ suite('REPL - register native repl command', () => { interpreterService.object, executionHelper.object, commandManager.object, - extensionContext.object, ); expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized'); From 6bb40562c9498ad3a335be7c2de8ed243a2fc2be Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 18 Nov 2024 12:32:31 -0800 Subject: [PATCH 18/25] remove dup --- src/client/repl/nativeRepl.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 7bda5b84c8fd..56aeee99fc63 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -161,7 +161,6 @@ export class NativeRepl implements Disposable { if (!wsMementoUri || getTabNameForUri(wsMementoUri) !== 'Python REPL') { await this.cleanRepl(); wsMementoUri = undefined; - this.notebookDocument = undefined; } } From 60fbabc01b66faeac69ac70d4e910aeae614f246 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 18 Nov 2024 16:11:29 -0800 Subject: [PATCH 19/25] add different test for chagning to workspace memento from globalstate --- src/test/repl/nativeRepl.test.ts | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/test/repl/nativeRepl.test.ts b/src/test/repl/nativeRepl.test.ts index fb3213d95e8a..75af050c515f 100644 --- a/src/test/repl/nativeRepl.test.ts +++ b/src/test/repl/nativeRepl.test.ts @@ -2,24 +2,25 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import * as TypeMoq from 'typemoq'; import * as sinon from 'sinon'; -import { Disposable, ExtensionContext } from 'vscode'; +import { Disposable } from 'vscode'; import { expect } from 'chai'; import { IInterpreterService } from '../../client/interpreter/contracts'; import { PythonEnvironment } from '../../client/pythonEnvironments/info'; -import { getNativeRepl, NATIVE_REPL_URI_MEMENTO, NativeRepl } from '../../client/repl/nativeRepl'; -import { IExtensionContext } from '../../client/common/types'; +import { getNativeRepl, NativeRepl } from '../../client/repl/nativeRepl'; + import * as replUtils from '../../client/repl/replUtils'; +import * as persistentState from '../../client/common/persistentState'; suite('REPL - Native REPL', () => { let interpreterService: TypeMoq.IMock; - let extensionContext: TypeMoq.IMock; + let disposable: TypeMoq.IMock; let disposableArray: Disposable[] = []; let setReplDirectoryStub: sinon.SinonStub; let setReplControllerSpy: sinon.SinonSpy; - let memento: TypeMoq.IMock; - let getTabNameForUriStub: sinon.SinonStub; + let getWorkspaceStateValueStub: sinon.SinonStub; + setup(() => { interpreterService = TypeMoq.Mock.ofType(); interpreterService @@ -27,14 +28,11 @@ suite('REPL - Native REPL', () => { .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); disposable = TypeMoq.Mock.ofType(); disposableArray = [disposable.object]; - memento = TypeMoq.Mock.ofType(); + setReplDirectoryStub = sinon.stub(NativeRepl.prototype as any, 'setReplDirectory').resolves(); // Stubbing private method // Use a spy instead of a stub for setReplController - getTabNameForUriStub = sinon.stub(replUtils, 'getTabNameForUri').returns('tabName'); setReplControllerSpy = sinon.spy(NativeRepl.prototype, 'setReplController'); - extensionContext = TypeMoq.Mock.ofType(); - extensionContext.setup((c) => c.globalState).returns(() => memento.object); - memento.setup((m) => m.get(NATIVE_REPL_URI_MEMENTO)).returns(() => undefined); + getWorkspaceStateValueStub = sinon.stub(persistentState, 'getWorkspaceStateValue').returns(undefined); }); teardown(() => { @@ -45,8 +43,6 @@ suite('REPL - Native REPL', () => { }); disposableArray = []; sinon.restore(); - extensionContext?.reset(); - memento?.reset(); }); test('getNativeRepl should call create constructor', async () => { @@ -60,9 +56,7 @@ suite('REPL - Native REPL', () => { expect(createMethodStub.calledOnce).to.be.true; }); - test('sendToNativeRepl with undefined URI should not try to reload', async () => { - memento.setup((m) => m.get(NATIVE_REPL_URI_MEMENTO)).returns(() => undefined); - + test('sendToNativeRepl should look for memento URI if notebook document is undefined', async () => { interpreterService .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); @@ -71,7 +65,7 @@ suite('REPL - Native REPL', () => { nativeRepl.sendToNativeRepl(undefined, false); - expect(getTabNameForUriStub.notCalled).to.be.true; + expect(getWorkspaceStateValueStub.calledOnce).to.be.true; }); test('create should call setReplDirectory, setReplController', async () => { From 8d7e2acdfdaabe1adb66c2afb786986894b329f1 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 18 Nov 2024 16:14:00 -0800 Subject: [PATCH 20/25] make compiler happy --- src/test/repl/nativeRepl.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/repl/nativeRepl.test.ts b/src/test/repl/nativeRepl.test.ts index 75af050c515f..69b7e7268a70 100644 --- a/src/test/repl/nativeRepl.test.ts +++ b/src/test/repl/nativeRepl.test.ts @@ -8,8 +8,6 @@ import { expect } from 'chai'; import { IInterpreterService } from '../../client/interpreter/contracts'; import { PythonEnvironment } from '../../client/pythonEnvironments/info'; import { getNativeRepl, NativeRepl } from '../../client/repl/nativeRepl'; - -import * as replUtils from '../../client/repl/replUtils'; import * as persistentState from '../../client/common/persistentState'; suite('REPL - Native REPL', () => { From 992c7996e707e9c490938de06821eade867bf076 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 18 Nov 2024 16:26:25 -0800 Subject: [PATCH 21/25] do not leave leftover comments --- src/client/repl/nativeRepl.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 56aeee99fc63..11406b472dae 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -73,7 +73,6 @@ export class NativeRepl implements Disposable { if (this.notebookDocument && nb.uri.toString() === this.notebookDocument.uri.toString()) { this.notebookDocument = undefined; this.newReplSession = true; - // await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); } }), @@ -172,7 +171,6 @@ export class NativeRepl implements Disposable { ); this.notebookDocument = notebookEditor.notebook; - // await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); if (this.notebookDocument) { @@ -190,7 +188,6 @@ export class NativeRepl implements Disposable { */ private async cleanRepl(): Promise { this.notebookDocument = undefined; - // await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined); updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); } } From dc6dcb5a3f2c1a2f4a31a9255611efe4505859f8 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 18 Nov 2024 18:57:10 -0800 Subject: [PATCH 22/25] better sanity check, more test --- src/client/repl/nativeRepl.ts | 20 ++++++++++++-------- src/client/repl/replCommandHandler.ts | 19 ++++++++++++++----- src/test/repl/nativeRepl.test.ts | 17 ++++++++++++++++- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 11406b472dae..aff09cf1451e 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -169,15 +169,19 @@ export class NativeRepl implements Disposable { wsMementoUri, preserveFocus, ); + if (notebookEditor) { + this.notebookDocument = notebookEditor.notebook; + updateWorkspaceStateValue( + NATIVE_REPL_URI_MEMENTO, + this.notebookDocument.uri.toString(), + ); - this.notebookDocument = notebookEditor.notebook; - updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString()); - - if (this.notebookDocument) { - this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default); - await selectNotebookKernel(notebookEditor, this.replController.id, PVSC_EXTENSION_ID); - if (code) { - await executeNotebookCell(notebookEditor, code); + if (this.notebookDocument) { + this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default); + await selectNotebookKernel(notebookEditor, this.replController.id, PVSC_EXTENSION_ID); + if (code) { + await executeNotebookCell(notebookEditor, code); + } } } } diff --git a/src/client/repl/replCommandHandler.ts b/src/client/repl/replCommandHandler.ts index f695642cdcc5..ef5927228f48 100644 --- a/src/client/repl/replCommandHandler.ts +++ b/src/client/repl/replCommandHandler.ts @@ -12,7 +12,7 @@ import { workspace, Uri, } from 'vscode'; -import { getExistingReplViewColumn } from './replUtils'; +import { getExistingReplViewColumn, getTabNameForUri } from './replUtils'; import { PVSC_EXTENSION_ID } from '../common/constants'; /** @@ -23,7 +23,7 @@ export async function openInteractiveREPL( notebookDocument: NotebookDocument | undefined, mementoValue: Uri | undefined, preserveFocus: boolean = true, -): Promise { +): Promise { let viewColumn = ViewColumn.Beside; if (mementoValue) { if (!notebookDocument) { @@ -38,13 +38,22 @@ export async function openInteractiveREPL( // became outdated (untitled.ipynb created without Python extension knowing, effectively taking over original Python REPL's URI) notebookDocument = await workspace.openNotebookDocument('jupyter-notebook'); } - const editor = window.showNotebookDocument(notebookDocument!, { + const editor = await window.showNotebookDocument(notebookDocument!, { viewColumn, asRepl: 'Python REPL', preserveFocus, }); - // sanity check that we opened a Native REPL from showNotebookDocument. - // if not true, set notebook = undefined. + + // Sanity check that we opened a Native REPL from showNotebookDocument. + if ( + !editor || + !editor.notebook || + !editor.notebook.uri || + getTabNameForUri(editor.notebook.uri) !== 'Python REPL' + ) { + return undefined; + } + await commands.executeCommand('notebook.selectKernel', { editor, id: notebookController.id, diff --git a/src/test/repl/nativeRepl.test.ts b/src/test/repl/nativeRepl.test.ts index 69b7e7268a70..999bc656c64d 100644 --- a/src/test/repl/nativeRepl.test.ts +++ b/src/test/repl/nativeRepl.test.ts @@ -18,6 +18,7 @@ suite('REPL - Native REPL', () => { let setReplDirectoryStub: sinon.SinonStub; let setReplControllerSpy: sinon.SinonSpy; let getWorkspaceStateValueStub: sinon.SinonStub; + let updateWorkspaceStateValueStub: sinon.SinonStub; setup(() => { interpreterService = TypeMoq.Mock.ofType(); @@ -30,7 +31,7 @@ suite('REPL - Native REPL', () => { setReplDirectoryStub = sinon.stub(NativeRepl.prototype as any, 'setReplDirectory').resolves(); // Stubbing private method // Use a spy instead of a stub for setReplController setReplControllerSpy = sinon.spy(NativeRepl.prototype, 'setReplController'); - getWorkspaceStateValueStub = sinon.stub(persistentState, 'getWorkspaceStateValue').returns(undefined); + updateWorkspaceStateValueStub = sinon.stub(persistentState, 'updateWorkspaceStateValue').resolves(); }); teardown(() => { @@ -55,6 +56,7 @@ suite('REPL - Native REPL', () => { }); test('sendToNativeRepl should look for memento URI if notebook document is undefined', async () => { + getWorkspaceStateValueStub = sinon.stub(persistentState, 'getWorkspaceStateValue').returns(undefined); interpreterService .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); @@ -66,6 +68,19 @@ suite('REPL - Native REPL', () => { expect(getWorkspaceStateValueStub.calledOnce).to.be.true; }); + test('sendToNativeRepl should call updateWorkspaceStateValue', async () => { + getWorkspaceStateValueStub = sinon.stub(persistentState, 'getWorkspaceStateValue').returns('myNameIsMemento'); + interpreterService + .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment)); + const interpreter = await interpreterService.object.getActiveInterpreter(); + const nativeRepl = await getNativeRepl(interpreter as PythonEnvironment, disposableArray); + + nativeRepl.sendToNativeRepl(undefined, false); + + expect(updateWorkspaceStateValueStub.calledOnce).to.be.true; + }); + test('create should call setReplDirectory, setReplController', async () => { const interpreter = await interpreterService.object.getActiveInterpreter(); interpreterService From 435c20050f140e782eb87f45797880b9ec7eff2e Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 19 Nov 2024 07:49:00 -0800 Subject: [PATCH 23/25] make sure to use await --- src/client/repl/nativeRepl.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index aff09cf1451e..b4b44a8db964 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -73,7 +73,7 @@ export class NativeRepl implements Disposable { if (this.notebookDocument && nb.uri.toString() === this.notebookDocument.uri.toString()) { this.notebookDocument = undefined; this.newReplSession = true; - updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); + await updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); } }), ); @@ -171,7 +171,7 @@ export class NativeRepl implements Disposable { ); if (notebookEditor) { this.notebookDocument = notebookEditor.notebook; - updateWorkspaceStateValue( + await updateWorkspaceStateValue( NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString(), ); @@ -192,7 +192,7 @@ export class NativeRepl implements Disposable { */ private async cleanRepl(): Promise { this.notebookDocument = undefined; - updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); + await updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); } } From 639caafe154e4959c0398677b95632d99af7e923 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 19 Nov 2024 08:27:08 -0800 Subject: [PATCH 24/25] leverage notebookDocument parameter typing to be nb|Uri|undefined for openInteractive --- src/client/repl/nativeRepl.ts | 3 +-- src/client/repl/replCommandHandler.ts | 10 ++++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index b4b44a8db964..44c72da9a8f3 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -165,8 +165,7 @@ export class NativeRepl implements Disposable { const notebookEditor = await openInteractiveREPL( this.replController, - this.notebookDocument, - wsMementoUri, + this.notebookDocument ?? wsMementoUri, preserveFocus, ); if (notebookEditor) { diff --git a/src/client/repl/replCommandHandler.ts b/src/client/repl/replCommandHandler.ts index ef5927228f48..89ccbe11c337 100644 --- a/src/client/repl/replCommandHandler.ts +++ b/src/client/repl/replCommandHandler.ts @@ -20,15 +20,13 @@ import { PVSC_EXTENSION_ID } from '../common/constants'; */ export async function openInteractiveREPL( notebookController: NotebookController, - notebookDocument: NotebookDocument | undefined, - mementoValue: Uri | undefined, + notebookDocument: NotebookDocument | Uri | undefined, preserveFocus: boolean = true, ): Promise { let viewColumn = ViewColumn.Beside; - if (mementoValue) { - if (!notebookDocument) { - notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri); - } + if (notebookDocument instanceof Uri) { + // Case where NotebookDocument is undefined, but workspace mementoURI exists. + notebookDocument = await workspace.openNotebookDocument(notebookDocument); } else if (notebookDocument) { // Case where NotebookDocument (REPL document already exists in the tab) const existingReplViewColumn = getExistingReplViewColumn(notebookDocument); From c3648f5794c14390578026d2de0927d9a09bc8b3 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 19 Nov 2024 08:38:21 -0800 Subject: [PATCH 25/25] get rid of unncessary cleanRepl() --- src/client/repl/nativeRepl.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/client/repl/nativeRepl.ts b/src/client/repl/nativeRepl.ts index 44c72da9a8f3..6edd3cbd70a7 100644 --- a/src/client/repl/nativeRepl.ts +++ b/src/client/repl/nativeRepl.ts @@ -158,7 +158,7 @@ export class NativeRepl implements Disposable { wsMementoUri = wsMemento ? Uri.parse(wsMemento) : undefined; if (!wsMementoUri || getTabNameForUri(wsMementoUri) !== 'Python REPL') { - await this.cleanRepl(); + await updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); wsMementoUri = undefined; } } @@ -184,15 +184,6 @@ export class NativeRepl implements Disposable { } } } - - /** - * Properly clean up notebook document stored inside Native REPL. - * Also remove the Native REPL URI from memento to prepare for brand new REPL creation. - */ - private async cleanRepl(): Promise { - this.notebookDocument = undefined; - await updateWorkspaceStateValue(NATIVE_REPL_URI_MEMENTO, undefined); - } } /**