From cc78e8fe2bebc436cb75d5c284e2e587788a0f2f Mon Sep 17 00:00:00 2001 From: clemg Date: Tue, 16 Jun 2026 10:30:37 +0200 Subject: [PATCH 1/5] perf(diffs): store parsed diff lines as a per-file UTF-8 byte arena `additionLines`/`deletionLines` change from `string[]` to `DiffLines`: a plain data object holding a file's lines as one UTF-8 byte arena plus an offset table, decoded on demand via `lineAt` / `joinLines`. On a huge diff (linux v6..v7, ~22.8M lines across ~77k files) this avoids tens of millions of tiny `String` objects, so the V8 heap drops ~33% on that compare and the parser is faster: it no longer encode+decode-detaches every line, it encodes once on seal and decodes only the visible (virtualized) lines. It is plain data on purpose, so it survives structured clone (the highlight worker), `structuredClone`, and IndexedDB without a revive step (no class, no prototype to drop). `.length` stays a field, so the many `.length` consumers are unchanged; only content reads migrate (`x[i]` -> `lineAt(x, i)`, `x.join('')` -> `joinLines(x)`). Per-file offsets use the smallest int width that fits the file. A file with a lone surrogate keeps exact strings as a fallback, and merge-conflict diffs keep plain strings (no encode) so their parse stays at parity. The parsed model is byte-identical to before (snapshot + content-hash). The editor's realtime-update path edits addition lines in place (DiffHunksRenderer.updateRenderCache / updateDiffHunks), so diffLines also exposes mutableLines (decode an arena side into the plain form once, on the first edit) and joinLineRange (read a hunk's line range as one contiguous byte slice for the partial reparse). Whole-document changes reassign the side with plainLines(splitFileContents(...)), keeping the editor on plain strings while it is editing. --- packages/diffs/src/editor/editor.ts | 3 +- packages/diffs/src/index.ts | 1 + .../diffs/src/renderers/DiffHunksRenderer.ts | 54 +++- packages/diffs/src/types.ts | 26 +- .../diffs/src/utils/cloneFileDiffMetadata.ts | 5 +- packages/diffs/src/utils/diffLines.ts | 264 ++++++++++++++++++ packages/diffs/src/utils/editSessionHunks.ts | 37 ++- .../diffs/src/utils/hydratePartialDiff.ts | 11 +- .../utils/parseMergeConflictDiffFromFile.ts | 5 +- packages/diffs/src/utils/parsePatchFiles.ts | 101 ++++--- .../diffs/src/utils/realignChangeContent.ts | 18 +- .../src/utils/renderDiffWithHighlighter.ts | 9 +- packages/diffs/src/utils/resolveRegion.ts | 63 +++-- packages/diffs/src/utils/updateDiffHunks.ts | 62 ++-- .../test/CodeView.partialHydration.test.ts | 5 +- .../test/DiffHunksRendererRecompute.test.ts | 30 +- .../test/FileDiff.partialHydration.test.ts | 22 +- .../diffs/test/FileDiff.partialRender.test.ts | 15 +- .../FileDiff.unifiedEditSeparators.test.ts | 5 +- ...rtualizedFileDiff.partialHydration.test.ts | 25 +- packages/diffs/test/annotations.test.ts | 5 +- .../diffs/test/diffAcceptRejectHunk.test.ts | 47 ++-- packages/diffs/test/editSessionHunks.test.ts | 139 +++++---- .../diffs/test/hydratePartialDiff.test.ts | 21 +- packages/diffs/test/hydration.test.ts | 5 +- packages/diffs/test/iterateOverDiff.test.ts | 9 +- packages/diffs/test/onPostRenderPhase.test.ts | 5 +- packages/diffs/test/parseDiffFromFile.test.ts | 13 +- .../parseMergeConflictDiffFromFile.test.ts | 9 +- packages/diffs/test/parsePatchFiles.test.ts | 17 +- .../diffs/test/realignChangeContent.test.ts | 3 +- packages/diffs/test/testUtils.ts | 11 + packages/diffs/test/updateDiffHunks.test.ts | 43 +-- packages/diffs/test/virtualDiffLayout.test.ts | 7 +- ...irtualizedFileDiffEstimatedHeights.test.ts | 13 +- 35 files changed, 803 insertions(+), 305 deletions(-) create mode 100644 packages/diffs/src/utils/diffLines.ts diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index 64c2b4105..e6812e458 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -20,6 +20,7 @@ import type { SelectionSide, TextEdit, } from '../types'; +import { joinLines } from '../utils/diffLines'; import { getFiletypeFromFileName } from '../utils/getFiletypeFromFileName'; import { isGutterUtilityPath } from '../utils/isGutterUtilityPath'; import { @@ -905,7 +906,7 @@ export class Editor implements DiffsEditor { if ('contents' in fileOrDiff) { contents = fileOrDiff.contents; } else { - contents = fileOrDiff.additionLines.join(''); + contents = joinLines(fileOrDiff.additionLines); } const editStack = new EditStack({ maxEntries: this.#options.historyMaxEntries, diff --git a/packages/diffs/src/index.ts b/packages/diffs/src/index.ts index ba01e359d..3901bc0de 100644 --- a/packages/diffs/src/index.ts +++ b/packages/diffs/src/index.ts @@ -78,6 +78,7 @@ export * from './utils/createWindowFromScrollPosition'; export * from './utils/cssWrappers'; export * from './utils/detachString'; export * from './utils/diffAcceptRejectHunk'; +export * from './utils/diffLines'; export * from './utils/formatCSSVariablePrefix'; export * from './utils/getFiletypeFromFileName'; export * from './utils/getHighlighterOptions'; diff --git a/packages/diffs/src/renderers/DiffHunksRenderer.ts b/packages/diffs/src/renderers/DiffHunksRenderer.ts index 82dd77588..d091f6fd8 100644 --- a/packages/diffs/src/renderers/DiffHunksRenderer.ts +++ b/packages/diffs/src/renderers/DiffHunksRenderer.ts @@ -51,6 +51,13 @@ import { createFileHeaderElement } from '../utils/createFileHeaderElement'; import { createNoNewlineElement } from '../utils/createNoNewlineElement'; import { createPreElement } from '../utils/createPreElement'; import { createSeparator } from '../utils/createSeparator'; +import { + type DiffLines, + joinLines, + lineAt, + mutableLines, + plainLines, +} from '../utils/diffLines'; import { applySessionChangedLines, rebuildSessionHunks, @@ -471,18 +478,25 @@ export class DiffHunksRenderer { const hastLines = result.code.additionLines; const changedAdditionLines: number[] = []; const previousAdditionLines = new Map(); + // Line edits land in the plain-string form of the side; an arena side is + // decoded once on the first edit and stays plain from then on + const additionLines = mutableLines(diff.additionLines); + diff.additionLines = additionLines; for (const [line, tokens] of dirtyLines) { const prev = hastLines[line] as HASTElement | undefined; const prevProps = prev?.properties ?? {}; const lineText = tokens.map((a) => a[2]).join(''); - const canSyncDiffLine = line < diff.additionLines.length; - const prevLine = canSyncDiffLine ? (diff.additionLines[line] ?? '') : ''; + const canSyncDiffLine = line < additionLines.length; + const prevLine = canSyncDiffLine ? (additionLines.lines[line] ?? '') : ''; const prevText = cleanLastNewline(prevLine); // The host text document can expose one extra trailing empty line when // the file ends with a newline. Deferred tokenization must not grow // additionLines from that mismatch or hunk trailing context desyncs. if (canSyncDiffLine) { - diff.additionLines[line] = applyLineTextWithNewline(prevLine, lineText); + additionLines.lines[line] = applyLineTextWithNewline( + prevLine, + lineText + ); if (prevText !== lineText) { changedAdditionLines.push(line); previousAdditionLines.set(line, prevLine); @@ -533,7 +547,7 @@ export class DiffHunksRenderer { if (!lineCountChangeInFlight) { if ( diff.additionLines.length <= 1 && - diff.additionLines.join('') === '' + joinLines(diff.additionLines) === '' ) { Object.assign( diff, @@ -610,13 +624,16 @@ export class DiffHunksRenderer { // entire text. This preserves blank documents and the final editable empty // row after a trailing line break. const { additionLines: previousAdditionLines } = diff; - diff.additionLines = getEditorDocumentLines( + const nextAdditionLines = getEditorDocumentLines( textDocument, previousAdditionLines ); + // The editor stays on the plain-string form while it is editing; only a + // parse builds the byte arena + diff.additionLines = plainLines(nextAdditionLines); result.code.additionLines = realignAdditionHastLines( previousAdditionLines, - diff.additionLines, + nextAdditionLines, result.code.additionLines, textDocument ); @@ -624,7 +641,10 @@ export class DiffHunksRenderer { // to a diff with no editable rows and leave the attached host with no // line element for its caret (the additions column vanishes in split; // unified shows only deletions). Keep one empty editable line instead. - if (diff.additionLines.length <= 1 && diff.additionLines.join('') === '') { + if ( + diff.additionLines.length <= 1 && + joinLines(diff.additionLines) === '' + ) { Object.assign( diff, recomputeEmptyDocumentDiff(diff, this.options.parseDiffOptions) @@ -2197,21 +2217,28 @@ function withContentProperties( // line's stale tokens once they become visible. Entries outside the changed // window keep their highlighted content; entries inside it become plain-text // elements that the editor re-tokenizes on its next background pass. +// `previousLines` is the side as the last parse or edit left it, so it may +// still be in the byte-arena form; the prefix/suffix scans stop at the first +// difference, so reading them through `lineAt` decodes only a line or two +// rather than the whole side. function realignAdditionHastLines( - previousLines: string[], + previousLines: DiffLines, nextLines: string[], hastLines: ElementContent[], textDocument: DiffsTextDocument ): ElementContent[] { const maxShared = Math.min(previousLines.length, nextLines.length); let prefix = 0; - while (prefix < maxShared && previousLines[prefix] === nextLines[prefix]) { + while ( + prefix < maxShared && + lineAt(previousLines, prefix) === nextLines[prefix] + ) { prefix++; } let suffix = 0; while ( suffix < maxShared - prefix && - previousLines[previousLines.length - 1 - suffix] === + lineAt(previousLines, previousLines.length - 1 - suffix) === nextLines[nextLines.length - 1 - suffix] ) { suffix++; @@ -2268,7 +2295,7 @@ function createPlainAdditionLineElement( function getEditorDocumentLines( textDocument: DiffsTextDocument, - previousLines: string[] + previousLines: DiffLines ): string[] { const lines: string[] = []; const fallbackLineBreak = getFallbackLineBreak(previousLines); @@ -2287,8 +2314,9 @@ function hasLineBreakSuffix(line: string): boolean { return line.endsWith('\n') || line.endsWith('\r'); } -function getFallbackLineBreak(lines: string[]): string { - for (const line of lines) { +function getFallbackLineBreak(lines: DiffLines): string { + for (let index = 0; index < lines.length; index++) { + const line = lineAt(lines, index); if (line.endsWith('\r\n')) { return '\r\n'; } diff --git a/packages/diffs/src/types.ts b/packages/diffs/src/types.ts index 47cb5d959..c5f374e75 100644 --- a/packages/diffs/src/types.ts +++ b/packages/diffs/src/types.ts @@ -13,6 +13,8 @@ import type { ThemeRegistrationResolved, } from 'shiki'; +import type { DiffLines } from './utils/diffLines'; + export type { CreatePatchOptionsNonabortable }; export type CodeViewScrollBehavior = 'instant' | 'smooth' | 'smooth-auto'; @@ -335,21 +337,21 @@ export interface FileDiffMetadata { isPartial: boolean; /** - * Array of lines from previous version of the file. If `isPartial` is false, - * it means that `deletionLines` can be considered the entire contents of the - * old version of the file. Otherwise `deletionLines` will just be an array - * of all the content processed from the `context` and `deletion` lines of - * the patch. + * Lines from the previous version of the file. If `isPartial` is false, this + * can be considered the entire contents of the old version of the file. + * Otherwise it is all the content processed from the `context` and `deletion` + * lines of the patch. Read a line with `lineAt(deletionLines, i)` and the line + * count with `deletionLines.length` (see {@link DiffLines}). */ - deletionLines: string[]; + deletionLines: DiffLines; /** - * Array of lines from new version of the file. If `isPartial` is false, it - * means that `additionLines` can be considered the entire contents of the - * new version of the file. Otherwise `additionLines` will just be an array - * of all the content processed from the `context` and `addition` lines of - * the patch. + * Lines from the new version of the file. If `isPartial` is false, this can be + * considered the entire contents of the new version of the file. Otherwise it + * is all the content processed from the `context` and `addition` lines of the + * patch. Read a line with `lineAt(additionLines, i)` and the line count with + * `additionLines.length` (see {@link DiffLines}). */ - additionLines: string[]; + additionLines: DiffLines; /** * Set while an attached editor's session-scoped hunk updates have reshaped * `hunks` away from a plain recompute (regions stay frozen during an edit diff --git a/packages/diffs/src/utils/cloneFileDiffMetadata.ts b/packages/diffs/src/utils/cloneFileDiffMetadata.ts index e0040c2bd..af7984d1a 100644 --- a/packages/diffs/src/utils/cloneFileDiffMetadata.ts +++ b/packages/diffs/src/utils/cloneFileDiffMetadata.ts @@ -1,4 +1,5 @@ import type { FileDiffMetadata } from '../types'; +import { cloneLines } from './diffLines'; export function cloneFileDiffMetadata( fileDiff: FileDiffMetadata @@ -9,7 +10,7 @@ export function cloneFileDiffMetadata( ...hunk, hunkContent: hunk.hunkContent.map((content) => ({ ...content })), })), - deletionLines: [...fileDiff.deletionLines], - additionLines: [...fileDiff.additionLines], + deletionLines: cloneLines(fileDiff.deletionLines), + additionLines: cloneLines(fileDiff.additionLines), }; } diff --git a/packages/diffs/src/utils/diffLines.ts b/packages/diffs/src/utils/diffLines.ts new file mode 100644 index 000000000..f926d85ef --- /dev/null +++ b/packages/diffs/src/utils/diffLines.ts @@ -0,0 +1,264 @@ +// Compact storage for one side (additions or deletions) of a parsed file diff. +// +// Each side keeps its lines as either a UTF-8 byte arena or, for a few inputs, a +// plain string array. The arena concatenates every line's content (no separators) +// into one `Uint8Array` (`bytes`) and records a table of cumulative byte offsets +// (`offsets`) with one entry per line boundary. Line `i` is then the slice +// `bytes.subarray(offsets[i], offsets[i + 1])`, decoded on demand, so only +// the lines actually read (the virtualized visible ones) are turned back into +// strings. The bytes live off the V8 heap in the typed array's backing store, +// ASCII stays at one byte per char, and `offsets` uses the smallest int width +// that fits the side's byte length. +// +// A handful of inputs can't or shouldn't use the arena: a file with a lone +// surrogate (which `TextEncoder` would rewrite to U+FFFD), and sides that are +// built then mutated as string arrays (like merge-conflict resolution), where an +// arena encode would just be undone on the next edit. Those keep a plain `string[]` +// instead, so the two forms are an exclusive or type (see the type below). +// +// It is all plain data on purpose, so it survives `postMessage` / structured +// clone (the highlight worker), `structuredClone`, and IndexedDB with no revive +// step, since there is no prototype to drop. Read a line with `lineAt` (or the +// whole side with `joinLines`); build a `string[]` then seal it with `finishLines` + +export type DiffLines = { + /** Number of lines (useful for compatibility between the two forms) */ + length: number; +} & ( + | { + /** UTF-8 bytes of every line concatenated (so no separators) */ + bytes: Uint8Array; + /** + * Cumulative byte offsets with `length + 1` entries: line `i` is the slice + * `bytes.subarray(offsets[i], offsets[i + 1])`. The element width is the + * smallest that fits the file's byte length (most files are < 64KB, so + * `Uint16Array`), which roughly halves the table compared to using + * `Uint32Array` only + */ + offsets: Uint8Array | Uint16Array | Uint32Array; + } + | { + /** + * Plain-string form, used when the byte arena doesn't apply: the rare file + * that can't survive a UTF-8 round-trip (a lone surrogate code unit, which + * `TextEncoder` rewrites to U+FFFD), and sides built then mutated as string + * arrays (e.g. merge-conflict resolution) that aren't worth encoding + */ + lines: string[]; + } +); + +const encoder = new TextEncoder(); +// `ignoreBOM` is required: without it the decoder silently strips a leading +// U+FEFF, so a line that genuinly starts with a byte-order mark would lose it +// on read (the same reason `detachString` sets it) +const decoder = new TextDecoder('utf-8', { ignoreBOM: true }); + +const EMPTY_BYTES = new Uint8Array(0); +// A valid one-entry offset table (`offsets[0] === 0`) for empty/fallback lists +const EMPTY_OFFSETS = new Uint32Array(1); + +/** An empty, sealed `DiffLines`. Shared because it is read-only. */ +export const EMPTY_DIFF_LINES: DiffLines = { + length: 0, + bytes: EMPTY_BYTES, + offsets: EMPTY_OFFSETS, +}; + +/** + * Wrap an already-built `string[]` as a `DiffLines` without encoding it into the + * byte arena. For callers whose lines are small synthetic diffs (e.g. + * merge-conflict resolution): an arena encode would be undone on the next edit, + * so the plain form is better. Reads should still go through `lineAt`/`joinLines` + */ +export function plainLines(lines: string[]): DiffLines { + return { length: lines.length, lines }; +} + +/** + * Whether `text` contains a lone (unpaired) surrogate code unit, which cannot + * be represented in UTF-8 (`TextEncoder` rewrites it to U+FFFD). Valid surrogate + * pairs (emoji, CJKs, and other astral characters) are well-formed and are NOT flagged, + * so those files still get the compact byte arena. The parser checks a whole + * file once so `finishLines` can skip the per-line check on the common path + * + * See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/isWellFormed + * function stolen from: https://github.com/tc39/proposal-is-usv-string#algorithm + */ +export function isWellFormed(text: string): boolean { + return !/\p{Surrogate}/u.test(text); +} + +/** + * Typeguard to check if we have bytes or plain string. We could add a field on + * the type like `kind: 'bytes' | 'plain'`, but it would add ~4 bytes per object + */ +function isPlain(diff: DiffLines): diff is DiffLines & { lines: string[] } { + return 'lines' in diff; +} + +/** + * Line `index`, or `undefined` for an out-of-range index (matching the + * `string[]` semantics callers guard with `== null`) + */ +export function lineAt(diff: DiffLines, index: number): string { + if (index < 0 || index >= diff.length) { + // Not sure if I like that. We could return `string | undefined`, but we + // would have to check for correctness at runtime. + return undefined as unknown as string; + } + if (isPlain(diff)) { + return diff.lines[index]; + } + const { offsets } = diff; + return decoder.decode( + diff.bytes.subarray(offsets[index], offsets[index + 1]) + ); +} + +/** Concatenate every line, like `string[].join('')` */ +export function joinLines(diff: DiffLines): string { + return isPlain(diff) ? diff.lines.join('') : decoder.decode(diff.bytes); +} + +/** + * Concatenate the lines in `[start, end)`, like + * `string[].slice(start, end).join('')`. On the arena form the range is one + * contiguous byte slice, so it decodes in a single call + */ +export function joinLineRange( + diff: DiffLines, + start: number, + end: number +): string { + if (isPlain(diff)) { + return diff.lines.slice(start, end).join(''); + } + const from = Math.min(Math.max(start, 0), diff.length); + const to = Math.min(Math.max(end, from), diff.length); + return decoder.decode( + diff.bytes.subarray(diff.offsets[from], diff.offsets[to]) + ); +} + +/** + * The plain-string form of `lines`, for the editor paths that replace single + * lines in place. An arena side decodes into a fresh plain form once (the + * caller reassigns it); a side that is already plain comes back as-is. + * In-place edits must not change the line count, so `length` stays valid + */ +export function mutableLines(diff: DiffLines): DiffLines & { lines: string[] } { + if (isPlain(diff)) { + return diff; + } + return { length: diff.length, lines: linesToArray(diff) }; +} + +/** + * An independently mutable copy, for callers that used to deep-copy the side + * with `[...lines]`. The arena form's `bytes`/`offsets` are never written in + * place (an edit goes through `mutableLines`, which builds a new side), so the + * copy shares them and only the wrapper object is fresh; the plain form copies + * its `lines` array, which editors do write in place + */ +export function cloneLines(diff: DiffLines): DiffLines { + if (isPlain(diff)) { + return { length: diff.length, lines: diff.lines.slice() }; + } + return { length: diff.length, bytes: diff.bytes, offsets: diff.offsets }; +} + +/** Materialize a plain `string[]`, mainly for tests and interop */ +export function linesToArray(diff: DiffLines): string[] { + if (isPlain(diff)) { + return diff.lines.slice(); + } + const out = new Array(diff.length); + for (let index = 0; index < diff.length; index++) { + out[index] = lineAt(diff, index); + } + return out; +} + +// Using `lineScratch` here, because to seal a file we need a temporary byte +// buffer to `encodeInto` (we don't know the exact UTF-8 size upfront, so we +// over-allocate max `charTotal*3` = the theoritical max, fill it, then copy +// out exactly the bytes used). `lineScratch` reuses one such buffer across +// all the files instead of allocating a throwaway per file. The `required` / +// the grow-check size it to the biggest file, and `releaseLineScratch` shrinks +// it back afterward so one huge file doesn't pin a big buffer +const LINE_SCRATCH_INITIAL = 1024; +let lineScratch = new Uint8Array(LINE_SCRATCH_INITIAL); + +/** Drop the scratch buffer back to its initial size after a parse run */ +export function releaseLineScratch(): void { + if (lineScratch.length !== LINE_SCRATCH_INITIAL) { + lineScratch = new Uint8Array(LINE_SCRATCH_INITIAL); + } +} + +/** + * Seal a built-up `string[]` of line content into a `DiffLines`. It holds + * one byte arena per file instead of one `String` per line like before + * + * Set `knownLossless = true` when the caller has already checked the source + * text has no lone surrogate (the parser tests the whole file once, which is far + * cheaper than testing every line); otherwise each line is checked here and a + * file containing a lone surrogate falls back to keeping the exact strings. + * (I tried checking for isWellFormed here but it always adds about +8% of RAM + * usage because it needs to call it quite a few times) + */ +export function finishLines( + building: string[], + knownLossless = false +): DiffLines { + const count = building.length; + + let charTotal = 0; + for (let index = 0; index < count; index++) { + charTotal += building[index].length; + } + // Each UTF-16 code unit is at most 3 UTF-8 bytes, so a buffer this size can + // never overflow within a single `encodeInto` of any one line + const required = charTotal * 3; + if (lineScratch.length < required) { + lineScratch = new Uint8Array(required); + } + // Pick the smallest offset width that fits the file. `required` is the upper + // bound of the final byte length (the largest possible offset), so most files + // (< 64KB) get a `Uint16Array`, halving this table versus `Uint32Array`, with + // no per-line check and no copy. + const offsets = + required < 0x100 + ? new Uint8Array(count + 1) + : required < 0x10000 + ? new Uint16Array(count + 1) + : new Uint32Array(count + 1); + + let position = 0; + let lossy = false; + for (let index = 0; index < count; index++) { + const line = building[index]; + if (!knownLossless && !lossy && !isWellFormed(line)) { + lossy = true; + } + const { written } = encoder.encodeInto( + line, + position === 0 ? lineScratch : lineScratch.subarray(position) + ); + position += written; + offsets[index + 1] = position; + } + + // A lone surrogate can't be represented in UTF-8, so keep the exact line + // strings for that (essentially nonexistent) file instead of the arena + if (lossy) { + return { length: count, lines: building }; + } + + return { + length: count, + bytes: lineScratch.slice(0, position), + offsets, + }; +} diff --git a/packages/diffs/src/utils/editSessionHunks.ts b/packages/diffs/src/utils/editSessionHunks.ts index 12070712b..e2ef7e88c 100644 --- a/packages/diffs/src/utils/editSessionHunks.ts +++ b/packages/diffs/src/utils/editSessionHunks.ts @@ -7,6 +7,13 @@ import type { Hunk, HunkExpansionRegion, } from '../types'; +import { + type DiffLines, + joinLines, + lineAt, + linesToArray, + plainLines, +} from './diffLines'; import { getHunkSideStartBoundary } from './getHunkSideBoundaries'; import { parseDiffFromFile } from './parseDiffFromFile'; import { @@ -59,7 +66,7 @@ interface RegionPlan { const deletionLineSetCache = new WeakMap< FileDiffMetadata, - { lines: string[]; set: Set } + { lines: DiffLines; set: Set } >(); /** @@ -67,9 +74,11 @@ const deletionLineSetCache = new WeakMap< * in a newline exposes one extra empty line the parsed diff never contains) * so session line arrays compare like parse-derived ones. */ -export function normalizeEditorLines(lines: string[]): string[] { - if (lines.length > 1 && lines[lines.length - 1] === '') { - return lines.slice(0, -1); +// Returns `lines` itself when nothing is trimmed, so callers can keep using an +// identity check to tell whether the editor lines were already canonical. +export function normalizeEditorLines(lines: DiffLines): DiffLines { + if (lines.length > 1 && lineAt(lines, lines.length - 1) === '') { + return plainLines(linesToArray(lines).slice(0, -1)); } return lines; } @@ -79,12 +88,15 @@ export function normalizeEditorLines(lines: string[]): string[] { * during an edit session, so this result needs no prior-pass snapshot. */ export function findDivergenceCore( - deletionLines: string[], - additionLines: string[] + deletionLines: DiffLines, + additionLines: DiffLines ): DivergenceCore | undefined { const maxStart = Math.min(deletionLines.length, additionLines.length); let start = 0; - while (start < maxStart && deletionLines[start] === additionLines[start]) { + while ( + start < maxStart && + lineAt(deletionLines, start) === lineAt(additionLines, start) + ) { start++; } let deletionEnd = deletionLines.length; @@ -92,7 +104,8 @@ export function findDivergenceCore( while ( deletionEnd > start && additionEnd > start && - deletionLines[deletionEnd - 1] === additionLines[additionEnd - 1] + lineAt(deletionLines, deletionEnd - 1) === + lineAt(additionLines, additionEnd - 1) ) { deletionEnd--; additionEnd--; @@ -223,7 +236,7 @@ function canRetainCanonicalBlocks( } for (const line of changedLines) { const previousLine = previousAdditionLines.get(line); - const additionLine = diff.additionLines[line]; + const additionLine = lineAt(diff.additionLines, line); if ( previousLine == null || additionLine == null || @@ -256,7 +269,7 @@ function getDeletionLineSet(diff: FileDiffMetadata): Set { if (cached?.lines === diff.deletionLines) { return cached.set; } - const set = new Set(diff.deletionLines); + const set = new Set(linesToArray(diff.deletionLines)); deletionLineSetCache.set(diff, { lines: diff.deletionLines, set }); return set; } @@ -437,11 +450,11 @@ function parseCanonicalChangeBlocks( const parsed = parseDiffFromFile( { name: diff.prevName ?? diff.name, - contents: diff.deletionLines.join(''), + contents: joinLines(diff.deletionLines), }, { name: diff.name, - contents: diff.additionLines.join(''), + contents: joinLines(diff.additionLines), lang: diff.lang, }, parseDiffOptions diff --git a/packages/diffs/src/utils/hydratePartialDiff.ts b/packages/diffs/src/utils/hydratePartialDiff.ts index 83955e300..e4a8225e1 100644 --- a/packages/diffs/src/utils/hydratePartialDiff.ts +++ b/packages/diffs/src/utils/hydratePartialDiff.ts @@ -5,6 +5,7 @@ import type { Hunk, } from '../types'; import { cloneFileDiffMetadata } from './cloneFileDiffMetadata'; +import { plainLines } from './diffLines'; import { getHunkSideEndBoundary, getHunkSideStartBoundary, @@ -44,8 +45,10 @@ export function hydratePartialDiff( requireMissingOldFile(targetFileDiff, files); const lines = splitFileContents(newFile.contents); targetFileDiff.isPartial = false; - targetFileDiff.deletionLines = lines; - targetFileDiff.additionLines = lines; + // Both sides keep sharing one underlying array, as they did before the + // sides became `DiffLines` + targetFileDiff.deletionLines = plainLines(lines); + targetFileDiff.additionLines = plainLines(lines); setHydratedCacheKey(targetFileDiff, null, newFile); return targetFileDiff; } @@ -71,8 +74,8 @@ function hydrateTwoSidedFileDiff( fileDiff.splitLineCount = splitLineCount; fileDiff.unifiedLineCount = unifiedLineCount; fileDiff.isPartial = false; - fileDiff.deletionLines = deletionLines; - fileDiff.additionLines = additionLines; + fileDiff.deletionLines = plainLines(deletionLines); + fileDiff.additionLines = plainLines(additionLines); setHydratedCacheKey(fileDiff, oldFile, newFile); return fileDiff; } diff --git a/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts b/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts index a1218458d..e7569e3e8 100644 --- a/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts +++ b/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts @@ -47,6 +47,7 @@ import type { MergeConflictRegion, ProcessFileConflictData, } from '../types'; +import { plainLines } from './diffLines'; import { getHunkSideEndBoundary, getHunkSideStartBoundary, @@ -314,8 +315,8 @@ export function parseMergeConflictDiffFromFile( splitLineCount: s.splitLineCount, unifiedLineCount: s.unifiedLineCount, isPartial: false, - deletionLines: s.deletionLines, - additionLines: s.additionLines, + deletionLines: plainLines(s.deletionLines), + additionLines: plainLines(s.additionLines), cacheKey: file.cacheKey != null ? `${file.cacheKey}:merge-conflict-diff` diff --git a/packages/diffs/src/utils/parsePatchFiles.ts b/packages/diffs/src/utils/parsePatchFiles.ts index da54f13f6..d4ba7eb3c 100644 --- a/packages/diffs/src/utils/parsePatchFiles.ts +++ b/packages/diffs/src/utils/parsePatchFiles.ts @@ -17,6 +17,12 @@ import type { } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; import { detachString, releaseStringDetachBuffer } from './detachString'; +import { + EMPTY_DIFF_LINES, + finishLines, + isWellFormed, + releaseLineScratch, +} from './diffLines'; import { getHunkSideEndBoundary, getHunkSideStartBoundary, @@ -40,6 +46,7 @@ export function processPatch( return _processPatch(data, cacheKeyPrefix, throwOnError); } finally { releaseStringDetachBuffer(); + releaseLineScratch(); } } @@ -120,6 +127,7 @@ export function processFile( return _processFile(fileDiffString, options); } finally { releaseStringDetachBuffer(); + releaseLineScratch(); } } @@ -139,6 +147,8 @@ function _processFile( const isPartial = oldFile == null || newFile == null; let deletionLineIndex = 0; let additionLineIndex = 0; + let additionLineList: string[] = []; + let deletionLineList: string[] = []; for (const hunk of hunks) { const lines = splitWithNewlines(hunk); const firstLine = lines[0]; @@ -164,6 +174,22 @@ function _processFile( } continue; } + additionLineList = + !isPartial && oldFile != null && newFile != null + ? splitWithNewlines(newFile.contents) + : []; + deletionLineList = + !isPartial && oldFile != null && newFile != null + ? splitWithNewlines(oldFile.contents) + : []; + // If either file is technically empty, then we should empty the + // arrays respectively + if (additionLineList.length === 1 && newFile?.contents === '') { + additionLineList.length = 0; + } + if (deletionLineList.length === 1 && oldFile?.contents === '') { + deletionLineList.length = 0; + } currentFile = { name: '', type: 'change', @@ -171,24 +197,10 @@ function _processFile( splitLineCount: 0, unifiedLineCount: 0, isPartial, - additionLines: - !isPartial && oldFile != null && newFile != null - ? splitFileContents(newFile.contents) - : [], - deletionLines: - !isPartial && oldFile != null && newFile != null - ? splitFileContents(oldFile.contents) - : [], + additionLines: EMPTY_DIFF_LINES, + deletionLines: EMPTY_DIFF_LINES, cacheKey: maybeDetachOptionalString(cacheKey), }; - // If either file is technically empty, then we should empty the - // arrays respectively - if (currentFile.additionLines.length === 1 && newFile?.contents === '') { - currentFile.additionLines.length = 0; - } - if (currentFile.deletionLines.length === 1 && oldFile?.contents === '') { - currentFile.deletionLines.length = 0; - } for (const line of lines) { if (line.startsWith('diff --git')) { @@ -392,7 +404,7 @@ function _processFile( additionLineIndex++; parsedAdditionLines++; if (isPartial) { - currentFile.additionLines.push(line); + additionLineList.push(line); } currentContent.additions++; additionLines++; @@ -413,7 +425,7 @@ function _processFile( deletionLineIndex++; parsedDeletionLines++; if (isPartial) { - currentFile.deletionLines.push(line); + deletionLineList.push(line); } currentContent.deletions++; deletionLines++; @@ -440,8 +452,8 @@ function _processFile( parsedAdditionLines++; parsedDeletionLines++; if (isPartial) { - currentFile.deletionLines.push(line); - currentFile.additionLines.push(line); + deletionLineList.push(line); + additionLineList.push(line); } currentContent.lines++; lastLineType = 'context'; @@ -460,10 +472,10 @@ function _processFile( isPartial && (lastLineType === 'addition' || lastLineType === 'context') ) { - const lastIndex = currentFile.additionLines.length - 1; + const lastIndex = additionLineList.length - 1; if (lastIndex >= 0) { - currentFile.additionLines[lastIndex] = cleanLastNewline( - currentFile.additionLines[lastIndex] + additionLineList[lastIndex] = cleanLastNewline( + additionLineList[lastIndex] ); } } @@ -471,10 +483,10 @@ function _processFile( isPartial && (lastLineType === 'deletion' || lastLineType === 'context') ) { - const lastIndex = currentFile.deletionLines.length - 1; + const lastIndex = deletionLineList.length - 1; if (lastIndex >= 0) { - currentFile.deletionLines[lastIndex] = cleanLastNewline( - currentFile.deletionLines[lastIndex] + deletionLineList[lastIndex] = cleanLastNewline( + deletionLineList[lastIndex] ); } } @@ -541,15 +553,17 @@ function _processFile( if ( currentFile.hunks.length > 0 && !isPartial && - currentFile.additionLines.length > 0 && - currentFile.deletionLines.length > 0 + additionLineList.length > 0 && + deletionLineList.length > 0 ) { const lastHunk = currentFile.hunks[currentFile.hunks.length - 1]; const lastHunkEnd = getHunkSideEndBoundary( lastHunk.additionStart, lastHunk.additionCount ); - const totalFileLines = currentFile.additionLines.length; + // `currentFile.additionLines` is not sealed into its arena until the end of + // this function, so the live line count is still the local list + const totalFileLines = additionLineList.length; const collapsedAfter = Math.max(totalFileLines - lastHunkEnd, 0); currentFile.splitLineCount += collapsedAfter; currentFile.unifiedLineCount += collapsedAfter; @@ -589,9 +603,26 @@ function _processFile( ) { currentFile.prevName = undefined; } + // A lone surrogate can't survive the UTF-8 round-trip; checking the whole file + // once here lets `finishLines` skip the per-line check on the common path + const losslessFileDiff = isWellFormed(fileDiffString); + // Seal each side's lines into one compact byte arena per file. On the + // partial-patch path every line is a slice of `fileDiffString` so the + // single surrogate check above lets the seal skip per-line checks; a + // full-file diff is rarer and falls back to the safe per-line check + currentFile.additionLines = finishLines( + additionLineList, + isPartial && losslessFileDiff + ); + currentFile.deletionLines = finishLines( + deletionLineList, + isPartial && losslessFileDiff + ); // Pair change-block lines by similarity instead of the patch's positional // ordering. Partial diffs work too: their hunk line indexes point into the - // patch-built line arrays, which hold every line a block references. + // patch-built line arrays, which hold every line a block references. This + // runs after the seal above because it reads lines back off `currentFile`, + // and it only rewrites hunk content, never the sealed line arenas. realignChangeContentBySimilarity(currentFile); return currentFile; } @@ -641,14 +672,6 @@ function hasCommitMetadataBoundary(data: string): boolean { return data.startsWith('From ') || data.includes('\nFrom '); } -function splitFileContents(contents: string): string[] { - const lines = splitWithNewlines(contents); - for (let index = 0; index < lines.length; index++) { - lines[index] = detachString(lines[index]); - } - return lines; -} - function splitWithNewlines(contents: string): string[] { if (contents.length === 0) { return ['']; @@ -969,7 +992,7 @@ function parseRawLineType( function getParsedLineContent(rawLine: string): string { const processedLine = rawLine.slice(1); - return detachString(processedLine === '' ? '\n' : processedLine); + return processedLine === '' ? '\n' : processedLine; } function createContentGroup( diff --git a/packages/diffs/src/utils/realignChangeContent.ts b/packages/diffs/src/utils/realignChangeContent.ts index 4f26e5e88..6c2a9a058 100644 --- a/packages/diffs/src/utils/realignChangeContent.ts +++ b/packages/diffs/src/utils/realignChangeContent.ts @@ -4,6 +4,7 @@ import type { FileDiffMetadata, Hunk, } from '../types'; +import { lineAt } from './diffLines'; // The diff library emits one change block per replaced run, ordering every // deleted line before every added line. Renderers pair a block's lines @@ -95,13 +96,13 @@ export function slideBlankBoundaryBlocksUp( // Sliding through identical lines is only well-defined when the block is // a uniform run; require every block line to equal the first one, and // that line to be blank. - const unit = lines[blockStart] ?? ''; + const unit = lineAt(lines, blockStart) ?? ''; if (unit.trim() !== '') { continue; } let uniform = true; for (let offset = 1; offset < blockLength; offset++) { - if (lines[blockStart + offset] !== unit) { + if (lineAt(lines, blockStart + offset) !== unit) { uniform = false; break; } @@ -115,9 +116,10 @@ export function slideBlankBoundaryBlocksUp( let slide = 0; while ( slide < previous.lines && - diff.additionLines[ + lineAt( + diff.additionLines, previous.additionLineIndex + previous.lines - 1 - slide - ] === unit + ) === unit ) { slide++; } @@ -181,13 +183,17 @@ function realignChangeBlock( const strippedDeletions: string[] = []; for (let line = 0; line < deletions; line++) { strippedDeletions.push( - stripWhitespace(diff.deletionLines[deletionLineIndex + line] ?? '') + stripWhitespace( + lineAt(diff.deletionLines, deletionLineIndex + line) ?? '' + ) ); } const strippedAdditions: string[] = []; for (let line = 0; line < additions; line++) { strippedAdditions.push( - stripWhitespace(diff.additionLines[additionLineIndex + line] ?? '') + stripWhitespace( + lineAt(diff.additionLines, additionLineIndex + line) ?? '' + ) ); } diff --git a/packages/diffs/src/utils/renderDiffWithHighlighter.ts b/packages/diffs/src/utils/renderDiffWithHighlighter.ts index 709c65917..33acb1caf 100644 --- a/packages/diffs/src/utils/renderDiffWithHighlighter.ts +++ b/packages/diffs/src/utils/renderDiffWithHighlighter.ts @@ -21,6 +21,7 @@ import type { } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; import { createTransformerWithState } from './createTransformerWithState'; +import { lineAt } from './diffLines'; import { formatCSSVariablePrefix } from './formatCSSVariablePrefix'; import { getFiletypeFromFileName } from './getFiletypeFromFileName'; import { getHighlighterThemeStyles } from './getHighlighterThemeStyles'; @@ -135,8 +136,8 @@ export function renderDiffWithHighlighter( if (type === 'change' && additionLine != null && deletionLine != null) { computeLineDiffDecorations({ - additionLine: diff.additionLines[additionLine.lineIndex], - deletionLine: diff.deletionLines[deletionLine.lineIndex], + additionLine: lineAt(diff.additionLines, additionLine.lineIndex), + deletionLine: lineAt(diff.deletionLines, deletionLine.lineIndex), deletionLineIndex: bucket.deletionContent.length, additionLineIndex: bucket.additionContent.length, deletionDecorations: bucket.deletionDecorations, @@ -148,7 +149,7 @@ export function renderDiffWithHighlighter( if (deletionLine != null) { appendContent( - diff.deletionLines[deletionLine.lineIndex], + lineAt(diff.deletionLines, deletionLine.lineIndex), deletionLine.lineIndex, bucket.deletionSegments, bucket.deletionContent @@ -166,7 +167,7 @@ export function renderDiffWithHighlighter( if (additionLine != null) { appendContent( - diff.additionLines[additionLine.lineIndex], + lineAt(diff.additionLines, additionLine.lineIndex), additionLine.lineIndex, bucket.additionSegments, bucket.additionContent diff --git a/packages/diffs/src/utils/resolveRegion.ts b/packages/diffs/src/utils/resolveRegion.ts index 0776d617e..fdca938b5 100644 --- a/packages/diffs/src/utils/resolveRegion.ts +++ b/packages/diffs/src/utils/resolveRegion.ts @@ -4,6 +4,7 @@ import type { FileDiffMetadata, Hunk, } from '../types'; +import { type DiffLines, finishLines, lineAt } from './diffLines'; import { getHunkSideEndBoundary, getHunkSideStartBoundary, @@ -26,6 +27,14 @@ interface CursorState { unifiedLineCount: number; } +type ResolvedDiffBuilder = Omit< + FileDiffMetadata, + 'additionLines' | 'deletionLines' +> & { + additionLines: string[]; + deletionLines: string[]; +}; + export function resolveRegion( diff: FileDiffMetadata, target: RegionResolutionTarget @@ -54,7 +63,7 @@ export function resolveRegion( } const { hunks, additionLines, deletionLines } = diff; - const resolvedDiff: FileDiffMetadata = { + const resolvedDiff: ResolvedDiffBuilder = { ...diff, hunks: [], deletionLines: [], @@ -82,8 +91,9 @@ export function resolveRegion( for (const [index, hunk] of hunks.entries()) { processCollapsedContext( - diff, resolvedDiff, + deletionLines, + additionLines, cursor, getHunkSideStartBoundary(hunk.deletionStart, hunk.deletionCount) - hunk.collapsedBefore, @@ -236,20 +246,24 @@ export function resolveRegion( resolvedDiff.splitLineCount = cursor.splitLineCount; resolvedDiff.unifiedLineCount = cursor.unifiedLineCount; - return resolvedDiff; + return { + ...resolvedDiff, + deletionLines: finishLines(resolvedDiff.deletionLines), + additionLines: finishLines(resolvedDiff.additionLines), + }; } function pushCollapsedContextLines( - diff: FileDiffMetadata, - deletionLines: string[], - additionLines: string[], + diff: ResolvedDiffBuilder, + deletionLines: DiffLines, + additionLines: DiffLines, deletionLineIndex: number, additionLineIndex: number, lineCount: number ) { for (let index = 0; index < lineCount; index++) { - const deletionLine = deletionLines[deletionLineIndex + index]; - const additionLine = additionLines[additionLineIndex + index]; + const deletionLine = lineAt(deletionLines, deletionLineIndex + index); + const additionLine = lineAt(additionLines, additionLineIndex + index); if (deletionLine == null || additionLine == null) { throw new Error( 'pushCollapsedContextLines: missing collapsed context line' @@ -264,8 +278,9 @@ function pushCollapsedContextLines( // not exist in the diff's line arrays. Keep the virtual row counts and file // positions in sync without inventing hidden lines. function processCollapsedContext( - sourceDiff: FileDiffMetadata, - resolvedDiff: FileDiffMetadata, + diff: ResolvedDiffBuilder, + sourceDeletionLines: DiffLines, + sourceAdditionLines: DiffLines, cursor: CursorState, deletionLineIndex: number, additionLineIndex: number, @@ -278,9 +293,9 @@ function processCollapsedContext( if (shouldProcessContent) { pushCollapsedContextLines( - resolvedDiff, - sourceDiff.deletionLines, - sourceDiff.additionLines, + diff, + sourceDeletionLines, + sourceAdditionLines, deletionLineIndex, additionLineIndex, lineCount @@ -297,13 +312,13 @@ function processCollapsedContext( function pushContentLinesToDiff( content: ContextContent | ChangeContent, - diff: FileDiffMetadata, - deletionLines: string[], - additionLines: string[] + diff: ResolvedDiffBuilder, + deletionLines: DiffLines, + additionLines: DiffLines ) { if (content.type === 'context') { for (let i = 0; i < content.lines; i++) { - const line = additionLines[content.additionLineIndex + i]; + const line = lineAt(additionLines, content.additionLineIndex + i); if (line == null) { console.error({ additionLines, content, i }); throw new Error('pushContentLinesToDiff: Context line does not exist'); @@ -315,7 +330,7 @@ function pushContentLinesToDiff( const len = Math.max(content.deletions, content.additions); for (let i = 0; i < len; i++) { if (i < content.deletions) { - const line = deletionLines[content.deletionLineIndex + i]; + const line = lineAt(deletionLines, content.deletionLineIndex + i); if (line == null) { console.error({ deletionLines, content, i }); throw new Error( @@ -325,7 +340,7 @@ function pushContentLinesToDiff( diff.deletionLines.push(line); } if (i < content.additions) { - const line = additionLines[content.additionLineIndex + i]; + const line = lineAt(additionLines, content.additionLineIndex + i); if (line == null) { console.error({ additionLines, content, i }); throw new Error( @@ -341,13 +356,13 @@ function pushContentLinesToDiff( function pushResolveLinesToDiff( resolution: 'deletions' | 'additions' | 'both', content: ChangeContent, - diff: FileDiffMetadata, - deletionLines: string[], - additionLines: string[] + diff: ResolvedDiffBuilder, + deletionLines: DiffLines, + additionLines: DiffLines ) { if (resolution === 'deletions' || resolution === 'both') { for (let i = 0; i < content.deletions; i++) { - const line = deletionLines[content.deletionLineIndex + i]; + const line = lineAt(deletionLines, content.deletionLineIndex + i); if (line == null) { console.error({ deletionLines, content, i }); throw new Error('pushResolveLinesToDiff: Deletion line does not exist'); @@ -358,7 +373,7 @@ function pushResolveLinesToDiff( } if (resolution === 'additions' || resolution === 'both') { for (let i = 0; i < content.additions; i++) { - const line = additionLines[content.additionLineIndex + i]; + const line = lineAt(additionLines, content.additionLineIndex + i); if (line == null) { console.error({ additionLines, content, i }); throw new Error('pushResolveLinesToDiff: Addition line does not exist'); diff --git a/packages/diffs/src/utils/updateDiffHunks.ts b/packages/diffs/src/utils/updateDiffHunks.ts index 6d1aeb234..3b64818f9 100644 --- a/packages/diffs/src/utils/updateDiffHunks.ts +++ b/packages/diffs/src/utils/updateDiffHunks.ts @@ -7,6 +7,13 @@ import type { Hunk, } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; +import { + type DiffLines, + joinLineRange, + joinLines, + lineAt, + plainLines, +} from './diffLines'; import { getHunkSideEndBoundary, getHunkSideStartBoundary, @@ -32,11 +39,11 @@ export function recomputeDiffHunks( const recomputed = parseDiffFromFile( { name: diff.prevName ?? diff.name, - contents: diff.deletionLines.join(''), + contents: joinLines(diff.deletionLines), }, { name: diff.name, - contents: diff.additionLines.join(''), + contents: joinLines(diff.additionLines), lang: diff.lang, }, parseDiffOptions @@ -72,9 +79,9 @@ function buildTopAlignedAdditionSentinel( return sentinel; } -function hasOnlyBlankAdditionContents(additionLines: string[]): boolean { - for (const line of additionLines) { - if (line.trim().length > 0) { +function hasOnlyBlankAdditionContents(additionLines: DiffLines): boolean { + for (let index = 0; index < additionLines.length; index++) { + if (lineAt(additionLines, index).trim().length > 0) { return false; } } @@ -86,7 +93,7 @@ function hasOnlyBlankAdditionContents(additionLines: string[]): boolean { // render the first editable row deep in split view instead of at the top. export function shouldTopAlignAdditionRecompute( diff: FileDiffMetadata, - additionLines: string[] + additionLines: DiffLines ): boolean { return ( additionLines.length > 0 && @@ -100,10 +107,10 @@ export function shouldTopAlignAdditionRecompute( // preserved so the editor document stays the source of truth. export function recomputeTopAlignedAdditionDiff( diff: FileDiffMetadata, - additionLines: string[], + additionLines: DiffLines, parseDiffOptions?: CreatePatchOptionsNonabortable ): FullDiffHunkUpdate { - const deletionContents = diff.deletionLines.join(''); + const deletionContents = joinLines(diff.deletionLines); const additionSentinel = buildTopAlignedAdditionSentinel( additionLines.length, deletionContents @@ -140,7 +147,11 @@ export function recomputeEmptyDocumentDiff( diff: FileDiffMetadata, parseDiffOptions?: CreatePatchOptionsNonabortable ): FullDiffHunkUpdate { - return recomputeTopAlignedAdditionDiff(diff, [''], parseDiffOptions); + return recomputeTopAlignedAdditionDiff( + diff, + plainLines(['']), + parseDiffOptions + ); } /** Rebuilds diff hunks after an edit, top-aligning sparse addition sides when needed. */ @@ -164,8 +175,11 @@ export function recomputeDiffHunksForEdit( return recomputed; } -function hasTrailingEditorBlankLine(additionLines: string[]): boolean { - return additionLines.length > 1 && additionLines.at(-1) === ''; +function hasTrailingEditorBlankLine(additionLines: DiffLines): boolean { + return ( + additionLines.length > 1 && + lineAt(additionLines, additionLines.length - 1) === '' + ); } // Re-adds the editor's phantom trailing empty line (a document ending in a @@ -176,7 +190,7 @@ export function preserveTrailingEditorBlankLine( FullDiffHunkUpdate, 'hunks' | 'additionLines' | 'splitLineCount' | 'unifiedLineCount' >, - additionLines: string[] + additionLines: DiffLines ): void { if (!hasTrailingEditorBlankLine(additionLines)) { return; @@ -246,8 +260,8 @@ export function updateDiffHunks( }); } for (const line of changedLines) { - const additionLine = diff.additionLines[line]; - const deletionLine = diff.deletionLines[line]; + const additionLine = lineAt(diff.additionLines, line); + const deletionLine = lineAt(diff.deletionLines, line); if (additionLine == null || deletionLine == null) { return applyHunkUpdateResult( diff, @@ -344,11 +358,13 @@ function reparseHunkRegion( return false; } - const deletionSlice = diff.deletionLines.slice( + const deletionSlice = joinLineRange( + diff.deletionLines, hunk.deletionLineIndex, hunk.deletionLineIndex + hunk.deletionCount ); - const additionSlice = diff.additionLines.slice( + const additionSlice = joinLineRange( + diff.additionLines, hunk.additionLineIndex, hunk.additionLineIndex + hunk.additionCount ); @@ -356,11 +372,11 @@ function reparseHunkRegion( const reparsed = parseDiffFromFile( { name: diff.prevName ?? diff.name, - contents: deletionSlice.join(''), + contents: deletionSlice, }, { name: diff.name, - contents: additionSlice.join(''), + contents: additionSlice, lang: diff.lang, }, { ...parseDiffOptions, context: 0 } @@ -392,8 +408,14 @@ export function syncHunkNoEOFCRFromFullFile( return; } - const lastAdditionLine = diff.additionLines.at(-1); - const lastDeletionLine = diff.deletionLines.at(-1); + const lastAdditionLine = lineAt( + diff.additionLines, + diff.additionLines.length - 1 + ); + const lastDeletionLine = lineAt( + diff.deletionLines, + diff.deletionLines.length - 1 + ); hunk.noEOFCRAdditions = lastAdditionLine != null && lastAdditionLine !== '' && diff --git a/packages/diffs/test/CodeView.partialHydration.test.ts b/packages/diffs/test/CodeView.partialHydration.test.ts index 913852578..be4b865d7 100644 --- a/packages/diffs/test/CodeView.partialHydration.test.ts +++ b/packages/diffs/test/CodeView.partialHydration.test.ts @@ -8,6 +8,7 @@ import type { FileContents, FileDiffMetadata, } from '../src/types'; +import { linesToArray } from '../src/utils/diffLines'; import { parsePatchFiles } from '../src/utils/parsePatchFiles'; import { createRoot, @@ -114,13 +115,13 @@ describe('CodeView partial hydration', () => { expect(hydratedItem.fileDiff).toBe(partial); expect(renderedItem.instance.fileDiff).toBe(partial); expect(partial.isPartial).toBe(false); - expect(partial.deletionLines).toEqual([ + expect(linesToArray(partial.deletionLines)).toEqual([ 'keep 1\n', 'old value\n', 'keep 3\n', 'keep 4\n', ]); - expect(partial.additionLines).toEqual([ + expect(linesToArray(partial.additionLines)).toEqual([ 'keep 1\n', 'new value\n', 'keep 3\n', diff --git a/packages/diffs/test/DiffHunksRendererRecompute.test.ts b/packages/diffs/test/DiffHunksRendererRecompute.test.ts index 28a55e3e9..da8ab336f 100644 --- a/packages/diffs/test/DiffHunksRendererRecompute.test.ts +++ b/packages/diffs/test/DiffHunksRendererRecompute.test.ts @@ -8,6 +8,7 @@ import { import { TextDocument } from '../src/editor/textDocument'; import type { FileDiffMetadata, HighlightedToken } from '../src/types'; import type { DiffsTextDocument } from '../src/types'; +import { joinLines, lineAt, linesToArray } from '../src/utils/diffLines'; import { finishEditSessionForDiff } from '../src/utils/editSessionHunks'; import { iterateOverDiff } from '../src/utils/iterateOverDiff'; import { @@ -211,14 +212,14 @@ describe('DiffHunksRenderer content-edit recompute split', () => { expect(rendered).toBeDefined(); if (rendered == null) return; - expect(rendered.additionLines).toEqual([ + expect(linesToArray(rendered.additionLines)).toEqual([ 'function greet(name) {\n', ' console.log(\n', 'msg);\n', ' return msg;\n', '}\n', ]); - expect(rendered.additionLines.join('')).toBe(EDITED_LINES.join('\n')); + expect(joinLines(rendered.additionLines)).toBe(EDITED_LINES.join('\n')); }); }); @@ -296,7 +297,8 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { const mismatches = rows.filter( (row) => row.lineNumber != null && - row.text !== diff.additionLines[row.lineNumber - 1].replace(/\n$/, '') + row.text !== + lineAt(diff.additionLines, row.lineNumber - 1).replace(/\n$/, '') ); expect(mismatches.map((row) => `#${row.lineNumber}: ${row.text}`)).toEqual( [] @@ -497,7 +499,7 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { ); expect(diff.hunks[1].deletionCount).toBe(secondRegionBefore.deletionCount); expect(diff.hunks[1].additionCount).toBe(secondRegionBefore.additionCount); - expect(diff.additionLines.join('')).toBe(postEditLines.join('')); + expect(joinLines(diff.additionLines)).toBe(postEditLines.join('')); const full = parseDiffFromFile( { name: 'session.ts', contents: SESSION_OLD.join('') }, { name: 'session.ts', contents: postEditLines.join('') } @@ -508,7 +510,7 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { test('a same-line edit can rebuild after preserving the trailing editor row', async () => { const { renderer, diff } = await createSessionRenderer(); renderer.applyDocumentChange(makeTextDocumentFromText('line 1\n')); - expect(diff.additionLines).toEqual(['line 1\n', '']); + expect(linesToArray(diff.additionLines)).toEqual(['line 1\n', '']); expect(() => renderer.updateRenderCache( @@ -517,7 +519,7 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { ) ).not.toThrow(); - expect(diff.additionLines).toEqual(['line 1 edited\n', '']); + expect(linesToArray(diff.additionLines)).toEqual(['line 1 edited\n', '']); expect(renderer.renderDiff()).toBeDefined(); }); @@ -584,7 +586,7 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { const { renderer, diff } = await createSessionRenderer(); renderer.applyDocumentChange(makeTextDocument([''])); - expect(diff.additionLines).toEqual(['']); + expect(linesToArray(diff.additionLines)).toEqual(['']); const result = renderer.renderDiff(); expect(result).toBeDefined(); if (result == null) return; @@ -602,7 +604,7 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { ); expect(regionsChanged).toBe(true); - expect(diff.additionLines).toEqual(['']); + expect(linesToArray(diff.additionLines)).toEqual(['']); const result = renderer.renderDiff(); expect(result).toBeDefined(); if (result == null) return; @@ -612,13 +614,13 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { test('typing into a newline-only document rebuilds from canonical lines', async () => { const { renderer, diff } = await createSessionRenderer(); renderer.applyDocumentChange(makeTextDocumentFromText('\n')); - expect(diff.additionLines).toEqual(['\n', '']); + expect(linesToArray(diff.additionLines)).toEqual(['\n', '']); expect(() => renderer.updateRenderCache(makeDirtyLines([[0, 'typed']]), 'light') ).not.toThrow(); - expect(diff.additionLines).toEqual(['typed\n', '']); + expect(linesToArray(diff.additionLines)).toEqual(['typed\n', '']); expect(renderer.renderDiff()).toBeDefined(); }); @@ -681,7 +683,7 @@ describe('DiffHunksRenderer.applyDocumentChange empty document', () => { if (diff == null) return; // One empty line, not zero — this is the regression guard. - expect(diff.additionLines).toEqual(['']); + expect(linesToArray(diff.additionLines)).toEqual(['']); // The old content is still the deletion side. expect(diff.deletionLines.length).toBeGreaterThan(0); @@ -859,7 +861,7 @@ describe('DiffHunksRenderer.applyDocumentChange empty document', () => { const rendered = renderer.diffCache; expect(rendered).toBeDefined(); if (rendered == null) return; - expect(rendered.additionLines).toEqual(['\n', '']); + expect(linesToArray(rendered.additionLines)).toEqual(['\n', '']); const additionSplitLines: number[] = []; iterateOverDiff({ @@ -894,9 +896,9 @@ describe('DiffHunksRenderer.applyDocumentChange empty document', () => { const rendered = renderer.diffCache; expect(rendered).toBeDefined(); if (rendered == null) return; - expect(rendered.additionLines).toEqual(['']); + expect(linesToArray(rendered.additionLines)).toEqual(['']); // The blank old line is still recorded as the deletion side. - expect(rendered.deletionLines.join('')).toBe('\n'); + expect(joinLines(rendered.deletionLines)).toBe('\n'); // At least one hunk, so iterateOverDiff has a row to emit. expect(rendered.hunks.length).toBeGreaterThanOrEqual(1); diff --git a/packages/diffs/test/FileDiff.partialHydration.test.ts b/packages/diffs/test/FileDiff.partialHydration.test.ts index 66b934f50..b6f9fcc2c 100644 --- a/packages/diffs/test/FileDiff.partialHydration.test.ts +++ b/packages/diffs/test/FileDiff.partialHydration.test.ts @@ -14,7 +14,7 @@ import type { } from '../src/types'; import type { WorkerPoolManager } from '../src/worker'; import { installDom, wait } from './domHarness'; -import { assertDefined } from './testUtils'; +import { assertDefined, linesOf } from './testUtils'; afterAll(async () => { await disposeHighlighter(); @@ -322,13 +322,13 @@ describe('FileDiff partial hydration', () => { expect(instance.fileDiff).toBe(partial); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual([ + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ 'keep 1\n', 'new value\n', 'keep 3\n', 'keep 4\n', ]); - expect(instance.fileDiff?.deletionLines).toEqual([ + expect(linesOf(instance.fileDiff?.deletionLines)).toEqual([ 'keep 1\n', 'old value\n', 'keep 3\n', @@ -697,7 +697,7 @@ describe('FileDiff partial hydration', () => { expect(instance.fileDiff).toBe(nextDiff); expect(instance.fileDiff?.name).toBe('second.txt'); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual(['after\n']); + expect(linesOf(instance.fileDiff?.additionLines)).toEqual(['after\n']); } finally { instance?.cleanUp(); cleanup(); @@ -891,7 +891,7 @@ describe('FileDiff partial hydration', () => { expect(instance.fileDiff).toBe(nextDiff); expect(instance.fileDiff?.name).toBe('second.ts'); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual([ + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ 'const value = "after";\n', ]); } finally { @@ -930,8 +930,14 @@ describe('FileDiff partial hydration', () => { expect(instance.fileDiff?.type).toBe('rename-pure'); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual(['alpha\n', 'beta\n']); - expect(instance.fileDiff?.deletionLines).toEqual(['alpha\n', 'beta\n']); + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ + 'alpha\n', + 'beta\n', + ]); + expect(linesOf(instance.fileDiff?.deletionLines)).toEqual([ + 'alpha\n', + 'beta\n', + ]); } finally { instance?.cleanUp(); cleanup(); @@ -980,7 +986,7 @@ describe('FileDiff partial hydration', () => { expect(instance.fileDiff).toBe(partial); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual([ + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ 'keep 1\n', 'new value\n', 'keep 3\n', diff --git a/packages/diffs/test/FileDiff.partialRender.test.ts b/packages/diffs/test/FileDiff.partialRender.test.ts index 7b00eda54..67de2bb0d 100644 --- a/packages/diffs/test/FileDiff.partialRender.test.ts +++ b/packages/diffs/test/FileDiff.partialRender.test.ts @@ -3,6 +3,7 @@ import { afterAll, describe, expect, test } from 'bun:test'; import { disposeHighlighter, FileDiff, parseDiffFromFile } from '../src'; import type { DiffLineAnnotation } from '../src/types'; import { installDom } from './domHarness'; +import { linesOf } from './testUtils'; afterAll(async () => { await disposeHighlighter(); @@ -165,8 +166,11 @@ describe('FileDiff partial render', () => { expect(instance.fileDiff?.type).toBe('new'); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.deletionLines).toEqual([]); - expect(instance.fileDiff?.additionLines).toEqual(['alpha\n', 'beta\n']); + expect(linesOf(instance.fileDiff?.deletionLines)).toEqual([]); + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ + 'alpha\n', + 'beta\n', + ]); } finally { instance?.cleanUp(); cleanup(); @@ -196,8 +200,11 @@ describe('FileDiff partial render', () => { expect(instance.fileDiff?.type).toBe('deleted'); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.deletionLines).toEqual(['alpha\n', 'beta\n']); - expect(instance.fileDiff?.additionLines).toEqual([]); + expect(linesOf(instance.fileDiff?.deletionLines)).toEqual([ + 'alpha\n', + 'beta\n', + ]); + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([]); } finally { instance?.cleanUp(); cleanup(); diff --git a/packages/diffs/test/FileDiff.unifiedEditSeparators.test.ts b/packages/diffs/test/FileDiff.unifiedEditSeparators.test.ts index 09ca64d8b..4919e96be 100644 --- a/packages/diffs/test/FileDiff.unifiedEditSeparators.test.ts +++ b/packages/diffs/test/FileDiff.unifiedEditSeparators.test.ts @@ -2,6 +2,7 @@ import { afterAll, describe, expect, test } from 'bun:test'; import { disposeHighlighter, FileDiff, parseDiffFromFile } from '../src'; import type { DiffsTextDocument } from '../src/types'; +import { linesToArray } from '../src/utils/diffLines'; import { installDom } from './domHarness'; const twoHunkFileLineCount = 140; @@ -91,7 +92,9 @@ describe('FileDiff unified edit separators', () => { expect(countSeparatorSlots(fileContainer)).toBeGreaterThan(0); - instance.applyDocumentChange(makeTextDocument(fileDiff.deletionLines)); + instance.applyDocumentChange( + makeTextDocument(linesToArray(fileDiff.deletionLines)) + ); expect(countSeparatorSlots(fileContainer)).toBe(0); } finally { diff --git a/packages/diffs/test/VirtualizedFileDiff.partialHydration.test.ts b/packages/diffs/test/VirtualizedFileDiff.partialHydration.test.ts index 464c1ca0c..9842cbb57 100644 --- a/packages/diffs/test/VirtualizedFileDiff.partialHydration.test.ts +++ b/packages/diffs/test/VirtualizedFileDiff.partialHydration.test.ts @@ -5,8 +5,9 @@ import { disposeHighlighter, parseDiffFromFile, parsePatchFiles } from '../src'; import { VirtualizedFileDiff } from '../src/components/VirtualizedFileDiff'; import type { Virtualizer } from '../src/components/Virtualizer'; import type { FileContents, FileDiffMetadata } from '../src/types'; +import { linesToArray } from '../src/utils/diffLines'; import { installDom, wait } from './domHarness'; -import { assertDefined } from './testUtils'; +import { assertDefined, linesOf } from './testUtils'; afterAll(async () => { await disposeHighlighter(); @@ -192,13 +193,13 @@ describe('VirtualizedFileDiff partial hydration', () => { expect(instance.fileDiff).toBe(partial); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual([ + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ 'keep 1\n', 'new value\n', 'keep 3\n', 'keep 4\n', ]); - expect(instance.fileDiff?.deletionLines).toEqual([ + expect(linesOf(instance.fileDiff?.deletionLines)).toEqual([ 'keep 1\n', 'old value\n', 'keep 3\n', @@ -303,7 +304,7 @@ describe('VirtualizedFileDiff partial hydration', () => { expect(instance.fileDiff).toBe(nextDiff); expect(instance.fileDiff?.name).toBe('second.txt'); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual(['after\n']); + expect(linesOf(instance.fileDiff?.additionLines)).toEqual(['after\n']); expect(virtualizerState.instanceChangedCalls).toEqual([ { layoutDirty: true }, ]); @@ -420,7 +421,9 @@ describe('VirtualizedFileDiff partial hydration', () => { expect(partial.isPartial).toBe(true); expect(instance.fileDiff).toBe(partial); - expect(instance.fileDiff?.additionLines).toEqual(['new value\n']); + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ + 'new value\n', + ]); expect(virtualizerState.instanceChangedCalls).toEqual([ { layoutDirty: true }, { layoutDirty: true }, @@ -431,7 +434,7 @@ describe('VirtualizedFileDiff partial hydration', () => { assertDefined(nextDiff, 'expected next diff'); expect(nextDiff).not.toBe(partial); expect(nextDiff.isPartial).toBe(false); - expect(nextDiff.additionLines).toEqual([ + expect(linesToArray(nextDiff.additionLines)).toEqual([ 'keep 1\n', 'new value\n', 'keep 3\n', @@ -480,8 +483,14 @@ describe('VirtualizedFileDiff partial hydration', () => { expect(instance.fileDiff?.type).toBe('rename-pure'); expect(instance.fileDiff?.isPartial).toBe(false); - expect(instance.fileDiff?.additionLines).toEqual(['alpha\n', 'beta\n']); - expect(instance.fileDiff?.deletionLines).toEqual(['alpha\n', 'beta\n']); + expect(linesOf(instance.fileDiff?.additionLines)).toEqual([ + 'alpha\n', + 'beta\n', + ]); + expect(linesOf(instance.fileDiff?.deletionLines)).toEqual([ + 'alpha\n', + 'beta\n', + ]); expect(virtualizerState.instanceChangedCalls.at(-1)).toEqual({ layoutDirty: true, }); diff --git a/packages/diffs/test/annotations.test.ts b/packages/diffs/test/annotations.test.ts index e64203992..8fcd3924f 100644 --- a/packages/diffs/test/annotations.test.ts +++ b/packages/diffs/test/annotations.test.ts @@ -11,6 +11,7 @@ import type { FileDiffMetadata, LineTypes, } from '../src/types'; +import { plainLines } from '../src/utils/diffLines'; import { fileNew, fileOld } from './mocks'; import { annotationProjection, @@ -53,8 +54,8 @@ function createNoHunkDiff(): FileDiffMetadata { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: [], - additionLines: [], + deletionLines: plainLines([]), + additionLines: plainLines([]), }; } diff --git a/packages/diffs/test/diffAcceptRejectHunk.test.ts b/packages/diffs/test/diffAcceptRejectHunk.test.ts index 2ead51c3f..79278b0f0 100644 --- a/packages/diffs/test/diffAcceptRejectHunk.test.ts +++ b/packages/diffs/test/diffAcceptRejectHunk.test.ts @@ -6,6 +6,7 @@ import type { FileDiffMetadata, } from '../src/types'; import { diffAcceptRejectHunk } from '../src/utils/diffAcceptRejectHunk'; +import { linesToArray } from '../src/utils/diffLines'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { parseMergeConflictDiffFromFile } from '../src/utils/parseMergeConflictDiffFromFile'; import { parsePatchFiles } from '../src/utils/parsePatchFiles'; @@ -133,7 +134,7 @@ function snapshotBlock( if (content.type === 'context') { return { type: 'context', - lines: diff.additionLines.slice( + lines: linesToArray(diff.additionLines).slice( content.additionLineIndex, content.additionLineIndex + content.lines ), @@ -142,11 +143,11 @@ function snapshotBlock( return { type: 'change', - deletionLines: diff.deletionLines.slice( + deletionLines: linesToArray(diff.deletionLines).slice( content.deletionLineIndex, content.deletionLineIndex + content.deletions ), - additionLines: diff.additionLines.slice( + additionLines: linesToArray(diff.additionLines).slice( content.additionLineIndex, content.additionLineIndex + content.additions ), @@ -208,13 +209,13 @@ function assertUnresolvedHunkMatchesSnapshot( } expect( - diff.deletionLines.slice( + linesToArray(diff.deletionLines).slice( content.deletionLineIndex, content.deletionLineIndex + content.lines ) ).toEqual(expectedBlock.lines); expect( - diff.additionLines.slice( + linesToArray(diff.additionLines).slice( content.additionLineIndex, content.additionLineIndex + content.lines ) @@ -228,13 +229,13 @@ function assertUnresolvedHunkMatchesSnapshot( } expect( - diff.deletionLines.slice( + linesToArray(diff.deletionLines).slice( content.deletionLineIndex, content.deletionLineIndex + content.deletions ) ).toEqual(expectedBlock.deletionLines); expect( - diff.additionLines.slice( + linesToArray(diff.additionLines).slice( content.additionLineIndex, content.additionLineIndex + content.additions ) @@ -268,13 +269,13 @@ function assertResolvedHunkMatchesExpected( ) ).toBe(expectedLines.length); expect( - diff.deletionLines.slice( + linesToArray(diff.deletionLines).slice( hunk.deletionLineIndex, hunk.deletionLineIndex + expectedLines.length ) ).toEqual(expectedLines); expect( - diff.additionLines.slice( + linesToArray(diff.additionLines).slice( hunk.additionLineIndex, hunk.additionLineIndex + expectedLines.length ) @@ -300,8 +301,8 @@ describe('diffAcceptRejectHunk', () => { const result = diffAcceptRejectHunk(diff, 0, 'accept'); - expect(result.deletionLines).toEqual(newLines); - expect(result.additionLines).toEqual(newLines); + expect(linesToArray(result.deletionLines)).toEqual(newLines); + expect(linesToArray(result.additionLines)).toEqual(newLines); expect(verifyFileDiffHunkValues(result)).toEqual({ valid: true, errors: [], @@ -322,8 +323,8 @@ describe('diffAcceptRejectHunk', () => { const result = diffAcceptRejectHunk(diff, 0, 'reject'); - expect(result.deletionLines).toEqual(oldLines); - expect(result.additionLines).toEqual(oldLines); + expect(linesToArray(result.deletionLines)).toEqual(oldLines); + expect(linesToArray(result.additionLines)).toEqual(oldLines); expect(verifyFileDiffHunkValues(result)).toEqual({ valid: true, errors: [], @@ -486,8 +487,12 @@ describe('diffAcceptRejectHunk', () => { const [hunk] = result.hunks; expect(result.isPartial).toBe(true); - expect(result.deletionLines).toEqual(getResolvedLines(snapshot, 'accept')); - expect(result.additionLines).toEqual(getResolvedLines(snapshot, 'accept')); + expect(linesToArray(result.deletionLines)).toEqual( + getResolvedLines(snapshot, 'accept') + ); + expect(linesToArray(result.additionLines)).toEqual( + getResolvedLines(snapshot, 'accept') + ); expect(hunk?.collapsedBefore).toBe(5); expect(hunk?.additionStart).toBe(6); expect(hunk?.deletionStart).toBe(6); @@ -575,8 +580,8 @@ describe('diffAcceptRejectHunk', () => { expect(hunk?.noEOFCRAdditions).toBe(true); expect(hunk?.noEOFCRDeletions).toBe(true); - expect(result.deletionLines).toEqual(expectedLines); - expect(result.additionLines).toEqual(expectedLines); + expect(linesToArray(result.deletionLines)).toEqual(expectedLines); + expect(linesToArray(result.additionLines)).toEqual(expectedLines); }); test('resolveConflict strips merge conflict separators from a resolved region', () => { @@ -610,12 +615,12 @@ describe('diffAcceptRejectHunk', () => { 0 ) ).toBe(3); - expect(result.deletionLines).toEqual([ + expect(linesToArray(result.deletionLines)).toEqual([ 'before\n', 'const value = 2;\n', 'after\n', ]); - expect(result.additionLines).toEqual([ + expect(linesToArray(result.additionLines)).toEqual([ 'before\n', 'const value = 2;\n', 'after\n', @@ -645,14 +650,14 @@ describe('diffAcceptRejectHunk', () => { const result = resolveConflict(fileDiff, actions[0]!, 'current'); - expect(result.deletionLines).toEqual([ + expect(linesToArray(result.deletionLines)).toEqual([ 'start\n', 'ours one\n', 'middle\n', 'ours two\n', 'end\n', ]); - expect(result.additionLines).toEqual([ + expect(linesToArray(result.additionLines)).toEqual([ 'start\n', 'ours one\n', 'middle\n', diff --git a/packages/diffs/test/editSessionHunks.test.ts b/packages/diffs/test/editSessionHunks.test.ts index 8a66b01fd..2542ae446 100644 --- a/packages/diffs/test/editSessionHunks.test.ts +++ b/packages/diffs/test/editSessionHunks.test.ts @@ -2,6 +2,12 @@ import { describe, expect, test } from 'bun:test'; import type { CreatePatchOptionsNonabortable } from 'diff'; import type { FileDiffMetadata, HunkExpansionRegion } from '../src/types'; +import { + joinLines, + lineAt, + linesToArray, + plainLines, +} from '../src/utils/diffLines'; import { applySessionChangedLines, captureExpansionAnchors, @@ -17,6 +23,28 @@ import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { getTrailingContextRangeSize } from '../src/utils/virtualDiffLayout'; import { verifyFileDiffHunkValues } from './testUtils'; +// These fixtures stand in for an attached editor, which only ever edits the +// addition side through its plain-string form. Read the side out, apply the +// edit, and hand it back as a plain `DiffLines` the way the renderer does. +function editAdditionLines( + diff: FileDiffMetadata, + edit: (lines: string[]) => void +): void { + const lines = linesToArray(diff.additionLines); + edit(lines); + diff.additionLines = plainLines(lines); +} + +function setAdditionLine( + diff: FileDiffMetadata, + index: number, + text: string +): void { + editAdditionLines(diff, (lines) => { + lines[index] = text; + }); +} + function makeLines( count: number, edits: Record = {} @@ -87,12 +115,12 @@ function pairingProjection(diff: FileDiffMetadata) { deletionText: deletionIndex == null ? undefined - : diff.deletionLines[deletionIndex], + : lineAt(diff.deletionLines, deletionIndex), additionIndex, additionText: additionIndex == null ? undefined - : diff.additionLines[additionIndex], + : lineAt(diff.additionLines, additionIndex), }); } } @@ -105,8 +133,15 @@ function expectPairingParity( parseDiffOptions?: CreatePatchOptionsNonabortable ): void { const expected = parseDiffFromFile( - { name: diff.prevName ?? diff.name, contents: diff.deletionLines.join('') }, - { name: diff.name, contents: diff.additionLines.join(''), lang: diff.lang }, + { + name: diff.prevName ?? diff.name, + contents: joinLines(diff.deletionLines), + }, + { + name: diff.name, + contents: joinLines(diff.additionLines), + lang: diff.lang, + }, parseDiffOptions ); expect(pairingProjection(diff)).toEqual(pairingProjection(expected)); @@ -114,24 +149,32 @@ function expectPairingParity( } describe('normalizeEditorLines', () => { + const normalized = (lines: string[]) => + linesToArray(normalizeEditorLines(plainLines(lines))); + test('drops only the phantom trailing empty line', () => { - expect(normalizeEditorLines(['a\n', 'b\n', ''])).toEqual(['a\n', 'b\n']); - expect(normalizeEditorLines(['a\n', 'b'])).toEqual(['a\n', 'b']); - expect(normalizeEditorLines([''])).toEqual(['']); + expect(normalized(['a\n', 'b\n', ''])).toEqual(['a\n', 'b\n']); + expect(normalized(['a\n', 'b'])).toEqual(['a\n', 'b']); + expect(normalized([''])).toEqual(['']); }); }); describe('findDivergenceCore', () => { + const divergence = (deletions: string[], additions: string[]) => + findDivergenceCore(plainLines(deletions), plainLines(additions)); + test('finds replacement, insertion, and the identical case', () => { - expect( - findDivergenceCore(['a\n', 'b\n', 'c\n'], ['a\n', 'x\n', 'c\n']) - ).toEqual({ start: 1, deletionEnd: 2, additionEnd: 2 }); - expect(findDivergenceCore(['a\n', 'c\n'], ['a\n', 'b\n', 'c\n'])).toEqual({ + expect(divergence(['a\n', 'b\n', 'c\n'], ['a\n', 'x\n', 'c\n'])).toEqual({ + start: 1, + deletionEnd: 2, + additionEnd: 2, + }); + expect(divergence(['a\n', 'c\n'], ['a\n', 'b\n', 'c\n'])).toEqual({ start: 1, deletionEnd: 1, additionEnd: 2, }); - expect(findDivergenceCore(['a\n'], ['a\n'])).toBeUndefined(); + expect(divergence(['a\n'], ['a\n'])).toBeUndefined(); }); }); @@ -139,7 +182,7 @@ describe('rebuildSessionHunks', () => { test('derives downstream coordinates without moving old-side boundaries', () => { const diff = makeDiff(); const before = oldBounds(diff); - diff.additionLines.splice(3, 0, 'inserted\n'); + editAdditionLines(diff, (lines) => lines.splice(3, 0, 'inserted\n')); const change = rebuildSessionHunks(diff); @@ -157,7 +200,7 @@ describe('rebuildSessionHunks', () => { const diff = makeDiff(); const before = hunkBounds(diff); const rowsBefore = countRenderedRows(diff); - diff.additionLines[2] = 'l3\n'; + setAdditionLine(diff, 2, 'l3\n'); expect(rebuildSessionHunks(diff)).toBeUndefined(); @@ -177,8 +220,8 @@ describe('rebuildSessionHunks', () => { test('synthesizes separate regions for separate canonical blocks in one gap', () => { const diff = makeDiff(); const before = oldBounds(diff); - diff.additionLines[10] = 'replaced a\n'; - diff.additionLines[13] = 'replaced b\n'; + setAdditionLine(diff, 10, 'replaced a\n'); + setAdditionLine(diff, 13, 'replaced b\n'); const change = rebuildSessionHunks(diff); @@ -199,7 +242,9 @@ describe('rebuildSessionHunks', () => { test('merges regions only when a canonical block crosses their gap', () => { const diff = makeDiff(); const before = oldBounds(diff); - diff.additionLines.splice(5, 16, 'one bridge\n', 'two bridge\n'); + editAdditionLines(diff, (lines) => + lines.splice(5, 16, 'one bridge\n', 'two bridge\n') + ); const change = rebuildSessionHunks(diff); @@ -238,15 +283,15 @@ describe('rebuildSessionHunks', () => { { name: 'repeated.tsx', contents: initialContents } ); - diff.additionLines.splice(4, 0, '\n'); + editAdditionLines(diff, (lines) => lines.splice(4, 0, '\n')); rebuildSessionHunks(diff); expectPairingParity(diff); - diff.additionLines.splice(6, 1); + editAdditionLines(diff, (lines) => lines.splice(6, 1)); rebuildSessionHunks(diff); expectPairingParity(diff); - diff.additionLines[3] = '\n'; + setAdditionLine(diff, 3, '\n'); rebuildSessionHunks(diff); expectPairingParity(diff); }); @@ -275,7 +320,7 @@ describe('rebuildSessionHunks', () => { { name: 'boundary.ts', contents: oldLines.join('') }, { name: 'boundary.ts', contents: initialLines.join('') } ); - diff.additionLines.splice(1, 0, 'y\n'); + editAdditionLines(diff, (lines) => lines.splice(1, 0, 'y\n')); rebuildSessionHunks(diff); @@ -289,7 +334,7 @@ describe('rebuildSessionHunks', () => { { name: 't.ts', contents: oldContents }, { name: 't.ts', contents: newContents } ); - diff.additionLines.splice(3, 0, '\n'); + editAdditionLines(diff, (lines) => lines.splice(3, 0, '\n')); rebuildSessionHunks(diff); @@ -302,7 +347,7 @@ describe('rebuildSessionHunks', () => { { name: 'plain.ts', contents }, { name: 'plain.ts', contents } ); - diff.additionLines.splice(4, 0, 'inserted\n'); + editAdditionLines(diff, (lines) => lines.splice(4, 0, 'inserted\n')); expect(rebuildSessionHunks(diff)).toBeDefined(); @@ -314,7 +359,7 @@ describe('rebuildSessionHunks', () => { test('anchors a pure trailing deletion so the session hunk stays renderable', () => { const diff = makeDiff(); - diff.additionLines.splice(29, 1); + editAdditionLines(diff, (lines) => lines.splice(29, 1)); rebuildSessionHunks(diff); @@ -330,9 +375,9 @@ describe('rebuildSessionHunks', () => { test('keeps trailing context valid when a persisted region becomes deletion-only', () => { const diff = makeDiff(); - diff.additionLines[27] = 'temporary region\n'; + setAdditionLine(diff, 27, 'temporary region\n'); rebuildSessionHunks(diff); - diff.additionLines.splice(27, 1); + editAdditionLines(diff, (lines) => lines.splice(27, 1)); rebuildSessionHunks(diff); @@ -355,7 +400,7 @@ describe('rebuildSessionHunks', () => { { name: 'blank-run.ts', contents: oldLines.join('') }, { name: 'blank-run.ts', contents: initialLines.join('') } ); - diff.additionLines.splice(1, 0, '\n'); + editAdditionLines(diff, (lines) => lines.splice(1, 0, '\n')); rebuildSessionHunks(diff); @@ -382,8 +427,8 @@ describe('applySessionChangedLines', () => { const firstBefore = diff.hunks[0]; const secondBefore = diff.hunks[1]; const boundsBefore = oldBounds(diff); - const previousLine = diff.additionLines[2]; - diff.additionLines[2] = 'retyped 3\n'; + const previousLine = lineAt(diff.additionLines, 2); + setAdditionLine(diff, 2, 'retyped 3\n'); const change = applySessionChangedLines( diff, @@ -402,7 +447,7 @@ describe('applySessionChangedLines', () => { test('routes a gap edit through the structural rebuild', () => { const diff = makeDiff(); - diff.additionLines[11] = 'replaced in gap\n'; + setAdditionLine(diff, 11, 'replaced in gap\n'); const change = applySessionChangedLines(diff, [11]); @@ -414,8 +459,8 @@ describe('applySessionChangedLines', () => { test('routes edits in several regions through one rebuild without merging them', () => { const diff = makeDiff(); - diff.additionLines[2] = 'retyped 3\n'; - diff.additionLines[19] = 'retyped 20\n'; + setAdditionLine(diff, 2, 'retyped 3\n'); + setAdditionLine(diff, 19, 'retyped 20\n'); const change = applySessionChangedLines(diff, [2, 19]); @@ -453,7 +498,7 @@ describe('applySessionChangedLines', () => { { name: 'edge.ts', contents: oldLines.join('') }, { name: 'edge.ts', contents: initialLines.join('') } ); - diff.additionLines[6] = 'x!\n'; + setAdditionLine(diff, 6, 'x!\n'); applySessionChangedLines(diff, [6]); @@ -469,7 +514,7 @@ describe('applySessionChangedLines', () => { { name: 'blank-edge.ts', contents: initialLines.join('') } ); const hunksBefore = diff.hunks; - diff.additionLines[1] = '\n'; + setAdditionLine(diff, 1, '\n'); applySessionChangedLines(diff, [1]); @@ -484,8 +529,8 @@ describe('applySessionChangedLines', () => { { name: 'context-zero.ts', contents: 'a\n' }, parseDiffOptions ); - const previousLine = diff.additionLines[0]; - diff.additionLines[0] = '\n'; + const previousLine = lineAt(diff.additionLines, 0); + setAdditionLine(diff, 0, '\n'); applySessionChangedLines( diff, @@ -503,8 +548,8 @@ describe('applySessionChangedLines', () => { { name: 'realigned.ts', contents: 'junk\ntarget!\n' } ); const hunksBefore = diff.hunks; - const previousLine = diff.additionLines[1]; - diff.additionLines[1] = 'other\n'; + const previousLine = lineAt(diff.additionLines, 1); + setAdditionLine(diff, 1, 'other\n'); applySessionChangedLines( diff, @@ -526,8 +571,8 @@ describe('applySessionChangedLines', () => { } ); const hunksBefore = diff.hunks; - const previousLine = diff.additionLines[3]; - diff.additionLines[3] = 'other\n'; + const previousLine = lineAt(diff.additionLines, 3); + setAdditionLine(diff, 3, 'other\n'); applySessionChangedLines( diff, @@ -623,7 +668,7 @@ describe('expansion anchors across the exit recompute', () => { [1, { fromStart: 2, fromEnd: 3 }], ]); const anchors = captureExpansionAnchors(diff, expandedHunks, THRESHOLD); - diff.additionLines.splice(3, 0, 'inserted\n'); + editAdditionLines(diff, (lines) => lines.splice(3, 0, 'inserted\n')); rebuildSessionHunks(diff); finishEditSessionForDiff(diff); @@ -639,7 +684,7 @@ describe('expansion anchors across the exit recompute', () => { [1, { fromStart: 2, fromEnd: 3 }], ]); const anchors = captureExpansionAnchors(diff, expandedHunks, THRESHOLD); - diff.additionLines[2] = 'l3\n'; + setAdditionLine(diff, 2, 'l3\n'); rebuildSessionHunks(diff); finishEditSessionForDiff(diff); @@ -655,7 +700,7 @@ describe('expansion anchors across the exit recompute', () => { [diff.hunks.length, { fromStart: 4, fromEnd: 0 }], ]); const anchors = captureExpansionAnchors(diff, expandedHunks, THRESHOLD); - diff.additionLines[2] = 'changed differently\n'; + setAdditionLine(diff, 2, 'changed differently\n'); rebuildSessionHunks(diff); finishEditSessionForDiff(diff); @@ -668,9 +713,9 @@ describe('expansion anchors across the exit recompute', () => { describe('finishEditSessionForDiff', () => { test('a dirty session recomputes to the plain edit pipeline result', () => { const diff = makeDiff(); - diff.additionLines[2] = 'l3\n'; + setAdditionLine(diff, 2, 'l3\n'); rebuildSessionHunks(diff); - diff.additionLines[11] = 'gap edit\n'; + setAdditionLine(diff, 11, 'gap edit\n'); applySessionChangedLines(diff, [11]); expect(diff.hunks).toHaveLength(3); @@ -678,8 +723,8 @@ describe('finishEditSessionForDiff', () => { expect(diff.editSessionDirty).toBeUndefined(); const expected = parseDiffFromFile( - { name: 'a.ts', contents: diff.deletionLines.join('') }, - { name: 'a.ts', contents: diff.additionLines.join('') } + { name: 'a.ts', contents: joinLines(diff.deletionLines) }, + { name: 'a.ts', contents: joinLines(diff.additionLines) } ); expect(diff.hunks).toEqual(expected.hunks); expect(diff.splitLineCount).toBe(expected.splitLineCount); diff --git a/packages/diffs/test/hydratePartialDiff.test.ts b/packages/diffs/test/hydratePartialDiff.test.ts index 1ef72bd79..dfddac8b2 100644 --- a/packages/diffs/test/hydratePartialDiff.test.ts +++ b/packages/diffs/test/hydratePartialDiff.test.ts @@ -6,6 +6,7 @@ import type { FileDiffLoadedFiles, FileDiffMetadata, } from '../src/types'; +import { linesToArray } from '../src/utils/diffLines'; import { hydratePartialDiff } from '../src/utils/hydratePartialDiff'; import { parsePatchFiles } from '../src/utils/parsePatchFiles'; import { splitFileContents } from '../src/utils/splitFileContents'; @@ -143,8 +144,8 @@ describe('hydratePartialDiff', () => { expect(hydrated.prevObjectId).toBe('1111111'); expect(hydrated.newObjectId).toBe('2222222'); expect(hydrated.cacheKey).toBe('partial-cache:hydrated'); - expect(hydrated.additionLines).toEqual(newFileLines); - expect(hydrated.deletionLines).toEqual(oldFileLines); + expect(linesToArray(hydrated.additionLines)).toEqual(newFileLines); + expect(linesToArray(hydrated.deletionLines)).toEqual(oldFileLines); expect(hydrated.hunks.map((hunk) => hunk.hunkSpecs)).toEqual( partialHunkSpecs ); @@ -239,19 +240,19 @@ describe('hydratePartialDiff', () => { expect(hydrated).not.toBe(partial); expect(hydrated.isPartial).toBe(false); - expect(hydrated.additionLines).toEqual([ + expect(linesToArray(hydrated.additionLines)).toEqual([ 'keep 1\n', 'new value\n', 'keep 3\n', ]); - expect(hydrated.deletionLines).toEqual([ + expect(linesToArray(hydrated.deletionLines)).toEqual([ 'keep 1\n', 'old value\n', 'keep 3\n', ]); expect(partial.isPartial).toBe(true); - expect(partial.additionLines).toEqual(['new value\n']); - expect(partial.deletionLines).toEqual(['old value\n']); + expect(linesToArray(partial.additionLines)).toEqual(['new value\n']); + expect(linesToArray(partial.deletionLines)).toEqual(['old value\n']); }); test('falls back to a hydrated cache segment when loaded files are unkeyed', () => { @@ -332,8 +333,8 @@ describe('hydratePartialDiff', () => { expect(hydrated.prevName).toBe(oldFile.name); expect(hydrated.name).toBe(newFile.name); expect(hydrated.isPartial).toBe(false); - expect(hydrated.additionLines).toEqual(newFileLines); - expect(hydrated.deletionLines).toEqual(oldFileLines); + expect(linesToArray(hydrated.additionLines)).toEqual(newFileLines); + expect(linesToArray(hydrated.deletionLines)).toEqual(oldFileLines); expect(verifyHunkLineValues(hydrated)).toEqual([]); }); @@ -405,8 +406,8 @@ describe('hydratePartialDiff', () => { expect(hydrated.hunks).toEqual([]); expect(hydrated.splitLineCount).toBe(0); expect(hydrated.unifiedLineCount).toBe(0); - expect(hydrated.additionLines).toEqual(newFileLines); - expect(hydrated.deletionLines).toEqual(newFileLines); + expect(linesToArray(hydrated.additionLines)).toEqual(newFileLines); + expect(linesToArray(hydrated.deletionLines)).toEqual(newFileLines); expect(hydrated.cacheKey).toBe('partial-rename-cache:hydrated'); }); }); diff --git a/packages/diffs/test/hydration.test.ts b/packages/diffs/test/hydration.test.ts index c4e3621b9..9ef061126 100644 --- a/packages/diffs/test/hydration.test.ts +++ b/packages/diffs/test/hydration.test.ts @@ -21,6 +21,7 @@ import type { Virtualizer } from '../src/components/Virtualizer'; import { DEFAULT_VIRTUAL_FILE_METRICS } from '../src/constants'; import type { FileContents, FileDiffMetadata } from '../src/types'; import { linesFromFileContents } from '../src/utils/computeFileOffsets'; +import { plainLines } from '../src/utils/diffLines'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { assertDefined } from './testUtils'; @@ -257,8 +258,8 @@ const fileDiff: FileDiffMetadata = { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: [], - additionLines: [], + deletionLines: plainLines([]), + additionLines: plainLines([]), }; describe('collapsed hydration', () => { diff --git a/packages/diffs/test/iterateOverDiff.test.ts b/packages/diffs/test/iterateOverDiff.test.ts index 8b11ecc8a..a2a7c6f98 100644 --- a/packages/diffs/test/iterateOverDiff.test.ts +++ b/packages/diffs/test/iterateOverDiff.test.ts @@ -3,6 +3,7 @@ import { describe, expect, test } from 'bun:test'; import { parseDiffFromFile } from '../src'; import { DEFAULT_COLLAPSED_CONTEXT_THRESHOLD } from '../src/constants'; import type { ChangeContent, FileDiffMetadata, Hunk } from '../src/types'; +import { type DiffLines, lineAt, plainLines } from '../src/utils/diffLines'; import { type DiffLineCallbackProps, type DiffLineMetadata, @@ -516,7 +517,7 @@ describe('iterateOverDiff', () => { { lineIndex: row.additionLine.lineIndex, lineNumber: row.additionLine.lineNumber, - text: deletionDiff.additionLines[row.additionLine.lineIndex], + text: lineAt(deletionDiff.additionLines, row.additionLine.lineIndex), }, ]; }); @@ -893,6 +894,8 @@ function getHunkContentCounts(hunkContent: Hunk['hunkContent']): { }; } -function createLines(count: number): string[] { - return Array.from({ length: count }, (_, index) => `line ${index}\n`); +function createLines(count: number): DiffLines { + return plainLines( + Array.from({ length: count }, (_, index) => `line ${index}\n`) + ); } diff --git a/packages/diffs/test/onPostRenderPhase.test.ts b/packages/diffs/test/onPostRenderPhase.test.ts index 0b383236f..6219207da 100644 --- a/packages/diffs/test/onPostRenderPhase.test.ts +++ b/packages/diffs/test/onPostRenderPhase.test.ts @@ -11,6 +11,7 @@ import type { FileDiffMetadata, PostRenderPhase, } from '../src/types'; +import { plainLines } from '../src/utils/diffLines'; import { createRoot, dispatchScroll, @@ -91,8 +92,8 @@ const fileDiff: FileDiffMetadata = { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: [], - additionLines: [], + deletionLines: plainLines([]), + additionLines: plainLines([]), }; describe('onPostRender phases', () => { diff --git a/packages/diffs/test/parseDiffFromFile.test.ts b/packages/diffs/test/parseDiffFromFile.test.ts index b083eecbe..b546a7fd6 100644 --- a/packages/diffs/test/parseDiffFromFile.test.ts +++ b/packages/diffs/test/parseDiffFromFile.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import type { FileContents } from '../src/types'; +import { linesToArray } from '../src/utils/diffLines'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { splitFileContents } from '../src/utils/splitFileContents'; import { fileNew, fileOld } from './mocks'; @@ -127,8 +128,10 @@ describe('parseDiffFromFile', () => { expect(result.prevName).toBeUndefined(); expect(result.lang).toBe('typescript'); expect(result.isPartial).toBe(false); - expect(result.deletionLines).toEqual([]); - expect(result.additionLines).toEqual(splitFileContents(newFile.contents)); + expect(linesToArray(result.deletionLines)).toEqual([]); + expect(linesToArray(result.additionLines)).toEqual( + splitFileContents(newFile.contents) + ); expect(result.cacheKey).toBe('created-cache'); expect(verifyHunkLineValues(result)).toEqual([]); }); @@ -148,8 +151,10 @@ describe('parseDiffFromFile', () => { expect(result.prevName).toBeUndefined(); expect(result.lang).toBe('typescript'); expect(result.isPartial).toBe(false); - expect(result.deletionLines).toEqual(splitFileContents(oldFile.contents)); - expect(result.additionLines).toEqual([]); + expect(linesToArray(result.deletionLines)).toEqual( + splitFileContents(oldFile.contents) + ); + expect(linesToArray(result.additionLines)).toEqual([]); expect(result.cacheKey).toBe('deleted-cache'); expect(verifyHunkLineValues(result)).toEqual([]); }); diff --git a/packages/diffs/test/parseMergeConflictDiffFromFile.test.ts b/packages/diffs/test/parseMergeConflictDiffFromFile.test.ts index 51ba55f23..46779604b 100644 --- a/packages/diffs/test/parseMergeConflictDiffFromFile.test.ts +++ b/packages/diffs/test/parseMergeConflictDiffFromFile.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import { readFileSync } from 'fs'; import { resolve } from 'path'; +import { linesToArray } from '../src/utils/diffLines'; import { parseMergeConflictDiffFromFile } from '../src/utils/parseMergeConflictDiffFromFile'; import { splitFileContents } from '../src/utils/splitFileContents'; import { hunkDigest, verifyHunkLineValues } from './testUtils'; @@ -42,10 +43,10 @@ describe('parseMergeConflictDiffFromFile', () => { expect(incomingFile.contents).not.toContain('>>>>>>> feature\n'); expect(incomingFile.contents).not.toContain('const ttl = 12;\n'); - expect(fileDiff.deletionLines).toEqual( + expect(linesToArray(fileDiff.deletionLines)).toEqual( splitFileContents(currentFile.contents) ); - expect(fileDiff.additionLines).toEqual( + expect(linesToArray(fileDiff.additionLines)).toEqual( splitFileContents(incomingFile.contents) ); @@ -214,10 +215,10 @@ describe('parseMergeConflictDiffFromFile', () => { expect(verifyHunkLineValues(fileDiff)).toEqual([]); // The diff sides are exactly the conflict-free current/incoming texts - expect(fileDiff.deletionLines).toEqual( + expect(linesToArray(fileDiff.deletionLines)).toEqual( splitFileContents(currentFile.contents) ); - expect(fileDiff.additionLines).toEqual( + expect(linesToArray(fileDiff.additionLines)).toEqual( splitFileContents(incomingFile.contents) ); expect(currentFile.contents).not.toMatch(/^<{7} /m); diff --git a/packages/diffs/test/parsePatchFiles.test.ts b/packages/diffs/test/parsePatchFiles.test.ts index 0d485eb3b..11844f959 100644 --- a/packages/diffs/test/parsePatchFiles.test.ts +++ b/packages/diffs/test/parsePatchFiles.test.ts @@ -13,6 +13,7 @@ import { assertDefined, countRenderedLines, countSplitRows, + linesOf, patchDigest, verifyPatchHunkValues, } from './testUtils'; @@ -174,14 +175,14 @@ describe('parsePatchFiles', () => { const file = result[0]?.files[0]; expect(result[0]?.files).toHaveLength(1); expect(file?.name).toBe('sql/test.sql'); - expect(file?.deletionLines).toEqual([ + expect(linesOf(file?.deletionLines)).toEqual([ '-- This is a test sql file\n', '-- This is an sql comment\n', '\n', 'CREATE TABLE users (\n', 'id BIGSERIAL PRIMARY KEY,\n', ]); - expect(file?.additionLines).toEqual([ + expect(linesOf(file?.additionLines)).toEqual([ '-- This is a test sql file\n', '\n', 'CREATE TABLE users (\n', @@ -205,8 +206,8 @@ describe('parsePatchFiles', () => { const file = result[0]?.files[0]; expect(result[0]?.files).toHaveLength(1); expect(file?.name).toBe('markers.txt'); - expect(file?.deletionLines).toEqual(['-- old marker\n']); - expect(file?.additionLines).toEqual(['++ new marker\n']); + expect(linesOf(file?.deletionLines)).toEqual(['-- old marker\n']); + expect(linesOf(file?.additionLines)).toEqual(['++ new marker\n']); }); test('preserves leading BOM characters in parsed hunk lines', () => { @@ -223,8 +224,8 @@ describe('parsePatchFiles', () => { ); const file = result[0]?.files[0]; - expect(file?.deletionLines[0]).toBe('\uFEFFold\n'); - expect(file?.additionLines[0]).toBe('\uFEFFnew\n'); + expect(linesOf(file?.deletionLines)?.[0]).toBe('\uFEFFold\n'); + expect(linesOf(file?.additionLines)?.[0]).toBe('\uFEFFnew\n'); }); test('preserves lone surrogate characters in parsed hunk lines', () => { @@ -241,8 +242,8 @@ describe('parsePatchFiles', () => { ); const file = result[0]?.files[0]; - expect(file?.deletionLines[0]).toBe('old\ud800\n'); - expect(file?.additionLines[0]).toBe('new\ud800\n'); + expect(linesOf(file?.deletionLines)?.[0]).toBe('old\ud800\n'); + expect(linesOf(file?.additionLines)?.[0]).toBe('new\ud800\n'); }); test('parses quoted git diff headers with escaped file names', () => { diff --git a/packages/diffs/test/realignChangeContent.test.ts b/packages/diffs/test/realignChangeContent.test.ts index f5141a89e..95ac1fd8c 100644 --- a/packages/diffs/test/realignChangeContent.test.ts +++ b/packages/diffs/test/realignChangeContent.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import type { FileDiffMetadata } from '../src/types'; +import { lineAt } from '../src/utils/diffLines'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; // parseDiffFromFile pairs change-block lines by similarity: the diff library @@ -155,7 +156,7 @@ describe('parseDiffFromFile change-block realignment', () => { expect(block.additionLineIndex - block.deletionLineIndex).toBe(2); } } - expect(diff.additionLines[4]).toBe('// note\n'); + expect(lineAt(diff.additionLines, 4)).toBe('// note\n'); }); }); diff --git a/packages/diffs/test/testUtils.ts b/packages/diffs/test/testUtils.ts index 944e67c4f..c57ce83df 100644 --- a/packages/diffs/test/testUtils.ts +++ b/packages/diffs/test/testUtils.ts @@ -3,9 +3,20 @@ import type { ElementContent, Element as HASTElement } from 'hast'; import { DEFAULT_COLLAPSED_CONTEXT_THRESHOLD } from '../src/constants'; import type { HunksRenderResult } from '../src/renderers/DiffHunksRenderer'; import type { FileDiffMetadata, ParsedPatch } from '../src/types'; +import { type DiffLines, linesToArray } from '../src/utils/diffLines'; // Assertion helpers +/** + * `linesToArray` for the assertions that read a side off an optional file diff + * (`expect(file?.additionLines)`). Passing the missing case through as + * `undefined` keeps those assertions failing on a missing file exactly as they + * did when the sides were plain `string[]`. + */ +export function linesOf(lines: DiffLines | undefined): string[] | undefined { + return lines == null ? undefined : linesToArray(lines); +} + export function assertDefined( value: T | undefined | null, message: string diff --git a/packages/diffs/test/updateDiffHunks.test.ts b/packages/diffs/test/updateDiffHunks.test.ts index 8ba62441e..76776c59c 100644 --- a/packages/diffs/test/updateDiffHunks.test.ts +++ b/packages/diffs/test/updateDiffHunks.test.ts @@ -2,6 +2,12 @@ import { describe, expect, test } from 'bun:test'; import type { FileDiffMetadata, Hunk } from '../src/types'; import { cleanLastNewline } from '../src/utils/cleanLastNewline'; +import { + joinLines, + lineAt, + linesToArray, + plainLines, +} from '../src/utils/diffLines'; import { iterateOverDiff } from '../src/utils/iterateOverDiff'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { @@ -68,7 +74,7 @@ function findAdditionLineIndex( diff: FileDiffMetadata, lineText: string ): number { - const line = diff.additionLines.findIndex( + const line = linesToArray(diff.additionLines).findIndex( (value) => cleanLastNewline(value) === lineText ); if (line < 0) { @@ -95,19 +101,22 @@ function setAdditionLineText( line: number, lineText: string ): void { - const prevLine = diff.additionLines[line]; + const prevLine = lineAt(diff.additionLines, line); if (prevLine == null) { throw new Error(`Missing addition line ${line}`); } + // Edit through the plain form and hand the side back, like the editor does + const lines = linesToArray(diff.additionLines); if (prevLine.endsWith('\r\n')) { - diff.additionLines[line] = `${lineText}\r\n`; + lines[line] = `${lineText}\r\n`; } else if (prevLine.endsWith('\n')) { - diff.additionLines[line] = `${lineText}\n`; + lines[line] = `${lineText}\n`; } else if (prevLine.endsWith('\r')) { - diff.additionLines[line] = `${lineText}\r`; + lines[line] = `${lineText}\r`; } else { - diff.additionLines[line] = lineText; + lines[line] = lineText; } + diff.additionLines = plainLines(lines); } function cloneDiff(diff: FileDiffMetadata): FileDiffMetadata { @@ -194,7 +203,7 @@ describe('updateDiffHunks', () => { const diff = runUpdateDiffHunksEdit(base, line, 'line 10 replace newer'); expect(diff.hunks[hunkIndex]).toEqual(hunkBefore); - expect(cleanLastNewline(diff.additionLines[line])).toBe( + expect(cleanLastNewline(lineAt(diff.additionLines, line))).toBe( 'line 10 replace newer' ); }); @@ -209,11 +218,11 @@ describe('updateDiffHunks', () => { const fromParse = parseDiffFromFile( { name: diff.prevName ?? diff.name, - contents: diff.deletionLines.join(''), + contents: joinLines(diff.deletionLines), }, { name: diff.name, - contents: diff.additionLines.join(''), + contents: joinLines(diff.additionLines), lang: diff.lang, }, PARSE_OPTIONS @@ -241,7 +250,7 @@ describe('updateDiffHunks', () => { ); expect(diff.hunks[hunkIndex]).toEqual(hunkBefore); - expect(cleanLastNewline(diff.additionLines[line])).toBe( + expect(cleanLastNewline(lineAt(diff.additionLines, line))).toBe( 'line 10 replace newer' ); }); @@ -268,7 +277,7 @@ describe('updateDiffHunks', () => { // Simulate deferred tokenization growing additionLines for an editor-only // trailing line without updating hunk metadata. - diff.additionLines.push(''); + diff.additionLines = plainLines([...linesToArray(diff.additionLines), '']); expect(hasTrailingContextMismatch(diff)).toBe(true); @@ -292,11 +301,11 @@ describe('updateDiffHunks', () => { // Mirrors edit mode after deleting all but one matching line in a longer // file and pressing Enter: the editor has a final logical empty line that // patch-style splitting would normally drop. - diff.additionLines = ['kept\n', '']; + diff.additionLines = plainLines(['kept\n', '']); const recomputed = recomputeDiffHunksForEdit(diff, { context: 3 }); - expect(recomputed.additionLines).toEqual(['kept\n', '']); + expect(linesToArray(recomputed.additionLines)).toEqual(['kept\n', '']); expect(recomputed.splitLineCount).toBe(3); expect(recomputed.unifiedLineCount).toBe(4); expect(recomputed.hunks[0]?.hunkContent).toEqual([ @@ -356,11 +365,11 @@ describe('updateDiffHunks', () => { // Mirrors select-all, type "a", then press Enter: the editor has a second // logical row for the trailing blank line before tokenization sees content // on that row. - diff.additionLines = ['a\n', '']; + diff.additionLines = plainLines(['a\n', '']); const recomputed = recomputeDiffHunksForEdit(diff, { context: 3 }); - expect(recomputed.additionLines).toEqual(['a\n', '']); + expect(linesToArray(recomputed.additionLines)).toEqual(['a\n', '']); expect(recomputed.splitLineCount).toBe(4); expect(recomputed.unifiedLineCount).toBe(6); expect(recomputed.hunks[0]?.hunkContent).toEqual([ @@ -410,11 +419,11 @@ describe('updateDiffHunks', () => { // The editor exposes the final logical empty row for a file ending in a // newline. That row belongs to document state, not to diff hunk metadata. - diff.additionLines = ['line 1\n', 'add me\n', 'line 3\n', '']; + diff.additionLines = plainLines(['line 1\n', 'add me\n', 'line 3\n', '']); const recomputed = recomputeDiffHunksForEdit(diff, { context: 3 }); - expect(recomputed.additionLines).toEqual([ + expect(linesToArray(recomputed.additionLines)).toEqual([ 'line 1\n', 'add me\n', 'line 3\n', diff --git a/packages/diffs/test/virtualDiffLayout.test.ts b/packages/diffs/test/virtualDiffLayout.test.ts index 03a613dd9..08802ffce 100644 --- a/packages/diffs/test/virtualDiffLayout.test.ts +++ b/packages/diffs/test/virtualDiffLayout.test.ts @@ -6,6 +6,7 @@ import type { HunkSeparators, VirtualFileMetrics, } from '../src/types'; +import { type DiffLines, plainLines } from '../src/utils/diffLines'; import { getExpandedRegion, getLeadingHunkSeparatorLayout, @@ -297,6 +298,8 @@ function createTrailingDiff(trailingLineCount: number): FileDiffMetadata { }; } -function createLines(count: number): string[] { - return Array.from({ length: count }, (_, index) => `line ${index}\n`); +function createLines(count: number): DiffLines { + return plainLines( + Array.from({ length: count }, (_, index) => `line ${index}\n`) + ); } diff --git a/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts b/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts index a501dea13..fe2e84a23 100644 --- a/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts +++ b/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts @@ -13,6 +13,7 @@ import type { RenderWindow, VirtualFileMetrics, } from '../src/types'; +import { linesToArray, plainLines } from '../src/utils/diffLines'; import { iterateOverDiff } from '../src/utils/iterateOverDiff'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { recomputeDiffHunks } from '../src/utils/updateDiffHunks'; @@ -157,8 +158,8 @@ function createHugeSingleBlockDiff(lineCount: number): FileDiffMetadata { splitLineCount: lineCount, unifiedLineCount: lineCount, isPartial: true, - deletionLines: [], - additionLines: [], + deletionLines: plainLines([]), + additionLines: plainLines([]), }; } @@ -171,8 +172,8 @@ function createNoHunkDiff(): FileDiffMetadata { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: [], - additionLines: [], + deletionLines: plainLines([]), + additionLines: plainLines([]), }; } @@ -968,11 +969,11 @@ describe('VirtualizedFileDiff estimated height cache', () => { fileDiff.additionLines.length ); - const edited = [...fileDiff.additionLines]; + const edited = linesToArray(fileDiff.additionLines); for (let index = 0; index < 10; index++) { edited.push(`new line ${index}\n`); } - fileDiff = { ...fileDiff, additionLines: edited }; + fileDiff = { ...fileDiff, additionLines: plainLines(edited) }; Object.assign(fileDiff, recomputeDiffHunks(fileDiff)); inspect(instance).cache.totalLines = fileDiff.additionLines.length - 10; From ebe718a8893b1eefe211d423b21870a770678b5c Mon Sep 17 00:00:00 2001 From: clemg Date: Tue, 16 Jun 2026 11:50:59 +0200 Subject: [PATCH 2/5] test(diffs): cover the byte-arena DiffLines model and edge cases Adds diffLines.test.ts (arena round-trip, multibyte, emoji-keeps-arena, lone-surrogate fallback, BOM, offset-width, plainLines, joinLines, isWellFormed) and a withPlainLines snapshot converter so the existing parsed-model snapshots assert byte-identical line content. The audited tests' direct string[] reads move to the DiffLines helpers (lineAt / linesToArray / plainLines), and the updateDiffHunks fixtures edit lines through the plain form like the editor does. --- packages/diffs/test/annotations.test.ts | 6 +- .../diffs/test/diffAcceptRejectHunk.test.ts | 34 +++-- packages/diffs/test/diffLines.test.ts | 135 ++++++++++++++++++ packages/diffs/test/hydration.test.ts | 6 +- packages/diffs/test/onPostRenderPhase.test.ts | 6 +- packages/diffs/test/parsePatchFiles.test.ts | 31 ++-- packages/diffs/test/testUtils.ts | 21 +++ packages/diffs/test/updateDiffHunks.test.ts | 6 +- ...irtualizedFileDiffEstimatedHeights.test.ts | 15 +- 9 files changed, 220 insertions(+), 40 deletions(-) create mode 100644 packages/diffs/test/diffLines.test.ts diff --git a/packages/diffs/test/annotations.test.ts b/packages/diffs/test/annotations.test.ts index 8fcd3924f..f7bfae98e 100644 --- a/packages/diffs/test/annotations.test.ts +++ b/packages/diffs/test/annotations.test.ts @@ -11,7 +11,7 @@ import type { FileDiffMetadata, LineTypes, } from '../src/types'; -import { plainLines } from '../src/utils/diffLines'; +import { EMPTY_DIFF_LINES } from '../src/utils/diffLines'; import { fileNew, fileOld } from './mocks'; import { annotationProjection, @@ -54,8 +54,8 @@ function createNoHunkDiff(): FileDiffMetadata { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: plainLines([]), - additionLines: plainLines([]), + deletionLines: EMPTY_DIFF_LINES, + additionLines: EMPTY_DIFF_LINES, }; } diff --git a/packages/diffs/test/diffAcceptRejectHunk.test.ts b/packages/diffs/test/diffAcceptRejectHunk.test.ts index 79278b0f0..ab6a1a038 100644 --- a/packages/diffs/test/diffAcceptRejectHunk.test.ts +++ b/packages/diffs/test/diffAcceptRejectHunk.test.ts @@ -6,7 +6,7 @@ import type { FileDiffMetadata, } from '../src/types'; import { diffAcceptRejectHunk } from '../src/utils/diffAcceptRejectHunk'; -import { linesToArray } from '../src/utils/diffLines'; +import { type DiffLines, linesToArray } from '../src/utils/diffLines'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { parseMergeConflictDiffFromFile } from '../src/utils/parseMergeConflictDiffFromFile'; import { parsePatchFiles } from '../src/utils/parsePatchFiles'; @@ -32,6 +32,11 @@ interface HunkSnapshot { blocks: BlockSnapshot[]; } +// Read a sub-range of one side's lines as a plain string[] for comparison. +function sliceLines(lines: DiffLines, start: number, end: number): string[] { + return linesToArray(lines).slice(start, end); +} + // Create one realistic parsed diff with several hunk shapes. The tests use // this as the starting point, then check whether resolving one hunk leaves the // resulting diff metadata internally consistent. @@ -134,7 +139,8 @@ function snapshotBlock( if (content.type === 'context') { return { type: 'context', - lines: linesToArray(diff.additionLines).slice( + lines: sliceLines( + diff.additionLines, content.additionLineIndex, content.additionLineIndex + content.lines ), @@ -143,11 +149,13 @@ function snapshotBlock( return { type: 'change', - deletionLines: linesToArray(diff.deletionLines).slice( + deletionLines: sliceLines( + diff.deletionLines, content.deletionLineIndex, content.deletionLineIndex + content.deletions ), - additionLines: linesToArray(diff.additionLines).slice( + additionLines: sliceLines( + diff.additionLines, content.additionLineIndex, content.additionLineIndex + content.additions ), @@ -209,13 +217,15 @@ function assertUnresolvedHunkMatchesSnapshot( } expect( - linesToArray(diff.deletionLines).slice( + sliceLines( + diff.deletionLines, content.deletionLineIndex, content.deletionLineIndex + content.lines ) ).toEqual(expectedBlock.lines); expect( - linesToArray(diff.additionLines).slice( + sliceLines( + diff.additionLines, content.additionLineIndex, content.additionLineIndex + content.lines ) @@ -229,13 +239,15 @@ function assertUnresolvedHunkMatchesSnapshot( } expect( - linesToArray(diff.deletionLines).slice( + sliceLines( + diff.deletionLines, content.deletionLineIndex, content.deletionLineIndex + content.deletions ) ).toEqual(expectedBlock.deletionLines); expect( - linesToArray(diff.additionLines).slice( + sliceLines( + diff.additionLines, content.additionLineIndex, content.additionLineIndex + content.additions ) @@ -269,13 +281,15 @@ function assertResolvedHunkMatchesExpected( ) ).toBe(expectedLines.length); expect( - linesToArray(diff.deletionLines).slice( + sliceLines( + diff.deletionLines, hunk.deletionLineIndex, hunk.deletionLineIndex + expectedLines.length ) ).toEqual(expectedLines); expect( - linesToArray(diff.additionLines).slice( + sliceLines( + diff.additionLines, hunk.additionLineIndex, hunk.additionLineIndex + expectedLines.length ) diff --git a/packages/diffs/test/diffLines.test.ts b/packages/diffs/test/diffLines.test.ts new file mode 100644 index 000000000..cad512e68 --- /dev/null +++ b/packages/diffs/test/diffLines.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, test } from 'bun:test'; + +import { + type DiffLines, + EMPTY_DIFF_LINES, + finishLines, + isWellFormed, + joinLines, + lineAt, + linesToArray, + plainLines, +} from '../src/utils/diffLines'; + +// `DiffLines` is an arena-or-plain union, so a field assertion has to narrow +// first. These mirror the runtime `'lines' in value` check the module uses. +function arena(dl: DiffLines) { + if ('lines' in dl) { + throw new Error('expected an arena DiffLines, got the string fallback'); + } + return dl; +} + +function plain(dl: DiffLines) { + if (!('lines' in dl)) { + throw new Error('expected the string fallback, got an arena DiffLines'); + } + return dl; +} + +describe('diffLines', () => { + test('empty list seals to a valid empty arena', () => { + const dl = finishLines([]); + expect(dl.length).toBe(0); + expect('lines' in dl).toBe(false); // arena path + expect(lineAt(dl, 0)).toBeUndefined(); + }); + + test('ascii content round-trips through the byte arena', () => { + const lines = ['const x = 1;\n', 'return x;\n']; + const dl = finishLines(lines); + expect('lines' in dl).toBe(false); // arena path, not the string fallback + expect(lineAt(dl, 0)).toBe('const x = 1;\n'); + expect(linesToArray(dl)).toEqual(lines); + }); + + test('out-of-range reads return undefined', () => { + const dl = finishLines(['a']); + expect(lineAt(dl, -1)).toBeUndefined(); + expect(lineAt(dl, 1)).toBeUndefined(); + }); + + test('multibyte UTF-8 (accents, CJK) uses the arena and round-trips', () => { + const lines = ['cœur de pierre précieuse\n', '日本語のテスト\n']; + const dl = finishLines(lines); + expect('lines' in dl).toBe(false); + expect(linesToArray(dl)).toEqual(lines); + // Multibyte content takes more bytes than UTF-16 code units. + const charTotal = lines.reduce((n, l) => n + l.length, 0); + expect(arena(dl).bytes.length).toBeGreaterThan(charTotal); + }); + + test('valid surrogate pairs (emoji, astral) still use the arena', () => { + // Regression guard: a valid pair is well-formed and survives a UTF-8 + // round-trip, so it must NOT be forced onto the string[] fallback. + const lines = ['hi 😀 there\n', 'math 𝕏 sym\n']; + expect(isWellFormed(lines[0])).toBe(true); + expect(isWellFormed(lines[1])).toBe(true); + const dl = finishLines(lines); + expect('lines' in dl).toBe(false); + expect(linesToArray(dl)).toEqual(lines); + }); + + test('a lone surrogate falls back to the exact strings', () => { + const lines = ['ok\n', 'bad \uD800 here\n']; + expect(isWellFormed(lines[1])).toBe(false); + const dl = finishLines(lines); + expect('lines' in dl).toBe(true); // fallback path + expect(lineAt(dl, 1)).toBe('bad \uD800 here\n'); // preserved verbatim + expect(linesToArray(dl)).toEqual(lines); + }); + + test('knownLossless skips the per-line check on the common path', () => { + const lines = ['a\n', 'b\n']; + const dl = finishLines(lines, true); + expect('lines' in dl).toBe(false); + expect(linesToArray(dl)).toEqual(lines); + }); + + test('a leading BOM is preserved, not silently stripped', () => { + const dl = finishLines(['first\n', 'second\n']); + expect(lineAt(dl, 0)).toBe('first\n'); + }); + + test('offset width scales with the file byte length (u8/u16/u32)', () => { + const small = finishLines(['ab']); // upper bound < 256 bytes + expect(arena(small).offsets.BYTES_PER_ELEMENT).toBe(1); + const medium = finishLines(['x'.repeat(300)]); // 256..65535 + expect(arena(medium).offsets.BYTES_PER_ELEMENT).toBe(2); + const large = finishLines(['y'.repeat(70_000)]); // >= 65536 + expect(arena(large).offsets.BYTES_PER_ELEMENT).toBe(4); + expect(lineAt(large, 0)).toBe('y'.repeat(70_000)); + }); + + test('plainLines wraps strings without encoding into the arena', () => { + const dl = plainLines(['one\n', 'two\n']); + expect('lines' in dl).toBe(true); // string fallback, no byte arena + expect(plain(dl).lines).toEqual(['one\n', 'two\n']); + expect(dl.length).toBe(2); + expect(lineAt(dl, 1)).toBe('two\n'); + }); + + test('EMPTY_DIFF_LINES is a valid empty list', () => { + expect(EMPTY_DIFF_LINES.length).toBe(0); + expect(lineAt(EMPTY_DIFF_LINES, 0)).toBeUndefined(); + }); + + test('joinLines concatenates the arena (the editor whole-side accessor)', () => { + const lines = ['const x = 1;\n', 'return x;\n']; + const dl = finishLines(lines); + expect('lines' in dl).toBe(false); // arena path + // The default empty separator decodes the whole arena in one pass. + expect(joinLines(dl)).toBe('const x = 1;\nreturn x;\n'); + }); + + test('joinLines round-trips multibyte content from the arena', () => { + const dl = finishLines(['cœur de pierre précieuse\n', '日本語のテスト\n']); + expect('lines' in dl).toBe(false); + expect(joinLines(dl)).toBe('cœur de pierre précieuse\n日本語のテスト\n'); + }); + + test('joinLines reads the string fallback when the arena is unused', () => { + const dl = plainLines(['a\n', 'b\n']); + expect(joinLines(dl)).toBe('a\nb\n'); + }); +}); diff --git a/packages/diffs/test/hydration.test.ts b/packages/diffs/test/hydration.test.ts index 9ef061126..207d1ee01 100644 --- a/packages/diffs/test/hydration.test.ts +++ b/packages/diffs/test/hydration.test.ts @@ -21,7 +21,7 @@ import type { Virtualizer } from '../src/components/Virtualizer'; import { DEFAULT_VIRTUAL_FILE_METRICS } from '../src/constants'; import type { FileContents, FileDiffMetadata } from '../src/types'; import { linesFromFileContents } from '../src/utils/computeFileOffsets'; -import { plainLines } from '../src/utils/diffLines'; +import { EMPTY_DIFF_LINES } from '../src/utils/diffLines'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { assertDefined } from './testUtils'; @@ -258,8 +258,8 @@ const fileDiff: FileDiffMetadata = { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: plainLines([]), - additionLines: plainLines([]), + deletionLines: EMPTY_DIFF_LINES, + additionLines: EMPTY_DIFF_LINES, }; describe('collapsed hydration', () => { diff --git a/packages/diffs/test/onPostRenderPhase.test.ts b/packages/diffs/test/onPostRenderPhase.test.ts index 6219207da..72750c2c9 100644 --- a/packages/diffs/test/onPostRenderPhase.test.ts +++ b/packages/diffs/test/onPostRenderPhase.test.ts @@ -11,7 +11,7 @@ import type { FileDiffMetadata, PostRenderPhase, } from '../src/types'; -import { plainLines } from '../src/utils/diffLines'; +import { EMPTY_DIFF_LINES } from '../src/utils/diffLines'; import { createRoot, dispatchScroll, @@ -92,8 +92,8 @@ const fileDiff: FileDiffMetadata = { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: plainLines([]), - additionLines: plainLines([]), + deletionLines: EMPTY_DIFF_LINES, + additionLines: EMPTY_DIFF_LINES, }; describe('onPostRender phases', () => { diff --git a/packages/diffs/test/parsePatchFiles.test.ts b/packages/diffs/test/parsePatchFiles.test.ts index 11844f959..96881bf83 100644 --- a/packages/diffs/test/parsePatchFiles.test.ts +++ b/packages/diffs/test/parsePatchFiles.test.ts @@ -2,6 +2,7 @@ import { afterAll, describe, expect, spyOn, test } from 'bun:test'; import { disposeHighlighter } from '../src/highlighter/shared_highlighter'; import { DiffHunksRenderer } from '../src/renderers/DiffHunksRenderer'; +import { lineAt, linesToArray } from '../src/utils/diffLines'; import { parsePatchFiles, processFile } from '../src/utils/parsePatchFiles'; import { diffPatch, @@ -13,9 +14,9 @@ import { assertDefined, countRenderedLines, countSplitRows, - linesOf, patchDigest, verifyPatchHunkValues, + withPlainLines, } from './testUtils'; afterAll(async () => { @@ -32,7 +33,7 @@ describe('parsePatchFiles', () => { test('patches with a final blank line should have a \\n added', () => { const result = parsePatchFiles(finalBlankLinePatch); - expect(result).toMatchSnapshot('final blank line patch'); + expect(withPlainLines(result)).toMatchSnapshot('final blank line patch'); }); test('should have accurate hunk line values', () => { @@ -57,7 +58,7 @@ describe('parsePatchFiles', () => { const hunk = result[0].files[0].hunks[0]; expect(hunk.deletionCount).toBe(87); expect(hunk.deletionLines).toBe(86); - expect(result).toMatchSnapshot('malformed patch'); + expect(withPlainLines(result)).toMatchSnapshot('malformed patch'); } finally { consoleError.mockRestore(); } @@ -173,16 +174,17 @@ describe('parsePatchFiles', () => { ); const file = result[0]?.files[0]; + assertDefined(file, 'expected a parsed file'); expect(result[0]?.files).toHaveLength(1); - expect(file?.name).toBe('sql/test.sql'); - expect(linesOf(file?.deletionLines)).toEqual([ + expect(file.name).toBe('sql/test.sql'); + expect(linesToArray(file.deletionLines)).toEqual([ '-- This is a test sql file\n', '-- This is an sql comment\n', '\n', 'CREATE TABLE users (\n', 'id BIGSERIAL PRIMARY KEY,\n', ]); - expect(linesOf(file?.additionLines)).toEqual([ + expect(linesToArray(file.additionLines)).toEqual([ '-- This is a test sql file\n', '\n', 'CREATE TABLE users (\n', @@ -204,10 +206,11 @@ describe('parsePatchFiles', () => { ); const file = result[0]?.files[0]; + assertDefined(file, 'expected a parsed file'); expect(result[0]?.files).toHaveLength(1); - expect(file?.name).toBe('markers.txt'); - expect(linesOf(file?.deletionLines)).toEqual(['-- old marker\n']); - expect(linesOf(file?.additionLines)).toEqual(['++ new marker\n']); + expect(file.name).toBe('markers.txt'); + expect(linesToArray(file.deletionLines)).toEqual(['-- old marker\n']); + expect(linesToArray(file.additionLines)).toEqual(['++ new marker\n']); }); test('preserves leading BOM characters in parsed hunk lines', () => { @@ -224,8 +227,9 @@ describe('parsePatchFiles', () => { ); const file = result[0]?.files[0]; - expect(linesOf(file?.deletionLines)?.[0]).toBe('\uFEFFold\n'); - expect(linesOf(file?.additionLines)?.[0]).toBe('\uFEFFnew\n'); + assertDefined(file, 'expected a parsed file'); + expect(lineAt(file.deletionLines, 0)).toBe('\uFEFFold\n'); + expect(lineAt(file.additionLines, 0)).toBe('\uFEFFnew\n'); }); test('preserves lone surrogate characters in parsed hunk lines', () => { @@ -242,8 +246,9 @@ describe('parsePatchFiles', () => { ); const file = result[0]?.files[0]; - expect(linesOf(file?.deletionLines)?.[0]).toBe('old\ud800\n'); - expect(linesOf(file?.additionLines)?.[0]).toBe('new\ud800\n'); + assertDefined(file, 'expected a parsed file'); + expect(lineAt(file.deletionLines, 0)).toBe('old\ud800\n'); + expect(lineAt(file.additionLines, 0)).toBe('new\ud800\n'); }); test('parses quoted git diff headers with escaped file names', () => { diff --git a/packages/diffs/test/testUtils.ts b/packages/diffs/test/testUtils.ts index c57ce83df..43ae8b846 100644 --- a/packages/diffs/test/testUtils.ts +++ b/packages/diffs/test/testUtils.ts @@ -5,6 +5,27 @@ import type { HunksRenderResult } from '../src/renderers/DiffHunksRenderer'; import type { FileDiffMetadata, ParsedPatch } from '../src/types'; import { type DiffLines, linesToArray } from '../src/utils/diffLines'; +// The parser stores each side's lines compactly as `DiffLines`. Expand those two +// fields back to plain `string[]` so snapshots show line content instead of the +// byte-arena internals and keep matching the original format. +// Same as the parsed types, but with each side's lines expanded back to string[]. +type PlainFile = Omit & { + additionLines: string[]; + deletionLines: string[]; +}; +type PlainPatch = Omit & { files: PlainFile[] }; + +export function withPlainLines(patches: ParsedPatch[]): PlainPatch[] { + return patches.map((patch) => ({ + ...patch, + files: patch.files.map((file) => ({ + ...file, + additionLines: linesToArray(file.additionLines), + deletionLines: linesToArray(file.deletionLines), + })), + })); +} + // Assertion helpers /** diff --git a/packages/diffs/test/updateDiffHunks.test.ts b/packages/diffs/test/updateDiffHunks.test.ts index 76776c59c..e87c8a8ae 100644 --- a/packages/diffs/test/updateDiffHunks.test.ts +++ b/packages/diffs/test/updateDiffHunks.test.ts @@ -101,12 +101,12 @@ function setAdditionLineText( line: number, lineText: string ): void { - const prevLine = lineAt(diff.additionLines, line); + // Edit through the plain form and hand the side back, like the editor does + const lines = linesToArray(diff.additionLines); + const prevLine = lines[line]; if (prevLine == null) { throw new Error(`Missing addition line ${line}`); } - // Edit through the plain form and hand the side back, like the editor does - const lines = linesToArray(diff.additionLines); if (prevLine.endsWith('\r\n')) { lines[line] = `${lineText}\r\n`; } else if (prevLine.endsWith('\n')) { diff --git a/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts b/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts index fe2e84a23..7b2b3c48a 100644 --- a/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts +++ b/packages/diffs/test/virtualizedFileDiffEstimatedHeights.test.ts @@ -13,7 +13,12 @@ import type { RenderWindow, VirtualFileMetrics, } from '../src/types'; -import { linesToArray, plainLines } from '../src/utils/diffLines'; +import { + EMPTY_DIFF_LINES, + finishLines, + linesToArray, + plainLines, +} from '../src/utils/diffLines'; import { iterateOverDiff } from '../src/utils/iterateOverDiff'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { recomputeDiffHunks } from '../src/utils/updateDiffHunks'; @@ -158,8 +163,8 @@ function createHugeSingleBlockDiff(lineCount: number): FileDiffMetadata { splitLineCount: lineCount, unifiedLineCount: lineCount, isPartial: true, - deletionLines: plainLines([]), - additionLines: plainLines([]), + deletionLines: finishLines([]), + additionLines: finishLines([]), }; } @@ -172,8 +177,8 @@ function createNoHunkDiff(): FileDiffMetadata { splitLineCount: 0, unifiedLineCount: 0, isPartial: false, - deletionLines: plainLines([]), - additionLines: plainLines([]), + deletionLines: EMPTY_DIFF_LINES, + additionLines: EMPTY_DIFF_LINES, }; } From aa107665febdf94da28225bfea73f1db19b91058 Mon Sep 17 00:00:00 2001 From: clemg Date: Tue, 16 Jun 2026 18:58:51 +0200 Subject: [PATCH 3/5] perf(diffs): parse hunk content from UTF-8 bytes processFileBytes parses a single file's diff straight from its UTF-8 bytes: only the file header and the @@ hunk header lines are decoded into JS strings, every content line's bytes are copied verbatim into the per-side arenas and decoded on demand by lineAt, so a parse allocates no per-line strings and no per-line garbage. The per-side byte arena is filled by a small SideBuilder (appendLine / sealSide) that sits in diffLines.ts next to finishLines, so every way of building a DiffLines lives in one module, while parsePatchFiles keeps only the byte scanners that walk the patch structure. It is the only hunk-content parser: processFile encodes its string once up front and hands the bytes over, and the full-file path (parseDiffFromFile, merge conflicts) rides the same loop: the patch bytes drive the hunk structure while the sides keep coming from the caller's contents strings through finishLines, like before. The previous per-line string loop and its helpers go away. Two byte-only behaviors to know about: invalid UTF-8 stays as-is in the arena and decodes to U+FFFD on read (the same text a stream decode produces), and a patch string holding a lone surrogate (which the up-front encode would corrupt) gets its exact line strings rebuilt from the original text into the plain-string form, so the surrogate-preservation behavior is unchanged. The parsed model is hash-identical to the previous parser on linux v6.0..v7.0 (76,872 files, 22.8M lines, every line compared). --- packages/diffs/src/utils/diffLines.ts | 143 ++++ packages/diffs/src/utils/parsePatchFiles.ts | 823 +++++++++++++------- 2 files changed, 681 insertions(+), 285 deletions(-) diff --git a/packages/diffs/src/utils/diffLines.ts b/packages/diffs/src/utils/diffLines.ts index f926d85ef..2aa875840 100644 --- a/packages/diffs/src/utils/diffLines.ts +++ b/packages/diffs/src/utils/diffLines.ts @@ -262,3 +262,146 @@ export function finishLines( offsets, }; } + +// Byte values the side builder compares against while accumulating lines +// oxfmt-ignore +const NEWLINE = '\n'.charCodeAt(0), // 10 + CARRIAGE_RETURN = '\r'.charCodeAt(0); // 13 + +// Accumulates one side's line content during a byte parse: content bytes are +// appended into `scratch` and each line's end offset lands in `offsets` +// (entry 0 stays 0), so sealing is a copy of exactly the bytes written plus +// an offsets downcast to the smallest element width that fits +export interface SideBuilder { + // acutal byte buffer for a side's content + scratch: Uint8Array; + // line-end offsets into `scratch` (always 0 at index 0, then the end of each line) + offsets: Uint32Array; + // line count + count: number; + // next write position in `scratch` + position: number; +} + +function createSideBuilder(byteCapacity: number): SideBuilder { + return { + scratch: new Uint8Array(byteCapacity), + offsets: new Uint32Array(256), + count: 0, + position: 0, + }; +} + +// Streamed parses call processFileBytes once per file, so we keep the two +// side builders at module scope and reuse them across calls: the typical file +// is a few KB and would otherwise pay a fresh zeroed allocation per side per +// file. Files larger than the cap get a throwaway builder instead, so the +// persistent footprint stays bounded (at most ~2MB across both sides) without +// needing a release hook +const SIDE_SCRATCH_BYTE_CAP = 1 << 20; // 1 MiB +// we start with 64KiB scratch buffers, which should be enough for most files (?) +const persistentAdditionSide = createSideBuilder(1 << 16); +const persistentDeletionSide = createSideBuilder(1 << 16); + +// When a file's content is getting parsed, the builder is reset to empty +// (count = 0, position = 0) and its scratch buffer is grown if necessary, but +// never shrunk (up to the cap). Larger files get disposable builders instead, +// see comments above +export function acquireSideBuilder( + side: 'addition' | 'deletion', + byteCapacity: number +): SideBuilder { + // Disposable builders, will get GC'd after the file is done + if (byteCapacity > SIDE_SCRATCH_BYTE_CAP) { + return createSideBuilder(byteCapacity); + } + const persistent = + side === 'addition' ? persistentAdditionSide : persistentDeletionSide; + if (persistent.scratch.length < byteCapacity) { + let capacity = persistent.scratch.length * 2; + while (capacity < byteCapacity) { + capacity *= 2; + } + persistent.scratch = new Uint8Array(capacity); + } + persistent.count = 0; + persistent.position = 0; + return persistent; +} + +function ensureOffsetCapacity(builder: SideBuilder): void { + if (builder.count + 2 > builder.offsets.length) { + const grown = new Uint32Array(builder.offsets.length * 2); + grown.set(builder.offsets); + builder.offsets = grown; + } +} + +// Append one line's content bytes. An empty content slice stores a single +// newline, so a bare `+` line reads back as '\n' like every other line-end +export function appendLine( + builder: SideBuilder, + source: Uint8Array, + start: number, + end: number +): void { + ensureOffsetCapacity(builder); + const { scratch } = builder; + let { position } = builder; + if (start === end) { + scratch[position++] = NEWLINE; + } else { + // This will allocate a new Uint8Array for each line (+ call cost). We + // could avoid that by copying the bytes one by one with a loop, but that's + // not very readable and it's probably not worth on huge lines, so probably + // not worth the micro-optimization. I don't know what I think about this + // tradeoff of having only this though + scratch.set(source.subarray(start, end), position); + position += end - start; + } + builder.position = position; + builder.offsets[++builder.count] = position; +} + +// Strip one trailing newline (and a preceding carriage return) from the last +// appended line (the byte equivalent of running `cleanLastNewline` on the +// last entry of the side's line list when a `\ No newline at end of file` +// marker arrives) +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/cleanLastNewline.ts#L1-L10 +export function trimLastLineNewline(builder: SideBuilder): void { + if (builder.count === 0) { + return; + } + const lineStart = builder.offsets[builder.count - 1]; + let { position } = builder; + if (position > lineStart && builder.scratch[position - 1] === NEWLINE) { + position--; + if ( + position > lineStart && + builder.scratch[position - 1] === CARRIAGE_RETURN + ) { + position--; + } + } + builder.position = position; + builder.offsets[builder.count] = position; +} + +export function sealSide(builder: SideBuilder): DiffLines { + const { count, position } = builder; + // Same width thresholds as `finishLines`, but chosen from the actual byte + // length instead of its `charTotal * 3` upper bound, so a side never gets a + // bigger table than it needs + const offsets = + position < 0x100 + ? new Uint8Array(count + 1) + : position < 0x10000 + ? new Uint16Array(count + 1) + : new Uint32Array(count + 1); + offsets.set(builder.offsets.subarray(0, count + 1)); + return { + length: count, + bytes: builder.scratch.slice(0, position), + offsets, + }; +} diff --git a/packages/diffs/src/utils/parsePatchFiles.ts b/packages/diffs/src/utils/parsePatchFiles.ts index d4ba7eb3c..4652f048f 100644 --- a/packages/diffs/src/utils/parsePatchFiles.ts +++ b/packages/diffs/src/utils/parsePatchFiles.ts @@ -12,16 +12,20 @@ import type { FileContents, FileDiffMetadata, Hunk, - HunkLineType, ParsedPatch, } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; import { detachString, releaseStringDetachBuffer } from './detachString'; import { + acquireSideBuilder, + appendLine, EMPTY_DIFF_LINES, finishLines, isWellFormed, + plainLines, releaseLineScratch, + sealSide, + trimLastLineNewline, } from './diffLines'; import { getHunkSideEndBoundary, @@ -29,6 +33,8 @@ import { } from './getHunkSideBoundaries'; import { realignChangeContentBySimilarity } from './realignChangeContent'; +const patchEncoder = new TextEncoder(); + interface ParsedHunkHeader { additionCount: number; additionStart: number; @@ -131,6 +137,11 @@ export function processFile( } } +// A string input encodes once up front and parses as bytes, so there is a +// single hunk-content parser. A lone surrogate can't survive that encode, so +// those (essentially nonexistent) patch inputs get their exact line strings +// rebuilt from the original text afterwards; full-file sides never come from +// the patch text, so they don't need it function _processFile( fileDiffString: string, { @@ -141,183 +152,139 @@ function _processFile( throwOnError = false, }: ProcessFileOptions = {} ): FileDiffMetadata | undefined { - let lastHunkEnd = 0; - const hunks = splitAtLinePrefix(fileDiffString, '@@ '); - let currentFile: FileDiffMetadata | undefined; const isPartial = oldFile == null || newFile == null; - let deletionLineIndex = 0; - let additionLineIndex = 0; + const lossless = !isPartial || isWellFormed(fileDiffString); + const currentFile = _processFileBytes(patchEncoder.encode(fileDiffString), { + cacheKey, + isGitDiff, + oldFile, + newFile, + throwOnError, + }); + if (currentFile != null && !lossless) { + rebuildLossySides(currentFile, fileDiffString); + } + return currentFile; +} + +/** + * Parse a single file's diff from UTF-8 bytes into a `FileDiffMetadata`. Only + * the file header and each `@@` hunk header line are decoded into JS strings; + * every content line's bytes are copied verbatim into the per-side byte + * arenas and decoded on demand by `lineAt`, so parsing allocates no per-line + * strings. Meant for streamed patches where the bytes are already at hand; + * pass `isGitDiff` when known to skip detection. + * + * Invalid UTF-8 stays as-is in the arena and decodes to U+FFFD on read, the + * same text a whole-stream decode would have produced + */ +export function processFileBytes( + fileBytes: Uint8Array, + options: ProcessFileOptions = {} +): FileDiffMetadata | undefined { + try { + return _processFileBytes(fileBytes, options); + } finally { + releaseStringDetachBuffer(); + releaseLineScratch(); + } +} + +function _processFileBytes( + fileBytes: Uint8Array, + options: ProcessFileOptions = {} +): FileDiffMetadata | undefined { + const { cacheKey, oldFile, newFile, throwOnError = false } = options; + const length = fileBytes.length; + const isGitDiff = options.isGitDiff ?? isGitDiffBytes(fileBytes, length); + const isPartial = oldFile == null || newFile == null; + + // The header region is everything before the first hunk line. When the + // input itself starts with `@@ `, that first chunk still plays the header + // role (a file diff can't open with content), so the region ends at the + // SECOND hunk line + const headerEnd = isHunkLineAt(fileBytes, 0, length) + ? findNextHunkLine( + fileBytes, + lineEndExclusive(fileBytes, 0, length), + length + ) + : findNextHunkLine(fileBytes, 0, length); + + const currentFile: FileDiffMetadata = { + name: '', + type: 'change', + hunks: [], + splitLineCount: 0, + unifiedLineCount: 0, + isPartial, + additionLines: EMPTY_DIFF_LINES, + deletionLines: EMPTY_DIFF_LINES, + cacheKey: maybeDetachOptionalString(cacheKey), + }; + + const headerText = metadataDecoder.decode(fileBytes.subarray(0, headerEnd)); + for (const line of splitWithNewlines(headerText)) { + applyFileHeaderLine(currentFile, line, isGitDiff, throwOnError); + } + + // A full-file diff's sides are the caller's file contents; the patch text + // only drives the hunk structure. A partial (patch-only) diff accumulates + // its sides from the content lines instead let additionLineList: string[] = []; let deletionLineList: string[] = []; - for (const hunk of hunks) { - const lines = splitWithNewlines(hunk); - const firstLine = lines[0]; - if (firstLine == null) { - if (throwOnError) { - throw Error('parsePatchContent: invalid hunk'); - } else { - console.error('parsePatchContent: invalid hunk', hunk); - } - continue; + if (oldFile != null && newFile != null) { + additionLineList = splitWithNewlines(newFile.contents); + deletionLineList = splitWithNewlines(oldFile.contents); + // If either file is technically empty, then we should empty the + // arrays respectively + if (additionLineList.length === 1 && newFile.contents === '') { + additionLineList.length = 0; } - const fileHeader = parseHunkHeader(firstLine); - let additionLines = 0; - let deletionLines = 0; - // Setup currentFile, this should be the first iteration of our hunks, and - // technically not a hunk - if (fileHeader == null || currentFile == null) { - if (currentFile != null) { - if (throwOnError) { - throw Error('parsePatchContent: Invalid hunk'); - } else { - console.error('parsePatchContent: Invalid hunk', hunk); - } - continue; - } - additionLineList = - !isPartial && oldFile != null && newFile != null - ? splitWithNewlines(newFile.contents) - : []; - deletionLineList = - !isPartial && oldFile != null && newFile != null - ? splitWithNewlines(oldFile.contents) - : []; - // If either file is technically empty, then we should empty the - // arrays respectively - if (additionLineList.length === 1 && newFile?.contents === '') { - additionLineList.length = 0; - } - if (deletionLineList.length === 1 && oldFile?.contents === '') { - deletionLineList.length = 0; - } - currentFile = { - name: '', - type: 'change', - hunks: [], - splitLineCount: 0, - unifiedLineCount: 0, - isPartial, - additionLines: EMPTY_DIFF_LINES, - deletionLines: EMPTY_DIFF_LINES, - cacheKey: maybeDetachOptionalString(cacheKey), - }; - - for (const line of lines) { - if (line.startsWith('diff --git')) { - const filenameMatch = line.trim().match(ALTERNATE_FILE_NAMES_GIT); - const prevName = filenameMatch?.[1] ?? filenameMatch?.[2]; - const name = filenameMatch?.[3] ?? filenameMatch?.[4]; - if (prevName == null || name == null) { - if (throwOnError) { - throw Error('parsePatchContent: invalid git diff header'); - } else { - console.error('parsePatchContent: invalid git diff header', line); - } - continue; - } - currentFile.name = detachString(name.trim()); - if (prevName !== name) { - currentFile.prevName = detachString(prevName.trim()); - } - continue; - } - - const filenameMatch = - line.startsWith('---') || line.startsWith('+++') - ? line.match( - isGitDiff ? FILENAME_HEADER_REGEX_GIT : FILENAME_HEADER_REGEX - ) - : null; - if (filenameMatch != null) { - const [, type, fileName] = filenameMatch; - if (type === '---' && fileName !== '/dev/null') { - const detachedFileName = detachString(fileName.trim()); - currentFile.prevName = detachedFileName; - currentFile.name = detachedFileName; - } else if (type === '+++' && fileName !== '/dev/null') { - currentFile.name = detachString(fileName.trim()); - } - } - // Git diffs have a bunch of additional metadata we can pull from - else if (isGitDiff) { - if (line.startsWith('new mode ')) { - currentFile.mode = detachString( - line.slice('new mode'.length).trim() - ); - } - if (line.startsWith('old mode ')) { - currentFile.prevMode = detachString( - line.slice('old mode'.length).trim() - ); - } - if (line.startsWith('new file mode')) { - currentFile.type = 'new'; - currentFile.mode = detachString( - line.slice('new file mode'.length).trim() - ); - } - if (line.startsWith('deleted file mode')) { - currentFile.type = 'deleted'; - currentFile.mode = detachString( - line.slice('deleted file mode'.length).trim() - ); - } - if (line.startsWith('similarity index')) { - if (line.startsWith('similarity index 100%')) { - currentFile.type = 'rename-pure'; - } else { - currentFile.type = 'rename-changed'; - } - } - if (line.startsWith('index ')) { - const [, prevObjectId, newObjectId, mode] = - line.trim().match(INDEX_LINE_METADATA) ?? []; - if (prevObjectId != null) { - currentFile.prevObjectId = detachString(prevObjectId); - } - if (newObjectId != null) { - currentFile.newObjectId = detachString(newObjectId); - } - if (mode != null) { - currentFile.mode = detachString(mode); - } - } - // We have to handle these for pure renames because there won't be - // --- and +++ lines - if (line.startsWith('rename from ')) { - currentFile.prevName = detachString( - line.slice('rename from '.length).trim() - ); - } - if (line.startsWith('rename to ')) { - currentFile.name = detachString( - line.slice('rename to '.length).trim() - ); - } - } - } - continue; + if (deletionLineList.length === 1 && oldFile.contents === '') { + deletionLineList.length = 0; } + } - // Otherwise, time to start parsing out the hunk - let currentContent: ContextContent | ChangeContent | undefined; - let lastLineType: 'context' | 'addition' | 'deletion' | undefined; + // The side builders only collect content bytes on the partial path; a + // full-file diff reads its sides from the contents strings above + const sideCapacity = isPartial ? length : 0; + const additionSide = acquireSideBuilder('addition', sideCapacity); + const deletionSide = acquireSideBuilder('deletion', sideCapacity); + let lastHunkEnd = 0; + let position = headerEnd; - // Strip trailing bare newlines (format-patch separators between commits) - // if needed - while ( - lines.length > 0 && - (lines[lines.length - 1] === '\n' || - lines[lines.length - 1] === '\r' || - lines[lines.length - 1] === '\r\n' || - lines[lines.length - 1] === '') - ) { - lines.pop(); + while (position < length) { + // `position` is always at a `@@ ` line here + const hunkHeaderEnd = lineEndExclusive(fileBytes, position, length); + const firstLine = metadataDecoder.decode( + fileBytes.subarray(position, hunkHeaderEnd) + ); + const fileHeader = parseHunkHeader(firstLine); + if (fileHeader == null) { + // Report the malformed chunk and skip everything up to the next hunk + // line + const chunkEnd = findNextHunkLine(fileBytes, hunkHeaderEnd, length); + if (throwOnError) { + throw Error('parsePatchContent: Invalid hunk'); + } else { + console.error( + 'parsePatchContent: Invalid hunk', + metadataDecoder.decode(fileBytes.subarray(position, chunkEnd)) + ); + } + position = chunkEnd; + continue; } const { additionStart, deletionStart } = fileHeader; - deletionLineIndex = isPartial ? deletionLineIndex : deletionStart - 1; - additionLineIndex = isPartial ? additionLineIndex : additionStart - 1; + // On the partial path the running line indexes are exactly the number of + // lines accumulated on each side so far; a full-file diff indexes into + // the whole file's lines instead + let deletionLineIndex = isPartial ? deletionSide.count : deletionStart - 1; + let additionLineIndex = isPartial ? additionSide.count : additionStart - 1; + let additionLines = 0; + let deletionLines = 0; const hunkData: Hunk = { collapsedBefore: 0, @@ -341,58 +308,48 @@ function _processFile( hunkContent: [], hunkContext: maybeDetachOptionalString(fileHeader.hunkContext), - hunkSpecs: detachString(firstLine), + hunkSpecs: firstLine, noEOFCRAdditions: false, noEOFCRDeletions: false, }; - // Now we process each line of the hunk + let currentContent: ContextContent | ChangeContent | undefined; + let lastLineType: 'context' | 'addition' | 'deletion' | undefined; let parsedAdditionLines = 0; let parsedDeletionLines = 0; - for (let lineIndex = 1; lineIndex < lines.length; lineIndex++) { - const rawLine = lines[lineIndex]; + + position = hunkHeaderEnd; + while (position < length && !isHunkLineAt(fileBytes, position, length)) { + const lineEnd = lineEndExclusive(fileBytes, position, length); + const firstByte = fileBytes[position]; if ( parsedAdditionLines >= hunkData.additionCount && parsedDeletionLines >= hunkData.deletionCount && - !rawLine.startsWith('\\') - ) { - if ( - throwOnError && - isHunkBodyLine(rawLine) && - !isFormatPatchVersionSeparator(rawLine) - ) { - throw Error('parsePatchContent: hunk has more lines than expected'); - } - break; - } - - const firstChar = rawLine[0]; - // If we can't properly process the line, well, lets just try to salvage - // things and continue... It's possible an AI generated diff might have - // some stray blank lines or something in there - if ( - firstChar !== '+' && - firstChar !== '-' && - firstChar !== ' ' && - firstChar !== '\\' + firstByte !== BACKSLASH ) { if (throwOnError) { - throw Error('parsePatchContent: invalid hunk line'); + const rawLine = metadataDecoder.decode( + fileBytes.subarray(position, lineEnd) + ); + if ( + isHunkBodyLine(rawLine) && + !isFormatPatchVersionSeparator(rawLine) + ) { + throw Error('parsePatchContent: hunk has more lines than expected'); + } } - console.error( - `parseLineType: Invalid firstChar: "${firstChar}", full line: "${rawLine}"` - ); - console.error('processFile: invalid rawLine:', rawLine); - continue; + // Counts are satisfied: whatever else sits inside this chunk is + // trailer junk (e.g. a format-patch signature), so skip to the next + // hunk + position = findNextHunkLine(fileBytes, lineEnd, length); + break; } - const type = parseRawLineType(firstChar); - if (type === 'addition') { + if (firstByte === PLUS) { if (throwOnError && parsedAdditionLines >= hunkData.additionCount) { throw Error('parsePatchContent: hunk has too many addition lines'); } - const line = getParsedLineContent(rawLine); if (currentContent == null || currentContent.type !== 'change') { currentContent = createContentGroup( 'change', @@ -404,16 +361,15 @@ function _processFile( additionLineIndex++; parsedAdditionLines++; if (isPartial) { - additionLineList.push(line); + appendLine(additionSide, fileBytes, position + 1, lineEnd); } currentContent.additions++; additionLines++; lastLineType = 'addition'; - } else if (type === 'deletion') { + } else if (firstByte === MINUS) { if (throwOnError && parsedDeletionLines >= hunkData.deletionCount) { throw Error('parsePatchContent: hunk has too many deletion lines'); } - const line = getParsedLineContent(rawLine); if (currentContent == null || currentContent.type !== 'change') { currentContent = createContentGroup( 'change', @@ -425,12 +381,12 @@ function _processFile( deletionLineIndex++; parsedDeletionLines++; if (isPartial) { - deletionLineList.push(line); + appendLine(deletionSide, fileBytes, position + 1, lineEnd); } currentContent.deletions++; deletionLines++; lastLineType = 'deletion'; - } else if (type === 'context') { + } else if (firstByte === SPACE) { if ( throwOnError && (parsedDeletionLines >= hunkData.deletionCount || @@ -438,7 +394,6 @@ function _processFile( ) { throw Error('parsePatchContent: hunk has too many context lines'); } - const line = getParsedLineContent(rawLine); if (currentContent == null || currentContent.type !== 'context') { currentContent = createContentGroup( 'context', @@ -452,45 +407,61 @@ function _processFile( parsedAdditionLines++; parsedDeletionLines++; if (isPartial) { - deletionLineList.push(line); - additionLineList.push(line); + appendLine(deletionSide, fileBytes, position + 1, lineEnd); + appendLine(additionSide, fileBytes, position + 1, lineEnd); } currentContent.lines++; lastLineType = 'context'; - } else if (type === 'metadata' && currentContent != null) { - if (currentContent.type === 'context') { - hunkData.noEOFCRAdditions = true; - hunkData.noEOFCRDeletions = true; - } else if (lastLineType === 'deletion') { - hunkData.noEOFCRDeletions = true; - } else if (lastLineType === 'addition') { - hunkData.noEOFCRAdditions = true; - } - // If we're dealing with partial content from a diff, we need to strip - // newlines manually from the content - if ( - isPartial && - (lastLineType === 'addition' || lastLineType === 'context') - ) { - const lastIndex = additionLineList.length - 1; - if (lastIndex >= 0) { - additionLineList[lastIndex] = cleanLastNewline( - additionLineList[lastIndex] - ); + } else if (firstByte === BACKSLASH) { + if (currentContent != null) { + if (currentContent.type === 'context') { + hunkData.noEOFCRAdditions = true; + hunkData.noEOFCRDeletions = true; + } else if (lastLineType === 'deletion') { + hunkData.noEOFCRDeletions = true; + } else if (lastLineType === 'addition') { + hunkData.noEOFCRAdditions = true; } - } - if ( - isPartial && - (lastLineType === 'deletion' || lastLineType === 'context') - ) { - const lastIndex = deletionLineList.length - 1; - if (lastIndex >= 0) { - deletionLineList[lastIndex] = cleanLastNewline( - deletionLineList[lastIndex] - ); + // Only partial content needs the manual newline strip; full-file + // sides keep the contents' own line endings + if ( + isPartial && + (lastLineType === 'addition' || lastLineType === 'context') + ) { + trimLastLineNewline(additionSide); + } + if ( + isPartial && + (lastLineType === 'deletion' || lastLineType === 'context') + ) { + trimLastLineNewline(deletionSide); } } + } else if ( + isBlankLine(fileBytes, position, lineEnd) && + restOfChunkIsBlank(fileBytes, lineEnd, length) + ) { + // A run of bare newlines closing a chunk (format-patch separators + // between commits) is dropped silently + position = findNextHunkLine(fileBytes, lineEnd, length); + break; + } else { + if (throwOnError) { + throw Error('parsePatchContent: invalid hunk line'); + } + // If we can't properly process the line, well, lets just try to + // salvage things and continue... It's possible an AI generated diff + // might have some stray blank lines or something in there + const rawLine = metadataDecoder.decode( + fileBytes.subarray(position, lineEnd) + ); + const firstChar = rawLine[0]; + console.error( + `parseLineType: Invalid firstChar: "${firstChar}", full line: "${rawLine}"` + ); + console.error('processFile: invalid rawLine:', rawLine); } + position = lineEnd; } if ( @@ -536,9 +507,7 @@ function _processFile( currentFile.unifiedLineCount += hunkData.collapsedBefore + hunkData.unifiedLineCount; } - if (currentFile == null) { - return undefined; - } + if ( throwOnError && isPartial && @@ -557,14 +526,16 @@ function _processFile( deletionLineList.length > 0 ) { const lastHunk = currentFile.hunks[currentFile.hunks.length - 1]; - const lastHunkEnd = getHunkSideEndBoundary( + const lastHunkEndLine = getHunkSideEndBoundary( lastHunk.additionStart, lastHunk.additionCount ); - // `currentFile.additionLines` is not sealed into its arena until the end of - // this function, so the live line count is still the local list - const totalFileLines = additionLineList.length; - const collapsedAfter = Math.max(totalFileLines - lastHunkEnd, 0); + // The sides are not sealed into their arenas until the end of this + // function, so the live line count is still the local list + const collapsedAfter = Math.max( + additionLineList.length - lastHunkEndLine, + 0 + ); currentFile.splitLineCount += collapsedAfter; currentFile.unifiedLineCount += collapsedAfter; } @@ -576,11 +547,8 @@ function _processFile( currentFile.prevName != null && currentFile.name !== currentFile.prevName ) { - if (currentFile.hunks.length > 0) { - currentFile.type = 'rename-changed'; - } else { - currentFile.type = 'rename-pure'; - } + currentFile.type = + currentFile.hunks.length > 0 ? 'rename-changed' : 'rename-pure'; } // Sort of a hack for detecting deleted/added files... else if ( @@ -603,21 +571,17 @@ function _processFile( ) { currentFile.prevName = undefined; } - // A lone surrogate can't survive the UTF-8 round-trip; checking the whole file - // once here lets `finishLines` skip the per-line check on the common path - const losslessFileDiff = isWellFormed(fileDiffString); - // Seal each side's lines into one compact byte arena per file. On the - // partial-patch path every line is a slice of `fileDiffString` so the - // single surrogate check above lets the seal skip per-line checks; a - // full-file diff is rarer and falls back to the safe per-line check - currentFile.additionLines = finishLines( - additionLineList, - isPartial && losslessFileDiff - ); - currentFile.deletionLines = finishLines( - deletionLineList, - isPartial && losslessFileDiff - ); + + if (isPartial) { + currentFile.additionLines = sealSide(additionSide); + currentFile.deletionLines = sealSide(deletionSide); + } else { + // Full-file sides come from the caller's strings; finishLines keeps its + // per-line surrogate check here, so a lossy file falls back to the exact + // strings + currentFile.additionLines = finishLines(additionLineList); + currentFile.deletionLines = finishLines(deletionLineList); + } // Pair change-block lines by similarity instead of the patch's positional // ordering. Partial diffs work too: their hunk line indexes point into the // patch-built line arrays, which hold every line a block references. This @@ -627,6 +591,184 @@ function _processFile( return currentFile; } +/** + * Rebuild both sides' exact line strings from the original patch text, for a + * file whose text can't round-trip through UTF-8 (a lone surrogate). The + * byte-parsed structure is sound (the encode's replacement characters never + * land on a first column or a line break), but the arena's content bytes + * would read back as U+FFFD, so the sides are re-split from the string and + * stored in the plain-string form + */ +function rebuildLossySides( + currentFile: FileDiffMetadata, + fileDiffString: string +): void { + const additionList: string[] = []; + const deletionList: string[] = []; + const chunks = splitAtLinePrefix(fileDiffString, '@@ '); + for (let chunkIndex = 1; chunkIndex < chunks.length; chunkIndex++) { + const lines = splitWithNewlines(chunks[chunkIndex]); + const fileHeader = parseHunkHeader(lines[0] ?? ''); + if (fileHeader == null) { + continue; + } + while ( + lines.length > 0 && + (lines[lines.length - 1] === '\n' || + lines[lines.length - 1] === '\r' || + lines[lines.length - 1] === '\r\n' || + lines[lines.length - 1] === '') + ) { + lines.pop(); + } + let lastLineType: 'context' | 'addition' | 'deletion' | undefined; + let parsedAdditionLines = 0; + let parsedDeletionLines = 0; + for (let lineIndex = 1; lineIndex < lines.length; lineIndex++) { + const rawLine = lines[lineIndex]; + const firstChar = rawLine[0]; + if ( + parsedAdditionLines >= fileHeader.additionCount && + parsedDeletionLines >= fileHeader.deletionCount && + firstChar !== '\\' + ) { + break; + } + const content = rawLine.slice(1); + const line = content === '' ? '\n' : content; + if (firstChar === '+') { + additionList.push(line); + parsedAdditionLines++; + lastLineType = 'addition'; + } else if (firstChar === '-') { + deletionList.push(line); + parsedDeletionLines++; + lastLineType = 'deletion'; + } else if (firstChar === ' ') { + additionList.push(line); + deletionList.push(line); + parsedAdditionLines++; + parsedDeletionLines++; + lastLineType = 'context'; + } else if (firstChar === '\\') { + if (lastLineType === 'addition' || lastLineType === 'context') { + const lastIndex = additionList.length - 1; + if (lastIndex >= 0) { + additionList[lastIndex] = cleanLastNewline(additionList[lastIndex]); + } + } + if (lastLineType === 'deletion' || lastLineType === 'context') { + const lastIndex = deletionList.length - 1; + if (lastIndex >= 0) { + deletionList[lastIndex] = cleanLastNewline(deletionList[lastIndex]); + } + } + } + } + } + currentFile.additionLines = plainLines(additionList); + currentFile.deletionLines = plainLines(deletionList); +} + +/** + * Apply one file-header line (everything before the first `@@` hunk line) to + * the file's metadata: names from `diff --git` / `---` / `+++`, and the extra + * git metadata lines (modes, object ids, renames, similarity) + */ +function applyFileHeaderLine( + currentFile: FileDiffMetadata, + line: string, + isGitDiff: boolean, + throwOnError: boolean +): void { + if (line.startsWith('diff --git')) { + const filenameMatch = line.trim().match(ALTERNATE_FILE_NAMES_GIT); + const prevName = filenameMatch?.[1] ?? filenameMatch?.[2]; + const name = filenameMatch?.[3] ?? filenameMatch?.[4]; + if (prevName == null || name == null) { + if (throwOnError) { + throw Error('parsePatchContent: invalid git diff header'); + } else { + console.error('parsePatchContent: invalid git diff header', line); + } + return; + } + currentFile.name = detachString(name.trim()); + if (prevName !== name) { + currentFile.prevName = detachString(prevName.trim()); + } + return; + } + + const filenameMatch = + line.startsWith('---') || line.startsWith('+++') + ? line.match( + isGitDiff ? FILENAME_HEADER_REGEX_GIT : FILENAME_HEADER_REGEX + ) + : null; + if (filenameMatch != null) { + const [, type, fileName] = filenameMatch; + if (type === '---' && fileName !== '/dev/null') { + const detachedFileName = detachString(fileName.trim()); + currentFile.prevName = detachedFileName; + currentFile.name = detachedFileName; + } else if (type === '+++' && fileName !== '/dev/null') { + currentFile.name = detachString(fileName.trim()); + } + } + // Git diffs have a bunch of additional metadata we can pull from + else if (isGitDiff) { + if (line.startsWith('new mode ')) { + currentFile.mode = detachString(line.slice('new mode'.length).trim()); + } + if (line.startsWith('old mode ')) { + currentFile.prevMode = detachString(line.slice('old mode'.length).trim()); + } + if (line.startsWith('new file mode')) { + currentFile.type = 'new'; + currentFile.mode = detachString( + line.slice('new file mode'.length).trim() + ); + } + if (line.startsWith('deleted file mode')) { + currentFile.type = 'deleted'; + currentFile.mode = detachString( + line.slice('deleted file mode'.length).trim() + ); + } + if (line.startsWith('similarity index')) { + if (line.startsWith('similarity index 100%')) { + currentFile.type = 'rename-pure'; + } else { + currentFile.type = 'rename-changed'; + } + } + if (line.startsWith('index ')) { + const [, prevObjectId, newObjectId, mode] = + line.trim().match(INDEX_LINE_METADATA) ?? []; + if (prevObjectId != null) { + currentFile.prevObjectId = detachString(prevObjectId); + } + if (newObjectId != null) { + currentFile.newObjectId = detachString(newObjectId); + } + if (mode != null) { + currentFile.mode = detachString(mode); + } + } + // We have to handle these for pure renames because there won't be + // --- and +++ lines + if (line.startsWith('rename from ')) { + currentFile.prevName = detachString( + line.slice('rename from '.length).trim() + ); + } + if (line.startsWith('rename to ')) { + currentFile.name = detachString(line.slice('rename to '.length).trim()); + } + } +} + /** * Parses a patch file string into an array of parsed patches. * @@ -978,23 +1120,6 @@ function maybeDetachOptionalString(value: T): T { return (value == null ? value : detachString(value)) as T; } -function parseRawLineType( - firstChar: string | undefined -): Exclude { - return firstChar === ' ' - ? 'context' - : firstChar === '\\' - ? 'metadata' - : firstChar === '+' - ? 'addition' - : 'deletion'; -} - -function getParsedLineContent(rawLine: string): string { - const processedLine = rawLine.slice(1); - return processedLine === '' ? '\n' : processedLine; -} - function createContentGroup( type: 'change', deletionLineIndex: number, @@ -1026,3 +1151,131 @@ function createContentGroup( deletionLineIndex, }; } + +// oxfmt-ignore +const NEWLINE = '\n'.charCodeAt(0), // 10 + CARRIAGE_RETURN = '\r'.charCodeAt(0), // 13 + SPACE = ' '.charCodeAt(0), // 32 + PLUS = '+'.charCodeAt(0), // 43 + MINUS = '-'.charCodeAt(0), // 45 + AT = '@'.charCodeAt(0), // 64 + BACKSLASH = '\\'.charCodeAt(0); // 92 + +// Only metadata slices go through this decoder; `ignoreBOM` keeps a U+FEFF at +// the start of a slice intact, because mid-patch text can legitimately hold +// one (only a whole-stream decode should strip the stream-leading BOM) +const metadataDecoder = new TextDecoder('utf-8', { ignoreBOM: true }); + +function isHunkLineAt( + bytes: Uint8Array, + index: number, + length: number +): boolean { + return ( + index + 2 < length && + bytes[index] === AT && + bytes[index + 1] === AT && + bytes[index + 2] === SPACE + ); +} + +// End of the line starting at `index`, exclusive and including the newline +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L764-L770 +function lineEndExclusive( + bytes: Uint8Array, + index: number, + length: number +): number { + const newlineIndex = bytes.indexOf(NEWLINE, index); + return newlineIndex === -1 ? length : newlineIndex + 1; +} + +// First line at or after `index` (itself a line start) that begins a hunk +// (`@@ `), or `length` when there is none. Hunk chunks span from one such +// line to the next +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L910-L918 +function findNextHunkLine( + bytes: Uint8Array, + index: number, + length: number +): number { + let lineStart = index; + while (lineStart < length) { + if (isHunkLineAt(bytes, lineStart, length)) { + return lineStart; + } + lineStart = lineEndExclusive(bytes, lineStart, length); + } + return length; +} + +// A line that is only a line terminator (or empty at EOF). A run of these +// closing a hunk chunk (format-patch separators) is dropped silently; +// anywhere else they fall through to the invalid-line handling +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L777-L790 +function isBlankLine(bytes: Uint8Array, start: number, end: number): boolean { + const lineLength = end - start; + if (lineLength === 1) { + return bytes[start] === NEWLINE || bytes[start] === CARRIAGE_RETURN; + } + if (lineLength === 2) { + return bytes[start] === CARRIAGE_RETURN && bytes[start + 1] === NEWLINE; + } + return lineLength === 0; +} + +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L345-L346 +function restOfChunkIsBlank( + bytes: Uint8Array, + index: number, + length: number +): boolean { + let lineStart = index; + while (lineStart < length) { + if (isHunkLineAt(bytes, lineStart, length)) { + return true; + } + const lineEnd = lineEndExclusive(bytes, lineStart, length); + if (!isBlankLine(bytes, lineStart, lineEnd)) { + return false; + } + lineStart = lineEnd; + } + return true; +} + +// Byte equivalent of `isGitDiffPatch`: a `diff --git` at the start or at any +// line start +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L889-L891 +function isGitDiffBytes(bytes: Uint8Array, length: number): boolean { + const prefix = 'diff --git'; + let lineStart = 0; + while (lineStart < length) { + if (matchesAscii(bytes, lineStart, length, prefix)) { + return true; + } + const newlineIndex = bytes.indexOf(NEWLINE, lineStart); + if (newlineIndex === -1) { + return false; + } + lineStart = newlineIndex + 1; + } + return false; +} + +function matchesAscii( + bytes: Uint8Array, + index: number, + length: number, + text: string +): boolean { + if (index + text.length > length) { + return false; + } + for (let offset = 0; offset < text.length; offset++) { + if (bytes[index + offset] !== text.charCodeAt(offset)) { + return false; + } + } + return true; +} From 6a3853bf59194b34a4d38e3a171f11dbb85c317a Mon Sep 17 00:00:00 2001 From: clemg Date: Wed, 17 Jun 2026 01:49:05 +0200 Subject: [PATCH 4/5] perf(diffs): split streamed patches into per-file bytes streamGitPatchFiles moves into the package as a byte-level splitter: the patch stream buffers as bytes and each file is handed over as a Uint8Array slice at its diff --git boundary, ready for processFileBytes, so the bulk of the patch is never decoded into JS strings. Format-patch commit metadata splits on the From boundary like the string splitter did, a stream-leading BOM is stripped like a whole-stream TextDecoder would, and a boundary-less patch falls back to one decoded string for parsePatchFiles. The byte scanners and ASCII byte codes shared by the splitter and the file parser (matchesAscii, lineEndExclusive, a generic findNextLineStartingWith, isBlankLine, hasNonWhitespace, and the byte constants) now live in one byteScan module, defined once and kept out of the package's public exports. diffshub's viewer feeds the slices straight to processFileBytes; its own string splitter goes away, and it imports COMMIT_HASH_METADATA_PATTERN from the package instead of keeping a second copy. --- apps/diffshub/components/usePatchLoader.ts | 14 +- apps/diffshub/lib/gitPatchMetadata.ts | 2 +- apps/diffshub/lib/streamGitPatchFiles.ts | 243 --------------- packages/diffs/src/index.ts | 1 + packages/diffs/src/utils/byteScan.ts | 100 ++++++ packages/diffs/src/utils/diffLines.ts | 7 +- packages/diffs/src/utils/parsePatchFiles.ts | 100 ++---- packages/diffs/src/utils/parsePatchStream.ts | 301 +++++++++++++++++++ 8 files changed, 428 insertions(+), 340 deletions(-) delete mode 100644 apps/diffshub/lib/streamGitPatchFiles.ts create mode 100644 packages/diffs/src/utils/byteScan.ts create mode 100644 packages/diffs/src/utils/parsePatchStream.ts diff --git a/apps/diffshub/components/usePatchLoader.ts b/apps/diffshub/components/usePatchLoader.ts index 87537c975..7f9711036 100644 --- a/apps/diffshub/components/usePatchLoader.ts +++ b/apps/diffshub/components/usePatchLoader.ts @@ -4,7 +4,9 @@ import { areSelectionsEqual, type CodeViewItem, type CodeViewLineSelection, - processFile, + getStreamedPatchMetadata, + processFileBytes, + streamGitPatchFiles, } from '@pierre/diffs'; import { type CodeViewHandle, useStableCallback } from '@pierre/diffs/react'; import { @@ -32,10 +34,6 @@ import { formatDiffsHubLineHash, parseDiffsHubLineHash, } from '@/lib/lineHash'; -import { - getStreamedPatchMetadata, - streamGitPatchFiles, -} from '@/lib/streamGitPatchFiles'; import type { CommentMetadata, DiffsHubCommentFileByItemId, @@ -404,12 +402,12 @@ export function usePatchLoader({ publishTreeSource(); }; - const appendStreamedFile = async (fileText: string) => { + const appendStreamedFile = async (fileBytes: Uint8Array) => { if (!hasReceivedFirstStreamedFile) { hasReceivedFirstStreamedFile = true; } - const patchMetadata = getStreamedPatchMetadata(fileText); + const patchMetadata = getStreamedPatchMetadata(fileBytes); if (patchMetadata != null) { streamTreePathPrefix = getPatchTreePathPrefix( patchMetadata, @@ -417,7 +415,7 @@ export function usePatchLoader({ ); } - const fileDiff = processFile(fileText, { + const fileDiff = processFileBytes(fileBytes, { cacheKey: `${cacheKeyPrefix}-0-${accumulator.fileIndex}`, isGitDiff: true, }); diff --git a/apps/diffshub/lib/gitPatchMetadata.ts b/apps/diffshub/lib/gitPatchMetadata.ts index e1190b1bf..9fe9141cf 100644 --- a/apps/diffshub/lib/gitPatchMetadata.ts +++ b/apps/diffshub/lib/gitPatchMetadata.ts @@ -1,4 +1,4 @@ -export const COMMIT_HASH_METADATA_PATTERN = /^From\s+([a-f0-9]+)\s/im; +import { COMMIT_HASH_METADATA_PATTERN } from '@pierre/diffs'; const commitPrefixEncoder = new TextEncoder(); const commitPrefixDecoder = new TextDecoder(); diff --git a/apps/diffshub/lib/streamGitPatchFiles.ts b/apps/diffshub/lib/streamGitPatchFiles.ts deleted file mode 100644 index c5872d46d..000000000 --- a/apps/diffshub/lib/streamGitPatchFiles.ts +++ /dev/null @@ -1,243 +0,0 @@ -import { COMMIT_HASH_METADATA_PATTERN } from './gitPatchMetadata'; - -const GIT_FILE_BOUNDARY = 'diff --git '; -const GIT_FILE_BOUNDARY_WITH_NEWLINE = `\n${GIT_FILE_BOUNDARY}`; -const GIT_FILE_BOUNDARY_SCAN_OVERLAP = - GIT_FILE_BOUNDARY_WITH_NEWLINE.length - 1; -const NON_WHITESPACE_PATTERN = /\S/; - -export async function streamGitPatchFiles( - body: ReadableStream, - onFileText: (fileText: string) => Promise -): Promise { - const reader = body.getReader(); - const decoder = new TextDecoder(); - const parser = createGitPatchFileStreamParser(); - - try { - for (;;) { - const result = await reader.read(); - if (result.done) { - break; - } - if (result.value.byteLength > 0) { - parser.push(decoder.decode(result.value, { stream: true })); - await consumeAvailableStreamedFiles(parser, onFileText); - } - } - - const finalText = decoder.decode(); - if (finalText.length > 0) { - parser.push(finalText); - await consumeAvailableStreamedFiles(parser, onFileText); - } - const result = parser.finish(); - if (result.fileText != null) { - await onFileText(result.fileText); - } - let fileText: string | undefined; - while ((fileText = parser.takeAvailableFile()) != null) { - await onFileText(fileText); - } - return result.fallbackPatchContent; - } finally { - reader.releaseLock(); - } -} - -export function getStreamedPatchMetadata(fileText: string): string | undefined { - const diffBoundaryIndex = findNextGitFileBoundary(fileText, 0); - if (diffBoundaryIndex == null || diffBoundaryIndex <= 0) { - return undefined; - } - - const metadata = fileText.slice(0, diffBoundaryIndex); - return COMMIT_HASH_METADATA_PATTERN.test(metadata) ? metadata : undefined; -} - -interface GitPatchFileStreamFinishResult { - fallbackPatchContent?: string; - fileText?: string; -} - -interface GitPatchFileStreamParser { - finish(): GitPatchFileStreamFinishResult; - push(chunk: string): void; - takeAvailableFile(): string | undefined; -} - -async function consumeAvailableStreamedFiles( - parser: GitPatchFileStreamParser, - onFileText: (fileText: string) => Promise -): Promise { - let fileText: string | undefined; - while ((fileText = parser.takeAvailableFile()) != null) { - await onFileText(fileText); - } -} - -// Buffers the current file until the following `diff --git` header arrives so -// each parsed file is complete before it is appended to the viewer. -function createGitPatchFileStreamParser(): GitPatchFileStreamParser { - let buffer = ''; - let currentFileBoundaryIndex: number | undefined; - let nextBoundarySearchIndex = 0; - let sawFileBoundary = false; - - function takeAvailableFile(): string | undefined { - if (currentFileBoundaryIndex == null) { - currentFileBoundaryIndex = findNextGitFileBoundary( - buffer, - nextBoundarySearchIndex - ); - if (currentFileBoundaryIndex == null) { - nextBoundarySearchIndex = getNextBoundarySearchIndex(buffer, 0); - return undefined; - } - - sawFileBoundary = true; - nextBoundarySearchIndex = currentFileBoundaryIndex + 1; - } - - for (;;) { - const fileBoundaryIndex = currentFileBoundaryIndex; - if (fileBoundaryIndex == null) { - return undefined; - } - - const nextBoundaryIndex = findNextGitFileBoundary( - buffer, - nextBoundarySearchIndex - ); - if (nextBoundaryIndex == null) { - nextBoundarySearchIndex = getNextBoundarySearchIndex( - buffer, - fileBoundaryIndex + 1 - ); - return undefined; - } - - const splitIndex = getStreamedFileSplitIndex( - buffer, - fileBoundaryIndex, - nextBoundaryIndex - ); - const fileText = buffer.slice(0, splitIndex); - - buffer = buffer.slice(splitIndex); - currentFileBoundaryIndex = findNextGitFileBoundary(buffer, 0); - nextBoundarySearchIndex = - currentFileBoundaryIndex == null ? 0 : currentFileBoundaryIndex + 1; - if (NON_WHITESPACE_PATTERN.test(fileText)) { - return fileText; - } - } - } - - return { - push(chunk: string) { - if (chunk.length === 0) { - return; - } - buffer += chunk; - }, - takeAvailableFile, - finish() { - const fileText = takeAvailableFile(); - if (fileText != null) { - return { fileText }; - } - - if (!NON_WHITESPACE_PATTERN.test(buffer)) { - buffer = ''; - return {}; - } - if (!sawFileBoundary) { - const fullPatchText = buffer; - buffer = ''; - return { fallbackPatchContent: fullPatchText }; - } - - const finalFileText = buffer; - buffer = ''; - return { fileText: finalFileText }; - }, - }; -} - -function getNextBoundarySearchIndex( - text: string, - minimumIndex: number -): number { - return Math.max(minimumIndex, text.length - GIT_FILE_BOUNDARY_SCAN_OVERLAP); -} - -function findNextGitFileBoundary( - text: string, - fromIndex: number -): number | undefined { - const startIndex = Math.max(fromIndex, 0); - if (startIndex === 0 && text.startsWith(GIT_FILE_BOUNDARY)) { - return 0; - } - - const boundaryIndex = text.indexOf( - GIT_FILE_BOUNDARY_WITH_NEWLINE, - startIndex - ); - return boundaryIndex === -1 ? undefined : boundaryIndex + 1; -} - -function getStreamedFileSplitIndex( - text: string, - firstBoundaryIndex: number, - nextBoundaryIndex: number -): number { - return ( - findLastCommitMetadataBoundary( - text, - firstBoundaryIndex + 1, - nextBoundaryIndex - ) ?? nextBoundaryIndex - ); -} - -function findLastCommitMetadataBoundary( - text: string, - startIndex: number, - endIndex: number -): number | undefined { - const minimumBoundaryIndex = Math.max(startIndex, 0); - const maximumBoundaryIndex = Math.min(endIndex, text.length); - if (minimumBoundaryIndex >= maximumBoundaryIndex) { - return undefined; - } - - let newlineIndex = text.lastIndexOf('\nFrom ', maximumBoundaryIndex - 1); - for (;;) { - if (newlineIndex === -1) { - return undefined; - } - - const boundaryIndex = newlineIndex + 1; - if (boundaryIndex < minimumBoundaryIndex) { - return undefined; - } - if (boundaryIndex >= maximumBoundaryIndex) { - newlineIndex = text.lastIndexOf('\nFrom ', newlineIndex - 1); - continue; - } - - const lineEndIndex = text.indexOf('\n', boundaryIndex + 1); - const line = text.slice( - boundaryIndex, - lineEndIndex === -1 || lineEndIndex > maximumBoundaryIndex - ? maximumBoundaryIndex - : lineEndIndex - ); - if (COMMIT_HASH_METADATA_PATTERN.test(line)) { - return boundaryIndex; - } - newlineIndex = text.lastIndexOf('\nFrom ', newlineIndex - 1); - } -} diff --git a/packages/diffs/src/index.ts b/packages/diffs/src/index.ts index 3901bc0de..fce392789 100644 --- a/packages/diffs/src/index.ts +++ b/packages/diffs/src/index.ts @@ -100,6 +100,7 @@ export * from './utils/parseDiffDecorations'; export * from './utils/parseDiffFromFile'; export * from './utils/parseLineType'; export * from './utils/parsePatchFiles'; +export * from './utils/parsePatchStream'; export * from './utils/prefersReducedMotion'; export * from './utils/prerenderHTMLIfNecessary'; export * from './utils/processLine'; diff --git a/packages/diffs/src/utils/byteScan.ts b/packages/diffs/src/utils/byteScan.ts new file mode 100644 index 000000000..300ab2e7b --- /dev/null +++ b/packages/diffs/src/utils/byteScan.ts @@ -0,0 +1,100 @@ +// oxfmt-ignore +export const TAB: number = '\t'.charCodeAt(0), // 9 + NEWLINE: number = '\n'.charCodeAt(0), // 10 + VERTICAL_TAB: number = '\v'.charCodeAt(0), // 11 + FORM_FEED: number = '\f'.charCodeAt(0), // 12 + CARRIAGE_RETURN: number = '\r'.charCodeAt(0), // 13 + SPACE: number = ' '.charCodeAt(0), // 32 + PLUS: number = '+'.charCodeAt(0), // 43 + MINUS: number = '-'.charCodeAt(0), // 45 + BACKSLASH: number = '\\'.charCodeAt(0); // 92 + +// True when the bytes at `index` spell the ASCII `text`, compared byte by byte +// so neither a slice of `bytes` nor an encoding of `text` is ever allocated +export function matchesAscii( + bytes: Uint8Array, + index: number, + end: number, + text: string +): boolean { + if (index + text.length > end) { + return false; + } + for (let offset = 0; offset < text.length; offset++) { + if (bytes[index + offset] !== text.charCodeAt(offset)) { + return false; + } + } + return true; +} + +// End of the line starting at `index`, exclusive and including the newline. +// A newline at or beyond `end` is treated as absent, so a streaming buffer can +// pass its live cursor as `end` and ignore not-yet-consumed bytes +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L764-L770 +export function lineEndExclusive( + bytes: Uint8Array, + index: number, + end: number +): number { + const newlineIndex = bytes.indexOf(NEWLINE, index); + return newlineIndex === -1 || newlineIndex >= end ? end : newlineIndex + 1; +} + +// First line start at or after `from` (itself a line start) whose bytes begin +// with the ASCII `prefix`, or `end` when there is none +export function findNextLineStartingWith( + bytes: Uint8Array, + from: number, + end: number, + prefix: string +): number { + let lineStart = from; + while (lineStart < end) { + if (matchesAscii(bytes, lineStart, end, prefix)) { + return lineStart; + } + lineStart = lineEndExclusive(bytes, lineStart, end); + } + return end; +} + +// True when the byte range is an empty line: just a `\n`, a `\r\n`, or nothing +export function isBlankLine( + bytes: Uint8Array, + start: number, + end: number +): boolean { + const lineLength = end - start; + if (lineLength === 1) { + return bytes[start] === NEWLINE || bytes[start] === CARRIAGE_RETURN; + } + if (lineLength === 2) { + return bytes[start] === CARRIAGE_RETURN && bytes[start + 1] === NEWLINE; + } + return lineLength === 0; +} + +// True when `bytes` holds anything other than ASCII whitespace, used to drop the +// blank runs between format-patch files. It stands in for the string splitter's +// `/\S/` test but scans the bytes directly, so a file slice never has to be +// decoded to a string just to be checked. (`/\S/` would also match Unicode +// whitespace; only ASCII can appear in these separators, so checking the six +// ASCII whitespace bytes gives the same answer here) +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/apps/diffshub/lib/streamGitPatchFiles.ts#L7 +export function hasNonWhitespace(bytes: Uint8Array): boolean { + for (let index = 0; index < bytes.length; index++) { + const byte = bytes[index]; + if ( + byte !== SPACE && + byte !== TAB && + byte !== NEWLINE && + byte !== CARRIAGE_RETURN && + byte !== FORM_FEED && + byte !== VERTICAL_TAB + ) { + return true; + } + } + return false; +} diff --git a/packages/diffs/src/utils/diffLines.ts b/packages/diffs/src/utils/diffLines.ts index 2aa875840..e9792e516 100644 --- a/packages/diffs/src/utils/diffLines.ts +++ b/packages/diffs/src/utils/diffLines.ts @@ -21,6 +21,8 @@ // step, since there is no prototype to drop. Read a line with `lineAt` (or the // whole side with `joinLines`); build a `string[]` then seal it with `finishLines` +import { CARRIAGE_RETURN, NEWLINE } from './byteScan'; + export type DiffLines = { /** Number of lines (useful for compatibility between the two forms) */ length: number; @@ -263,11 +265,6 @@ export function finishLines( }; } -// Byte values the side builder compares against while accumulating lines -// oxfmt-ignore -const NEWLINE = '\n'.charCodeAt(0), // 10 - CARRIAGE_RETURN = '\r'.charCodeAt(0); // 13 - // Accumulates one side's line content during a byte parse: content bytes are // appended into `scratch` and each line's end offset lands in `offsets` // (entry 0 stays 0), so sealing is a copy of exactly the bytes written plus diff --git a/packages/diffs/src/utils/parsePatchFiles.ts b/packages/diffs/src/utils/parsePatchFiles.ts index 4652f048f..b6b7f7f15 100644 --- a/packages/diffs/src/utils/parsePatchFiles.ts +++ b/packages/diffs/src/utils/parsePatchFiles.ts @@ -14,6 +14,16 @@ import type { Hunk, ParsedPatch, } from '../types'; +import { + BACKSLASH, + findNextLineStartingWith, + isBlankLine, + lineEndExclusive, + matchesAscii, + MINUS, + PLUS, + SPACE, +} from './byteScan'; import { cleanLastNewline } from './cleanLastNewline'; import { detachString, releaseStringDetachBuffer } from './detachString'; import { @@ -1152,79 +1162,32 @@ function createContentGroup( }; } -// oxfmt-ignore -const NEWLINE = '\n'.charCodeAt(0), // 10 - CARRIAGE_RETURN = '\r'.charCodeAt(0), // 13 - SPACE = ' '.charCodeAt(0), // 32 - PLUS = '+'.charCodeAt(0), // 43 - MINUS = '-'.charCodeAt(0), // 45 - AT = '@'.charCodeAt(0), // 64 - BACKSLASH = '\\'.charCodeAt(0); // 92 - // Only metadata slices go through this decoder; `ignoreBOM` keeps a U+FEFF at -// the start of a slice intact, because mid-patch text can legitimately hold -// one (only a whole-stream decode should strip the stream-leading BOM) +// the start of a slice intact, because mid-patch text can truly hold one +// (only a whole-stream decode should strip the stream-leading BOM) const metadataDecoder = new TextDecoder('utf-8', { ignoreBOM: true }); +// A hunk header line begins with `@@ ` function isHunkLineAt( bytes: Uint8Array, index: number, length: number ): boolean { - return ( - index + 2 < length && - bytes[index] === AT && - bytes[index + 1] === AT && - bytes[index + 2] === SPACE - ); -} - -// End of the line starting at `index`, exclusive and including the newline -// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L764-L770 -function lineEndExclusive( - bytes: Uint8Array, - index: number, - length: number -): number { - const newlineIndex = bytes.indexOf(NEWLINE, index); - return newlineIndex === -1 ? length : newlineIndex + 1; + return matchesAscii(bytes, index, length, '@@ '); } // First line at or after `index` (itself a line start) that begins a hunk // (`@@ `), or `length` when there is none. Hunk chunks span from one such // line to the next -// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L910-L918 function findNextHunkLine( bytes: Uint8Array, index: number, length: number ): number { - let lineStart = index; - while (lineStart < length) { - if (isHunkLineAt(bytes, lineStart, length)) { - return lineStart; - } - lineStart = lineEndExclusive(bytes, lineStart, length); - } - return length; -} - -// A line that is only a line terminator (or empty at EOF). A run of these -// closing a hunk chunk (format-patch separators) is dropped silently; -// anywhere else they fall through to the invalid-line handling -// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L777-L790 -function isBlankLine(bytes: Uint8Array, start: number, end: number): boolean { - const lineLength = end - start; - if (lineLength === 1) { - return bytes[start] === NEWLINE || bytes[start] === CARRIAGE_RETURN; - } - if (lineLength === 2) { - return bytes[start] === CARRIAGE_RETURN && bytes[start + 1] === NEWLINE; - } - return lineLength === 0; + return findNextLineStartingWith(bytes, index, length, '@@ '); } -// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L345-L346 +// True when every line from `index` to the next hunk header (or end) is blank function restOfChunkIsBlank( bytes: Uint8Array, index: number, @@ -1248,34 +1211,5 @@ function restOfChunkIsBlank( // line start // Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/packages/diffs/src/utils/parsePatchFiles.ts#L889-L891 function isGitDiffBytes(bytes: Uint8Array, length: number): boolean { - const prefix = 'diff --git'; - let lineStart = 0; - while (lineStart < length) { - if (matchesAscii(bytes, lineStart, length, prefix)) { - return true; - } - const newlineIndex = bytes.indexOf(NEWLINE, lineStart); - if (newlineIndex === -1) { - return false; - } - lineStart = newlineIndex + 1; - } - return false; -} - -function matchesAscii( - bytes: Uint8Array, - index: number, - length: number, - text: string -): boolean { - if (index + text.length > length) { - return false; - } - for (let offset = 0; offset < text.length; offset++) { - if (bytes[index + offset] !== text.charCodeAt(offset)) { - return false; - } - } - return true; + return findNextLineStartingWith(bytes, 0, length, 'diff --git') < length; } diff --git a/packages/diffs/src/utils/parsePatchStream.ts b/packages/diffs/src/utils/parsePatchStream.ts new file mode 100644 index 000000000..488053628 --- /dev/null +++ b/packages/diffs/src/utils/parsePatchStream.ts @@ -0,0 +1,301 @@ +import { + findNextLineStartingWith, + hasNonWhitespace, + lineEndExclusive, + matchesAscii, + NEWLINE, +} from './byteScan'; + +// A format-patch commit boundary: `From ` at a line start +export const COMMIT_HASH_METADATA_PATTERN: RegExp = /^From\s+([a-f0-9]+)\s/im; + +const GIT_FILE_BOUNDARY = 'diff --git '; +const COMMIT_BOUNDARY = 'From '; + +// Only metadata slices and the non-git fallback go through this decoder. +// `ignoreBOM` matches parsePatchFiles, keeping a U+FEFF that appears mid-slice +// (the stream-leading BOM is stripped up front in `push`) +const metadataDecoder = new TextDecoder('utf-8', { ignoreBOM: true }); + +/** + * Read a git patch byte stream and call `onFileBytes` once per file, with the + * file's bytes ready for `processFileBytes`. Each slice is a view into the + * splitter's internal buffer and is only valid until `onFileBytes` resolves. + * Parse it (or copy it) before returning. Returns the whole patch as one + * decoded string instead when no `diff --git` boundary ever shows up (a + * unified diff), for `parsePatchFiles` + */ +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/apps/diffshub/lib/streamGitPatchFiles.ts#L9-L46 +export async function streamGitPatchFiles( + body: ReadableStream, + onFileBytes: (fileBytes: Uint8Array) => Promise +): Promise { + const reader = body.getReader(); + const parser = createGitPatchFileStreamParser(); + + try { + for (;;) { + const result = await reader.read(); + if (result.done) { + break; + } + if (result.value.byteLength > 0) { + parser.push(result.value); + await consumeAvailableStreamedFiles(parser, onFileBytes); + } + } + + const result = parser.finish(); + if (result.kind === 'file') { + await onFileBytes(result.fileBytes); + } + let fileBytes: Uint8Array | undefined; + while ((fileBytes = parser.takeAvailableFile()) != null) { + await onFileBytes(fileBytes); + } + return result.kind === 'fallback' ? result.fallbackPatchContent : undefined; + } finally { + reader.releaseLock(); + } +} + +// The spliter keeps a format-patch's commit metadata (the `From ` +// block) at the head of the commit's first file slice, so read it back from +// there, if any +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/apps/diffshub/lib/streamGitPatchFiles.ts#L48-L56 +export function getStreamedPatchMetadata( + fileBytes: Uint8Array +): string | undefined { + const boundaryIndex = findNextLineStartingWith( + fileBytes, + 0, + fileBytes.length, + GIT_FILE_BOUNDARY + ); + if (boundaryIndex <= 0 || boundaryIndex >= fileBytes.length) { + return undefined; + } + + const metadata = metadataDecoder.decode(fileBytes.subarray(0, boundaryIndex)); + return COMMIT_HASH_METADATA_PATTERN.test(metadata) ? metadata : undefined; +} + +// What finishing the stream yields. `finish` runs once per stream, not once per +// file: the last buffered file's bytes, the whole patch as one string when no +// `diff --git` boundary was ever seen (a plain unified diff), or nothing when +// only trailing whitespace remained +type PatchStreamFinish = + // not sure about the kind though... can't think of anything better + | { kind: 'file'; fileBytes: Uint8Array } + | { kind: 'fallback'; fallbackPatchContent: string } + | { kind: 'empty' }; + +interface GitPatchFileStreamParser { + finish(): PatchStreamFinish; + push(chunk: Uint8Array): void; + takeAvailableFile(): Uint8Array | undefined; +} + +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/apps/diffshub/lib/streamGitPatchFiles.ts#L69-L77 +async function consumeAvailableStreamedFiles( + parser: GitPatchFileStreamParser, + onFileBytes: (fileBytes: Uint8Array) => Promise +): Promise { + let fileBytes: Uint8Array | undefined; + while ((fileBytes = parser.takeAvailableFile()) != null) { + await onFileBytes(fileBytes); + } +} + +// Buffers the current file until the following `diff --git` header arrives so +// each parsed file is complete before it is appended to the viewer. Emitted +// slices are views into the internal buffer: they stay valid until the next +// `push`, which is enough because each file is consumed before more bytes are +// read from the stream +// Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/apps/diffshub/lib/streamGitPatchFiles.ts#L81-L166 +function createGitPatchFileStreamParser(): GitPatchFileStreamParser { + let buffer = new Uint8Array(1 << 16); + // Consumed/used window into `buffer`; emitted files start at `start` + let start = 0; + let end = 0; + // Absolute offset of the current file's `diff --git` line, or null until + // one is found + let currentFileBoundaryIndex: number | null = null; + // Line start where the boundary scan resumes (never re-scans settled bytes) + let nextBoundarySearchIndex = 0; + let sawFileBoundary = false; + let strippedLeadingBOM = false; + + function push(chunk: Uint8Array): void { + if (chunk.length === 0) { + return; + } + if (end + chunk.length > buffer.length) { + // Bytes before `start` are settled (emitted or skipped) and every scan + // position sits at or after `start`, so compaction can always drop them + const liveLength = end - start; + if (liveLength + chunk.length > buffer.length) { + let capacity = buffer.length * 2; + while (capacity < liveLength + chunk.length) { + capacity *= 2; + } + const grown = new Uint8Array(capacity); + grown.set(buffer.subarray(start, end)); + buffer = grown; + } else { + buffer.copyWithin(0, start, end); + } + end -= start; + nextBoundarySearchIndex -= start; + if (currentFileBoundaryIndex != null) { + currentFileBoundaryIndex -= start; + } + start = 0; + } + buffer.set(chunk, end); + end += chunk.length; + // A stream-leading UTF-8 BOM is the three bytes 0xEF 0xBB 0xBF (the UTF-8 + // encoding of U+FEFF; see https://en.wikipedia.org/wiki/Byte_order_mark). + // The string pipeline's TextDecoder drops it for free; this byte path never + // decodes the bulk, so strip it by hand to match, or a BOM'd patch would not + // match its first `diff --git` header + if (!strippedLeadingBOM && end - start >= 3) { + strippedLeadingBOM = true; + if ( + buffer[start] === 0xef && + buffer[start + 1] === 0xbb && + buffer[start + 2] === 0xbf + ) { + start += 3; + nextBoundarySearchIndex = Math.max(nextBoundarySearchIndex, start); + } + } + } + + // Next line start at or after `from` that begins with `diff --git `, or + // null. On a miss, records the first line whose terminator has not arrived + // yet as the resume point + // Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/apps/diffshub/lib/streamGitPatchFiles.ts#L168-L189 + function findBoundary(from: number): number | null { + let lineStart = from; + for (;;) { + const newlineIndex = buffer.indexOf(NEWLINE, lineStart); + if (newlineIndex === -1 || newlineIndex >= end) { + nextBoundarySearchIndex = lineStart; + return null; + } + if (matchesAscii(buffer, lineStart, end, GIT_FILE_BOUNDARY)) { + return lineStart; + } + lineStart = newlineIndex + 1; + } + } + + function takeAvailableFile(): Uint8Array | undefined { + if (currentFileBoundaryIndex == null) { + currentFileBoundaryIndex = findBoundary(nextBoundarySearchIndex); + if (currentFileBoundaryIndex == null) { + return undefined; + } + sawFileBoundary = true; + nextBoundarySearchIndex = lineEndExclusive( + buffer, + currentFileBoundaryIndex, + end + ); + } + + for (;;) { + const fileBoundaryIndex = currentFileBoundaryIndex; + if (fileBoundaryIndex == null) { + return undefined; + } + + const nextBoundaryIndex = findBoundary(nextBoundarySearchIndex); + if (nextBoundaryIndex == null) { + return undefined; + } + + const splitIndex = + findLastCommitMetadataBoundary( + fileBoundaryIndex + 1, + nextBoundaryIndex + ) ?? nextBoundaryIndex; + const fileBytes = buffer.subarray(start, splitIndex); + + start = splitIndex; + currentFileBoundaryIndex = nextBoundaryIndex; + nextBoundarySearchIndex = lineEndExclusive( + buffer, + nextBoundaryIndex, + end + ); + if (hasNonWhitespace(fileBytes)) { + return fileBytes; + } + } + } + + // Backwards scan for the last `From ` commit-metadata line strictly + // inside (startIndex, endIndex), so a new commit's metadata attaches to the + // commit's first file rather than the previous file. Walks line starts back + // from the end via each preceding newline and matches `From ` on each: the + // byte equivalent of the string splitter's backward `\nFrom ` search + // Adapted from: https://github.com/pierrecomputer/pierre/blob/844cf495ae18d43c45cc8bd4455224480017241a/apps/diffshub/lib/streamGitPatchFiles.ts#L205-L243 + function findLastCommitMetadataBoundary( + startIndex: number, + endIndex: number + ): number | undefined { + const minimumBoundaryIndex = Math.max(startIndex, start); + const maximumBoundaryIndex = Math.min(endIndex, end); + let searchIndex = maximumBoundaryIndex - 1; + while (searchIndex >= minimumBoundaryIndex) { + const newlineIndex = buffer.lastIndexOf(NEWLINE, searchIndex); + if (newlineIndex === -1) { + return undefined; + } + const candidate = newlineIndex + 1; + if ( + candidate >= minimumBoundaryIndex && + matchesAscii(buffer, candidate, end, COMMIT_BOUNDARY) + ) { + const lineEnd = lineEndExclusive(buffer, candidate, end); + const line = metadataDecoder.decode( + buffer.subarray(candidate, Math.min(lineEnd, maximumBoundaryIndex)) + ); + if (COMMIT_HASH_METADATA_PATTERN.test(line)) { + return candidate; + } + } + searchIndex = newlineIndex - 1; + } + return undefined; + } + + return { + push, + takeAvailableFile, + finish(): PatchStreamFinish { + const fileBytes = takeAvailableFile(); + if (fileBytes != null) { + return { kind: 'file', fileBytes }; + } + + const remaining = buffer.subarray(start, end); + if (!hasNonWhitespace(remaining)) { + start = end; + return { kind: 'empty' }; + } + if (!sawFileBoundary) { + start = end; + return { + kind: 'fallback', + fallbackPatchContent: metadataDecoder.decode(remaining), + }; + } + + start = end; + return { kind: 'file', fileBytes: remaining }; + }, + }; +} From 06d9e018f10c20973b2d76c1eaedd103e8653d83 Mon Sep 17 00:00:00 2001 From: clemg Date: Wed, 17 Jun 2026 18:43:50 +0200 Subject: [PATCH 5/5] test(diffs): cover the byte stream splitter and the byte parser Chunk-size sweeps over the file boundary splitting (including one-byte chunks), per-file model parity between the streamed bytes and the string entry, format-patch metadata attachment, stream-leading vs in-file BOM, the unified-diff fallback, and the invalid-UTF-8 read-back. --- packages/diffs/test/parsePatchStream.test.ts | 166 +++++++++++++++++++ packages/diffs/test/testUtils.ts | 14 +- 2 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 packages/diffs/test/parsePatchStream.test.ts diff --git a/packages/diffs/test/parsePatchStream.test.ts b/packages/diffs/test/parsePatchStream.test.ts new file mode 100644 index 000000000..d731f3320 --- /dev/null +++ b/packages/diffs/test/parsePatchStream.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, test } from 'bun:test'; + +import { linesToArray } from '../src/utils/diffLines'; +import { processFile, processFileBytes } from '../src/utils/parsePatchFiles'; +import { + getStreamedPatchMetadata, + streamGitPatchFiles, +} from '../src/utils/parsePatchStream'; +import { diffPatch, formatPatchWithVersionTrailer } from './mocks'; +import { assertDefined, withPlainFile } from './testUtils'; + +const encoder = new TextEncoder(); + +// Stream `bytes` through a ReadableStream with the given repeating chunk +// sizes, so file boundaries land in the middle of chunks, on chunk edges, and +// across one-byte chunks +function makeStream( + bytes: Uint8Array, + chunkSizes: number[] +): ReadableStream { + let offset = 0; + let chunkIndex = 0; + return new ReadableStream({ + pull(controller) { + if (offset >= bytes.length) { + controller.close(); + return; + } + const size = Math.min( + chunkSizes[chunkIndex++ % chunkSizes.length], + bytes.length - offset + ); + controller.enqueue(bytes.subarray(offset, offset + size)); + offset += size; + }, + }); +} + +async function collectFiles( + body: ReadableStream +): Promise<{ files: Uint8Array[]; fallback: string | undefined }> { + const files: Uint8Array[] = []; + const fallback = await streamGitPatchFiles(body, (fileBytes) => { + // Slices are views into the splitter's buffer and only valid until the + // next push, so keep copies like a real consumer would keep parsed models + files.push(fileBytes.slice()); + return Promise.resolve(); + }); + return { files, fallback }; +} + +describe('streamGitPatchFiles', () => { + test('splits a multi-file git patch into the same files for any chunking', async () => { + const patchBytes = encoder.encode(diffPatch); + const wholeFiles = (await collectFiles(makeStream(patchBytes, [1 << 20]))) + .files; + expect(wholeFiles.length).toBeGreaterThan(1); + + for (const chunkSizes of [[1], [7, 1, 64], [1024, 3]]) { + const { files, fallback } = await collectFiles( + makeStream(patchBytes, chunkSizes) + ); + expect(fallback).toBeUndefined(); + expect(files.length).toBe(wholeFiles.length); + for (let index = 0; index < files.length; index++) { + expect(files[index]).toEqual(wholeFiles[index]); + } + } + }); + + test('parses streamed file bytes into the same model as the patch string', async () => { + const patchBytes = encoder.encode(diffPatch); + const { files } = await collectFiles(makeStream(patchBytes, [333])); + // processFileBytes (byte arena) and processFile (string) parse each file + // slice into the same model + const decoder = new TextDecoder('utf-8', { ignoreBOM: true }); + const streamed = files.map((fileBytes) => { + const file = processFileBytes(fileBytes, { isGitDiff: true }); + assertDefined(file, 'expected a parsed file from bytes'); + return withPlainFile(file); + }); + const direct = files.map((fileBytes) => { + const file = processFile(decoder.decode(fileBytes), { isGitDiff: true }); + assertDefined(file, 'expected a parsed file from string'); + return withPlainFile(file); + }); + expect(streamed).toEqual(direct); + }); + + test('keeps format-patch commit metadata at the head of its first file', async () => { + const patch = `${formatPatchWithVersionTrailer}\n${formatPatchWithVersionTrailer}`; + const { files, fallback } = await collectFiles( + makeStream(encoder.encode(patch), [11]) + ); + expect(fallback).toBeUndefined(); + expect(files.length).toBe(2); + for (const fileBytes of files) { + const metadata = getStreamedPatchMetadata(fileBytes); + expect(metadata).toStartWith('From 02a2e4e6806f7e8f3adf685fde57cc7'); + } + }); + + test('strips a stream-leading BOM but keeps one inside a file', async () => { + const patch = [ + 'diff --git a/a.txt b/a.txt\n', + '--- a/a.txt\n', + '+++ b/a.txt\n', + '@@ -1 +1 @@\n', + '-old\n', + '+new\n', + ].join(''); + const { files } = await collectFiles( + makeStream(encoder.encode(patch), [2]) + ); + expect(files.length).toBe(1); + const file = processFileBytes(files[0], { isGitDiff: true }); + assertDefined(file, 'expected a parsed file'); + expect(file.name).toBe('a.txt'); + expect(linesToArray(file.additionLines)).toEqual(['new\n']); + }); + + test('falls back to one patch string when no git boundary exists', async () => { + const unified = [ + '--- a.txt\n', + '+++ a.txt\n', + '@@ -1 +1 @@\n', + '-old\n', + '+new\n', + ].join(''); + const { files, fallback } = await collectFiles( + makeStream(encoder.encode(unified), [3]) + ); + expect(files.length).toBe(0); + expect(fallback).toBe(unified); + }); + + test('skips whitespace-only leading segments', async () => { + const patch = `\n\n \ndiff --git a/a.txt b/a.txt\n--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-x\n+y\n`; + const { files, fallback } = await collectFiles( + makeStream(encoder.encode(patch), [5]) + ); + expect(fallback).toBeUndefined(); + expect(files.length).toBe(1); + }); +}); + +describe('processFileBytes', () => { + test('keeps invalid UTF-8 in the arena and reads it back as U+FFFD', () => { + const head = encoder.encode( + [ + 'diff --git a/bin.txt b/bin.txt\n', + '--- a/bin.txt\n', + '+++ b/bin.txt\n', + '@@ -0,0 +1 @@\n', + '+ab', + ].join('') + ); + const fileBytes = new Uint8Array(head.length + 2); + fileBytes.set(head); + fileBytes[head.length] = 0xff; // not valid UTF-8 anywhere + fileBytes[head.length + 1] = 0x0a; + const file = processFileBytes(fileBytes, { isGitDiff: true }); + assertDefined(file, 'expected a parsed file'); + expect(linesToArray(file.additionLines)).toEqual(['ab�\n']); + }); +}); diff --git a/packages/diffs/test/testUtils.ts b/packages/diffs/test/testUtils.ts index 43ae8b846..a75dcd5c3 100644 --- a/packages/diffs/test/testUtils.ts +++ b/packages/diffs/test/testUtils.ts @@ -15,14 +15,18 @@ type PlainFile = Omit & { }; type PlainPatch = Omit & { files: PlainFile[] }; +export function withPlainFile(file: FileDiffMetadata): PlainFile { + return { + ...file, + additionLines: linesToArray(file.additionLines), + deletionLines: linesToArray(file.deletionLines), + }; +} + export function withPlainLines(patches: ParsedPatch[]): PlainPatch[] { return patches.map((patch) => ({ ...patch, - files: patch.files.map((file) => ({ - ...file, - additionLines: linesToArray(file.additionLines), - deletionLines: linesToArray(file.deletionLines), - })), + files: patch.files.map(withPlainFile), })); }