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

[WIP] Display initial loading message in terminal #6957

Closed
wants to merge 4 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: 2 additions & 0 deletions packages/task/src/browser/task-frontend-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import './tasks-monaco-contribution';
import { TaskNameResolver } from './task-name-resolver';
import { TaskSourceResolver } from './task-source-resolver';
import { TaskTemplateSelector } from './task-templates';
import { TaskTerminalManager } from './task-terminal-manager';

export default new ContainerModule(bind => {
bind(TaskFrontendContribution).toSelf().inSingletonScope();
Expand Down Expand Up @@ -77,6 +78,7 @@ export default new ContainerModule(bind => {
bind(TaskNameResolver).toSelf().inSingletonScope();
bind(TaskSourceResolver).toSelf().inSingletonScope();
bind(TaskTemplateSelector).toSelf().inSingletonScope();
bind(TaskTerminalManager).toSelf().inSingletonScope();

bindProcessTaskModule(bind);
bindTaskPreferences(bind);
Expand Down
64 changes: 30 additions & 34 deletions packages/task/src/browser/task-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import { TaskSchemaUpdater } from './task-schema-updater';
import { TaskConfigurationManager } from './task-configuration-manager';
import { PROBLEMS_WIDGET_ID, ProblemWidget } from '@theia/markers/lib/browser/problem/problem-widget';
import { TaskNode } from './task-node';
import { TaskTerminalManager } from './task-terminal-manager';

export interface QuickPickProblemMatcherItem {
problemMatchers: NamedProblemMatcher[] | undefined;
Expand Down Expand Up @@ -169,6 +170,10 @@ export class TaskService implements TaskConfigurationClient {

@inject(LabelProvider)
protected readonly labelProvider: LabelProvider;

@inject(TaskTerminalManager)
protected readonly taskTerminalManager: TaskTerminalManager;

/**
* @deprecated To be removed in 0.5.0
*/
Expand Down Expand Up @@ -454,6 +459,9 @@ export class TaskService implements TaskConfigurationClient {
* It looks for configured and detected tasks.
*/
async run(source: string, taskLabel: string, scope?: string): Promise<TaskInfo | undefined> {
// Open an empty terminal, display a message informing the user that terminal is starting
const widget = await this.taskTerminalManager.openEmptyTerminal(taskLabel);

let task = await this.getProvidedTask(source, taskLabel, scope);
if (!task) { // if a detected task cannot be found, search from tasks.json
task = this.taskConfigurations.getTask(source, taskLabel);
Expand All @@ -462,6 +470,7 @@ export class TaskService implements TaskConfigurationClient {
return;
}
}

const customizationObject = await this.getTaskCustomization(task);

if (!customizationObject.problemMatcher) {
Expand Down Expand Up @@ -502,7 +511,7 @@ export class TaskService implements TaskConfigurationClient {
return undefined;
}
return this.runTasksGraph(task, tasks, {
customization: { ...customizationObject, ...{ problemMatcher: resolvedMatchers } }
customization: { ...customizationObject, ...{ problemMatcher: resolvedMatchers }, widget}
}).catch(error => {
console.log(error.message);
return undefined;
Expand All @@ -513,7 +522,8 @@ export class TaskService implements TaskConfigurationClient {
* A recursive function that runs a task and all its sub tasks that it depends on.
* A task can be executed only when all of its dependencies have been executed, or when it doesn’t have any dependencies at all.
*/
async runTasksGraph(task: TaskConfiguration, tasks: TaskConfiguration[], option?: RunTaskOption): Promise<TaskInfo | undefined> {
async runTasksGraph(task: TaskConfiguration, tasks: TaskConfiguration[],
option?: RunTaskOption, widget?: TerminalWidget): Promise<TaskInfo | undefined> {
if (task && task.dependsOn) {
// In case it is an array of task dependencies
if (Array.isArray(task.dependsOn) && task.dependsOn.length > 0) {
Expand All @@ -528,15 +538,15 @@ export class TaskService implements TaskConfigurationClient {
// In case the 'dependsOrder' is 'sequence'
if (task.dependsOrder && task.dependsOrder === DependsOrder.Sequence) {
await this.runTasksGraph(dependentTask, tasks, {
customization: { ...taskCustomization, ...{ problemMatcher: resolvedMatchers } }
customization: { ...taskCustomization, ...{ problemMatcher: resolvedMatchers }, widget }
});
}
}
// In case the 'dependsOrder' is 'parallel'
if (((!task.dependsOrder) || (task.dependsOrder && task.dependsOrder === DependsOrder.Parallel))) {
const promises = dependentTasks.map(item =>
this.runTasksGraph(item.task, tasks, {
customization: { ...item.taskCustomization, ...{ problemMatcher: item.resolvedMatchers } }
customization: { ...item.taskCustomization, ...{ problemMatcher: item.resolvedMatchers }, widget }
})
);
await Promise.all(promises);
Expand All @@ -548,12 +558,12 @@ export class TaskService implements TaskConfigurationClient {
const taskCustomization = await this.getTaskCustomization(dependentTask);
const resolvedMatchers = await this.resolveProblemMatchers(dependentTask, taskCustomization);
await this.runTasksGraph(dependentTask, tasks, {
customization: { ...taskCustomization, ...{ problemMatcher: resolvedMatchers } }
customization: { ...taskCustomization, ...{ problemMatcher: resolvedMatchers }, widget }
});
}
}

const taskInfo = await this.runTask(task, option);
const taskInfo = await this.runTask(task, option, widget);
if (taskInfo) {
const getExitCodePromise: Promise<TaskEndedInfo> = this.getExitCode(taskInfo.taskId).then(result => ({ taskEndedType: TaskEndedTypes.TaskExited, value: result }));
const isBackgroundTaskEndedPromise: Promise<TaskEndedInfo> = this.isBackgroundTaskEnded(taskInfo.taskId).then(result =>
Expand Down Expand Up @@ -678,7 +688,7 @@ export class TaskService implements TaskConfigurationClient {
}
}

async runTask(task: TaskConfiguration, option?: RunTaskOption): Promise<TaskInfo | undefined> {
async runTask(task: TaskConfiguration, option?: RunTaskOption, widget?: TerminalWidget): Promise<TaskInfo | undefined> {
const runningTasksInfo: TaskInfo[] = await this.getRunningTasks();

// check if the task is active
Expand Down Expand Up @@ -706,7 +716,7 @@ export class TaskService implements TaskConfigurationClient {
return this.restartTask(matchedRunningTaskInfo, option);
}
} else { // run task as the task is not active
return this.doRunTask(task, option);
return this.doRunTask(task, option, widget);
}
}

Expand All @@ -728,7 +738,7 @@ export class TaskService implements TaskConfigurationClient {
return this.doRunTask(activeTaskInfo.config, option);
}

protected async doRunTask(task: TaskConfiguration, option?: RunTaskOption): Promise<TaskInfo | undefined> {
protected async doRunTask(task: TaskConfiguration, option?: RunTaskOption, widget?: TerminalWidget): Promise<TaskInfo | undefined> {
if (option && option.customization) {
const taskDefinition = this.taskDefinitionRegistry.getDefinition(task);
if (taskDefinition) { // use the customization object to override the task config
Expand All @@ -745,7 +755,7 @@ export class TaskService implements TaskConfigurationClient {
if (resolvedTask) {
// remove problem markers from the same source before running the task
await this.removeProblemMarks(option);
return this.runResolvedTask(resolvedTask, option);
return this.runResolvedTask(resolvedTask, option, widget);
}
}

Expand Down Expand Up @@ -893,7 +903,8 @@ export class TaskService implements TaskConfigurationClient {
* @param resolvedTask the resolved task
* @param option options to run the resolved task
*/
private async runResolvedTask(resolvedTask: TaskConfiguration, option?: RunTaskOption): Promise<TaskInfo | undefined> {
private async runResolvedTask(resolvedTask: TaskConfiguration, option?: RunTaskOption,
widget?: TerminalWidget): Promise<TaskInfo | undefined> {
const source = resolvedTask._source;
const taskLabel = resolvedTask.label;
try {
Expand All @@ -908,8 +919,9 @@ export class TaskService implements TaskConfigurationClient {
* 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);
this.attach(taskInfo.terminalId, taskInfo.taskId, widget);
}

return taskInfo;
} catch (error) {
const errorStr = `Error launching task '${taskLabel}': ${error.message}`;
Expand Down Expand Up @@ -969,32 +981,16 @@ export class TaskService implements TaskConfigurationClient {
terminal.sendText(selectedText);
}

async attach(processId: number, taskId: number): Promise<void> {
async attach(processId: number, taskId: number, widget?: TerminalWidget): 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);
// Create terminal widget to display an execution output of a task that was launched as a command inside a shell.
const widget = <TerminalWidget>await this.widgetManager.getOrCreateWidget(
TERMINAL_WIDGET_FACTORY_ID,
<TerminalWidgetFactoryOptions>{
created: new Date().toString(),
id: this.getTerminalWidgetId(processId),
title: taskInfo
? `Task: ${taskInfo.config.label}`
: `Task: #${taskId}`,
destroyTermOnClose: true
}
);
this.shell.addWidget(widget, { area: 'bottom' });
if (taskInfo && taskInfo.config.presentation && taskInfo.config.presentation.reveal === RevealKind.Always) {
if (taskInfo.config.presentation.focus) { // assign focus to the terminal if presentation.focus is true
this.shell.activateWidget(widget.id);
} else { // show the terminal but not assign focus
this.shell.revealWidget(widget.id);
}
}
widget.start(processId);

// Attach terminal to the running task
// to display an execution output of a task that was launched as a command inside a shell.
this.taskTerminalManager.attach(processId, taskId, taskInfo, this.getTerminalWidgetId(processId), widget);
}

private getTerminalWidgetId(terminalId: number): string {
Expand Down
80 changes: 80 additions & 0 deletions packages/task/src/browser/task-terminal-manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/********************************************************************************
* Copyright (C) 2020 Red Hat, Inc. and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { inject, injectable } from 'inversify';
import { TerminalWidgetFactoryOptions, TERMINAL_WIDGET_FACTORY_ID } from '@theia/terminal/lib/browser/terminal-widget-impl';
import { TerminalWidget } from '@theia/terminal/lib/browser/base/terminal-widget';
import { ApplicationShell, WidgetManager } from '@theia/core/lib/browser';
import { TaskInfo, RevealKind } from '../common';

@injectable()
export class TaskTerminalManager {

@inject(WidgetManager)
protected readonly widgetManager: WidgetManager;

@inject(ApplicationShell)
protected readonly shell: ApplicationShell;

async openEmptyTerminal(taskLabel: string): Promise<TerminalWidget> {
const id = TERMINAL_WIDGET_FACTORY_ID + '-connecting';

const widget = <TerminalWidget>await this.widgetManager.getOrCreateWidget(
TERMINAL_WIDGET_FACTORY_ID,
<TerminalWidgetFactoryOptions>{
created: new Date().toString(),
id: id,
title: taskLabel,
destroyTermOnClose: true,
loadingMessage: `Task '${taskLabel}' - Connecting...`
}
);

this.shell.addWidget(widget, { area: 'bottom' });
this.shell.revealWidget(widget.id);
return widget;
}

async attach(processId: number, taskId: number, taskInfo: TaskInfo | undefined,
terminalWidgetId: string, widget?: TerminalWidget): Promise<void> {
if (!widget) {
widget = <TerminalWidget>await this.widgetManager.getOrCreateWidget(
TERMINAL_WIDGET_FACTORY_ID,
<TerminalWidgetFactoryOptions>{
created: new Date().toString(),
id: terminalWidgetId,
title: taskInfo
? `Task: ${taskInfo.config.label}`
: `Task: #${taskId}`,
destroyTermOnClose: true
}
);

this.shell.addWidget(widget, { area: 'bottom' });
}

if (taskInfo && taskInfo.config.presentation && taskInfo.config.presentation.reveal === RevealKind.Always) {
if (taskInfo.config.presentation.focus) { // assign focus to the terminal if presentation.focus is true
this.shell.activateWidget(widget.id);
} else { // show the terminal but not assign focus
this.shell.revealWidget(widget.id);
}
}

widget.start(processId);
}

}
5 changes: 5 additions & 0 deletions packages/terminal/src/browser/base/terminal-widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,9 @@ export interface TerminalWidgetOptions {
* Terminal attributes. Can be useful to apply some implementation specific information.
*/
readonly attributes?: { [key: string]: string | null };

/**
* Initial message to be displayed while terminal is not started.
*/
readonly loadingMessage?: string;
Copy link
Member

Choose a reason for hiding this comment

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

We should test reload page reload. What would you expect should happen? We try to preserve terminals. I assume terminal should appear immediately with loading message till connection to the task process is not recovered. We need ways to test it with the vanilla Theia.

}
25 changes: 25 additions & 0 deletions packages/terminal/src/browser/terminal-widget-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget
protected waitForConnection: Deferred<MessageConnection> | undefined;
protected hoverMessage: HTMLDivElement;
protected lastTouchEnd: TouchEvent | undefined;
protected loadingMessage: HTMLDivElement;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you considering using some icon-loader for terminal tab till connection to the task process is not recovered instead of a loading message?
Like here, for example:

loader-icon


@inject(WorkspaceService) protected readonly workspaceService: WorkspaceService;
@inject(WebSocketConnectionProvider) protected readonly webSocketConnectionProvider: WebSocketConnectionProvider;
Expand Down Expand Up @@ -193,6 +194,24 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget
for (const contribution of this.terminalContributionProvider.getContributions()) {
contribution.onCreate(this);
}

if (this.options.loadingMessage) {
this.loadingMessage = document.createElement('div');
Copy link
Member

Choose a reason for hiding this comment

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

please use css, you can find css guidance in our coding guidelines

this.loadingMessage.style.zIndex = '20';
this.loadingMessage.style.display = 'block';
this.loadingMessage.style.position = 'absolute';
this.loadingMessage.style.left = '0px';
this.loadingMessage.style.right = '0px';
this.loadingMessage.style.top = '30%';
this.loadingMessage.style.textAlign = 'center';
this.loadingMessage.style.color = 'var(--theia-editorWidget-foreground)';

const text = document.createElement('pre');
text.textContent = this.options.loadingMessage;
this.loadingMessage.appendChild(text);

this.node.appendChild(this.loadingMessage);
}
}

/**
Expand Down Expand Up @@ -280,6 +299,12 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget
this.terminalId = typeof id !== 'number' ? await this.createTerminal() : await this.attachTerminal(id);
this.resizeTerminalProcess();
this.connectTerminalProcess();

// Hide loading message after starting the terminal.
if (this.loadingMessage) {
this.loadingMessage.style.display = 'none';
}

if (IBaseTerminalServer.validateId(this.terminalId)) {
this.onDidOpenEmitter.fire(undefined);
return this.terminalId;
Expand Down