From 55e7b1058e464489219dc2cc9583a727cb74246d Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Wed, 18 Jun 2025 18:00:11 -0700 Subject: [PATCH] Consolidate diff viewer state, various cleanup around that change --- .../components/diff/ConciseDiffView.svelte | 56 ++-- .../diff/concise-diff-view.svelte.ts | 91 ++++--- web/src/lib/diff-viewer-multi-file.svelte.ts | 239 +++++++++--------- web/src/lib/github.svelte.ts | 23 +- web/src/lib/util.ts | 101 +++++--- web/src/routes/+page.svelte | 19 +- web/src/routes/DiffTitle.svelte | 6 +- web/src/routes/FileHeader.svelte | 8 +- web/src/routes/OpenDiffDialog.svelte | 74 +----- 9 files changed, 301 insertions(+), 316 deletions(-) diff --git a/web/src/lib/components/diff/ConciseDiffView.svelte b/web/src/lib/components/diff/ConciseDiffView.svelte index 7049750..a0fe7eb 100644 --- a/web/src/lib/components/diff/ConciseDiffView.svelte +++ b/web/src/lib/components/diff/ConciseDiffView.svelte @@ -2,7 +2,6 @@ import { type ConciseDiffViewProps, ConciseDiffViewState, - getBaseColors, innerPatchLineTypeProps, type InnerPatchLineTypeProps, makeSearchSegments, @@ -54,20 +53,6 @@ cacheKey: box.with(() => cacheKey), }); - let baseColors: Promise = $state(new Promise(() => [])); - $effect(() => { - const promise = getBaseColors(syntaxHighlightingTheme, syntaxHighlighting); - // Same idea as above - promise.then( - () => { - baseColors = promise; - }, - () => { - baseColors = promise; - }, - ); - }); - function getDisplayLineNo(line: PatchLine, num: number | undefined) { if (line.type == PatchLineType.HEADER) { return "..."; @@ -117,10 +102,11 @@ if (!matchingLines || matchingLines.length === 0) { return []; } - const lines = await view.patchLines; + const diffViewerPatch = await view.diffViewerPatch; + const hunks = diffViewerPatch.hunks; const segments: SearchSegment[][][] = []; const count: MutableValue = { value: 0 }; - for (let i = 0; i < lines.length; i++) { + for (let i = 0; i < hunks.length; i++) { const hunkMatchingLines = matchingLines[i]; if (!hunkMatchingLines || hunkMatchingLines.length === 0) { continue; @@ -129,7 +115,7 @@ const hunkSegments: SearchSegment[][] = []; segments[i] = hunkSegments; - const hunkLines = lines[i]; + const hunkLines = hunks[i].lines; for (let j = 0; j < hunkLines.length; j++) { const line = hunkLines[j]; @@ -170,24 +156,24 @@ {#await searchSegments} {@render lineContent(line, lineType, innerLineType)} {:then completedSearchSegments} - {@const hunkSegments = completedSearchSegments[hunkIndex]} - {#if hunkSegments !== undefined && hunkSegments.length > 0} - {@const lineSegments = hunkSegments[lineIndex]} - {#if lineSegments !== undefined && lineSegments.length > 0} + {@const hunkSearchSegments = completedSearchSegments[hunkIndex]} + {#if hunkSearchSegments !== undefined && hunkSearchSegments.length > 0} + {@const lineSearchSegments = hunkSearchSegments[lineIndex]} + {#if lineSearchSegments !== undefined && lineSearchSegments.length > 0}
{@render lineContent(line, lineType, innerLineType)} - {#each lineSegments as segment, index (index)} - {#if segment.highlighted}{segment.text}{:else}{segment.text}{/if} + data-match-id={searchSegment.id}>{searchSegment.text}{:else}{searchSegment.text}{/if} {/each} @@ -214,16 +200,16 @@
{/snippet} -{#await Promise.all([baseColors, view.patchLines])} +{#await Promise.all([view.rootStyle, view.diffViewerPatch])}
-{:then [baseColors, lines]} +{:then [rootStyle, diffViewerPatch]}
- {#each lines as hunkLines, hunkIndex (hunkIndex)} - {#each hunkLines as line, lineIndex (lineIndex)} + {#each diffViewerPatch.hunks as hunk, hunkIndex (hunkIndex)} + {#each hunk.lines as line, lineIndex (lineIndex)} {@render renderLine(line, hunkIndex, lineIndex)} {/each} {/each} diff --git a/web/src/lib/components/diff/concise-diff-view.svelte.ts b/web/src/lib/components/diff/concise-diff-view.svelte.ts index 055d032..a89c861 100644 --- a/web/src/lib/components/diff/concise-diff-view.svelte.ts +++ b/web/src/lib/components/diff/concise-diff-view.svelte.ts @@ -19,6 +19,24 @@ import { onDestroy } from "svelte"; export const DEFAULT_THEME_LIGHT: BundledTheme = "github-light-default"; export const DEFAULT_THEME_DARK: BundledTheme = "github-dark-default"; +export type DiffViewerPatch = { + hunks: DiffViewerPatchHunk[]; +}; + +export type DiffViewerPatchHunk = { + lines: PatchLine[]; + innerPatchHeaderChangesOnly?: boolean; +}; + +export type PatchLine = { + type: PatchLineType; + content: LineSegment[]; + lineBreak?: boolean; + innerPatchLineType: InnerPatchLineType; + oldLineNo?: number; + newLineNo?: number; +}; + export type LineSegment = { text?: string | null; iconClass?: string | null; @@ -35,6 +53,12 @@ export enum PatchLineType { SPACER, } +export enum InnerPatchLineType { + ADD, + REMOVE, + NONE, +} + export type PatchLineTypeProps = { classes: string; lineNoClasses: string; @@ -69,12 +93,6 @@ export const patchLineTypeProps: Record = { }, }; -export enum InnerPatchLineType { - ADD, - REMOVE, - NONE, -} - export type InnerPatchLineTypeProps = { style: string; }; @@ -99,15 +117,6 @@ export const innerPatchLineTypeProps: Record(fn: (proc: LineProcessor) => Promise): Pr } } -export async function makeLines( +export async function parseDiffViewerPatch( patchPromise: StructuredPatch | Promise, syntaxHighlighting: boolean, syntaxHighlightingTheme: BundledTheme | undefined, omitPatchHeaderOnlyHunks: boolean, wordDiffs: boolean, -): Promise { +): Promise { const patch = await patchPromise; - const lines: PatchLine[][] = []; + const hunks: DiffViewerPatchHunk[] = []; for (let i = 0; i < patch.hunks.length; i++) { const hunk = patch.hunks[i]; - const hunkLines = await makeHunkLines(patch, hunk, syntaxHighlighting, syntaxHighlightingTheme, omitPatchHeaderOnlyHunks, wordDiffs); - lines.push(hunkLines); + hunks.push(await makeHunk(patch, hunk, syntaxHighlighting, syntaxHighlightingTheme, omitPatchHeaderOnlyHunks, wordDiffs)); } - return lines; + return { hunks }; } -async function makeHunkLines( +async function makeHunk( patch: StructuredPatch, hunk: StructuredPatchHunk, syntaxHighlighting: boolean, syntaxHighlightingTheme: BundledTheme | undefined, omitPatchHeaderOnlyHunks: boolean, wordDiffs: boolean, -): Promise { - const lines: PatchLine[] = []; - +): Promise { // Skip this hunk if it only contains header changes if (omitPatchHeaderOnlyHunks && !hasNonHeaderChanges(hunk.lines)) { - return lines; + return { innerPatchHeaderChangesOnly: true, lines: [] }; } + const lines: PatchLine[] = []; + // Add the hunk header const header = `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@`; lines.push({ @@ -665,7 +673,7 @@ async function makeHunkLines( // Add a separator between hunks lines.push({ content: [{ text: "" }], type: PatchLineType.SPACER, innerPatchLineType: InnerPatchLineType.NONE }); - return lines; + return { lines }; } export function hasNonHeaderChanges(contentLines: string[]) { @@ -971,14 +979,14 @@ async function getTheme(theme: BundledTheme | undefined): Promise; + diffViewerPatch: Promise; syntaxHighlighting: boolean; syntaxHighlightingTheme: BundledTheme | undefined; omitPatchHeaderOnlyHunks: boolean; wordDiffs: boolean; - constructor(patchLines: Promise, props: ConciseDiffViewStateProps) { - this.patchLines = patchLines; + constructor(diffViewerPatch: Promise, props: ConciseDiffViewStateProps) { + this.diffViewerPatch = diffViewerPatch; this.syntaxHighlighting = props.syntaxHighlighting.current; this.syntaxHighlightingTheme = props.syntaxHighlightingTheme.current; this.omitPatchHeaderOnlyHunks = props.omitPatchHeaderOnlyHunks.current; @@ -1034,8 +1042,9 @@ export type ConciseDiffViewStateProps = ReadableBoxedValues<{ }>; export class ConciseDiffViewState { - patchLines: Promise = $state(new Promise(() => [])); + diffViewerPatch: Promise = $state(new Promise(() => [])); cachedState: ConciseDiffViewCachedState | undefined = undefined; + rootStyle: Promise = $state(new Promise(() => [])); private readonly props: ConciseDiffViewStateProps; @@ -1046,6 +1055,18 @@ export class ConciseDiffViewState { this.update(); }); + $effect(() => { + const promise = getBaseColors(this.props.syntaxHighlightingTheme.current, this.props.syntaxHighlighting.current); + promise.then( + () => { + this.rootStyle = promise; + }, + () => { + this.rootStyle = promise; + }, + ); + }); + onDestroy(() => { if (this.props.cache.current !== undefined && this.props.cacheKey.current !== undefined && this.cachedState !== undefined) { this.props.cache.current.set(this.props.cacheKey.current, this.cachedState); @@ -1068,7 +1089,7 @@ export class ConciseDiffViewState { } } - const promise = makeLines( + const promise = parseDiffViewerPatch( this.props.patch.current, this.props.syntaxHighlighting.current, this.props.syntaxHighlightingTheme.current, @@ -1079,17 +1100,17 @@ export class ConciseDiffViewState { promise.then( () => { // Don't replace a potentially completed promise with a pending one, wait until the replacement is ready for smooth transitions - this.patchLines = promise; + this.diffViewerPatch = promise; }, () => { // Propagate errors - this.patchLines = promise; + this.diffViewerPatch = promise; }, ); } restore(state: ConciseDiffViewCachedState) { - this.patchLines = state.patchLines; + this.diffViewerPatch = state.diffViewerPatch; this.cachedState = state; } } diff --git a/web/src/lib/diff-viewer-multi-file.svelte.ts b/web/src/lib/diff-viewer-multi-file.svelte.ts index 6dd94ff..52b9864 100644 --- a/web/src/lib/diff-viewer-multi-file.svelte.ts +++ b/web/src/lib/diff-viewer-multi-file.svelte.ts @@ -1,12 +1,4 @@ -import { - fetchGithubCommitDiff, - fetchGithubComparison, - fetchGithubFile, - fetchGithubPRComparison, - type FileStatus, - getGithubToken, - type GithubDiff, -} from "./github.svelte"; +import { fetchGithubCommitDiff, fetchGithubComparison, fetchGithubPRComparison, type FileStatus, getGithubToken, type GithubDiff } from "./github.svelte"; import { type StructuredPatch, parsePatch } from "diff"; import { ConciseDiffViewCachedState, @@ -19,7 +11,7 @@ import { import type { BundledTheme } from "shiki"; import { browser } from "$app/environment"; import { getEffectiveGlobalTheme } from "$lib/theme.svelte"; -import { countOccurrences, type FileTreeNodeData, isImageFile, makeFileTree, type LazyPromise, lazyPromise, watchLocalStorage } from "$lib/util"; +import { countOccurrences, type FileTreeNodeData, makeFileTree, type LazyPromise, lazyPromise, watchLocalStorage } from "$lib/util"; import { onDestroy } from "svelte"; import { type TreeNode, TreeState } from "$lib/components/tree/index.svelte"; import { VList } from "virtua/svelte"; @@ -150,15 +142,68 @@ export const staticSidebar = new MediaQuery("(width >= 64rem)"); export type AddOrRemove = "add" | "remove"; -export type FileDetails = { - content: string; +export type CommonFileDetails = { fromFile: string; toFile: string; - fromBlob?: Blob; - toBlob?: Blob; status: FileStatus; }; +export type TextFileDetails = CommonFileDetails & { + type: "text"; + patchText: string; + structuredPatch: Promise; +}; + +export type ImageFileDetails = CommonFileDetails & { + type: "image"; + image: ImageDiffDetails; +}; + +export function makeTextDetails(fromFile: string, toFile: string, status: FileStatus, patchText: string): TextFileDetails { + return { + type: "text", + fromFile, + toFile, + status, + patchText, + structuredPatch: (async () => parseSinglePatch(patchText))(), + }; +} + +export function makeImageDetails( + fromFile: string, + toFile: string, + status: FileStatus, + fromBlob?: Promise | Blob, + toBlob?: Promise | Blob, +): ImageFileDetails { + return { + type: "image", + fromFile, + toFile, + status, + image: { + fileA: fromBlob !== undefined ? lazyPromise(async () => URL.createObjectURL(await fromBlob)) : null, + fileB: toBlob !== undefined ? lazyPromise(async () => URL.createObjectURL(await toBlob)) : null, + load: false, + }, + }; +} + +export type FileDetails = TextFileDetails | ImageFileDetails; + +export type ImageDiffDetails = { + fileA: LazyPromise | null; + fileB: LazyPromise | null; + load: boolean; +}; + +export function requireEitherImage(details: ImageDiffDetails) { + if (details.fileA) return details.fileA; + if (details.fileB) return details.fileB; + throw new Error("Neither image is available"); +} + // Sort such that when displayed as a file tree, directories come before files and each level is sorted by name function compareFileDetails(a: FileDetails, b: FileDetails): number { const aName = a.toFile; @@ -240,7 +285,14 @@ export function getFileStatusProps(status: FileStatus): FileStatusProps { } } -export function findHeaderChangeOnlyPatches(patchStrings: string[]) { +export function findHeaderChangeOnlyPatches(fileDetails: FileDetails[]) { + const patchStrings = fileDetails.map((details) => { + if (details.type === "text") { + return details.patchText; + } + return undefined; + }); + const result: boolean[] = []; for (let i = 0; i < patchStrings.length; i++) { @@ -272,18 +324,6 @@ export function findHeaderChangeOnlyPatches(patchStrings: string[]) { return result; } -export type ImageDiffDetails = { - fileA: LazyPromise | null; - fileB: LazyPromise | null; - load: boolean; -}; - -export function requireEitherImage(details: ImageDiffDetails) { - if (details.fileA) return details.fileA; - if (details.fileB) return details.fileB; - throw new Error("Neither image is available"); -} - export type ViewerStatistics = { addedLines: number; removedLines: number; @@ -291,12 +331,18 @@ export type ViewerStatistics = { fileRemovedLines: number[]; }; -export type DiffMetadata = { - type: "file" | "github"; - fileName?: string; - githubDetails?: GithubDiff; +export type GithubDiffMetadata = { + type: "github"; + details: GithubDiff; }; +export type FileDiffMetadata = { + type: "file"; + fileName: string; +}; + +export type DiffMetadata = GithubDiffMetadata | FileDiffMetadata; + export class MultiFileDiffViewerState { private static readonly context = new Context("MultiFileDiffViewerState"); @@ -313,10 +359,7 @@ export class MultiFileDiffViewerState { collapsed: boolean[] = $state([]); checked: boolean[] = $state([]); fileDetails: FileDetails[] = $state([]); - diffText: string[] = $state([]); - diffs: Promise[] = $state([]); diffViewCache: Map = new Map(); - images: ImageDiffDetails[] = $state([]); vlist: VList | undefined = $state(); tree: TreeState | undefined = $state(); activeSearchResult: ActiveSearchResult | null = $state(null); @@ -330,7 +373,7 @@ export class MultiFileDiffViewerState { readonly filteredFileDetails: FileDetails[] = $derived( this.fileTreeFilterDebounced.current ? this.fileDetails.filter((f) => this.filterFile(f)) : this.fileDetails, ); - readonly patchHeaderDiffOnly: boolean[] = $derived(findHeaderChangeOnlyPatches(this.diffText)); + readonly patchHeaderDiffOnly: boolean[] = $derived(findHeaderChangeOnlyPatches(this.fileDetails)); readonly searchResults: Promise = $derived(this.findSearchResults()); private constructor() { @@ -421,104 +464,41 @@ export class MultiFileDiffViewerState { } clearImages() { - for (let i = 0; i < this.images.length; i++) { - const image = this.images[i]; - if (image !== null && image !== undefined) { - image.load = false; - const fileA = image.fileA; - if (fileA?.hasValue()) { - (async () => { - const a = await fileA.getValue(); - URL.revokeObjectURL(a); - })(); - } - const fileB = image.fileB; - if (fileB?.hasValue()) { - (async () => { - const b = await fileB.getValue(); - URL.revokeObjectURL(b); - })(); - } + for (let i = 0; i < this.fileDetails.length; i++) { + const details = this.fileDetails[i]; + if (details.type !== "image") { + continue; + } + const image = details.image; + image.load = false; + const fileA = image.fileA; + if (fileA?.hasValue()) { + (async () => { + const a = await fileA.getValue(); + URL.revokeObjectURL(a); + })(); + } + const fileB = image.fileB; + if (fileB?.hasValue()) { + (async () => { + const b = await fileB.getValue(); + URL.revokeObjectURL(b); + })(); } } - this.images = []; } - loadPatches(patches: FileDetails[], meta: { githubDetails?: GithubDiff; fileName?: string }) { + loadPatches(patches: FileDetails[], meta: DiffMetadata | null) { // Reset state this.collapsed = []; this.checked = []; this.fileDetails = []; - this.diffText = []; - this.diffs = []; this.clearImages(); this.vlist?.scrollToIndex(0, { align: "start" }); - - if (meta.fileName) { - this.diffMetadata = { type: "file", fileName: meta.fileName }; - } else if (meta.githubDetails) { - this.diffMetadata = { - type: "github", - githubDetails: meta.githubDetails, - }; - } else { - this.diffMetadata = null; - } + this.diffMetadata = meta; patches.sort(compareFileDetails); - // Load new state - for (let i = 0; i < patches.length; i++) { - const patch = patches[i]; - - const isImageDiff = isImageFile(patch.fromFile) && isImageFile(patch.toFile); - if (isImageDiff && meta.githubDetails) { - const githubDetailsCopy = meta.githubDetails; - - let fileA: LazyPromise | null = null; - if (patch.status !== "added") { - fileA = lazyPromise(async () => - URL.createObjectURL( - await fetchGithubFile(getGithubToken(), githubDetailsCopy.owner, githubDetailsCopy.repo, patch.fromFile, githubDetailsCopy.base), - ), - ); - } - - let fileB: LazyPromise | null = null; - if (patch.status !== "removed") { - fileB = lazyPromise(async () => - URL.createObjectURL( - await fetchGithubFile(getGithubToken(), githubDetailsCopy.owner, githubDetailsCopy.repo, patch.toFile, githubDetailsCopy.head), - ), - ); - } - - this.images[i] = { fileA, fileB, load: false }; - continue; - } - const fromBlob = patch.fromBlob; - const toBlob = patch.toBlob; - if (isImageDiff && fromBlob && toBlob) { - let fileA: LazyPromise | null = null; - if (patch.status !== "added") { - fileA = lazyPromise(async () => URL.createObjectURL(fromBlob)); - } - - let fileB: LazyPromise | null = null; - if (patch.status !== "removed") { - fileB = lazyPromise(async () => URL.createObjectURL(toBlob)); - } - - this.images[i] = { fileA, fileB, load: false }; - continue; - } - - this.diffText[i] = patch.content; - this.diffs[i] = (async () => { - return parseSinglePatch(patch.content); - })(); - } - // Set this last since it's what the VList loads this.fileDetails.push(...patches); } @@ -532,11 +512,11 @@ export class MultiFileDiffViewerState { try { if (type === "commit") { const { info, files } = await fetchGithubCommitDiff(token, owner, repo, id.split("/")[0]); - this.loadPatches(files, { githubDetails: info }); + this.loadPatches(files, { type: "github", details: info }); return true; } else if (type === "pull") { const { info, files } = await fetchGithubPRComparison(token, owner, repo, id.split("/")[0]); - this.loadPatches(files, { githubDetails: info }); + this.loadPatches(files, { type: "github", details: info }); return true; } else if (type === "compare") { let refs = id.split("..."); @@ -550,7 +530,7 @@ export class MultiFileDiffViewerState { const base = refs[0]; const head = refs[1]; const { info, files } = await fetchGithubComparison(token, owner, repo, base, head); - this.loadPatches(files, { githubDetails: info }); + this.loadPatches(files, { type: "github", details: info }); return true; } } catch (error) { @@ -570,10 +550,11 @@ export class MultiFileDiffViewerState { const fileRemovedLines: number[] = []; for (let i = 0; i < this.fileDetails.length; i++) { - const diff = await this.diffs[i]; - if (!diff) { + const details = this.fileDetails[i]; + if (details.type !== "text") { continue; } + const diff = await details.structuredPatch; for (let j = 0; j < diff.hunks.length; j++) { const hunk = diff.hunks[j]; @@ -602,7 +583,13 @@ export class MultiFileDiffViewerState { } query = query.toLowerCase(); - const diffs = await Promise.all(this.diffs); + const diffPromises = this.fileDetails.map((details) => { + if (details.type !== "text") { + return undefined; + } + return details.structuredPatch; + }); + const diffs = await Promise.all(diffPromises); let total = 0; const lines: Map = new Map(); diff --git a/web/src/lib/github.svelte.ts b/web/src/lib/github.svelte.ts index 4cf4af3..b2fab2e 100644 --- a/web/src/lib/github.svelte.ts +++ b/web/src/lib/github.svelte.ts @@ -1,7 +1,7 @@ import { browser } from "$app/environment"; import type { components } from "@octokit/openapi-types"; import { splitMultiFilePatch, trimCommitHash } from "$lib/util"; -import type { FileDetails } from "$lib/diff-viewer-multi-file.svelte"; +import { type FileDetails, makeImageDetails } from "$lib/diff-viewer-multi-file.svelte"; import { PUBLIC_GITHUB_APP_NAME, PUBLIC_GITHUB_CLIENT_ID } from "$env/static/public"; export const GITHUB_USERNAME_KEY = "github_username"; @@ -139,6 +139,19 @@ export async function fetchGithubPRInfo(token: string | null, owner: string, rep } } +function splitMultiFilePatchGithub(details: GithubDiff, patch: string) { + return splitMultiFilePatch(patch, (from, to, status) => { + const token = getGithubToken(); + return makeImageDetails( + from, + to, + status, + status != "added" ? fetchGithubFile(token, details.owner, details.repo, from, details.base) : undefined, + status != "removed" ? fetchGithubFile(token, details.owner, details.repo, to, details.head) : undefined, + ); + }); +} + export async function fetchGithubComparison( token: string | null, owner: string, @@ -162,7 +175,8 @@ export async function fetchGithubComparison( if (!url) { url = `https://github.com/${owner}/${repo}/compare/${base}...${head}`; } - return { files: splitMultiFilePatch(await response.text()), info: { owner, repo, base, head, description, backlink: url } }; + const info = { owner, repo, base, head, description, backlink: url }; + return { files: splitMultiFilePatchGithub(info, await response.text()), info }; } else { throw Error(`Failed to retrieve comparison (${response.status}): ${await response.text()}`); } @@ -191,9 +205,10 @@ export async function fetchGithubCommitDiff(token: string | null, owner: string, const meta: GithubCommitDetails = await metaResponse.json(); const firstParent = meta.parents[0].sha; const description = `${meta.commit.message.split("\n")[0]} (${trimCommitHash(commit)})`; + const info = { owner, repo, base: firstParent, head: commit, description, backlink: meta.html_url }; return { - files: splitMultiFilePatch(await response.text()), - info: { owner, repo, base: firstParent, head: commit, description, backlink: meta.html_url }, + files: splitMultiFilePatchGithub(info, await response.text()), + info, }; } else { throw Error(`Failed to retrieve commit diff (${response.status}): ${await response.text()}`); diff --git a/web/src/lib/util.ts b/web/src/lib/util.ts index 1f91001..ca69d97 100644 --- a/web/src/lib/util.ts +++ b/web/src/lib/util.ts @@ -1,4 +1,4 @@ -import type { FileDetails } from "./diff-viewer-multi-file.svelte"; +import { type FileDetails, type ImageFileDetails, makeTextDetails } from "./diff-viewer-multi-file.svelte"; import type { FileStatus } from "./github.svelte"; import type { TreeNode } from "$lib/components/tree/index.svelte"; import type { BundledLanguage, SpecialLanguage } from "shiki"; @@ -93,55 +93,82 @@ export function binaryFileDummyDetails(fromFile: string, toFile: string, status: fakeContent = `diff --git a/${fromFile} b/${toFile}\n--- a/${fromFile}\n+++ b/${toFile}\n@@ -1,1 +1,1 @@\n-Cannot show binary file\n+Cannot show binary file`; break; } - return { - content: fakeContent, - fromFile: fromFile, - toFile: toFile, - status, - }; + return makeTextDetails(fromFile, toFile, status, fakeContent); } const fileRegex = /diff --git a\/(\S+) b\/(\S+)\r?\n(?:.+\r?\n)*?(?=-- *\r?\n|diff --git|$)/g; -export function splitMultiFilePatch(patchContent: string): FileDetails[] { +function parseHeader( + patch: string, + fromFile: string, + toFile: string, +): { + status: FileStatus; + binary: boolean; +} { + let status: FileStatus = "modified"; + if (fromFile !== toFile) { + status = "renamed_modified"; + } + let binary = false; + let foundIndex = false; + + let lineStart = 0; + while (true) { + const lineEnd = patch.indexOf("\n", lineStart); + if (lineEnd === -1) { + break; // No more lines + } + const line = patch.substring(lineStart, lineEnd); + if (line.startsWith("similarity index 100%")) { + status = "renamed"; + if (isImageFile(fromFile) && isImageFile(toFile)) { + binary = true; // Treat renamed images as binary + } + } else if (line.startsWith("deleted file mode")) { + status = "removed"; + } else if (line.startsWith("new file mode")) { + status = "added"; + } else if (line.startsWith("index ")) { + foundIndex = true; + } else if (foundIndex) { + if (line.startsWith("Binary files")) { + binary = true; + } + // end of header + break; + } + lineStart = lineEnd + 1; + } + + return { status, binary }; +} + +export function splitMultiFilePatch( + patchContent: string, + imageFactory?: (fromFile: string, toFile: string, status: FileStatus) => ImageFileDetails | null, +): FileDetails[] { const patches: FileDetails[] = []; // Process each file in the diff let fileMatch; while ((fileMatch = fileRegex.exec(patchContent)) !== null) { const [fullFileMatch, fromFile, toFile] = fileMatch; - - let status: FileStatus = "modified"; - - const newlineIndex = fullFileMatch.indexOf("\n"); - if (newlineIndex !== -1) { - const secondNewlineIndex = fullFileMatch.indexOf("\n", newlineIndex + 1); - if (secondNewlineIndex !== -1) { - const line2 = fullFileMatch.substring(newlineIndex + 1, secondNewlineIndex); - - if (fromFile !== toFile) { - if (line2 === "similarity index 100%") { - status = "renamed"; - } else { - status = "renamed_modified"; - } - } else if (line2.match(/^deleted file mode/)) { - status = "removed"; - } else if (line2.match(/^new file mode/)) { - status = "added"; - } - - const thirdNewlineIndex = fullFileMatch.indexOf("\n", secondNewlineIndex + 1); - if (thirdNewlineIndex !== -1) { - const line3 = fullFileMatch.substring(secondNewlineIndex + 1, thirdNewlineIndex); - if (line3.match(/^Binary/)) { - patches.push(binaryFileDummyDetails(fromFile, toFile, status)); - continue; - } + const { status, binary } = parseHeader(fullFileMatch, fromFile, toFile); + + if (binary) { + if (imageFactory !== undefined && isImageFile(fromFile) && isImageFile(toFile)) { + const imageDetails = imageFactory(fromFile, toFile, status); + if (imageDetails != null) { + patches.push(imageDetails); + continue; } + } else { + patches.push(binaryFileDummyDetails(fromFile, toFile, status)); + continue; } } - patches.push({ content: fullFileMatch, fromFile: fromFile, toFile: toFile, status }); + patches.push(makeTextDetails(fromFile, toFile, status, fullFileMatch)); } return patches; } diff --git a/web/src/routes/+page.svelte b/web/src/routes/+page.svelte index decdd30..df13691 100644 --- a/web/src/routes/+page.svelte +++ b/web/src/routes/+page.svelte @@ -92,9 +92,9 @@ function getPageTitle() { if (viewer.diffMetadata) { const meta = viewer.diffMetadata; - if (meta.type === "github" && meta.githubDetails) { - return `${meta.githubDetails.description} - GitHub/${meta.githubDetails.owner}/${meta.githubDetails.repo} - diffs.dev`; - } else if (meta.type === "file" && meta.fileName) { + if (meta.type === "github") { + return `${meta.details.description} - GitHub/${meta.details.owner}/${meta.details.repo} - diffs.dev`; + } else if (meta.type === "file") { return `${meta.fileName} - diffs.dev`; } } @@ -293,13 +293,10 @@
i} bind:this={viewer.vlist} overscan={3}> {#snippet children(value, index)} - {@const lines = viewer.diffText[index] !== undefined ? viewer.diffText[index] : null} - {@const image = viewer.images[index] !== undefined ? viewer.images[index] : null} - {@const patch = viewer.diffs[index]} -
- - {#if !viewer.collapsed[index] && image !== null} + + {#if !viewer.collapsed[index] && value.type === "image"} + {@const image = value.image}
{#if image.load} {#if image.fileA !== null && image.fileB !== null} @@ -328,10 +325,10 @@ {/if}
{/if} - {#if !viewer.collapsed[index] && lines !== null && (!viewer.patchHeaderDiffOnly[index] || !globalOptions.omitPatchHeaderOnlyHunks)} + {#if !viewer.collapsed[index] && value.type === "text" && (!viewer.patchHeaderDiffOnly[index] || !globalOptions.omitPatchHeaderOnlyHunks)}
{/snippet} -{#if meta.type === "github" && meta.githubDetails} - {@render github(meta.githubDetails)} +{#if meta.type === "github"} + {@render github(meta.details)} {/if} -{#if meta.type === "file" && meta.fileName} +{#if meta.type === "file"} {@render file(meta.fileName)} {/if} diff --git a/web/src/routes/FileHeader.svelte b/web/src/routes/FileHeader.svelte index 382844b..3c23518 100644 --- a/web/src/routes/FileHeader.svelte +++ b/web/src/routes/FileHeader.svelte @@ -8,12 +8,11 @@ interface Props { index: number; value: FileDetails; - isImage: boolean; } const viewer = MultiFileDiffViewerState.get(); const globalOptions = GlobalOptions.get(); - let { index, value, isImage }: Props = $props(); + let { index, value }: Props = $props(); let popoverOpen = $state(false); @@ -97,8 +96,7 @@ onclick={() => viewer.scrollToFile(index, { autoExpand: false, smooth: true })} onkeyup={(event) => event.key === "Enter" && viewer.scrollToFile(index, { autoExpand: false, smooth: true })} > - - {#if viewer.diffs[index] !== undefined} + {#if value.type === "text"} {#await viewer.stats} {:then stats} @@ -111,7 +109,7 @@ Patch-header-only diff {/if} {@render actionsPopover()} - {#if !viewer.patchHeaderDiffOnly[index] || !globalOptions.omitPatchHeaderOnlyHunks || isImage} + {#if !viewer.patchHeaderDiffOnly[index] || !globalOptions.omitPatchHeaderOnlyHunks || value.type === "image"} {@render collapseToggle()} {/if}
diff --git a/web/src/routes/OpenDiffDialog.svelte b/web/src/routes/OpenDiffDialog.svelte index efc43de..f37d0a2 100644 --- a/web/src/routes/OpenDiffDialog.svelte +++ b/web/src/routes/OpenDiffDialog.svelte @@ -4,7 +4,7 @@ import InfoPopup from "$lib/components/InfoPopup.svelte"; import { page } from "$app/state"; import { goto } from "$app/navigation"; - import { type FileDetails, MultiFileDiffViewerState } from "$lib/diff-viewer-multi-file.svelte"; + import { type FileDetails, makeImageDetails, makeTextDetails, MultiFileDiffViewerState } from "$lib/diff-viewer-multi-file.svelte"; import { binaryFileDummyDetails, bytesEqual, isBinaryFile, isImageFile, splitMultiFilePatch } from "$lib/util"; import { onMount } from "svelte"; import { createTwoFilesPatch } from "diff"; @@ -96,14 +96,9 @@ status = "renamed_modified"; } - fileDetails.push({ - content: "", - fromFile: fileA.metadata.name, - toFile: fileB.metadata.name, - fromBlob: blobA, - toBlob: blobB, - status, - }); + const img = makeImageDetails(fileA.metadata.name, fileB.metadata.name, status, blobA, blobB); + img.image.load = true; // load images by default when comparing two files directly + fileDetails.push(img); } else { const [textA, textB] = await Promise.all([blobA.text(), blobB.text()]); if (textA === textB) { @@ -117,15 +112,10 @@ status = "renamed_modified"; } - fileDetails.push({ - content: diff, - fromFile: fileA.metadata.name, - toFile: fileB.metadata.name, - status, - }); + fileDetails.push(makeTextDetails(fileA.metadata.name, fileB.metadata.name, status, diff)); } - viewer.loadPatches(fileDetails, { fileName: `${fileA.metadata.name}...${fileB.metadata.name}.patch` }); + viewer.loadPatches(fileDetails, { type: "file", fileName: `${fileA.metadata.name}...${fileB.metadata.name}.patch` }); await updateUrlParams(); modalOpen = false; } @@ -164,14 +154,7 @@ continue; } if (isImageFile(entry.file.name) && isImageFile(entryB.file.name)) { - fileDetails.push({ - content: "", - fromFile: entry.path, - toFile: entryB.path, - fromBlob: entry.file, - toBlob: entryB.file, - status: "modified", - }); + fileDetails.push(makeImageDetails(entry.path, entryB.path, "modified", entry.file, entryB.file)); } else { fileDetails.push(binaryFileDummyDetails(entry.path, entryB.path, "modified")); } @@ -181,34 +164,17 @@ // Files are identical continue; } - fileDetails.push({ - content: createTwoFilesPatch(entry.path, entryB.path, textA, textB), - fromFile: entry.path, - toFile: entryB.path, - status: "modified", - }); + fileDetails.push(makeTextDetails(entry.path, entryB.path, "modified", createTwoFilesPatch(entry.path, entryB.path, textA, textB))); } } else if (isImageFile(entry.file.name)) { // Image file removed - fileDetails.push({ - content: "", - fromFile: entry.path, - toFile: entry.path, - fromBlob: entry.file, - toBlob: entry.file, - status: "removed", - }); + fileDetails.push(makeImageDetails(entry.path, entry.path, "removed", entry.file, entry.file)); } else if (await isBinaryFile(entry.file)) { // Binary file removed fileDetails.push(binaryFileDummyDetails(entry.path, entry.path, "removed")); } else { // Text file removed - fileDetails.push({ - content: createTwoFilesPatch(entry.path, "", await entry.file.text(), ""), - fromFile: entry.path, - toFile: entry.path, - status: "removed", - }); + fileDetails.push(makeTextDetails(entry.path, entry.path, "removed", createTwoFilesPatch(entry.path, "", await entry.file.text(), ""))); } } @@ -217,28 +183,16 @@ const entryA = entriesAMap.get(entry.path); if (!entryA) { if (isImageFile(entry.file.name)) { - fileDetails.push({ - content: "", - fromFile: entry.path, - toFile: entry.path, - fromBlob: entry.file, - toBlob: entry.file, - status: "added", - }); + fileDetails.push(makeImageDetails(entry.path, entry.path, "added", entry.file, entry.file)); } else if (await isBinaryFile(entry.file)) { fileDetails.push(binaryFileDummyDetails(entry.path, entry.path, "added")); } else { - fileDetails.push({ - content: createTwoFilesPatch("", entry.path, "", await entry.file.text()), - fromFile: entry.path, - toFile: entry.path, - status: "added", - }); + fileDetails.push(makeTextDetails(entry.path, entry.path, "added", createTwoFilesPatch("", entry.path, "", await entry.file.text()))); } } } - viewer.loadPatches(fileDetails, { fileName: `${dirA.fileName}...${dirB.fileName}.patch` }); + viewer.loadPatches(fileDetails, { type: "file", fileName: `${dirA.fileName}...${dirB.fileName}.patch` }); await updateUrlParams(); modalOpen = false; } @@ -292,7 +246,7 @@ return; } modalOpen = false; - viewer.loadPatches(files, { fileName: patchFile.metadata.name }); + viewer.loadPatches(files, { type: "file", fileName: patchFile.metadata.name }); await updateUrlParams(); }