From 8a5aaf93a1f00781786c87d422af6bbc309cd794 Mon Sep 17 00:00:00 2001 From: lzr <43104914+lizhuoran1019@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:28:38 +0000 Subject: [PATCH 1/8] feat(git): add list/tree toggle for changed files in the Git panel Extract the Git panel into a dedicated `workspace-git-panel` Lit component (mirroring the Files panel) and add a segmented List/Tree toggle next to Refresh. Tree view builds an in-memory, collapsible directory tree from the changed files, starts fully collapsed, and offers a single expand-all/collapse-all button (visible only in tree view). List view keeps the existing flat, full-path rows. The selected view mode persists in localStorage under `pi-web.gitFileView`; per-directory expand state is intentionally ephemeral. - gitFileViewPreference.ts: localStorage-backed view preference (+ test) - gitFileTree.ts: pure flat-paths -> nested-tree builder (+ test) - WorkspaceGitPanel.ts: the panel component with toggle + tree state --- .changeset/git-file-tree-view.md | 5 + .../src/components/WorkspaceGitPanel.ts | 208 ++++++++++++++++++ src/client/src/gitFileTree.test.ts | 75 +++++++ src/client/src/gitFileTree.ts | 97 ++++++++ src/client/src/gitFileViewPreference.test.ts | 66 ++++++ src/client/src/gitFileViewPreference.ts | 40 ++++ src/client/src/plugins/core/panels.ts | 67 +----- 7 files changed, 493 insertions(+), 65 deletions(-) create mode 100644 .changeset/git-file-tree-view.md create mode 100644 src/client/src/components/WorkspaceGitPanel.ts create mode 100644 src/client/src/gitFileTree.test.ts create mode 100644 src/client/src/gitFileTree.ts create mode 100644 src/client/src/gitFileViewPreference.test.ts create mode 100644 src/client/src/gitFileViewPreference.ts diff --git a/.changeset/git-file-tree-view.md b/.changeset/git-file-tree-view.md new file mode 100644 index 0000000..34c7dfa --- /dev/null +++ b/.changeset/git-file-tree-view.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Add a List/Tree toggle to the Git panel's changed-file list. Tree view groups changes by directory and opens fully collapsed, with a one-click expand-all/collapse-all control, and the chosen view is remembered across sessions. diff --git a/src/client/src/components/WorkspaceGitPanel.ts b/src/client/src/components/WorkspaceGitPanel.ts new file mode 100644 index 0000000..5cca4f8 --- /dev/null +++ b/src/client/src/components/WorkspaceGitPanel.ts @@ -0,0 +1,208 @@ +import { css, html, LitElement, type PropertyValues, type TemplateResult } from "lit"; +import { customElement, property, state } from "lit/decorators.js"; +import type { GitDiffResponse, GitStatusFile, GitStatusResponse } from "../api"; +import { buildGitFileTree, collectGitFileTreeDirectoryPaths, type GitFileTreeNode } from "../gitFileTree"; +import { readGitFileView, writeGitFileView, type GitFileView } from "../gitFileViewPreference"; +import type { WorkspacePanelContext } from "../plugins/types"; +import { workspacePanelStyles } from "./shared"; + +interface GitTreeState { + readonly nodes: readonly GitFileTreeNode[]; + readonly directoryPaths: readonly string[]; +} + +const EMPTY_TREE_STATE: GitTreeState = { nodes: [], directoryPaths: [] }; + +@customElement("workspace-git-panel") +export class WorkspaceGitPanel extends LitElement { + static override styles = [ + workspacePanelStyles, + css` + :host { flex: 1 1 auto; } + .toolbar-actions { display: flex; align-items: center; gap: 8px; margin-left: auto; } + .toolbar .toolbar-actions button { margin-left: 0; } + .view-toggle { display: inline-flex; } + .view-toggle button { border-radius: 0; } + .view-toggle button:first-child { border-top-left-radius: 7px; border-bottom-left-radius: 7px; } + .view-toggle button:last-child { border-top-right-radius: 7px; border-bottom-right-radius: 7px; margin-left: -1px; } + .view-toggle button.selected { position: relative; z-index: 1; } + .row .twisty { color: var(--pi-dim, var(--pi-muted)); } + `, + ]; + + @property({ attribute: false }) context: WorkspacePanelContext | undefined; + + // Persisted across sessions via localStorage; only the mode is remembered. + @state() private view: GitFileView = readGitFileView(); + + // Ephemeral by design: the tree always opens fully collapsed, and expand + // state is intentionally not persisted. Reassigned (never mutated in place) + // so Lit observes the change. + @state() private expandedDirectories = new Set(); + + protected override willUpdate(changedProperties: PropertyValues): void { + if (!changedProperties.has("context")) return; + const previous = changedProperties.get("context"); + if (previous !== undefined && this.context !== undefined && gitPanelContextKey(previous) !== gitPanelContextKey(this.context)) { + // Switching workspace/machine resets the ephemeral expand state. + this.expandedDirectories = new Set(); + } + } + + override render(): TemplateResult { + const context = this.context; + if (context === undefined) return html`

Git unavailable.

`; + const status = context.gitStatus; + const treeState = this.computeTreeState(status); + return html` +
+ Git + ${context.gitStale ? html`stale` : null} +
+ ${this.renderViewToggle()} + ${this.view === "tree" && treeState.directoryPaths.length > 0 ? this.renderExpandCollapseAll(treeState.directoryPaths) : null} + +
+
+
+
+ ${this.renderFileList(context, status, treeState.nodes)} +
+
${renderDiffViewer(context)}
+
+ `; + } + + private renderViewToggle(): TemplateResult { + return html` +
+ ${this.renderViewToggleButton("list", "List")} + ${this.renderViewToggleButton("tree", "Tree")} +
+ `; + } + + private renderViewToggleButton(view: GitFileView, label: string): TemplateResult { + const active = this.view === view; + return html` + + `; + } + + private renderExpandCollapseAll(directoryPaths: readonly string[]): TemplateResult { + const allExpanded = directoryPaths.every((path) => this.expandedDirectories.has(path)); + return html` + + `; + } + + private renderFileList(context: WorkspacePanelContext, status: GitStatusResponse | undefined, nodes: readonly GitFileTreeNode[]): TemplateResult { + if (status === undefined) return html`

No status loaded.

`; + if (!status.isGitRepo) return html`

Not a git repository.

`; + const summary = html`

${gitSummary(status)}

`; + if (status.files.length === 0) return html`${summary}

No changes.

`; + const body = this.view === "tree" + ? nodes.map((node) => this.renderTreeNode(context, node, 0)) + : status.files.map((file) => this.renderFileRow(context, file)); + return html`${summary}${body}`; + } + + private renderTreeNode(context: WorkspacePanelContext, node: GitFileTreeNode, depth: number): TemplateResult { + if (node.kind === "directory") { + const expanded = this.expandedDirectories.has(node.path); + return html` + + ${expanded ? node.children.map((child) => this.renderTreeNode(context, child, depth + 1)) : null} + `; + } + const selected = context.selectedDiffPath === node.path; + return html` + + `; + } + + private renderFileRow(context: WorkspacePanelContext, file: GitStatusFile): TemplateResult { + const selected = context.selectedDiffPath === file.path; + return html` + + `; + } + + private computeTreeState(status: GitStatusResponse | undefined): GitTreeState { + if (this.view !== "tree" || status === undefined || !status.isGitRepo || status.files.length === 0) return EMPTY_TREE_STATE; + const nodes = buildGitFileTree(status.files); + return { nodes, directoryPaths: collectGitFileTreeDirectoryPaths(nodes) }; + } + + private setView(view: GitFileView): void { + if (this.view === view) return; + this.view = view; + writeGitFileView(view); + // The tree always starts fully collapsed when entered. + if (view === "tree") this.expandedDirectories = new Set(); + } + + private toggleDirectory(path: string): void { + const next = new Set(this.expandedDirectories); + if (next.has(path)) next.delete(path); + else next.add(path); + this.expandedDirectories = next; + } + + private toggleExpandAll(directoryPaths: readonly string[], allExpanded: boolean): void { + this.expandedDirectories = allExpanded ? new Set() : new Set(directoryPaths); + } +} + +function renderDiffViewer(context: WorkspacePanelContext): TemplateResult { + if (context.selectedDiffPath === undefined || context.selectedDiffPath === "") return html`

Select a changed file.

`; + const unstaged = context.selectedDiff; + const staged = context.selectedStagedDiff; + if (unstaged === undefined || staged === undefined) return html`

Loading diff…

`; + const diffs = [staged, unstaged].filter((diff) => diff.diff !== ""); + if (diffs.length === 0) return html`

No staged or unstaged diff.

