Skip to content

Commit

Permalink
Add terminalGroup to tasks to allow running them in split panes (#65973)
Browse files Browse the repository at this point in the history
* Return splitted instance from ITerminalService::splitInstance

* Add support for terminalGroup to tasks

* Early-exit when the tab could not be split

https://github.com/Microsoft/vscode/pull/65973/files/42e3171a71ae4b6963e47318fd289a66eb48a96a#r245762395

* Use .get()! instead of the unsupported [] to access LinkedMap

* Move api changes into `vscode.proposed.d.ts`

* Rename "terminalGroup" to "group"

* Only keep references to terminals in sameTaskTerminals and idleTaskTerminals if the terminal is not disposed

* Type result variable

Fixes #47265
  • Loading branch information
cmfcmf authored and alexr00 committed Jan 25, 2019
1 parent 2cddb77 commit 7a9f7e5
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 30 deletions.
8 changes: 8 additions & 0 deletions src/vs/vscode.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,14 @@ declare module 'vscode' {
}
//#endregion

//#region Tasks
export interface TaskPresentationOptions {
/**
* Controls whether the task is executed in a specific terminal group using split panes.
*/
group?: string;
}
//#endregion

//#region Autofix - mjbvz
export namespace CodeActionKind {
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/api/shared/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface TaskPresentationOptionsDTO {
panel?: number;
showReuseMessage?: boolean;
clear?: boolean;
group?: string;
}

export interface RunOptionsDTO {
Expand Down
5 changes: 5 additions & 0 deletions src/vs/workbench/parts/tasks/common/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@ export interface PresentationOptions {
* Controls whether to clear the terminal before executing the task.
*/
clear: boolean;

/**
* Controls whether the task is executed in a specific terminal group using split panes.
*/
group?: string;
}

export namespace PresentationOptions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ const presentation: IJSONSchema = {
type: 'boolean',
default: false,
description: nls.localize('JsonSchema.tasks.presentation.clear', 'Controls whether the terminal is cleared before executing the task.')
}
},
group: {
type: 'string',
description: nls.localize('JsonSchema.tasks.presentation.group', 'Controls whether the task is executed in a specific terminal group using split panes.')
},
}
};

