From 0e58d730ab02dd6cf68dd7cfbee73ec772896977 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:46:10 -0700 Subject: [PATCH] fix(diff-view): defer loading large untracked files and refactor fallbac MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split on-demand load decision logic to distinguish tracked vs untracked files — large untracked files now properly defer loading while untracked images remain automatic. Extract fallback height computation into a dedicated function to centralize the logic for render-limited and in-flight-loading states, reducing code duplication and clarifying when to use bounded fallback heights. --- .../editor/LargeDiffLoadPrompt.test.tsx | 2 +- .../combined-diff-on-demand-load.test.ts | 14 ++++++++++-- .../editor/combined-diff-on-demand-load.ts | 22 +++++++++++++++---- .../use-combined-diff-view-restore.ts | 4 +++- .../use-combined-diff-virtualizer.ts | 6 ++--- .../editor/diff-section-layout.test.ts | 21 +++++++++++++++++- .../components/editor/diff-section-layout.ts | 21 ++++++++++++++++++ .../editor/useDiffSectionLayoutMetrics.ts | 5 +++-- 8 files changed, 81 insertions(+), 14 deletions(-) diff --git a/src/renderer/src/components/editor/LargeDiffLoadPrompt.test.tsx b/src/renderer/src/components/editor/LargeDiffLoadPrompt.test.tsx index d04a65b4531..f6ee3d009b1 100644 --- a/src/renderer/src/components/editor/LargeDiffLoadPrompt.test.tsx +++ b/src/renderer/src/components/editor/LargeDiffLoadPrompt.test.tsx @@ -16,7 +16,7 @@ describe('LargeDiffLoadPrompt', () => { ) - expect(screen.getByText('Large diffs are not rendered by default.')).toBeDefined() + screen.getByText('Large diffs are not rendered by default.') fireEvent.click(screen.getByRole('button', { name: 'Load diff' })) expect(onLoad).toHaveBeenCalledOnce() diff --git a/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts b/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts index 8931f4baea3..c0216a30ee8 100644 --- a/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts +++ b/src/renderer/src/components/editor/combined-diff-on-demand-load.test.ts @@ -23,8 +23,18 @@ describe('combined diff on-demand loading', () => { ).toBe(false) }) - it('automatically loads diffs when line counts are unavailable', () => { - expect(shouldLoadCombinedDiffOnDemand({ added: undefined, removed: undefined })).toBe(false) + it('automatically loads tracked diffs when line counts are unavailable', () => { + expect( + shouldLoadCombinedDiffOnDemand({ added: undefined, removed: undefined, area: 'unstaged' }) + ).toBe(false) + }) + + it('defers untracked files whose line counts were skipped as too large', () => { + expect(shouldLoadCombinedDiffOnDemand({ area: 'untracked', path: 'data/dump.json' })).toBe(true) + }) + + it('automatically loads untracked images that report no line counts', () => { + expect(shouldLoadCombinedDiffOnDemand({ area: 'untracked', path: 'docs/Shot.PNG' })).toBe(false) }) it('defers untracked diffs when only additions are reported', () => { diff --git a/src/renderer/src/components/editor/combined-diff-on-demand-load.ts b/src/renderer/src/components/editor/combined-diff-on-demand-load.ts index 1b1dc9bcb42..4cc4c959d77 100644 --- a/src/renderer/src/components/editor/combined-diff-on-demand-load.ts +++ b/src/renderer/src/components/editor/combined-diff-on-demand-load.ts @@ -1,16 +1,30 @@ +import type { GitStatusEntry } from '../../../../shared/git-status-types' +import { IMAGE_FILE_EXTENSIONS } from '../../../../shared/image-file-extensions' + export const MAX_AUTOMATIC_DIFF_CHANGED_LINES = 10_000 +function isImagePath(path: string | undefined): boolean { + const lowerPath = path?.toLowerCase() + return lowerPath !== undefined && IMAGE_FILE_EXTENSIONS.some((ext) => lowerPath.endsWith(ext)) +} + export function shouldLoadCombinedDiffOnDemand({ added, - removed + removed, + area, + path }: { added?: number removed?: number + area?: GitStatusEntry['area'] + path?: string }): boolean { - // Untracked text files report additions only; both fields are absent when - // line counts are unavailable (for example, binary or oversized files). if (added === undefined && removed === undefined) { - return false + // Untracked files lose their counts once they exceed the status-scan size cap + // (MAX_UNTRACKED_LINE_COUNT_BYTES), so an uncounted untracked file is either + // oversized text or binary — both too costly to auto-load. Images stay automatic + // because their preview is the point of the row. + return area === 'untracked' && !isImagePath(path) } return (added ?? 0) + (removed ?? 0) > MAX_AUTOMATIC_DIFF_CHANGED_LINES } diff --git a/src/renderer/src/components/editor/combined-diff/remember-view/use-combined-diff-view-restore.ts b/src/renderer/src/components/editor/combined-diff/remember-view/use-combined-diff-view-restore.ts index f67ad22158a..39c5c6ca290 100644 --- a/src/renderer/src/components/editor/combined-diff/remember-view/use-combined-diff-view-restore.ts +++ b/src/renderer/src/components/editor/combined-diff/remember-view/use-combined-diff-view-restore.ts @@ -138,7 +138,9 @@ export function useCombinedDiffViewRestore({ entries.map((entry) => { const loadOnDemand = shouldLoadCombinedDiffOnDemand({ added: 'added' in entry ? entry.added : undefined, - removed: 'removed' in entry ? entry.removed : undefined + removed: 'removed' in entry ? entry.removed : undefined, + area: 'area' in entry ? entry.area : undefined, + path: entry.path }) return { key: getCombinedDiffFileTreeSectionKey(treeMode, entry), diff --git a/src/renderer/src/components/editor/combined-diff/scroll-viewport/use-combined-diff-virtualizer.ts b/src/renderer/src/components/editor/combined-diff/scroll-viewport/use-combined-diff-virtualizer.ts index b97e36e3b41..88801dd1c9e 100644 --- a/src/renderer/src/components/editor/combined-diff/scroll-viewport/use-combined-diff-virtualizer.ts +++ b/src/renderer/src/components/editor/combined-diff/scroll-viewport/use-combined-diff-virtualizer.ts @@ -5,7 +5,8 @@ import type { ProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks' import type { DiffSection } from '../../diff-section-types' import { getDiffSectionEstimatedHeight, - isIntrinsicHeightImageDiff + isIntrinsicHeightImageDiff, + usesLargeDiffFallbackHeight } from '../../diff-section-layout' const COMBINED_DIFF_OVERSCAN = 5 @@ -48,8 +49,7 @@ export function useCombinedDiffVirtualizer({ ? undefined : (section.added ?? 0) + (section.removed ?? 0), useIntrinsicImageHeight: isIntrinsicHeightImageDiff(section.diffResult), - isLargeDiffLimited: - section.largeDiffRenderLimit?.limited === true || section.loadOnDemand === true, + isLargeDiffLimited: usesLargeDiffFallbackHeight(section), lineCounts: section.largeDiffRenderLimit?.lineCounts ?? undefined }) }, diff --git a/src/renderer/src/components/editor/diff-section-layout.test.ts b/src/renderer/src/components/editor/diff-section-layout.test.ts index 2728622879a..dcd2a6990e4 100644 --- a/src/renderer/src/components/editor/diff-section-layout.test.ts +++ b/src/renderer/src/components/editor/diff-section-layout.test.ts @@ -3,7 +3,8 @@ import { getDiffSectionBodyHeight, getLargeDiffFallbackBodyHeight, getDiffSectionEstimatedHeight, - isIntrinsicHeightImageDiff + isIntrinsicHeightImageDiff, + usesLargeDiffFallbackHeight } from './diff-section-layout' import type { GitDiffResult } from '../../../../shared/git-diff-compare-types' @@ -227,6 +228,24 @@ describe('diff section layout', () => { ).toBe(188) }) + it('keeps the deferred fallback height while a loaded-on-demand diff fetches', () => { + const section = { + path: 'big.txt', + added: 50_000, + removed: 0, + largeDiffRenderLimit: null + } + expect(usesLargeDiffFallbackHeight({ ...section, loading: false, loadOnDemand: true })).toBe( + true + ) + expect(usesLargeDiffFallbackHeight({ ...section, loading: true, loadOnDemand: false })).toBe( + true + ) + expect( + usesLargeDiffFallbackHeight({ ...section, added: 3, loading: true, loadOnDemand: false }) + ).toBe(false) + }) + it('estimates collapsed virtualized sections as header-only rows', () => { expect( getDiffSectionEstimatedHeight({ diff --git a/src/renderer/src/components/editor/diff-section-layout.ts b/src/renderer/src/components/editor/diff-section-layout.ts index e6aa6043fea..f8b7875c89f 100644 --- a/src/renderer/src/components/editor/diff-section-layout.ts +++ b/src/renderer/src/components/editor/diff-section-layout.ts @@ -1,4 +1,6 @@ import type { GitDiffResult } from '../../../../shared/git-diff-compare-types' +import { shouldLoadCombinedDiffOnDemand } from './combined-diff-on-demand-load' +import type { DiffSection } from './diff-section-types' import { countLinesLikeSplit, type DiffLineCounts } from './large-diff-render-limit' const DIFF_LINE_HEIGHT = 19 @@ -28,6 +30,25 @@ export function getLargeDiffFallbackBodyHeight(): number { return LARGE_DIFF_FALLBACK_BODY_HEIGHT } +/** + * Rows that size from the bounded fallback instead of their (absent) content: + * render-limited rows, rows still waiting behind the load prompt, and — while + * the fetch is in flight — rows the user just loaded, so starting a load does + * not shrink the section to the empty-content minimum and shift the list. + */ +export function usesLargeDiffFallbackHeight( + section: Pick< + DiffSection, + 'added' | 'area' | 'largeDiffRenderLimit' | 'loading' | 'loadOnDemand' | 'path' | 'removed' + > +): boolean { + return ( + section.largeDiffRenderLimit?.limited === true || + section.loadOnDemand === true || + (section.loading && shouldLoadCombinedDiffOnDemand(section)) + ) +} + export function getDiffSectionBodyHeight({ measuredContentHeight, originalContent, diff --git a/src/renderer/src/components/editor/useDiffSectionLayoutMetrics.ts b/src/renderer/src/components/editor/useDiffSectionLayoutMetrics.ts index d49805dc146..4727216b8e8 100644 --- a/src/renderer/src/components/editor/useDiffSectionLayoutMetrics.ts +++ b/src/renderer/src/components/editor/useDiffSectionLayoutMetrics.ts @@ -3,7 +3,8 @@ import { computeLineStats } from './diff-line-stats' import { getDiffSectionBodyHeight, getLargeDiffFallbackBodyHeight, - isIntrinsicHeightImageDiff + isIntrinsicHeightImageDiff, + usesLargeDiffFallbackHeight } from './diff-section-layout' import type { DiffSection } from './diff-section-types' @@ -20,7 +21,7 @@ export function useDiffSectionLayoutMetrics({ isLargeDiffLimited: boolean } { const renderLimit = section.largeDiffRenderLimit - const isLargeDiffLimited = renderLimit?.limited === true || section.loadOnDemand === true + const isLargeDiffLimited = usesLargeDiffFallbackHeight(section) const lineStats = useMemo( () => section.loading || section.error || isLargeDiffLimited