`; + return html` +
+ ${diffs.map((diff) => renderDiffSection(diff))} +
+ `; +} + +function renderDiffSection(diff: GitDiffResponse): TemplateResult { + loadUnifiedDiffViewer(); + return html` +
+
${diff.path ?? "diff"}${diff.staged ? "staged" : "unstaged"}${diff.truncated ? " · truncated" : ""}
+ +
+ `; +} + +function loadUnifiedDiffViewer(): void { + void import("./UnifiedDiffViewer"); +} + +function gitSummary(status: GitStatusResponse): string { + const branch = status.branch ?? "detached"; + const ahead = status.ahead ?? 0; + const behind = status.behind ?? 0; + return ahead === 0 && behind === 0 ? branch : `${branch} · ↑${String(ahead)} ↓${String(behind)}`; +} + +function stateLabel(index: string, workingTree: string): string { + const label = workingTree !== "unmodified" ? workingTree : index; + return label.slice(0, 1).toUpperCase(); +} + +function gitPanelContextKey(context: WorkspacePanelContext): string { + return `${context.machine.id}:${context.workspace.projectId}:${context.workspace.id}`; +} diff --git a/src/client/src/gitFileTree.test.ts b/src/client/src/gitFileTree.test.ts new file mode 100644 index 0000000..efa1816 --- /dev/null +++ b/src/client/src/gitFileTree.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from "vitest"; +import type { GitStatusFile } from "./api"; +import { + buildGitFileTree, + collectGitFileTreeDirectoryPaths, + type GitFileTreeDirectoryNode, + type GitFileTreeFileNode, + type GitFileTreeNode, +} from "./gitFileTree"; + +describe("buildGitFileTree", () => { + it("returns an empty tree for no changes", () => { + expect(buildGitFileTree([])).toEqual([]); + expect(collectGitFileTreeDirectoryPaths([])).toEqual([]); + }); + + it("keeps root-level files at the root", () => { + const tree = buildGitFileTree([changed("b.txt"), changed("a.txt")]); + expect(tree.map((node) => node.kind)).toEqual(["file", "file"]); + expect(tree.map((node) => node.name)).toEqual(["a.txt", "b.txt"]); + expect(collectGitFileTreeDirectoryPaths(tree)).toEqual([]); + }); + + it("nests files under merged directories, directories before files, alphabetical", () => { + const tree = buildGitFileTree([ + changed("src/a.ts"), + changed("src/b/c.ts"), + changed("README.md"), + changed("src/b/a.ts"), + ]); + + expect(tree).toHaveLength(2); + const src = expectDirectory(tree[0]); + expect(src.name).toBe("src"); + expect(src.path).toBe("src"); + expect(expectFile(tree[1]).name).toBe("README.md"); + + // src groups the shared "b" directory once, ordered before the loose file. + expect(src.children).toHaveLength(2); + const srcB = expectDirectory(src.children[0]); + expect(srcB.name).toBe("b"); + expect(srcB.path).toBe("src/b"); + expect(expectFile(src.children[1]).path).toBe("src/a.ts"); + + expect(srcB.children.map((node) => node.name)).toEqual(["a.ts", "c.ts"]); + expect(srcB.children.map((node) => expectFile(node).path)).toEqual(["src/b/a.ts", "src/b/c.ts"]); + }); + + it("shows the basename on leaves while keeping the full path and original file", () => { + const original = changed("deep/nested/file.ts"); + const leaf = expectFile(expectDirectory(expectDirectory(buildGitFileTree([original])[0]).children[0]).children[0]); + expect(leaf.name).toBe("file.ts"); + expect(leaf.path).toBe("deep/nested/file.ts"); + expect(leaf.file).toBe(original); + }); + + it("collects every directory path in pre-order", () => { + const tree = buildGitFileTree([changed("src/b/c.ts"), changed("src/a.ts"), changed("docs/x.md")]); + expect(collectGitFileTreeDirectoryPaths(tree)).toEqual(["docs", "src", "src/b"]); + }); +}); + +function changed(path: string): GitStatusFile { + return { path, index: "modified", workingTree: "modified" }; +} + +function expectDirectory(node: GitFileTreeNode | undefined): GitFileTreeDirectoryNode { + if (node?.kind !== "directory") throw new Error(`expected a directory node, received ${node?.kind ?? "undefined"}`); + return node; +} + +function expectFile(node: GitFileTreeNode | undefined): GitFileTreeFileNode { + if (node?.kind !== "file") throw new Error(`expected a file node, received ${node?.kind ?? "undefined"}`); + return node; +} diff --git a/src/client/src/gitFileTree.ts b/src/client/src/gitFileTree.ts new file mode 100644 index 0000000..2d0f097 --- /dev/null +++ b/src/client/src/gitFileTree.ts @@ -0,0 +1,97 @@ +import type { GitStatusFile } from "./api"; + +/** + * A changed file placed at a leaf of the Git file tree. `path` is the full + * repository-relative path (used to load its diff); `name` is just the final + * segment shown in the tree row. + */ +export interface GitFileTreeFileNode { + readonly kind: "file"; + readonly name: string; + readonly path: string; + readonly file: GitStatusFile; +} + +/** + * A directory grouping in the Git file tree. `path` is the full directory path + * and doubles as the stable key used to track expand/collapse state. + */ +export interface GitFileTreeDirectoryNode { + readonly kind: "directory"; + readonly name: string; + readonly path: string; + readonly children: readonly GitFileTreeNode[]; +} + +export type GitFileTreeNode = GitFileTreeDirectoryNode | GitFileTreeFileNode; + +interface DirectoryAccumulator { + readonly path: string; + readonly directories: Map; + readonly files: GitFileTreeFileNode[]; +} + +/** + * Build a nested directory/file tree from Git's flat changed-file list. The + * status response already carries every changed path, so this is a pure + * client-side transform (no lazy per-directory loading like the Files tab). + * Directories sort before files, and both sort alphabetically within a level. + */ +export function buildGitFileTree(files: readonly GitStatusFile[]): GitFileTreeNode[] { + const root = createDirectoryAccumulator(""); + for (const file of files) { + const segments = file.path.split("/").filter((segment) => segment.length > 0); + const name = segments[segments.length - 1]; + if (name === undefined) continue; + let directory = root; + for (let index = 0; index < segments.length - 1; index += 1) { + const segment = segments[index]; + if (segment === undefined) continue; + const childPath = directory.path.length === 0 ? segment : `${directory.path}/${segment}`; + const existing = directory.directories.get(childPath); + if (existing === undefined) { + const created = createDirectoryAccumulator(childPath); + directory.directories.set(childPath, created); + directory = created; + } else { + directory = existing; + } + } + directory.files.push({ kind: "file", name, path: file.path, file }); + } + return finalizeChildren(root); +} + +/** + * Every directory path in the tree, in a stable order. Used to drive the + * expand-all / collapse-all control and to decide whether the tree is fully + * expanded. + */ +export function collectGitFileTreeDirectoryPaths(nodes: readonly GitFileTreeNode[]): string[] { + const paths: string[] = []; + for (const node of nodes) { + if (node.kind === "directory") { + paths.push(node.path); + paths.push(...collectGitFileTreeDirectoryPaths(node.children)); + } + } + return paths; +} + +function createDirectoryAccumulator(path: string): DirectoryAccumulator { + return { path, directories: new Map(), files: [] }; +} + +function finalizeChildren(directory: DirectoryAccumulator): GitFileTreeNode[] { + const directories: GitFileTreeDirectoryNode[] = [...directory.directories.values()] + .map((child): GitFileTreeDirectoryNode => ({ kind: "directory", name: segmentName(child.path), path: child.path, children: finalizeChildren(child) })) + .sort((left, right) => left.name.localeCompare(right.name)); + const files = [...directory.files].sort((left, right) => left.name.localeCompare(right.name)); + return [...directories, ...files]; +} + +function segmentName(path: string): string { + const segments = path.split("/"); + const last = segments[segments.length - 1]; + return last ?? path; +} diff --git a/src/client/src/gitFileViewPreference.test.ts b/src/client/src/gitFileViewPreference.test.ts new file mode 100644 index 0000000..470ac70 --- /dev/null +++ b/src/client/src/gitFileViewPreference.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; +import { + GIT_FILE_VIEW_STORAGE_KEY, + parseGitFileView, + readGitFileView, + writeGitFileView, +} from "./gitFileViewPreference"; + +describe("gitFileViewPreference", () => { + it("parses stored values and defaults unknown/missing input to list", () => { + expect(parseGitFileView("tree")).toBe("tree"); + expect(parseGitFileView("list")).toBe("list"); + expect(parseGitFileView(null)).toBe("list"); + expect(parseGitFileView("")).toBe("list"); + expect(parseGitFileView("grid")).toBe("list"); + }); + + it("defaults to list when nothing is stored yet", () => { + expect(readGitFileView(new FakeStorage())).toBe("list"); + }); + + it("reads and writes the stored view mode", () => { + const storage = new FakeStorage(); + + writeGitFileView("tree", storage); + expect(storage.value(GIT_FILE_VIEW_STORAGE_KEY)).toBe("tree"); + expect(readGitFileView(storage)).toBe("tree"); + + writeGitFileView("list", storage); + expect(storage.value(GIT_FILE_VIEW_STORAGE_KEY)).toBe("list"); + expect(readGitFileView(storage)).toBe("list"); + }); + + it("ignores storage failures and falls back to list", () => { + const storage = new ThrowingStorage(); + + expect(readGitFileView(storage)).toBe("list"); + expect(() => { writeGitFileView("tree", storage); }).not.toThrow(); + }); +}); + +class FakeStorage { + private readonly values = new Map(); + + getItem(key: string): string | null { + return this.values.get(key) ?? null; + } + + setItem(key: string, value: string): void { + this.values.set(key, value); + } + + value(key: string): string | undefined { + return this.values.get(key); + } +} + +class ThrowingStorage { + getItem(): string | null { + throw new Error("blocked"); + } + + setItem(): void { + throw new Error("blocked"); + } +} diff --git a/src/client/src/gitFileViewPreference.ts b/src/client/src/gitFileViewPreference.ts new file mode 100644 index 0000000..5f7ebb7 --- /dev/null +++ b/src/client/src/gitFileViewPreference.ts @@ -0,0 +1,40 @@ +export const GIT_FILE_VIEW_STORAGE_KEY = "pi-web.gitFileView"; + +/** + * How the Git panel lists changed files. `list` is the flat, full-path view + * (the historical default); `tree` nests files under their directories. + */ +export type GitFileView = "list" | "tree"; + +export type GitFileViewStorage = Pick; + +export function parseGitFileView(value: string | null): GitFileView { + return value === "tree" ? "tree" : "list"; +} + +export function readGitFileView(storage = browserStorage()): GitFileView { + if (storage === undefined) return "list"; + try { + return parseGitFileView(storage.getItem(GIT_FILE_VIEW_STORAGE_KEY)); + } catch { + return "list"; + } +} + +export function writeGitFileView(view: GitFileView, storage = browserStorage()): void { + if (storage === undefined) return; + try { + storage.setItem(GIT_FILE_VIEW_STORAGE_KEY, view); + } catch { + // Ignore localStorage quota/privacy errors; the chosen view still applies in memory for this tab. + } +} + +function browserStorage(): GitFileViewStorage | undefined { + if (typeof window === "undefined") return undefined; + try { + return window.localStorage; + } catch { + return undefined; + } +} diff --git a/src/client/src/plugins/core/panels.ts b/src/client/src/plugins/core/panels.ts index 4c2dca6..7d57e18 100644 --- a/src/client/src/plugins/core/panels.ts +++ b/src/client/src/plugins/core/panels.ts @@ -1,7 +1,7 @@ import { html, type TemplateResult } from "lit"; -import type { GitDiffResponse, GitStatusResponse } from "../../api"; import { renderBuiltinTabIcon } from "../../components/tabIcons"; import "../../components/WorkspaceFilesPanel"; +import "../../components/WorkspaceGitPanel"; import type { WorkspacePanelContribution, WorkspacePanelContext } from "../types"; export function createCoreWorkspacePanels(): WorkspacePanelContribution[] { @@ -42,72 +42,9 @@ function renderTerminal(context: WorkspacePanelContext): TemplateResult { } function renderGit(context: WorkspacePanelContext): TemplateResult { - const status = context.gitStatus; - return html` -
- Git - ${context.gitStale ? html`stale` : null} - -
-
-
- ${status === undefined ? html`

No status loaded.

` : !status.isGitRepo ? html`

Not a git repository.

` : html` -

${gitSummary(status)}

- ${status.files.length === 0 ? html`

No changes.

` : status.files.map((file) => html` - - `)} - `} -
-
- ${renderDiffViewer(context)} -
-
- `; -} - -function renderDiffViewer(context: WorkspacePanelContext): TemplateResult { - if (context.selectedDiffPath === undefined || context.selectedDiffPath === "") return html`

Select a changed file.

`; - const unstaged = context.selectedDiff; - const staged = context.selectedStagedDiff; - if (unstaged === undefined || staged === undefined) return html`

Loading diff…

`; - const diffs = [staged, unstaged].filter((diff) => diff.diff !== ""); - if (diffs.length === 0) return html`

No staged or unstaged diff.

