From ab13b068b55f468ba2d1914ea4a13bcb88ec8f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Moreno?= Date: Mon, 2 May 2022 14:47:04 +0200 Subject: [PATCH] Remove SCMRepository instances from context service (#148047) * have SCMService expose iterable repositories list * remove SCMRepository instances from context service Related-to: #147926 --- src/vs/base/common/iterator.ts | 15 +++++++++ .../workbench/contrib/scm/browser/activity.ts | 16 +++++---- .../contrib/scm/browser/dirtydiffDecorator.ts | 11 ++++--- .../contrib/scm/browser/scm.contribution.ts | 24 ++++++++++---- .../contrib/scm/browser/scmViewPane.ts | 14 ++++---- src/vs/workbench/contrib/scm/common/scm.ts | 5 ++- .../contrib/scm/common/scmService.ts | 33 ++++++++----------- 7 files changed, 73 insertions(+), 45 deletions(-) diff --git a/src/vs/base/common/iterator.ts b/src/vs/base/common/iterator.ts index 4e325694222..2771802f1de 100644 --- a/src/vs/base/common/iterator.ts +++ b/src/vs/base/common/iterator.ts @@ -92,6 +92,13 @@ 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()`. */ @@ -137,6 +144,14 @@ 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. diff --git a/src/vs/workbench/contrib/scm/browser/activity.ts b/src/vs/workbench/contrib/scm/browser/activity.ts index 399e48dcce8..91629caa98b 100644 --- a/src/vs/workbench/contrib/scm/browser/activity.ts +++ b/src/vs/workbench/contrib/scm/browser/activity.ts @@ -18,6 +18,7 @@ import { EditorResourceAccessor } from 'vs/workbench/common/editor'; import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity'; import { stripIcons } from 'vs/base/common/iconLabels'; import { Schemas } from 'vs/base/common/network'; +import { Iterable } from 'vs/base/common/iterator'; function getCount(repository: ISCMRepository): number { if (typeof repository.provider.count === 'number') { @@ -116,7 +117,7 @@ export class SCMStatusController implements IWorkbenchContribution { return; } - this.focusRepository(this.scmService.repositories[0]); + this.focusRepository(Iterable.first(this.scmService.repositories)); } private focusRepository(repository: ISCMRepository | undefined): void { @@ -172,7 +173,7 @@ export class SCMStatusController implements IWorkbenchContribution { let count = 0; if (countBadgeType === 'all') { - count = this.scmService.repositories.reduce((r, repository) => r + getCount(repository), 0); + count = Iterable.reduce(this.scmService.repositories, (r, repository) => r + getCount(repository), 0); } else if (countBadgeType === 'focused' && this.focusedRepository) { count = getCount(this.focusedRepository); } @@ -198,7 +199,7 @@ export class SCMStatusController implements IWorkbenchContribution { export class SCMActiveResourceContextKeyController implements IWorkbenchContribution { private activeResourceHasChangesContextKey: IContextKey; - private activeResourceRepositoryContextKey: IContextKey; + private activeResourceRepositoryContextKey: IContextKey; private disposables = new DisposableStore(); private repositoryDisposables = new Set(); @@ -239,11 +240,12 @@ export class SCMActiveResourceContextKeyController implements IWorkbenchContribu const activeResource = EditorResourceAccessor.getOriginalUri(this.editorService.activeEditor); if (activeResource?.scheme === Schemas.file || activeResource?.scheme === Schemas.vscodeRemote) { - const activeResourceRepository = this.scmService.repositories - .find(r => r.provider.rootUri && - this.uriIdentityService.extUri.isEqualOrParent(activeResource, r.provider.rootUri)); + const activeResourceRepository = Iterable.find( + this.scmService.repositories, + r => Boolean(r.provider.rootUri && this.uriIdentityService.extUri.isEqualOrParent(activeResource, r.provider.rootUri)) + ); - this.activeResourceRepositoryContextKey.set(activeResourceRepository); + this.activeResourceRepositoryContextKey.set(activeResourceRepository?.id); for (const resourceGroup of activeResourceRepository?.provider.groups.elements ?? []) { if (resourceGroup.elements diff --git a/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts b/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts index 8bae9fbc794..0457fde2584 100644 --- a/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts +++ b/src/vs/workbench/contrib/scm/browser/dirtydiffDecorator.ts @@ -53,6 +53,7 @@ import { IProgressService, ProgressLocation } from 'vs/platform/progress/common/ import { IChange } from 'vs/editor/common/diff/diffComputer'; import { Color } from 'vs/base/common/color'; import { editorGutter } from 'vs/editor/common/core/editorColorRegistry'; +import { Iterable } from 'vs/base/common/iterator'; class DiffActionRunner extends ActionRunner { @@ -1079,8 +1080,8 @@ export function createProviderComparer(uri: URI): (a: ISCMProvider, b: ISCMProvi } export async function getOriginalResource(scmService: ISCMService, uri: URI): Promise { - const providers = scmService.repositories.map(r => r.provider); - const rootedProviders = providers.filter(p => !!p.rootUri); + const providers = Iterable.map(scmService.repositories, r => r.provider); + const rootedProviders = Iterable.collect(Iterable.filter(providers, p => !!p.rootUri)); rootedProviders.sort(createProviderComparer(uri)); @@ -1090,8 +1091,8 @@ export async function getOriginalResource(scmService: ISCMService, uri: URI): Pr return result; } - const nonRootedProviders = providers.filter(p => !p.rootUri); - return first(nonRootedProviders.map(p => () => p.getOriginalResource(uri))); + const nonRootedProviders = Iterable.filter(providers, p => !p.rootUri); + return first(Iterable.collect(Iterable.map(nonRootedProviders, p => () => p.getOriginalResource(uri)))); } export class DirtyDiffModel extends Disposable { @@ -1132,7 +1133,7 @@ export class DirtyDiffModel extends Disposable { )(this.triggerDiff, this) ); this._register(scmService.onDidAddRepository(this.onDidAddRepository, this)); - scmService.repositories.forEach(r => this.onDidAddRepository(r)); + Iterable.forEach(scmService.repositories, r => this.onDidAddRepository(r)); this._register(this._model.onDidChangeEncoding(() => { this.diffDelayer.cancel(); diff --git a/src/vs/workbench/contrib/scm/browser/scm.contribution.ts b/src/vs/workbench/contrib/scm/browser/scm.contribution.ts index 89b85c7ecae..956e867a21f 100644 --- a/src/vs/workbench/contrib/scm/browser/scm.contribution.ts +++ b/src/vs/workbench/contrib/scm/browser/scm.contribution.ts @@ -7,7 +7,7 @@ import { localize } from 'vs/nls'; import { Registry } from 'vs/platform/registry/common/platform'; import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions } from 'vs/workbench/common/contributions'; import { DirtyDiffWorkbenchController } from './dirtydiffDecorator'; -import { VIEWLET_ID, ISCMRepository, ISCMService, VIEW_PANE_ID, ISCMProvider, ISCMViewService, REPOSITORIES_VIEW_PANE_ID } from 'vs/workbench/contrib/scm/common/scm'; +import { VIEWLET_ID, ISCMService, VIEW_PANE_ID, ISCMProvider, ISCMViewService, REPOSITORIES_VIEW_PANE_ID } from 'vs/workbench/contrib/scm/common/scm'; import { KeyMod, KeyCode } from 'vs/base/common/keyCodes'; import { MenuRegistry, MenuId } from 'vs/platform/actions/common/actions'; import { SCMActiveResourceContextKeyController, SCMStatusController } from './activity'; @@ -292,15 +292,23 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({ handler: accessor => { const contextKeyService = accessor.get(IContextKeyService); const context = contextKeyService.getContext(document.activeElement); - const repository = context.getValue('scmRepository'); + const repositoryId = context.getValue('scmRepository'); - if (!repository || !repository.provider.acceptInputCommand) { + if (!repositoryId) { return Promise.resolve(null); } + + const scmService = accessor.get(ISCMService); + const repository = scmService.getRepository(repositoryId); + + if (!repository?.provider.acceptInputCommand) { + return Promise.resolve(null); + } + const id = repository.provider.acceptInputCommand.id; const args = repository.provider.acceptInputCommand.arguments; - const commandService = accessor.get(ICommandService); + return commandService.executeCommand(id, ...(args || [])); } }); @@ -310,8 +318,10 @@ const viewNextCommitCommand = { weight: KeybindingWeight.WorkbenchContrib, handler: (accessor: ServicesAccessor) => { const contextKeyService = accessor.get(IContextKeyService); + const scmService = accessor.get(ISCMService); const context = contextKeyService.getContext(document.activeElement); - const repository = context.getValue('scmRepository'); + const repositoryId = context.getValue('scmRepository'); + const repository = repositoryId ? scmService.getRepository(repositoryId) : undefined; repository?.input.showNextHistoryValue(); } }; @@ -321,8 +331,10 @@ const viewPreviousCommitCommand = { weight: KeybindingWeight.WorkbenchContrib, handler: (accessor: ServicesAccessor) => { const contextKeyService = accessor.get(IContextKeyService); + const scmService = accessor.get(ISCMService); const context = contextKeyService.getContext(document.activeElement); - const repository = context.getValue('scmRepository'); + const repositoryId = context.getValue('scmRepository'); + const repository = repositoryId ? scmService.getRepository(repositoryId) : undefined; repository?.input.showPreviousHistoryValue(); } }; diff --git a/src/vs/workbench/contrib/scm/browser/scmViewPane.ts b/src/vs/workbench/contrib/scm/browser/scmViewPane.ts index fd1a31c8767..7e87c10e0fc 100644 --- a/src/vs/workbench/contrib/scm/browser/scmViewPane.ts +++ b/src/vs/workbench/contrib/scm/browser/scmViewPane.ts @@ -1724,7 +1724,7 @@ class SCMInputWidget extends Disposable { private inputEditor: CodeEditorWidget; private model: { readonly input: ISCMInput; readonly textModel: ITextModel } | undefined; - private repositoryContextKey: IContextKey; + private repositoryIdContextKey: IContextKey; private repositoryDisposables = new DisposableStore(); private validation: IInputValidation | undefined; @@ -1753,7 +1753,7 @@ class SCMInputWidget extends Disposable { this.repositoryDisposables.dispose(); this.repositoryDisposables = new DisposableStore(); - this.repositoryContextKey.set(input?.repository); + this.repositoryIdContextKey.set(input?.repository.id); if (!input) { this.model?.textModel.dispose(); @@ -1919,7 +1919,7 @@ class SCMInputWidget extends Disposable { this.setPlaceholderFontStyles(fontFamily, fontSize, lineHeight); const contextKeyService2 = contextKeyService.createScoped(this.element); - this.repositoryContextKey = contextKeyService2.createKey('scmRepository', undefined); + this.repositoryIdContextKey = contextKeyService2.createKey('scmRepository', undefined); const editorOptions: IEditorConstructionOptions = { ...getSimpleEditorOptions(), @@ -2326,14 +2326,14 @@ export class SCMViewPane extends ViewPane { return; } else if (isSCMResourceGroup(e.element)) { const provider = e.element.provider; - const repository = this.scmService.repositories.find(r => r.provider === provider); + const repository = Iterable.find(this.scmService.repositories, r => r.provider === provider); if (repository) { this.scmViewService.focus(repository); } return; } else if (ResourceTree.isResourceNode(e.element)) { const provider = e.element.context.provider; - const repository = this.scmService.repositories.find(r => r.provider === provider); + const repository = Iterable.find(this.scmService.repositories, r => r.provider === provider); if (repository) { this.scmViewService.focus(repository); } @@ -2376,7 +2376,7 @@ export class SCMViewPane extends ViewPane { } const provider = e.element.resourceGroup.provider; - const repository = this.scmService.repositories.find(r => r.provider === provider); + const repository = Iterable.find(this.scmService.repositories, r => r.provider === provider); if (repository) { this.scmViewService.focus(repository); @@ -2451,7 +2451,7 @@ export class SCMViewPane extends ViewPane { } override shouldShowWelcome(): boolean { - return this.scmService.repositories.length === 0; + return this.scmService.repositoryCount === 0; } override getActionsContext(): unknown { diff --git a/src/vs/workbench/contrib/scm/common/scm.ts b/src/vs/workbench/contrib/scm/common/scm.ts index a70726447a1..e180e8e9d3c 100644 --- a/src/vs/workbench/contrib/scm/common/scm.ts +++ b/src/vs/workbench/contrib/scm/common/scm.ts @@ -135,6 +135,7 @@ export interface ISCMInput { } export interface ISCMRepository extends IDisposable { + readonly id: string; readonly provider: ISCMProvider; readonly input: ISCMInput; } @@ -144,9 +145,11 @@ export interface ISCMService { readonly _serviceBrand: undefined; readonly onDidAddRepository: Event; readonly onDidRemoveRepository: Event; - readonly repositories: ISCMRepository[]; + readonly repositories: Iterable; + readonly repositoryCount: number; registerSCMProvider(provider: ISCMProvider): ISCMRepository; + getRepository(id: string): ISCMRepository | undefined; } export interface ISCMTitleMenu { diff --git a/src/vs/workbench/contrib/scm/common/scmService.ts b/src/vs/workbench/contrib/scm/common/scmService.ts index a4670d7aff9..7877ba1d34e 100644 --- a/src/vs/workbench/contrib/scm/common/scmService.ts +++ b/src/vs/workbench/contrib/scm/common/scmService.ts @@ -242,6 +242,7 @@ class SCMRepository implements ISCMRepository { readonly input: ISCMInput = new SCMInput(this, this.storageService); constructor( + public readonly id: string, public readonly provider: ISCMProvider, private disposable: IDisposable, @IStorageService private storageService: IStorageService @@ -266,9 +267,9 @@ export class SCMService implements ISCMService { declare readonly _serviceBrand: undefined; - private _providerIds = new Set(); - private _repositories: ISCMRepository[] = []; - get repositories(): ISCMRepository[] { return [...this._repositories]; } + private _repositories = new Map(); + get repositories(): Iterable { return this._repositories.values(); } + get repositoryCount(): number { return this._repositories.size; } private providerCount: IContextKey; @@ -289,31 +290,25 @@ export class SCMService implements ISCMService { registerSCMProvider(provider: ISCMProvider): ISCMRepository { this.logService.trace('SCMService#registerSCMProvider'); - if (this._providerIds.has(provider.id)) { + if (this._repositories.has(provider.id)) { throw new Error(`SCM Provider ${provider.id} already exists.`); } - this._providerIds.add(provider.id); - const disposable = toDisposable(() => { - const index = this._repositories.indexOf(repository); - - if (index < 0) { - return; - } - - this._providerIds.delete(provider.id); - this._repositories.splice(index, 1); + this._repositories.delete(provider.id); this._onDidRemoveProvider.fire(repository); - - this.providerCount.set(this._repositories.length); + this.providerCount.set(this._repositories.size); }); - const repository = new SCMRepository(provider, disposable, this.storageService); - this._repositories.push(repository); + const repository = new SCMRepository(provider.id, provider, disposable, this.storageService); + this._repositories.set(provider.id, repository); this._onDidAddProvider.fire(repository); - this.providerCount.set(this._repositories.length); + this.providerCount.set(this._repositories.size); return repository; } + + getRepository(id: string): ISCMRepository | undefined { + return this._repositories.get(id); + } }