diff --git a/src/vs/editor/common/services/editorSimpleWorker.ts b/src/vs/editor/common/services/editorSimpleWorker.ts index b7302571032..046b0f32902 100644 --- a/src/vs/editor/common/services/editorSimpleWorker.ts +++ b/src/vs/editor/common/services/editorSimpleWorker.ts @@ -22,6 +22,7 @@ import { getWordAtText, ensureValidWordDefinition } from 'vs/editor/common/model import { createMonacoBaseAPI } from 'vs/editor/common/standalone/standaloneBase'; import { IWordAtPosition, EndOfLineSequence } from 'vs/editor/common/model'; import { globals } from 'vs/base/common/platform'; +import { IIterator } from 'vs/base/common/iterator'; export interface IMirrorModel { readonly uri: URI; @@ -58,8 +59,8 @@ export interface ICommonModel { getLinesContent(): string[]; getLineCount(): number; getLineContent(lineNumber: number): string; + createWordIterator(wordDefinition: RegExp): IIterator; getWordUntilPosition(position: IPosition, wordDefinition: RegExp): IWordAtPosition; - getAllUniqueWords(wordDefinition: RegExp, skipWordOnce?: string): string[]; getValueInRange(range: IRange): string; getWordAtPosition(position: IPosition, wordDefinition: RegExp): Range; offsetAt(position: IPosition): number; @@ -146,30 +147,37 @@ class MirrorModel extends BaseMirrorModel implements ICommonModel { }; } - private _getAllWords(wordDefinition: RegExp): string[] { - let result: string[] = []; - this._lines.forEach((line) => { - this._wordenize(line, wordDefinition).forEach((info) => { - result.push(line.substring(info.start, info.end)); - }); - }); - return result; - } + public createWordIterator(wordDefinition: RegExp): IIterator { + let obj = { + done: false, + value: '' + }; + let lineNumber = 0; + let lineText: string; + let wordRangesIdx = 0; + let wordRanges: IWordRange[] = []; + let next = () => { + + if (wordRangesIdx < wordRanges.length) { + obj.done = false; + obj.value = lineText.substring(wordRanges[wordRangesIdx].start, wordRanges[wordRangesIdx].end); + wordRangesIdx += 1; + + } else if (lineNumber >= this._lines.length) { + obj.done = true; + obj.value = undefined; - public getAllUniqueWords(wordDefinition: RegExp, skipWordOnce?: string): string[] { - let foundSkipWord = false; - let uniqueWords = Object.create(null); - return this._getAllWords(wordDefinition).filter((word) => { - if (skipWordOnce && !foundSkipWord && skipWordOnce === word) { - foundSkipWord = true; - return false; - } else if (uniqueWords[word]) { - return false; } else { - uniqueWords[word] = true; - return true; + lineText = this._lines[lineNumber]; + wordRanges = this._wordenize(lineText, wordDefinition); + wordRangesIdx = 0; + lineNumber += 1; + return next(); } - }); + + return obj; + }; + return { next }; } private _wordenize(content: string, wordDefinition: RegExp): IWordRange[] { @@ -427,6 +435,8 @@ export abstract class BaseEditorSimpleWorker { // ---- BEGIN suggest -------------------------------------------------------------------------- + private static readonly _suggestionsLimit = 10000; + public textualSuggest(modelUrl: string, position: IPosition, wordDef: string, wordDefFlags: string): TPromise { const model = this._getModel(modelUrl); if (model) { @@ -434,17 +444,32 @@ export abstract class BaseEditorSimpleWorker { const wordDefRegExp = new RegExp(wordDef, wordDefFlags); const currentWord = model.getWordUntilPosition(position, wordDefRegExp).word; - for (const word of model.getAllUniqueWords(wordDefRegExp)) { - if (word !== currentWord && isNaN(Number(word))) { - suggestions.push({ - type: 'text', - label: word, - insertText: word, - noAutoAccept: true, - overwriteBefore: currentWord.length - }); + const seen: Record = Object.create(null); + seen[currentWord] = true; + + for ( + let iter = model.createWordIterator(wordDefRegExp), e = iter.next(); + !e.done && suggestions.length <= BaseEditorSimpleWorker._suggestionsLimit; + e = iter.next() + ) { + const word = e.value; + if (seen[word]) { + continue; } + seen[word] = true; + if (!isNaN(Number(word))) { + continue; + } + + suggestions.push({ + type: 'text', + label: word, + insertText: word, + noAutoAccept: true, + overwriteBefore: currentWord.length + }); } + return TPromise.as({ suggestions }); } return undefined; diff --git a/src/vs/editor/contrib/quickFix/quickFixWidget.ts b/src/vs/editor/contrib/quickFix/quickFixWidget.ts index 1fd65fa824b..aa223cc3cda 100644 --- a/src/vs/editor/contrib/quickFix/quickFixWidget.ts +++ b/src/vs/editor/contrib/quickFix/quickFixWidget.ts @@ -15,6 +15,7 @@ import { IContextMenuService } from 'vs/platform/contextview/browser/contextView import { Action } from 'vs/base/common/actions'; import { Event, Emitter } from 'vs/base/common/event'; import { ScrollType } from 'vs/editor/common/editorCommon'; +import { canceled } from 'vs/base/common/errors'; export class QuickFixContextMenu { @@ -39,6 +40,12 @@ export class QuickFixContextMenu { () => this._onDidExecuteCodeAction.fire(undefined)); }); }); + }).then(actions => { + if (!this._editor.getDomNode()) { + // cancel when editor went off-dom + return TPromise.wrapError(canceled()); + } + return actions; }); this._contextMenuService.showContextMenu({ diff --git a/src/vs/editor/contrib/referenceSearch/referencesController.ts b/src/vs/editor/contrib/referenceSearch/referencesController.ts index 88079826a6d..349e1eb662c 100644 --- a/src/vs/editor/contrib/referenceSearch/referencesController.ts +++ b/src/vs/editor/contrib/referenceSearch/referencesController.ts @@ -155,16 +155,17 @@ export class ReferencesController implements editorCommon.IEditorContribution { // show widget return this._widget.setModel(this._model).then(() => { + if (this._widget) { // might have been closed + // set title + this._widget.setMetaTitle(options.getMetaTitle(this._model)); - // set title - this._widget.setMetaTitle(options.getMetaTitle(this._model)); - - // set 'best' selection - let uri = this._editor.getModel().uri; - let pos = new Position(range.startLineNumber, range.startColumn); - let selection = this._model.nearestReference(uri, pos); - if (selection) { - return this._widget.setSelection(selection); + // set 'best' selection + let uri = this._editor.getModel().uri; + let pos = new Position(range.startLineNumber, range.startColumn); + let selection = this._model.nearestReference(uri, pos); + if (selection) { + return this._widget.setSelection(selection); + } } return undefined; }); diff --git a/src/vs/editor/test/common/services/editorSimpleWorker.test.ts b/src/vs/editor/test/common/services/editorSimpleWorker.test.ts index 0a8b62e8a55..0d55c58d7c1 100644 --- a/src/vs/editor/test/common/services/editorSimpleWorker.test.ts +++ b/src/vs/editor/test/common/services/editorSimpleWorker.test.ts @@ -168,4 +168,25 @@ suite('EditorSimpleWorker', () => { assert.equal(suggestions[0].label, 'foobar'); }); }); + + test('get words via iterator, issue #46930', function () { + + let model = worker.addModel([ + 'one line', // 1 + 'two line', // 2 + '', + 'past empty', + 'single', + '', + 'and now we are done' + ]); + + let words: string[] = []; + + for (let iter = model.createWordIterator(/[a-z]+/img), e = iter.next(); !e.done; e = iter.next()) { + words.push(e.value); + } + + assert.deepEqual(words, ['one', 'line', 'two', 'line', 'past', 'empty', 'single', 'and', 'now', 'we', 'are', 'done']); + }); }); diff --git a/src/vs/workbench/api/electron-browser/mainThreadErrors.ts b/src/vs/workbench/api/electron-browser/mainThreadErrors.ts index a1ff2a2dd6c..6c283a0d98c 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadErrors.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadErrors.ts @@ -16,7 +16,7 @@ export class MainThreadErrors implements MainThreadErrorsShape { } $onUnexpectedError(err: any | SerializedError): void { - if (err.$isError) { + if (err && err.$isError) { const { name, message, stack } = err; err = new Error(); err.message = message; diff --git a/src/vs/workbench/api/electron-browser/mainThreadFileSystem.ts b/src/vs/workbench/api/electron-browser/mainThreadFileSystem.ts index df14909d35f..35ea17279f0 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadFileSystem.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadFileSystem.ts @@ -56,8 +56,8 @@ export class MainThreadFileSystem implements MainThreadFileSystemShape { this._fileProvider.get(handle).$onFileSystemChange(changes); } - $reportFileChunk(handle: number, session: number, chunk: number[]): void { - this._fileProvider.get(handle).reportFileChunk(session, chunk); + $reportFileChunk(handle: number, session: number, base64Chunk: string): void { + this._fileProvider.get(handle).reportFileChunk(session, base64Chunk); } // --- search @@ -126,11 +126,14 @@ class RemoteFileSystemProvider implements IFileSystemProvider { return value; }); } - reportFileChunk(session: number, chunk: number[]): void { - this._reads.get(session).progress.report(Buffer.from(chunk)); + reportFileChunk(session: number, encodedChunk: string): void { + this._reads.get(session).progress.report(Buffer.from(encodedChunk, 'base64')); } write(resource: URI, content: Uint8Array): TPromise { - return this._proxy.$write(this._handle, resource, [].slice.call(content)); + let encoded = Buffer.isBuffer(content) + ? content.toString('base64') + : Buffer.from(content.buffer).toString('base64'); + return this._proxy.$write(this._handle, resource, encoded); } unlink(resource: URI): TPromise { return this._proxy.$unlink(this._handle, resource); diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 1a1336e3367..fe1dfcdb6ed 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -382,7 +382,7 @@ export interface MainThreadFileSystemShape extends IDisposable { $unregisterProvider(handle: number): void; $onFileSystemChange(handle: number, resource: IFileChangeDto[]): void; - $reportFileChunk(handle: number, session: number, chunk: number[] | null): void; + $reportFileChunk(handle: number, session: number, base64Encoded: string | null): void; $handleFindMatch(handle: number, session, data: UriComponents | [UriComponents, ILineMatch]): void; } @@ -561,7 +561,7 @@ export interface ExtHostFileSystemShape { $utimes(handle: number, resource: UriComponents, mtime: number, atime: number): TPromise; $stat(handle: number, resource: UriComponents): TPromise; $read(handle: number, session: number, offset: number, count: number, resource: UriComponents): TPromise; - $write(handle: number, resource: UriComponents, content: number[]): TPromise; + $write(handle: number, resource: UriComponents, base64Encoded: string): TPromise; $unlink(handle: number, resource: UriComponents): TPromise; $move(handle: number, resource: UriComponents, target: UriComponents): TPromise; $mkdir(handle: number, resource: UriComponents): TPromise; diff --git a/src/vs/workbench/api/node/extHostDecorations.ts b/src/vs/workbench/api/node/extHostDecorations.ts index 89987b137bb..77d9b441fc8 100644 --- a/src/vs/workbench/api/node/extHostDecorations.ts +++ b/src/vs/workbench/api/node/extHostDecorations.ts @@ -43,6 +43,10 @@ export class ExtHostDecorations implements ExtHostDecorationsShape { return TPromise.join(requests.map(request => { const { handle, uri, id } = request; const provider = this._provider.get(handle); + if (!provider) { + // might have been unregistered in the meantime + return void 0; + } return asWinJsPromise(token => provider.provideDecoration(URI.revive(uri), token)).then(data => { result[id] = data && [data.priority, data.bubble, data.title, data.abbreviation, data.color, data.source]; }, err => { diff --git a/src/vs/workbench/api/node/extHostFileSystem.ts b/src/vs/workbench/api/node/extHostFileSystem.ts index 00dfa69b6df..5d8499e44fb 100644 --- a/src/vs/workbench/api/node/extHostFileSystem.ts +++ b/src/vs/workbench/api/node/extHostFileSystem.ts @@ -15,6 +15,7 @@ import { IPatternInfo } from 'vs/platform/search/common/search'; import { values } from 'vs/base/common/map'; import { Range } from 'vs/workbench/api/node/extHostTypes'; import { ExtHostLanguageFeatures } from 'vs/workbench/api/node/extHostLanguageFeatures'; +import { IProgress } from 'vs/platform/progress/common/progress'; class FsLinkProvider implements vscode.DocumentLinkProvider { @@ -109,15 +110,19 @@ export class ExtHostFileSystem implements ExtHostFileSystemShape { return asWinJsPromise(token => this._fsProvider.get(handle).stat(URI.revive(resource))); } $read(handle: number, session: number, offset: number, count: number, resource: UriComponents): TPromise { - const progress = { + const progress: IProgress = { report: chunk => { - this._proxy.$reportFileChunk(handle, session, [].slice.call(chunk)); + let base64Chunk = Buffer.isBuffer(chunk) + ? chunk.toString('base64') + : Buffer.from(chunk.buffer).toString('base64'); + + this._proxy.$reportFileChunk(handle, session, base64Chunk); } }; return asWinJsPromise(token => this._fsProvider.get(handle).read(URI.revive(resource), offset, count, progress)); } - $write(handle: number, resource: UriComponents, content: number[]): TPromise { - return asWinJsPromise(token => this._fsProvider.get(handle).write(URI.revive(resource), Buffer.from(content))); + $write(handle: number, resource: UriComponents, base64Content: string): TPromise { + return asWinJsPromise(token => this._fsProvider.get(handle).write(URI.revive(resource), Buffer.from(base64Content, 'base64'))); } $unlink(handle: number, resource: UriComponents): TPromise { return asWinJsPromise(token => this._fsProvider.get(handle).unlink(URI.revive(resource))); diff --git a/src/vs/workbench/services/decorations/browser/decorationsService.ts b/src/vs/workbench/services/decorations/browser/decorationsService.ts index 4f55f3044ab..81e0dc727ed 100644 --- a/src/vs/workbench/services/decorations/browser/decorationsService.ts +++ b/src/vs/workbench/services/decorations/browser/decorationsService.ts @@ -245,8 +245,14 @@ class DecorationProviderWrapper { } else { // selective changes -> drop for resource, fetch again, send event + // perf: the map stores thenables, decorations, or `null`-markers. + // we make us of that and ignore all uris in which we have never + // been interested. for (const uri of uris) { - this._fetchData(uri); + const value = this.data.get(uri.toString()); + if (value !== undefined) { + this._fetchData(uri); + } } } }); @@ -363,6 +369,8 @@ export class FileDecorationsService implements IDecorationsService { dispose(): void { dispose(this._disposables); + dispose(this._onDidChangeDecorations); + dispose(this._onDidChangeDecorationsDelayed); } registerDecorationsProvider(provider: IDecorationsProvider): IDisposable { diff --git a/src/vs/workbench/services/decorations/test/browser/decorationsService.test.ts b/src/vs/workbench/services/decorations/test/browser/decorationsService.test.ts index b8c9e471fab..aae61fb6c20 100644 --- a/src/vs/workbench/services/decorations/test/browser/decorationsService.test.ts +++ b/src/vs/workbench/services/decorations/test/browser/decorationsService.test.ts @@ -9,7 +9,7 @@ import * as assert from 'assert'; import { FileDecorationsService } from 'vs/workbench/services/decorations/browser/decorationsService'; import { IDecorationsProvider, IDecorationData } from 'vs/workbench/services/decorations/browser/decorations'; import URI from 'vs/base/common/uri'; -import { Event, toPromise } from 'vs/base/common/event'; +import { Event, toPromise, Emitter } from 'vs/base/common/event'; import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService'; suite('DecorationsService', function () { @@ -75,7 +75,7 @@ suite('DecorationsService', function () { assert.equal(callCounter, 1); }); - test('Clear decorations on provider dispose', function () { + test('Clear decorations on provider dispose', async function () { let uri = URI.parse('foo:bar'); let callCounter = 0; @@ -94,13 +94,14 @@ suite('DecorationsService', function () { // un-register -> ensure good event let didSeeEvent = false; - service.onDidChangeDecorations(e => { + let p = toPromise(service.onDidChangeDecorations).then(e => { assert.equal(e.affectsResource(uri), true); assert.deepEqual(service.getDecoration(uri, false), undefined); assert.equal(callCounter, 1); didSeeEvent = true; }); reg.dispose(); + await p; assert.equal(didSeeEvent, true); }); @@ -168,4 +169,40 @@ suite('DecorationsService', function () { reg.dispose(); }); + + test('Avoid unnecessary decoration change events #46938', async function () { + + let uri1 = URI.parse('file:///uri1.txt'); + let uri2 = URI.parse('file:///uri2.txt'); + + let emitter = new Emitter(); + + let asked = new Set(); + + let reg = service.registerDecorationsProvider({ + label: 'Test', + onDidChange: emitter.event, + provideDecorations(uri: URI) { + asked.add(uri.toString()); + return { tooltip: uri.path, source: 'foo' }; + } + }); + + let deco = service.getDecoration(uri1, false); + assert.equal(deco.tooltip, '/uri1.txt'); + assert.equal(asked.size, 1); + assert.ok(asked.has(uri1.toString())); + + let didChange = toPromise(service.onDidChangeDecorations); + emitter.fire([uri1, uri2]); + + let e = await didChange; + + assert.equal(e.affectsResource(uri1), true); + assert.equal(e.affectsResource(uri2), false); + assert.equal(asked.size, 1); + assert.ok(asked.has(uri1.toString())); + + reg.dispose(); + }); });