Skip to content

Commit

Permalink
[vscode] fix #6929, fix #5764: handle quick pick/input cancellation a…
Browse files Browse the repository at this point in the history 
…nd closing properly

- resolve callback was never called if a user closed the widget
- token was never passed to the main side, so the widget was not hidden if an extension closes it

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
  • Loading branch information
@akosyakov
akosyakov committed Jan 23, 2020
1 parent 95068f9 commit f3d9b50
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 110 deletions.
19 changes: 12 additions & 7 deletions packages/core/src/browser/quick-open/quick-input-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { MessageType } from '../../common/message-service-protocol';
import { Emitter, Event } from '../../common/event';
import { QuickTitleBar } from './quick-title-bar';
import { QuickTitleButton } from '../../common/quick-open-model';
import { CancellationToken } from '../../common/cancellation';

export interface QuickInputOptions {

Expand Down Expand Up @@ -113,14 +114,22 @@ export class QuickInputService {
@inject(QuickTitleBar)
protected readonly quickTitleBar: QuickTitleBar;

open(options: QuickInputOptions): Promise<string | undefined> {
open(options: QuickInputOptions, token: CancellationToken = CancellationToken.None): Promise<string | undefined> {
const result = new Deferred<string | undefined>();
const prompt = this.createPrompt(options.prompt);
let label = prompt;
let currentText = '';
const validateInput = options && options.validateInput;
let initial: boolean = true;

const toDispose = token.onCancellationRequested(() =>
this.quickOpenService.hide()
);
const resolve = (value: string | undefined) => {
toDispose.dispose();
result.resolve(value);
this.quickTitleBar.hide();
};
this.quickOpenService.open({
onType: async (lookFor, acceptor) => {
let error: string | undefined;
Expand All @@ -140,9 +149,8 @@ export class QuickInputService {
label,
run: mode => {
if (!error && mode === QuickOpenMode.OPEN) {
result.resolve(currentText);
this.onDidAcceptEmitter.fire(undefined);
this.quickTitleBar.hide();
resolve(currentText);
return true;
}
return false;
Expand All @@ -157,10 +165,7 @@ export class QuickInputService {
ignoreFocusOut: options.ignoreFocusOut,
enabled: options.enabled,
valueSelection: options.valueSelection,
onClose: () => {
result.resolve(undefined);
this.quickTitleBar.hide();
}
onClose: () => resolve(undefined)
});

if (options && this.quickTitleBar.shouldShowTitleBar(options.title, options.step)) {
Expand Down
46 changes: 0 additions & 46 deletions packages/plugin-ext/src/common/async-util.ts
View file

This file was deleted.

5 changes: 2 additions & 3 deletions packages/plugin-ext/src/common/plugin-api-rpc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,9 @@ export interface TransferQuickPick<T extends theia.QuickPickItem> extends Transf
}

export interface QuickOpenMain {
$show(options: PickOptions): Promise<number | number[]>;
$show(options: PickOptions, token: CancellationToken): Promise<number | number[]>;
$setItems(items: PickOpenItem[]): Promise<any>;
$setError(error: Error): Promise<any>;
$input(options: theia.InputBoxOptions, validateInput: boolean): Promise<string | undefined>;
$input(options: theia.InputBoxOptions, validateInput: boolean, token: CancellationToken): Promise<string | undefined>;
$hide(): void;
$showInputBox(inputBox: TransferInputBox, validateInput: boolean): void;
$showCustomQuickPick<T extends theia.QuickPickItem>(inputBox: TransferQuickPick<T>): void;
Expand Down
44 changes: 24 additions & 20 deletions packages/plugin-ext/src/main/browser/quick-open-main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { QuickPickService, QuickPickItem, QuickPickValue } from '@theia/core/lib
import { QuickTitleBar } from '@theia/core/lib/browser/quick-open/quick-title-bar';
import { DisposableCollection, Disposable } from '@theia/core/lib/common/disposable';
import { QuickTitleButtonSide, QuickOpenGroupItem } from '@theia/core/lib/common/quick-open-model';
import { CancellationToken } from '@theia/core/lib/common/cancellation';

export class QuickOpenMainImpl implements QuickOpenMain, QuickOpenModel, Disposable {

Expand Down Expand Up @@ -84,21 +85,29 @@ export class QuickOpenMainImpl implements QuickOpenMain, QuickOpenModel, Disposa
this.activeElement = undefined;
}

$show(options: PickOptions): Promise<number | number[]> {
this.activeElement = window.document.activeElement as HTMLElement;
this.delegate.open(this, {
fuzzyMatchDescription: options.matchOnDescription,
fuzzyMatchLabel: true,
fuzzyMatchDetail: options.matchOnDetail,
placeholder: options.placeHolder,
ignoreFocusOut: options.ignoreFocusLost,
onClose: () => {
this.cleanUp();
}
});

$show(options: PickOptions, token: CancellationToken): Promise<number | number[]> {
return new Promise((resolve, reject) => {
if (token.isCancellationRequested) {
resolve(undefined);
return;
}
this.doResolve = resolve;
this.activeElement = window.document.activeElement as HTMLElement;
const toDispose = token.onCancellationRequested(() =>
this.delegate.hide()
);
this.delegate.open(this, {
fuzzyMatchDescription: options.matchOnDescription,
fuzzyMatchLabel: true,
fuzzyMatchDetail: options.matchOnDetail,
placeholder: options.placeHolder,
ignoreFocusOut: options.ignoreFocusLost,
onClose: () => {
this.doResolve(undefined);
toDispose.dispose();
this.cleanUp();
}
});
});
}

Expand Down Expand Up @@ -153,17 +162,12 @@ export class QuickOpenMainImpl implements QuickOpenMain, QuickOpenModel, Disposa
return convertedItems;
}

// tslint:disable-next-line:no-any
$setError(error: Error): Promise<any> {
throw new Error('Method not implemented.');
}

$input(options: InputBoxOptions, validateInput: boolean): Promise<string | undefined> {
$input(options: InputBoxOptions, validateInput: boolean, token: CancellationToken): Promise<string | undefined> {
if (validateInput) {
options.validateInput = val => this.proxy.$validateInput(val);
}

return this.quickInput.open(options);
return this.quickInput.open(options, token);
}

protected convertQuickInputButton(quickInputButton: QuickInputButton, index: number, toDispose: DisposableCollection): QuickInputTitleButtonHandle {
Expand Down
16 changes: 2 additions & 14 deletions packages/plugin-ext/src/plugin/plugin-context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,7 @@ export function createAPIFactory(
},
// tslint:disable-next-line:no-any
showQuickPick(items: any, options: theia.QuickPickOptions, token?: theia.CancellationToken): any {
if (token) {
const coreEvent = Object.assign(token.onCancellationRequested, { maxListeners: 0 });
const coreCancellationToken = { isCancellationRequested: token.isCancellationRequested, onCancellationRequested: coreEvent };
return quickOpenExt.showQuickPick(items, options, coreCancellationToken);
} else {
return quickOpenExt.showQuickPick(items, options);
}
return quickOpenExt.showQuickPick(items, options, token);
},
createQuickPick<T extends QuickPickItem>(): QuickPick<T> {
return quickOpenExt.createQuickPick(plugin);
Expand All @@ -327,13 +321,7 @@ export function createAPIFactory(
return statusBarMessageRegistryExt.setStatusBarMessage(text, arg);
},
showInputBox(options?: theia.InputBoxOptions, token?: theia.CancellationToken): PromiseLike<string | undefined> {
if (token) {
const coreEvent = Object.assign(token.onCancellationRequested, { maxListeners: 0 });
const coreCancellationToken = { isCancellationRequested: token.isCancellationRequested, onCancellationRequested: coreEvent };
return quickOpenExt.showInput(options, coreCancellationToken);
} else {
return quickOpenExt.showInput(options);
}
return quickOpenExt.showInput(options, token);
},
createStatusBarItem(alignment?: theia.StatusBarAlignment, priority?: number): theia.StatusBarItem {
return statusBarMessageRegistryExt.createStatusBarItem(alignment, priority);
Expand Down
39 changes: 19 additions & 20 deletions packages/plugin-ext/src/plugin/quick-open.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { QuickOpenExt, PLUGIN_RPC_CONTEXT as Ext, QuickOpenMain, TransferInputBox, Plugin, TransferQuickPick, QuickInputTitleButtonHandle } from '../common/plugin-api-rpc';
import * as theia from '@theia/plugin';
import { QuickPickOptions, QuickPickItem, InputBoxOptions, InputBox, QuickPick, QuickInput } from '@theia/plugin';
import { CancellationToken } from '@theia/core/lib/common/cancellation';
import { RPCProtocol } from '../common/rpc-protocol';
import { anyPromise } from '../common/async-util';
import { hookCancellationToken } from '../common/async-util';
import { Emitter, Event } from '@theia/core/lib/common/event';
import { DisposableCollection } from '@theia/core/lib/common/disposable';
import { QuickInputButtons, QuickInputButton, ThemeIcon } from './types-impl';
Expand Down Expand Up @@ -53,28 +52,33 @@ export class QuickOpenExtImpl implements QuickOpenExt {
return undefined;
}

showQuickPick(promiseOrItems: QuickPickItem[] | PromiseLike<QuickPickItem[]>, options?: QuickPickOptions, token?: CancellationToken): PromiseLike<QuickPickItem | undefined>;
// tslint:disable-next-line:max-line-length
showQuickPick(promiseOrItems: QuickPickItem[] | PromiseLike<QuickPickItem[]>, options?: QuickPickOptions & { canSelectMany: true; }, token?: CancellationToken): PromiseLike<QuickPickItem[] | undefined>;
showQuickPick(promiseOrItems: string[] | PromiseLike<string[]>, options?: QuickPickOptions, token?: CancellationToken): PromiseLike<string | undefined>;
// tslint:disable-next-line:max-line-length
showQuickPick(promiseOrItems: Item[] | PromiseLike<Item[]>, options?: QuickPickOptions, token: CancellationToken = CancellationToken.None): PromiseLike<Item | Item[] | undefined> {
// tslint:disable:max-line-length
showQuickPick(promiseOrItems: QuickPickItem[] | PromiseLike<QuickPickItem[]>, options?: QuickPickOptions, token?: theia.CancellationToken): PromiseLike<QuickPickItem | undefined>;
showQuickPick(promiseOrItems: QuickPickItem[] | PromiseLike<QuickPickItem[]>, options?: QuickPickOptions & { canSelectMany: true; }, token?: theia.CancellationToken): PromiseLike<QuickPickItem[] | undefined>;
showQuickPick(promiseOrItems: string[] | PromiseLike<string[]>, options?: QuickPickOptions, token?: theia.CancellationToken): PromiseLike<string | undefined>;
showQuickPick(itemsOrItemsPromise: Item[] | PromiseLike<Item[]>, options?: QuickPickOptions, token: theia.CancellationToken = CancellationToken.None): PromiseLike<Item | Item[] | undefined> {
// tslint:enable:max-line-length
this.selectItemHandler = undefined;
const itemPromise = Promise.resolve(promiseOrItems);

const itemsPromise = <Promise<Item[]>>Promise.resolve(itemsOrItemsPromise);

const widgetPromise = this.proxy.$show({
canSelectMany: options && options.canPickMany,
placeHolder: options && options.placeHolder,
autoFocus: { autoFocusFirstEntry: true },
matchOnDescription: options && options.matchOnDescription,
matchOnDetail: options && options.matchOnDetail,
ignoreFocusLost: options && options.ignoreFocusOut
});
}, token);

const widgetClosedMarker = {};
const widgetClosedPromise = widgetPromise.then(() => widgetClosedMarker);

const promise = anyPromise(<PromiseLike<number | Item[]>[]>[widgetPromise, itemPromise]).then(values => {
if (values.key === 0) {
return Promise.race([widgetClosedPromise, itemsPromise]).then(result => {
if (result === widgetClosedMarker) {
return undefined;
}
return itemPromise.then(items => {
return itemsPromise.then(items => {

const pickItems = quickPickItemToPickOpenItem(items);

Expand All @@ -99,12 +103,8 @@ export class QuickOpenExtImpl implements QuickOpenExt {
}
return undefined;
});
}, err => {
this.proxy.$setError(err);
return Promise.reject(err);
});
});
return hookCancellationToken<Item | Item[] | undefined>(token, promise);
}

showCustomQuickPick<T extends QuickPickItem>(options: TransferQuickPick<T>): void {
Expand All @@ -118,7 +118,7 @@ export class QuickOpenExtImpl implements QuickOpenExt {
return newQuickInput;
}

showInput(options?: InputBoxOptions, token: CancellationToken = CancellationToken.None): PromiseLike<string | undefined> {
showInput(options?: InputBoxOptions, token: theia.CancellationToken = CancellationToken.None): PromiseLike<string | undefined> {
this.validateInputHandler = options && options.validateInput;

if (!options) {
Expand All @@ -127,8 +127,7 @@ export class QuickOpenExtImpl implements QuickOpenExt {
};
}

const promise = this.proxy.$input(options, typeof this.validateInputHandler === 'function');
return hookCancellationToken(token, promise);
return this.proxy.$input(options, typeof this.validateInputHandler === 'function', token);
}

hide(): void {
Expand Down

0 comments on commit f3d9b50

Please sign in to comment.