Skip to content

Consolidate diff viewer state, various cleanup around that change #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jpenilla merged 1 commit into from
Jun 28, 2025
Merged

Consolidate diff viewer state, various cleanup around that change #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 21 additions & 35 deletions web/src/lib/components/diff/ConciseDiffView.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import {
type ConciseDiffViewProps,
ConciseDiffViewState,
getBaseColors,
innerPatchLineTypeProps,
type InnerPatchLineTypeProps,
makeSearchSegments,
Expand Down Expand Up @@ -54,20 +53,6 @@
cacheKey: box.with(() => cacheKey),
});

let baseColors: Promise<string> = $state(new Promise<string>(() => []));
$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 "...";
Expand Down Expand Up @@ -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<number> = { 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;
Expand All @@ -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];

Expand Down Expand Up @@ -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}
<div class="relative">
{@render lineContent(line, lineType, innerLineType)}
<span class="pointer-events-none absolute top-0 left-0 text-transparent select-none">
<span class="inline leading-[0.875rem]">
{#each lineSegments as segment, index (index)}
{#if segment.highlighted}<span
bind:this={searchResultElements[segment.id ?? -1]}
{#each lineSearchSegments as searchSegment, index (index)}
{#if searchSegment.highlighted}<span
bind:this={searchResultElements[searchSegment.id ?? -1]}
class={{
"bg-[#d4a72c66]": segment.id !== activeSearchResult,
"bg-[#ff9632]": segment.id === activeSearchResult,
"text-em-high-light": segment.id === activeSearchResult,
"bg-[#d4a72c66]": searchSegment.id !== activeSearchResult,
"bg-[#ff9632]": searchSegment.id === activeSearchResult,
"text-em-high-light": searchSegment.id === activeSearchResult,
}}
data-match-id={segment.id}>{segment.text}</span
>{:else}{segment.text}{/if}
data-match-id={searchSegment.id}>{searchSegment.text}</span
>{:else}{searchSegment.text}{/if}
{/each}
</span>
</span>
Expand All @@ -214,16 +200,16 @@
</div>
{/snippet}

{#await Promise.all([baseColors, view.patchLines])}
{#await Promise.all([view.rootStyle, view.diffViewerPatch])}
<div class="flex items-center justify-center bg-neutral-2 p-4"><Spinner /></div>
{:then [baseColors, lines]}
{:then [rootStyle, diffViewerPatch]}
<div
style={baseColors}
style={rootStyle}
class="diff-content text-patch-line w-full bg-[var(--editor-bg)] font-mono text-xs leading-[1.25rem] text-[var(--editor-fg)] selection:bg-[var(--select-bg)]"
data-wrap={lineWrap}
>
{#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}
Expand Down
91 changes: 56 additions & 35 deletions web/src/lib/components/diff/concise-diff-view.svelte.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,6 +53,12 @@ export enum PatchLineType {
SPACER,
}

export enum InnerPatchLineType {
ADD,
REMOVE,
NONE,
}

export type PatchLineTypeProps = {
classes: string;
lineNoClasses: string;
Expand Down Expand Up @@ -69,12 +93,6 @@ export const patchLineTypeProps: Record<PatchLineType, PatchLineTypeProps> = {
},
};

export enum InnerPatchLineType {
ADD,
REMOVE,
NONE,
}

export type InnerPatchLineTypeProps = {
style: string;
};
Expand All @@ -99,15 +117,6 @@ export const innerPatchLineTypeProps: Record<InnerPatchLineType, InnerPatchLineT
},
};

export type PatchLine = {
type: PatchLineType;
content: LineSegment[];
lineBreak?: boolean;
innerPatchLineType: InnerPatchLineType;
oldLineNo?: number;
newLineNo?: number;
};

const joiner = "\uE000";
const noTrailingNewlineMarker: string = joiner + joiner + ["PATCH", "ROULETTE", "NO", "TRAILING", "NEWLINE", "MARKER"].join(joiner) + joiner + joiner;

Expand Down Expand Up @@ -613,40 +622,39 @@ async function withLineProcessor<R>(fn: (proc: LineProcessor) => Promise<R>): Pr
}
}

export async function makeLines(
export async function parseDiffViewerPatch(
patchPromise: StructuredPatch | Promise<StructuredPatch>,
syntaxHighlighting: boolean,
syntaxHighlightingTheme: BundledTheme | undefined,
omitPatchHeaderOnlyHunks: boolean,
wordDiffs: boolean,
): Promise<PatchLine[][]> {
): Promise<DiffViewerPatch> {
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<PatchLine[]> {
const lines: PatchLine[] = [];

): Promise<DiffViewerPatchHunk> {
// 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({
Expand All @@ -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[]) {
Expand Down Expand Up @@ -971,14 +979,14 @@ async function getTheme(theme: BundledTheme | undefined): Promise<null | { defau
}

export class ConciseDiffViewCachedState {
patchLines: Promise<PatchLine[][]>;
diffViewerPatch: Promise<DiffViewerPatch>;
syntaxHighlighting: boolean;
syntaxHighlightingTheme: BundledTheme | undefined;
omitPatchHeaderOnlyHunks: boolean;
wordDiffs: boolean;

constructor(patchLines: Promise<PatchLine[][]>, props: ConciseDiffViewStateProps<unknown>) {
this.patchLines = patchLines;
constructor(diffViewerPatch: Promise<DiffViewerPatch>, props: ConciseDiffViewStateProps<unknown>) {
this.diffViewerPatch = diffViewerPatch;
this.syntaxHighlighting = props.syntaxHighlighting.current;
this.syntaxHighlightingTheme = props.syntaxHighlightingTheme.current;
this.omitPatchHeaderOnlyHunks = props.omitPatchHeaderOnlyHunks.current;
Expand Down Expand Up @@ -1034,8 +1042,9 @@ export type ConciseDiffViewStateProps<K> = ReadableBoxedValues<{
}>;

export class ConciseDiffViewState<K> {
patchLines: Promise<PatchLine[][]> = $state(new Promise<PatchLine[][]>(() => []));
diffViewerPatch: Promise<DiffViewerPatch> = $state(new Promise<DiffViewerPatch>(() => []));
cachedState: ConciseDiffViewCachedState | undefined = undefined;
rootStyle: Promise<string> = $state(new Promise<string>(() => []));

private readonly props: ConciseDiffViewStateProps<K>;

Expand All @@ -1046,6 +1055,18 @@ export class ConciseDiffViewState<K> {
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);
Expand All @@ -1068,7 +1089,7 @@ export class ConciseDiffViewState<K> {
}
}

const promise = makeLines(
const promise = parseDiffViewerPatch(
this.props.patch.current,
this.props.syntaxHighlighting.current,
this.props.syntaxHighlightingTheme.current,
Expand All @@ -1079,17 +1100,17 @@ export class ConciseDiffViewState<K> {
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;
}
}
Expand Down
Loading