`; - return html` -
- ${diffs.map((diff) => renderDiffSection(diff))} -
- `; -} - -function renderDiffSection(diff: GitDiffResponse): TemplateResult { - loadUnifiedDiffViewer(); - return html` -
-
${diff.path ?? "diff"}${diff.staged ? "staged" : "unstaged"}${diff.truncated ? " · truncated" : ""}
- -
- `; -} - -function loadUnifiedDiffViewer(): void { - void import("../../components/UnifiedDiffViewer"); + return html``; } function loadTerminalPanel(): void { void import("../../components/TerminalPanel"); } - -function gitSummary(status: GitStatusResponse): string { - const branch = status.branch ?? "detached"; - const ahead = status.ahead ?? 0; - const behind = status.behind ?? 0; - return ahead === 0 && behind === 0 ? branch : `${branch} · ↑${String(ahead)} ↓${String(behind)}`; -} - -function stateLabel(index: string, workingTree: string): string { - const label = workingTree !== "unmodified" ? workingTree : index; - return label.slice(0, 1).toUpperCase(); -} From dd435cb1b0d9016546965b7079adef7d613fd3bb Mon Sep 17 00:00:00 2001 From: lzr <43104914+lizhuoran1019@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:34:54 +0000 Subject: [PATCH 2/8] feat(git): view submodule working-tree changes in the Git panel Recurse into dirty submodules when building the Git status so their modified and untracked files appear as full-path entries, and add a commit-pointer entry (with short SHAs) only when the recorded commit actually moved. Route diffs whose path falls inside a submodule to run in that submodule's working tree so real per-file diffs are shown. The changed-file list groups these under the submodule: tree view keeps the nested structure and marks the submodule root with a badge, list view flattens them into one expandable group pinned above the ordinary files. Depth 1 only; ignored files are excluded; the panel stays read-only. Covered by client tree/list-grouping tests, parser tests, and a server test that drives a real temporary repository and submodule. --- .changeset/git-submodule-changes.md | 5 + src/client/src/api/parsers.test.ts | 22 +++ src/client/src/api/parsers.ts | 4 +- .../src/components/WorkspaceGitPanel.ts | 100 ++++++---- .../src/controllers/machineController.test.ts | 2 +- src/client/src/gitFileList.test.ts | 47 +++++ src/client/src/gitFileList.ts | 84 +++++++++ src/client/src/gitFileTree.test.ts | 42 +++++ src/client/src/gitFileTree.ts | 68 +++++-- src/server/git/gitService.test.ts | 111 +++++++++++ src/server/git/gitService.ts | 172 +++++++++++++++++- src/shared/apiTypes.ts | 9 + 12 files changed, 610 insertions(+), 56 deletions(-) create mode 100644 .changeset/git-submodule-changes.md create mode 100644 src/client/src/gitFileList.test.ts create mode 100644 src/client/src/gitFileList.ts create mode 100644 src/server/git/gitService.test.ts diff --git a/.changeset/git-submodule-changes.md b/.changeset/git-submodule-changes.md new file mode 100644 index 0000000..adb8277 --- /dev/null +++ b/.changeset/git-submodule-changes.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Expand a changed submodule in the Git panel to see the work inside it. Tree view nests the submodule's own modified and untracked files (keeping their folder structure) and list view flattens them into one group, with a moved commit pointer shown as ` → ` when it changed. Selecting any inner file shows its real diff instead of the bare `Subproject commit` line. diff --git a/src/client/src/api/parsers.test.ts b/src/client/src/api/parsers.test.ts index 4c02766..2533399 100644 --- a/src/client/src/api/parsers.test.ts +++ b/src/client/src/api/parsers.test.ts @@ -505,6 +505,28 @@ describe("API parsers", () => { expect(() => parseGitStatusResponse({ isGitRepo: true, hash: "h", files: [{ path: "a", index: "weird", workingTree: "modified" }] })).toThrow("Invalid git file state"); }); + it("parses submodule paths and pointer commit fields", () => { + const parsed = parseGitStatusResponse({ + isGitRepo: true, + hash: "h", + branch: "main", + files: [ + { path: "HARL", index: "unmodified", workingTree: "modified", submoduleFromCommit: "1111111", submoduleToCommit: "2222222" }, + { path: "HARL/inner.txt", index: "modified", workingTree: "modified" }, + ], + submodules: ["HARL"], + }); + expect(parsed.submodules).toEqual(["HARL"]); + expect(parsed.files[0]?.submoduleFromCommit).toBe("1111111"); + expect(parsed.files[0]?.submoduleToCommit).toBe("2222222"); + expect(parsed.files[1]?.submoduleFromCommit).toBeUndefined(); + }); + + it("defaults submodules to an empty list when absent", () => { + const parsed = parseGitStatusResponse({ isGitRepo: true, hash: "h", files: [] }); + expect(parsed.submodules).toEqual([]); + }); + it("validates file content responses", () => { const textFile = { path: "README.md", diff --git a/src/client/src/api/parsers.ts b/src/client/src/api/parsers.ts index e9fb592..52ce3da 100644 --- a/src/client/src/api/parsers.ts +++ b/src/client/src/api/parsers.ts @@ -818,12 +818,12 @@ function optionalFileMediaType(value: unknown): FileContentResponse["mediaType"] export function parseGitStatusResponse(value: unknown): GitStatusResponse { const record = requireRecord(value); - return { isGitRepo: requireBoolean(record, "isGitRepo"), hash: requireString(record, "hash"), ...optionalField("branch", optionalString(record, "branch")), ...optionalField("upstream", optionalString(record, "upstream")), ...optionalField("ahead", optionalNumber(record, "ahead")), ...optionalField("behind", optionalNumber(record, "behind")), files: arrayOf(parseGitStatusFile)(record["files"]) }; + return { isGitRepo: requireBoolean(record, "isGitRepo"), hash: requireString(record, "hash"), ...optionalField("branch", optionalString(record, "branch")), ...optionalField("upstream", optionalString(record, "upstream")), ...optionalField("ahead", optionalNumber(record, "ahead")), ...optionalField("behind", optionalNumber(record, "behind")), files: arrayOf(parseGitStatusFile)(record["files"]), submodules: record["submodules"] === undefined ? [] : arrayOfString(record["submodules"], "submodules") }; } function parseGitStatusFile(value: unknown): GitStatusFile { const record = requireRecord(value); - return { path: requireString(record, "path"), ...optionalField("oldPath", optionalString(record, "oldPath")), index: parseGitFileState(record["index"]), workingTree: parseGitFileState(record["workingTree"]) }; + return { path: requireString(record, "path"), ...optionalField("oldPath", optionalString(record, "oldPath")), index: parseGitFileState(record["index"]), workingTree: parseGitFileState(record["workingTree"]), ...optionalField("submoduleFromCommit", optionalString(record, "submoduleFromCommit")), ...optionalField("submoduleToCommit", optionalString(record, "submoduleToCommit")) }; } function parseGitFileState(value: unknown): GitFileState { diff --git a/src/client/src/components/WorkspaceGitPanel.ts b/src/client/src/components/WorkspaceGitPanel.ts index 5cca4f8..aa93add 100644 --- a/src/client/src/components/WorkspaceGitPanel.ts +++ b/src/client/src/components/WorkspaceGitPanel.ts @@ -2,16 +2,19 @@ import { css, html, LitElement, type PropertyValues, type TemplateResult } from import { customElement, property, state } from "lit/decorators.js"; import type { GitDiffResponse, GitStatusFile, GitStatusResponse } from "../api"; import { buildGitFileTree, collectGitFileTreeDirectoryPaths, type GitFileTreeNode } from "../gitFileTree"; +import { buildGitFileList, type GitFileListModel, type GitFileListSubmoduleFile, type GitFileListSubmoduleGroup } from "../gitFileList"; import { readGitFileView, writeGitFileView, type GitFileView } from "../gitFileViewPreference"; import type { WorkspacePanelContext } from "../plugins/types"; import { workspacePanelStyles } from "./shared"; -interface GitTreeState { +interface GitViewState { readonly nodes: readonly GitFileTreeNode[]; - readonly directoryPaths: readonly string[]; + readonly listModel: GitFileListModel; + readonly expandablePaths: readonly string[]; } -const EMPTY_TREE_STATE: GitTreeState = { nodes: [], directoryPaths: [] }; +const EMPTY_LIST_MODEL: GitFileListModel = { submodules: [], files: [] }; +const EMPTY_VIEW_STATE: GitViewState = { nodes: [], listModel: EMPTY_LIST_MODEL, expandablePaths: [] }; @customElement("workspace-git-panel") export class WorkspaceGitPanel extends LitElement { @@ -27,6 +30,7 @@ export class WorkspaceGitPanel extends LitElement { .view-toggle button:last-child { border-top-right-radius: 7px; border-bottom-right-radius: 7px; margin-left: -1px; } .view-toggle button.selected { position: relative; z-index: 1; } .row .twisty { color: var(--pi-dim, var(--pi-muted)); } + .submodule-badge { display: inline-block; margin-left: 6px; border: 1px solid var(--pi-border); border-radius: 999px; color: var(--pi-muted); padding: 0 5px; font-size: 11px; font-weight: 400; vertical-align: baseline; } `, ]; @@ -37,7 +41,8 @@ export class WorkspaceGitPanel extends LitElement { // Ephemeral by design: the tree always opens fully collapsed, and expand // state is intentionally not persisted. Reassigned (never mutated in place) - // so Lit observes the change. + // so Lit observes the change. Shared by tree directories and, in list view, + // submodule groups, both keyed by their path. @state() private expandedDirectories = new Set(); protected override willUpdate(changedProperties: PropertyValues): void { @@ -53,20 +58,20 @@ export class WorkspaceGitPanel extends LitElement { const context = this.context; if (context === undefined) return html`

Git unavailable.

`; const status = context.gitStatus; - const treeState = this.computeTreeState(status); + const viewState = this.computeViewState(status); return html`
Git ${context.gitStale ? html`stale` : null}
${this.renderViewToggle()} - ${this.view === "tree" && treeState.directoryPaths.length > 0 ? this.renderExpandCollapseAll(treeState.directoryPaths) : null} + ${viewState.expandablePaths.length > 0 ? this.renderExpandCollapseAll(viewState.expandablePaths) : null}
- ${this.renderFileList(context, status, treeState.nodes)} + ${this.renderFileList(context, status, viewState)}
${renderDiffViewer(context)}
@@ -89,66 +94,93 @@ export class WorkspaceGitPanel extends LitElement { `; } - private renderExpandCollapseAll(directoryPaths: readonly string[]): TemplateResult { - const allExpanded = directoryPaths.every((path) => this.expandedDirectories.has(path)); + private renderExpandCollapseAll(expandablePaths: readonly string[]): TemplateResult { + const allExpanded = expandablePaths.every((path) => this.expandedDirectories.has(path)); return html` - + `; } - private renderFileList(context: WorkspacePanelContext, status: GitStatusResponse | undefined, nodes: readonly GitFileTreeNode[]): TemplateResult { + private renderFileList(context: WorkspacePanelContext, status: GitStatusResponse | undefined, viewState: GitViewState): TemplateResult { if (status === undefined) return html`

No status loaded.

`; if (!status.isGitRepo) return html`

Not a git repository.

`; const summary = html`

${gitSummary(status)}

`; if (status.files.length === 0) return html`${summary}

No changes.

`; const body = this.view === "tree" - ? nodes.map((node) => this.renderTreeNode(context, node, 0)) - : status.files.map((file) => this.renderFileRow(context, file)); + ? viewState.nodes.map((node) => this.renderTreeNode(context, node, 0)) + : this.renderListBody(context, viewState.listModel); return html`${summary}${body}`; } + private renderListBody(context: WorkspacePanelContext, model: GitFileListModel): TemplateResult { + return html` + ${model.submodules.map((group) => this.renderSubmoduleGroup(context, group))} + ${model.files.map((file) => this.renderFileRow(context, file))} + `; + } + + private renderSubmoduleGroup(context: WorkspacePanelContext, group: GitFileListSubmoduleGroup): TemplateResult { + const expanded = this.expandedDirectories.has(group.path); + return html` + + ${expanded ? html` + ${group.pointer === undefined ? null : this.renderSelectableRow(context, group.path, group.pointer.name, group.pointer.file, 1)} + ${group.files.map((entry) => this.renderSubmoduleFileRow(context, entry))} + ` : null} + `; + } + + private renderSubmoduleFileRow(context: WorkspacePanelContext, entry: GitFileListSubmoduleFile): TemplateResult { + return this.renderSelectableRow(context, entry.path, entry.relativePath, entry.file, 1); + } + private renderTreeNode(context: WorkspacePanelContext, node: GitFileTreeNode, depth: number): TemplateResult { if (node.kind === "directory") { const expanded = this.expandedDirectories.has(node.path); return html` ${expanded ? node.children.map((child) => this.renderTreeNode(context, child, depth + 1)) : null} `; } - const selected = context.selectedDiffPath === node.path; - return html` - - `; + return this.renderSelectableRow(context, node.path, node.name, node.file, depth); } private renderFileRow(context: WorkspacePanelContext, file: GitStatusFile): TemplateResult { - const selected = context.selectedDiffPath === file.path; + return this.renderSelectableRow(context, file.path, file.path, file, 0); + } + + private renderSelectableRow(context: WorkspacePanelContext, path: string, label: string, file: GitStatusFile, depth: number): TemplateResult { + const selected = context.selectedDiffPath === path; return html` - `; } - private computeTreeState(status: GitStatusResponse | undefined): GitTreeState { - if (this.view !== "tree" || status === undefined || !status.isGitRepo || status.files.length === 0) return EMPTY_TREE_STATE; - const nodes = buildGitFileTree(status.files); - return { nodes, directoryPaths: collectGitFileTreeDirectoryPaths(nodes) }; + private computeViewState(status: GitStatusResponse | undefined): GitViewState { + if (status === undefined || !status.isGitRepo || status.files.length === 0) return EMPTY_VIEW_STATE; + if (this.view === "tree") { + const nodes = buildGitFileTree(status.files, status.submodules); + return { nodes, listModel: EMPTY_LIST_MODEL, expandablePaths: collectGitFileTreeDirectoryPaths(nodes) }; + } + const listModel = buildGitFileList(status.files, status.submodules); + return { nodes: [], listModel, expandablePaths: listModel.submodules.map((group) => group.path) }; } private setView(view: GitFileView): void { if (this.view === view) return; this.view = view; writeGitFileView(view); - // The tree always starts fully collapsed when entered. - if (view === "tree") this.expandedDirectories = new Set(); + // Entering either view starts fully collapsed. + this.expandedDirectories = new Set(); } private toggleDirectory(path: string): void { @@ -158,11 +190,15 @@ export class WorkspaceGitPanel extends LitElement { this.expandedDirectories = next; } - private toggleExpandAll(directoryPaths: readonly string[], allExpanded: boolean): void { - this.expandedDirectories = allExpanded ? new Set() : new Set(directoryPaths); + private toggleExpandAll(expandablePaths: readonly string[], allExpanded: boolean): void { + this.expandedDirectories = allExpanded ? new Set() : new Set(expandablePaths); } } +function submoduleBadge(): TemplateResult { + return html`submodule`; +} + function renderDiffViewer(context: WorkspacePanelContext): TemplateResult { if (context.selectedDiffPath === undefined || context.selectedDiffPath === "") return html`

Select a changed file.

`; const unstaged = context.selectedDiff; diff --git a/src/client/src/controllers/machineController.test.ts b/src/client/src/controllers/machineController.test.ts index d8cca31..0078b37 100644 --- a/src/client/src/controllers/machineController.test.ts +++ b/src/client/src/controllers/machineController.test.ts @@ -58,7 +58,7 @@ describe("MachineController", () => { selectedSession: session, fileTree: [{ name: "index.ts", path: "src/index.ts", type: "file" }], selectedFilePath: "src/index.ts", - gitStatus: { isGitRepo: true, hash: "abc123", branch: "main", files: [{ path: "src/index.ts", index: "modified", workingTree: "modified" }] }, + gitStatus: { isGitRepo: true, hash: "abc123", branch: "main", files: [{ path: "src/index.ts", index: "modified", workingTree: "modified" }], submodules: [] }, activeTerminalCount: 2, error: "stale error", }; diff --git a/src/client/src/gitFileList.test.ts b/src/client/src/gitFileList.test.ts new file mode 100644 index 0000000..60903b6 --- /dev/null +++ b/src/client/src/gitFileList.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from "vitest"; +import type { GitStatusFile } from "./api"; +import { buildGitFileList } from "./gitFileList"; + +describe("buildGitFileList", () => { + it("leaves non-submodule files flat in their original order", () => { + const model = buildGitFileList([changed("src/b.ts"), changed("a.txt"), changed("src/a.ts")], []); + expect(model.submodules).toEqual([]); + expect(model.files.map((file) => file.path)).toEqual(["src/b.ts", "a.txt", "src/a.ts"]); + }); + + it("groups submodule files flat with submodule-relative paths and the pointer first", () => { + const pointer = submodulePointer("HARL", "1111111", "2222222"); + const model = buildGitFileList([changed("README.md"), pointer, changed("HARL/src/foo.ts"), changed("HARL/a.txt"), changed("HARL/b.txt")], ["HARL"]); + + expect(model.files.map((file) => file.path)).toEqual(["README.md"]); + expect(model.submodules).toHaveLength(1); + const group = model.submodules[0]; + expect(group?.path).toBe("HARL"); + expect(group?.name).toBe("HARL"); + expect(group?.pointer?.name).toBe("1111111 → 2222222"); + expect(group?.pointer?.file).toBe(pointer); + // Flattened (no nesting) and sorted by relative path. + expect(group?.files.map((entry) => entry.relativePath)).toEqual(["a.txt", "b.txt", "src/foo.ts"]); + expect(group?.files.map((entry) => entry.path)).toEqual(["HARL/a.txt", "HARL/b.txt", "HARL/src/foo.ts"]); + }); + + it("places submodule groups first, sorted by name, ahead of the flat files", () => { + const model = buildGitFileList([changed("root.txt"), changed("Zsub/x"), changed("Asub/y")], ["Zsub", "Asub"]); + expect(model.submodules.map((group) => group.name)).toEqual(["Asub", "Zsub"]); + expect(model.files.map((file) => file.path)).toEqual(["root.txt"]); + }); + + it("omits the pointer for a dirty submodule whose commit did not move", () => { + const model = buildGitFileList([changed("HARL/x.txt")], ["HARL"]); + expect(model.submodules[0]?.pointer).toBeUndefined(); + expect(model.submodules[0]?.files.map((entry) => entry.relativePath)).toEqual(["x.txt"]); + }); +}); + +function changed(path: string): GitStatusFile { + return { path, index: "modified", workingTree: "modified" }; +} + +function submodulePointer(path: string, from: string, to: string): GitStatusFile { + return { path, index: "unmodified", workingTree: "modified", submoduleFromCommit: from, submoduleToCommit: to }; +} diff --git a/src/client/src/gitFileList.ts b/src/client/src/gitFileList.ts new file mode 100644 index 0000000..32faa6e --- /dev/null +++ b/src/client/src/gitFileList.ts @@ -0,0 +1,84 @@ +import type { GitStatusFile } from "./api"; + +/** A changed file inside a submodule, shown flat in list view. `path` is the + * full superproject-relative path (the diff key); `relativePath` is shown in + * the row and is relative to the submodule root (may still contain slashes). */ +export interface GitFileListSubmoduleFile { + readonly path: string; + readonly relativePath: string; + readonly file: GitStatusFile; +} + +/** An expandable submodule group in list view. Children are flat (no nesting), + * with the pointer row pinned first when present. The pointer is the + * submodule's own moved commit, keyed by the submodule path; its `name` is the + * ` → ` short-SHA summary. */ +export interface GitFileListSubmoduleGroup { + readonly path: string; + readonly name: string; + readonly pointer?: { readonly name: string; readonly file: GitStatusFile }; + readonly files: readonly GitFileListSubmoduleFile[]; +} + +/** List-view model: submodule groups first (alphabetical), then non-submodule + * files in their original Git status order. */ +export interface GitFileListModel { + readonly submodules: readonly GitFileListSubmoduleGroup[]; + readonly files: readonly GitStatusFile[]; +} + +/** + * Group the flat changed-file list for list view. Unlike the tree, submodule + * contents are flattened: each submodule becomes one expandable group holding + * its pointer row and its changed files, and everything else stays a flat list. + */ +export function buildGitFileList(files: readonly GitStatusFile[], submodules: readonly string[] = []): GitFileListModel { + const submoduleSet = new Set(submodules); + const pointers = new Map(); + const grouped = new Map(); + for (const submodule of submodules) grouped.set(submodule, []); + const flat: GitStatusFile[] = []; + + for (const file of files) { + if (submoduleSet.has(file.path)) { + pointers.set(file.path, { name: pointerName(file), file }); + continue; + } + const owner = ownerSubmodule(file.path, submodules); + if (owner !== undefined) { + grouped.get(owner)?.push({ path: file.path, relativePath: file.path.slice(owner.length + 1), file }); + continue; + } + flat.push(file); + } + + const groups: GitFileListSubmoduleGroup[] = submodules + .map((submodule): GitFileListSubmoduleGroup => { + const pointer = pointers.get(submodule); + const inner = grouped.get(submodule) ?? []; + return { path: submodule, name: segmentName(submodule), ...(pointer === undefined ? {} : { pointer }), files: [...inner].sort((left, right) => left.relativePath.localeCompare(right.relativePath)) }; + }) + .sort((left, right) => left.name.localeCompare(right.name)); + + return { submodules: groups, files: flat }; +} + +function ownerSubmodule(path: string, submodules: readonly string[]): string | undefined { + let best: string | undefined; + for (const submodule of submodules) { + if (submodule !== "" && path.startsWith(`${submodule}/`) && (best === undefined || submodule.length > best.length)) best = submodule; + } + return best; +} + +function pointerName(file: GitStatusFile): string { + const from = file.submoduleFromCommit; + const to = file.submoduleToCommit; + return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; +} + +function segmentName(path: string): string { + const segments = path.split("/"); + const last = segments[segments.length - 1]; + return last ?? path; +} diff --git a/src/client/src/gitFileTree.test.ts b/src/client/src/gitFileTree.test.ts index efa1816..e5e0a04 100644 --- a/src/client/src/gitFileTree.test.ts +++ b/src/client/src/gitFileTree.test.ts @@ -58,8 +58,50 @@ describe("buildGitFileTree", () => { const tree = buildGitFileTree([changed("src/b/c.ts"), changed("src/a.ts"), changed("docs/x.md")]); expect(collectGitFileTreeDirectoryPaths(tree)).toEqual(["docs", "src", "src/b"]); }); + + it("marks a submodule directory and pins its pointer as the first child", () => { + const pointer = submodulePointer("HARL", "1111111", "2222222"); + const tree = buildGitFileTree([pointer, changed("HARL/tracked.txt"), changed("HARL/src/foo.ts"), changed("README.md")], ["HARL"]); + + expect(tree.map((node) => node.name)).toEqual(["HARL", "README.md"]); + const harl = expectDirectory(tree[0]); + expect(harl.isSubmodule).toBe(true); + + // Pointer first, then directories, then files (submodule-relative nesting kept). + const pointerRow = expectFile(harl.children[0]); + expect(pointerRow.isSubmodulePointer).toBe(true); + expect(pointerRow.name).toBe("1111111 → 2222222"); + expect(pointerRow.path).toBe("HARL"); + expect(pointerRow.file).toBe(pointer); + + const src = expectDirectory(harl.children[1]); + expect(src.path).toBe("HARL/src"); + expect(expectFile(src.children[0]).path).toBe("HARL/src/foo.ts"); + expect(expectFile(harl.children[2]).path).toBe("HARL/tracked.txt"); + }); + + it("creates a submodule node for a pointer-only change with no inner files", () => { + const tree = buildGitFileTree([submodulePointer("HARL", "aaaaaaa", "bbbbbbb")], ["HARL"]); + const harl = expectDirectory(tree[0]); + expect(harl.isSubmodule).toBe(true); + expect(harl.children).toHaveLength(1); + expect(expectFile(harl.children[0]).isSubmodulePointer).toBe(true); + expect(collectGitFileTreeDirectoryPaths(tree)).toEqual(["HARL"]); + }); + + it("groups a dirty submodule without a moved pointer and emits no pointer row", () => { + const tree = buildGitFileTree([changed("HARL/x.txt"), changed("HARL/nested/y.txt")], ["HARL"]); + const harl = expectDirectory(tree[0]); + expect(harl.isSubmodule).toBe(true); + expect(harl.children.every((node) => node.kind !== "file" || node.isSubmodulePointer !== true)).toBe(true); + expect(collectGitFileTreeDirectoryPaths(tree)).toEqual(["HARL", "HARL/nested"]); + }); }); +function submodulePointer(path: string, from: string, to: string): GitStatusFile { + return { path, index: "unmodified", workingTree: "modified", submoduleFromCommit: from, submoduleToCommit: to }; +} + function changed(path: string): GitStatusFile { return { path, index: "modified", workingTree: "modified" }; } diff --git a/src/client/src/gitFileTree.ts b/src/client/src/gitFileTree.ts index 2d0f097..972f15e 100644 --- a/src/client/src/gitFileTree.ts +++ b/src/client/src/gitFileTree.ts @@ -3,23 +3,29 @@ import type { GitStatusFile } from "./api"; /** * A changed file placed at a leaf of the Git file tree. `path` is the full * repository-relative path (used to load its diff); `name` is just the final - * segment shown in the tree row. + * segment shown in the tree row. A submodule commit-pointer row reuses this + * shape: `isSubmodulePointer` is set, `path` is the submodule path, and `name` + * is the ` → ` short-SHA summary. */ export interface GitFileTreeFileNode { readonly kind: "file"; readonly name: string; readonly path: string; readonly file: GitStatusFile; + readonly isSubmodulePointer?: boolean; } /** * A directory grouping in the Git file tree. `path` is the full directory path * and doubles as the stable key used to track expand/collapse state. + * `isSubmodule` marks a directory that is actually a submodule root so the UI + * can label it and treat its contents as the submodule's own changes. */ export interface GitFileTreeDirectoryNode { readonly kind: "directory"; readonly name: string; readonly path: string; + readonly isSubmodule?: boolean; readonly children: readonly GitFileTreeNode[]; } @@ -29,17 +35,34 @@ interface DirectoryAccumulator { readonly path: string; readonly directories: Map; readonly files: GitFileTreeFileNode[]; + isSubmodule: boolean; + pointer?: GitFileTreeFileNode; } /** * Build a nested directory/file tree from Git's flat changed-file list. The * status response already carries every changed path, so this is a pure * client-side transform (no lazy per-directory loading like the Files tab). - * Directories sort before files, and both sort alphabetically within a level. + * + * `submodules` lists submodule roots: a directory matching one is marked as a + * submodule, a file whose path equals one becomes that submodule's pinned + * commit-pointer row, and files below one nest inside it. Directories sort + * before files (pointer row first within a submodule), both alphabetically. */ -export function buildGitFileTree(files: readonly GitStatusFile[]): GitFileTreeNode[] { +export function buildGitFileTree(files: readonly GitStatusFile[], submodules: readonly string[] = []): GitFileTreeNode[] { const root = createDirectoryAccumulator(""); + const submoduleSet = new Set(submodules); + // Ensure a node exists (and is marked) for every submodule, so a submodule + // whose only change is a moved pointer still renders as an expandable root. + for (const submodule of submodules) ensureDirectory(root, submodule).isSubmodule = true; + for (const file of files) { + if (submoduleSet.has(file.path)) { + const directory = ensureDirectory(root, file.path); + directory.isSubmodule = true; + directory.pointer = { kind: "file", name: pointerName(file), path: file.path, file, isSubmodulePointer: true }; + continue; + } const segments = file.path.split("/").filter((segment) => segment.length > 0); const name = segments[segments.length - 1]; if (name === undefined) continue; @@ -47,15 +70,7 @@ export function buildGitFileTree(files: readonly GitStatusFile[]): GitFileTreeNo for (let index = 0; index < segments.length - 1; index += 1) { const segment = segments[index]; if (segment === undefined) continue; - const childPath = directory.path.length === 0 ? segment : `${directory.path}/${segment}`; - const existing = directory.directories.get(childPath); - if (existing === undefined) { - const created = createDirectoryAccumulator(childPath); - directory.directories.set(childPath, created); - directory = created; - } else { - directory = existing; - } + directory = childDirectory(directory, segment); } directory.files.push({ kind: "file", name, path: file.path, file }); } @@ -65,7 +80,7 @@ export function buildGitFileTree(files: readonly GitStatusFile[]): GitFileTreeNo /** * Every directory path in the tree, in a stable order. Used to drive the * expand-all / collapse-all control and to decide whether the tree is fully - * expanded. + * expanded. Submodule roots are directories and are included. */ export function collectGitFileTreeDirectoryPaths(nodes: readonly GitFileTreeNode[]): string[] { const paths: string[] = []; @@ -78,16 +93,37 @@ export function collectGitFileTreeDirectoryPaths(nodes: readonly GitFileTreeNode return paths; } +function pointerName(file: GitStatusFile): string { + const from = file.submoduleFromCommit; + const to = file.submoduleToCommit; + return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; +} + function createDirectoryAccumulator(path: string): DirectoryAccumulator { - return { path, directories: new Map(), files: [] }; + return { path, directories: new Map(), files: [], isSubmodule: false }; +} + +function childDirectory(parent: DirectoryAccumulator, segment: string): DirectoryAccumulator { + const childPath = parent.path.length === 0 ? segment : `${parent.path}/${segment}`; + const existing = parent.directories.get(childPath); + if (existing !== undefined) return existing; + const created = createDirectoryAccumulator(childPath); + parent.directories.set(childPath, created); + return created; +} + +function ensureDirectory(root: DirectoryAccumulator, path: string): DirectoryAccumulator { + let directory = root; + for (const segment of path.split("/").filter((part) => part.length > 0)) directory = childDirectory(directory, segment); + return directory; } function finalizeChildren(directory: DirectoryAccumulator): GitFileTreeNode[] { const directories: GitFileTreeDirectoryNode[] = [...directory.directories.values()] - .map((child): GitFileTreeDirectoryNode => ({ kind: "directory", name: segmentName(child.path), path: child.path, children: finalizeChildren(child) })) + .map((child): GitFileTreeDirectoryNode => ({ kind: "directory", name: segmentName(child.path), path: child.path, ...(child.isSubmodule ? { isSubmodule: true } : {}), children: finalizeChildren(child) })) .sort((left, right) => left.name.localeCompare(right.name)); const files = [...directory.files].sort((left, right) => left.name.localeCompare(right.name)); - return [...directories, ...files]; + return [...(directory.pointer === undefined ? [] : [directory.pointer]), ...directories, ...files]; } function segmentName(path: string): string { diff --git a/src/server/git/gitService.test.ts b/src/server/git/gitService.test.ts new file mode 100644 index 0000000..a5336bf --- /dev/null +++ b/src/server/git/gitService.test.ts @@ -0,0 +1,111 @@ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync, renameSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, describe, expect, it } from "vitest"; +import { gitDiff, gitStatus } from "./gitService.js"; + +// Isolate from any global/system git config and force a deterministic identity; +// `protocol.file.allow` is required for `submodule add` from a local path. +const GIT_FLAGS = ["-c", "user.name=Test", "-c", "user.email=test@example.com", "-c", "protocol.file.allow=always", "-c", "commit.gpgsign=false"]; +const GIT_ENV = { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null", GIT_TERMINAL_PROMPT: "0" }; + +const created: string[] = []; +afterAll(() => { for (const dir of created) rmSync(dir, { recursive: true, force: true }); }); + +function git(cwd: string, args: string[]): string { + return execFileSync("git", [...GIT_FLAGS, ...args], { cwd, encoding: "utf8", env: GIT_ENV }); +} + +/** Superproject at `dir` with a submodule `HARL` recorded at commit `c2`; the + * submodule origin has two commits `c1` (a.txt=v1) then `c2` (a.txt=v2). */ +function createFixture(): { dir: string; c1: string; c2: string } { + const base = mkdtempSync(join(tmpdir(), "pi-web-sub-")); + created.push(base); + const origin = join(base, "origin"); + const sup = join(base, "sup"); + + git(base, ["init", "-b", "main", origin]); + writeFileSync(join(origin, "a.txt"), "v1\n"); + git(origin, ["add", "-A"]); + git(origin, ["commit", "-m", "c1"]); + const c1 = git(origin, ["rev-parse", "HEAD"]).trim(); + writeFileSync(join(origin, "a.txt"), "v2\n"); + git(origin, ["add", "-A"]); + git(origin, ["commit", "-m", "c2"]); + const c2 = git(origin, ["rev-parse", "HEAD"]).trim(); + + git(base, ["init", "-b", "main", sup]); + git(sup, ["submodule", "add", origin, "HARL"]); + writeFileSync(join(sup, "root.txt"), "root\n"); + git(sup, ["add", "-A"]); + git(sup, ["commit", "-m", "init"]); + return { dir: sup, c1, c2 }; +} + +describe("gitStatus with submodules", () => { + it("surfaces a moved commit pointer with short SHAs and no inner files", async () => { + const { dir, c1, c2 } = createFixture(); + git(join(dir, "HARL"), ["checkout", c1]); // move the pointer, leave the tree clean + + const status = await gitStatus(dir); + expect(status.submodules).toContain("HARL"); + const pointer = status.files.find((file) => file.path === "HARL"); + expect(pointer?.submoduleFromCommit).toBe(c2.slice(0, 7)); + expect(pointer?.submoduleToCommit).toBe(c1.slice(0, 7)); + expect(status.files.some((file) => file.path.startsWith("HARL/"))).toBe(false); + }); + + it("lists modified and untracked inner files and omits the pointer when the commit is unchanged", async () => { + const { dir } = createFixture(); + writeFileSync(join(dir, "HARL", "a.txt"), "v2\nchanged\n"); + writeFileSync(join(dir, "HARL", "new.txt"), "brand-new\n"); + + const status = await gitStatus(dir); + expect(status.submodules).toContain("HARL"); + expect(status.files.find((file) => file.path === "HARL")).toBeUndefined(); + const inner = status.files.filter((file) => file.path.startsWith("HARL/")).map((file) => file.path); + expect(inner).toContain("HARL/a.txt"); + expect(inner).toContain("HARL/new.txt"); + }); + + it("skips inner recursion without throwing when the submodule repo is unreadable", async () => { + const { dir } = createFixture(); + writeFileSync(join(dir, "HARL", "new.txt"), "brand-new\n"); // untracked → would trigger recursion + renameSync(join(dir, "HARL", ".git"), join(dir, "HARL", ".git.bak")); // break the inner repo + + const status = await gitStatus(dir); + expect(status.isGitRepo).toBe(true); + expect(status.files.some((file) => file.path.startsWith("HARL/"))).toBe(false); + }); +}); + +describe("gitDiff routing into submodules", () => { + it("returns real content for a tracked file inside the submodule", async () => { + const { dir } = createFixture(); + writeFileSync(join(dir, "HARL", "a.txt"), "v2\nchanged\n"); + + const diff = await gitDiff(dir, { path: "HARL/a.txt" }); + expect(diff.path).toBe("HARL/a.txt"); + expect(diff.diff).toContain("@@"); + expect(diff.diff).toContain("changed"); + }); + + it("produces an untracked-file diff inside the submodule via --no-index", async () => { + const { dir } = createFixture(); + writeFileSync(join(dir, "HARL", "new.txt"), "brand-new\n"); + + const diff = await gitDiff(dir, { path: "HARL/new.txt" }); + expect(diff.path).toBe("HARL/new.txt"); + expect(diff.diff).toContain("brand-new"); + }); + + it("diffs the submodule path itself against the superproject pointer", async () => { + const { dir, c1 } = createFixture(); + git(join(dir, "HARL"), ["checkout", c1]); + + const diff = await gitDiff(dir, { path: "HARL" }); + expect(diff.path).toBe("HARL"); + expect(diff.diff).toContain("Subproject commit"); + }); +}); diff --git a/src/server/git/gitService.ts b/src/server/git/gitService.ts index cfa3c4c..7113a44 100644 --- a/src/server/git/gitService.ts +++ b/src/server/git/gitService.ts @@ -1,15 +1,103 @@ import { createHash } from "node:crypto"; import { spawn } from "node:child_process"; +import { join } from "node:path"; import type { GitDiffResponse, GitFileState, GitStatusFile, GitStatusResponse } from "../../shared/apiTypes.js"; import { normalizeRelativePath } from "../workspaces/pathSafety.js"; import { sanitizedGitEnv } from "./gitEnv.js"; const MAX_OUTPUT = 2 * 1024 * 1024; +/** + * A submodule row parsed from the superproject status. `git status` reports a + * submodule as a single path with an `S` flag field (commit changed / + * modified tracked content / untracked content) but never lists the files that + * changed inside it, so we recurse in `expandSubmodules`. + */ +interface SubmoduleRecord { + path: string; + index: GitFileState; + workingTree: GitFileState; + commitChanged: boolean; + hasModifiedContent: boolean; + hasUntrackedContent: boolean; + headOid: string; + indexOid: string; +} + +interface ParsedStatus { + isGitRepo: true; + branch?: string; + upstream?: string; + ahead?: number; + behind?: number; + files: GitStatusFile[]; + submodules: SubmoduleRecord[]; +} + export async function gitStatus(cwd: string): Promise { const result = await runGit(cwd, ["status", "--porcelain=v2", "--branch", "--untracked-files=all", "-z"]); - if (result.code !== 0) return { isGitRepo: false, hash: hash(result.stdout + result.stderr), files: [] }; - return parseStatus(result.stdout); + if (result.code !== 0) return { isGitRepo: false, hash: hash(result.stdout + result.stderr), files: [], submodules: [] }; + const parsed = parseStatus(result.stdout, { deferSubmodules: true }); + return expandSubmodules(cwd, parsed, result.stdout); +} + +/** + * Merge each dirty submodule's own changes into the flat file list. A moved + * commit pointer becomes a single entry keyed by the submodule path (carrying + * the short SHAs for display); modified/untracked content is listed as regular + * entries under `/`. A plain `-dirty` pointer (commit + * unchanged) is intentionally not surfaced as a pointer entry. + */ +async function expandSubmodules(cwd: string, parsed: ParsedStatus, topRaw: string): Promise { + const files: GitStatusFile[] = [...parsed.files]; + const submodulePaths: string[] = []; + let extraForHash = ""; + + for (const sub of parsed.submodules) { + submodulePaths.push(sub.path); + if (sub.commitChanged) { + files.push({ + path: sub.path, + index: sub.index, + workingTree: sub.workingTree, + submoduleFromCommit: short(sub.headOid), + submoduleToCommit: short(await resolveSubmoduleToCommit(cwd, sub)), + }); + } + if (sub.hasModifiedContent || sub.hasUntrackedContent) { + const inner = await runGit(join(cwd, sub.path), ["status", "--porcelain=v2", "--untracked-files=all", "-z"]); + if (inner.code !== 0) continue; // uninitialized / unreadable submodule: skip silently + extraForHash += `\0${sub.path}\0${inner.stdout}`; + const innerFiles = parseStatus(inner.stdout, { deferSubmodules: false }).files; + for (const file of innerFiles) { + files.push({ + ...file, + path: `${sub.path}/${file.path}`, + ...(file.oldPath === undefined ? {} : { oldPath: `${sub.path}/${file.oldPath}` }), + }); + } + } + } + + return { + isGitRepo: true, + hash: hash(topRaw + extraForHash), + ...(parsed.branch === undefined ? {} : { branch: parsed.branch }), + ...(parsed.upstream === undefined ? {} : { upstream: parsed.upstream }), + ...(parsed.ahead === undefined ? {} : { ahead: parsed.ahead }), + ...(parsed.behind === undefined ? {} : { behind: parsed.behind }), + files, + submodules: submodulePaths, + }; +} + +async function resolveSubmoduleToCommit(cwd: string, sub: SubmoduleRecord): Promise { + // Staged pointer moves already expose the new commit as the index OID; an + // unstaged move only records the old OID, so read the submodule's HEAD. + if (sub.indexOid !== sub.headOid) return sub.indexOid; + const head = await runGit(join(cwd, sub.path), ["rev-parse", "HEAD"]); + const resolved = head.stdout.trim(); + return head.code === 0 && resolved !== "" ? resolved : sub.indexOid; } export async function gitDiff(cwd: string, options: { path?: string; staged?: boolean }): Promise { @@ -17,6 +105,11 @@ export async function gitDiff(cwd: string, options: { path?: string; staged?: bo let path: string | undefined; if (options.path !== undefined && options.path !== "") path = normalizeRelativePath(options.path); + if (path !== undefined) { + const owner = await submoduleForPath(cwd, path); + if (owner !== undefined) return submoduleDiff(cwd, owner, path, staged); + } + const args = ["diff", "--no-ext-diff", "--color=never"]; if (staged) args.push("--cached"); if (path !== undefined) args.push("--", path); @@ -31,14 +124,64 @@ export async function gitDiff(cwd: string, options: { path?: string; staged?: bo return { ...(path === undefined ? {} : { path }), staged, hash: hash(result.stdout), diff: result.stdout, truncated: result.truncated }; } +/** + * Run the diff inside the owning submodule's working tree, since `git diff` at + * the superproject root never shows content changes below a submodule boundary. + * The response path stays the full superproject-relative path so the viewer and + * the selected row line up. + */ +async function submoduleDiff(cwd: string, owner: string, path: string, staged: boolean): Promise { + const subCwd = join(cwd, owner); + const rel = normalizeRelativePath(path.slice(owner.length + 1)); + + const args = ["diff", "--no-ext-diff", "--color=never"]; + if (staged) args.push("--cached"); + args.push("--", rel); + + const result = await runGit(subCwd, args); + if (result.code !== 0) throw new Error(result.stderr.trim() || "git diff failed"); + if (!staged && result.stdout === "" && await isUntracked(subCwd, rel)) { + const untracked = await runGit(subCwd, ["diff", "--no-ext-diff", "--color=never", "--no-index", "/dev/null", "--", rel]); + if (untracked.code !== 0 && untracked.code !== 1) throw new Error(untracked.stderr.trim() || "git diff failed"); + return { path, staged, hash: hash(untracked.stdout), diff: untracked.stdout, truncated: untracked.truncated }; + } + return { path, staged, hash: hash(result.stdout), diff: result.stdout, truncated: result.truncated }; +} + async function isUntracked(cwd: string, path: string): Promise { const result = await runGit(cwd, ["ls-files", "--others", "--exclude-standard", "-z", "--", path]); return result.code === 0 && result.stdout.split("\0").includes(path); } -function parseStatus(raw: string): GitStatusResponse { +/** Configured direct-submodule paths (depth 1), read from `.gitmodules`. */ +async function submodulePaths(cwd: string): Promise { + const result = await runGit(cwd, ["config", "--file", ".gitmodules", "--get-regexp", "^submodule\\..+\\.path$"]); + if (result.code !== 0) return []; + const paths: string[] = []; + for (const line of result.stdout.split("\n")) { + const trimmed = line.trim(); + if (trimmed === "") continue; + const spaceAt = trimmed.indexOf(" "); + if (spaceAt === -1) continue; + paths.push(trimmed.slice(spaceAt + 1)); + } + return paths; +} + +/** The submodule that strictly contains `path`, if any (longest match wins). */ +async function submoduleForPath(cwd: string, path: string): Promise { + const subs = await submodulePaths(cwd); + let best: string | undefined; + for (const sub of subs) { + if (sub !== "" && path.startsWith(`${sub}/`) && (best === undefined || sub.length > best.length)) best = sub; + } + return best; +} + +function parseStatus(raw: string, options: { deferSubmodules: boolean }): ParsedStatus { const records = raw.split("\0").filter((record) => record !== ""); const files: GitStatusFile[] = []; + const submodules: SubmoduleRecord[] = []; let branch: string | undefined; let upstream: string | undefined; let ahead: number | undefined; @@ -56,7 +199,22 @@ function parseStatus(raw: string): GitStatusResponse { else if (record.startsWith("! ")) files.push({ path: record.slice(2), index: "ignored", workingTree: "ignored" }); else if (record.startsWith("1 ")) { const parts = record.split(" "); - files.push({ path: parts.slice(8).join(" "), index: stateFor(parts[1]?.[0]), workingTree: stateFor(parts[1]?.[1]) }); + const sub = parts[2]; + const path = parts.slice(8).join(" "); + if (options.deferSubmodules && sub?.startsWith("S") === true) { + submodules.push({ + path, + index: stateFor(parts[1]?.[0]), + workingTree: stateFor(parts[1]?.[1]), + commitChanged: sub[1] === "C", + hasModifiedContent: sub[2] === "M", + hasUntrackedContent: sub[3] === "U", + headOid: parts[6] ?? "", + indexOid: parts[7] ?? "", + }); + } else { + files.push({ path, index: stateFor(parts[1]?.[0]), workingTree: stateFor(parts[1]?.[1]) }); + } } else if (record.startsWith("2 ")) { const parts = record.split(" "); const path = parts.slice(9).join(" "); @@ -69,7 +227,7 @@ function parseStatus(raw: string): GitStatusResponse { } } - return { isGitRepo: true, hash: hash(raw), ...(branch === undefined ? {} : { branch }), ...(upstream === undefined ? {} : { upstream }), ...(ahead === undefined ? {} : { ahead }), ...(behind === undefined ? {} : { behind }), files }; + return { isGitRepo: true, ...(branch === undefined ? {} : { branch }), ...(upstream === undefined ? {} : { upstream }), ...(ahead === undefined ? {} : { ahead }), ...(behind === undefined ? {} : { behind }), files, submodules }; } function stateFor(code: string | undefined): GitFileState { @@ -90,6 +248,10 @@ function normalizeBranch(value: string): string | undefined { return value === "(detached)" ? undefined : value; } +function short(oid: string): string { + return oid.slice(0, 7); +} + function hash(value: string): string { return createHash("sha1").update(value).digest("hex"); } diff --git a/src/shared/apiTypes.ts b/src/shared/apiTypes.ts index 32c4f9a..4070e3c 100644 --- a/src/shared/apiTypes.ts +++ b/src/shared/apiTypes.ts @@ -681,6 +681,10 @@ export interface GitStatusFile { oldPath?: string; index: GitFileState; workingTree: GitFileState; + // Set only on a submodule commit-pointer entry (path equals the submodule's + // superproject-relative path). Short SHAs of the recorded and current commit. + submoduleFromCommit?: string; + submoduleToCommit?: string; } export interface GitStatusResponse { @@ -691,6 +695,11 @@ export interface GitStatusResponse { ahead?: number; behind?: number; files: GitStatusFile[]; + // Superproject-relative paths of submodules that carry a change. Files inside + // a submodule appear in `files` under `/`; the client + // uses this list to group and label them and to distinguish a submodule root + // from an ordinary directory with the same name. + submodules: string[]; } export interface GitDiffResponse { From 95102b8d78fe6c5d91951c4b15afe6b0a0fe028d Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 11:43:58 +0200 Subject: [PATCH 3/8] fix(git): show staged submodule pointer moves and support spaced submodule paths - parseStatus: detect staged submodule pointer moves by comparing the recorded HEAD/index OIDs (porcelain reports S... for a staged move, so the c flag never fires); staged moves previously vanished from the status response (PR #92 review finding B1). - parseStatus: keep deleted gitlinks (index or working tree) as plain deletion rows instead of deferring them as submodules. Unstaged deletions vanished entirely, and staged deletions would render as a bogus pointer move to the zero OID. The finding assumed N... porcelain; git 2.54 actually emits .D/D. S... (finding S3's stated outcome). - submodulePaths: parse 'git config -z' records so submodule paths with spaces survive .gitmodules key parsing instead of splitting lines at the first space (finding S1). - tests: strip inherited GIT_* env vars in the fixture helper so the suite also passes when run from a git hook (pre-commit sets GIT_DIR). Adds real-git fixture tests for staged moves, staged+dirty combos, deleted submodules, inner renames, and spaced submodule/file paths. --- src/server/git/gitService.test.ts | 126 +++++++++++++++++++++++++++++- src/server/git/gitService.ts | 39 +++++---- 2 files changed, 150 insertions(+), 15 deletions(-) diff --git a/src/server/git/gitService.test.ts b/src/server/git/gitService.test.ts index a5336bf..c1dce50 100644 --- a/src/server/git/gitService.test.ts +++ b/src/server/git/gitService.test.ts @@ -8,7 +8,15 @@ import { gitDiff, gitStatus } from "./gitService.js"; // Isolate from any global/system git config and force a deterministic identity; // `protocol.file.allow` is required for `submodule add` from a local path. const GIT_FLAGS = ["-c", "user.name=Test", "-c", "user.email=test@example.com", "-c", "protocol.file.allow=always", "-c", "commit.gpgsign=false"]; -const GIT_ENV = { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null", GIT_TERMINAL_PROMPT: "0" }; +// Strip all GIT_* variables (e.g. GIT_DIR/GIT_INDEX_FILE, set by git hooks such +// as this repo's pre-commit verify run) so fixture commands never pick up an +// outer repository's environment, then pin the handful we rely on. +const GIT_ENV = Object.fromEntries([ + ...Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_")), + ["GIT_CONFIG_GLOBAL", "/dev/null"], + ["GIT_CONFIG_SYSTEM", "/dev/null"], + ["GIT_TERMINAL_PROMPT", "0"], +]); const created: string[] = []; afterAll(() => { for (const dir of created) rmSync(dir, { recursive: true, force: true }); }); @@ -43,6 +51,27 @@ function createFixture(): { dir: string; c1: string; c2: string } { return { dir: sup, c1, c2 }; } +/** Superproject at `dir` whose only submodule lives at the spaced path + * `my sub`; the submodule origin has a single commit (a.txt=v1). */ +function createSpacedPathFixture(): { dir: string } { + const base = mkdtempSync(join(tmpdir(), "pi-web-sub-space-")); + created.push(base); + const origin = join(base, "origin"); + const sup = join(base, "sup"); + + git(base, ["init", "-b", "main", origin]); + writeFileSync(join(origin, "a.txt"), "v1\n"); + git(origin, ["add", "-A"]); + git(origin, ["commit", "-m", "c1"]); + + git(base, ["init", "-b", "main", sup]); + git(sup, ["submodule", "add", origin, "my sub"]); + writeFileSync(join(sup, "root.txt"), "root\n"); + git(sup, ["add", "-A"]); + git(sup, ["commit", "-m", "init"]); + return { dir: sup }; +} + describe("gitStatus with submodules", () => { it("surfaces a moved commit pointer with short SHAs and no inner files", async () => { const { dir, c1, c2 } = createFixture(); @@ -69,6 +98,85 @@ describe("gitStatus with submodules", () => { expect(inner).toContain("HARL/new.txt"); }); + it("surfaces a staged pointer move with the recorded OID as from and the staged OID as to", async () => { + const { dir, c1, c2 } = createFixture(); + git(join(dir, "HARL"), ["checkout", c1]); // move the pointer + git(dir, ["add", "HARL"]); // stage the move: porcelain `1 M. S... HARL` + + const status = await gitStatus(dir); + expect(status.submodules).toContain("HARL"); + const pointer = status.files.find((file) => file.path === "HARL"); + expect(pointer?.index).toBe("modified"); + expect(pointer?.workingTree).toBe("unmodified"); + expect(pointer?.submoduleFromCommit).toBe(c2.slice(0, 7)); + expect(pointer?.submoduleToCommit).toBe(c1.slice(0, 7)); + }); + + it("reports both the pointer entry and inner files for a staged move with dirty content", async () => { + const { dir, c1, c2 } = createFixture(); + git(join(dir, "HARL"), ["checkout", c1]); + git(dir, ["add", "HARL"]); + writeFileSync(join(dir, "HARL", "a.txt"), "v1\ndirty\n"); // combined `1 MM S.M.` + + const status = await gitStatus(dir); + const pointer = status.files.find((file) => file.path === "HARL"); + expect(pointer?.index).toBe("modified"); + expect(pointer?.workingTree).toBe("modified"); + expect(pointer?.submoduleFromCommit).toBe(c2.slice(0, 7)); + expect(pointer?.submoduleToCommit).toBe(c1.slice(0, 7)); + const inner = status.files.find((file) => file.path === "HARL/a.txt"); + expect(inner?.workingTree).toBe("modified"); + }); + + it("reports a deleted submodule as a plain deleted row", async () => { + const { dir } = createFixture(); + rmSync(join(dir, "HARL"), { recursive: true, force: true }); // unstaged deletion: `1 .D S...` + + const status = await gitStatus(dir); + const row = status.files.find((file) => file.path === "HARL"); + expect(row?.workingTree).toBe("deleted"); + expect(row?.submoduleFromCommit).toBeUndefined(); + expect(status.submodules).not.toContain("HARL"); + expect(status.files.some((file) => file.path.startsWith("HARL/"))).toBe(false); + }); + + it("reports a staged submodule deletion as a plain deleted row, not a pointer move", async () => { + const { dir } = createFixture(); + git(dir, ["rm", "-q", "HARL"]); // staged deletion: `1 D. S...` with a zero index OID + + const status = await gitStatus(dir); + const row = status.files.find((file) => file.path === "HARL"); + expect(row?.index).toBe("deleted"); + expect(row?.submoduleFromCommit).toBeUndefined(); + expect(status.submodules).not.toContain("HARL"); + }); + + it("prefixes oldPath with the submodule path for renames inside a submodule", async () => { + const { dir } = createFixture(); + git(join(dir, "HARL"), ["mv", "a.txt", "renamed.txt"]); + + const status = await gitStatus(dir); + const renamed = status.files.find((file) => file.path === "HARL/renamed.txt"); + expect(renamed?.index).toBe("renamed"); + expect(renamed?.oldPath).toBe("HARL/a.txt"); + }); + + it("keeps inner filenames with spaces intact through expansion", async () => { + const { dir } = createFixture(); + writeFileSync(join(dir, "HARL", "my file.txt"), "tracked\n"); + git(join(dir, "HARL"), ["add", "my file.txt"]); + git(join(dir, "HARL"), ["commit", "-m", "track spaced file"]); + git(dir, ["add", "HARL"]); + git(dir, ["commit", "-m", "record new pointer"]); // HARL clean at the new recorded commit + writeFileSync(join(dir, "HARL", "my file.txt"), "tracked\nchanged\n"); + writeFileSync(join(dir, "HARL", "untracked file.txt"), "new\n"); + + const status = await gitStatus(dir); + expect(status.files.find((file) => file.path === "HARL/my file.txt")?.workingTree).toBe("modified"); + expect(status.files.some((file) => file.path === "HARL/untracked file.txt")).toBe(true); + expect(status.files.find((file) => file.path === "HARL")).toBeUndefined(); // pointer unchanged + }); + it("skips inner recursion without throwing when the submodule repo is unreadable", async () => { const { dir } = createFixture(); writeFileSync(join(dir, "HARL", "new.txt"), "brand-new\n"); // untracked → would trigger recursion @@ -80,6 +188,22 @@ describe("gitStatus with submodules", () => { }); }); +describe("submodule paths containing spaces", () => { + it("expands status and routes diffs into the space-named submodule", async () => { + const { dir } = createSpacedPathFixture(); + writeFileSync(join(dir, "my sub", "a.txt"), "v1\nchanged\n"); + + const status = await gitStatus(dir); + expect(status.submodules).toContain("my sub"); + expect(status.files.some((file) => file.path === "my sub/a.txt")).toBe(true); + + const diff = await gitDiff(dir, { path: "my sub/a.txt" }); + expect(diff.path).toBe("my sub/a.txt"); + expect(diff.diff).toContain("@@"); + expect(diff.diff).toContain("changed"); + }); +}); + describe("gitDiff routing into submodules", () => { it("returns real content for a tracked file inside the submodule", async () => { const { dir } = createFixture(); diff --git a/src/server/git/gitService.ts b/src/server/git/gitService.ts index 7113a44..9dd2199 100644 --- a/src/server/git/gitService.ts +++ b/src/server/git/gitService.ts @@ -155,15 +155,17 @@ async function isUntracked(cwd: string, path: string): Promise { /** Configured direct-submodule paths (depth 1), read from `.gitmodules`. */ async function submodulePaths(cwd: string): Promise { - const result = await runGit(cwd, ["config", "--file", ".gitmodules", "--get-regexp", "^submodule\\..+\\.path$"]); + // `-z` emits `\n\0` records; keys may themselves contain spaces + // (`submodule.my sub.path`), so splitting lines at the first space mangles + // paths with spaces in them. + const result = await runGit(cwd, ["config", "-z", "--file", ".gitmodules", "--get-regexp", "^submodule\\..+\\.path$"]); if (result.code !== 0) return []; const paths: string[] = []; - for (const line of result.stdout.split("\n")) { - const trimmed = line.trim(); - if (trimmed === "") continue; - const spaceAt = trimmed.indexOf(" "); - if (spaceAt === -1) continue; - paths.push(trimmed.slice(spaceAt + 1)); + for (const record of result.stdout.split("\0")) { + if (record === "") continue; + const newlineAt = record.indexOf("\n"); + if (newlineAt === -1) continue; + paths.push(record.slice(newlineAt + 1)); } return paths; } @@ -201,19 +203,28 @@ function parseStatus(raw: string, options: { deferSubmodules: boolean }): Parsed const parts = record.split(" "); const sub = parts[2]; const path = parts.slice(8).join(" "); - if (options.deferSubmodules && sub?.startsWith("S") === true) { + const index = stateFor(parts[1]?.[0]); + const workingTree = stateFor(parts[1]?.[1]); + // A deleted gitlink has no pointer move or inner content to expand (a + // staged deletion even reports the index OID as all zeros), so keep it + // as a plain row instead of deferring it as a submodule. + if (options.deferSubmodules && sub?.startsWith("S") === true && index !== "deleted" && workingTree !== "deleted") { + const headOid = parts[6] ?? ""; + const indexOid = parts[7] ?? ""; submodules.push({ path, - index: stateFor(parts[1]?.[0]), - workingTree: stateFor(parts[1]?.[1]), - commitChanged: sub[1] === "C", + index, + workingTree, + // `c` only flags unstaged moves (submodule HEAD left the index OID); + // a staged move leaves HEAD == index, so compare the recorded OIDs. + commitChanged: sub[1] === "C" || headOid !== indexOid, hasModifiedContent: sub[2] === "M", hasUntrackedContent: sub[3] === "U", - headOid: parts[6] ?? "", - indexOid: parts[7] ?? "", + headOid, + indexOid, }); } else { - files.push({ path, index: stateFor(parts[1]?.[0]), workingTree: stateFor(parts[1]?.[1]) }); + files.push({ path, index, workingTree }); } } else if (record.startsWith("2 ")) { const parts = record.split(" "); From 842160f9646974866c6f8e8accf3dc308a2acc28 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 11:55:56 +0200 Subject: [PATCH 4/8] perf(git): parallelize submodule expansion and skip impossible submodule lookups - expandSubmodules now fans out with Promise.all over the dirty submodules and concatenates results in input order, so the polled status endpoint no longer pays serial git status/rev-parse spawns (P1). - submoduleForPath bails out before spawning git config when the path contains no '/' or the repo has no .gitmodules, removing a spawn from every diff call in plain repos (P2). - Rename submodulePaths() to configuredSubmodulePaths() and the expandSubmodules local to dirtySubmodulePaths to disambiguate the two concepts (N2). --- src/server/git/gitService.ts | 81 +++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 30 deletions(-) diff --git a/src/server/git/gitService.ts b/src/server/git/gitService.ts index 9dd2199..8bf33bb 100644 --- a/src/server/git/gitService.ts +++ b/src/server/git/gitService.ts @@ -1,5 +1,6 @@ import { createHash } from "node:crypto"; import { spawn } from "node:child_process"; +import { existsSync } from "node:fs"; import { join } from "node:path"; import type { GitDiffResponse, GitFileState, GitStatusFile, GitStatusResponse } from "../../shared/apiTypes.js"; import { normalizeRelativePath } from "../workspaces/pathSafety.js"; @@ -49,34 +50,18 @@ export async function gitStatus(cwd: string): Promise { * unchanged) is intentionally not surfaced as a pointer entry. */ async function expandSubmodules(cwd: string, parsed: ParsedStatus, topRaw: string): Promise { - const files: GitStatusFile[] = [...parsed.files]; - const submodulePaths: string[] = []; - let extraForHash = ""; + // Fan out concurrently — one `git status` per dirty submodule plus one + // `git rev-parse` per unstaged pointer move — then concatenate in input + // order so the file list and hash are identical to a serial pass. + const expanded = await Promise.all(parsed.submodules.map(async (sub) => ({ path: sub.path, ...(await expandSubmodule(cwd, sub)) }))); - for (const sub of parsed.submodules) { - submodulePaths.push(sub.path); - if (sub.commitChanged) { - files.push({ - path: sub.path, - index: sub.index, - workingTree: sub.workingTree, - submoduleFromCommit: short(sub.headOid), - submoduleToCommit: short(await resolveSubmoduleToCommit(cwd, sub)), - }); - } - if (sub.hasModifiedContent || sub.hasUntrackedContent) { - const inner = await runGit(join(cwd, sub.path), ["status", "--porcelain=v2", "--untracked-files=all", "-z"]); - if (inner.code !== 0) continue; // uninitialized / unreadable submodule: skip silently - extraForHash += `\0${sub.path}\0${inner.stdout}`; - const innerFiles = parseStatus(inner.stdout, { deferSubmodules: false }).files; - for (const file of innerFiles) { - files.push({ - ...file, - path: `${sub.path}/${file.path}`, - ...(file.oldPath === undefined ? {} : { oldPath: `${sub.path}/${file.oldPath}` }), - }); - } - } + const files: GitStatusFile[] = [...parsed.files]; + const dirtySubmodulePaths: string[] = []; + let extraForHash = ""; + for (const part of expanded) { + dirtySubmodulePaths.push(part.path); + files.push(...part.files); + extraForHash += part.extraForHash; } return { @@ -87,10 +72,41 @@ async function expandSubmodules(cwd: string, parsed: ParsedStatus, topRaw: strin ...(parsed.ahead === undefined ? {} : { ahead: parsed.ahead }), ...(parsed.behind === undefined ? {} : { behind: parsed.behind }), files, - submodules: submodulePaths, + submodules: dirtySubmodulePaths, }; } +/** Expand one dirty submodule: the pointer entry first, then its inner files. */ +async function expandSubmodule(cwd: string, sub: SubmoduleRecord): Promise<{ files: GitStatusFile[]; extraForHash: string }> { + const files: GitStatusFile[] = []; + let extraForHash = ""; + if (sub.commitChanged) { + files.push({ + path: sub.path, + index: sub.index, + workingTree: sub.workingTree, + submoduleFromCommit: short(sub.headOid), + submoduleToCommit: short(await resolveSubmoduleToCommit(cwd, sub)), + }); + } + if (sub.hasModifiedContent || sub.hasUntrackedContent) { + const inner = await runGit(join(cwd, sub.path), ["status", "--porcelain=v2", "--untracked-files=all", "-z"]); + if (inner.code === 0) { + extraForHash = `\0${sub.path}\0${inner.stdout}`; + const innerFiles = parseStatus(inner.stdout, { deferSubmodules: false }).files; + for (const file of innerFiles) { + files.push({ + ...file, + path: `${sub.path}/${file.path}`, + ...(file.oldPath === undefined ? {} : { oldPath: `${sub.path}/${file.oldPath}` }), + }); + } + } + // non-zero exit: uninitialized / unreadable submodule — skip silently + } + return { files, extraForHash }; +} + async function resolveSubmoduleToCommit(cwd: string, sub: SubmoduleRecord): Promise { // Staged pointer moves already expose the new commit as the index OID; an // unstaged move only records the old OID, so read the submodule's HEAD. @@ -154,7 +170,7 @@ async function isUntracked(cwd: string, path: string): Promise { } /** Configured direct-submodule paths (depth 1), read from `.gitmodules`. */ -async function submodulePaths(cwd: string): Promise { +async function configuredSubmodulePaths(cwd: string): Promise { // `-z` emits `\n\0` records; keys may themselves contain spaces // (`submodule.my sub.path`), so splitting lines at the first space mangles // paths with spaces in them. @@ -172,7 +188,12 @@ async function submodulePaths(cwd: string): Promise { /** The submodule that strictly contains `path`, if any (longest match wins). */ async function submoduleForPath(cwd: string, path: string): Promise { - const subs = await submodulePaths(cwd); + // Cheap bail-outs before spawning `git config`: a path strictly inside a + // submodule always contains `/`, and without `.gitmodules` there are no + // configured submodules to look up (every diff call used to pay this spawn). + if (!path.includes("/")) return undefined; + if (!existsSync(join(cwd, ".gitmodules"))) return undefined; + const subs = await configuredSubmodulePaths(cwd); let best: string | undefined; for (const sub of subs) { if (sub !== "" && path.startsWith(`${sub}/`) && (best === undefined || sub.length > best.length)) best = sub; From 65a20b59fe6a09730ce244e3fdc3d8a1f6d56f13 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 11:56:54 +0200 Subject: [PATCH 5/8] =?UTF-8?q?fix(git):=20render=20newly=20staged=20submo?= =?UTF-8?q?dule=20pointer=20as=20new=20=E2=86=92=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A staged submodule add records an all-zero head OID, which rendered as 0000000 → . Display the zero OID as "new" instead; the client pointer label needs no change (N4). --- src/server/git/gitService.test.ts | 12 ++++++++++++ src/server/git/gitService.ts | 7 ++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/server/git/gitService.test.ts b/src/server/git/gitService.test.ts index c1dce50..2be94e4 100644 --- a/src/server/git/gitService.test.ts +++ b/src/server/git/gitService.test.ts @@ -151,6 +151,18 @@ describe("gitStatus with submodules", () => { expect(status.submodules).not.toContain("HARL"); }); + it("renders a newly staged submodule pointer as new → (zero head OID)", async () => { + const { dir, c2 } = createFixture(); + git(dir, ["submodule", "add", join(dir, "..", "origin"), "NEWSUB"]); // staged add: `1 A. S...` with a zero head OID + + const status = await gitStatus(dir); + const pointer = status.files.find((file) => file.path === "NEWSUB"); + expect(pointer?.index).toBe("added"); + expect(pointer?.submoduleFromCommit).toBe("new"); + expect(pointer?.submoduleToCommit).toBe(c2.slice(0, 7)); + expect(status.submodules).toContain("NEWSUB"); + }); + it("prefixes oldPath with the submodule path for renames inside a submodule", async () => { const { dir } = createFixture(); git(join(dir, "HARL"), ["mv", "a.txt", "renamed.txt"]); diff --git a/src/server/git/gitService.ts b/src/server/git/gitService.ts index 8bf33bb..718a55f 100644 --- a/src/server/git/gitService.ts +++ b/src/server/git/gitService.ts @@ -85,7 +85,7 @@ async function expandSubmodule(cwd: string, sub: SubmoduleRecord): Promise<{ fil path: sub.path, index: sub.index, workingTree: sub.workingTree, - submoduleFromCommit: short(sub.headOid), + submoduleFromCommit: displayFromCommit(sub.headOid), submoduleToCommit: short(await resolveSubmoduleToCommit(cwd, sub)), }); } @@ -284,6 +284,11 @@ function short(oid: string): string { return oid.slice(0, 7); } +/** A newly staged submodule records an all-zero head OID; display the pointer as `new → `. */ +function displayFromCommit(headOid: string): string { + return /^0+$/.test(headOid) ? "new" : short(headOid); +} + function hash(value: string): string { return createHash("sha1").update(value).digest("hex"); } From 10ee3daa0cd5b47c1378cd1cf3d9ba0d6a28e579 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 12:06:33 +0200 Subject: [PATCH 6/8] fix(ui): keep git view toggle stationary when expand-all appears The right-anchored .toolbar-actions group rendered the view toggle left of the conditional expand/collapse-all button, so the toggle jumped left whenever the button appeared. Render expand/collapse-all first (leftmost) so only the space to its left changes; the toggle and Refresh stay put. --- src/client/src/components/WorkspaceGitPanel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/src/components/WorkspaceGitPanel.ts b/src/client/src/components/WorkspaceGitPanel.ts index aa93add..73b5c68 100644 --- a/src/client/src/components/WorkspaceGitPanel.ts +++ b/src/client/src/components/WorkspaceGitPanel.ts @@ -64,8 +64,8 @@ export class WorkspaceGitPanel extends LitElement { Git ${context.gitStale ? html`stale` : null}
- ${this.renderViewToggle()} ${viewState.expandablePaths.length > 0 ? this.renderExpandCollapseAll(viewState.expandablePaths) : null} + ${this.renderViewToggle()}
From 6db065e984f9010e03c0cadd4c9b378312e2edd8 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 12:09:55 +0200 Subject: [PATCH 7/8] refactor(ui): dedupe git file helpers and memoize git view model - N1: extract the copy-pasted pointerName/segmentName helpers from gitFileList.ts and gitFileTree.ts into gitFileShared.ts. - N3: drop the dead conditional "tree" class (no CSS rule exists). - N4 (P3): memoize computeViewState on (status, view) identity so renders from expand/collapse or diff selection skip the full model rebuild; expand state is read live at render time, never cached. --- .../src/components/WorkspaceGitPanel.ts | 21 ++++++++++- src/client/src/gitFileList.ts | 13 +------ src/client/src/gitFileShared.test.ts | 35 +++++++++++++++++++ src/client/src/gitFileShared.ts | 18 ++++++++++ src/client/src/gitFileTree.ts | 13 +------ 5 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 src/client/src/gitFileShared.test.ts create mode 100644 src/client/src/gitFileShared.ts diff --git a/src/client/src/components/WorkspaceGitPanel.ts b/src/client/src/components/WorkspaceGitPanel.ts index 73b5c68..006cc94 100644 --- a/src/client/src/components/WorkspaceGitPanel.ts +++ b/src/client/src/components/WorkspaceGitPanel.ts @@ -13,6 +13,12 @@ interface GitViewState { readonly expandablePaths: readonly string[]; } +interface GitViewStateCache { + readonly status: GitStatusResponse | undefined; + readonly view: GitFileView; + readonly viewState: GitViewState; +} + const EMPTY_LIST_MODEL: GitFileListModel = { submodules: [], files: [] }; const EMPTY_VIEW_STATE: GitViewState = { nodes: [], listModel: EMPTY_LIST_MODEL, expandablePaths: [] }; @@ -45,6 +51,11 @@ export class WorkspaceGitPanel extends LitElement { // submodule groups, both keyed by their path. @state() private expandedDirectories = new Set(); + // Memoized on (status, view) identity: renders triggered by expand/collapse + // or diff selection reuse the model, while a new poll or view switch rebuilds + // it. Expand state is read live at render time, never baked into the model. + private viewStateCache: GitViewStateCache | undefined; + protected override willUpdate(changedProperties: PropertyValues): void { if (!changedProperties.has("context")) return; const previous = changedProperties.get("context"); @@ -70,7 +81,7 @@ export class WorkspaceGitPanel extends LitElement {
-
+
${this.renderFileList(context, status, viewState)}
${renderDiffViewer(context)}
@@ -166,6 +177,14 @@ export class WorkspaceGitPanel extends LitElement { } private computeViewState(status: GitStatusResponse | undefined): GitViewState { + const cached = this.viewStateCache; + if (cached !== undefined && cached.status === status && cached.view === this.view) return cached.viewState; + const viewState = this.buildViewState(status); + this.viewStateCache = { status, view: this.view, viewState }; + return viewState; + } + + private buildViewState(status: GitStatusResponse | undefined): GitViewState { if (status === undefined || !status.isGitRepo || status.files.length === 0) return EMPTY_VIEW_STATE; if (this.view === "tree") { const nodes = buildGitFileTree(status.files, status.submodules); diff --git a/src/client/src/gitFileList.ts b/src/client/src/gitFileList.ts index 32faa6e..133ff72 100644 --- a/src/client/src/gitFileList.ts +++ b/src/client/src/gitFileList.ts @@ -1,4 +1,5 @@ import type { GitStatusFile } from "./api"; +import { pointerName, segmentName } from "./gitFileShared"; /** A changed file inside a submodule, shown flat in list view. `path` is the * full superproject-relative path (the diff key); `relativePath` is shown in @@ -70,15 +71,3 @@ function ownerSubmodule(path: string, submodules: readonly string[]): string | u } return best; } - -function pointerName(file: GitStatusFile): string { - const from = file.submoduleFromCommit; - const to = file.submoduleToCommit; - return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; -} - -function segmentName(path: string): string { - const segments = path.split("/"); - const last = segments[segments.length - 1]; - return last ?? path; -} diff --git a/src/client/src/gitFileShared.test.ts b/src/client/src/gitFileShared.test.ts new file mode 100644 index 0000000..9ffda1a --- /dev/null +++ b/src/client/src/gitFileShared.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import type { GitStatusFile } from "./api"; +import { pointerName, segmentName } from "./gitFileShared"; + +describe("pointerName", () => { + it("renders the moved pointer as → ", () => { + expect(pointerName(pointer("1111111", "2222222"))).toBe("1111111 → 2222222"); + }); + + it("falls back to a plain label when either end is unresolved", () => { + expect(pointerName(pointer(undefined, "2222222"))).toBe("commit"); + expect(pointerName(pointer("1111111", undefined))).toBe("commit"); + expect(pointerName(pointer(undefined, undefined))).toBe("commit"); + }); +}); + +describe("segmentName", () => { + it("returns the final path segment", () => { + expect(segmentName("deps/sub/module")).toBe("module"); + }); + + it("returns the whole path when there is no separator", () => { + expect(segmentName("HARL")).toBe("HARL"); + }); +}); + +function pointer(from: string | undefined, to: string | undefined): GitStatusFile { + return { + path: "HARL", + index: "unmodified", + workingTree: "modified", + ...(from === undefined ? {} : { submoduleFromCommit: from }), + ...(to === undefined ? {} : { submoduleToCommit: to }), + }; +} diff --git a/src/client/src/gitFileShared.ts b/src/client/src/gitFileShared.ts new file mode 100644 index 0000000..d1e5d1d --- /dev/null +++ b/src/client/src/gitFileShared.ts @@ -0,0 +1,18 @@ +import type { GitStatusFile } from "./api"; + +/** + * Display label for a submodule commit-pointer row: the ` → ` + * short-SHA summary, or "commit" when the server did not resolve both ends. + */ +export function pointerName(file: GitStatusFile): string { + const from = file.submoduleFromCommit; + const to = file.submoduleToCommit; + return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; +} + +/** Final segment of a path, used as the display name for tree/list rows. */ +export function segmentName(path: string): string { + const segments = path.split("/"); + const last = segments[segments.length - 1]; + return last ?? path; +} diff --git a/src/client/src/gitFileTree.ts b/src/client/src/gitFileTree.ts index 972f15e..4c2813e 100644 --- a/src/client/src/gitFileTree.ts +++ b/src/client/src/gitFileTree.ts @@ -1,4 +1,5 @@ import type { GitStatusFile } from "./api"; +import { pointerName, segmentName } from "./gitFileShared"; /** * A changed file placed at a leaf of the Git file tree. `path` is the full @@ -93,12 +94,6 @@ export function collectGitFileTreeDirectoryPaths(nodes: readonly GitFileTreeNode return paths; } -function pointerName(file: GitStatusFile): string { - const from = file.submoduleFromCommit; - const to = file.submoduleToCommit; - return from !== undefined && to !== undefined ? `${from} → ${to}` : "commit"; -} - function createDirectoryAccumulator(path: string): DirectoryAccumulator { return { path, directories: new Map(), files: [], isSubmodule: false }; } @@ -125,9 +120,3 @@ function finalizeChildren(directory: DirectoryAccumulator): GitFileTreeNode[] { const files = [...directory.files].sort((left, right) => left.name.localeCompare(right.name)); return [...(directory.pointer === undefined ? [] : [directory.pointer]), ...directories, ...files]; } - -function segmentName(path: string): string { - const segments = path.split("/"); - const last = segments[segments.length - 1]; - return last ?? path; -} From 095adfe55018eddec4b74aa8011989ede7c01c94 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 12:30:56 +0200 Subject: [PATCH 8/8] test(ui): add WorkspaceGitPanel component-boundary tests --- .../src/components/WorkspaceGitPanel.test.ts | 392 ++++++++++++++++++ 1 file changed, 392 insertions(+) create mode 100644 src/client/src/components/WorkspaceGitPanel.test.ts diff --git a/src/client/src/components/WorkspaceGitPanel.test.ts b/src/client/src/components/WorkspaceGitPanel.test.ts new file mode 100644 index 0000000..c87b216 --- /dev/null +++ b/src/client/src/components/WorkspaceGitPanel.test.ts @@ -0,0 +1,392 @@ +import type { TemplateResult } from "lit"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { GitStatusFile, GitStatusResponse } from "../api"; +import { initialAppState } from "../appState"; +import { GIT_FILE_VIEW_STORAGE_KEY } from "../gitFileViewPreference"; +import type { WorkspacePanelContext } from "../plugins/types"; +// Genuine Lit event-wiring extraction (view-toggle, expand/collapse-all, and +// row clicks) routes through the shared, type-guarded template-inspection +// escape hatch; see ../templateInspection.testSupport for the proportionality +// rationale. Vitest runs in the node environment (no DOM), so the few +// rendered-output assertions (toolbar order, badge, state labels) read the +// returned TemplateResult through the shared templateText/templateStrings/ +// templateValues primitives, anchored to stable user-facing labels and markup +// — kept narrow per the testing guide. +import { + findOptionalTemplateClickHandlerForText, + isTemplateResult, + templateClickHandlerForText, + templateStrings, + templateText, + templateValues, +} from "../templateInspection.testSupport"; +import { WorkspaceGitPanel } from "./WorkspaceGitPanel"; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("workspace-git-panel view toggle", () => { + it("switches between list and tree rendering and persists the choice", () => { + const { panel, storage } = newGitPanel(); + const onSelectDiff = vi.fn(); + panel.context = workspacePanelContext({ gitStatus: gitStatus({ files: [gitFile("src/main.ts")] }), onSelectDiff }); + + // List view (the default) renders full-path rows that load their diff. + templateClickHandlerForText(panel.render(), "src/main.ts")(new Event("click")); + expect(onSelectDiff).toHaveBeenCalledWith("src/main.ts"); + + templateClickHandlerForText(panel.render(), "Tree")(new Event("click")); + expect(storage.value(GIT_FILE_VIEW_STORAGE_KEY)).toBe("tree"); + + // Tree view nests the file under its (collapsed) directory instead. + let rendered = panel.render(); + expect(findOptionalTemplateClickHandlerForText(rendered, "src/main.ts")).toBeUndefined(); + templateClickHandlerForText(rendered, "src"); + + templateClickHandlerForText(panel.render(), "List")(new Event("click")); + expect(storage.value(GIT_FILE_VIEW_STORAGE_KEY)).toBe("list"); + rendered = panel.render(); + expect(findOptionalTemplateClickHandlerForText(rendered, "src/main.ts")).toBeDefined(); + }); + + it("restores the persisted view on construction", () => { + const { panel } = newGitPanel({ [GIT_FILE_VIEW_STORAGE_KEY]: "tree" }); + panel.context = workspacePanelContext({ gitStatus: gitStatus({ files: [gitFile("src/main.ts")] }) }); + + const rendered = panel.render(); + expect(findOptionalTemplateClickHandlerForText(rendered, "src/main.ts")).toBeUndefined(); + + templateClickHandlerForText(rendered, "src")(new Event("click")); + expect(findOptionalTemplateClickHandlerForText(panel.render(), "main.ts")).toBeDefined(); + }); +}); + +describe("workspace-git-panel toolbar order", () => { + it("renders expand/collapse-all before the view toggle, which stays anchored next to Refresh", () => { + // Pins the UX fix: the right-anchored view toggle and Refresh button must + // not move when the expand/collapse-all control appears or disappears. + const listPanel = newGitPanel().panel; + listPanel.context = workspacePanelContext({ gitStatus: gitStatus({ files: [gitFile("one.ts")] }) }); + const withoutExpandAll = toolbarSlots(listPanel.render()); + expect(withoutExpandAll.expandCollapseSlot).toBeNull(); + + const treePanel = newGitPanel({ [GIT_FILE_VIEW_STORAGE_KEY]: "tree" }).panel; + treePanel.context = workspacePanelContext({ gitStatus: treeStatus() }); + const withExpandAll = toolbarSlots(treePanel.render()); + const expandCollapse = withExpandAll.expandCollapseSlot; + if (!isTemplateResult(expandCollapse)) throw new Error("expand/collapse-all control missing in tree view"); + expect(templateText(expandCollapse)).toContain("Expand all"); + + // The toggle occupies the same template slot whether or not the + // expand/collapse-all control renders in the slot before it. + expect(withExpandAll.toggleIndex).toBe(withoutExpandAll.toggleIndex); + }); +}); + +describe("workspace-git-panel expand/collapse all", () => { + it("expands and collapses every directory, flipping its label", () => { + const { panel } = newGitPanel({ [GIT_FILE_VIEW_STORAGE_KEY]: "tree" }); + panel.context = workspacePanelContext({ gitStatus: treeStatus() }); + + expect(findOptionalTemplateClickHandlerForText(panel.render(), "b.ts")).toBeUndefined(); + + templateClickHandlerForText(panel.render(), "Expand all")(new Event("click")); + let rendered = panel.render(); + expect(findOptionalTemplateClickHandlerForText(rendered, "Expand all")).toBeUndefined(); + templateClickHandlerForText(rendered, "Collapse all"); + templateClickHandlerForText(rendered, "a.ts"); + templateClickHandlerForText(rendered, "b.ts"); + + templateClickHandlerForText(panel.render(), "Collapse all")(new Event("click")); + rendered = panel.render(); + templateClickHandlerForText(rendered, "Expand all"); + expect(findOptionalTemplateClickHandlerForText(rendered, "b.ts")).toBeUndefined(); + }); + + it("keeps the Expand all label while expansion is partial", () => { + const { panel } = newGitPanel({ [GIT_FILE_VIEW_STORAGE_KEY]: "tree" }); + panel.context = workspacePanelContext({ gitStatus: treeStatus() }); + + templateClickHandlerForText(panel.render(), "src")(new Event("click")); + + const rendered = panel.render(); + templateClickHandlerForText(rendered, "Expand all"); + templateClickHandlerForText(rendered, "a.ts"); + expect(findOptionalTemplateClickHandlerForText(rendered, "b.ts")).toBeUndefined(); + }); + + it("expands and collapses submodule groups in list view", () => { + const { panel } = newGitPanel(); + panel.context = workspacePanelContext({ gitStatus: submoduleStatus() }); + + expect(findOptionalTemplateClickHandlerForText(panel.render(), "abc1234 → def5678")).toBeUndefined(); + + templateClickHandlerForText(panel.render(), "Expand all")(new Event("click")); + let rendered = panel.render(); + templateClickHandlerForText(rendered, "abc1234 → def5678"); + templateClickHandlerForText(rendered, "lib.ts"); + templateClickHandlerForText(rendered, "Collapse all"); + + templateClickHandlerForText(panel.render(), "Collapse all")(new Event("click")); + rendered = panel.render(); + expect(findOptionalTemplateClickHandlerForText(rendered, "abc1234 → def5678")).toBeUndefined(); + expect(findOptionalTemplateClickHandlerForText(rendered, "lib.ts")).toBeUndefined(); + }); +}); + +describe("workspace-git-panel rows", () => { + it("toggles a directory's children when its row is clicked", () => { + const { panel } = newGitPanel({ [GIT_FILE_VIEW_STORAGE_KEY]: "tree" }); + panel.context = workspacePanelContext({ gitStatus: treeStatus() }); + + templateClickHandlerForText(panel.render(), "src")(new Event("click")); + expect(findOptionalTemplateClickHandlerForText(panel.render(), "a.ts")).toBeDefined(); + + templateClickHandlerForText(panel.render(), "src")(new Event("click")); + expect(findOptionalTemplateClickHandlerForText(panel.render(), "a.ts")).toBeUndefined(); + }); + + it("renders submodule groups with a basename header and badge, and pointer rows when expanded", () => { + const { panel } = newGitPanel(); + const onSelectDiff = vi.fn(); + panel.context = workspacePanelContext({ gitStatus: submoduleStatus(), onSelectDiff }); + + // Collapsed: only the basename header (with its submodule badge) shows. + const collapsed = panel.render(); + const headerText = rowTextFor(collapsed, "harl"); + expect(headerText).toContain("harl"); + expect(headerText).toContain("submodule"); + expect(findOptionalTemplateClickHandlerForText(collapsed, "abc1234 → def5678")).toBeUndefined(); + + templateClickHandlerForText(collapsed, "harl")(new Event("click")); + + // Expanded: the → pointer row and inner files select diffs. + const rendered = panel.render(); + templateClickHandlerForText(rendered, "abc1234 → def5678")(new Event("click")); + expect(onSelectDiff).toHaveBeenCalledWith("vendor/harl"); + templateClickHandlerForText(rendered, "lib.ts")(new Event("click")); + expect(onSelectDiff).toHaveBeenCalledWith("vendor/harl/lib.ts"); + }); + + it("renders M/A/D/U state labels for changed files", () => { + const { panel } = newGitPanel(); + panel.context = workspacePanelContext({ + gitStatus: gitStatus({ + files: [ + gitFile("m.ts", { index: "unmodified", workingTree: "modified" }), + gitFile("a.ts", { index: "added", workingTree: "unmodified" }), + gitFile("d.ts", { index: "unmodified", workingTree: "deleted" }), + gitFile("u.ts", { index: "unmodified", workingTree: "untracked" }), + ], + }), + }); + + const rendered = panel.render(); + expect(rowTextFor(rendered, "m.ts")).toContain("M"); + expect(rowTextFor(rendered, "a.ts")).toContain("A"); + expect(rowTextFor(rendered, "d.ts")).toContain("D"); + expect(rowTextFor(rendered, "u.ts")).toContain("U"); + }); +}); + +describe("workspace-git-panel context reset", () => { + it("resets expansion state when the workspace context key changes", () => { + const { panel } = newGitPanel({ [GIT_FILE_VIEW_STORAGE_KEY]: "tree" }); + const context = workspacePanelContext({ gitStatus: treeStatus() }); + panel.context = context; + templateClickHandlerForText(panel.render(), "src")(new Event("click")); + expect(findOptionalTemplateClickHandlerForText(panel.render(), "a.ts")).toBeDefined(); + + const switched = workspacePanelContext({ + gitStatus: treeStatus(), + workspace: { id: "workspace-2", projectId: "project-1", path: "/tmp/project-2", label: "other", isMain: false, isGitRepo: true, isGitWorktree: false }, + }); + panel.context = switched; + callPanelWillUpdate(panel, context); + + const rendered = panel.render(); + templateClickHandlerForText(rendered, "Expand all"); + expect(findOptionalTemplateClickHandlerForText(rendered, "a.ts")).toBeUndefined(); + }); + + it("keeps expansion state across disconnect/reconnect with the same context key", () => { + const { panel } = newGitPanel({ [GIT_FILE_VIEW_STORAGE_KEY]: "tree" }); + const context = workspacePanelContext({ gitStatus: treeStatus() }); + panel.context = context; + templateClickHandlerForText(panel.render(), "src")(new Event("click")); + + panel.context = undefined; + callPanelWillUpdate(panel, context); + expect(templateText(panel.render())).toContain("Git unavailable."); + + const reconnected = workspacePanelContext({ gitStatus: treeStatus() }); + panel.context = reconnected; + callPanelWillUpdate(panel, undefined); + expect(findOptionalTemplateClickHandlerForText(panel.render(), "a.ts")).toBeDefined(); + + // A fresh context object with the same key (defined → defined) also keeps state. + const sameKey = workspacePanelContext({ gitStatus: treeStatus() }); + panel.context = sameKey; + callPanelWillUpdate(panel, reconnected); + expect(findOptionalTemplateClickHandlerForText(panel.render(), "a.ts")).toBeDefined(); + }); +}); + +function newGitPanel(storageValues: Record = {}): { panel: WorkspaceGitPanel; storage: FakeStorage } { + const storage = new FakeStorage(storageValues); + vi.stubGlobal("window", { localStorage: storage }); + return { panel: new WorkspaceGitPanel(), storage }; +} + +/** + * Structural pin for the toolbar layout contract: the expand/collapse-all + * slot precedes the view-toggle slot inside `.toolbar-actions`, and the + * toggle is always immediately followed by the Refresh button — so the + * right-anchored controls never move when expand/collapse-all appears. + */ +function toolbarSlots(rendered: TemplateResult): { expandCollapseSlot: unknown; toggleIndex: number } { + const strings = templateStrings(rendered); + const values = templateValues(rendered); + const actionsIndex = strings.findIndex((chunk) => chunk.includes("toolbar-actions")); + if (actionsIndex < 0) throw new Error("toolbar-actions markup missing from the rendered panel"); + const toggleIndex = actionsIndex + 1; + const toggle = values[toggleIndex]; + if (!isTemplateResult(toggle) || !templateStrings(toggle).some((chunk) => chunk.includes("view-toggle"))) { + throw new Error("view toggle is not the control immediately after the expand/collapse-all slot"); + } + const refreshOpenChunk = strings[toggleIndex + 1] ?? ""; + const refreshLabelChunk = strings[toggleIndex + 2] ?? ""; + if (!refreshOpenChunk.includes("Refresh")) { + throw new Error("Refresh button does not immediately follow the view toggle"); + } + return { expandCollapseSlot: values[actionsIndex], toggleIndex }; +} + +/** Flattened rendered text of the deepest template containing `anchor`. */ +function rowTextFor(rendered: TemplateResult, anchor: string): string { + const text = findDeepestTemplateText(rendered, anchor); + if (text === undefined) throw new Error(`Expected a rendered row containing ${anchor}`); + return text; +} + +function findDeepestTemplateText(value: unknown, anchor: string): string | undefined { + if (Array.isArray(value)) { + for (const item of value) { + const found = findDeepestTemplateText(item, anchor); + if (found !== undefined) return found; + } + return undefined; + } + if (!isTemplateResult(value)) return undefined; + for (const item of templateValues(value)) { + const found = findDeepestTemplateText(item, anchor); + if (found !== undefined) return found; + } + const text = templateText(value); + return text.includes(anchor) ? text : undefined; +} + +// Protected lifecycle methods are invoked through Reflect, matching the +// SettingsSessiondPanel.test.ts precedent. +function callPanelWillUpdate(panel: WorkspaceGitPanel, previous: WorkspacePanelContext | undefined): unknown { + const method: unknown = Reflect.get(panel, "willUpdate"); + if (typeof method !== "function") throw new Error("WorkspaceGitPanel.willUpdate is not callable"); + return Reflect.apply(method, panel, [new Map([["context", previous]])]); +} + +function treeStatus(): GitStatusResponse { + return gitStatus({ + files: [ + gitFile("src/a.ts"), + gitFile("src/nested/b.ts"), + gitFile("README.md"), + ], + }); +} + +function submoduleStatus(): GitStatusResponse { + return gitStatus({ + submodules: ["vendor/harl"], + files: [ + gitFile("vendor/harl", { submoduleFromCommit: "abc1234", submoduleToCommit: "def5678" }), + gitFile("vendor/harl/lib.ts"), + ], + }); +} + +function gitFile(path: string, patch: Partial = {}): GitStatusFile { + return { path, index: "unmodified", workingTree: "modified", ...patch }; +} + +function gitStatus(patch: Partial = {}): GitStatusResponse { + return { + isGitRepo: true, + hash: "hash-1", + branch: "main", + files: [], + submodules: [], + ...patch, + }; +} + +class FakeStorage { + private readonly values = new Map(); + + constructor(initial: Record = {}) { + for (const [key, value] of Object.entries(initial)) this.values.set(key, value); + } + + getItem(key: string): string | null { + return this.values.get(key) ?? null; + } + + setItem(key: string, value: string): void { + this.values.set(key, value); + } + + value(key: string): string | undefined { + return this.values.get(key); + } +} + +function workspacePanelContext(patch: Partial = {}): WorkspacePanelContext { + const workspace = patch.workspace ?? { id: "workspace-1", projectId: "project-1", path: "/tmp/project", label: "main", isMain: true, isGitRepo: true, isGitWorktree: false }; + return { + machine: patch.machine ?? { id: "local", name: "Local", kind: "local" }, + workspace, + state: patch.state ?? { ...initialAppState(), workspaceUploadBatches: {} }, + files: patch.files ?? { + readFile: vi.fn(() => Promise.reject(new Error("not implemented"))), + writeFile: vi.fn(() => Promise.reject(new Error("not implemented"))), + deleteFile: vi.fn(() => Promise.reject(new Error("not implemented"))), + moveFile: vi.fn(() => Promise.reject(new Error("not implemented"))), + }, + prompt: patch.prompt ?? { insertText: vi.fn(), getText: vi.fn(() => ""), getSelection: vi.fn(() => null) }, + terminal: patch.terminal ?? { open: vi.fn(), runCommand: vi.fn(() => Promise.reject(new Error("not implemented"))) }, + host: patch.host ?? { requestRender: vi.fn() }, + fileTree: patch.fileTree ?? [], + expandedDirs: patch.expandedDirs ?? {}, + selectedFilePath: patch.selectedFilePath, + selectedFileContent: patch.selectedFileContent, + fileTreeStale: patch.fileTreeStale ?? false, + gitStatus: patch.gitStatus, + selectedDiffPath: patch.selectedDiffPath, + selectedDiff: patch.selectedDiff, + selectedStagedDiff: patch.selectedStagedDiff, + gitStale: patch.gitStale ?? false, + activeTerminalCount: patch.activeTerminalCount ?? 0, + selectedTerminalId: patch.selectedTerminalId, + terminalAutoStart: patch.terminalAutoStart ?? false, + workspaceUploadDefaultFolder: patch.workspaceUploadDefaultFolder ?? ".pi-web/uploads", + onRefreshFiles: patch.onRefreshFiles ?? vi.fn(), + onExpandDir: patch.onExpandDir ?? vi.fn(), + onSelectFile: patch.onSelectFile ?? vi.fn(), + onStartWorkspaceUpload: patch.onStartWorkspaceUpload ?? vi.fn(() => undefined), + onCancelWorkspaceUpload: patch.onCancelWorkspaceUpload ?? vi.fn(), + onClearWorkspaceUpload: patch.onClearWorkspaceUpload ?? vi.fn(), + onRefreshGit: patch.onRefreshGit ?? vi.fn(), + onSelectDiff: patch.onSelectDiff ?? vi.fn(), + onSelectTerminal: patch.onSelectTerminal ?? vi.fn(), + }; +}