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

Reuse terminals when applicable #1888

Closed
wants to merge 7 commits into from
Closed
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
2 changes: 1 addition & 1 deletion extension.bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export { LineSplitter } from './src/debugging/coreclr/lineSplitter';
export { delay } from './src/utils/delay';
export { Lazy, AsyncLazy } from './src/utils/lazy';
export { OSProvider } from './src/utils/LocalOSProvider';
export { bufferToString } from './src/utils/spawnAsync';
export { DockerDaemonIsLinuxPrerequisite, DockerfileExistsPrerequisite, DotNetSdkInstalledPrerequisite, LinuxUserInDockerGroupPrerequisite, MacNuGetFallbackFolderSharedPrerequisite } from './src/debugging/coreclr/prereqManager';
export { ext } from './src/extensionVariables';
export { globAsync } from './src/utils/globAsync';
Expand All @@ -40,7 +41,6 @@ export { inferCommand, inferPackageName, InspectMode, NodePackage } from './src/
export { nonNullProp } from './src/utils/nonNull';
export { getDockerOSType, isWindows10RS3OrNewer, isWindows10RS4OrNewer, isWindows10RS5OrNewer, isWindows1019H1OrNewer } from "./src/utils/osUtils";
export { Platform, PlatformOS } from './src/utils/platform';
export { DefaultTerminalProvider } from './src/utils/TerminalProvider';
export { trimWithElipsis } from './src/utils/trimWithElipsis';
export { recursiveFindTaskByType } from './src/tasks/TaskHelper';
export { TaskDefinitionBase } from './src/tasks/TaskDefinitionBase';
Expand Down
9 changes: 3 additions & 6 deletions src/commands/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

import * as vscode from 'vscode';
import { IActionContext } from 'vscode-azureextensionui';
import { ext } from '../extensionVariables';
import { localize } from "../localize";
import { executeAsTask } from '../utils/executeAsTask';
import { createFileItem, Item, quickPickDockerComposeFileItem } from '../utils/quickPickFile';
import { quickPickWorkspaceFolder } from '../utils/quickPickWorkspaceFolder';
import { selectComposeCommand } from './selectCommandTemplate';
Expand All @@ -30,12 +30,10 @@ async function compose(context: IActionContext, commands: ('up' | 'down')[], mes
selectedItems = selectedItem ? [selectedItem] : [];
}

const terminal: vscode.Terminal = ext.terminalProvider.createTerminal('Docker Compose');
const configOptions: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration('docker');
const build: boolean = configOptions.get('dockerComposeBuild', true);
const detached: boolean = configOptions.get('dockerComposeDetached', true);

terminal.sendText(`cd "${folder.uri.fsPath}"`);
for (const command of commands) {
if (selectedItems.length === 0) {
const terminalCommand = await selectComposeCommand(
Expand All @@ -46,7 +44,7 @@ async function compose(context: IActionContext, commands: ('up' | 'down')[], mes
detached,
build
);
terminal.sendText(terminalCommand);
await executeAsTask(context, terminalCommand, 'Docker Compose', /* addDockerEnv: */ true, folder);
} else {
for (const item of selectedItems) {
const terminalCommand = await selectComposeCommand(
Expand All @@ -57,10 +55,9 @@ async function compose(context: IActionContext, commands: ('up' | 'down')[], mes
detached,
build
);
terminal.sendText(terminalCommand);
await executeAsTask(context, terminalCommand, 'Docker Compose', /* addDockerEnv: */ true, folder);
}
}
terminal.show();
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/commands/containers/attachShellContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IActionContext } from 'vscode-azureextensionui';
import { ext } from '../../extensionVariables';
import { localize } from '../../localize';
import { ContainerTreeItem } from '../../tree/containers/ContainerTreeItem';
import { executeAsTask } from '../../utils/executeAsTask';
import { getDockerOSType } from '../../utils/osUtils';
import { selectAttachCommand } from '../selectCommandTemplate';

Expand Down Expand Up @@ -39,7 +40,5 @@ export async function attachShellContainer(context: IActionContext, node?: Conta
shellCommand
);

const terminal = ext.terminalProvider.createTerminal(`Shell: ${node.containerName}`);
terminal.sendText(terminalCommand);
terminal.show();
await executeAsTask(context, terminalCommand, `Shell: ${node.containerName}`, /* addDockerEnv: */ true);
Copy link
Collaborator Author

@bwateratmsft bwateratmsft Apr 21, 2020

Choose a reason for hiding this comment

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

Shell quoting is causing problems here...e.g. attach shell fails on Windows w/ PowerShell.

}
5 changes: 2 additions & 3 deletions src/commands/containers/viewContainerLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { IActionContext } from 'vscode-azureextensionui';
import { ext } from '../../extensionVariables';
import { localize } from '../../localize';
import { ContainerTreeItem } from '../../tree/containers/ContainerTreeItem';
import { executeAsTask } from '../../utils/executeAsTask';
import { selectLogsCommand } from '../selectCommandTemplate';

export async function viewContainerLogs(context: IActionContext, node?: ContainerTreeItem): Promise<void> {
Expand All @@ -24,7 +25,5 @@ export async function viewContainerLogs(context: IActionContext, node?: Containe
node.containerId
);

const terminal = ext.terminalProvider.createTerminal(node.fullTag);
terminal.sendText(terminalCommand);
terminal.show();
await executeAsTask(context, terminalCommand, node.fullTag, /* addDockerEnv: */ true);
}
18 changes: 10 additions & 8 deletions src/commands/dockerInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import { IActionContext } from 'vscode-azureextensionui';
import ChildProcessProvider from '../debugging/coreclr/ChildProcessProvider';
import { LocalFileSystemProvider } from '../debugging/coreclr/fsProvider';
import { OSTempFileProvider } from '../debugging/coreclr/tempFileProvider';
import { ext } from '../extensionVariables';
import { localize } from '../localize';
import { executeAsTask } from '../utils/executeAsTask';
import { streamToFile } from '../utils/httpRequest';
import LocalOSProvider from '../utils/LocalOSProvider';
import { execAsync } from '../utils/spawnAsync';
Expand All @@ -18,7 +20,7 @@ export abstract class DockerInstallerBase {
protected abstract fileExtension: string;
protected abstract getInstallCommand(fileName: string): string;

public async downloadAndInstallDocker(): Promise<void> {
public async downloadAndInstallDocker(context: IActionContext): Promise<void> {
const confirmInstall: string = localize('vscode-docker.commands.DockerInstallerBase.confirm', 'Are you sure you want to install Docker on this machine?');
const installTitle: string = localize('vscode-docker.commands.DockerInstallerBase.install', 'Install');
const downloadingMessage: string = localize('vscode-docker.commands.DockerInstallerBase.downloading', 'Downloading Docker installer...');
Expand All @@ -34,7 +36,7 @@ export abstract class DockerInstallerBase {
const command = this.getInstallCommand(downloadedFileName);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
vscode.window.showInformationMessage(installationMessage);
await this.install(downloadedFileName, command);
await this.install(context, downloadedFileName, command);
}

private async downloadInstaller(): Promise<string> {
Expand All @@ -46,7 +48,7 @@ export abstract class DockerInstallerBase {
return fileName;
}

protected abstract install(fileName: string, cmd: string): Promise<void>;
protected abstract install(context: IActionContext, fileName: string, cmd: string): Promise<void>;
}

export class WindowsDockerInstaller extends DockerInstallerBase {
Expand All @@ -57,7 +59,7 @@ export class WindowsDockerInstaller extends DockerInstallerBase {
return `"${fileName}"`;
}

protected async install(fileName: string, cmd: string): Promise<void> {
protected async install(context: IActionContext, fileName: string, cmd: string): Promise<void> {
const fsProvider = new LocalFileSystemProvider();
try {
ext.outputChannel.appendLine(localize('vscode-docker.commands.DockerInstallerBase.downloadCompleteMessage', 'Executing command {0}', cmd));
Expand All @@ -77,10 +79,10 @@ export class MacDockerInstaller extends DockerInstallerBase {
return `chmod +x '${fileName}' && open '${fileName}'`;
}

protected async install(fileName: string): Promise<void> {
const terminal = ext.terminalProvider.createTerminal(localize('vscode-docker.commands.MacDockerInstaller.terminalTitle', 'Docker Install'));
protected async install(context: IActionContext, fileName: string): Promise<void> {
const title = localize('vscode-docker.commands.MacDockerInstaller.terminalTitle', 'Docker Install');
const command = this.getInstallCommand(fileName);
terminal.sendText(command);
terminal.show();

await executeAsTask(context, command, title);
}
}
5 changes: 2 additions & 3 deletions src/commands/images/buildImage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ext } from "../../extensionVariables";
import { localize } from '../../localize';
import { getOfficialBuildTaskForDockerfile } from "../../tasks/TaskHelper";
import { delay } from "../../utils/delay";
import { executeAsTask } from "../../utils/executeAsTask";
import { getValidImageName } from "../../utils/getValidImageName";
import { quickPickDockerFileItem } from "../../utils/quickPickFile";
import { quickPickWorkspaceFolder } from "../../utils/quickPickWorkspaceFolder";
Expand Down Expand Up @@ -64,8 +65,6 @@ export async function buildImage(context: IActionContext, dockerFileUri: vscode.
terminalCommand = terminalCommand.replace(tagRegex, imageName);
}

const terminal: vscode.Terminal = ext.terminalProvider.createTerminal('Docker');
terminal.sendText(terminalCommand);
terminal.show();
await executeAsTask(context, terminalCommand, 'Docker', /* addDockerEnv: */ true, rootFolder);
}
}
5 changes: 2 additions & 3 deletions src/commands/images/pushImage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { localize } from '../../localize';
import { ImageTreeItem } from '../../tree/images/ImageTreeItem';
import { registryExpectedContextValues } from '../../tree/registries/registryContextValues';
import { RegistryTreeItemBase } from '../../tree/registries/RegistryTreeItemBase';
import { executeAsTask } from '../../utils/executeAsTask';
import { addImageTaggingTelemetry, tagImage } from './tagImage';

export async function pushImage(context: IActionContext, node: ImageTreeItem | undefined): Promise<void> {
Expand Down Expand Up @@ -64,9 +65,7 @@ export async function pushImage(context: IActionContext, node: ImageTreeItem | u
addImageTaggingTelemetry(context, finalTag, '');

// Finally push the image
const terminal = ext.terminalProvider.createTerminal(finalTag);
terminal.sendText(`docker push ${finalTag}`);
terminal.show();
await executeAsTask(context, `docker push ${finalTag}`, finalTag, /* addDockerEnv: */ true);
}

async function tryGetConnectedRegistryForPath(context: IActionContext, baseImagePath: string): Promise<RegistryTreeItemBase | undefined> {
Expand Down
7 changes: 2 additions & 5 deletions src/commands/images/runAzureCliImage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import * as fse from 'fs-extra';
import * as os from 'os';
import * as vscode from 'vscode';
import { DialogResponses, IActionContext } from 'vscode-azureextensionui';
import { ext } from '../../extensionVariables';
import { localize } from '../../localize';
import { executeAsTask } from '../../utils/executeAsTask';
import { openExternal } from '../../utils/openExternal';
import { getDockerOSType } from '../../utils/osUtils';

Expand Down Expand Up @@ -38,9 +38,6 @@ export async function runAzureCliImage(context: IActionContext): Promise<void> {
vol += ` -v ${homeDir}/.kube:/root/.kube`;
}

const cmd: string = `docker run ${option} ${vol.trim()} -it --rm azuresdk/azure-cli-python:latest`;
const terminal: vscode.Terminal = ext.terminalProvider.createTerminal('Azure CLI');
terminal.sendText(cmd);
terminal.show();
await executeAsTask(context, `docker run ${option} ${vol.trim()} -it --rm azuresdk/azure-cli-python:latest`, 'Azure CLI', /* addDockerEnv: */ true);
}
}
5 changes: 2 additions & 3 deletions src/commands/images/runImage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ext } from '../../extensionVariables';
import { localize } from '../../localize';
import { ImageTreeItem } from '../../tree/images/ImageTreeItem';
import { callDockerode } from '../../utils/callDockerode';
import { executeAsTask } from '../../utils/executeAsTask';
import { selectRunCommand } from '../selectCommandTemplate';

export async function runImage(context: IActionContext, node?: ImageTreeItem): Promise<void> {
Expand Down Expand Up @@ -36,7 +37,5 @@ async function runImageCore(context: IActionContext, node: ImageTreeItem | undef
inspectInfo?.Config?.ExposedPorts
);

const terminal = ext.terminalProvider.createTerminal(node.fullTag);
terminal.sendText(terminalCommand);
terminal.show();
await executeAsTask(context, terminalCommand, node.fullTag, /* addDockerEnv: */ true);
}
4 changes: 2 additions & 2 deletions src/commands/installDocker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { MacDockerInstaller, WindowsDockerInstaller } from './dockerInstaller';

export async function installDocker(context: IActionContext): Promise<void> {
if (os.platform() === 'win32') {
await (new WindowsDockerInstaller()).downloadAndInstallDocker();
await (new WindowsDockerInstaller()).downloadAndInstallDocker(context);
} else if (os.platform() === 'darwin') {
await (new MacDockerInstaller()).downloadAndInstallDocker();
await (new MacDockerInstaller()).downloadAndInstallDocker(context);
} else {
await openExternal('https://aka.ms/download-docker-linux-vscode');
}
Expand Down
6 changes: 2 additions & 4 deletions src/commands/registries/logOutOfDockerCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { Terminal } from 'vscode';
import { IActionContext } from 'vscode-azureextensionui';
import { ext } from '../../extensionVariables';
import { registryExpectedContextValues } from '../../tree/registries/registryContextValues';
import { RegistryTreeItemBase } from '../../tree/registries/RegistryTreeItemBase';
import { executeAsTask } from '../../utils/executeAsTask';

export async function logOutOfDockerCli(context: IActionContext, node?: RegistryTreeItemBase): Promise<void> {
if (!node) {
node = await ext.registriesTree.showTreeItemPicker<RegistryTreeItemBase>(registryExpectedContextValues.all.registry, context);
}

let creds = await node.getDockerCliCredentials();
const terminal: Terminal = ext.terminalProvider.createTerminal('docker logout');
terminal.sendText(`docker logout ${creds.registryPath}`);
terminal.show();
await executeAsTask(context, `docker logout ${creds.registryPath}`, 'Docker', /* addDockerEnv: */ true);
}
6 changes: 2 additions & 4 deletions src/commands/registries/pullImages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { Terminal } from "vscode";
import { IActionContext } from "vscode-azureextensionui";
import { ext } from '../../extensionVariables';
import { registryExpectedContextValues } from "../../tree/registries/registryContextValues";
import { RegistryTreeItemBase } from '../../tree/registries/RegistryTreeItemBase';
import { RemoteRepositoryTreeItemBase } from "../../tree/registries/RemoteRepositoryTreeItemBase";
import { RemoteTagTreeItem } from '../../tree/registries/RemoteTagTreeItem';
import { executeAsTask } from "../../utils/executeAsTask";
import { logInToDockerCli } from "./logInToDockerCli";

export async function pullRepository(context: IActionContext, node?: RemoteRepositoryTreeItemBase): Promise<void> {
Expand All @@ -31,7 +31,5 @@ export async function pullImage(context: IActionContext, node?: RemoteTagTreeIte
async function pullImages(context: IActionContext, node: RegistryTreeItemBase, imageRequest: string): Promise<void> {
await logInToDockerCli(context, node);

const terminal: Terminal = ext.terminalProvider.createTerminal("docker pull");
terminal.show();
terminal.sendText(`docker pull ${node.baseImagePath}/${imageRequest}`);
await executeAsTask(context, `docker pull ${node.baseImagePath}/${imageRequest}`, 'Docker', /* addDockerEnv: */ true);
}
5 changes: 0 additions & 5 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import { AzureAccountExtensionListener } from './utils/AzureAccountExtensionList
import { Keytar } from './utils/keytar';
import { refreshDockerode } from './utils/refreshDockerode';
import { bufferToString } from './utils/spawnAsync';
import { DefaultTerminalProvider } from './utils/TerminalProvider';

export type KeyInfo = { [keyName: string]: string };

Expand All @@ -58,10 +57,6 @@ function initializeExtensionVariables(ctx: vscode.ExtensionContext): void {
ext.outputChannel = createAzExtOutputChannel('Docker', ext.prefix);
ctx.subscriptions.push(ext.outputChannel);

if (!ext.terminalProvider) {
ext.terminalProvider = new DefaultTerminalProvider();
}

const publisher = new TelemetryPublisher();
ctx.subscriptions.push(publisher);

Expand Down
2 changes: 0 additions & 2 deletions src/extensionVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { NetworksTreeItem } from './tree/networks/NetworksTreeItem';
import { RegistriesTreeItem } from './tree/registries/RegistriesTreeItem';
import { VolumesTreeItem } from './tree/volumes/VolumesTreeItem';
import { IKeytar } from './utils/keytar';
import { ITerminalProvider } from "./utils/TerminalProvider";

/**
* Namespace for common variables used throughout the extension. They must be initialized in the activate() method of extension.ts
Expand All @@ -24,7 +23,6 @@ export namespace ext {
export let outputChannel: IAzExtOutputChannel;
export let ui: IAzureUserInput;
export let reporter: ITelemetryReporter;
export let terminalProvider: ITerminalProvider;
export let keytar: IKeytar | undefined;
export let dockerode: Dockerode;
export let dockerodeInitError: unknown;
Expand Down
22 changes: 0 additions & 22 deletions src/utils/TerminalProvider.ts

This file was deleted.

29 changes: 29 additions & 0 deletions src/utils/executeAsTask.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import { IActionContext } from 'vscode-azureextensionui';
import { addDockerSettingsToEnv } from './addDockerSettingsToEnv';

export async function executeAsTask(context: IActionContext, command: string, name: string, addDockerEnv?: boolean, workspaceFolder?: vscode.WorkspaceFolder, cwd?: string): Promise<vscode.TaskExecution> {
let newEnv: { [key: string]: string } | undefined;

if (addDockerEnv) {
// We don't need to merge process.env into newEnv, since ShellExecution does that automatically via ShellExecutionOptions
newEnv = {};
addDockerSettingsToEnv(newEnv, process.env);
}

const task = new vscode.Task(
{ type: 'shell' },
workspaceFolder ?? vscode.TaskScope.Workspace,
name,
'Docker',
new vscode.ShellExecution(command, { cwd: cwd || workspaceFolder.uri.fsPath, env: newEnv }),
[] // problemMatchers
);

return vscode.tasks.executeTask(task);
}
Loading