Skip to content

Commit

Permalink
fix eclipse-theia#2961: a workaround to show output and title of shor…
Browse files Browse the repository at this point in the history 
…t-lived tasks.

Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
  • Loading branch information
@a1994846931931
a1994846931931 committed May 8, 2020
1 parent 7cf845b commit 6691f88
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 54 deletions.
22 changes: 22 additions & 0 deletions packages/process/src/node/process-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ export class ProcessManager implements BackendApplicationContribution {

protected id: number = 0;
protected readonly processes: Map<number, Process>;
protected readonly willReapProcesses: Map<number, Process>;
protected readonly deleteEmitter: Emitter<number>;
protected readonly REAP_TIMEOUT: number = 1000;

constructor(
@inject(ILogger) @named('process') protected logger: ILogger
) {
this.processes = new Map();
this.willReapProcesses = new Map();
this.deleteEmitter = new Emitter<number>();
}

Expand Down Expand Up @@ -63,13 +66,32 @@ export class ProcessManager implements BackendApplicationContribution {
}
if (this.processes.delete(process.id)) {
this.deleteEmitter.fire(process.id);
// Process will be recorded in willReapProcesses for a while so that it's still possible to get information of it before it's automatically reaped
this.willReapProcesses.set(process.id, process);
setTimeout(() => {
this.willReapProcesses.delete(process.id);
}, this.REAP_TIMEOUT);
this.logger.debug(`The process was successfully unregistered. ${processLabel}`);
} else {
this.logger.warn(`This process was not registered or was already unregistered. ${processLabel}`);
}
}

/**
* Get a process given its id. The process could be either alive, or dead and will be reaped soon.
*
* @param id the process id.
*/
get(id: number): Process | undefined {
return this.processes.get(id) || this.willReapProcesses.get(id);
}

