Skip to content

Use options object in terminal methods #9739

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 6 commits into from
Jan 23, 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
14 changes: 7 additions & 7 deletions src/client/common/terminal/activator/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@

'use strict';

import { Terminal, Uri } from 'vscode';
import { Terminal } from 'vscode';
import { createDeferred, sleep } from '../../utils/async';
import { ITerminalActivator, ITerminalHelper, TerminalShellType } from '../types';
import { ITerminalActivator, ITerminalHelper, TerminalActivationOptions, TerminalShellType } from '../types';

export class BaseTerminalActivator implements ITerminalActivator {
private readonly activatedTerminals: Map<Terminal, Promise<boolean>> = new Map<Terminal, Promise<boolean>>();
constructor(private readonly helper: ITerminalHelper) {}
public async activateEnvironmentInTerminal(terminal: Terminal, resource: Uri | undefined, preserveFocus: boolean = true) {
public async activateEnvironmentInTerminal(terminal: Terminal, options?: TerminalActivationOptions): Promise<boolean> {
if (this.activatedTerminals.has(terminal)) {
return this.activatedTerminals.get(terminal)!;
}
const deferred = createDeferred<boolean>();
this.activatedTerminals.set(terminal, deferred.promise);
const terminalShellType = this.helper.identifyTerminalShell(terminal);

const activationCommamnds = await this.helper.getEnvironmentActivationCommands(terminalShellType, resource);
const activationCommands = await this.helper.getEnvironmentActivationCommands(terminalShellType, options?.resource, options?.interpreter);
let activated = false;
if (activationCommamnds) {
for (const command of activationCommamnds!) {
terminal.show(preserveFocus);
if (activationCommands) {
for (const command of activationCommands!) {
terminal.show(options?.preserveFocus);
terminal.sendText(command);
await this.waitForCommandToProcess(terminalShellType);
activated = true;
Expand Down
10 changes: 5 additions & 5 deletions src/client/common/terminal/activator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
'use strict';

import { inject, injectable, multiInject } from 'inversify';
import { Terminal, Uri } from 'vscode';
import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper } from '../types';
import { Terminal } from 'vscode';
import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, TerminalActivationOptions } from '../types';
import { BaseTerminalActivator } from './base';

@injectable()
Expand All @@ -14,9 +14,9 @@ export class TerminalActivator implements ITerminalActivator {
constructor(@inject(ITerminalHelper) readonly helper: ITerminalHelper, @multiInject(ITerminalActivationHandler) private readonly handlers: ITerminalActivationHandler[]) {
this.initialize();
}
public async activateEnvironmentInTerminal(terminal: Terminal, resource: Uri | undefined, preserveFocus: boolean = true) {
const activated = await this.baseActivator.activateEnvironmentInTerminal(terminal, resource, preserveFocus);
this.handlers.forEach(handler => handler.handleActivation(terminal, resource, preserveFocus, activated).ignoreErrors());
public async activateEnvironmentInTerminal(terminal: Terminal, options?: TerminalActivationOptions): Promise<boolean> {
const activated = await this.baseActivator.activateEnvironmentInTerminal(terminal, options);
this.handlers.forEach(handler => handler.handleActivation(terminal, options?.resource, options?.preserveFocus === true, activated).ignoreErrors());
return activated;
}
protected initialize() {
Expand Down
38 changes: 26 additions & 12 deletions src/client/common/terminal/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@

import { inject, injectable } from 'inversify';
import { Uri } from 'vscode';
import { IInterpreterService } from '../../interpreter/contracts';
import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { IWorkspaceService } from '../application/types';
import { IFileSystem } from '../platform/types';
import { isUri } from '../utils/misc';
import { TerminalService } from './service';
import { SynchronousTerminalService } from './syncTerminalService';
import { ITerminalService, ITerminalServiceFactory } from './types';
import { ITerminalService, ITerminalServiceFactory, TerminalCreationOptions } from './types';

@injectable()
export class TerminalServiceFactory implements ITerminalServiceFactory {
Expand All @@ -22,26 +23,39 @@ export class TerminalServiceFactory implements ITerminalServiceFactory {
) {
this.terminalServices = new Map<string, TerminalService>();
}
public getTerminalService(resource?: Uri, title?: string): ITerminalService {
public getTerminalService(options?: TerminalCreationOptions): ITerminalService;
public getTerminalService(resource?: Uri, title?: string): ITerminalService;
public getTerminalService(arg1?: Uri | TerminalCreationOptions, arg2?: string): ITerminalService {
const resource = isUri(arg1) ? arg1 : undefined;
const title = isUri(arg1) ? undefined : arg1?.title || arg2;
const terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
const id = this.getTerminalId(terminalTitle, resource);
const interpreter = isUri(arg1) ? undefined : arg1?.interpreter;
const env = isUri(arg1) ? undefined : arg1?.env;

const options: TerminalCreationOptions = {
env,
interpreter,
resource,
title
};
const id = this.getTerminalId(terminalTitle, resource, interpreter);
if (!this.terminalServices.has(id)) {
const terminalService = new TerminalService(this.serviceContainer, resource, terminalTitle);
const terminalService = new TerminalService(this.serviceContainer, options);
this.terminalServices.set(id, terminalService);
}

// Decorate terminal service with the synchronous service.
return new SynchronousTerminalService(this.fs, this.interpreterService, this.terminalServices.get(id)!);
return new SynchronousTerminalService(this.fs, this.interpreterService, this.terminalServices.get(id)!, interpreter);
}
public createTerminalService(resource?: Uri, title?: string): ITerminalService {
const terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
return new TerminalService(this.serviceContainer, resource, terminalTitle);
title = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
return new TerminalService(this.serviceContainer, { resource, title });
}
private getTerminalId(title: string, resource?: Uri): string {
if (!resource) {
private getTerminalId(title: string, resource?: Uri, interpreter?: PythonInterpreter): string {
if (!resource && !interpreter) {
return title;
}
const workspaceFolder = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService).getWorkspaceFolder(resource!);
return workspaceFolder ? `${title}:${workspaceFolder.uri.fsPath}` : title;
const workspaceFolder = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService).getWorkspaceFolder(resource || undefined);
return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}`;
}
}
4 changes: 2 additions & 2 deletions src/client/common/terminal/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export class TerminalHelper implements ITerminalHelper {
const commandPrefix = isPowershell ? '& ' : '';
return `${commandPrefix}${command.fileToCommandArgument()} ${args.join(' ')}`.trim();
}
public async getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise<string[] | undefined> {
public async getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri, interpreter?: PythonInterpreter): Promise<string[] | undefined> {
const providers = [this.pipenv, this.pyenv, this.bashCShellFish, this.commandPromptAndPowerShell];
const promise = this.getActivationCommands(resource || undefined, undefined, terminalShellType, providers);
const promise = this.getActivationCommands(resource || undefined, interpreter, terminalShellType, providers);
this.sendTelemetry(resource, terminalShellType, EventName.PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL, promise).ignoreErrors();
return promise;
}
Expand Down
18 changes: 11 additions & 7 deletions src/client/common/terminal/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { CancellationToken, Disposable, Event, EventEmitter, Terminal, Uri } from 'vscode';
import { CancellationToken, Disposable, Event, EventEmitter, Terminal } from 'vscode';
import '../../common/extensions';
import { IInterpreterService } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { captureTelemetry } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { ITerminalManager } from '../application/types';
import { IConfigurationService, IDisposableRegistry } from '../types';
import { ITerminalActivator, ITerminalHelper, ITerminalService, TerminalShellType } from './types';
import { ITerminalActivator, ITerminalHelper, ITerminalService, TerminalCreationOptions, TerminalShellType } from './types';

@injectable()
export class TerminalService implements ITerminalService, Disposable {
Expand All @@ -23,7 +23,7 @@ export class TerminalService implements ITerminalService, Disposable {
public get onDidCloseTerminal(): Event<void> {
return this.terminalClosed.event.bind(this.terminalClosed);
}
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, private resource?: Uri, private title: string = 'Python') {
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, private readonly options?: TerminalCreationOptions) {
const disposableRegistry = this.serviceContainer.get<Disposable[]>(IDisposableRegistry);
disposableRegistry.push(this);
this.terminalHelper = this.serviceContainer.get<ITerminalHelper>(ITerminalHelper);
Expand Down Expand Up @@ -56,12 +56,16 @@ export class TerminalService implements ITerminalService, Disposable {
return;
}
this.terminalShellType = this.terminalHelper.identifyTerminalShell(this.terminal);
this.terminal = this.terminalManager.createTerminal({ name: this.title });
this.terminal = this.terminalManager.createTerminal({ name: this.options?.title || 'Python', env: this.options?.env });

// Sometimes the terminal takes some time to start up before it can start accepting input.
await new Promise(resolve => setTimeout(resolve, 100));

await this.terminalActivator.activateEnvironmentInTerminal(this.terminal!, this.resource, preserveFocus);
await this.terminalActivator.activateEnvironmentInTerminal(this.terminal!, {
resource: this.options?.resource,
preserveFocus,
interpreter: this.options?.interpreter
});

this.terminal!.show(preserveFocus);

Expand All @@ -75,8 +79,8 @@ export class TerminalService implements ITerminalService, Disposable {
}

private async sendTelemetry() {
const pythonPath = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(this.resource).pythonPath;
const interpreterInfo = await this.serviceContainer.get<IInterpreterService>(IInterpreterService).getInterpreterDetails(pythonPath);
const pythonPath = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(this.options?.resource).pythonPath;
const interpreterInfo = this.options?.interpreter || (await this.serviceContainer.get<IInterpreterService>(IInterpreterService).getInterpreterDetails(pythonPath));
const pythonVersion = interpreterInfo && interpreterInfo.version ? interpreterInfo.version.raw : undefined;
const interpreterType = interpreterInfo ? interpreterInfo.type : undefined;
captureTelemetry(EventName.TERMINAL_CREATE, { terminal: this.terminalShellType, pythonVersion, interpreterType });
Expand Down
7 changes: 4 additions & 3 deletions src/client/common/terminal/syncTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { inject } from 'inversify';
import * as path from 'path';
import { CancellationToken, Disposable, Event } from 'vscode';
import { EXTENSION_ROOT_DIR } from '../../constants';
import { IInterpreterService } from '../../interpreter/contracts';
import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
import { Cancellation } from '../cancellation';
import { traceVerbose } from '../logger';
import { IFileSystem, TemporaryFile } from '../platform/types';
Expand Down Expand Up @@ -97,7 +97,8 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
constructor(
@inject(IFileSystem) private readonly fs: IFileSystem,
@inject(IInterpreterService) private readonly interpreter: IInterpreterService,
public readonly terminalService: TerminalService
public readonly terminalService: TerminalService,
private readonly pythonInterpreter?: PythonInterpreter
) {}
public dispose() {
this.terminalService.dispose();
Expand All @@ -121,7 +122,7 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
const lockFile = await this.createLockFile();
const state = new ExecutionState(lockFile.filePath, this.fs, [command, ...args]);
try {
const pythonExec = await this.interpreter.getActiveInterpreter(undefined);
const pythonExec = this.pythonInterpreter || (await this.interpreter.getActiveInterpreter(undefined));
await this.terminalService.sendCommand(pythonExec?.path || 'python', [
shellExecFile.fileToCommandArgument(),
command.fileToCommandArgument(),
Expand Down
47 changes: 43 additions & 4 deletions src/client/common/terminal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { CancellationToken, Event, Terminal, Uri } from 'vscode';
import { PythonInterpreter } from '../../interpreter/contracts';
import { IEventNamePropertyMapping } from '../../telemetry/index';
import { Resource } from '../types';
import { IDisposable, Resource } from '../types';

export enum TerminalActivationProviders {
bashCShellFish = 'bashCShellFish',
Expand All @@ -31,7 +31,7 @@ export enum TerminalShellType {
other = 'other'
}

export interface ITerminalService {
export interface ITerminalService extends IDisposable {
readonly onDidCloseTerminal: Event<void>;
sendCommand(command: string, args: string[], cancel?: CancellationToken): Promise<void>;
sendText(text: string): Promise<void>;
Expand All @@ -40,6 +40,31 @@ export interface ITerminalService {

export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory');

export type TerminalCreationOptions = {
/**
* Object with environment variables that will be added to the Terminal.
*/
env?: { [key: string]: string | null };
/**
* Resource identifier. E.g. used to determine python interpreter that needs to be used or environment variables or the like.
*
* @type {Uri}
*/
resource?: Uri;
/**
* Title.
*
* @type {string}
*/
title?: string;
/**
* Associated Python Interpreter.
*
* @type {PythonInterpreter}
*/
interpreter?: PythonInterpreter;
};

export interface ITerminalServiceFactory {
/**
* Gets a terminal service with a specific title.
Expand All @@ -50,6 +75,15 @@ export interface ITerminalServiceFactory {
* @memberof ITerminalServiceFactory
*/
getTerminalService(resource?: Uri, title?: string): ITerminalService;
/**
* Gets a terminal service.
* If one exists with the same information, that is returned else a new one is created.
*
* @param {TerminalCreationOptions} [options]
* @returns {ITerminalService}
* @memberof ITerminalServiceFactory
*/
getTerminalService(options?: TerminalCreationOptions): ITerminalService;
createTerminalService(resource?: Uri, title?: string): ITerminalService;
}

Expand All @@ -59,13 +93,18 @@ export interface ITerminalHelper {
createTerminal(title?: string): Terminal;
identifyTerminalShell(terminal?: Terminal): TerminalShellType;
buildCommandForTerminal(terminalShellType: TerminalShellType, command: string, args: string[]): string;
getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise<string[] | undefined>;
getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri, interpreter?: PythonInterpreter): Promise<string[] | undefined>;
getEnvironmentActivationShellCommands(resource: Resource, shell: TerminalShellType, interpreter?: PythonInterpreter): Promise<string[] | undefined>;
}

export const ITerminalActivator = Symbol('ITerminalActivator');
export type TerminalActivationOptions = {
resource?: Resource;
preserveFocus?: boolean;
interpreter?: PythonInterpreter;
};
export interface ITerminalActivator {
activateEnvironmentInTerminal(terminal: Terminal, resource: Uri | undefined, preserveFocus?: boolean): Promise<boolean>;
activateEnvironmentInTerminal(terminal: Terminal, options?: TerminalActivationOptions): Promise<boolean>;
}

export const ITerminalActivationCommandProvider = Symbol('ITerminalActivationCommandProvider');
Expand Down
18 changes: 18 additions & 0 deletions src/client/common/utils/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,21 @@ export function isResource(resource?: InterpreterUri): resource is Resource {
const uri = resource as Uri;
return typeof uri.path === 'string' && typeof uri.scheme === 'string';
}

/**
* Checking whether something is a Uri.
* Using `instanceof Uri` doesn't always work as the object is not an instance of Uri (at least not in tests).
* That's why VSC too has a helper method `URI.isUri` (though not public).
*
* @export
* @param {InterpreterUri} [resource]
* @returns {resource is Uri}
*/
// tslint:disable-next-line: no-any
export function isUri(resource?: Uri | any): resource is Uri {
if (!resource) {
return false;
}
const uri = resource as Uri;
return typeof uri.path === 'string' && typeof uri.scheme === 'string';
}
13 changes: 8 additions & 5 deletions src/client/common/variables/environmentVariablesProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
}
@cacheResourceSpecificInterpreterData('getEnvironmentVariables', cacheDuration)
public async getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables> {
const settings = this.configurationService.getSettings(resource);
const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : '');
this.createFileWatcher(settings.envFile, workspaceFolderUri);
let mergedVars = await this.envVarsService.parseFile(settings.envFile, this.process.env);
let mergedVars = await this.getCustomEnvironmentVariables(resource);
if (!mergedVars) {
mergedVars = {};
}
Expand All @@ -63,6 +59,13 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
}
return mergedVars;
}
public async getCustomEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined> {
const settings = this.configurationService.getSettings(resource);
const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : '');
this.createFileWatcher(settings.envFile, workspaceFolderUri);
return this.envVarsService.parseFile(settings.envFile, this.process.env);
}
public configurationChanged(e: ConfigurationChangeEvent) {
this.trackedWorkspaceFolders.forEach(item => {
const uri = item && item.length > 0 ? Uri.file(item) : undefined;
Expand Down
1 change: 1 addition & 0 deletions src/client/common/variables/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ export const IEnvironmentVariablesProvider = Symbol('IEnvironmentVariablesProvid
export interface IEnvironmentVariablesProvider {
onDidEnvironmentVariablesChange: Event<Uri | undefined>;
getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables>;
getCustomEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined>;
}
Loading