Repository navigation
SCM - keep previously hidden repositories hidden when they are discovered late - #348396
Draft
Ladislau Szomoru (lszomoru) wants to merge 2 commits into
Draft
Ladislau Szomoru (lszomoru) wants to merge 2 commits into
Ladislau Szomoru (lszomoru) wants to merge 2 commits into
Conversation
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>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A routine storage flush can discard a saved hidden state for an undiscovered submodule, causing it to be selected when registered later; the state must be preserved and covered by a regression test.
1 open issue
What changed in this PR
This PR updates SCM selection restoration to keep previously deselected repositories hidden when discovery occurs after the initial loading window, addressing late submodule discovery in #265510.
Changes:
- Consults saved selection state for late-discovered repositories while preserving visibility for new repositories.
- Adds a regression test covering a late hidden submodule and a newly discovered repository.
| File | Description |
|---|---|
src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts |
Tests late-discovery selection behavior using fake timers. |
src/vs/workbench/contrib/scm/browser/scmViewService.ts |
Applies saved hidden state after the loading window ends. |
Review effort: Balanced
…shes `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>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #265510
Repro steps
deps/sub1,deps/sub2), so that they are found through submodule detection rather than the workspace folder scan (git.repositoryScanMaxDepth).scm.repositories.selectionMode: multiple,git.detectSubmodules: true) and show the Repositories view.Developer: Reload Window.The submodules are only opened after the parent repository's first
git statusfinishes. With large repositories (or many repositories, as in the issue's recording) that takes longer than the 5s after which the SCM view considers the repositories "finished loading".Root cause
SCMViewService.onDidAddRepositoryonly consults the persistedscm:view:visibleRepositoriesstate whiledidFinishLoadingRepositoriesisfalse. That flag flips 5s after the last repository was added. Any repository added after that point (late submodules) skips the persisted state. In multiple selection mode it is then always made visible, even if the user explicitly hid it in the previous session.Fix
When a repository is added after loading has finished, keep it hidden if:
didSelectRepository) and at least one repository is visible, andall) but was not visible.This leaves the other cases unchanged:
Validation
pnpm run typecheck-clientpnpm run oxlint src/vs/workbench/contrib/scm/browser/scmViewService.ts src/vs/workbench/contrib/scm/test/browser/scmViewService.test.ts./scripts/test.sh --run src/vs/workbench/contrib/scm/test/browser/scmViewService.test.tsscmViewService.tschange revertedparent/submoduleis visible againThe git extension is unchanged.
Evidence
I recorded the repro steps above with the
validate-ui-scenarioharness on a Code OSS dev build (macOS, throwaway profile, scratch repository), once with the fix reverted and once with the fix. To make the timing deterministic, agitwrapper onPATHdelaysgit statusin the parent repository by 8s. In both runs the Git log shows the submodules opened about 9s after the parent, both before and after the reload.ws,sub1andsub2all selected, and the Changes view lists all three repositories. The scenario's final assertion fails.wsis selected,sub1andsub2stay deselected, and the Changes view only showsws. All steps pass.A side-by-side video (before on the left, after on the right, aligned by step) and before/after screenshots of the moment after the reload are available on request.
ghcannot attach them here.