From c840a7a70f8d9c1938a4e11fc05f6aa43392565f Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Sun, 15 Nov 2026 11:47:28 +0100 Subject: [PATCH 1/2] SCM - keep previously hidden repositories hidden when discovered late Repositories (ex: submodules) that are discovered after the SCM view finished restoring its state were always made visible, even if the user had explicitly hidden them in the previous session. Fixes #265510 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../contrib/scm/browser/scmViewService.ts | 11 +++ .../scm/test/browser/scmViewService.test.ts | 86 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts diff --git a/src/vs/workbench/contrib/scm/browser/scmViewService.ts b/src/vs/workbench/contrib/scm/browser/scmViewService.ts index d7b3583901c7c8..6940747d1c73c5 100644 --- a/src/vs/workbench/contrib/scm/browser/scmViewService.ts +++ b/src/vs/workbench/contrib/scm/browser/scmViewService.ts @@ -405,6 +405,17 @@ export class SCMViewService implements ISCMViewService { this.didSelectRepository = true; } } + } else if (this.previousState && this.didSelectRepository && this.visibleRepositories.length > 0) { + // Repositories (ex: submodules) can be discovered after the repositories + // finished loading. Keep the repository hidden if it was explicitly hidden + // in the previous state. + const index = this.previousState.all.indexOf(getProviderStorageKey(repository.provider)); + + if (index !== -1 && this.previousState.visible.indexOf(index) === -1) { + this.insertRepositoryView(this._repositories, repositoryView); + this._onDidChangeRepositories.fire({ added: Iterable.empty(), removed: Iterable.empty() }); + return; + } } if (this.selectionModeConfig.get() === ISCMRepositorySelectionMode.Multiple || !this._repositories.find(r => r.selectionIndex !== -1)) { diff --git a/src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts b/src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts new file mode 100644 index 00000000000000..450bc1dd1fd638 --- /dev/null +++ b/src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts @@ -0,0 +1,86 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { timeout } from '../../../../../base/common/async.js'; +import { Emitter, Event } from '../../../../../base/common/event.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { mock } from '../../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { runWithFakedTimers } from '../../../../../base/test/common/virtualScheduling/runWithFakedTimers.js'; +import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; +import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; +import { StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js'; +import { IEditorService } from '../../../../services/editor/common/editorService.js'; +import { IExtensionService } from '../../../../services/extensions/common/extensions.js'; +import { TestStorageService } from '../../../../test/common/workbenchTestServices.js'; +import { SCMViewService } from '../../browser/scmViewService.js'; +import { ISCMProvider, ISCMRepository, ISCMService } from '../../common/scm.js'; + +suite('SCMViewService', () => { + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + function createRepository(id: string): ISCMRepository { + const provider = new class extends mock() { + override readonly providerId = 'git'; + override readonly label = 'Git'; + override readonly rootUri = URI.file(`/${id}`); + }(); + + return new class extends mock() { + override readonly id = id; + override readonly provider = provider; + }(); + } + + test('keeps a previously hidden repository hidden when it is added after loading finished', () => runWithFakedTimers({}, async () => { + const onDidAddRepository = disposables.add(new Emitter()); + const scmService = new class extends mock() { + override readonly onDidAddRepository = onDidAddRepository.event; + override readonly onDidRemoveRepository = Event.None; + override readonly repositories = []; + }(); + + const editorService = new class extends mock() { + override readonly onDidActiveEditorChange = Event.None; + }(); + + const extensionService = new class extends mock() { + override readonly onWillStop = Event.None; + }(); + + const instantiationService = new class extends mock() { + override createInstance = (() => ({})) as IInstantiationService['createInstance']; + }(); + + const parent = createRepository('parent'); + const submodule = createRepository('parent/submodule'); + const newRepository = createRepository('new'); + + const storageService = disposables.add(new TestStorageService()); + storageService.store('scm:view:visibleRepositories', JSON.stringify({ + all: [parent, submodule].map(r => `git:Git:${r.provider.rootUri!.toString()}`), + visible: [0], + sortKey: 'discoveryTime' + }), StorageScope.WORKSPACE, StorageTarget.MACHINE); + + const scmViewService = new SCMViewService(scmService, new MockContextKeyService(), editorService, extensionService, instantiationService, + new TestConfigurationService({ scm: { repositories: { selectionMode: 'multiple' } } }), storageService, new (mock())()); + + onDidAddRepository.fire(parent); + + // Repositories finish loading 5s after the last repository was added + await timeout(5000); + + onDidAddRepository.fire(submodule); + onDidAddRepository.fire(newRepository); + + assert.deepStrictEqual(scmViewService.visibleRepositories.map(r => r.id), ['parent', 'new']); + + scmViewService.dispose(); + })); +}); From b15eceda7d4fbd2b3eaed389414d11d4d14961bf Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Sun, 15 Nov 2026 20:18:03 +0100 Subject: [PATCH 2/2] SCM - keep visibility of undiscovered repositories across storage flushes `onWillSaveState()` replaced the in-memory previous state with only the registered repositories, so a routine storage flush before a submodule was discovered lost its saved (hidden) visibility. Keep the previous visibility of repositories that have not been discovered yet in memory, and extend the test to cover a storage flush before the late repository is added. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../contrib/scm/browser/scmViewService.ts | 15 +++- .../scm/test/browser/scmViewService.test.ts | 78 ++++++++++--------- 2 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/vs/workbench/contrib/scm/browser/scmViewService.ts b/src/vs/workbench/contrib/scm/browser/scmViewService.ts index 6940747d1c73c5..c6f303808de54e 100644 --- a/src/vs/workbench/contrib/scm/browser/scmViewService.ts +++ b/src/vs/workbench/contrib/scm/browser/scmViewService.ts @@ -584,9 +584,20 @@ export class SCMViewService implements ISCMViewService { const all = this.repositories.map(r => getProviderStorageKey(r.provider)); const visible = this.visibleRepositories.map(r => all.indexOf(getProviderStorageKey(r.provider))); - this.previousState = { all, visible, sortKey: this._repositoriesSortKey } satisfies ISCMViewServiceState; + const state = { all, visible, sortKey: this._repositoriesSortKey } satisfies ISCMViewServiceState; - this.storageService.store('scm:view:visibleRepositories', JSON.stringify(this.previousState), StorageScope.WORKSPACE, StorageTarget.MACHINE); + this.storageService.store('scm:view:visibleRepositories', JSON.stringify(state), StorageScope.WORKSPACE, StorageTarget.MACHINE); + + // Remember the previous visibility of repositories that have not been + // discovered yet (ex: submodules) so that it is used when they are added + const previousState = this.previousState; + const undiscovered = previousState?.all.filter(key => !all.includes(key)) ?? []; + + this.previousState = previousState && undiscovered.length > 0 ? { + ...state, + all: [...all, ...undiscovered], + visible: [...visible, ...undiscovered.flatMap((key, index) => previousState.visible.includes(previousState.all.indexOf(key)) ? [all.length + index] : [])] + } : state; } @debounce(5000) diff --git a/src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts b/src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts index 450bc1dd1fd638..69ddee6caa7650 100644 --- a/src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts +++ b/src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts @@ -13,7 +13,7 @@ import { runWithFakedTimers } from '../../../../../base/test/common/virtualSched import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; -import { StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { StorageScope, StorageTarget, WillSaveStateReason } from '../../../../../platform/storage/common/storage.js'; import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js'; import { IEditorService } from '../../../../services/editor/common/editorService.js'; import { IExtensionService } from '../../../../services/extensions/common/extensions.js'; @@ -37,50 +37,56 @@ suite('SCMViewService', () => { }(); } - test('keeps a previously hidden repository hidden when it is added after loading finished', () => runWithFakedTimers({}, async () => { - const onDidAddRepository = disposables.add(new Emitter()); - const scmService = new class extends mock() { - override readonly onDidAddRepository = onDidAddRepository.event; - override readonly onDidRemoveRepository = Event.None; - override readonly repositories = []; - }(); + for (const flushStorage of [false, true]) { + test(`keeps a previously hidden repository hidden when it is added after loading finished${flushStorage ? ' and storage was flushed' : ''}`, () => runWithFakedTimers({}, async () => { + const onDidAddRepository = disposables.add(new Emitter()); + const scmService = new class extends mock() { + override readonly onDidAddRepository = onDidAddRepository.event; + override readonly onDidRemoveRepository = Event.None; + override readonly repositories = []; + }(); - const editorService = new class extends mock() { - override readonly onDidActiveEditorChange = Event.None; - }(); + const editorService = new class extends mock() { + override readonly onDidActiveEditorChange = Event.None; + }(); - const extensionService = new class extends mock() { - override readonly onWillStop = Event.None; - }(); + const extensionService = new class extends mock() { + override readonly onWillStop = Event.None; + }(); - const instantiationService = new class extends mock() { - override createInstance = (() => ({})) as IInstantiationService['createInstance']; - }(); + const instantiationService = new class extends mock() { + override createInstance = (() => ({})) as IInstantiationService['createInstance']; + }(); + + const parent = createRepository('parent'); + const submodule = createRepository('parent/submodule'); + const newRepository = createRepository('new'); - const parent = createRepository('parent'); - const submodule = createRepository('parent/submodule'); - const newRepository = createRepository('new'); + const storageService = disposables.add(new TestStorageService()); + storageService.store('scm:view:visibleRepositories', JSON.stringify({ + all: [parent, submodule].map(r => `git:Git:${r.provider.rootUri!.toString()}`), + visible: [0], + sortKey: 'discoveryTime' + }), StorageScope.WORKSPACE, StorageTarget.MACHINE); - const storageService = disposables.add(new TestStorageService()); - storageService.store('scm:view:visibleRepositories', JSON.stringify({ - all: [parent, submodule].map(r => `git:Git:${r.provider.rootUri!.toString()}`), - visible: [0], - sortKey: 'discoveryTime' - }), StorageScope.WORKSPACE, StorageTarget.MACHINE); + const scmViewService = new SCMViewService(scmService, new MockContextKeyService(), editorService, extensionService, instantiationService, + new TestConfigurationService({ scm: { repositories: { selectionMode: 'multiple' } } }), storageService, new (mock())()); - const scmViewService = new SCMViewService(scmService, new MockContextKeyService(), editorService, extensionService, instantiationService, - new TestConfigurationService({ scm: { repositories: { selectionMode: 'multiple' } } }), storageService, new (mock())()); + onDidAddRepository.fire(parent); - onDidAddRepository.fire(parent); + // Repositories finish loading 5s after the last repository was added + await timeout(5000); - // Repositories finish loading 5s after the last repository was added - await timeout(5000); + if (flushStorage) { + storageService.testEmitWillSaveState(WillSaveStateReason.NONE); + } - onDidAddRepository.fire(submodule); - onDidAddRepository.fire(newRepository); + onDidAddRepository.fire(submodule); + onDidAddRepository.fire(newRepository); - assert.deepStrictEqual(scmViewService.visibleRepositories.map(r => r.id), ['parent', 'new']); + assert.deepStrictEqual(scmViewService.visibleRepositories.map(r => r.id), ['parent', 'new']); - scmViewService.dispose(); - })); + scmViewService.dispose(); + })); + } });