From 6152160e86f588d0b225d4c97fe28f702a2a722b Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 17 May 2018 15:19:15 +0200 Subject: [PATCH] wait a little when between edits the ratio of symbols and lines changes by more than half, #49975 --- src/vs/base/common/async.ts | 18 +++++ src/vs/base/test/common/async.test.ts | 81 +++++++++++-------- .../outline/electron-browser/outlineModel.ts | 8 ++ .../outline/electron-browser/outlinePanel.ts | 54 ++++++++----- 4 files changed, 106 insertions(+), 55 deletions(-) diff --git a/src/vs/base/common/async.ts b/src/vs/base/common/async.ts index f9dcbbc5b60..288654dd344 100644 --- a/src/vs/base/common/async.ts +++ b/src/vs/base/common/async.ts @@ -73,6 +73,24 @@ export function wireCancellationToken(token: CancellationToken, promise: TPro return always(promise, () => subscription.dispose()); } +export function asDisposablePromise(input: Thenable, cancelValue?: T, bucket?: IDisposable[]): { promise: Thenable } & IDisposable { + let dispose: () => void; + let promise = new TPromise((resolve, reject) => { + dispose = function () { + resolve(cancelValue); + }; + input.then(resolve, reject); + }); + let res = { + promise, + dispose + }; + if (Array.isArray(bucket)) { + bucket.push(res); + } + return res; +} + export interface ITask { (): T; } diff --git a/src/vs/base/test/common/async.test.ts b/src/vs/base/test/common/async.test.ts index c7b0f56a989..58dfab4737e 100644 --- a/src/vs/base/test/common/async.test.ts +++ b/src/vs/base/test/common/async.test.ts @@ -5,11 +5,22 @@ 'use strict'; import * as assert from 'assert'; -import { Promise, TPromise } from 'vs/base/common/winjs.base'; +import { TPromise } from 'vs/base/common/winjs.base'; import * as Async from 'vs/base/common/async'; import URI from 'vs/base/common/uri'; suite('Async', () => { + + test('asDisposablePromise', async function () { + let value = await Async.asDisposablePromise(TPromise.as(1)).promise; + assert.equal(value, 1); + + let disposablePromise = Async.asDisposablePromise(TPromise.timeout(1000).then(_ => 1), 2); + disposablePromise.dispose(); + value = await disposablePromise.promise; + assert.equal(value, 2); + }); + test('Throttler - non async', function () { let count = 0; let factory = () => { @@ -18,7 +29,7 @@ suite('Async', () => { let throttler = new Async.Throttler(); - return Promise.join([ + return TPromise.join([ throttler.queue(factory).then((result) => { assert.equal(result, 1); }), throttler.queue(factory).then((result) => { assert.equal(result, 2); }), throttler.queue(factory).then((result) => { assert.equal(result, 3); }), @@ -37,14 +48,14 @@ suite('Async', () => { let throttler = new Async.Throttler(); - return Promise.join([ + return TPromise.join([ throttler.queue(factory).then((result) => { assert.equal(result, 1); }), throttler.queue(factory).then((result) => { assert.equal(result, 2); }), throttler.queue(factory).then((result) => { assert.equal(result, 2); }), throttler.queue(factory).then((result) => { assert.equal(result, 2); }), throttler.queue(factory).then((result) => { assert.equal(result, 2); }) ]).then(() => { - Promise.join([ + TPromise.join([ throttler.queue(factory).then((result) => { assert.equal(result, 3); }), throttler.queue(factory).then((result) => { assert.equal(result, 4); }), throttler.queue(factory).then((result) => { assert.equal(result, 4); }), @@ -63,9 +74,9 @@ suite('Async', () => { }; let throttler = new Async.Throttler(); - let p1: Promise; + let p1: TPromise; - const p = Promise.join([ + const p = TPromise.join([ p1 = throttler.queue(factory).then((result) => { assert(false, 'should not be here, 1'); }, () => { assert(true, 'yes, it was cancelled'); }), throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 2'); }), throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 3'); }), @@ -86,9 +97,9 @@ suite('Async', () => { }; let throttler = new Async.Throttler(); - let p2: Promise; + let p2: TPromise; - const p = Promise.join([ + const p = TPromise.join([ throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 1'); }), p2 = throttler.queue(factory).then((result) => { assert(false, 'should not be here, 2'); }, () => { assert(true, 'yes, it was cancelled'); }), throttler.queue(factory).then((result) => { assert.equal(result, 2); }, () => { assert(false, 'should not be here, 3'); }), @@ -109,9 +120,9 @@ suite('Async', () => { }; let throttler = new Async.Throttler(); - let p3: Promise; + let p3: TPromise; - const p = Promise.join([ + const p = TPromise.join([ throttler.queue(factory).then((result) => { assert.equal(result, 1); }, () => { assert(false, 'should not be here, 1'); }), throttler.queue(factory).then((result) => { assert.equal(result, 2); }, () => { assert(false, 'should not be here, 2'); }), p3 = throttler.queue(factory).then((result) => { assert(false, 'should not be here, 3'); }, () => { assert(true, 'yes, it was cancelled'); }), @@ -130,13 +141,13 @@ suite('Async', () => { let throttler = new Async.Throttler(); - let promises: Promise[] = []; + let promises: TPromise[] = []; promises.push(throttler.queue(factoryFactory(1)).then((n) => { assert.equal(n, 1); })); promises.push(throttler.queue(factoryFactory(2)).then((n) => { assert.equal(n, 3); })); promises.push(throttler.queue(factoryFactory(3)).then((n) => { assert.equal(n, 3); })); - return Promise.join(promises); + return TPromise.join(promises); }); test('Throttler - progress should work', function () { @@ -149,14 +160,14 @@ suite('Async', () => { }); let throttler = new Async.Throttler(); - let promises: Promise[] = []; + let promises: TPromise[] = []; let progresses: any[][] = [[], [], []]; promises.push(throttler.queue(factory).then(null, null, (p) => progresses[0].push(p))); promises.push(throttler.queue(factory).then(null, null, (p) => progresses[1].push(p))); promises.push(throttler.queue(factory).then(null, null, (p) => progresses[2].push(p))); - return Promise.join(promises).then(() => { + return TPromise.join(promises).then(() => { assert.deepEqual(progresses[0], [0]); assert.deepEqual(progresses[1], [0]); assert.deepEqual(progresses[2], [0]); @@ -170,7 +181,7 @@ suite('Async', () => { }; let delayer = new Async.Delayer(0); - let promises: Promise[] = []; + let promises: TPromise[] = []; assert(!delayer.isTriggered()); @@ -183,7 +194,7 @@ suite('Async', () => { promises.push(delayer.trigger(factory).then((result) => { assert.equal(result, 1); assert(!delayer.isTriggered()); })); assert(delayer.isTriggered()); - return Promise.join(promises).then(() => { + return TPromise.join(promises).then(() => { assert(!delayer.isTriggered()); }); }); @@ -218,7 +229,7 @@ suite('Async', () => { }; let delayer = new Async.Delayer(0); - let promises: Promise[] = []; + let promises: TPromise[] = []; assert(!delayer.isTriggered()); @@ -233,7 +244,7 @@ suite('Async', () => { delayer.cancel(); - return Promise.join(promises).then(() => { + return TPromise.join(promises).then(() => { assert(!delayer.isTriggered()); }); }); @@ -245,7 +256,7 @@ suite('Async', () => { }; let delayer = new Async.Delayer(0); - let promises: Promise[] = []; + let promises: TPromise[] = []; assert(!delayer.isTriggered()); @@ -261,7 +272,7 @@ suite('Async', () => { delayer.cancel(); - const p = Promise.join(promises).then(() => { + const p = TPromise.join(promises).then(() => { promises = []; assert(!delayer.isTriggered()); @@ -272,7 +283,7 @@ suite('Async', () => { promises.push(delayer.trigger(factory).then(() => { assert.equal(result, 1); assert(!delayer.isTriggered()); })); assert(delayer.isTriggered()); - const p = Promise.join(promises).then(() => { + const p = TPromise.join(promises).then(() => { assert(!delayer.isTriggered()); }); @@ -297,7 +308,7 @@ suite('Async', () => { }; let delayer = new Async.Delayer(0); - let promises: Promise[] = []; + let promises: TPromise[] = []; assert(!delayer.isTriggered()); @@ -305,7 +316,7 @@ suite('Async', () => { promises.push(delayer.trigger(factoryFactory(2)).then((n) => { assert.equal(n, 3); })); promises.push(delayer.trigger(factoryFactory(3)).then((n) => { assert.equal(n, 3); })); - const p = Promise.join(promises).then(() => { + const p = TPromise.join(promises).then(() => { assert(!delayer.isTriggered()); }); @@ -324,14 +335,14 @@ suite('Async', () => { }); let delayer = new Async.Delayer(0); - let promises: Promise[] = []; + let promises: TPromise[] = []; let progresses: any[][] = [[], [], []]; promises.push(delayer.trigger(factory).then(null, null, (p) => progresses[0].push(p))); promises.push(delayer.trigger(factory).then(null, null, (p) => progresses[1].push(p))); promises.push(delayer.trigger(factory).then(null, null, (p) => progresses[2].push(p))); - return Promise.join(promises).then(() => { + return TPromise.join(promises).then(() => { assert.deepEqual(progresses[0], [0]); assert.deepEqual(progresses[1], [0]); assert.deepEqual(progresses[2], [0]); @@ -348,14 +359,14 @@ suite('Async', () => { }); let delayer = new Async.ThrottledDelayer(0); - let promises: Promise[] = []; + let promises: TPromise[] = []; let progresses: any[][] = [[], [], []]; promises.push(delayer.trigger(factory).then(null, null, (p) => progresses[0].push(p))); promises.push(delayer.trigger(factory).then(null, null, (p) => progresses[1].push(p))); promises.push(delayer.trigger(factory).then(null, null, (p) => progresses[2].push(p))); - return Promise.join(promises).then(() => { + return TPromise.join(promises).then(() => { assert.deepEqual(progresses[0], [0]); assert.deepEqual(progresses[1], [0]); assert.deepEqual(progresses[2], [0]); @@ -390,10 +401,10 @@ suite('Async', () => { let limiter = new Async.Limiter(1); - let promises: Promise[] = []; + let promises: TPromise[] = []; [0, 1, 2, 3, 4, 5, 6, 7, 8, 9].forEach(n => promises.push(limiter.queue(factoryFactory(n)))); - return Promise.join(promises).then((res) => { + return TPromise.join(promises).then((res) => { assert.equal(10, res.length); limiter = new Async.Limiter(100); @@ -401,7 +412,7 @@ suite('Async', () => { promises = []; [0, 1, 2, 3, 4, 5, 6, 7, 8, 9].forEach(n => promises.push(limiter.queue(factoryFactory(n)))); - return Promise.join(promises).then((res) => { + return TPromise.join(promises).then((res) => { assert.equal(10, res.length); }); }); @@ -413,10 +424,10 @@ suite('Async', () => { }; let limiter = new Async.Limiter(1); - let promises: Promise[] = []; + let promises: TPromise[] = []; [0, 1, 2, 3, 4, 5, 6, 7, 8, 9].forEach(n => promises.push(limiter.queue(factoryFactory(n)))); - return Promise.join(promises).then((res) => { + return TPromise.join(promises).then((res) => { assert.equal(10, res.length); limiter = new Async.Limiter(100); @@ -424,7 +435,7 @@ suite('Async', () => { promises = []; [0, 1, 2, 3, 4, 5, 6, 7, 8, 9].forEach(n => promises.push(limiter.queue(factoryFactory(n)))); - return Promise.join(promises).then((res) => { + return TPromise.join(promises).then((res) => { assert.equal(10, res.length); }); }); @@ -440,10 +451,10 @@ suite('Async', () => { let limiter = new Async.Limiter(5); - let promises: Promise[] = []; + let promises: TPromise[] = []; [0, 1, 2, 3, 4, 5, 6, 7, 8, 9].forEach(n => promises.push(limiter.queue(factoryFactory(n)))); - return Promise.join(promises).then((res) => { + return TPromise.join(promises).then((res) => { assert.equal(10, res.length); assert.deepEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9], res); }); diff --git a/src/vs/workbench/parts/outline/electron-browser/outlineModel.ts b/src/vs/workbench/parts/outline/electron-browser/outlineModel.ts index 102ce9b0616..80adc3d19e7 100644 --- a/src/vs/workbench/parts/outline/electron-browser/outlineModel.ts +++ b/src/vs/workbench/parts/outline/electron-browser/outlineModel.ts @@ -43,6 +43,14 @@ export abstract class TreeElement { } return undefined; } + + static size(element: TreeElement): number { + let res = 1; + for (const key in element.children) { + res += TreeElement.size(element.children[key]); + } + return res; + } } export class OutlineElement extends TreeElement { diff --git a/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts b/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts index c4ea8d73809..ca5c89ebda3 100644 --- a/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts +++ b/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts @@ -37,11 +37,13 @@ import { IViewOptions, ViewsViewletPanel } from 'vs/workbench/browser/parts/view import { CollapseAction } from 'vs/workbench/browser/viewlet'; import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IEditorGroupService } from 'vs/workbench/services/group/common/groupService'; -import { OutlineElement, OutlineModel } from './outlineModel'; +import { OutlineElement, OutlineModel, TreeElement } from './outlineModel'; import { OutlineController, OutlineDataSource, OutlineItemComparator, OutlineItemCompareType, OutlineItemFilter, OutlineRenderer, OutlineTreeState } from './outlineTree'; import { StandardMouseEvent } from 'vs/base/browser/mouseEvent'; import { KeyboardMapperFactory } from 'vs/workbench/services/keybinding/electron-browser/keybindingService'; -import { isPromiseCanceledError, onUnexpectedError } from 'vs/base/common/errors'; +import { onUnexpectedError } from 'vs/base/common/errors'; +import { IModelContentChangedEvent } from 'vs/editor/common/model/textModelEvents'; +import { asDisposablePromise } from 'vs/base/common/async'; class RequestState { @@ -70,7 +72,7 @@ class RequestOracle { private _lastState: RequestState; constructor( - private readonly _callback: (editor: ICodeEditor) => any, + private readonly _callback: (editor: ICodeEditor, change: IModelContentChangedEvent) => any, private readonly _featureRegistry: LanguageFeatureRegistry, @IEditorGroupService editorGroupService: IEditorGroupService, @IWorkbenchEditorService private readonly _workbenchEditorService: IWorkbenchEditorService, @@ -97,7 +99,7 @@ class RequestOracle { } if (!codeEditor || !codeEditor.getModel()) { - this._callback(undefined); + this._callback(undefined, undefined); return; } @@ -114,11 +116,11 @@ class RequestOracle { } dispose(this._sessionDisposable); this._lastState = thisState; - this._callback(codeEditor); + this._callback(codeEditor, undefined); let handle: number; - let listener = codeEditor.onDidChangeModelContent(_ => { - handle = setTimeout(() => this._callback(codeEditor), 150); + let listener = codeEditor.onDidChangeModelContent(event => { + handle = setTimeout(() => this._callback(codeEditor, event), 150); }); this._sessionDisposable = { dispose() { @@ -312,11 +314,11 @@ export class OutlinePanel extends ViewsViewletPanel { setVisible(visible: boolean): TPromise { if (visible) { - this._requestOracle = this._requestOracle || this._instantiationService.createInstance(RequestOracle, editor => this._onEditor(editor), DocumentSymbolProviderRegistry); + this._requestOracle = this._requestOracle || this._instantiationService.createInstance(RequestOracle, (editor, event) => this._doUpdate(editor, event).then(undefined, onUnexpectedError), DocumentSymbolProviderRegistry); } else { dispose(this._requestOracle); this._requestOracle = undefined; - this._onEditor(undefined); + this._doUpdate(undefined, undefined); } return super.setVisible(visible); } @@ -362,7 +364,7 @@ export class OutlinePanel extends ViewsViewletPanel { this._message.innerText = escape(message); } - private async _onEditor(editor: ICodeEditor): TPromise { + private async _doUpdate(editor: ICodeEditor, event: IModelContentChangedEvent): TPromise { dispose(this._editorDisposables); this._editorDisposables = new Array(); @@ -376,21 +378,33 @@ export class OutlinePanel extends ViewsViewletPanel { dom.removeClass(this._domNode, 'message'); let textModel = editor.getModel(); - let modelPromise = OutlineModel.create(textModel); - this._editorDisposables.push({ dispose() { modelPromise.cancel(); } }); - - let model: OutlineModel; - try { - model = await modelPromise; - } catch (e) { - if (!isPromiseCanceledError(e)) { - onUnexpectedError(e); - } + let model = await asDisposablePromise(OutlineModel.create(textModel), undefined, this._editorDisposables).promise; + if (!model) { return; } let oldModel = this._tree.getInput(); + if (event && oldModel) { + // heuristic: when the symbols-to-lines ratio changes by 50% between edits + // wait a little (and hope that the next change isn't as drastic). + let newSize = TreeElement.size(model); + let newLength = textModel.getValueLength(); + let newRatio = newSize / newLength; + let oldSize = TreeElement.size(oldModel); + let oldLength = newLength - event.changes.reduce((prev, value) => prev + value.rangeLength, 0); + let oldRatio = oldSize / oldLength; + if (newRatio <= oldRatio * 0.5 || newRatio >= oldRatio * 1.5) { + if (!await asDisposablePromise( + TPromise.timeout(2000).then(_ => true), + false, + this._editorDisposables).promise + ) { + return; + } + } + } + if (oldModel && oldModel.adopt(model)) { this._tree.refresh(undefined, true); model = oldModel; -- GitLab