Consolidate diff viewer state, various cleanup around that change

Author: jpenilla

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<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 "...";
@@ -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;
@@ -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}
  <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>
@@ -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}

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<PatchLineType, PatchLineTypeProps> = {
  },
 };
 
-export enum InnerPatchLineType {
- ADD,
- REMOVE,
- NONE,
-}
-
 export type InnerPatchLineTypeProps = {
  style: string;
 };
@@ -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;
 
@@ -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({
@@ -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<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;
@@ -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>;
 
@@ -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);
@@ -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,
@@ -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;
  }
 }