Expand Down
76 changes: 58 additions & 18 deletions src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as Objects from 'vs/base/common/objects';
import * as Types from 'vs/base/common/types';
import * as Platform from 'vs/base/common/platform';
import * as Async from 'vs/base/common/async';
import { IStringDictionary } from 'vs/base/common/collections';
import { IStringDictionary, values } from 'vs/base/common/collections';
import { LinkedMap, Touch } from 'vs/base/common/map';
import Severity from 'vs/base/common/severity';
import { Event, Emitter } from 'vs/base/common/event';
Expand Down Expand Up @@ -42,6 +42,7 @@ import {
interface TerminalData {
terminal: ITerminalInstance;
lastTask: string;
group?: string;
}

interface ActiveTerminalData {
Expand Down Expand Up @@ -538,13 +539,16 @@ export class TerminalTaskSystem implements ITaskSystem {
let key = task.getMapKey();
delete this.activeTasks[key];
this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Changed));
switch (task.command.presentation!.panel) {
case PanelKind.Dedicated:
this.sameTaskTerminals[key] = terminal!.id.toString();
break;
case PanelKind.Shared:
this.idleTaskTerminals.set(key, terminal!.id.toString(), Touch.AsOld);
break;
if (exitCode !== undefined) {
// Only keep a reference to the terminal if it is not being disposed.
switch (task.command.presentation!.panel) {
case PanelKind.Dedicated:
this.sameTaskTerminals[key] = terminal!.id.toString();
break;
case PanelKind.Shared:
this.idleTaskTerminals.set(key, terminal!.id.toString(), Touch.AsOld);
break;
}
}
let reveal = task.command.presentation!.reveal;
if ((reveal === RevealKind.Silent) && ((exitCode !== 0) || (watchingProblemMatcher.numberOfMatches > 0) && watchingProblemMatcher.maxMarkerSeverity &&
Expand Down Expand Up @@ -601,13 +605,16 @@ export class TerminalTaskSystem implements ITaskSystem {
let key = task.getMapKey();
delete this.activeTasks[key];
this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Changed));
switch (task.command.presentation!.panel) {
case PanelKind.Dedicated:
this.sameTaskTerminals[key] = terminal!.id.toString();
break;
case PanelKind.Shared:
this.idleTaskTerminals.set(key, terminal!.id.toString(), Touch.AsOld);
break;
if (exitCode !== undefined) {
// Only keep a reference to the terminal if it is not being disposed.
switch (task.command.presentation!.panel) {
case PanelKind.Dedicated:
this.sameTaskTerminals[key] = terminal!.id.toString();
break;
case PanelKind.Shared:
this.idleTaskTerminals.set(key, terminal!.id.toString(), Touch.AsOld);
break;
}
}
let reveal = task.command.presentation!.reveal;
if (terminal && (reveal === RevealKind.Silent) && ((exitCode !== 0) || (startStopProblemMatcher.numberOfMatches > 0) && startStopProblemMatcher.maxMarkerSeverity &&
Expand Down Expand Up @@ -857,6 +864,7 @@ export class TerminalTaskSystem implements ITaskSystem {
}
let prefersSameTerminal = presentationOptions.panel === PanelKind.Dedicated;
let allowsSharedTerminal = presentationOptions.panel === PanelKind.Shared;
let group = presentationOptions.group;

let taskKey = task.getMapKey();
let terminalToReuse: TerminalData | undefined;
Expand All @@ -867,7 +875,20 @@ export class TerminalTaskSystem implements ITaskSystem {
delete this.sameTaskTerminals[taskKey];
}
} else if (allowsSharedTerminal) {
let terminalId = this.idleTaskTerminals.remove(taskKey) || this.idleTaskTerminals.shift();
// Always allow to reuse the terminal previously used by the same task.
let terminalId = this.idleTaskTerminals.remove(taskKey);
if (!terminalId) {
// There is no idle terminal which was used by the same task.
// Search for any idle terminal used previously by a task of the same group
// (or, if the task has no group, a terminal used by a task without group).
for (const taskId of this.idleTaskTerminals.keys()) {
const idleTerminalId = this.idleTaskTerminals.get(taskId)!;
if (this.terminals[idleTerminalId].group === group) {
terminalId = this.idleTaskTerminals.remove(taskId);
break;
}
}
}
if (terminalId) {
terminalToReuse = this.terminals[terminalId];
}
Expand All @@ -880,7 +901,26 @@ export class TerminalTaskSystem implements ITaskSystem {
return [terminalToReuse.terminal, commandExecutable, undefined];
}

const result = this.terminalService.createTerminal(this.currentTask.shellLaunchConfig);
let result: ITerminalInstance | null = null;
if (group) {
// Try to find an existing terminal to split.
// Even if an existing terminal is found, the split can fail if the terminal width is too small.
for (const terminal of values(this.terminals)) {
if (terminal.group === group) {
const originalInstance = terminal.terminal;
const config = this.currentTask.shellLaunchConfig;
result = this.terminalService.splitInstance(originalInstance, config);
if (result) {
break;
}
}
}
}
if (!result) {
// Either no group is used, no terminal with the group exists or splitting an existing terminal failed.
result = this.terminalService.createTerminal(this.currentTask.shellLaunchConfig);
}

const terminalKey = result.id.toString();
result.onDisposed((terminal) => {
let terminalData = this.terminals[terminalKey];
Expand All @@ -895,7 +935,7 @@ export class TerminalTaskSystem implements ITaskSystem {
delete this.activeTasks[task.getMapKey()];
}
});
this.terminals[terminalKey] = { terminal: result, lastTask: taskKey };
this.terminals[terminalKey] = { terminal: result, lastTask: taskKey, group };
return [result, commandExecutable, undefined];
}

Expand Down
13 changes: 11 additions & 2 deletions src/vs/workbench/parts/tasks/node/taskConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ export interface PresentationOptionsConfig {
* Controls whether the terminal should be cleared before running the task.
*/
clear?: boolean;

/**
* Controls whether the task is executed in a specific terminal group using split panes.
*/
group?: string;
}

export interface RunOptionsConfig {
Expand Down Expand Up @@ -794,7 +799,7 @@ namespace CommandOptions {
namespace CommandConfiguration {

export namespace PresentationOptions {
const properties: MetaData<Tasks.PresentationOptions, void>[] = [{ property: 'echo' }, { property: 'reveal' }, { property: 'focus' }, { property: 'panel' }, { property: 'showReuseMessage' }, { property: 'clear' }];
const properties: MetaData<Tasks.PresentationOptions, void>[] = [{ property: 'echo' }, { property: 'reveal' }, { property: 'focus' }, { property: 'panel' }, { property: 'showReuseMessage' }, { property: 'clear' }, { property: 'group' }];

interface PresentationOptionsShape extends LegacyCommandProperties {
presentation?: PresentationOptionsConfig;
Expand All @@ -807,6 +812,7 @@ namespace CommandConfiguration {
let panel: Tasks.PanelKind;
let showReuseMessage: boolean;
let clear: boolean;
let group: string | undefined;
let hasProps = false;
if (Types.isBoolean(config.echoCommand)) {
echo = config.echoCommand;
Expand Down Expand Up @@ -836,12 +842,15 @@ namespace CommandConfiguration {
if (Types.isBoolean(presentation.clear)) {
clear = presentation.clear;
}
if (Types.isString(presentation.group)) {
group = presentation.group;
}
hasProps = true;
}
if (!hasProps) {
return undefined;
}
return { echo: echo!, reveal: reveal!, focus: focus!, panel: panel!, showReuseMessage: showReuseMessage!, clear: clear! };
return { echo: echo!, reveal: reveal!, focus: focus!, panel: panel!, showReuseMessage: showReuseMessage!, clear: clear!, group };
}

export function assignProperties(target: Tasks.PresentationOptions, source: Tasks.PresentationOptions | undefined): Tasks.PresentationOptions | undefined {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/parts/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export interface ITerminalService {
setActiveInstance(terminalInstance: ITerminalInstance): void;
setActiveInstanceByIndex(terminalIndex: number): void;
getActiveOrCreateInstance(wasNewTerminalAction?: boolean): ITerminalInstance;
splitInstance(instance: ITerminalInstance, shell?: IShellLaunchConfig): void;
splitInstance(instance: ITerminalInstance, shell?: IShellLaunchConfig): ITerminalInstance | null;

getActiveTab(): ITerminalTab | null;
setActiveTabToNext(): void;
Expand Down
18 changes: 10 additions & 8 deletions src/vs/workbench/parts/terminal/common/terminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,21 +264,23 @@ export abstract class TerminalService implements ITerminalService {
this.setActiveTabByIndex(newIndex);
}

public splitInstance(instanceToSplit: ITerminalInstance, shellLaunchConfig: IShellLaunchConfig = {}): void {
public splitInstance(instanceToSplit: ITerminalInstance, shellLaunchConfig: IShellLaunchConfig = {}): ITerminalInstance | null {
const tab = this._getTabForInstance(instanceToSplit);
if (!tab) {
return;
return null;
}

const instance = tab.split(this._terminalFocusContextKey, this.configHelper, shellLaunchConfig);
if (instance) {
this._initInstanceListeners(instance);
this._onInstancesChanged.fire();

this._terminalTabs.forEach((t, i) => t.setVisible(i === this._activeTabIndex));
} else {
if (!instance) {
this._showNotEnoughSpaceToast();
return null;
}

this._initInstanceListeners(instance);
this._onInstancesChanged.fire();

this._terminalTabs.forEach((t, i) => t.setVisible(i === this._activeTabIndex));
return instance;
}

protected _initInstanceListeners(instance: ITerminalInstance): void {
Expand Down

0 comments on commit 7a9f7e5

Please sign in to comment.