diff --git a/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx index d7959dc966f..c429396ef86 100644 --- a/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/combined-diff/CombinedDiffViewer.tsx @@ -178,7 +178,7 @@ export default function CombinedDiffViewer({ registry, requestSectionReload, sectionIndexByKeyRef: treeNavigation.sectionIndexByKeyRef, - sections, + sectionEntries: entrySet.entries, shouldAutoReloadFromGitStatus: entrySet.shouldAutoReloadFromGitStatus, treeMode: entrySet.treeMode }) diff --git a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-filter.ts b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-filter.ts index 93e1dd3de26..8fb0dc51dd1 100644 --- a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-filter.ts +++ b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-filter.ts @@ -45,6 +45,34 @@ function getEntrySearchText(entry: CombinedDiffFileTreeEntry): string { .toLowerCase() } +/** + * Applies filters that describe the file set itself. Viewed state is deliberately not included: + * section loading changes viewed flags without changing the tree's path structure. + */ +export function getCombinedDiffFileTreeEntriesMatchingStaticFilters({ + entries, + query, + excludedExtensions +}: { + entries: readonly CombinedDiffFileTreeEntry[] + query: string + excludedExtensions: ReadonlySet +}): readonly CombinedDiffFileTreeEntry[] { + if (isCombinedDiffFileTreeQueryTooLarge(query)) { + return [] + } + const normalizedQuery = query.trim().toLowerCase() + if (normalizedQuery.length === 0 && excludedExtensions.size === 0) { + return entries + } + return entries.filter((entry) => { + if (excludedExtensions.has(getEntryExtension(entry))) { + return false + } + return normalizedQuery.length === 0 || getEntrySearchText(entry).includes(normalizedQuery) + }) +} + export function getFilteredCombinedDiffFileTreeEntries({ entries, mode, @@ -60,19 +88,19 @@ export function getFilteredCombinedDiffFileTreeEntries({ includeViewed: boolean viewedSectionKeys: ReadonlySet }): CombinedDiffFileTreeEntry[] { - if (isCombinedDiffFileTreeQueryTooLarge(query)) { - return [] + const staticFilteredEntries = getCombinedDiffFileTreeEntriesMatchingStaticFilters({ + entries, + query, + excludedExtensions + }) + if (includeViewed) { + return [...staticFilteredEntries] } - const trimmedQuery = query.trim() - const normalizedQuery = trimmedQuery.toLowerCase() - return entries.filter((entry) => { - if (excludedExtensions.has(getEntryExtension(entry))) { - return false - } + return staticFilteredEntries.filter((entry) => { if (!includeViewed && viewedSectionKeys.has(getCombinedDiffFileTreeSectionKey(mode, entry))) { return false } - return normalizedQuery.length === 0 || getEntrySearchText(entry).includes(normalizedQuery) + return true }) } diff --git a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-model.ts b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-model.ts new file mode 100644 index 00000000000..a0b12ad7510 --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-model.ts @@ -0,0 +1,179 @@ +import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types' +import type { GitStatusEntry, GitStagingArea } from '../../../../../../shared/git-status-types' +import { + buildGitStatusSourceControlTree, + buildSourceControlTree, + compactSourceControlTree, + flattenSourceControlTree, + type SourceControlTreeNode +} from '@/components/right-sidebar/source-control-tree' +import { + getCombinedDiffFileTreeSectionKey, + isGitStatusEntry, + type CombinedDiffBranchTreeArea, + type CombinedDiffFileTreeEntry, + type CombinedDiffFileTreeMode +} from '../resolve-changes/combined-diff-section-identity' +import type { CombinedDiffTreeNode } from './combined-diff-file-tree-row' + +const UNCOMMITTED_AREA_ORDER: readonly GitStagingArea[] = ['unstaged', 'staged', 'untracked'] +const UNCOMMITTED_AREA_LABELS: Record = { + unstaged: 'Changes', + staged: 'Staged Changes', + untracked: 'Untracked Files' +} + +export type CombinedDiffTreeGroup = { + area: GitStagingArea + label: string + roots: CombinedDiffTreeNode[] +} + +export type CombinedDiffTreeVisibility = { + rows: CombinedDiffTreeNode[] + visibleFileCount: number + visibleFileCounts: ReadonlyMap +} + +export type { CombinedDiffTreeNode } + +/** Build the uncommitted tree shape without volatile viewed/loading flags. */ +export function buildCombinedDiffUncommittedTreeGroups( + entries: readonly CombinedDiffFileTreeEntry[] +): CombinedDiffTreeGroup[] { + return UNCOMMITTED_AREA_ORDER.map((area) => { + const areaEntries = entries.filter( + (entry): entry is GitStatusEntry => isGitStatusEntry(entry) && entry.area === area + ) + if (areaEntries.length === 0) { + return null + } + + const roots = compactSourceControlTree(buildGitStatusSourceControlTree(area, areaEntries)) + return { + area, + label: UNCOMMITTED_AREA_LABELS[area], + roots: roots as CombinedDiffTreeNode[] + } + }).filter((group): group is CombinedDiffTreeGroup => group !== null) +} + +/** Build the committed tree shape without volatile viewed/loading flags. */ +export function buildCombinedDiffBranchTreeRoots( + mode: Extract, + entries: readonly CombinedDiffFileTreeEntry[] +): CombinedDiffTreeNode[] { + const branchEntries = entries.filter( + (entry): entry is GitBranchChangeEntry => !isGitStatusEntry(entry) + ) + const area: CombinedDiffBranchTreeArea = mode === 'commit' ? 'combined-commit' : 'combined-branch' + const roots = compactSourceControlTree(buildSourceControlTree(area, [...branchEntries])) + return roots as CombinedDiffTreeNode[] +} + +/** Flatten a stable tree shape; this is the path used when viewed files are included. */ +export function flattenCombinedDiffTreeRoots( + roots: readonly CombinedDiffTreeNode[], + collapsedDirectoryKeys: ReadonlySet +): CombinedDiffTreeNode[] { + return flattenSourceControlTree( + roots as SourceControlTreeNode[], + collapsedDirectoryKeys + ) as CombinedDiffTreeNode[] +} + +/** + * Apply viewed state as a lightweight overlay. It filters and re-compacts the already-sorted tree + * in linear time, preserving the file-tree shape while avoiding a fresh path build and sort. + */ +export function getViewedCombinedDiffTreeVisibility({ + roots, + collapsedDirectoryKeys, + mode, + viewedSectionKeys +}: { + roots: readonly CombinedDiffTreeNode[] + collapsedDirectoryKeys: ReadonlySet + mode: CombinedDiffFileTreeMode + viewedSectionKeys: ReadonlySet +}): CombinedDiffTreeVisibility { + const visibleFileCounts = new Map() + const rows: CombinedDiffTreeNode[] = [] + + type VisibleTreeNode = { + source: CombinedDiffTreeNode + children: VisibleTreeNode[] + fileCount: number + } + + const projectVisibleTree = (node: CombinedDiffTreeNode): VisibleTreeNode | null => { + if (node.type === 'file') { + return viewedSectionKeys.has(getCombinedDiffFileTreeSectionKey(mode, node.entry)) + ? null + : { source: node, children: [], fileCount: 1 } + } + const children = node.children + .map((child) => projectVisibleTree(child as CombinedDiffTreeNode)) + .filter((child): child is VisibleTreeNode => child !== null) + if (children.length === 0) { + return null + } + return { + source: node, + children, + fileCount: children.reduce((count, child) => count + child.fileCount, 0) + } + } + + const compactVisibleTree = (projected: VisibleTreeNode, depth: number): CombinedDiffTreeNode => { + if (projected.source.type === 'file') { + return { ...projected.source, depth } + } + const names = [projected.source.name] + let compacted = projected + // Keep a collapsed directory as a visible boundary; filtering must not compact it away and + // accidentally expose descendants that the user explicitly hid. + while ( + !collapsedDirectoryKeys.has(compacted.source.key) && + compacted.children.length === 1 && + compacted.children[0]?.source.type === 'directory' + ) { + compacted = compacted.children[0] + names.push(compacted.source.name) + } + const compactedSource = compacted.source + if (compactedSource.type !== 'directory') { + throw new Error('Combined diff directory projection lost its source node') + } + const node = { + ...compactedSource, + name: names.join('/'), + depth, + fileCount: compacted.fileCount, + children: compacted.children.map((child) => compactVisibleTree(child, depth + 1)) + } satisfies CombinedDiffTreeNode + visibleFileCounts.set(node.key, node.fileCount) + return node + } + + const visit = (node: CombinedDiffTreeNode): void => { + rows.push(node) + if (node.type === 'directory' && !collapsedDirectoryKeys.has(node.key)) { + for (const child of node.children) { + visit(child) + } + } + } + + let visibleFileCount = 0 + for (const root of roots) { + const projected = projectVisibleTree(root) + if (!projected) { + continue + } + const compacted = compactVisibleTree(projected, 0) + visibleFileCount += projected.fileCount + visit(compacted) + } + return { rows, visibleFileCount, visibleFileCounts } +} diff --git a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-row.tsx b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-row.tsx index e7ccab9b470..b71aab643b5 100644 --- a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-row.tsx +++ b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree-row.tsx @@ -1,4 +1,4 @@ -import { createElement } from 'react' +import { createElement, memo } from 'react' import { ChevronDown, Folder, FolderOpen } from 'lucide-react' import { STATUS_COLORS, STATUS_LABELS } from '@/components/right-sidebar/status-display' import type { SourceControlTreeNode } from '@/components/right-sidebar/source-control-tree' @@ -28,13 +28,14 @@ const COMBINED_DIFF_TREE_INDENT_PX = 12 const COMBINED_DIFF_TREE_DIRECTORY_PADDING_PX = 8 const COMBINED_DIFF_TREE_FILE_PADDING_PX = 20 -export function CombinedDiffFileTreeRow({ +export const CombinedDiffFileTreeRow = memo(function CombinedDiffFileTreeRow({ node, mode, worktreePath, activeSectionKey, sectionIndexByKey, isCollapsed, + visibleFileCount, onToggleDirectory, onNavigate }: { @@ -44,6 +45,7 @@ export function CombinedDiffFileTreeRow({ activeSectionKey: string | null sectionIndexByKey: ReadonlyMap isCollapsed: boolean + visibleFileCount?: number onToggleDirectory: (key: string) => void onNavigate: (entry: CombinedDiffFileTreeEntry) => void }): React.JSX.Element { @@ -77,7 +79,7 @@ export function CombinedDiffFileTreeRow({ {node.name} - {node.fileCount} + {visibleFileCount ?? node.fileCount} ) @@ -132,4 +134,4 @@ export function CombinedDiffFileTreeRow({ ) -} +}) diff --git a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.test.ts b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.test.ts index 1b8d686c3a9..63209d37de3 100644 --- a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.test.ts +++ b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.test.ts @@ -10,10 +10,15 @@ import { import { COMBINED_DIFF_FILE_TREE_QUERY_MAX_BYTES, getCombinedDiffBranchEntriesInTreeOrder, + getCombinedDiffFileTreeEntriesMatchingStaticFilters, getFilteredCombinedDiffFileTreeEntries, isCombinedDiffFileTreeQueryTooLarge, isCombinedDiffSectionViewed } from './combined-diff-file-tree-filter' +import { + buildCombinedDiffBranchTreeRoots, + getViewedCombinedDiffTreeVisibility +} from './combined-diff-file-tree-model' import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types' import type { GitStatusEntry } from '../../../../../../shared/git-status-types' @@ -145,4 +150,69 @@ describe('CombinedDiffFileTree navigation mapping', () => { }) ).toEqual([]) }) + + it('retains the structural entry list when only viewed state can change', () => { + const entries: GitBranchChangeEntry[] = [ + { path: 'src/a.ts', status: 'modified' }, + { path: 'src/b.ts', status: 'modified' } + ] + + expect( + getCombinedDiffFileTreeEntriesMatchingStaticFilters({ + entries, + query: '', + excludedExtensions: new Set() + }) + ).toBe(entries) + }) + + it('overlays viewed files while preserving filtered-tree compaction', () => { + const entries: GitBranchChangeEntry[] = [ + { path: 'src/a.ts', status: 'modified' }, + { path: 'src/nested/b.ts', status: 'modified' }, + { path: 'docs/readme.md', status: 'modified' } + ] + const roots = buildCombinedDiffBranchTreeRoots('branch', entries) + const visibility = getViewedCombinedDiffTreeVisibility({ + roots, + collapsedDirectoryKeys: new Set(), + mode: 'branch', + viewedSectionKeys: new Set(['combined-branch:src/a.ts', 'combined-branch:docs/readme.md']) + }) + + expect( + visibility.rows.filter((node) => node.type === 'file').map((node) => node.entry.path) + ).toEqual(['src/nested/b.ts']) + expect(visibility.visibleFileCount).toBe(1) + const compactedDirectory = visibility.rows.find((node) => node.type === 'directory') + expect(compactedDirectory).toMatchObject({ + path: 'src/nested', + name: 'src/nested', + fileCount: 1 + }) + expect(compactedDirectory && visibility.visibleFileCounts.get(compactedDirectory.key)).toBe(1) + }) + + it('preserves a collapsed directory boundary while filtering viewed siblings', () => { + const entries: GitBranchChangeEntry[] = [ + { path: 'src/a/one.ts', status: 'modified' }, + { path: 'src/b/two.ts', status: 'modified' } + ] + const roots = buildCombinedDiffBranchTreeRoots('branch', entries) + const visibility = getViewedCombinedDiffTreeVisibility({ + roots, + collapsedDirectoryKeys: new Set(['dir::combined-branch::src']), + mode: 'branch', + viewedSectionKeys: new Set(['combined-branch:src/b/two.ts']) + }) + + expect(visibility.rows).toEqual([ + expect.objectContaining({ + type: 'directory', + key: 'dir::combined-branch::src', + path: 'src' + }) + ]) + expect(visibility.visibleFileCount).toBe(1) + }) }) diff --git a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.tsx b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.tsx index 5afb79ccac3..dc08942f8d5 100644 --- a/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.tsx +++ b/src/renderer/src/components/editor/combined-diff/browse-files/combined-diff-file-tree.tsx @@ -5,70 +5,23 @@ import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { - buildGitStatusSourceControlTree, - buildSourceControlTree, - compactSourceControlTree, - flattenSourceControlTree -} from '@/components/right-sidebar/source-control-tree' -import type { GitBranchChangeEntry } from '../../../../../../shared/git-diff-compare-types' -import type { GitStagingArea, GitStatusEntry } from '../../../../../../shared/git-status-types' -import { - getEntryExtension, - getFilteredCombinedDiffFileTreeEntries + getCombinedDiffFileTreeEntriesMatchingStaticFilters, + getEntryExtension } from './combined-diff-file-tree-filter' -import { - isGitStatusEntry, - type CombinedDiffBranchTreeArea, - type CombinedDiffFileTreeEntry, - type CombinedDiffFileTreeMode +import type { + CombinedDiffFileTreeEntry, + CombinedDiffFileTreeMode } from '../resolve-changes/combined-diff-section-identity' -import { CombinedDiffFileTreeRow, type CombinedDiffTreeNode } from './combined-diff-file-tree-row' +import { CombinedDiffFileTreeRow } from './combined-diff-file-tree-row' import { useCombinedDiffFileTreeResize } from './use-combined-diff-file-tree-resize' import { translate } from '@/i18n/i18n' - -const UNCOMMITTED_AREA_ORDER: readonly GitStagingArea[] = ['unstaged', 'staged', 'untracked'] -const UNCOMMITTED_AREA_LABELS: Record = { - unstaged: 'Changes', - staged: 'Staged Changes', - untracked: 'Untracked Files' -} - -function buildUncommittedRows( - entries: readonly CombinedDiffFileTreeEntry[], - collapsedDirectoryKeys: ReadonlySet -): { area: GitStagingArea; label: string; rows: CombinedDiffTreeNode[] }[] { - return UNCOMMITTED_AREA_ORDER.map((area) => { - const areaEntries = entries.filter( - (entry): entry is GitStatusEntry => isGitStatusEntry(entry) && entry.area === area - ) - if (areaEntries.length === 0) { - return null - } - - const roots = compactSourceControlTree(buildGitStatusSourceControlTree(area, areaEntries)) - return { - area, - label: UNCOMMITTED_AREA_LABELS[area], - rows: flattenSourceControlTree(roots, collapsedDirectoryKeys) as CombinedDiffTreeNode[] - } - }).filter( - (group): group is { area: GitStagingArea; label: string; rows: CombinedDiffTreeNode[] } => - Boolean(group) - ) -} - -function buildBranchRows( - mode: Extract, - entries: readonly CombinedDiffFileTreeEntry[], - collapsedDirectoryKeys: ReadonlySet -): CombinedDiffTreeNode[] { - const branchEntries = entries.filter( - (entry): entry is GitBranchChangeEntry => !isGitStatusEntry(entry) - ) - const area: CombinedDiffBranchTreeArea = mode === 'commit' ? 'combined-commit' : 'combined-branch' - const roots = compactSourceControlTree(buildSourceControlTree(area, branchEntries)) - return flattenSourceControlTree(roots, collapsedDirectoryKeys) as CombinedDiffTreeNode[] -} +import { + buildCombinedDiffBranchTreeRoots, + buildCombinedDiffUncommittedTreeGroups, + flattenCombinedDiffTreeRoots, + getViewedCombinedDiffTreeVisibility, + type CombinedDiffTreeNode +} from './combined-diff-file-tree-model' export function CombinedDiffFileTree({ mode, @@ -115,17 +68,16 @@ export function CombinedDiffFileTree({ () => Array.from(new Set(entries.map(getEntryExtension))).sort(), [entries] ) - const filteredEntries = React.useMemo( + // Why: viewed/loading state changes for one section must not invalidate the path filter or tree + // construction. It is applied below as a visibility overlay. + const structurallyFilteredEntries = React.useMemo( () => - getFilteredCombinedDiffFileTreeEntries({ + getCombinedDiffFileTreeEntriesMatchingStaticFilters({ entries, - mode, query, - excludedExtensions, - includeViewed, - viewedSectionKeys + excludedExtensions }), - [entries, excludedExtensions, includeViewed, mode, query, viewedSectionKeys] + [entries, excludedExtensions, query] ) const toggleExtension = React.useCallback((extension: string) => { setExcludedExtensions((prev) => { @@ -146,20 +98,74 @@ export function CombinedDiffFileTree({ const activeFilterCount = excludedExtensions.size + (includeViewed ? 0 : 1) + (query.trim().length > 0 ? 1 : 0) - const uncommittedGroups = React.useMemo( + const uncommittedTreeGroups = React.useMemo( () => mode === 'all' || mode === 'uncommitted' - ? buildUncommittedRows(filteredEntries, collapsedDirectoryKeys) + ? buildCombinedDiffUncommittedTreeGroups(structurallyFilteredEntries) : [], - [collapsedDirectoryKeys, filteredEntries, mode] + [mode, structurallyFilteredEntries] ) - const branchRows = React.useMemo( + const branchTreeRoots = React.useMemo( () => mode === 'all' || mode === 'branch' || mode === 'commit' - ? buildBranchRows(mode, filteredEntries, collapsedDirectoryKeys) + ? buildCombinedDiffBranchTreeRoots(mode, structurallyFilteredEntries) : [], - [collapsedDirectoryKeys, filteredEntries, mode] + [mode, structurallyFilteredEntries] ) + // Why: the viewed overlay below replaces these rows entirely when viewed files are hidden, so + // flattening the unfiltered tree there is pure dead work. + const uncommittedRowsByArea = React.useMemo(() => { + const rowsByArea = new Map() + if (!includeViewed) { + return rowsByArea + } + for (const group of uncommittedTreeGroups) { + rowsByArea.set(group.area, flattenCombinedDiffTreeRoots(group.roots, collapsedDirectoryKeys)) + } + return rowsByArea + }, [collapsedDirectoryKeys, includeViewed, uncommittedTreeGroups]) + const branchRows = React.useMemo( + () => + includeViewed ? flattenCombinedDiffTreeRoots(branchTreeRoots, collapsedDirectoryKeys) : [], + [branchTreeRoots, collapsedDirectoryKeys, includeViewed] + ) + const uncommittedVisibleRowsByArea = React.useMemo(() => { + if (includeViewed) { + return null + } + const rowsByArea = new Map>() + for (const group of uncommittedTreeGroups) { + rowsByArea.set( + group.area, + getViewedCombinedDiffTreeVisibility({ + roots: group.roots, + collapsedDirectoryKeys, + mode, + viewedSectionKeys + }) + ) + } + return rowsByArea + }, [collapsedDirectoryKeys, includeViewed, mode, uncommittedTreeGroups, viewedSectionKeys]) + const branchVisibleRows = React.useMemo( + () => + includeViewed + ? null + : getViewedCombinedDiffTreeVisibility({ + roots: branchTreeRoots, + collapsedDirectoryKeys, + mode, + viewedSectionKeys + }), + [branchTreeRoots, collapsedDirectoryKeys, includeViewed, mode, viewedSectionKeys] + ) + const visibleEntryCount = includeViewed + ? structurallyFilteredEntries.length + : (branchVisibleRows?.visibleFileCount ?? 0) + + Array.from(uncommittedVisibleRowsByArea?.values() ?? []).reduce( + (count, visibility) => count + visibility.visibleFileCount, + 0 + ) if (collapsed) { return null @@ -278,7 +284,7 @@ export function CombinedDiffFileTree({
- {filteredEntries.length === 0 ? ( + {visibleEntryCount === 0 ? (
{translate( 'auto.components.editor.CombinedDiffFileTree.f984289373', @@ -287,27 +293,40 @@ export function CombinedDiffFileTree({
) : mode === 'all' || mode === 'uncommitted' ? ( <> - {uncommittedGroups.map((group) => ( -
-
- {group.label} + {uncommittedTreeGroups.map((group) => { + const rows = + uncommittedVisibleRowsByArea?.get(group.area)?.rows ?? + uncommittedRowsByArea.get(group.area) ?? + [] + const visibleFileCounts = uncommittedVisibleRowsByArea?.get( + group.area + )?.visibleFileCounts + if (rows.length === 0) { + return null + } + return ( +
+
+ {group.label} +
+ {rows.map((node) => ( + + ))}
- {group.rows.map((node) => ( - - ))} -
- ))} - {mode === 'all' && branchRows.length > 0 ? ( + ) + })} + {mode === 'all' && (branchVisibleRows?.rows ?? branchRows).length > 0 ? (
{translate( @@ -315,7 +334,7 @@ export function CombinedDiffFileTree({ 'Committed on Branch' )}
- {branchRows.map((node) => ( + {(branchVisibleRows?.rows ?? branchRows).map((node) => ( @@ -332,7 +352,7 @@ export function CombinedDiffFileTree({ ) : null} ) : ( - branchRows.map((node) => ( + (branchVisibleRows?.rows ?? branchRows).map((node) => ( diff --git a/src/renderer/src/components/editor/combined-diff/browse-files/use-combined-diff-tree-navigation.test.ts b/src/renderer/src/components/editor/combined-diff/browse-files/use-combined-diff-tree-navigation.test.ts new file mode 100644 index 00000000000..66f1f23beee --- /dev/null +++ b/src/renderer/src/components/editor/combined-diff/browse-files/use-combined-diff-tree-navigation.test.ts @@ -0,0 +1,73 @@ +// @vitest-environment happy-dom + +import React from 'react' +import { renderHook } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' +import type { DiffSection } from '../../diff-section-types' +import { useCombinedDiffTreeNavigation } from './use-combined-diff-tree-navigation' + +function makeSection(key: string, viewed: boolean): DiffSection { + return { + key, + path: key, + status: 'modified', + originalContent: '', + modifiedContent: '', + collapsed: false, + loading: !viewed, + loadOnDemand: !viewed, + dirty: false, + diffResult: null, + largeDiffRenderLimit: null + } +} + +function renderNavigation(sections: DiffSection[], entrySignature: string) { + const sectionsRef = { current: sections } as React.RefObject + return renderHook( + (props: { sections: DiffSection[]; entrySignature: string }) => { + sectionsRef.current = props.sections + return useCombinedDiffTreeNavigation({ + ensureSectionLoaded: vi.fn(), + entrySignature: props.entrySignature, + markDirectScrollInput: vi.fn(), + scrollToIndex: vi.fn(), + sections: props.sections, + sectionsRef, + toggleSection: vi.fn(), + treeMode: 'all' + }) + }, + { initialProps: { sections, entrySignature } } + ) +} + +describe('useCombinedDiffTreeNavigation viewedSectionKeys', () => { + it('keeps every viewed key when sections are reordered under one entry signature', () => { + const a = makeSection('a', true) + const b = makeSection('b', true) + const view = renderNavigation([a, b], 'sig') + expect([...view.result.current.viewedSectionKeys].sort()).toEqual(['a', 'b']) + + view.rerender({ sections: [b, a], entrySignature: 'sig' }) + expect([...view.result.current.viewedSectionKeys].sort()).toEqual(['a', 'b']) + }) + + it('patches only the flipped section while keys stay in place', () => { + const a = makeSection('a', true) + const view = renderNavigation([a, makeSection('b', false)], 'sig') + expect([...view.result.current.viewedSectionKeys]).toEqual(['a']) + + view.rerender({ sections: [a, makeSection('b', true)], entrySignature: 'sig' }) + expect([...view.result.current.viewedSectionKeys].sort()).toEqual(['a', 'b']) + }) + + it('reuses the cached set when no section changed viewed state', () => { + const sections = [makeSection('a', true), makeSection('b', false)] + const view = renderNavigation(sections, 'sig') + const first = view.result.current.viewedSectionKeys + + view.rerender({ sections: [...sections], entrySignature: 'sig' }) + expect(view.result.current.viewedSectionKeys).toBe(first) + }) +}) diff --git a/src/renderer/src/components/editor/combined-diff/browse-files/use-combined-diff-tree-navigation.ts b/src/renderer/src/components/editor/combined-diff/browse-files/use-combined-diff-tree-navigation.ts index 21f6afd369e..a237bc24c0d 100644 --- a/src/renderer/src/components/editor/combined-diff/browse-files/use-combined-diff-tree-navigation.ts +++ b/src/renderer/src/components/editor/combined-diff/browse-files/use-combined-diff-tree-navigation.ts @@ -37,10 +37,33 @@ export function useCombinedDiffTreeNavigation({ toggleSection: (index: number) => void treeMode: CombinedDiffFileTreeMode }): CombinedDiffTreeNavigation { - const sectionIndexByKey = React.useMemo( - () => createCombinedDiffSectionIndexMap(sections), - [sections] - ) + const sectionIndexCacheRef = useRef<{ + entrySignature: string + sectionCount: number + map: Map + keys: string[] + } | null>(null) + const sectionIndexByKey = React.useMemo(() => { + const previous = sectionIndexCacheRef.current + // Section content/loading updates preserve entry order and keys. The entry signature and + // count usually change when the navigable structure changes, but compare keys as a guard for + // same-sized/reused signatures (and to keep this cache correct if a caller rebuilds sections). + if ( + previous?.entrySignature === entrySignature && + previous.sectionCount === sections.length && + sections.every((section, index) => previous.keys[index] === section.key) + ) { + return previous.map + } + const map = createCombinedDiffSectionIndexMap(sections) + sectionIndexCacheRef.current = { + entrySignature, + sectionCount: sections.length, + map, + keys: sections.map((section) => section.key) + } + return map + }, [entrySignature, sections]) const sectionIndexByKeyRef = useRef>(sectionIndexByKey) sectionIndexByKeyRef.current = sectionIndexByKey @@ -54,15 +77,59 @@ export function useCombinedDiffTreeNavigation({ // Why: the tree highlight belongs to one entry set; reset now so it can't flash on another before an Effect would. setActiveTreeSectionState({ entrySignature, key: null }) } - const viewedSectionKeys = React.useMemo( - () => - new Set( + const viewedSectionCacheRef = useRef<{ + entrySignature: string + sections: DiffSection[] + keys: Set + } | null>(null) + const viewedSectionKeys = React.useMemo(() => { + const recomputeAllViewedKeys = (): Set => { + const keys = new Set( sections .filter((section) => isCombinedDiffSectionViewed(section)) .map((section) => section.key) - ), - [sections] - ) + ) + viewedSectionCacheRef.current = { entrySignature, sections, keys } + return keys + } + const previous = viewedSectionCacheRef.current + if ( + previous === null || + previous.entrySignature !== entrySignature || + previous.sections.length !== sections.length + ) { + return recomputeAllViewedKeys() + } + + let keys = previous.keys + let copied = false + for (let index = 0; index < sections.length; index += 1) { + const previousSection = previous.sections[index] + const section = sections[index] + if (!previousSection || !section) { + continue + } + // Why: reordered keys can't be patched index by index — a later delete would drop an earlier add. + if (previousSection.key !== section.key) { + return recomputeAllViewedKeys() + } + const viewed = isCombinedDiffSectionViewed(section) + if (isCombinedDiffSectionViewed(previousSection) === viewed) { + continue + } + if (!copied) { + keys = new Set(previous.keys) + copied = true + } + if (viewed) { + keys.add(section.key) + } else { + keys.delete(section.key) + } + } + viewedSectionCacheRef.current = { entrySignature, sections, keys } + return keys + }, [entrySignature, sections]) const handleTreeNavigate = useCallback( (entry: GitStatusEntry | GitBranchChangeEntry) => { markDirectScrollInput() diff --git a/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-revalidation.ts b/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-revalidation.ts index d8f080a0bc4..3f603131a7f 100644 --- a/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-revalidation.ts +++ b/src/renderer/src/components/editor/combined-diff/load-sections/use-combined-diff-section-revalidation.ts @@ -1,7 +1,6 @@ import React, { useEffect, useRef } from 'react' import type { OpenFile } from '@/store/slices/editor' import type { GitStatusEntry } from '../../../../../../shared/git-status-types' -import type { DiffSection } from '../../diff-section-types' import { ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, type EditorPathMutationTarget @@ -21,7 +20,7 @@ export function useCombinedDiffSectionRevalidation({ registry, requestSectionReload, sectionIndexByKeyRef, - sections, + sectionEntries, shouldAutoReloadFromGitStatus, treeMode }: { @@ -30,7 +29,9 @@ export function useCombinedDiffSectionRevalidation({ registry: CombinedDiffSectionLoadRegistry requestSectionReload: (index: number) => void sectionIndexByKeyRef: React.RefObject> - sections: DiffSection[] + // The entry set is structurally stable while individual section content/loading state changes. + // Use it for the status signature so progressive loads do not rescan the section array. + sectionEntries: readonly { path: string }[] shouldAutoReloadFromGitStatus: boolean treeMode: CombinedDiffFileTreeMode }): string { @@ -39,8 +40,8 @@ export function useCombinedDiffSectionRevalidation({ if (!shouldAutoReloadFromGitStatus) { return '' } - return buildCombinedGitStatusSignature(sections, gitStatusEntries) - }, [gitStatusEntries, sections, shouldAutoReloadFromGitStatus]) + return buildCombinedGitStatusSignature(sectionEntries, gitStatusEntries) + }, [gitStatusEntries, sectionEntries, shouldAutoReloadFromGitStatus]) const prevCombinedGitStatusSignatureRef = useRef(null) useEffect(() => {