Skip to content

Commit 5698f49

Browse files
anthonykim1anthonykim1
committed
remove context, unncessary openNB check, use workspace memento
1 parent 418c1a3 commit 5698f49

File tree

6 files changed

+41
-68
lines changed

6 files changed

+41
-68
lines changed

‎src/client/extensionActivation.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,9 @@ export function activateFeatures(ext: ExtensionState, _components: Components):
112112
const executionHelper = ext.legacyIOC.serviceContainer.get<ICodeExecutionHelper>(ICodeExecutionHelper);
113113
const commandManager = ext.legacyIOC.serviceContainer.get<ICommandManager>(ICommandManager);
114114
registerTriggerForTerminalREPL(ext.disposables);
115-
registerStartNativeReplCommand(ext.disposables, interpreterService, ext.context);
116-
registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager, ext.context);
117-
registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager, ext.context);
115+
registerStartNativeReplCommand(ext.disposables, interpreterService);
116+
registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager);
117+
registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager);
118118
}
119119

120120
/// //////////////////////////

‎src/client/repl/nativeRepl.ts‎

Lines changed: 23 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Native Repl class that holds instance of pythonServer and replController
22

33
import {
4-
ExtensionContext,
54
NotebookController,
65
NotebookControllerAffinity,
76
NotebookDocument,
@@ -24,6 +23,7 @@ import { sendTelemetryEvent } from '../telemetry';
2423
import { VariablesProvider } from './variables/variablesProvider';
2524
import { VariableRequester } from './variables/variableRequester';
2625
import { getTabNameForUri } from './replUtils';
26+
import { getWorkspaceStateValue, updateWorkspaceStateValue } from '../common/persistentState';
2727

2828
export const NATIVE_REPL_URI_MEMENTO = 'nativeReplUri';
2929
let nativeRepl: NativeRepl | undefined;
@@ -43,17 +43,14 @@ export class NativeRepl implements Disposable {
4343

4444
public newReplSession: boolean | undefined = true;
4545

46-
private context: ExtensionContext;
47-
4846
// TODO: In the future, could also have attribute of URI for file specific REPL.
49-
private constructor(context: ExtensionContext) {
47+
private constructor() {
5048
this.watchNotebookClosed();
51-
this.context = context;
5249
}
5350

5451
// Static async factory method to handle asynchronous initialization
55-
public static async create(interpreter: PythonEnvironment, context: ExtensionContext): Promise<NativeRepl> {
56-
const nativeRepl = new NativeRepl(context);
52+
public static async create(interpreter: PythonEnvironment): Promise<NativeRepl> {
53+
const nativeRepl = new NativeRepl();
5754
nativeRepl.interpreter = interpreter;
5855
await nativeRepl.setReplDirectory();
5956
nativeRepl.pythonServer = createPythonServer([interpreter.path as string], nativeRepl.cwd);
@@ -76,7 +73,8 @@ export class NativeRepl implements Disposable {
7673
if (this.notebookDocument && nb.uri.toString() === this.notebookDocument.uri.toString()) {
7774
this.notebookDocument = undefined;
7875
this.newReplSession = true;
79-
await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined);
76+
// await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined);
77+
updateWorkspaceStateValue<string | undefined>(NATIVE_REPL_URI_MEMENTO, undefined);
8078
}
8179
}),
8280
);
@@ -154,38 +152,29 @@ export class NativeRepl implements Disposable {
154152
* Function that opens interactive repl, selects kernel, and send/execute code to the native repl.
155153
*/
156154
public async sendToNativeRepl(code?: string | undefined, preserveFocus: boolean = true): Promise<void> {
157-
const mementoValue = (await this.context.globalState.get(NATIVE_REPL_URI_MEMENTO)) as string | undefined;
158-
let mementoUri = mementoValue ? Uri.parse(mementoValue) : undefined;
159-
const openNotebookDocuments = workspace.notebookDocuments.map((doc) => doc.uri);
160-
161-
if (mementoUri) {
162-
const replTabBeforeReload = openNotebookDocuments.find((uri) => uri.fsPath === mementoUri?.fsPath);
163-
if (replTabBeforeReload) {
164-
this.notebookDocument = workspace.notebookDocuments.find(
165-
(doc) => doc.uri.fsPath === replTabBeforeReload.fsPath,
166-
);
167-
await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, replTabBeforeReload.toString());
168-
169-
// If repl URI does not have tabLabel 'Python REPL', something has changed:
170-
// e.g. creation of untitled notebook without Python extension knowing.
171-
if (getTabNameForUri(replTabBeforeReload) !== 'Python REPL') {
172-
mementoUri = undefined;
173-
await this.cleanRepl();
174-
}
155+
let wsMementoUri: Uri | undefined;
156+
157+
if (!this.notebookDocument) {
158+
const wsMemento = getWorkspaceStateValue<string>(NATIVE_REPL_URI_MEMENTO);
159+
wsMementoUri = wsMemento ? Uri.parse(wsMemento) : undefined;
160+
161+
if (!wsMementoUri || getTabNameForUri(wsMementoUri) !== 'Python REPL') {
162+
await this.cleanRepl();
163+
wsMementoUri = undefined;
164+
this.notebookDocument = undefined;
175165
}
176-
} else {
177-
await this.cleanRepl();
178166
}
179167

180168
const notebookEditor = await openInteractiveREPL(
181169
this.replController,
182170
this.notebookDocument,
183-
mementoUri,
171+
wsMementoUri,
184172
preserveFocus,
185173
);
186174

187175
this.notebookDocument = notebookEditor.notebook;
188-
await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString());
176+
// await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString());
177+
updateWorkspaceStateValue<string | undefined>(NATIVE_REPL_URI_MEMENTO, this.notebookDocument.uri.toString());
189178

190179
if (this.notebookDocument) {
191180
this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default);
@@ -202,7 +191,8 @@ export class NativeRepl implements Disposable {
202191
*/
203192
private async cleanRepl(): Promise<void> {
204193
this.notebookDocument = undefined;
205-
await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined);
194+
// await this.context.globalState.update(NATIVE_REPL_URI_MEMENTO, undefined);
195+
updateWorkspaceStateValue<string | undefined>(NATIVE_REPL_URI_MEMENTO, undefined);
206196
}
207197
}
208198

@@ -211,13 +201,9 @@ export class NativeRepl implements Disposable {
211201
* @param interpreter
212202
* @returns Native REPL instance
213203
*/
214-
export async function getNativeRepl(
215-
interpreter: PythonEnvironment,
216-
disposables: Disposable[],
217-
context: ExtensionContext,
218-
): Promise<NativeRepl> {
204+
export async function getNativeRepl(interpreter: PythonEnvironment, disposables: Disposable[]): Promise<NativeRepl> {
219205
if (!nativeRepl) {
220-
nativeRepl = await NativeRepl.create(interpreter, context);
206+
nativeRepl = await NativeRepl.create(interpreter);
221207
disposables.push(nativeRepl);
222208
}
223209
if (nativeRepl && nativeRepl.newReplSession) {

‎src/client/repl/replCommandHandler.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ export async function openInteractiveREPL(
2626
): Promise<NotebookEditor> {
2727
let viewColumn = ViewColumn.Beside;
2828
if (mementoValue) {
29-
// also check if memento value URI tab has file name of Python REPL
30-
// Cached NotebookDocument exists.
31-
notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri);
29+
if (!notebookDocument) {
30+
notebookDocument = await workspace.openNotebookDocument(mementoValue as Uri);
31+
}
3232
} else if (notebookDocument) {
3333
// Case where NotebookDocument (REPL document already exists in the tab)
3434
const existingReplViewColumn = getExistingReplViewColumn(notebookDocument);
@@ -43,6 +43,8 @@ export async function openInteractiveREPL(
4343
asRepl: 'Python REPL',
4444
preserveFocus,
4545
});
46+
// sanity check that we opened a Native REPL from showNotebookDocument.
47+
// if not true, set notebook = undefined.
4648
await commands.executeCommand('notebook.selectKernel', {
4749
editor,
4850
id: notebookController.id,

‎src/client/repl/replCommands.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { commands, ExtensionContext, Uri, window } from 'vscode';
1+
import { commands, Uri, window } from 'vscode';
22
import { Disposable } from 'vscode-jsonrpc';
33
import { ICommandManager } from '../common/application/types';
44
import { Commands } from '../common/constants';
@@ -25,15 +25,14 @@ import { ReplType } from './types';
2525
export async function registerStartNativeReplCommand(
2626
disposables: Disposable[],
2727
interpreterService: IInterpreterService,
28-
context: ExtensionContext,
2928
): Promise<void> {
3029
disposables.push(
3130
registerCommand(Commands.Start_Native_REPL, async (uri: Uri) => {
3231
sendTelemetryEvent(EventName.REPL, undefined, { replType: 'Native' });
3332
const interpreter = await getActiveInterpreter(uri, interpreterService);
3433
if (interpreter) {
3534
if (interpreter) {
36-
const nativeRepl = await getNativeRepl(interpreter, disposables, context);
35+
const nativeRepl = await getNativeRepl(interpreter, disposables);
3736
await nativeRepl.sendToNativeRepl(undefined, false);
3837
}
3938
}
@@ -49,7 +48,6 @@ export async function registerReplCommands(
4948
interpreterService: IInterpreterService,
5049
executionHelper: ICodeExecutionHelper,
5150
commandManager: ICommandManager,
52-
context: ExtensionContext,
5351
): Promise<void> {
5452
disposables.push(
5553
commandManager.registerCommand(Commands.Exec_In_REPL, async (uri: Uri) => {
@@ -62,7 +60,7 @@ export async function registerReplCommands(
6260
const interpreter = await getActiveInterpreter(uri, interpreterService);
6361

6462
if (interpreter) {
65-
const nativeRepl = await getNativeRepl(interpreter, disposables, context);
63+
const nativeRepl = await getNativeRepl(interpreter, disposables);
6664
const activeEditor = window.activeTextEditor;
6765
if (activeEditor) {
6866
const code = await getSelectedTextToExecute(activeEditor);
@@ -92,16 +90,15 @@ export async function registerReplExecuteOnEnter(
9290
disposables: Disposable[],
9391
interpreterService: IInterpreterService,
9492
commandManager: ICommandManager,
95-
context: ExtensionContext,
9693
): Promise<void> {
9794
disposables.push(
9895
commandManager.registerCommand(Commands.Exec_In_REPL_Enter, async (uri: Uri) => {
99-
await onInputEnter(uri, 'repl.execute', interpreterService, disposables, context);
96+
await onInputEnter(uri, 'repl.execute', interpreterService, disposables);
10097
}),
10198
);
10299
disposables.push(
103100
commandManager.registerCommand(Commands.Exec_In_IW_Enter, async (uri: Uri) => {
104-
await onInputEnter(uri, 'interactive.execute', interpreterService, disposables, context);
101+
await onInputEnter(uri, 'interactive.execute', interpreterService, disposables);
105102
}),
106103
);
107104
}
@@ -111,15 +108,14 @@ async function onInputEnter(
111108
commandName: string,
112109
interpreterService: IInterpreterService,
113110
disposables: Disposable[],
114-
context: ExtensionContext,
115111
): Promise<void> {
116112
const interpreter = await interpreterService.getActiveInterpreter(uri);
117113
if (!interpreter) {
118114
commands.executeCommand(Commands.TriggerEnvironmentSelection, uri).then(noop, noop);
119115
return;
120116
}
121117

122-
const nativeRepl = await getNativeRepl(interpreter, disposables, context);
118+
const nativeRepl = await getNativeRepl(interpreter, disposables);
123119
const completeCode = await nativeRepl?.checkUserInputCompleteCode(window.activeTextEditor);
124120
const editor = window.activeTextEditor;
125121

‎src/test/repl/nativeRepl.test.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ suite('REPL - Native REPL', () => {
5555
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
5656
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));
5757
const interpreter = await interpreterService.object.getActiveInterpreter();
58-
await getNativeRepl(interpreter as PythonEnvironment, disposableArray, extensionContext.object);
58+
await getNativeRepl(interpreter as PythonEnvironment, disposableArray);
5959

6060
expect(createMethodStub.calledOnce).to.be.true;
6161
});
@@ -67,11 +67,7 @@ suite('REPL - Native REPL', () => {
6767
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
6868
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));
6969
const interpreter = await interpreterService.object.getActiveInterpreter();
70-
const nativeRepl = await getNativeRepl(
71-
interpreter as PythonEnvironment,
72-
disposableArray,
73-
extensionContext.object,
74-
);
70+
const nativeRepl = await getNativeRepl(interpreter as PythonEnvironment, disposableArray);
7571

7672
nativeRepl.sendToNativeRepl(undefined, false);
7773

@@ -84,7 +80,7 @@ suite('REPL - Native REPL', () => {
8480
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
8581
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));
8682

87-
await NativeRepl.create(interpreter as PythonEnvironment, extensionContext.object);
83+
await NativeRepl.create(interpreter as PythonEnvironment);
8884

8985
expect(setReplDirectoryStub.calledOnce).to.be.true;
9086
expect(setReplControllerSpy.calledOnce).to.be.true;

‎src/test/repl/replCommand.test.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import * as replUtils from '../../client/repl/replUtils';
1111
import * as nativeRepl from '../../client/repl/nativeRepl';
1212
import { Commands } from '../../client/common/constants';
1313
import { PythonEnvironment } from '../../client/pythonEnvironments/info';
14-
import { IExtensionContext } from '../../client/common/types';
1514

1615
suite('REPL - register native repl command', () => {
1716
let interpreterService: TypeMoq.IMock<IInterpreterService>;
@@ -25,7 +24,7 @@ suite('REPL - register native repl command', () => {
2524
let getNativeReplStub: sinon.SinonStub;
2625
let disposable: TypeMoq.IMock<Disposable>;
2726
let disposableArray: Disposable[] = [];
28-
let extensionContext: TypeMoq.IMock<IExtensionContext>;
27+
2928
setup(() => {
3029
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
3130
commandManager = TypeMoq.Mock.ofType<ICommandManager>();
@@ -41,7 +40,6 @@ suite('REPL - register native repl command', () => {
4140
registerCommandSpy = sinon.spy(commandManager.object, 'registerCommand');
4241
disposable = TypeMoq.Mock.ofType<Disposable>();
4342
disposableArray = [disposable.object];
44-
extensionContext = TypeMoq.Mock.ofType<IExtensionContext>();
4543
});
4644

4745
teardown(() => {
@@ -65,7 +63,6 @@ suite('REPL - register native repl command', () => {
6563
interpreterService.object,
6664
executionHelper.object,
6765
commandManager.object,
68-
extensionContext.object,
6966
);
7067

7168
commandManager.verify(
@@ -96,7 +93,6 @@ suite('REPL - register native repl command', () => {
9693
interpreterService.object,
9794
executionHelper.object,
9895
commandManager.object,
99-
extensionContext.object,
10096
);
10197

10298
expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized');
@@ -129,7 +125,6 @@ suite('REPL - register native repl command', () => {
129125
interpreterService.object,
130126
executionHelper.object,
131127
commandManager.object,
132-
extensionContext.object,
133128
);
134129

135130
expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized');
@@ -163,7 +158,6 @@ suite('REPL - register native repl command', () => {
163158
interpreterService.object,
164159
executionHelper.object,
165160
commandManager.object,
166-
extensionContext.object,
167161
);
168162

169163
expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized');
@@ -201,7 +195,6 @@ suite('REPL - register native repl command', () => {
201195
interpreterService.object,
202196
executionHelper.object,
203197
commandManager.object,
204-
extensionContext.object,
205198
);
206199

207200
expect(commandHandler).not.to.be.an('undefined', 'Command handler not initialized');

0 commit comments

Comments
 (0)