Skip to content

Make shift+enter work in DS and non-DS scenarios #9445

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

Merged
merged 5 commits into from
Jan 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/9437.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Shift+Enter can no longer send multiple lines to the interactive window.
1 change: 1 addition & 0 deletions news/2 Fixes/9439.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Shift+Enter can no longer run code in the terminal.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Disposable, Uri } from 'vscode';
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
import '../../common/extensions';
import { IFileSystem, IPlatformService } from '../../common/platform/types';
import { IPythonExecutionFactory, PythonExecutionInfo } from '../../common/process/types';
import { PythonExecutionInfo } from '../../common/process/types';
import { ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { DjangoContextInitializer } from './djangoContext';
Expand All @@ -25,10 +25,9 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
@inject(IPlatformService) platformService: IPlatformService,
@inject(ICommandManager) commandManager: ICommandManager,
@inject(IFileSystem) fileSystem: IFileSystem,
@inject(IPythonExecutionFactory) pythonExecFactory: IPythonExecutionFactory,
@inject(IDisposableRegistry) disposableRegistry: Disposable[]
) {
super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService, pythonExecFactory);
super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService);
this.terminalTitle = 'Django Shell';
disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager));
}
Expand Down
17 changes: 11 additions & 6 deletions src/client/terminals/codeExecution/helper.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,30 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import '../../common/extensions';

import { inject, injectable } from 'inversify';
import * as path from 'path';
import { Range, TextEditor, Uri } from 'vscode';

import { IApplicationShell, IDocumentManager } from '../../common/application/types';
import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../common/constants';
import '../../common/extensions';
import { traceError } from '../../common/logger';
import { IPythonExecutionFactory } from '../../common/process/types';
import { IProcessServiceFactory } from '../../common/process/types';
import { IInterpreterService } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { ICodeExecutionHelper } from '../types';

@injectable()
export class CodeExecutionHelper implements ICodeExecutionHelper {
private readonly documentManager: IDocumentManager;
private readonly applicationShell: IApplicationShell;
private readonly pythonServiceFactory: IPythonExecutionFactory;
private readonly processServiceFactory: IProcessServiceFactory;
private readonly interpreterService: IInterpreterService;
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
this.documentManager = serviceContainer.get<IDocumentManager>(IDocumentManager);
this.applicationShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
this.pythonServiceFactory = serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory);
this.processServiceFactory = serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
}
public async normalizeLines(code: string, resource?: Uri): Promise<string> {
try {
Expand All @@ -30,9 +34,10 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
// On windows cr is not handled well by python when passing in/out via stdin/stdout.
// So just remove cr from the input.
code = code.replace(new RegExp('\\r', 'g'), '');
const interpreter = await this.interpreterService.getActiveInterpreter(resource);
const processService = await this.processServiceFactory.create(resource);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using this.interpreterService.getActiveInterpreter(resource); instead of reverting to the previous code that used an IConfigurationService instance to get the python path (61a6282#diff-50459a25c7de099fb1ddc1602ce70271)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getActiveIntepreter should be more up to date than the configuration service AFAIK. @DonJayamanne what do you think?

const args = [path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'normalizeForInterpreter.py'), code];
const processService = await this.pythonServiceFactory.create({ resource });
const proc = await processService.exec(args, { throwOnStdErr: true });
const proc = await processService.exec(interpreter?.path || 'python', args, { throwOnStdErr: true });

return proc.stdout;
} catch (ex) {
Expand Down
4 changes: 1 addition & 3 deletions src/client/terminals/codeExecution/repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { inject, injectable } from 'inversify';
import { Disposable } from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import { IPlatformService } from '../../common/platform/types';
import { IPythonExecutionFactory } from '../../common/process/types';
import { ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { TerminalCodeExecutionProvider } from './terminalCodeExecution';
Expand All @@ -18,11 +17,10 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
@inject(ITerminalServiceFactory) terminalServiceFactory: ITerminalServiceFactory,
@inject(IConfigurationService) configurationService: IConfigurationService,
@inject(IWorkspaceService) workspace: IWorkspaceService,
@inject(IPythonExecutionFactory) pythonExecFactory: IPythonExecutionFactory,
@inject(IDisposableRegistry) disposableRegistry: Disposable[],
@inject(IPlatformService) platformService: IPlatformService
) {
super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService, pythonExecFactory);
super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService);
this.terminalTitle = 'REPL';
}
}
10 changes: 2 additions & 8 deletions src/client/terminals/codeExecution/terminalCodeExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Disposable, Uri } from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import '../../common/extensions';
import { IPlatformService } from '../../common/platform/types';
import { IPythonExecutionFactory, PythonExecutionInfo } from '../../common/process/types';
import { PythonExecutionInfo } from '../../common/process/types';
import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { ICodeExecutionService } from '../../terminals/types';
Expand All @@ -24,8 +24,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
@inject(IConfigurationService) protected readonly configurationService: IConfigurationService,
@inject(IWorkspaceService) protected readonly workspace: IWorkspaceService,
@inject(IDisposableRegistry) protected readonly disposables: Disposable[],
@inject(IPlatformService) protected readonly platformService: IPlatformService,
@inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory
@inject(IPlatformService) protected readonly platformService: IPlatformService
) {}

