Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow native REPL launch from command palette & set cwd #23912

Merged
merged 14 commits into from
Aug 7, 2024
Merged
11 changes: 0 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,12 @@
{
"category": "Python",
"command": "python.startREPL",
"title": "%python.command.python.startREPL.title%"
"title": "%python.command.python.startTerminalREPL.title%"
},
{
"category": "Python",
"command": "python.startNativeREPL",
"title": "%python.command.python.startNativeREPL.title%"
},
{
"category": "Python",
Expand Down Expand Up @@ -1328,7 +1333,13 @@
{
"category": "Python",
"command": "python.startREPL",
"title": "%python.command.python.startREPL.title%",
"title": "%python.command.python.startTerminalREPL.title%",
"when": "!virtualWorkspace && shellExecutionSupported"
},
{
"category": "Python",
"command": "python.startNativeREPL",
"title": "%python.command.python.startNativeREPL.title%",
"when": "!virtualWorkspace && shellExecutionSupported"
},
{
Expand Down
3 changes: 2 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"python.command.python.startREPL.title": "Start Terminal REPL",
"python.command.python.startTerminalREPL.title": "Start Terminal REPL",
"python.command.python.startNativeREPL.title": "Start Native Python REPL",
anthonykim1 marked this conversation as resolved.
Show resolved Hide resolved
"python.command.python.createEnvironment.title": "Create Environment...",
"python.command.python.createNewFile.title": "New Python File",
"python.command.python.createTerminal.title": "Create Terminal",
Expand Down
1 change: 1 addition & 0 deletions src/client/common/application/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
['workbench.action.openIssueReporter']: [{ extensionId: string; issueBody: string }];
[Commands.GetSelectedInterpreterPath]: [{ workspaceFolder: string } | string[]];
[Commands.TriggerEnvironmentSelection]: [undefined | Uri];
[Commands.Start_Native_REPL]: [undefined | Uri];
[Commands.Exec_In_REPL]: [undefined | Uri];
[Commands.Exec_In_REPL_Enter]: [undefined | Uri];
[Commands.Exec_In_Terminal]: [undefined, Uri];
Expand Down
1 change: 1 addition & 0 deletions src/client/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export namespace Commands {
export const Set_Interpreter = 'python.setInterpreter';
export const Set_ShebangInterpreter = 'python.setShebangInterpreter';
export const Start_REPL = 'python.startREPL';
export const Start_Native_REPL = 'python.startNativeREPL';
export const Tests_Configure = 'python.configureTests';
export const TriggerEnvironmentSelection = 'python.triggerEnvSelection';
export const ViewOutput = 'python.viewOutput';
Expand Down
3 changes: 2 additions & 1 deletion src/client/extensionActivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import { initializePersistentStateForTriggers } from './common/persistentState';
import { logAndNotifyOnLegacySettings } from './logging/settingLogs';
import { DebuggerTypeName } from './debugger/constants';
import { StopWatch } from './common/utils/stopWatch';
import { registerReplCommands, registerReplExecuteOnEnter } from './repl/replCommands';
import { registerReplCommands, registerReplExecuteOnEnter, registerStartNativeReplCommand } from './repl/replCommands';

export async function activateComponents(
// `ext` is passed to any extra activation funcs.
Expand Down Expand Up @@ -108,6 +108,7 @@ export function activateFeatures(ext: ExtensionState, _components: Components):
);
const executionHelper = ext.legacyIOC.serviceContainer.get<ICodeExecutionHelper>(ICodeExecutionHelper);
const commandManager = ext.legacyIOC.serviceContainer.get<ICommandManager>(ICommandManager);
registerStartNativeReplCommand(ext.disposables, interpreterService);
registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager);
registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager);
}
Expand Down
82 changes: 68 additions & 14 deletions src/client/repl/nativeRepl.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,51 @@
// Native Repl class that holds instance of pythonServer and replController

import { NotebookController, NotebookControllerAffinity, NotebookDocument, TextEditor, workspace } from 'vscode';
import {
NotebookController,
NotebookControllerAffinity,
NotebookDocument,
QuickPickItem,
TextEditor,
workspace,
WorkspaceFolder,
} from 'vscode';
import { Disposable } from 'vscode-jsonrpc';
import { PVSC_EXTENSION_ID } from '../common/constants';
import { showQuickPick } from '../common/vscodeApis/windowApis';
import { getWorkspaceFolders } from '../common/vscodeApis/workspaceApis';
import { PythonEnvironment } from '../pythonEnvironments/info';
import { createPythonServer, PythonServer } from './pythonServer';
import { executeNotebookCell, openInteractiveREPL, selectNotebookKernel } from './replCommandHandler';
import { createReplController } from './replController';

