From 89f0c65eb4f5eb55dceeb12fdde05742ed083447 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 17 Oct 2022 07:58:48 -0700 Subject: [PATCH] Avoid extra uses of `Iterator.ts` (#163699) - Removes `Iterator.equals` as this was unused - Replace `Iterator.collect` with `Array.from` - Use `asArray` instead of `Iterable.single` in a few cases - Remove `Iterable.forEach` and replace single caller with `for/of` - If passing an array to `Iterable`, we don't need to call `Iterable.from` on it - When we create an iterable from an array and then convert that iterable back to an array right away, we should just operate on the array (this lets us remove the one caller to `concatNested`) --- src/vs/base/common/iterator.ts | 44 ------------------- src/vs/base/test/common/iterator.test.ts | 8 ---- .../gotoSymbol/browser/goToCommands.ts | 5 +-- .../contrib/scm/browser/dirtydiffDecorator.ts | 8 ++-- .../contrib/scm/common/scmService.ts | 4 +- .../contrib/snippets/browser/snippetsFile.ts | 5 +-- .../testing/browser/testExplorerActions.ts | 5 +-- .../actions/common/menusExtensionPoint.ts | 3 +- 8 files changed, 14 insertions(+), 68 deletions(-) diff --git a/src/vs/base/common/iterator.ts b/src/vs/base/common/iterator.ts index 7d6e1da0c0d..0822dea9f9c 100644 --- a/src/vs/base/common/iterator.ts +++ b/src/vs/base/common/iterator.ts @@ -76,14 +76,6 @@ export namespace Iterable { } } - export function* concatNested(iterables: Iterable>): Iterable { - for (const iterable of iterables) { - for (const element of iterable) { - yield element; - } - } - } - export function reduce(iterable: Iterable, reducer: (previousValue: R, currentValue: T) => R, initialValue: R): R { let value = initialValue; for (const element of iterable) { @@ -92,13 +84,6 @@ export namespace Iterable { return value; } - export function forEach(iterable: Iterable, fn: (t: T, index: number) => any): void { - let index = 0; - for (const element of iterable) { - fn(element, index++); - } - } - /** * Returns an iterable slice of the array, with the same semantics as `array.slice()`. */ @@ -143,33 +128,4 @@ export namespace Iterable { return [consumed, { [Symbol.iterator]() { return iterator; } }]; } - - /** - * Consumes `atMost` elements from iterable and returns the consumed elements, - * and an iterable for the rest of the elements. - */ - export function collect(iterable: Iterable): T[] { - return consume(iterable)[0]; - } - - /** - * Returns whether the iterables are the same length and all items are - * equal using the comparator function. - */ - export function equals(a: Iterable, b: Iterable, comparator = (at: T, bt: T) => at === bt) { - const ai = a[Symbol.iterator](); - const bi = b[Symbol.iterator](); - while (true) { - const an = ai.next(); - const bn = bi.next(); - - if (an.done !== bn.done) { - return false; - } else if (an.done) { - return true; - } else if (!comparator(an.value, bn.value)) { - return false; - } - } - } } diff --git a/src/vs/base/test/common/iterator.test.ts b/src/vs/base/test/common/iterator.test.ts index 186ebc866b6..2346293f640 100644 --- a/src/vs/base/test/common/iterator.test.ts +++ b/src/vs/base/test/common/iterator.test.ts @@ -24,12 +24,4 @@ suite('Iterable', function () { assert.strictEqual(Iterable.first(customIterable), 'one'); assert.strictEqual(Iterable.first(customIterable), 'one'); // fresh }); - - test('equals', () => { - assert.strictEqual(Iterable.equals([1, 2], [1, 2]), true); - assert.strictEqual(Iterable.equals([1, 2], [1]), false); - assert.strictEqual(Iterable.equals([1], [1, 2]), false); - assert.strictEqual(Iterable.equals([2, 1], [1, 2]), false); - }); - }); diff --git a/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts b/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts index 6360be59725..99d41fecc99 100644 --- a/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts +++ b/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts @@ -40,7 +40,7 @@ import { IEditorProgressService } from 'vs/platform/progress/common/progress'; import { getDeclarationsAtPosition, getDefinitionsAtPosition, getImplementationsAtPosition, getReferencesAtPosition, getTypeDefinitionsAtPosition } from './goToSymbol'; import { IWordAtPosition } from 'vs/editor/common/core/wordHelper'; import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures'; -import { Iterable } from 'vs/base/common/iterator'; +import { asArray } from 'vs/base/common/arrays'; MenuRegistry.appendMenuItem(MenuId.EditorContext, { submenu: MenuId.EditorContextPeek, @@ -86,8 +86,7 @@ export abstract class SymbolNavigationAction extends EditorAction2 { const result = { ...opts, f1: true }; // patch context menu when clause if (result.menu) { - const iterable = Array.isArray(result.menu) ? result.menu : Iterable.single(result.menu); - for (const item of iterable) { + for (const item of asArray(result.menu)) { if (item.id === MenuId.EditorContext || item.id === MenuId.EditorContextPeek) { item.when = ContextKeyExpr.and(opts.precondition, item.when); } diff --git a/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts b/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts index b79405c2229..ab44706fbf9 100644 --- a/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts +++ b/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts @@ -1084,7 +1084,7 @@ export function createProviderComparer(uri: URI): (a: ISCMProvider, b: ISCMProvi export async function getOriginalResource(scmService: ISCMService, uri: URI): Promise { const providers = Iterable.map(scmService.repositories, r => r.provider); - const rootedProviders = Iterable.collect(Iterable.filter(providers, p => !!p.rootUri)); + const rootedProviders = Array.from(Iterable.filter(providers, p => !!p.rootUri)); rootedProviders.sort(createProviderComparer(uri)); @@ -1095,7 +1095,7 @@ export async function getOriginalResource(scmService: ISCMService, uri: URI): Pr } const nonRootedProviders = Iterable.filter(providers, p => !p.rootUri); - return first(Iterable.collect(Iterable.map(nonRootedProviders, p => () => p.getOriginalResource(uri)))); + return first(Array.from(nonRootedProviders, p => () => p.getOriginalResource(uri))); } export class DirtyDiffModel extends Disposable { @@ -1136,7 +1136,9 @@ export class DirtyDiffModel extends Disposable { )(this.triggerDiff, this) ); this._register(scmService.onDidAddRepository(this.onDidAddRepository, this)); - Iterable.forEach(scmService.repositories, r => this.onDidAddRepository(r)); + for (const r of scmService.repositories) { + this.onDidAddRepository(r); + } this._register(this._model.onDidChangeEncoding(() => { this.diffDelayer.cancel(); diff --git a/src/vs/workbench/contrib/scm/common/scmService.ts b/src/vs/workbench/contrib/scm/common/scmService.ts index 9231a8ac8b8..5e219444be9 100644 --- a/src/vs/workbench/contrib/scm/common/scmService.ts +++ b/src/vs/workbench/contrib/scm/common/scmService.ts @@ -104,7 +104,7 @@ class SCMInput implements ISCMInput { } // Migrate from old format // TODO@joao: remove this migration code a few releases - const userKeys = Iterable.filter(Iterable.from(storageService.keys(StorageScope.APPLICATION, StorageTarget.USER)), key => key.startsWith('scm/input:')); + const userKeys = Iterable.filter(storageService.keys(StorageScope.APPLICATION, StorageTarget.USER), key => key.startsWith('scm/input:')); for (const key of userKeys) { try { @@ -130,7 +130,7 @@ class SCMInput implements ISCMInput { } // Garbage collect - const machineKeys = Iterable.filter(Iterable.from(storageService.keys(StorageScope.APPLICATION, StorageTarget.MACHINE)), key => key.startsWith('scm/input:')); + const machineKeys = Iterable.filter(storageService.keys(StorageScope.APPLICATION, StorageTarget.MACHINE), key => key.startsWith('scm/input:')); for (const key of machineKeys) { try { diff --git a/src/vs/workbench/contrib/snippets/browser/snippetsFile.ts b/src/vs/workbench/contrib/snippets/browser/snippetsFile.ts index 7d88688f68d..329a2431377 100644 --- a/src/vs/workbench/contrib/snippets/browser/snippetsFile.ts +++ b/src/vs/workbench/contrib/snippets/browser/snippetsFile.ts @@ -15,8 +15,7 @@ import { IdleValue } from 'vs/base/common/async'; import { IExtensionResourceLoaderService } from 'vs/platform/extensionResourceLoader/common/extensionResourceLoader'; import { relativePath } from 'vs/base/common/resources'; import { isObject } from 'vs/base/common/types'; -import { Iterable } from 'vs/base/common/iterator'; -import { tail } from 'vs/base/common/arrays'; +import { asArray, tail } from 'vs/base/common/arrays'; class SnippetBodyInsights { @@ -304,7 +303,7 @@ export class SnippetFile { } } - for (const _prefix of Array.isArray(prefix) ? prefix : Iterable.single(prefix)) { + for (const _prefix of asArray(prefix)) { bucket.push(new Snippet( Boolean(isFileTemplate), scopes, diff --git a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts index 6e1f48fa599..6c8f9c91db8 100644 --- a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts +++ b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts @@ -5,7 +5,6 @@ import { distinct } from 'vs/base/common/arrays'; import { Codicon } from 'vs/base/common/codicons'; -import { Iterable } from 'vs/base/common/iterator'; import { KeyChord, KeyCode, KeyMod } from 'vs/base/common/keyCodes'; import { isDefined } from 'vs/base/common/types'; import { Position } from 'vs/editor/common/core/position'; @@ -154,7 +153,7 @@ export class DebugAction extends Action2 { public override run(acessor: ServicesAccessor, ...elements: IActionableTestTreeElement[]): Promise { return acessor.get(ITestService).runTests({ - tests: [...Iterable.concatNested(elements.map(e => e.tests))], + tests: elements.flatMap(e => [...e.tests]), group: TestRunProfileBitset.Debug, }); } @@ -216,7 +215,7 @@ export class RunAction extends Action2 { */ public override run(acessor: ServicesAccessor, ...elements: IActionableTestTreeElement[]): Promise { return acessor.get(ITestService).runTests({ - tests: [...Iterable.concatNested(elements.map(e => e.tests))], + tests: elements.flatMap(e => [...e.tests]), group: TestRunProfileBitset.Run, }); } diff --git a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts index 160b2b6e226..bd6abb41e08 100644 --- a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts +++ b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts @@ -13,7 +13,6 @@ import { MenuId, MenuRegistry, IMenuItem, ISubmenuItem } from 'vs/platform/actio import { URI } from 'vs/base/common/uri'; import { DisposableStore } from 'vs/base/common/lifecycle'; import { ThemeIcon } from 'vs/platform/theme/common/themeService'; -import { Iterable } from 'vs/base/common/iterator'; import { index } from 'vs/base/common/arrays'; import { isProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; import { ApiProposalName } from 'vs/workbench/services/extensions/common/extensionsApiProposals'; @@ -748,7 +747,7 @@ submenusExtensionPoint.setHandler(extensions => { } }); -const _apiMenusByKey = new Map(Iterable.map(Iterable.from(apiMenus), menu => ([menu.key, menu]))); +const _apiMenusByKey = new Map(apiMenus.map(menu => ([menu.key, menu]))); const _menuRegistrations = new DisposableStore(); const _submenuMenuItems = new Map>();