Skip to content

repository: Only report repo roots that form a real submodule chain - #1423

Open
huntie wants to merge 1 commit into
facebook:mainfrom
huntie:isl-nested-repos
Open

repository: Only report repo roots that form a real submodule chain#1423
huntie wants to merge 1 commit into
facebook:mainfrom
huntie:isl-nested-repos

Conversation

@huntie

@huntie huntie commented Aug 28, 2026

Copy link
Copy Markdown

Summary

Fixes an edge case when using ISL inside a VS Code workspace with nested (non-submodule) Git repos.

Example: ~/devroot tracked with git, holding projects/react/ and projects/react-native/ as independent clones.

The UI suggests this is entirely supported — with one inner Git repo per configured workspace folder — and indeed the switching behaviour works.

However, ISL:

  • Bug 1: Misreports the repo name in the cwd dropdown when selected. The button is labelled with the outer repo (devroot) instead of the selected repo.
  • Bug 2: Overflows the UI component in this state (the dropdown arrow disappears).
Screenshot 2026-08-28 at 16 38 50

Root cause

debugroots walks up to the system root, so it reports every repo above the cwd, related or not. RepoInfo.repoRoots passes that through, but its consumers read it as a submodule chain: CwdSelector labels the button from repoRoots[0] and treats repoRoots.length > 1 as "submodule breadcrumbs follow", which suppresses the ButtonDropdown. Nothing renders in their place, because the outer repo has no submodules to offer. fetchSubmoduleMap also queried the unrelated ancestors.

Fix

Narrow the roots in getRepoInfo to the innermost run where each root is a submodule of the one above it, so the most local repo always leads. Ancestors are dropped when they do not declare the root below them, and when their submodules cannot be listed at all — an older sl without debuggitmodules already renders no breadcrumbs, so truncating costs nothing there.

Command count is unchanged in practice: the ancestors this drops are exactly the ones fetchSubmoduleMap no longer has to query.

Notes

The fetchSubmoduleMap 'nested' test mocked debugroots outermost-first, the reverse of what the command emits. Order never mattered before, since fetchSubmoduleMap iterates the list; it does now, so the mock is corrected.

Test Plan

After this fix:

Screenshot 2026-08-28 at 16 40 33

@meta-cla meta-cla Bot added the CLA Signed label Aug 28, 2026
@meta-codesync

meta-codesync Bot commented Aug 28, 2026

Copy link
Copy Markdown

This pull request has been imported. If you are a Meta employee, you can view this in D117876027. (Because this pull request was imported automatically, there will not be any future comments.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant