Git - respect configured submodule names in SCM - #269902
Git - respect configured submodule names in SCM#269902Tci Gravifer Fang (Gravifer) wants to merge 9 commits into
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the ability to use custom labels for repositories in the Source Control Management (SCM) panel instead of always using the root directory name. The changes focus on allowing repository labels to be specified, particularly useful for submodules which can now display their meaningful names from .gitmodules instead of just their directory names.
Key changes:
- Added support for optional repository labels that override the default root directory basename
- Modified the label resolution logic to prefer supplied labels while falling back to directory names
- Updated submodule handling to use submodule names as repository labels
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| extensions/git/src/repository.ts | Adds getter for accessing repository label property |
| extensions/git/src/model.ts | Updates label logic to use custom labels and modifies repository tracking to support labels |
| extensions/git/src/git.ts | Adds label parameter to Repository constructor and open method |
Tci Gravifer Fang (Gravifer)
changed the title
7df8a97 to
dd3148d
Compare
Tci Gravifer Fang (Gravifer)
changed the title
66b657c to
c08db38
Compare
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Ladislau Szomoru (@lszomoru)Matched files:
|
Tci Gravifer Fang (Gravifer)
changed the title
|
I have substantially revised the approach taken. My original attempt carried an optional label through the repository scan queue, The branch tip removes that plumbing. Instead, it adds an optional, creation-time repository name to the existing proposed The Git extension now determines the name at the point where it creates the child repository’s source control. If the child is a submodule, its superproject is open, and the matching This yields deliberately contextual behavior:
I think this distinction is useful. When a repository is opened independently, its folder or workspace name is the most immediate identity available to the user. When it is discovered as part of a superproject, however, the superproject’s A customized submodule name takes precedence over a workspace-folder alias when the child is created while its superproject is open and resolvable. This avoids returning to the earlier split behavior, where different SCM surfaces could disagree about the same repository. Existing SCM rendering already handles duplicate names by showing parent and path context where necessary, so no new disambiguation mechanism is required. The implementation is intentionally narrow from a maintenance perspective. It does not alter the stable extension API, add a setting, create persistent state, introduce a migration, or require dynamic rename events. The proposed API gains one optional final argument; callers that omit it retain the current workspace-folder, basename, and provider-label fallbacks. No existing extension needs to change. Likewise, the workbench renderers are untouched. They already consume I hope this would not introduce much burden for the maintainers or become obstructive for future contributors. Thanks to you all for the excellent project that benefits all developers. |
9e34767 to
ffb5ac8
Compare
4ed8279 to
efb78c8
Compare
|
Hi Ladislau Szomoru (@lszomoru) — when you have a chance, would you mind taking a look at the revised approach? The PR now routes repository display names through SCM core and removes the original Git-specific discovery plumbing. I’d particularly appreciate feedback on whether this direction fits the SCM API. No rush, and thank you for your time. CodeRabbit (@coderabbitai) Copilot can you initiate reviews as well? Perhaps focus on whether there are potential drawbacks to this. I'm force pushing to make rebases from time to time. The material commits are:
|
1ccdd0a to
768cc05
Compare
768cc05 to
5ca65eb
Compare
|
Extends SCM naming so Git submodules display customized .gitmodules names consistently while preserving existing fallback behavior. Changes:
Reviewed Changes
|
There was a problem hiding this comment.
Minor follow-up from local validation
For example:
| Configured name | Root basename | Displayed label |
|---|---|---|
zebra |
alpha |
parent / zebra |
apple |
zulu |
parent / apple |
The current implementation displays zebra and apple, but sorts using alpha and zulu. Selecting “Sort by Name” could therefore produce:
parent / zebra
parent / apple
The PR already makes the explicit name authoritative while preserving the existing workspace-folder, basename, and provider-label fallbacks.
I am going to try a small follow-up correction that sorts by the displayed provider name:
const nameComparison = compareFileNames(
op1.repository.provider.name,
op2.repository.provider.name
);The existing path tie-breaker and locale-aware comparator would remain unchanged.
I think this is justified within the PR because customized names would otherwise be displayed correctly but behave unexpectedly under the existing “Sort by Name” option. For callers that do not supply an explicit name, provider.name already resolves through the current fallbacks, so ordinary repositories should retain their existing ordering.
I do not plan to modify the pre-existing comparer tests. Their initial local failures came from the direct Electron test host inheriting zh-CN; all 51 relevant tests passed when the host was run with en-US. That appears separate from this small SCM sort-key adjustment.
8b7faad to
e30c9a0
Compare
|
I have pushed the follow-up correction discussed above.
The new regression test deliberately opposes provider names and root basenames, while giving two repositories the same provider name in reverse path order. It therefore verifies both that the displayed name controls sorting and that the existing path tie-breaker is preserved. The transport test now explicitly waits for the test RPC queue: Local validation included:
None of the 11 Electron failures involved Git or SCM, and all occurred in source/test files byte-identical to upstream The targeted coverage command also passed its tests, but the existing Electron coverage path produced an empty These results were obtained on the patch-equivalent validation worktree immediately before the branch was refreshed onto newer |
e4e1568 to
69d2baa
Compare
Document where repository names are rendered and sorted without changing runtime behavior. This exposes the relevant SCM lines for discussion before the sorting adjustment.
Synchronize the test RPC protocol before asserting registration and after disposing the source controls. This keeps the transport test aligned with the asynchronous RPC contract. Tested with the focused ExtHostSCM Electron unit suite.
Use the canonical SCM provider name for name sorting so explicit repository names follow the same order users see. Preserve the existing path tie-breaker and remove the redundant workspace-based name derivation. Add a focused SCMViewService regression test covering display names that oppose root basenames and duplicate-name path ordering. Tested with typecheck-client, targeted ESLint, and the focused Electron unit suite.
69d2baa to
c47bd38
Compare
Summary
.gitmodulesthroughout SCM UIscmProviderOptionsAPIAddresses #269812
Behavior
A submodule discovered through an open superproject uses its configured name only when that name differs from its path. The explicit name flows into
ISCMProvider.name, so the repositories tree, active repository status, SCM and history pickers, accessibility text, and Git repository picker remain consistent. Opening the submodule independently retains the current behavior.Validation
npm run typecheck-clientnpm run gulp compile-extension:gitnpm run valid-layers-checkgit diff --check upstream/main...HEADThe full
compile-extensionscommand was also attempted; it stopped in the unrelated CSS language extension because that extension's separate Node typings were not installed in this clone. Focused core and Git test launchers could not start because the local.build/electrontest runtime is absent.