export class NativeRepl implements Disposable {
private pythonServer: PythonServer;
// Adding ! since it will get initialized in create method, not the constructor.
private pythonServer!: PythonServer;

private interpreter: PythonEnvironment;
private cwd: string | undefined;

private interpreter!: PythonEnvironment;

private disposables: Disposable[] = [];

private replController: NotebookController;
private replController!: NotebookController;

private notebookDocument: NotebookDocument | undefined;

// TODO: In the future, could also have attribute of URI for file specific REPL.
constructor(interpreter: PythonEnvironment) {
this.interpreter = interpreter;
private constructor() {
this.watchNotebookClosed();
}

this.pythonServer = createPythonServer([interpreter.path as string]);
this.replController = this.setReplController();
// Static async factory method to handle asynchronous initialization
public static async create(interpreter: PythonEnvironment): Promise<NativeRepl> {
const nativeRepl = new NativeRepl();
nativeRepl.interpreter = interpreter;
await nativeRepl.setReplDirectory();
nativeRepl.pythonServer = createPythonServer([interpreter.path as string], nativeRepl.cwd);
nativeRepl.replController = nativeRepl.setReplController();

this.watchNotebookClosed();
return nativeRepl;
}

dispose(): void {
Expand All @@ -47,13 +66,46 @@ export class NativeRepl implements Disposable {
);
}

/**
* Function that set up desired directory for REPL.
* If there is multiple workspaces, prompt the user to choose
* which directory we should set in context of native REPL.
*/
private async setReplDirectory(): Promise<void> {
// Figure out uri via workspaceFolder as uri parameter always
// seem to be undefined from parameter when trying to access from replCommands.ts
const workspaces: readonly WorkspaceFolder[] | undefined = getWorkspaceFolders();

if (workspaces) {
// eslint-disable-next-line no-shadow
const workspacesQuickPickItems: QuickPickItem[] = workspaces.map((workspace) => ({
label: workspace.name,
description: workspace.uri.fsPath,
}));

if (workspacesQuickPickItems.length === 0) {
this.cwd = process.cwd(); // Yields '/' on no workspace scenario.
} else if (workspacesQuickPickItems.length === 1) {
this.cwd = workspacesQuickPickItems[0].description;
} else {
// Show choices of workspaces for user to choose from.
const selection = (await showQuickPick(workspacesQuickPickItems, {
placeHolder: 'Select current working directory for new REPL',
matchOnDescription: true,
ignoreFocusOut: true,
})) as QuickPickItem;
this.cwd = selection?.description;
}
}
}

/**
* Function that check if NotebookController for REPL exists, and returns it in Singleton manner.
* @returns NotebookController
*/
public setReplController(): NotebookController {
if (!this.replController) {
return createReplController(this.interpreter.path, this.disposables);
return createReplController(this.interpreter!.path, this.disposables, this.cwd);
}
return this.replController;
}
Expand Down Expand Up @@ -84,14 +136,16 @@ export class NativeRepl implements Disposable {
* Function that opens interactive repl, selects kernel, and send/execute code to the native repl.
* @param code
*/
public async sendToNativeRepl(code: string): Promise<void> {
public async sendToNativeRepl(code?: string): Promise<void> {
const notebookEditor = await openInteractiveREPL(this.replController, this.notebookDocument);
this.notebookDocument = notebookEditor.notebook;

if (this.notebookDocument) {
this.replController.updateNotebookAffinity(this.notebookDocument, NotebookControllerAffinity.Default);
await selectNotebookKernel(notebookEditor, this.replController.id, PVSC_EXTENSION_ID);
await executeNotebookCell(this.notebookDocument, code);
if (code) {
await executeNotebookCell(this.notebookDocument, code);
}
}
}
}
Expand All @@ -103,9 +157,9 @@ let nativeRepl: NativeRepl | undefined; // In multi REPL scenario, hashmap of UR
* @param interpreter
* @returns Native REPL instance
*/
export function getNativeRepl(interpreter: PythonEnvironment, disposables: Disposable[]): NativeRepl {
export async function getNativeRepl(interpreter: PythonEnvironment, disposables: Disposable[]): Promise<NativeRepl> {
if (!nativeRepl) {
nativeRepl = new NativeRepl(interpreter);
nativeRepl = await NativeRepl.create(interpreter);
disposables.push(nativeRepl);
}
return nativeRepl;
Expand Down
6 changes: 4 additions & 2 deletions src/client/repl/pythonServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,14 @@ class PythonServerImpl implements Disposable {
}
}

export function createPythonServer(interpreter: string[]): PythonServer {
export function createPythonServer(interpreter: string[], cwd?: string): PythonServer {
if (serverInstance) {
return serverInstance;
}

const pythonServer = ch.spawn(interpreter[0], [...interpreter.slice(1), SERVER_PATH]);
const pythonServer = ch.spawn(interpreter[0], [...interpreter.slice(1), SERVER_PATH], {
cwd, // Launch with correct workspace directory
});

pythonServer.stderr.on('data', (data) => {
traceError(data.toString());
Expand Down
30 changes: 28 additions & 2 deletions src/client/repl/replCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,32 @@ import {
insertNewLineToREPLInput,
isMultiLineText,
} from './replUtils';
import { registerCommand } from '../common/vscodeApis/commandApis';

/**
* Register Start Native REPL command in the command palette
*
* @param disposables
* @param interpreterService
* @param commandManager
* @returns Promise<void>
*/
export async function registerStartNativeReplCommand(
disposables: Disposable[],
interpreterService: IInterpreterService,
): Promise<void> {
disposables.push(
registerCommand(Commands.Start_Native_REPL, async (uri: Uri) => {
const interpreter = await getActiveInterpreter(uri, interpreterService);
if (interpreter) {
if (interpreter) {
const nativeRepl = await getNativeRepl(interpreter, disposables);
await nativeRepl.sendToNativeRepl();
}
}
}),
);
}

/**
* Registers REPL command for shift+enter if sendToNativeREPL setting is enabled.
Expand All @@ -39,7 +65,7 @@ export async function registerReplCommands(
const interpreter = await getActiveInterpreter(uri, interpreterService);

if (interpreter) {
const nativeRepl = getNativeRepl(interpreter, disposables);
const nativeRepl = await getNativeRepl(interpreter, disposables);
const activeEditor = window.activeTextEditor;
if (activeEditor) {
const code = await getSelectedTextToExecute(activeEditor);
Expand Down Expand Up @@ -76,7 +102,7 @@ export async function registerReplExecuteOnEnter(
return;
}

const nativeRepl = getNativeRepl(interpreter, disposables);
const nativeRepl = await getNativeRepl(interpreter, disposables);
const completeCode = await nativeRepl?.checkUserInputCompleteCode(window.activeTextEditor);
const editor = window.activeTextEditor;

Expand Down
3 changes: 2 additions & 1 deletion src/client/repl/replController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import { createPythonServer } from './pythonServer';
export function createReplController(
interpreterPath: string,
disposables: vscode.Disposable[],
cwd?: string,
): vscode.NotebookController {
const server = createPythonServer([interpreterPath]);
const server = createPythonServer([interpreterPath], cwd);
disposables.push(server);

const controller = vscode.notebooks.createNotebookController('pythonREPL', 'interactive', 'Python REPL');
Expand Down
70 changes: 70 additions & 0 deletions src/test/repl/nativeRepl.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +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';

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<IInterpreterService>;

let disposable: TypeMoq.IMock<Disposable>;
let disposableArray: Disposable[] = [];

let setReplDirectoryStub: sinon.SinonStub;
let setReplControllerSpy: sinon.SinonSpy;

setup(() => {
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
interpreterService
.setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny()))
.returns(() => Promise.resolve(({ path: 'ps' } as unknown) as PythonEnvironment));
disposable = TypeMoq.Mock.ofType<Disposable>();
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');
});

teardown(() => {
disposableArray.forEach((d) => {
if (d) {
d.dispose();
}
});

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);

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));

await NativeRepl.create(interpreter as PythonEnvironment);

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

setReplDirectoryStub.restore();
setReplControllerSpy.restore();
});
});
Loading