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

Exposed preserveFocus on OutputChannel#show. #8243

Merged
merged 1 commit into from
Jul 30, 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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Change Log

## v1.5.0

<a name="breaking_changes_1.5.0">[Breaking Changes:](#breaking_changes_1.5.0)</a>

- [output] `OutputWidget#setInput` has been removed. The _Output_ view automatically shows the channel when calling `OutputChannel#show`. Moved the `OutputCommands` namespace from the `output-contribution` to its dedicated `output-commands` module to overcome a DI cycle. [#8243](https://github.com/eclipse-theia/theia/pull/8243)


## v1.4.0

- [core] added support for Node.js `12.x` [#7968](https://github.com/eclipse-theia/theia/pull/7968)
Expand Down
98 changes: 98 additions & 0 deletions packages/output/src/browser/output-commands.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/********************************************************************************
* Copyright (C) 2020 TypeFox 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 { Command } from '@theia/core/lib/common';

export namespace OutputCommands {

const OUTPUT_CATEGORY = 'Output';

/* #region VS Code `OutputChannel` API */
// Based on: https://github.com/theia-ide/vscode/blob/standalone/0.19.x/src/vs/vscode.d.ts#L4692-L4745

export const APPEND: Command = {
id: 'output:append'
};

export const APPEND_LINE: Command = {
id: 'output:appendLine'
};

export const CLEAR: Command = {
id: 'output:clear'
};

export const SHOW: Command = {
id: 'output:show'
};

export const HIDE: Command = {
id: 'output:hide'
};

export const DISPOSE: Command = {
id: 'output:dispose'
};

/* #endregion VS Code `OutputChannel` API */

export const CLEAR__WIDGET: Command = {
id: 'output:widget:clear',
category: OUTPUT_CATEGORY,
iconClass: 'clear-all'
};

export const LOCK__WIDGET: Command = {
id: 'output:widget:lock',
category: OUTPUT_CATEGORY,
iconClass: 'fa fa-unlock'
};

export const UNLOCK__WIDGET: Command = {
id: 'output:widget:unlock',
category: OUTPUT_CATEGORY,
iconClass: 'fa fa-lock'
};

export const CLEAR__QUICK_PICK: Command = {
id: 'output:pick-clear',
label: 'Clear Output Channel...',
category: OUTPUT_CATEGORY
};

export const SHOW__QUICK_PICK: Command = {
id: 'output:pick-show',
label: 'Show Output Channel...',
category: OUTPUT_CATEGORY
};

export const HIDE__QUICK_PICK: Command = {
id: 'output:pick-hide',
label: 'Hide Output Channel...',
category: OUTPUT_CATEGORY
};

export const DISPOSE__QUICK_PICK: Command = {
id: 'output:pick-dispose',
label: 'Close Output Channel...',
category: OUTPUT_CATEGORY
};

export const COPY_ALL: Command = {
id: 'output:copy-all',
};

}
104 changes: 20 additions & 84 deletions packages/output/src/browser/output-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,104 +14,35 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { injectable, inject } from 'inversify';
import { injectable, inject, postConstruct } from 'inversify';
import URI from '@theia/core/lib/common/uri';
import { Widget } from '@theia/core/lib/browser/widgets/widget';
import { MaybePromise } from '@theia/core/lib/common/types';
import { CommonCommands, quickCommand, OpenHandler, OpenerOptions } from '@theia/core/lib/browser';
import { Command, CommandRegistry, MenuModelRegistry } from '@theia/core/lib/common';
import { CommonCommands, quickCommand, OpenHandler, open, OpenerOptions, OpenerService } from '@theia/core/lib/browser';
import { CommandRegistry, MenuModelRegistry, CommandService } from '@theia/core/lib/common';
import { AbstractViewContribution } from '@theia/core/lib/browser/shell/view-contribution';
import { OutputWidget } from './output-widget';
import { OutputContextMenu } from './output-context-menu';
import { OutputUri } from '../common/output-uri';
import { ClipboardService } from '@theia/core/lib/browser/clipboard-service';

export namespace OutputCommands {

const OUTPUT_CATEGORY = 'Output';

/* #region VS Code `OutputChannel` API */
// Based on: https://github.com/theia-ide/vscode/blob/standalone/0.19.x/src/vs/vscode.d.ts#L4692-L4745

export const APPEND: Command = {
id: 'output:append'
};

export const APPEND_LINE: Command = {
id: 'output:appendLine'
};

export const CLEAR: Command = {
id: 'output:clear'
};

export const SHOW: Command = {
id: 'output:show'
};

export const HIDE: Command = {
id: 'output:hide'
};

export const DISPOSE: Command = {
id: 'output:dispose'
};

/* #endregion VS Code `OutputChannel` API */

export const CLEAR__WIDGET: Command = {
id: 'output:widget:clear',
category: OUTPUT_CATEGORY,
iconClass: 'clear-all'
};

export const LOCK__WIDGET: Command = {
id: 'output:widget:lock',
category: OUTPUT_CATEGORY,
iconClass: 'fa fa-unlock'
};

export const UNLOCK__WIDGET: Command = {
id: 'output:widget:unlock',
category: OUTPUT_CATEGORY,
iconClass: 'fa fa-lock'
};

export const CLEAR__QUICK_PICK: Command = {
id: 'output:pick-clear',
label: 'Clear Output Channel...',
category: OUTPUT_CATEGORY
};

export const SHOW__QUICK_PICK: Command = {
id: 'output:pick-show',
label: 'Show Output Channel...',
category: OUTPUT_CATEGORY
};

export const HIDE__QUICK_PICK: Command = {
id: 'output:pick-hide',
label: 'Hide Output Channel...',
category: OUTPUT_CATEGORY
};

export const DISPOSE__QUICK_PICK: Command = {
id: 'output:pick-dispose',
label: 'Close Output Channel...',
category: OUTPUT_CATEGORY
};

export const COPY_ALL: Command = {
id: 'output:copy-all',
};
}
import { OutputChannelManager } from '../common/output-channel';
import { OutputCommands } from './output-commands';

@injectable()
export class OutputContribution extends AbstractViewContribution<OutputWidget> implements OpenHandler {

@inject(ClipboardService)
protected readonly clipboardService: ClipboardService;

@inject(CommandService)
protected readonly commandService: CommandService;

@inject(OutputChannelManager)
protected readonly outputChannelManager: OutputChannelManager;

@inject(OpenerService)
protected readonly openerService: OpenerService;

readonly id: string = `${OutputWidget.ID}-opener`;

constructor() {
Expand All @@ -126,6 +57,12 @@ export class OutputContribution extends AbstractViewContribution<OutputWidget> i
});
}

@postConstruct()
protected init(): void {
this.outputChannelManager.onChannelWasShown(({ name, preserveFocus }) =>
open(this.openerService, OutputUri.create(name), { activate: !preserveFocus, reveal: true }));
}

registerCommands(registry: CommandRegistry): void {
super.registerCommands(registry);
registry.registerCommand(OutputCommands.CLEAR__WIDGET, {
Expand Down Expand Up @@ -204,7 +141,6 @@ export class OutputContribution extends AbstractViewContribution<OutputWidget> i
throw new Error(`Expected '${OutputUri.SCHEME}' URI scheme. Got: ${uri} instead.`);
}
const widget = await this.openView(options);
widget.setInput(OutputUri.channelName(uri));
return widget;
}

Expand Down
5 changes: 3 additions & 2 deletions packages/output/src/browser/output-toolbar-contribution.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import { inject, injectable, postConstruct } from 'inversify';
import { Emitter } from '@theia/core/lib/common/event';
import { TabBarToolbarContribution, TabBarToolbarRegistry } from '@theia/core/lib/browser/shell/tab-bar-toolbar';
import { OutputWidget } from './output-widget';
import { OutputCommands, OutputContribution } from './output-contribution';
import { OutputCommands } from './output-commands';
import { OutputContribution } from './output-contribution';
import { OutputChannelManager } from '../common/output-channel';

@injectable()
Expand Down Expand Up @@ -103,7 +104,7 @@ export class OutputToolbarContribution implements TabBarToolbarContribution {
protected changeChannel = (event: React.ChangeEvent<HTMLSelectElement>) => {
const channelName = event.target.value;
if (channelName !== this.NONE) {
this.outputChannelManager.selectedChannel = this.outputChannelManager.getChannel(channelName);
this.outputChannelManager.getChannel(channelName).show();
}
};
}
49 changes: 25 additions & 24 deletions packages/output/src/browser/output-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ export class OutputWidget extends BaseWidget implements StatefulWidget {
@postConstruct()
protected init(): void {
this.toDispose.pushAll([
this.outputChannelManager.onSelectedChannelChanged(this.refreshEditorWidget.bind(this)),
this.outputChannelManager.onChannelWasHidden(() => this.refreshEditorWidget()),
this.outputChannelManager.onChannelWasShown(({ preserveFocus }) => this.refreshEditorWidget({ preserveFocus: !!preserveFocus })),
this.toDisposeOnSelectedChannelChanged,
this.onStateChangedEmitter,
this.onStateChanged(() => this.update())
Expand Down Expand Up @@ -93,13 +94,16 @@ export class OutputWidget extends BaseWidget implements StatefulWidget {
this.onStateChangedEmitter.fire(this._state);
}

protected async refreshEditorWidget(): Promise<void> {
protected async refreshEditorWidget({ preserveFocus }: { preserveFocus: boolean } = { preserveFocus: false }): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

why not have boolean argument instead of object with single boolean property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can easily extend it without breaking the API in the future; it's pretty common that we use an object instead of a single property. I can make the change, though. I am OK with both.

Copy link
Member

Choose a reason for hiding this comment

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

i'm ok with both if we think it might expand in the future. consider having a named interface so it is more readable.

const { selectedChannel } = this;
const editor = this.editor;
if (selectedChannel && editor) {
const editorWidget = this.editorWidget;
if (selectedChannel && editorWidget) {
// If the input is the current one, do nothing.
const model = editor.getControl().getModel();
const model = (editorWidget.editor as MonacoEditor).getControl().getModel();
if (model && model.uri.toString() === selectedChannel.uri.toString()) {
if (!preserveFocus) {
this.activate();
}
return;
}
}
Expand All @@ -112,7 +116,9 @@ export class OutputWidget extends BaseWidget implements StatefulWidget {
Disposable.create(() => widget.close()),
selectedChannel.onContentChange(() => this.revealLastLine())
]);
MessageLoop.sendMessage(widget, Widget.Msg.ActivateRequest);
if (!preserveFocus) {
this.activate();
}
this.revealLastLine();
}
}
Expand All @@ -126,10 +132,8 @@ export class OutputWidget extends BaseWidget implements StatefulWidget {

protected onActivateRequest(message: Message): void {
super.onActivateRequest(message);
if (this.selectedChannel) {
for (const widget of toArray(this.editorContainer.widgets())) {
MessageLoop.sendMessage(widget, Widget.Msg.ActivateRequest);
}
if (this.editor) {
this.editor.focus();
} else {
this.node.focus();
}
Expand Down Expand Up @@ -177,10 +181,6 @@ export class OutputWidget extends BaseWidget implements StatefulWidget {
return !!this.state.locked;
}

setInput(channelName: string): void {
this.outputChannelManager.getChannel(channelName).setVisibility(true);
}

protected revealLastLine(): void {
if (this.isLocked) {
return;
Expand Down Expand Up @@ -209,28 +209,29 @@ export class OutputWidget extends BaseWidget implements StatefulWidget {
return new EditorWidget(editor, this.selectionService);
}

private get editor(): MonacoEditor | undefined {
private get editorWidget(): EditorWidget | undefined {
for (const widget of toArray(this.editorContainer.children())) {
if (widget instanceof EditorWidget) {
if (widget.editor instanceof MonacoEditor) {
return widget.editor;
}
return widget;
}
}
return undefined;
}

getText(): string | undefined {
const editor = this.editor;
if (editor) {
const model = editor.getControl().getModel();
if (model) {
return model.getValue();
private get editor(): MonacoEditor | undefined {
const widget = this.editorWidget;
if (widget instanceof EditorWidget) {
if (widget.editor instanceof MonacoEditor) {
return widget.editor;
}
}
return undefined;
}

getText(): string | undefined {
return this.editor?.getControl().getModel()?.getValue();
}

}

export namespace OutputWidget {
Expand Down
Loading