From 40af4db05f940d73b79cbde79bb5faf0ab98320b Mon Sep 17 00:00:00 2001 From: janbajc Date: Thu, 19 Mar 2026 23:28:28 +0100 Subject: [PATCH 1/5] feat: Add submodule discovery to repo selector --- .vscode/launch.json | 17 +++++ .vscode/tasks.json | 19 +++++ src/dataSource.ts | 42 +++++++++++ src/repoManager.ts | 1 + tests/backend/dataSource.submodules.test.ts | 81 +++++++++++++++++++++ 5 files changed, 160 insertions(+) create mode 100644 .vscode/launch.json create mode 100644 .vscode/tasks.json create mode 100644 tests/backend/dataSource.submodules.test.ts diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 00000000..28adecd6 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,17 @@ +{ + "version": "0.2.0", + "configurations": [ + { + "name": "Extension", + "type": "extensionHost", + "request": "launch", + "args": [ + "--extensionDevelopmentPath=${workspaceFolder}" + ], + "outFiles": [ + "${workspaceFolder}/out/**/*.js" + ], + "preLaunchTask": "npm: compile" + } + ] +} diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 00000000..92f2fac9 --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,19 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "npm: compile", + "type": "shell", + "command": "npm", + "args": [ + "run", + "compile" + ], + "problemMatcher": [], + "group": { + "kind": "build", + "isDefault": true + } + } + ] +} diff --git a/src/dataSource.ts b/src/dataSource.ts index 1e05a6aa..64ebaa59 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -1,3 +1,5 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; import * as cp from "node:child_process"; import { escapeRefName, getPathFromStr } from "./backend/utils"; @@ -463,3 +465,43 @@ export class DataSource { }); } } + + public getSubmodules(repo: string) { + return new Promise((resolve) => { + fs.readFile(path.join(repo, ".gitmodules"), { encoding: "utf8" }, async (err, data) => { + const submodules: string[] = []; + if (err) { + resolve(submodules); + return; + } + + const lines = data.split(eolRegex); + const sectionRegex = /^\s*\[.*\]\s*$/; + const submoduleSectionRegex = /^\s*\[submodule "([^"]+)"\]\s*$/; + const pathRegex = /^\s*path\s*=\s*(.*)$/; + let inSubmoduleSection = false; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (sectionRegex.test(line)) { + inSubmoduleSection = submoduleSectionRegex.test(line); + continue; + } + + if (!inSubmoduleSection) continue; + + const match = line.match(pathRegex); + if (match === null) continue; + + const submodulePath = getPathFromStr(path.resolve(repo, match[1].trim())); + if (submodules.includes(submodulePath)) continue; + + if (await this.isGitRepository(submodulePath)) { + submodules.push(submodulePath); + } + } + + resolve(submodules); + }); + }); + } diff --git a/src/repoManager.ts b/src/repoManager.ts index 3ad1fc21..b248ceda 100644 --- a/src/repoManager.ts +++ b/src/repoManager.ts @@ -95,6 +95,7 @@ export class RepoManager { if (!(await this.checkReposExist())) this.sendRepos(); await this.searchWorkspaceForRepos(); this.startWatchingFolders(); + if (await this.checkReposForNewSubmodules()) this.sendRepos(); } private removeReposNotInWorkspace() { diff --git a/tests/backend/dataSource.submodules.test.ts b/tests/backend/dataSource.submodules.test.ts new file mode 100644 index 00000000..d6f440fb --- /dev/null +++ b/tests/backend/dataSource.submodules.test.ts @@ -0,0 +1,81 @@ +import * as cp from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; + +vi.mock("vscode", () => ({ + workspace: { + getConfiguration: () => ({ + get: (_key: string, defaultValue: unknown) => defaultValue + }) + } +})); + +import { DataSource } from "../../src/dataSource"; + +function git(args: string[], cwd: string) { + cp.execFileSync("git", args, { cwd, stdio: "pipe" }); +} + +function makeRepo(prefix: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + try { + git(["init", "-b", "main"], dir); + } catch { + git(["init"], dir); + git(["checkout", "-b", "main"], dir); + } + git(["config", "user.email", "t@t.com"], dir); + git(["config", "user.name", "T"], dir); + fs.writeFileSync(path.join(dir, "f"), "x"); + git(["add", "."], dir); + git(["-c", "commit.gpgsign=false", "commit", "-m", "init"], dir); + return dir; +} + +let parentRepo: string; +let submoduleRepo: string; +let standaloneRepo: string; + +beforeAll(() => { + submoduleRepo = makeRepo("ngg-submodule-"); + parentRepo = makeRepo("ngg-parent-"); + standaloneRepo = makeRepo("ngg-plain-"); + + git( + [ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + submoduleRepo, + "modules/child" + ], + parentRepo + ); + git(["-c", "commit.gpgsign=false", "commit", "-am", "add submodule"], parentRepo); +}); + +afterAll(() => { + fs.rmSync(parentRepo, { recursive: true, force: true }); + fs.rmSync(submoduleRepo, { recursive: true, force: true }); + fs.rmSync(standaloneRepo, { recursive: true, force: true }); +}); + +describe("getSubmodules", () => { + it("returns initialized submodules declared in .gitmodules", async () => { + const dataSource = new DataSource(); + + const submodules = await dataSource.getSubmodules(parentRepo); + + expect(submodules).toEqual([path.join(parentRepo, "modules/child").replace(/\\/g, "/")]); + }); + + it("returns an empty array when a repository has no .gitmodules file", async () => { + const dataSource = new DataSource(); + + await expect(dataSource.getSubmodules(standaloneRepo)).resolves.toEqual([]); + }); +}); \ No newline at end of file From 9917f475d9bf42af555404a92d2f40d6189f8ef4 Mon Sep 17 00:00:00 2001 From: janbajc Date: Fri, 27 Mar 2026 05:56:26 +0100 Subject: [PATCH 2/5] fix: clean submodule discovery branch --- .vscode/launch.json | 17 ----------------- .vscode/tasks.json | 19 ------------------- src/dataSource.ts | 2 +- src/repoManager.ts | 39 +++++++++++++++++++++++++++++++++++---- 4 files changed, 36 insertions(+), 41 deletions(-) delete mode 100644 .vscode/launch.json delete mode 100644 .vscode/tasks.json diff --git a/.vscode/launch.json b/.vscode/launch.json deleted file mode 100644 index 28adecd6..00000000 --- a/.vscode/launch.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "version": "0.2.0", - "configurations": [ - { - "name": "Extension", - "type": "extensionHost", - "request": "launch", - "args": [ - "--extensionDevelopmentPath=${workspaceFolder}" - ], - "outFiles": [ - "${workspaceFolder}/out/**/*.js" - ], - "preLaunchTask": "npm: compile" - } - ] -} diff --git a/.vscode/tasks.json b/.vscode/tasks.json deleted file mode 100644 index 92f2fac9..00000000 --- a/.vscode/tasks.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "version": "2.0.0", - "tasks": [ - { - "label": "npm: compile", - "type": "shell", - "command": "npm", - "args": [ - "run", - "compile" - ], - "problemMatcher": [], - "group": { - "kind": "build", - "isDefault": true - } - } - ] -} diff --git a/src/dataSource.ts b/src/dataSource.ts index 64ebaa59..917e0986 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -464,7 +464,6 @@ export class DataSource { }); }); } -} public getSubmodules(repo: string) { return new Promise((resolve) => { @@ -505,3 +504,4 @@ export class DataSource { }); }); } +} diff --git a/src/repoManager.ts b/src/repoManager.ts index b248ceda..9418e921 100644 --- a/src/repoManager.ts +++ b/src/repoManager.ts @@ -124,9 +124,12 @@ export class RepoManager { public getRepos() { return sortRepos(this.repos); } - private addRepo(repo: string) { + private async addRepo(repo: string) { + if (typeof this.repos[repo] !== "undefined") return false; this.repos[repo] = { columnWidths: null }; + await this.searchRepoForSubmodules(repo); this.extensionState.saveRepos(this.repos); + return true; } private removeRepo(repo: string) { delete this.repos[repo]; @@ -175,6 +178,31 @@ export class RepoManager { this.repos[repo] = state; this.extensionState.saveRepos(this.repos); } + private async searchRepoForSubmodules(repo: string) { + let changes = false; + const submodules = await this.dataSource.getSubmodules(repo); + + for (let i = 0; i < submodules.length; i++) { + if (typeof this.repos[submodules[i]] !== "undefined") continue; + + this.repos[submodules[i]] = { columnWidths: null }; + changes = true; + if (await this.searchRepoForSubmodules(submodules[i])) changes = true; + } + + return changes; + } + private async checkReposForNewSubmodules() { + const repoPaths = Object.keys(this.repos); + let changes = false; + + for (let i = 0; i < repoPaths.length; i++) { + if (await this.searchRepoForSubmodules(repoPaths[i])) changes = true; + } + + if (changes) this.extensionState.saveRepos(this.repos); + return changes; + } /* Repo Searching */ private async searchWorkspaceForRepos() { @@ -205,8 +233,7 @@ export class RepoManager { .isGitRepository(directory) .then((isRepo) => { if (isRepo) { - this.addRepo(directory); - resolve(true); + this.addRepo(directory).then(resolve); } else if (maxDepth > 0) { fs.readdir(directory, async (err, dirContents) => { if (err) { @@ -290,6 +317,8 @@ export class RepoManager { while ((path = this.createEventPaths.shift())) { if (await isDirectory(path)) { if (await this.searchDirectoryForRepos(path, this.maxDepthOfRepoSearch)) changes = true; + } else if (path.endsWith("/.gitmodules")) { + if (await this.checkReposForNewSubmodules()) changes = true; } } this.processCreateEventsTimeout = null; @@ -299,7 +328,9 @@ export class RepoManager { let path, changes = false; while ((path = this.changeEventPaths.shift())) { - if (!(await doesPathExist(path))) { + if (path.endsWith("/.gitmodules")) { + if (await this.checkReposForNewSubmodules()) changes = true; + } else if (!(await doesPathExist(path))) { if (this.removeReposWithinFolder(path)) changes = true; } } From 444503375f4c853c9d50f6cff709f99edd573268 Mon Sep 17 00:00:00 2001 From: janbajc Date: Fri, 27 Mar 2026 06:22:22 +0100 Subject: [PATCH 3/5] fix: harden submodule path handling and improve rescan performance --- src/dataSource.ts | 7 ++++++- src/repoManager.ts | 9 +++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/dataSource.ts b/src/dataSource.ts index 917e0986..397a811f 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -492,7 +492,12 @@ export class DataSource { const match = line.match(pathRegex); if (match === null) continue; - const submodulePath = getPathFromStr(path.resolve(repo, match[1].trim())); + const resolvedPath = path.resolve(repo, match[1].trim()); + const relativeToRepo = path.relative(repo, resolvedPath); + // Ignore entries that escape the repository root. + if (relativeToRepo.startsWith("..") || path.isAbsolute(relativeToRepo)) continue; + + const submodulePath = getPathFromStr(resolvedPath); if (submodules.includes(submodulePath)) continue; if (await this.isGitRepository(submodulePath)) { diff --git a/src/repoManager.ts b/src/repoManager.ts index 9418e921..e6acfa68 100644 --- a/src/repoManager.ts +++ b/src/repoManager.ts @@ -194,11 +194,12 @@ export class RepoManager { } private async checkReposForNewSubmodules() { const repoPaths = Object.keys(this.repos); - let changes = false; + if (repoPaths.length === 0) return false; - for (let i = 0; i < repoPaths.length; i++) { - if (await this.searchRepoForSubmodules(repoPaths[i])) changes = true; - } + const results = await evalPromises(repoPaths, 2, (repoPath) => + this.searchRepoForSubmodules(repoPath) + ); + const changes = results.indexOf(true) > -1; if (changes) this.extensionState.saveRepos(this.repos); return changes; From 6e910bc6131aa7fdda76acfd8582f25bf99e33ed Mon Sep 17 00:00:00 2001 From: janbajc Date: Fri, 27 Mar 2026 06:31:47 +0100 Subject: [PATCH 4/5] fix: address copilot review feedback --- src/dataSource.ts | 30 ++++++++++----------- src/repoManager.ts | 13 +++++++-- tests/backend/dataSource.submodules.test.ts | 12 ++++----- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/dataSource.ts b/src/dataSource.ts index 397a811f..1814baf7 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -1,4 +1,3 @@ -import * as fs from "node:fs"; import * as path from "node:path"; import * as cp from "node:child_process"; @@ -467,32 +466,30 @@ export class DataSource { public getSubmodules(repo: string) { return new Promise((resolve) => { - fs.readFile(path.join(repo, ".gitmodules"), { encoding: "utf8" }, async (err, data) => { + this.execGit( + 'config -f .gitmodules --get-regexp "^submodule\\..*\\.path$"', + repo, + async (err, stdout) => { const submodules: string[] = []; if (err) { + // Missing .gitmodules or no submodule.path entries. resolve(submodules); return; } - const lines = data.split(eolRegex); - const sectionRegex = /^\s*\[.*\]\s*$/; - const submoduleSectionRegex = /^\s*\[submodule "([^"]+)"\]\s*$/; - const pathRegex = /^\s*path\s*=\s*(.*)$/; - let inSubmoduleSection = false; + const lines = stdout.split(eolRegex); for (let i = 0; i < lines.length; i++) { const line = lines[i]; - if (sectionRegex.test(line)) { - inSubmoduleSection = submoduleSectionRegex.test(line); - continue; - } + if (line.trim() === "") continue; - if (!inSubmoduleSection) continue; + const separatorPos = line.search(/\s/); + if (separatorPos === -1) continue; - const match = line.match(pathRegex); - if (match === null) continue; + const rawPath = line.slice(separatorPos + 1).trim(); + if (rawPath === "") continue; - const resolvedPath = path.resolve(repo, match[1].trim()); + const resolvedPath = path.resolve(repo, rawPath); const relativeToRepo = path.relative(repo, resolvedPath); // Ignore entries that escape the repository root. if (relativeToRepo.startsWith("..") || path.isAbsolute(relativeToRepo)) continue; @@ -506,7 +503,8 @@ export class DataSource { } resolve(submodules); - }); + } + ); }); } } diff --git a/src/repoManager.ts b/src/repoManager.ts index e6acfa68..6e9ce348 100644 --- a/src/repoManager.ts +++ b/src/repoManager.ts @@ -204,6 +204,13 @@ export class RepoManager { if (changes) this.extensionState.saveRepos(this.repos); return changes; } + private async checkRepoForNewSubmodules(repo: string) { + if (typeof this.repos[repo] === "undefined") return false; + + const changes = await this.searchRepoForSubmodules(repo); + if (changes) this.extensionState.saveRepos(this.repos); + return changes; + } /* Repo Searching */ private async searchWorkspaceForRepos() { @@ -319,7 +326,8 @@ export class RepoManager { if (await isDirectory(path)) { if (await this.searchDirectoryForRepos(path, this.maxDepthOfRepoSearch)) changes = true; } else if (path.endsWith("/.gitmodules")) { - if (await this.checkReposForNewSubmodules()) changes = true; + const repo = path.slice(0, -"/.gitmodules".length); + if (await this.checkRepoForNewSubmodules(repo)) changes = true; } } this.processCreateEventsTimeout = null; @@ -330,7 +338,8 @@ export class RepoManager { changes = false; while ((path = this.changeEventPaths.shift())) { if (path.endsWith("/.gitmodules")) { - if (await this.checkReposForNewSubmodules()) changes = true; + const repo = path.slice(0, -"/.gitmodules".length); + if (await this.checkRepoForNewSubmodules(repo)) changes = true; } else if (!(await doesPathExist(path))) { if (this.removeReposWithinFolder(path)) changes = true; } diff --git a/tests/backend/dataSource.submodules.test.ts b/tests/backend/dataSource.submodules.test.ts index d6f440fb..3bbd12f5 100644 --- a/tests/backend/dataSource.submodules.test.ts +++ b/tests/backend/dataSource.submodules.test.ts @@ -35,9 +35,9 @@ function makeRepo(prefix: string): string { return dir; } -let parentRepo: string; -let submoduleRepo: string; -let standaloneRepo: string; +let parentRepo = ""; +let submoduleRepo = ""; +let standaloneRepo = ""; beforeAll(() => { submoduleRepo = makeRepo("ngg-submodule-"); @@ -59,9 +59,9 @@ beforeAll(() => { }); afterAll(() => { - fs.rmSync(parentRepo, { recursive: true, force: true }); - fs.rmSync(submoduleRepo, { recursive: true, force: true }); - fs.rmSync(standaloneRepo, { recursive: true, force: true }); + if (parentRepo !== "") fs.rmSync(parentRepo, { recursive: true, force: true }); + if (submoduleRepo !== "") fs.rmSync(submoduleRepo, { recursive: true, force: true }); + if (standaloneRepo !== "") fs.rmSync(standaloneRepo, { recursive: true, force: true }); }); describe("getSubmodules", () => { From a66f016957c6e8a2306a88cae9e85cfbacb586dc Mon Sep 17 00:00:00 2001 From: janbajc Date: Fri, 27 Mar 2026 06:38:53 +0100 Subject: [PATCH 5/5] fix: use execFile for gitmodules parsing --- src/dataSource.ts | 59 ++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/src/dataSource.ts b/src/dataSource.ts index 1814baf7..6c02a1d8 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -204,9 +204,9 @@ export class DataSource { }); } - public isGitRepository(path: string) { + public isGitRepository(repoPath: string) { return new Promise((resolve) => { - this.execGit("rev-parse --git-dir", path, (err) => { + this.execGit("rev-parse --git-dir", repoPath, (err) => { resolve(!err); }); }); @@ -466,43 +466,44 @@ export class DataSource { public getSubmodules(repo: string) { return new Promise((resolve) => { - this.execGit( - 'config -f .gitmodules --get-regexp "^submodule\\..*\\.path$"', - repo, + cp.execFile( + this.gitPath, + ["config", "-f", ".gitmodules", "--get-regexp", "^submodule\\..*\\.path$"], + { cwd: repo }, async (err, stdout) => { - const submodules: string[] = []; - if (err) { - // Missing .gitmodules or no submodule.path entries. - resolve(submodules); - return; - } + const submodules: string[] = []; + if (err) { + // Missing .gitmodules or no submodule.path entries. + resolve(submodules); + return; + } - const lines = stdout.split(eolRegex); + const lines = stdout.split(eolRegex); - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - if (line.trim() === "") continue; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (line.trim() === "") continue; - const separatorPos = line.search(/\s/); - if (separatorPos === -1) continue; + const separatorPos = line.search(/\s/); + if (separatorPos === -1) continue; - const rawPath = line.slice(separatorPos + 1).trim(); - if (rawPath === "") continue; + const rawPath = line.slice(separatorPos + 1).trim(); + if (rawPath === "") continue; - const resolvedPath = path.resolve(repo, rawPath); - const relativeToRepo = path.relative(repo, resolvedPath); - // Ignore entries that escape the repository root. - if (relativeToRepo.startsWith("..") || path.isAbsolute(relativeToRepo)) continue; + const resolvedPath = path.resolve(repo, rawPath); + const relativeToRepo = path.relative(repo, resolvedPath); + // Ignore entries that escape the repository root. + if (relativeToRepo.startsWith("..") || path.isAbsolute(relativeToRepo)) continue; - const submodulePath = getPathFromStr(resolvedPath); - if (submodules.includes(submodulePath)) continue; + const submodulePath = getPathFromStr(resolvedPath); + if (submodules.includes(submodulePath)) continue; - if (await this.isGitRepository(submodulePath)) { - submodules.push(submodulePath); + if (await this.isGitRepository(submodulePath)) { + submodules.push(submodulePath); + } } - } - resolve(submodules); + resolve(submodules); } ); });