public async executeFile(file: Uri) {
Expand Down Expand Up @@ -64,11 +63,6 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
const command = pythonSettings.pythonPath;
const launchArgs = pythonSettings.terminal.launchArgs;

const condaExecutionService = await this.pythonExecFactory.createCondaExecutionService(command, undefined, resource);
if (condaExecutionService) {
return condaExecutionService.getExecutionInfo([...launchArgs, ...args]);
}

const isWindows = this.platformService.isWindows;

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ suite('Terminal - Django Shell Code Execution', () => {
platform.object,
commandManager.object,
fileSystem.object,
pythonExecutionFactory.object,
disposables
);

Expand Down Expand Up @@ -186,18 +185,16 @@ suite('Terminal - Django Shell Code Execution', () => {
const serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
const processService = TypeMoq.Mock.ofType<IProcessService>();
const condaExecutionService = new CondaExecutionService(serviceContainer.object, processService.object, pythonPath, condaFile, condaEnv);
const hasEnvName = condaEnv.name !== '';
const condaArgs = ['run', ...(hasEnvName ? ['-n', condaEnv.name] : ['-p', condaEnv.path]), 'python'];
const expectedTerminalArgs = [...condaArgs, ...terminalArgs, 'manage.py', 'shell'];
const expectedTerminalArgs = [...terminalArgs, 'manage.py', 'shell'];
pythonExecutionFactory
.setup(p => p.createCondaExecutionService(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns(() => Promise.resolve(condaExecutionService));

const replCommandArgs = await (executor as DjangoShellCodeExecutionProvider).getExecutableInfo(resource);

expect(replCommandArgs).not.to.be.an('undefined', 'Conda command args are undefined');
expect(replCommandArgs.command).to.be.equal(condaFile, 'Incorrect conda path');
expect(replCommandArgs.args).to.be.deep.equal(expectedTerminalArgs, 'Incorrect conda arguments');
expect(replCommandArgs.command).to.be.equal(pythonPath, 'Repl should use python not conda');
expect(replCommandArgs.args).to.be.deep.equal(expectedTerminalArgs, 'Incorrect terminal arguments');
}

test('Ensure conda args including env name are passed when using a conda environment with a name', async () => {
Expand Down
46 changes: 31 additions & 15 deletions src/test/terminals/codeExecution/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@ import { expect } from 'chai';
import * as fs from 'fs-extra';
import { EOL } from 'os';
import * as path from 'path';
import { SemVer } from 'semver';
import * as TypeMoq from 'typemoq';
import { Range, Selection, TextDocument, TextEditor, TextLine, Uri } from 'vscode';
import { IApplicationShell, IDocumentManager } from '../../../client/common/application/types';
import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../../client/common/constants';
import '../../../client/common/extensions';
import { BufferDecoder } from '../../../client/common/process/decoder';
import { ProcessService } from '../../../client/common/process/proc';
import { IPythonExecutionFactory, IPythonExecutionService } from '../../../client/common/process/types';
import { OSType } from '../../../client/common/utils/platform';
import { IProcessService, IProcessServiceFactory } from '../../../client/common/process/types';
import { Architecture, OSType } from '../../../client/common/utils/platform';
import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types';
import { IInterpreterService, InterpreterType, PythonInterpreter } from '../../../client/interpreter/contracts';
import { IServiceContainer } from '../../../client/ioc/types';
import { CodeExecutionHelper } from '../../../client/terminals/codeExecution/helper';
import { ICodeExecutionHelper } from '../../../client/terminals/types';
Expand All @@ -31,19 +33,33 @@ suite('Terminal - Code Execution Helper', () => {
let helper: ICodeExecutionHelper;
let document: TypeMoq.IMock<TextDocument>;
let editor: TypeMoq.IMock<TextEditor>;
let pythonService: TypeMoq.IMock<IPythonExecutionService>;
let processService: TypeMoq.IMock<IProcessService>;
let interpreterService: TypeMoq.IMock<IInterpreterService>;
const workingPython: PythonInterpreter = {
path: PYTHON_PATH,
version: new SemVer('3.6.6-final'),
sysVersion: '1.0.0.0',
sysPrefix: 'Python',
displayName: 'Python',
type: InterpreterType.Unknown,
architecture: Architecture.x64
};

setup(() => {
const serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
documentManager = TypeMoq.Mock.ofType<IDocumentManager>();
applicationShell = TypeMoq.Mock.ofType<IApplicationShell>();
const envVariablesProvider = TypeMoq.Mock.ofType<IEnvironmentVariablesProvider>();
pythonService = TypeMoq.Mock.ofType<IPythonExecutionService>();
processService = TypeMoq.Mock.ofType<IProcessService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
// tslint:disable-next-line:no-any
pythonService.setup((x: any) => x.then).returns(() => undefined);
processService.setup((x: any) => x.then).returns(() => undefined);
interpreterService.setup(i => i.getActiveInterpreter(TypeMoq.It.isAny())).returns(() => Promise.resolve(workingPython));
const processServiceFactory = TypeMoq.Mock.ofType<IProcessServiceFactory>();
processServiceFactory.setup(p => p.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(processService.object));
envVariablesProvider.setup(e => e.getEnvironmentVariables(TypeMoq.It.isAny())).returns(() => Promise.resolve({}));
const pythonExecFactory = TypeMoq.Mock.ofType<IPythonExecutionFactory>();
pythonExecFactory.setup(p => p.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(pythonService.object));
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonExecutionFactory), TypeMoq.It.isAny())).returns(() => pythonExecFactory.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IProcessServiceFactory), TypeMoq.It.isAny())).returns(() => processServiceFactory.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IInterpreterService), TypeMoq.It.isAny())).returns(() => interpreterService.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDocumentManager), TypeMoq.It.isAny())).returns(() => documentManager.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IApplicationShell), TypeMoq.It.isAny())).returns(() => applicationShell.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IEnvironmentVariablesProvider), TypeMoq.It.isAny())).returns(() => envVariablesProvider.object);
Expand All @@ -56,10 +72,10 @@ suite('Terminal - Code Execution Helper', () => {

async function ensureBlankLinesAreRemoved(source: string, expectedSource: string) {
const actualProcessService = new ProcessService(new BufferDecoder());
pythonService
.setup(p => p.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns((args, options) => {
return actualProcessService.exec.apply(actualProcessService, [PYTHON_PATH, args, options]);
processService
.setup(p => p.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns((file, args, options) => {
return actualProcessService.exec.apply(actualProcessService, [file, args, options]);
});
const normalizedZCode = await helper.normalizeLines(source);
// In case file has been saved with different line endings.
Expand All @@ -81,9 +97,9 @@ suite('Terminal - Code Execution Helper', () => {
test('Ensure there are no multiple-CR elements in the normalized code.', async () => {
const code = ['import sys', '', '', '', 'print(sys.executable)', '', 'print("1234")', '', '', 'print(1)', 'print(2)'];
const actualProcessService = new ProcessService(new BufferDecoder());
pythonService
.setup(p => p.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns((args, options) => {
processService
.setup(p => p.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns((_file, args, options) => {
return actualProcessService.exec.apply(actualProcessService, [PYTHON_PATH, args, options]);
});
const normalizedCode = await helper.normalizeLines(code.join(EOL));
Expand Down
Loading