/**
* Get a process given its id. Only process marked alive will be returned.
*
* @param id the process id.
*/
getAlive(id: number): Process | undefined {
return this.processes.get(id);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/task/src/browser/quick-open-task.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler {
if (mode !== QuickOpenMode.OPEN) {
return false;
}
this.taskService.attach(task.terminalId!, task.taskId);
if (TaskInfo.isAttachableTaskInfo(task)) {
this.taskService.attach(task);
}
return true;
}
},
Expand Down
53 changes: 22 additions & 31 deletions packages/task/src/browser/task-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ import {
DependsOrder,
RevealKind,
ApplyToKind,
TaskOutputPresentation
TaskOutputPresentation,
AttachableTaskInfo
} from '../common';
import { TaskWatcher } from '../common/task-watcher';
import { ProvidedTaskConfigurations } from './provided-task-configurations';
Expand Down Expand Up @@ -929,14 +930,9 @@ export class TaskService implements TaskConfigurationClient {
this.lastTask = { source, taskLabel, scope: resolvedTask._scope };
this.logger.debug(`Task created. Task id: ${taskInfo.taskId}`);

/**
* open terminal widget if the task is based on a terminal process (type: 'shell' or 'process')
*
* @todo Use a different mechanism to determine if the task should be attached?
* Reason: Maybe a new task type wants to also be displayed in a terminal.
*/
if (typeof taskInfo.terminalId === 'number') {
this.attach(taskInfo.terminalId, taskInfo.taskId);
// Open terminal widget if the task is attachable.
if (TaskInfo.isAttachableTaskInfo(taskInfo)) {
this.attach(taskInfo);
}
return taskInfo;
} catch (error) {
Expand Down Expand Up @@ -997,24 +993,19 @@ export class TaskService implements TaskConfigurationClient {
terminal.sendText(selectedText);
}

async attach(terminalId: number, taskId: number): Promise<void> {
// Get the list of all available running tasks.
const runningTasks: TaskInfo[] = await this.getRunningTasks();
// Get the corresponding task information based on task id if available.
const taskInfo: TaskInfo | undefined = runningTasks.find((t: TaskInfo) => t.taskId === taskId);
async attach(taskInfo: AttachableTaskInfo): Promise<void> {
let widgetOpenMode: WidgetOpenMode = 'open';
if (taskInfo) {
const terminalWidget = this.terminalService.getByTerminalId(terminalId);
if (terminalWidget) {
this.messageService.error('Task is already running in terminal');
return this.terminalService.open(terminalWidget, { mode: 'activate' });
}
if (TaskOutputPresentation.shouldAlwaysRevealTerminal(taskInfo.config)) {
if (TaskOutputPresentation.shouldSetFocusToTerminal(taskInfo.config)) { // assign focus to the terminal if presentation.focus is true
widgetOpenMode = 'activate';
} else { // show the terminal but not assign focus
widgetOpenMode = 'reveal';
}
const { taskId, terminalId } = taskInfo;
const terminalWidget = this.terminalService.getByTerminalId(terminalId);
if (terminalWidget) {
this.messageService.error('Task is already running in terminal');
return this.terminalService.open(terminalWidget, { mode: 'activate' });
}
if (TaskOutputPresentation.shouldAlwaysRevealTerminal(taskInfo.config)) {
if (TaskOutputPresentation.shouldSetFocusToTerminal(taskInfo.config)) { // assign focus to the terminal if presentation.focus is true
widgetOpenMode = 'activate';
} else { // show the terminal but not assign focus
widgetOpenMode = 'reveal';
}
}
// Create / find a terminal widget to display an execution output of a task that was launched as a command inside a shell.
Expand All @@ -1026,11 +1017,11 @@ export class TaskService implements TaskConfigurationClient {
: `Task: #${taskId}`,
destroyTermOnClose: true
}, {
taskId,
widgetOptions: { area: 'bottom' },
mode: widgetOpenMode,
taskInfo
});
taskId,
widgetOptions: { area: 'bottom' },
mode: widgetOpenMode,
taskInfo
});
widget.start(terminalId);
}

Expand Down
16 changes: 14 additions & 2 deletions packages/task/src/common/task-protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,6 @@ export interface TaskIdentifier {
export interface TaskInfo {
/** internal unique task id */
readonly taskId: number,
/** terminal id. Defined if task is run as a terminal process */
readonly terminalId?: number,
/** context that was passed as part of task creation, if any */
readonly ctx?: string,
/** task config used for launching a task */
Expand All @@ -187,6 +185,20 @@ export interface TaskInfo {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
readonly [key: string]: any;
}
export namespace TaskInfo {
export function isAttachableTaskInfo(taskInfo: TaskInfo): taskInfo is AttachableTaskInfo {
return typeof taskInfo.terminalId === 'number';
}
}

/**
* @todo Use a different mechanism to determine if the task should be attached?
* Reason: Maybe a new task type wants to also be displayed in a terminal.
*/
export interface AttachableTaskInfo extends TaskInfo {
/** terminal id. Defined if task is run as a terminal process */
readonly terminalId: number,
}

export interface TaskServer extends JsonRpcServer<TaskClient> {
/** Run a task. Optionally pass a context. */
Expand Down
40 changes: 20 additions & 20 deletions packages/terminal/src/node/base-terminal-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer {
abstract create(options: IBaseTerminalServerOptions): Promise<number>;

async attach(id: number): Promise<number> {
const term = this.processManager.get(id);
const terminal = this.processManager.get(id);

if (term && term instanceof TerminalProcess) {
return term.id;
if (terminal && terminal instanceof TerminalProcess) {
return terminal.id;
} else {
this.logger.error(`Couldn't attach - can't find terminal with id: ${id} `);
return -1;
Expand All @@ -53,15 +53,15 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer {

async getProcessId(id: number): Promise<number> {
const terminal = this.processManager.get(id);
if (!(terminal instanceof TerminalProcess)) {
if (!(terminal && terminal instanceof TerminalProcess)) {
throw new Error(`terminal "${id}" does not exist`);
}
return terminal.pid;
}

async getProcessInfo(id: number): Promise<TerminalProcessInfo> {
const terminal = this.processManager.get(id);
if (!(terminal instanceof TerminalProcess)) {
if (!(terminal && terminal instanceof TerminalProcess)) {
throw new Error(`terminal "${id}" does not exist`);
}
return {
Expand All @@ -71,18 +71,18 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer {
}

async getCwdURI(id: number): Promise<string> {
const terminal = this.processManager.get(id);
if (!(terminal instanceof TerminalProcess)) {
const terminal = this.processManager.getAlive(id);
if (!(terminal && terminal instanceof TerminalProcess)) {
throw new Error(`terminal "${id}" does not exist`);
}
return terminal.getCwdURI();
}

async close(id: number): Promise<void> {
const term = this.processManager.get(id);
const terminal = this.processManager.getAlive(id);

if (term instanceof TerminalProcess) {
term.kill();
if (terminal && terminal instanceof TerminalProcess) {
terminal.kill();
}
}

Expand All @@ -95,9 +95,9 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer {
}

async resize(id: number, cols: number, rows: number): Promise<void> {
const term = this.processManager.get(id);
if (term && term instanceof TerminalProcess) {
term.resize(cols, rows);
const terminal = this.processManager.getAlive(id);
if (terminal && terminal instanceof TerminalProcess) {
terminal.resize(cols, rows);
} else {
console.error("Couldn't resize terminal " + id + ", because it doesn't exist.");
}
Expand All @@ -108,31 +108,31 @@ export abstract class BaseTerminalServer implements IBaseTerminalServer {
this.client = client;
}

protected postCreate(term: TerminalProcess): void {
protected postCreate(terminal: TerminalProcess): void {
const toDispose = new DisposableCollection();

toDispose.push(term.onError(error => {
this.logger.error(`Terminal pid: ${term.pid} error: ${error}, closing it.`);
toDispose.push(terminal.onError(error => {
this.logger.error(`Terminal pid: ${terminal.pid} error: ${error}, closing it.`);

if (this.client !== undefined) {
this.client.onTerminalError({
'terminalId': term.id,
'terminalId': terminal.id,
'error': new Error(`Failed to execute terminal process (${error.code})`),
});
}
}));

toDispose.push(term.onExit(event => {
toDispose.push(terminal.onExit(event => {
if (this.client !== undefined) {
this.client.onTerminalExitChanged({
'terminalId': term.id,
'terminalId': terminal.id,
'code': event.code,
'signal': event.signal
});
}
}));

this.terminalToDispose.set(term.id, toDispose);
this.terminalToDispose.set(terminal.id, toDispose);
}

}

0 comments on commit 6691f88

Please sign in to comment.