From ac5915339a7ce5f640b5b6a9aaf885e5f20c2ee0 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Mon, 13 Jul 2026 18:48:14 -0700 Subject: [PATCH 01/12] First follow up task An attempt to ensure the edited diff better matches the fully parsed diff at the end. It's not perfect, and I think it can still break in certain ways, but I think it's a bit better. Will need more testing. I am a little worried about this code, it touches a lot of things, but it does appear to fix things... It's one of those things that I don't fully understand, which scares me a little. --- packages/diffs/src/components/FileDiff.ts | 22 +- .../diffs/src/renderers/DiffHunksRenderer.ts | 101 +- packages/diffs/src/utils/editSessionHunks.ts | 918 ++++++++++-------- .../diffs/src/utils/getHunkSideBoundaries.ts | 8 + .../src/utils/getTotalLineCountFromHunks.ts | 5 +- .../diffs/src/utils/hydratePartialDiff.ts | 18 +- packages/diffs/src/utils/iterateOverDiff.ts | 42 +- .../utils/parseMergeConflictDiffFromFile.ts | 17 +- packages/diffs/src/utils/parsePatchFiles.ts | 17 +- packages/diffs/src/utils/resolveRegion.ts | 48 +- packages/diffs/src/utils/updateDiffHunks.ts | 20 +- packages/diffs/src/utils/virtualDiffLayout.ts | 47 +- .../diffs/test/CodeView.rangeScroll.test.ts | 58 ++ .../test/DiffHunksRendererRecompute.test.ts | 140 ++- .../diffs/test/diffAcceptRejectHunk.test.ts | 44 + packages/diffs/test/editSessionHunks.test.ts | 717 +++++++++----- .../diffs/test/getHunkSideBoundaries.test.ts | 35 + packages/diffs/test/iterateOverDiff.test.ts | 36 + packages/diffs/test/testUtils.ts | 15 +- 19 files changed, 1547 insertions(+), 761 deletions(-) create mode 100644 packages/diffs/src/utils/getHunkSideBoundaries.ts create mode 100644 packages/diffs/test/getHunkSideBoundaries.test.ts diff --git a/packages/diffs/src/components/FileDiff.ts b/packages/diffs/src/components/FileDiff.ts index dcdfb8d13..9b38338ab 100644 --- a/packages/diffs/src/components/FileDiff.ts +++ b/packages/diffs/src/components/FileDiff.ts @@ -82,6 +82,7 @@ import { } from '../utils/editSessionHunks'; import { getDiffFileInput } from '../utils/getDiffFileInput'; import { getDiffHunksRendererOptions } from '../utils/getDiffHunksRendererOptions'; +import { getHunkSideStartBoundary } from '../utils/getHunkSideBoundaries'; import { getLineAnnotationName } from '../utils/getLineAnnotationName'; import { getOrCreateCodeNode } from '../utils/getOrCreateCodeNode'; import { upsertHostThemeStyle } from '../utils/hostTheme'; @@ -96,6 +97,7 @@ import { getMeasuredScrollbarGutter } from '../utils/scrollbarGutter'; import { setPreNodeProperties } from '../utils/setWrapperNodeProps'; import { getExpandedRegion, + getHunkAdditionLineRange, getNearestRenderableAdditionLine, getTrailingExpandedRegion, isAdditionLineRenderable, @@ -358,10 +360,12 @@ export class FileDiff< let targetUnifiedIndex: number | undefined; let targetSplitIndex: number | undefined; hunkIterator: for (const hunk of fileDiff.hunks) { - let currentLineNumber = + const hunkStart = side === 'deletions' ? hunk.deletionStart : hunk.additionStart; const hunkCount = side === 'deletions' ? hunk.deletionCount : hunk.additionCount; + let currentLineNumber = + getHunkSideStartBoundary(hunkStart, hunkCount) + 1; let splitIndex = hunk.splitLineStart; let unifiedIndex = hunk.unifiedLineStart; @@ -1324,7 +1328,7 @@ export class FileDiff< // Edit sessions are a plain-diff concern; subclasses with their own hunk // semantics (merge conflicts) keep the per-edit recompute pipeline. if (this.type === 'file-diff') { - this.hunksRenderer.beginEditSession(this.fileDiffCache); + this.hunksRenderer.beginEditSession(); } // The editor sync below refuses partial diffs (it needs the full file // contents); kick off hydration so the loaded re-render re-runs it. @@ -1515,7 +1519,8 @@ export class FileDiff< const expandedHunks = this.hunksRenderer.getExpandedHunksMap(); for (const [hunkIndex, hunk] of fileDiff.hunks.entries()) { - if (lineNumber < hunk.additionStart) { + const [hunkStart, hunkEnd] = getHunkAdditionLineRange(hunk); + if (lineNumber < hunkStart) { const region = getExpandedRegion({ isPartial: fileDiff.isPartial, rangeSize: hunk.collapsedBefore, @@ -1523,18 +1528,17 @@ export class FileDiff< hunkIndex, collapsedContextThreshold, }); - const gapStart = hunk.additionStart - region.rangeSize; + const gapStart = hunkStart - region.rangeSize; if ( region.renderAll || lineNumber < gapStart + region.fromStart || - lineNumber >= hunk.additionStart - region.fromEnd + lineNumber >= hunkStart - region.fromEnd ) { return false; } const fromStartDistance = lineNumber - (gapStart + region.fromStart) + 1; - const fromEndDistance = - hunk.additionStart - region.fromEnd - lineNumber; + const fromEndDistance = hunkStart - region.fromEnd - lineNumber; if (fromStartDistance <= fromEndDistance) { this.expandHunk( hunkIndex, @@ -1550,7 +1554,7 @@ export class FileDiff< } return true; } - if (lineNumber < hunk.additionStart + hunk.additionCount) { + if (lineNumber < hunkEnd) { return false; } } @@ -1566,7 +1570,7 @@ export class FileDiff< return false; } const lastHunk = fileDiff.hunks[fileDiff.hunks.length - 1]; - const trailingStart = lastHunk.additionStart + lastHunk.additionCount; + const [, trailingStart] = getHunkAdditionLineRange(lastHunk); if ( lineNumber < trailingStart + trailingRegion.fromStart || lineNumber >= trailingStart + trailingRegion.rangeSize diff --git a/packages/diffs/src/renderers/DiffHunksRenderer.ts b/packages/diffs/src/renderers/DiffHunksRenderer.ts index 8f80d66cf..32fcd94df 100644 --- a/packages/diffs/src/renderers/DiffHunksRenderer.ts +++ b/packages/diffs/src/renderers/DiffHunksRenderer.ts @@ -53,9 +53,7 @@ import { createPreElement } from '../utils/createPreElement'; import { createSeparator } from '../utils/createSeparator'; import { applySessionChangedLines, - applySessionEditWindow, - findChangedLineWindow, - normalizeEditorLines, + rebuildSessionHunks, remapExpandedHunksForRegionChange, type SessionRegionChange, } from '../utils/editSessionHunks'; @@ -83,7 +81,6 @@ import { iterateOverDiff } from '../utils/iterateOverDiff'; import { renderDiffWithHighlighter } from '../utils/renderDiffWithHighlighter'; import { shouldUseTokenTransformer } from '../utils/shouldUseTokenTransformer'; import { - preserveTrailingEditorBlankLine, recomputeDiffHunksForEdit, recomputeEmptyDocumentDiff, recomputeTopAlignedAdditionDiff, @@ -243,11 +240,6 @@ export class DiffHunksRenderer { // Edit-session state: while active, hunk updates go through the frozen // region skeleton (editSessionHunks) instead of the full recompute. private editSessionActive = false; - // Addition lines as of the last completed hunk-update pass, used by the - // prefix/suffix scan in applyDocumentChange. Line-count passes write dirty - // line text at stale indexes into `diff.additionLines` before the document - // rebuild lands, so the live array can never serve as the "before" side. - private editSessionLines: string[] | undefined; constructor( public options: DiffHunksRendererOptions = { theme: DEFAULT_THEMES }, @@ -284,22 +276,15 @@ export class DiffHunksRenderer { /** * Enter edit-session mode: hunk updates preserve the current region * skeleton instead of recomputing hunks. Called on every editor attach, - * including a re-attach after recycle, which losslessly re-seeds the pass - * snapshot because both hunk-update paths keep `diff.additionLines` - * current. + * including a re-attach after recycle. */ - public beginEditSession(diff: FileDiffMetadata | undefined): void { + public beginEditSession(): void { this.editSessionActive = true; - this.editSessionLines = - diff != null - ? normalizeEditorLines(diff.additionLines).slice() - : undefined; } /** Leave edit-session mode. The exit recompute is the host's concern. */ public endEditSession(): void { this.editSessionActive = false; - this.editSessionLines = undefined; } public get diffCache(): FileDiffMetadata | undefined { @@ -416,6 +401,7 @@ export class DiffHunksRenderer { const hastLines = result.code.additionLines; const changedAdditionLines: number[] = []; + const previousAdditionLines = new Map(); for (const [line, tokens] of dirtyLines) { const prev = hastLines[line] as HASTElement | undefined; const prevProps = prev?.properties ?? {}; @@ -430,6 +416,7 @@ export class DiffHunksRenderer { diff.additionLines[line] = applyLineTextWithNewline(prevLine, lineText); if (prevText !== lineText) { changedAdditionLines.push(line); + previousAdditionLines.set(line, prevLine); } } hastLines[line] = { @@ -475,16 +462,39 @@ export class DiffHunksRenderer { // genuine changes and are never followed by applyDocumentChange) the // explicit changed indexes are current. if (!lineCountChangeInFlight) { - const changes = applySessionChangedLines( - diff, - changedAdditionLines, - this.options.parseDiffOptions - ); - this.applyExpansionRemaps(changes); - regionsChanged = changes.length > 0; - this.editSessionLines = normalizeEditorLines( - diff.additionLines - ).slice(); + if ( + diff.additionLines.length <= 1 && + diff.additionLines.join('') === '' + ) { + Object.assign( + diff, + recomputeEmptyDocumentDiff(diff, this.options.parseDiffOptions) + ); + this.markEditSessionPass(diff); + regionsChanged = true; + } else if ( + shouldTopAlignAdditionRecompute(diff, diff.additionLines) + ) { + Object.assign( + diff, + recomputeTopAlignedAdditionDiff( + diff, + diff.additionLines, + this.options.parseDiffOptions + ) + ); + this.markEditSessionPass(diff); + regionsChanged = true; + } else { + const change = applySessionChangedLines( + diff, + changedAdditionLines, + this.options.parseDiffOptions, + previousAdditionLines + ); + this.applyExpansionRemap(change); + regionsChanged = change != null; + } } } else { Object.assign( @@ -503,8 +513,8 @@ export class DiffHunksRenderer { return regionsChanged; } - private applyExpansionRemaps(changes: SessionRegionChange[]): void { - for (const change of changes) { + private applyExpansionRemap(change: SessionRegionChange | undefined): void { + if (change != null) { this.expandedHunks = remapExpandedHunksForRegionChange( this.expandedHunks, change @@ -569,9 +579,8 @@ export class DiffHunksRenderer { this.renderCache.isDirty = true; } - // Session-mode counterpart of the line-count recompute: locate the changed - // window with a prefix/suffix scan of the pass snapshot against the rebuilt - // document lines, then update only the region skeleton it covers. + // Session-mode counterpart of the line-count recompute: derive canonical + // old/current pairing and rebuild the old-side region skeleton from it. private applySessionDocumentChange(diff: FileDiffMetadata): void { const { parseDiffOptions } = this.options; const rawLines = diff.additionLines; @@ -583,36 +592,16 @@ export class DiffHunksRenderer { this.markEditSessionPass(diff); return; } - const previousLines = this.editSessionLines; - const nextLines = normalizeEditorLines(rawLines); - if (previousLines == null) { - // No pass snapshot (the editor attached without diff data): fall back - // to the full recompute for this pass and start tracking from it. - Object.assign(diff, recomputeDiffHunksForEdit(diff, parseDiffOptions)); - this.markEditSessionPass(diff); - return; - } - diff.additionLines = nextLines; - const window = findChangedLineWindow(previousLines, nextLines); - if (window != null) { - const change = applySessionEditWindow(diff, window, parseDiffOptions); - if (change != null) { - this.applyExpansionRemaps([change]); - } - } - preserveTrailingEditorBlankLine(diff, rawLines); - this.editSessionLines = nextLines.slice(); + this.applyExpansionRemap(rebuildSessionHunks(diff, parseDiffOptions)); } - // Records a session pass that replaced hunks wholesale (empty-document / - // top-aligned shims or a snapshotless fallback): the skeleton collapsed to - // the recompute result, and the pass snapshot restarts from it. + // Records a session pass that replaced hunks wholesale (empty-document or + // top-aligned shims). private markEditSessionPass(diff: FileDiffMetadata): void { if (!this.editSessionActive) { return; } diff.editSessionDirty = true; - this.editSessionLines = normalizeEditorLines(diff.additionLines).slice(); } protected getUnifiedLineDecoration({ diff --git a/packages/diffs/src/utils/editSessionHunks.ts b/packages/diffs/src/utils/editSessionHunks.ts index ce483047e..12070712b 100644 --- a/packages/diffs/src/utils/editSessionHunks.ts +++ b/packages/diffs/src/utils/editSessionHunks.ts @@ -7,10 +7,11 @@ import type { Hunk, HunkExpansionRegion, } from '../types'; +import { getHunkSideStartBoundary } from './getHunkSideBoundaries'; import { parseDiffFromFile } from './parseDiffFromFile'; -import { slideBlankBoundaryBlocksUp } from './realignChangeContent'; import { offsetHunkContent, + preserveTrailingEditorBlankLine, recomputeDiffHunksForEdit, recomputeDiffRenderLineCounts, recomputeHunkRenderLineCounts, @@ -21,34 +22,26 @@ import { getTrailingExpandedRegion, } from './virtualDiffLayout'; -// While an editor is attached to a FileDiff, hunks are treated as a frozen -// "region skeleton": each hunk is one region spanning its full range, regions -// never merge/split/drop on their own, and each edit re-diffs only the region -// it lands in. A region whose re-diff comes back empty persists as a -// context-only hunk so its rows keep rendering. The functions here implement -// that per-edit region math; the real hunk recompute runs once on genuine -// session exit (finishEditSessionForDiff). +// While an editor is attached to a FileDiff, each hunk is a persistent region +// identified by its old-side range. Structural passes rebuild those regions +// from one canonical old/current diff; a reverted region remains as context so +// its rows keep rendering until the genuine session-exit recompute. -/** - * The addition-line span an edit changed, from a common prefix/suffix scan. - * `start` is the first changed line (identical in pre/post-edit coordinates), - * `prevEnd`/`nextEnd` are the exclusive ends in pre/post-edit lines. - */ -export interface ChangedLineWindow { +export interface DivergenceCore { start: number; - prevEnd: number; - nextEnd: number; + deletionEnd: number; + additionEnd: number; } -/** A structural change to the region skeleton (rendered row set changed). */ -export type SessionRegionChange = - | { - type: 'merge'; - firstIndex: number; - lastIndex: number; - previousHunkCount: number; - } - | { type: 'insert'; index: number; previousHunkCount: number }; +interface PreviousRegionSpan { + firstIndex: number; + lastIndex: number; +} + +/** Maps rebuilt regions back to the previous skeleton for expansion remapping. */ +export interface SessionRegionChange { + regions: Array; +} interface RegionBounds { additionStart: number; @@ -57,6 +50,18 @@ interface RegionBounds { deletionEnd: number; } +interface RegionPlan { + deletionStart: number; + deletionEnd: number; + blocks: ChangeContent[]; + previousSpan: PreviousRegionSpan | undefined; +} + +const deletionLineSetCache = new WeakMap< + FileDiffMetadata, + { lines: string[]; set: Set } +>(); + /** * Drops the editor document's phantom trailing empty line (a document ending * in a newline exposes one extra empty line the parsed diff never contains) @@ -70,243 +75,223 @@ export function normalizeEditorLines(lines: string[]): string[] { } /** - * Common prefix/suffix scan between the last completed pass's lines and the - * rebuilt document lines. Returns undefined when nothing changed. + * Find the complete old/current divergence core. The old side is immutable + * during an edit session, so this result needs no prior-pass snapshot. */ -export function findChangedLineWindow( - previousLines: string[], - nextLines: string[] -): ChangedLineWindow | undefined { - const maxStart = Math.min(previousLines.length, nextLines.length); +export function findDivergenceCore( + deletionLines: string[], + additionLines: string[] +): DivergenceCore | undefined { + const maxStart = Math.min(deletionLines.length, additionLines.length); let start = 0; - while (start < maxStart && previousLines[start] === nextLines[start]) { + while (start < maxStart && deletionLines[start] === additionLines[start]) { start++; } - let prevEnd = previousLines.length; - let nextEnd = nextLines.length; + let deletionEnd = deletionLines.length; + let additionEnd = additionLines.length; while ( - prevEnd > start && - nextEnd > start && - previousLines[prevEnd - 1] === nextLines[nextEnd - 1] + deletionEnd > start && + additionEnd > start && + deletionLines[deletionEnd - 1] === additionLines[additionEnd - 1] ) { - prevEnd--; - nextEnd--; + deletionEnd--; + additionEnd--; } - if (start === prevEnd && start === nextEnd) { + if (start === deletionEnd && start === additionEnd) { return undefined; } - return { start, prevEnd, nextEnd }; + return { start, deletionEnd, additionEnd }; } /** - * Applies one contiguous changed window to the session skeleton: regions the - * window touches merge/grow to cover it (absorbing unchanged gap lines in - * paired old/new correspondence), a window wholly inside a gap synthesizes a - * new region, and regions after the window shift by the line delta. The - * covered region is re-diffed in place. Returns the structural change when - * the region set itself changed shape (merge/growth/synthesis). + * Rebuild the session skeleton as a pure function of the immutable old lines, + * current new lines, and old-side ranges of the previous regions. One + * canonical parse supplies the same change blocks that session exit will use. */ -export function applySessionEditWindow( +export function rebuildSessionHunks( diff: FileDiffMetadata, - window: ChangedLineWindow, parseDiffOptions?: CreatePatchOptionsNonabortable ): SessionRegionChange | undefined { - const { hunks } = diff; - const delta = window.nextEnd - window.prevEnd; - diff.editSessionDirty = true; - - if (hunks.length === 0) { - const bounds: RegionBounds = { - additionStart: 0, - additionEnd: diff.additionLines.length, - deletionStart: 0, - deletionEnd: diff.deletionLines.length, - }; - hunks.push(rediffRegion(diff, bounds, parseDiffOptions)); - finalizeSessionHunks(diff, 0); - return { type: 'insert', index: 0, previousHunkCount: 0 }; - } - - // Locate the span of regions the window touches, treating windows adjacent - // to a region edge as touching so boundary edits grow the region instead of - // synthesizing a sibling next to it. - let firstIndex = -1; - let lastIndex = -1; - for (let index = 0; index < hunks.length; index++) { - const hunk = hunks[index]; - const regionStart = hunk.additionLineIndex; - const regionEnd = regionStart + hunk.additionCount; - if (regionStart > window.prevEnd) { - break; - } - if (window.start <= regionEnd) { - if (firstIndex === -1) { - firstIndex = index; - } - lastIndex = index; - } - } - - if (firstIndex === -1) { - return synthesizeRegion(diff, window, delta, parseDiffOptions); - } - - const firstHunk = hunks[firstIndex]; - const lastHunk = hunks[lastIndex]; - const lastHunkEnd = lastHunk.additionLineIndex + lastHunk.additionCount; - const additionStart = Math.min(firstHunk.additionLineIndex, window.start); - const additionEndBefore = Math.max(lastHunkEnd, window.prevEnd); - // Gap lines are 1:1 paired with old-side lines, so growing a region into a - // gap absorbs the same number of deletion lines from that gap. - const bounds: RegionBounds = { - additionStart, - additionEnd: additionEndBefore + delta, - deletionStart: - firstHunk.deletionLineIndex - - (firstHunk.additionLineIndex - additionStart), - deletionEnd: - lastHunk.deletionLineIndex + - lastHunk.deletionCount + - (additionEndBefore - lastHunkEnd), - }; - const regionHunk = rediffRegion(diff, bounds, parseDiffOptions); - if (delta !== 0) { - for (let index = lastIndex + 1; index < hunks.length; index++) { - shiftHunkAdditionCoords(hunks[index], delta); - } - } - const previousHunkCount = hunks.length; - hunks.splice(firstIndex, lastIndex - firstIndex + 1, regionHunk); - finalizeSessionHunks(diff, firstIndex); + const previousHunks = diff.hunks; + const editorAdditionLines = diff.additionLines; + const canonicalAdditionLines = normalizeEditorLines(editorAdditionLines); + const canonicalDiff = + canonicalAdditionLines === editorAdditionLines + ? diff + : { ...diff, additionLines: canonicalAdditionLines }; + const blocks = parseCanonicalChangeBlocks(canonicalDiff, parseDiffOptions); + const plans = buildRegionPlans( + previousHunks, + blocks, + diff.deletionLines.length + ); - const regionGrew = - additionStart < firstHunk.additionLineIndex || - additionEndBefore > lastHunkEnd; - if (firstIndex === lastIndex && !regionGrew) { + const nextHunks = buildRegionHunks(canonicalDiff, plans); + diff.additionLines = canonicalAdditionLines; + diff.hunks = nextHunks; + diff.editSessionDirty = true; + finalizeSessionHunks(diff); + preserveTrailingEditorBlankLine(diff, editorAdditionLines); + const layoutChanged = hasRegionOrSplitLayoutChanged( + previousHunks, + diff.hunks + ); + if (!layoutChanged) { return undefined; } - return { type: 'merge', firstIndex, lastIndex, previousHunkCount }; + return { regions: plans.map((plan) => plan.previousSpan) }; } /** - * Applies a set of changed addition-line indexes from a pass with no line - * count change: lines inside a region re-diff that region in place; lines in - * a gap synthesize a region per gap (one window spanning that gap's changed - * lines). Returns the structural changes, if any, for escalation/remapping. + * Keep a cheap content-only path when a same-line-count pass cannot alter the + * canonical blocks. Gap, ambiguous, or multi-region edits rebuild statelessly. */ export function applySessionChangedLines( diff: FileDiffMetadata, changedAdditionLineIndexes: Iterable, - parseDiffOptions?: CreatePatchOptionsNonabortable -): SessionRegionChange[] { + parseDiffOptions?: CreatePatchOptionsNonabortable, + previousAdditionLines?: ReadonlyMap +): SessionRegionChange | undefined { const lines = Array.from(new Set(changedAdditionLineIndexes)) .filter((line) => line >= 0 && line < diff.additionLines.length) .sort((a, b) => a - b); if (lines.length === 0) { - return []; + return undefined; } const { hunks } = diff; - const regionIndexes = new Set(); - // Changed lines outside every region, grouped per gap (keyed by the index - // of the region that follows the gap) into one [start, end) window each. - const gapWindows: Array<[start: number, end: number]> = []; + let regionIndex: number | undefined; let hunkIndex = 0; - let currentGapKey = -1; for (const line of lines) { - while ( - hunkIndex < hunks.length && - line >= - hunks[hunkIndex].additionLineIndex + hunks[hunkIndex].additionCount - ) { + while (hunkIndex < hunks.length) { + const hunk = hunks[hunkIndex]; + const start = getHunkAdditionStart(hunk); + if (line < start + hunk.additionCount) break; hunkIndex++; } const hunk = hunks[hunkIndex]; - if (hunk != null && line >= hunk.additionLineIndex) { - regionIndexes.add(hunkIndex); - continue; - } - const lastWindow = gapWindows[gapWindows.length - 1]; - if (currentGapKey === hunkIndex && lastWindow != null) { - lastWindow[1] = line + 1; - } else { - currentGapKey = hunkIndex; - gapWindows.push([line, line + 1]); + const start = hunk == null ? undefined : getHunkAdditionStart(hunk); + if ( + start == null || + line < start || + (regionIndex != null && regionIndex !== hunkIndex) + ) { + return rebuildSessionHunks(diff, parseDiffOptions); } + regionIndex = hunkIndex; } - // In-place region re-diffs first, while region indexes are still valid. - for (const index of regionIndexes) { - const hunk = hunks[index]; - const bounds: RegionBounds = { - additionStart: hunk.additionLineIndex, - additionEnd: hunk.additionLineIndex + hunk.additionCount, - deletionStart: hunk.deletionLineIndex, - deletionEnd: hunk.deletionLineIndex + hunk.deletionCount, - }; - hunks[index] = rediffRegion(diff, bounds, parseDiffOptions); - syncHunkNoEOFCRFromFullFile(diff, index); - diff.editSessionDirty = true; - } - if (regionIndexes.size > 0) { - recomputeDiffRenderLineCounts(diff); + if (regionIndex == null) { + return undefined; } - - // Gap windows in descending order so earlier windows keep valid coordinates - // while later synthesis inserts hunks behind them. - const changes: SessionRegionChange[] = []; - for (let index = gapWindows.length - 1; index >= 0; index--) { - const [start, end] = gapWindows[index]; - const change = applySessionEditWindow( + if ( + canRetainCanonicalBlocks( diff, - { start, prevEnd: end, nextEnd: end }, + lines, + regionIndex, + previousAdditionLines, parseDiffOptions - ); - if (change != null) { - changes.push(change); + ) + ) { + diff.editSessionDirty = true; + return undefined; + } + return rebuildSessionHunks(diff, parseDiffOptions); +} + +// Existing balanced change blocks remain canonical when every edited line was +// unmatched before and stays unmatched now: the old/new equality matrix is +// unchanged, and similarity realignment never reorders a balanced block. +function canRetainCanonicalBlocks( + diff: FileDiffMetadata, + changedLines: number[], + regionIndex: number, + previousAdditionLines: ReadonlyMap | undefined, + parseDiffOptions: CreatePatchOptionsNonabortable | undefined +): boolean { + if ( + previousAdditionLines == null || + parseDiffOptions?.ignoreWhitespace === true || + parseDiffOptions?.stripTrailingCr === true + ) { + return false; + } + const deletionLineSet = getDeletionLineSet(diff); + const hunk = diff.hunks[regionIndex]; + if (hunk.hunkContent.some(isPureChange)) { + return false; + } + for (const line of changedLines) { + const previousLine = previousAdditionLines.get(line); + const additionLine = diff.additionLines[line]; + if ( + previousLine == null || + additionLine == null || + deletionLineSet.has(previousLine) || + deletionLineSet.has(additionLine) + ) { + return false; + } + let insideBalancedChange = false; + for (const content of hunk.hunkContent) { + if ( + content.type === 'change' && + content.additions === content.deletions && + line >= content.additionLineIndex && + line < content.additionLineIndex + content.additions + ) { + insideBalancedChange = true; + break; + } + } + if (!insideBalancedChange) { + return false; } } - return changes; + return true; } -/** - * Rebuilds hunk-gap expansion keys after a structural region change so - * expansion state stays anchored to the same gaps. Merged-away gap keys drop; - * a synthesized region splits its gap's expansion across the two new gaps. - * The trailing pseudo-key (old hunk count) follows the same shift. - */ +function getDeletionLineSet(diff: FileDiffMetadata): Set { + const cached = deletionLineSetCache.get(diff); + if (cached?.lines === diff.deletionLines) { + return cached.set; + } + const set = new Set(diff.deletionLines); + deletionLineSetCache.set(diff, { lines: diff.deletionLines, set }); + return set; +} + +function isPureChange( + content: ContextContent | ChangeContent | undefined +): boolean { + return ( + content?.type === 'change' && + (content.additions === 0 || content.deletions === 0) + ); +} + +/** Preserve expansion at the surviving outer edges of rebuilt old-side gaps. */ export function remapExpandedHunksForRegionChange( expandedHunks: Map, change: SessionRegionChange ): Map { const remapped = new Map(); - if (change.type === 'merge') { - const removed = change.lastIndex - change.firstIndex; - for (const [key, region] of expandedHunks) { - if (key <= change.firstIndex) { - remapped.set(key, region); - } else if (key > change.lastIndex) { - remapped.set(key - removed, region); - } - // Keys inside (firstIndex, lastIndex] were gaps the merge absorbed. - } - return remapped; - } - for (const [key, region] of expandedHunks) { - if (key < change.index) { - remapped.set(key, region); - } else if (key > change.index) { - remapped.set(key + 1, region); - } else { - // The gap this key described was split by the new region: keep its - // start-anchored expansion on the first half and its end-anchored - // expansion on the second. - if (region.fromStart > 0) { - remapped.set(key, { fromStart: region.fromStart, fromEnd: 0 }); - } - if (region.fromEnd > 0) { - remapped.set(key + 1, { fromStart: 0, fromEnd: region.fromEnd }); - } + const { regions } = change; + for (let key = 0; key <= regions.length; key++) { + const previous = regions[key - 1]; + const next = regions[key]; + const fromStartSource = + key === 0 + ? expandedHunks.get(0) + : previous == null + ? undefined + : expandedHunks.get(previous.lastIndex + 1); + const fromEndSource = + next == null ? undefined : expandedHunks.get(next.firstIndex); + const fromStart = fromStartSource?.fromStart ?? 0; + const fromEnd = fromEndSource?.fromEnd ?? 0; + if (fromStart > 0 || fromEnd > 0) { + remapped.set(key, { fromStart, fromEnd }); } } return remapped; @@ -343,7 +328,7 @@ export function captureExpansionAnchors( if (region.rangeSize <= collapsedContextThreshold) { continue; } - const gapEnd = hunk.deletionLineIndex; + const gapEnd = getHunkDeletionStart(hunk); const gapStart = gapEnd - region.rangeSize; if (region.fromStart > 0) { anchors.push([gapStart, gapStart + region.fromStart]); @@ -365,7 +350,7 @@ export function captureExpansionAnchors( trailingRegion.rangeSize > collapsedContextThreshold ) { const lastHunk = diff.hunks[diff.hunks.length - 1]; - const gapStart = lastHunk.deletionLineIndex + lastHunk.deletionCount; + const gapStart = getHunkDeletionStart(lastHunk) + lastHunk.deletionCount; anchors.push([gapStart, gapStart + trailingRegion.fromStart]); } return anchors; @@ -407,17 +392,14 @@ export function rebuildExpansionFromAnchors( } }; for (const [hunkIndex, hunk] of diff.hunks.entries()) { - applyGap( - hunkIndex, - hunk.deletionLineIndex - Math.max(hunk.collapsedBefore, 0), - hunk.deletionLineIndex - ); + const gapEnd = getHunkDeletionStart(hunk); + applyGap(hunkIndex, gapEnd - Math.max(hunk.collapsedBefore, 0), gapEnd); } const lastHunk = diff.hunks[diff.hunks.length - 1]; if (lastHunk != null && !diff.isPartial && diff.deletionLines.length > 0) { applyGap( diff.hunks.length, - lastHunk.deletionLineIndex + lastHunk.deletionCount, + getHunkDeletionStart(lastHunk) + lastHunk.deletionCount, diff.deletionLines.length ); } @@ -441,181 +423,262 @@ export function finishEditSessionForDiff( return true; } -// Synthesizes a region for a window that touches no existing region. When the -// window is empty on either side (pure insert/delete inside a gap), one -// adjacent context line is absorbed so the re-diff anchors on real content. -function synthesizeRegion( +// Parse the complete old/current files once with the same context policy as +// session exit. Equal boundary lines affect Myers tie-breaking, while parsed +// context is required for the blank-run alignment post-pass; only canonical +// change blocks are retained. Running cursors normalize unified N,0 indexes. +function parseCanonicalChangeBlocks( diff: FileDiffMetadata, - window: ChangedLineWindow, - delta: number, parseDiffOptions?: CreatePatchOptionsNonabortable -): SessionRegionChange { - const { hunks } = diff; - let insertIndex = 0; - while ( - insertIndex < hunks.length && - hunks[insertIndex].additionLineIndex < window.start - ) { - insertIndex++; - } - - // Constant pairing offset between old and new line indexes within this gap. - const pairOffset = - insertIndex < hunks.length - ? hunks[insertIndex].deletionLineIndex - - hunks[insertIndex].additionLineIndex - : hunks[insertIndex - 1].deletionLineIndex + - hunks[insertIndex - 1].deletionCount - - (hunks[insertIndex - 1].additionLineIndex + - hunks[insertIndex - 1].additionCount); - - let { start, prevEnd, nextEnd } = window; - if (start === prevEnd || start === nextEnd) { - if (start > 0) { - start--; - } else if (nextEnd < diff.additionLines.length) { - prevEnd++; - nextEnd++; - } +): ChangeContent[] { + if (findDivergenceCore(diff.deletionLines, diff.additionLines) == null) { + return []; } + const parsed = parseDiffFromFile( + { + name: diff.prevName ?? diff.name, + contents: diff.deletionLines.join(''), + }, + { + name: diff.name, + contents: diff.additionLines.join(''), + lang: diff.lang, + }, + parseDiffOptions + ); - const bounds: RegionBounds = { - additionStart: start, - additionEnd: nextEnd, - deletionStart: start + pairOffset, - deletionEnd: prevEnd + pairOffset, - }; - const regionHunk = rediffRegion(diff, bounds, parseDiffOptions); - if (delta !== 0) { - for (let index = insertIndex; index < hunks.length; index++) { - shiftHunkAdditionCoords(hunks[index], delta); + const blocks: ChangeContent[] = []; + let coveredAdditions = 0; + let coveredDeletions = 0; + for (const hunk of parsed.hunks) { + const contextLines = + hunk.additionCount > 0 + ? hunk.additionLineIndex - coveredAdditions + : hunk.deletionLineIndex - coveredDeletions; + coveredAdditions += contextLines; + coveredDeletions += contextLines; + for (const content of hunk.hunkContent) { + if (content.type === 'context') { + coveredAdditions += content.lines; + coveredDeletions += content.lines; + continue; + } + const block = offsetHunkContent(content, 0, 0) as ChangeContent; + if (block.additions === 0) { + block.additionLineIndex = coveredAdditions; + } + if (block.deletions === 0) { + block.deletionLineIndex = coveredDeletions; + } + blocks.push(block); + coveredAdditions += block.additions; + coveredDeletions += block.deletions; } } - const previousHunkCount = hunks.length; - hunks.splice(insertIndex, 0, regionHunk); - finalizeSessionHunks(diff, insertIndex); - return { type: 'insert', index: insertIndex, previousHunkCount }; + return blocks; } -// Re-diffs a region's old/new slices with zero context, producing one Hunk -// spanning the region's full range. Zero result hunks yield a context-only -// hunk (region persists while the session is active); several result hunks -// merge into one hunkContent with the identical stretches between them -// re-expressed as context blocks. -function rediffRegion( - diff: FileDiffMetadata, - bounds: RegionBounds, - parseDiffOptions?: CreatePatchOptionsNonabortable -): Hunk { - const deletionSlice = diff.deletionLines.slice( - bounds.deletionStart, - bounds.deletionEnd - ); - const additionSlice = diff.additionLines.slice( - bounds.additionStart, - bounds.additionEnd - ); - const additionCount = additionSlice.length; - const deletionCount = deletionSlice.length; - - const hunkContent: (ContextContent | ChangeContent)[] = []; - let additionChangedLines = 0; - let deletionChangedLines = 0; - - const pushContext = ( - lines: number, - additionLineIndex: number, - deletionLineIndex: number - ) => { - if (lines > 0) { - hunkContent.push({ - type: 'context', - lines, - additionLineIndex, - deletionLineIndex, - }); +// Co-walk canonical blocks and previous old-side regions. A block touching a +// region grows it; touching several merges them; a block wholly in a gap gets +// its own region. Pure insertions/deletions absorb one adjacent context line +// when available so the region stays renderable on both sides and after undo. +function buildRegionPlans( + previousHunks: Hunk[], + blocks: ChangeContent[], + deletionLineCount: number +): RegionPlan[] { + const previousPlans = previousHunks.map((hunk, index): RegionPlan => { + const deletionStart = getHunkDeletionStart(hunk); + return { + deletionStart, + deletionEnd: deletionStart + hunk.deletionCount, + blocks: [], + previousSpan: { firstIndex: index, lastIndex: index }, + }; + }); + const plans: RegionPlan[] = []; + let previousIndex = 0; + + for (const block of blocks) { + const blockStart = block.deletionLineIndex; + const blockEnd = blockStart + block.deletions; + while ( + previousIndex < previousPlans.length && + previousPlans[previousIndex].deletionEnd < blockStart + ) { + plans.push(previousPlans[previousIndex]); + previousIndex++; } - }; - if (deletionSlice.join('') === additionSlice.join('')) { - pushContext(additionCount, bounds.additionStart, bounds.deletionStart); - } else { - const reparsed = parseDiffFromFile( - { - name: diff.prevName ?? diff.name, - contents: deletionSlice.join(''), - }, - { - name: diff.name, - contents: additionSlice.join(''), - lang: diff.lang, - }, - { ...parseDiffOptions, context: 0 } - ); - // Walk the zero-context hunks, re-deriving the unchanged stretches - // between them (context is paired, so both sides advance by the same - // amount). The stretch length must come from a side the hunk has content - // on: a zero-count side's start follows the unified `N,0` convention - // (the line *before* the change), so its lineIndex is one short of the - // lines consumed ahead of the hunk. - let coveredAdditions = 0; - let coveredDeletions = 0; - for (const parsedHunk of reparsed.hunks) { - const contextLines = - parsedHunk.additionCount > 0 - ? parsedHunk.additionLineIndex - coveredAdditions - : parsedHunk.deletionLineIndex - coveredDeletions; - pushContext( - contextLines, - bounds.additionStart + coveredAdditions, - bounds.deletionStart + coveredDeletions - ); - coveredAdditions += contextLines; - coveredDeletions += contextLines; - for (const content of parsedHunk.hunkContent) { - // Parsed content indexes are relative to the slice; offset them into - // full-file coordinates. A zero-count side carries the `N,0` - // convention (one short of the lines consumed), so pin it to the - // running counter instead. - const offset = offsetHunkContent( - content, - bounds.additionStart, - bounds.deletionStart - ); - if (offset.type === 'change') { - if (offset.additions === 0) { - offset.additionLineIndex = bounds.additionStart + coveredAdditions; - } - if (offset.deletions === 0) { - offset.deletionLineIndex = bounds.deletionStart + coveredDeletions; - } + let plan = + plans.length > 0 && + blockTouchesRegion(blockStart, blockEnd, plans[plans.length - 1]) + ? plans.pop() + : undefined; + while ( + previousIndex < previousPlans.length && + previousPlans[previousIndex].deletionStart <= blockEnd + ) { + plan = mergeRegionPlans(plan, previousPlans[previousIndex]); + previousIndex++; + } + + if (plan == null) { + let deletionStart = blockStart; + let deletionEnd = blockEnd; + if ( + (block.deletions === 0 || block.additions === 0) && + deletionLineCount > block.deletions + ) { + const previousEnd = plans[plans.length - 1]?.deletionEnd ?? 0; + const nextStart = + previousPlans[previousIndex]?.deletionStart ?? deletionLineCount; + if (blockStart > previousEnd) { + deletionStart--; + } else if (blockEnd < nextStart) { + deletionEnd++; } - hunkContent.push(offset); } - additionChangedLines += parsedHunk.additionLines; - deletionChangedLines += parsedHunk.deletionLines; - coveredAdditions += parsedHunk.additionCount; - coveredDeletions += parsedHunk.deletionCount; + plan = { + deletionStart, + deletionEnd, + blocks: [], + previousSpan: undefined, + }; } - pushContext( - additionCount - coveredAdditions, - bounds.additionStart + coveredAdditions, - bounds.deletionStart + coveredDeletions + plan.deletionStart = Math.min(plan.deletionStart, blockStart); + plan.deletionEnd = Math.max(plan.deletionEnd, blockEnd); + plan.blocks.push(block); + plans.push(plan); + } + + while (previousIndex < previousPlans.length) { + plans.push(previousPlans[previousIndex]); + previousIndex++; + } + return plans; +} + +function blockTouchesRegion( + blockStart: number, + blockEnd: number, + region: RegionPlan +): boolean { + return blockStart <= region.deletionEnd && blockEnd >= region.deletionStart; +} + +function mergeRegionPlans( + target: RegionPlan | undefined, + source: RegionPlan +): RegionPlan { + if (target == null) { + return source; + } + target.deletionStart = Math.min(target.deletionStart, source.deletionStart); + target.deletionEnd = Math.max(target.deletionEnd, source.deletionEnd); + target.blocks.push(...source.blocks); + if (source.previousSpan != null) { + target.previousSpan ??= { ...source.previousSpan }; + target.previousSpan.firstIndex = Math.min( + target.previousSpan.firstIndex, + source.previousSpan.firstIndex + ); + target.previousSpan.lastIndex = Math.max( + target.previousSpan.lastIndex, + source.previousSpan.lastIndex ); } + return target; +} + +// One paired-context walk constructs every region's new-side range. There is +// no downstream coordinate shifting: each boundary is derived from the +// canonical blocks that precede it. +function buildRegionHunks(diff: FileDiffMetadata, plans: RegionPlan[]): Hunk[] { + const hunks: Hunk[] = []; + let deletionCursor = 0; + let additionCursor = 0; + for (const plan of plans) { + const contextBefore = plan.deletionStart - deletionCursor; + if (contextBefore < 0) { + throw new Error('buildRegionHunks: overlapping old-side regions'); + } + deletionCursor += contextBefore; + additionCursor += contextBefore; + const additionStart = additionCursor; + const hunkContent: Array = []; + + for (const canonicalBlock of plan.blocks) { + const deletionContext = canonicalBlock.deletionLineIndex - deletionCursor; + const additionContext = canonicalBlock.additionLineIndex - additionCursor; + if (deletionContext < 0 || deletionContext !== additionContext) { + throw new Error('buildRegionHunks: canonical block context mismatch'); + } + pushContext(hunkContent, deletionContext, additionCursor, deletionCursor); + deletionCursor += deletionContext; + additionCursor += additionContext; + hunkContent.push({ ...canonicalBlock }); + deletionCursor += canonicalBlock.deletions; + additionCursor += canonicalBlock.additions; + } + const trailingContext = plan.deletionEnd - deletionCursor; + if (trailingContext < 0) { + throw new Error('buildRegionHunks: block exceeds its old-side region'); + } + pushContext(hunkContent, trailingContext, additionCursor, deletionCursor); + deletionCursor += trailingContext; + additionCursor += trailingContext; + hunks.push( + createRegionHunk( + diff, + { + additionStart, + additionEnd: additionCursor, + deletionStart: plan.deletionStart, + deletionEnd: plan.deletionEnd, + }, + hunkContent + ) + ); + } + + if ( + diff.deletionLines.length - deletionCursor !== + diff.additionLines.length - additionCursor + ) { + throw new Error('buildRegionHunks: trailing context mismatch'); + } + return hunks; +} + +function createRegionHunk( + diff: FileDiffMetadata, + bounds: RegionBounds, + hunkContent: Array +): Hunk { + const additionCount = bounds.additionEnd - bounds.additionStart; + const deletionCount = bounds.deletionEnd - bounds.deletionStart; + let additionLines = 0; + let deletionLines = 0; + for (const content of hunkContent) { + if (content.type === 'change') { + additionLines += content.additions; + deletionLines += content.deletions; + } + } const hunk: Hunk = { collapsedBefore: 0, - additionStart: bounds.additionStart + 1, + additionStart: getUnifiedStart(bounds.additionStart, additionCount), additionCount, - additionLines: additionChangedLines, - additionLineIndex: bounds.additionStart, - deletionStart: bounds.deletionStart + 1, + additionLines, + additionLineIndex: getUnifiedLineIndex(bounds.additionStart, additionCount), + deletionStart: getUnifiedStart(bounds.deletionStart, deletionCount), deletionCount, - deletionLines: deletionChangedLines, - deletionLineIndex: bounds.deletionStart, + deletionLines, + deletionLineIndex: getUnifiedLineIndex(bounds.deletionStart, deletionCount), hunkContent, - hunkSpecs: `@@ -${bounds.deletionStart + 1},${deletionCount} +${bounds.additionStart + 1},${additionCount} @@`, + hunkSpecs: `@@ -${getUnifiedStart(bounds.deletionStart, deletionCount)},${deletionCount} +${getUnifiedStart(bounds.additionStart, additionCount)},${additionCount} @@`, splitLineStart: 0, splitLineCount: 0, unifiedLineStart: 0, @@ -623,23 +686,116 @@ function rediffRegion( noEOFCRAdditions: false, noEOFCRDeletions: false, }; - // The inner parse runs with zero context, so blank-run slides can only - // apply once the context blocks are reassembled here — keeping the session - // rendering identical to the exit parse, which slides in its own post-pass. - slideBlankBoundaryBlocksUp(hunk, diff); recomputeHunkRenderLineCounts(hunk); return hunk; } -function shiftHunkAdditionCoords(hunk: Hunk, delta: number): void { - hunk.additionLineIndex += delta; - hunk.additionStart += delta; +function pushContext( + hunkContent: Array, + lines: number, + additionLineIndex: number, + deletionLineIndex: number +): void { + if (lines > 0) { + hunkContent.push({ + type: 'context', + lines, + additionLineIndex, + deletionLineIndex, + }); + } +} + +function hasRegionOrSplitLayoutChanged( + previous: Hunk[], + next: Hunk[] +): boolean { + if (previous.length !== next.length) { + return true; + } + for (let index = 0; index < previous.length; index++) { + const previousHunk = previous[index]; + const nextHunk = next[index]; + if ( + getHunkDeletionStart(previousHunk) !== getHunkDeletionStart(nextHunk) || + previousHunk.deletionCount !== nextHunk.deletionCount || + getHunkAdditionStart(previousHunk) !== getHunkAdditionStart(nextHunk) || + previousHunk.additionCount !== nextHunk.additionCount || + previousHunk.splitLineCount !== nextHunk.splitLineCount || + !haveSameSplitRowMapping(previousHunk, nextHunk) + ) { + return true; + } + } + return false; +} + +function haveSameSplitRowMapping(previous: Hunk, next: Hunk): boolean { + const previousRows = iterateSplitRowMapping(previous); + const nextRows = iterateSplitRowMapping(next); + while (true) { + const previousRow = previousRows.next(); + const nextRow = nextRows.next(); + if (previousRow.done === true || nextRow.done === true) { + return previousRow.done === nextRow.done; + } + if ( + previousRow.value[0] !== nextRow.value[0] || + previousRow.value[1] !== nextRow.value[1] + ) { + return false; + } + } +} + +function* iterateSplitRowMapping( + hunk: Hunk +): Generator< + [deletionLine: number | undefined, additionLine: number | undefined] +> { for (const content of hunk.hunkContent) { - content.additionLineIndex += delta; + if (content.type === 'context') { + for (let offset = 0; offset < content.lines; offset++) { + yield [ + content.deletionLineIndex + offset, + content.additionLineIndex + offset, + ]; + } + continue; + } + const rowCount = Math.max(content.deletions, content.additions); + for (let offset = 0; offset < rowCount; offset++) { + yield [ + offset < content.deletions + ? content.deletionLineIndex + offset + : undefined, + offset < content.additions + ? content.additionLineIndex + offset + : undefined, + ]; + } } } -function finalizeSessionHunks(diff: FileDiffMetadata, hunkIndex: number): void { +function getHunkAdditionStart(hunk: Hunk): number { + return getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount); +} + +function getHunkDeletionStart(hunk: Hunk): number { + return getHunkSideStartBoundary(hunk.deletionStart, hunk.deletionCount); +} + +function getUnifiedStart(lineIndex: number, count: number): number { + return count === 0 ? lineIndex : lineIndex + 1; +} + +function getUnifiedLineIndex(lineIndex: number, count: number): number { + return count === 0 ? lineIndex - 1 : lineIndex; +} + +function finalizeSessionHunks(diff: FileDiffMetadata): void { recomputeDiffRenderLineCounts(diff); - syncHunkNoEOFCRFromFullFile(diff, hunkIndex); + for (let index = 0; index < diff.hunks.length; index++) { + syncHunkNoEOFCRFromFullFile(diff, index); + } } diff --git a/packages/diffs/src/utils/getHunkSideBoundaries.ts b/packages/diffs/src/utils/getHunkSideBoundaries.ts new file mode 100644 index 000000000..2d34b786d --- /dev/null +++ b/packages/diffs/src/utils/getHunkSideBoundaries.ts @@ -0,0 +1,8 @@ +/** Converts a unified hunk side's start/count into its consumed-file range. */ +export function getHunkSideStartBoundary(start: number, count: number): number { + return start - (count === 0 ? 0 : 1); +} + +export function getHunkSideEndBoundary(start: number, count: number): number { + return getHunkSideStartBoundary(start, count) + count; +} diff --git a/packages/diffs/src/utils/getTotalLineCountFromHunks.ts b/packages/diffs/src/utils/getTotalLineCountFromHunks.ts index f6bfc946f..67bbc5105 100644 --- a/packages/diffs/src/utils/getTotalLineCountFromHunks.ts +++ b/packages/diffs/src/utils/getTotalLineCountFromHunks.ts @@ -1,4 +1,5 @@ import type { Hunk } from '../types'; +import { getHunkSideEndBoundary } from './getHunkSideBoundaries'; export function getTotalLineCountFromHunks(hunks: Hunk[]): number { const lastHunk = hunks.at(-1); @@ -6,7 +7,7 @@ export function getTotalLineCountFromHunks(hunks: Hunk[]): number { return 0; } return Math.max( - lastHunk.additionStart + lastHunk.additionCount, - lastHunk.deletionStart + lastHunk.deletionCount + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount), + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount) ); } diff --git a/packages/diffs/src/utils/hydratePartialDiff.ts b/packages/diffs/src/utils/hydratePartialDiff.ts index fa4c686c0..83955e300 100644 --- a/packages/diffs/src/utils/hydratePartialDiff.ts +++ b/packages/diffs/src/utils/hydratePartialDiff.ts @@ -5,6 +5,10 @@ import type { Hunk, } from '../types'; import { cloneFileDiffMetadata } from './cloneFileDiffMetadata'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; import { splitFileContents } from './splitFileContents'; interface HydratedHunksResult { @@ -122,7 +126,8 @@ function hydrateHunks( } const collapsedBefore = Math.max( - hunk.additionStart - 1 - lastHunkAdditionEnd, + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) - + lastHunkAdditionEnd, 0 ); hydratedHunks.push({ @@ -141,14 +146,17 @@ function hydrateHunks( splitLineCount += collapsedBefore + hunkSplitLineCount; unifiedLineCount += collapsedBefore + hunkUnifiedLineCount; - lastHunkAdditionEnd = hunk.additionStart + hunk.additionCount - 1; + lastHunkAdditionEnd = getHunkSideEndBoundary( + hunk.additionStart, + hunk.additionCount + ); } if (hydratedHunks.length > 0) { const lastHunk = hydratedHunks[hydratedHunks.length - 1]; - const lastHunkEnd = Math.max( - lastHunk.additionStart + lastHunk.additionCount - 1, - 0 + const lastHunkEnd = getHunkSideEndBoundary( + lastHunk.additionStart, + lastHunk.additionCount ); const collapsedAfter = Math.max(totalAdditionLines - lastHunkEnd, 0); splitLineCount += collapsedAfter; diff --git a/packages/diffs/src/utils/iterateOverDiff.ts b/packages/diffs/src/utils/iterateOverDiff.ts index cc53349cf..62a5fe1ac 100644 --- a/packages/diffs/src/utils/iterateOverDiff.ts +++ b/packages/diffs/src/utils/iterateOverDiff.ts @@ -5,6 +5,7 @@ import type { Hunk, HunkExpansionRegion, } from '../types'; +import { getHunkSideStartBoundary } from './getHunkSideBoundaries'; import { getExpandedRegion, getTrailingExpandedRegion, @@ -230,6 +231,23 @@ export function iterateOverDiff({ break; } + const deletionBoundary = getHunkSideStartBoundary( + hunk.deletionStart, + hunk.deletionCount + ); + const additionBoundary = getHunkSideStartBoundary( + hunk.additionStart, + hunk.additionCount + ); + const deletionStartIndex = + !diff.isPartial && hunk.deletionCount === 0 + ? deletionBoundary + : hunk.deletionLineIndex; + const additionStartIndex = + !diff.isPartial && hunk.additionCount === 0 + ? additionBoundary + : hunk.additionLineIndex; + const leadingRegion = getExpandedRegion({ isPartial: diff.isPartial, rangeSize: hunk.collapsedBefore, @@ -285,10 +303,10 @@ export function iterateOverDiff({ let unifiedLineIndex = hunk.unifiedLineStart - leadingRegion.rangeSize; let splitLineIndex = hunk.splitLineStart - leadingRegion.rangeSize; - let deletionLineIndex = hunk.deletionLineIndex - leadingRegion.rangeSize; - let additionLineIndex = hunk.additionLineIndex - leadingRegion.rangeSize; - let deletionLineNumber = hunk.deletionStart - leadingRegion.rangeSize; - let additionLineNumber = hunk.additionStart - leadingRegion.rangeSize; + let deletionLineIndex = deletionStartIndex - leadingRegion.rangeSize; + let additionLineIndex = additionStartIndex - leadingRegion.rangeSize; + let deletionLineNumber = deletionBoundary + 1 - leadingRegion.rangeSize; + let additionLineNumber = additionBoundary + 1 - leadingRegion.rangeSize; if ( walkContextLines(state, leadingRegion.fromStart, diffStyle, (index) => { @@ -321,10 +339,10 @@ export function iterateOverDiff({ unifiedLineIndex = hunk.unifiedLineStart - leadingRegion.fromEnd; splitLineIndex = hunk.splitLineStart - leadingRegion.fromEnd; - deletionLineIndex = hunk.deletionLineIndex - leadingRegion.fromEnd; - additionLineIndex = hunk.additionLineIndex - leadingRegion.fromEnd; - deletionLineNumber = hunk.deletionStart - leadingRegion.fromEnd; - additionLineNumber = hunk.additionStart - leadingRegion.fromEnd; + deletionLineIndex = deletionStartIndex - leadingRegion.fromEnd; + additionLineIndex = additionStartIndex - leadingRegion.fromEnd; + deletionLineNumber = deletionBoundary + 1 - leadingRegion.fromEnd; + additionLineNumber = additionBoundary + 1 - leadingRegion.fromEnd; if ( walkContextLines( state, @@ -371,10 +389,10 @@ export function iterateOverDiff({ let unifiedLineIndex = hunk.unifiedLineStart; let splitLineIndex = hunk.splitLineStart; - let deletionLineIndex = hunk.deletionLineIndex; - let additionLineIndex = hunk.additionLineIndex; - let deletionLineNumber = hunk.deletionStart; - let additionLineNumber = hunk.additionStart; + let deletionLineIndex = deletionStartIndex; + let additionLineIndex = additionStartIndex; + let deletionLineNumber = deletionBoundary + 1; + let additionLineNumber = additionBoundary + 1; const lastContent = hunk.hunkContent.at(-1); for (const content of hunk.hunkContent) { diff --git a/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts b/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts index c4438efbe..6daeab170 100644 --- a/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts +++ b/packages/diffs/src/utils/parseMergeConflictDiffFromFile.ts @@ -47,6 +47,10 @@ import type { MergeConflictRegion, ProcessFileConflictData, } from '../types'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; export interface ParseMergeConflictDiffFromFileResult { fileDiff: FileDiffMetadata; @@ -275,7 +279,7 @@ export function parseMergeConflictDiffFromFile( const lastHunk = s.hunks[s.hunks.length - 1]; const collapsedAfter = Math.max( s.additionLines.length - - (lastHunk.additionStart + lastHunk.additionCount - 1), + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount), 0 ); s.splitLineCount += collapsedAfter; @@ -594,7 +598,11 @@ function finalizeActiveHunk(s: ParseState): void { } } - const collapsedBefore = Math.max(hunk.additionStart - 1 - s.lastHunkEnd, 0); + const collapsedBefore = Math.max( + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) - + s.lastHunkEnd, + 0 + ); const finalizedHunk: Hunk = { collapsedBefore, additionStart: hunk.additionStart, @@ -619,7 +627,10 @@ function finalizeActiveHunk(s: ParseState): void { s.hunks.push(finalizedHunk); s.splitLineCount += collapsedBefore + hunkSplitLineCount; s.unifiedLineCount += collapsedBefore + hunkUnifiedLineCount; - s.lastHunkEnd = hunk.additionStart + hunk.additionCount - 1; + s.lastHunkEnd = getHunkSideEndBoundary( + hunk.additionStart, + hunk.additionCount + ); } // Called when the context buffer between two changes exceeds maxContextLines*2. diff --git a/packages/diffs/src/utils/parsePatchFiles.ts b/packages/diffs/src/utils/parsePatchFiles.ts index 1f52141e1..da54f13f6 100644 --- a/packages/diffs/src/utils/parsePatchFiles.ts +++ b/packages/diffs/src/utils/parsePatchFiles.ts @@ -17,6 +17,10 @@ import type { } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; import { detachString, releaseStringDetachBuffer } from './detachString'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; import { realignChangeContentBySimilarity } from './realignChangeContent'; interface ParsedHunkHeader { @@ -489,11 +493,15 @@ function _processFile( hunkData.deletionLines = deletionLines; hunkData.collapsedBefore = Math.max( - hunkData.additionStart - 1 - lastHunkEnd, + getHunkSideStartBoundary(hunkData.additionStart, hunkData.additionCount) - + lastHunkEnd, 0 ); currentFile.hunks.push(hunkData); - lastHunkEnd = hunkData.additionStart + hunkData.additionCount - 1; + lastHunkEnd = getHunkSideEndBoundary( + hunkData.additionStart, + hunkData.additionCount + ); for (const content of hunkData.hunkContent) { if (content.type === 'context') { hunkData.splitLineCount += content.lines; @@ -537,7 +545,10 @@ function _processFile( currentFile.deletionLines.length > 0 ) { const lastHunk = currentFile.hunks[currentFile.hunks.length - 1]; - const lastHunkEnd = lastHunk.additionStart + lastHunk.additionCount - 1; + const lastHunkEnd = getHunkSideEndBoundary( + lastHunk.additionStart, + lastHunk.additionCount + ); const totalFileLines = currentFile.additionLines.length; const collapsedAfter = Math.max(totalFileLines - lastHunkEnd, 0); currentFile.splitLineCount += collapsedAfter; diff --git a/packages/diffs/src/utils/resolveRegion.ts b/packages/diffs/src/utils/resolveRegion.ts index 3b9783c0e..0776d617e 100644 --- a/packages/diffs/src/utils/resolveRegion.ts +++ b/packages/diffs/src/utils/resolveRegion.ts @@ -4,6 +4,10 @@ import type { FileDiffMetadata, Hunk, } from '../types'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; interface RegionResolutionTarget { hunkIndex: number; @@ -81,8 +85,10 @@ export function resolveRegion( diff, resolvedDiff, cursor, - hunk.deletionLineIndex - hunk.collapsedBefore, - hunk.additionLineIndex - hunk.collapsedBefore, + getHunkSideStartBoundary(hunk.deletionStart, hunk.deletionCount) - + hunk.collapsedBefore, + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) - + hunk.collapsedBefore, hunk.collapsedBefore, shouldProcessCollapsedContext ); @@ -185,24 +191,46 @@ export function resolveRegion( newHunk.noEOFCRDeletions = noEOFCR; } + if (newHunk.additionCount === 0) { + newHunk.additionStart--; + if (!diff.isPartial) { + newHunk.additionLineIndex--; + } + } + if (newHunk.deletionCount === 0) { + newHunk.deletionStart--; + if (!diff.isPartial) { + newHunk.deletionLineIndex--; + } + } + resolvedDiff.hunks.push(newHunk); } const finalHunk = hunks.at(-1); if (finalHunk != null && !diff.isPartial) { + const finalDeletionEnd = getHunkSideEndBoundary( + finalHunk.deletionStart, + finalHunk.deletionCount + ); + const finalAdditionEnd = getHunkSideEndBoundary( + finalHunk.additionStart, + finalHunk.additionCount + ); + const trailingContext = Math.min( + deletionLines.length - finalDeletionEnd, + additionLines.length - finalAdditionEnd + ); pushCollapsedContextLines( resolvedDiff, deletionLines, additionLines, - finalHunk.deletionLineIndex + finalHunk.deletionCount, - finalHunk.additionLineIndex + finalHunk.additionCount, - Math.min( - deletionLines.length - - (finalHunk.deletionLineIndex + finalHunk.deletionCount), - additionLines.length - - (finalHunk.additionLineIndex + finalHunk.additionCount) - ) + finalDeletionEnd, + finalAdditionEnd, + trailingContext ); + cursor.splitLineCount += trailingContext; + cursor.unifiedLineCount += trailingContext; } resolvedDiff.splitLineCount = cursor.splitLineCount; diff --git a/packages/diffs/src/utils/updateDiffHunks.ts b/packages/diffs/src/utils/updateDiffHunks.ts index d001fc130..6d1aeb234 100644 --- a/packages/diffs/src/utils/updateDiffHunks.ts +++ b/packages/diffs/src/utils/updateDiffHunks.ts @@ -7,6 +7,10 @@ import type { Hunk, } from '../types'; import { cleanLastNewline } from './cleanLastNewline'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; import { parseDiffFromFile } from './parseDiffFromFile'; import { hasTrailingContextMismatch } from './virtualDiffLayout'; @@ -188,8 +192,10 @@ export function preserveTrailingEditorBlankLine( return; } - const lastHunkAdditionEnd = - lastHunk.additionLineIndex + lastHunk.additionCount; + const lastHunkAdditionEnd = getHunkSideEndBoundary( + lastHunk.additionStart, + lastHunk.additionCount + ); if (lastHunkAdditionEnd !== extraAdditionLineIndex) { return; } @@ -463,7 +469,8 @@ export function recomputeDiffRenderLineCounts( for (const hunk of diff.hunks) { hunk.collapsedBefore = Math.max( - hunk.additionStart - 1 - lastHunkAdditionEnd, + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) - + lastHunkAdditionEnd, 0 ); hunk.splitLineStart = splitTotal + hunk.collapsedBefore; @@ -473,14 +480,17 @@ export function recomputeDiffRenderLineCounts( splitTotal += hunk.collapsedBefore + hunk.splitLineCount; unifiedTotal += hunk.collapsedBefore + hunk.unifiedLineCount; - lastHunkAdditionEnd = hunk.additionStart + hunk.additionCount - 1; + lastHunkAdditionEnd = getHunkSideEndBoundary( + hunk.additionStart, + hunk.additionCount + ); } if (diff.hunks.length > 0) { const lastHunk = diff.hunks[diff.hunks.length - 1]; const collapsedAfter = Math.max( diff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount), + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount), 0 ); splitTotal += collapsedAfter; diff --git a/packages/diffs/src/utils/virtualDiffLayout.ts b/packages/diffs/src/utils/virtualDiffLayout.ts index 379373c4b..1a61f02a9 100644 --- a/packages/diffs/src/utils/virtualDiffLayout.ts +++ b/packages/diffs/src/utils/virtualDiffLayout.ts @@ -1,10 +1,15 @@ import type { FileDiffMetadata, + Hunk, HunkExpansionRegion, HunkSeparators, VirtualFileMetrics, } from '../types'; import { getDefaultHunkSeparatorHeight } from './computeVirtualFileMetrics'; +import { + getHunkSideEndBoundary, + getHunkSideStartBoundary, +} from './getHunkSideBoundaries'; export interface ExpandedRegionResult { fromStart: number; @@ -33,6 +38,14 @@ export interface GetTrailingExpandedRegionProps extends GetTrailingContextRangeS collapsedContextThreshold: number; } +/** One-based new-file line range covered by a hunk, as `[start, end)`. */ +export function getHunkAdditionLineRange(hunk: Hunk): [number, number] { + return [ + getHunkSideStartBoundary(hunk.additionStart, hunk.additionCount) + 1, + getHunkSideEndBoundary(hunk.additionStart, hunk.additionCount) + 1, + ]; +} + export interface HunkSeparatorLayout { height: number; gapBefore: number; @@ -116,10 +129,10 @@ export function hasTrailingContext(fileDiff: FileDiffMetadata): boolean { const additionRemaining = fileDiff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount); + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount); const deletionRemaining = fileDiff.deletionLines.length - - (lastHunk.deletionLineIndex + lastHunk.deletionCount); + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount); return additionRemaining > 0 || deletionRemaining > 0; } @@ -140,10 +153,10 @@ export function hasTrailingContextMismatch( const additionRemaining = fileDiff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount); + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount); const deletionRemaining = fileDiff.deletionLines.length - - (lastHunk.deletionLineIndex + lastHunk.deletionCount); + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount); if (additionRemaining <= 0 && deletionRemaining <= 0) { return false; @@ -170,10 +183,10 @@ export function getTrailingContextRangeSize({ const additionRemaining = fileDiff.additionLines.length - - (lastHunk.additionLineIndex + lastHunk.additionCount); + getHunkSideEndBoundary(lastHunk.additionStart, lastHunk.additionCount); const deletionRemaining = fileDiff.deletionLines.length - - (lastHunk.deletionLineIndex + lastHunk.deletionCount); + getHunkSideEndBoundary(lastHunk.deletionStart, lastHunk.deletionCount); if (additionRemaining <= 0 && deletionRemaining <= 0) { return 0; @@ -262,7 +275,8 @@ export function isAdditionLineRenderable({ } for (const [hunkIndex, hunk] of fileDiff.hunks.entries()) { - if (lineNumber < hunk.additionStart) { + const [hunkStart, hunkEnd] = getHunkAdditionLineRange(hunk); + if (lineNumber < hunkStart) { // Inside the collapsed gap before this hunk: renderable only within // the expanded slices at the gap's edges. const region = getExpandedRegion({ @@ -272,14 +286,14 @@ export function isAdditionLineRenderable({ hunkIndex, collapsedContextThreshold, }); - const gapStart = hunk.additionStart - region.rangeSize; + const gapStart = hunkStart - region.rangeSize; return ( region.renderAll || lineNumber < gapStart + region.fromStart || - lineNumber >= hunk.additionStart - region.fromEnd + lineNumber >= hunkStart - region.fromEnd ); } - if (lineNumber < hunk.additionStart + hunk.additionCount) { + if (lineNumber < hunkEnd) { return true; } } @@ -295,7 +309,7 @@ export function isAdditionLineRenderable({ return true; } const lastHunk = fileDiff.hunks[fileDiff.hunks.length - 1]; - const trailingStart = lastHunk.additionStart + lastHunk.additionCount; + const [, trailingStart] = getHunkAdditionLineRange(lastHunk); return ( lineNumber < trailingStart + trailingRegion.fromStart || lineNumber >= trailingStart + trailingRegion.rangeSize @@ -330,6 +344,7 @@ export function getNearestRenderableAdditionLine({ const ranges: Array<[start: number, end: number]> = []; let modeledEnd = 1; for (const [hunkIndex, hunk] of fileDiff.hunks.entries()) { + const [hunkStart, hunkEnd] = getHunkAdditionLineRange(hunk); const region = getExpandedRegion({ isPartial: fileDiff.isPartial, rangeSize: hunk.collapsedBefore, @@ -337,19 +352,19 @@ export function getNearestRenderableAdditionLine({ hunkIndex, collapsedContextThreshold, }); - const gapStart = hunk.additionStart - region.rangeSize; + const gapStart = hunkStart - region.rangeSize; if (region.renderAll) { - ranges.push([gapStart, hunk.additionStart]); + ranges.push([gapStart, hunkStart]); } else { if (region.fromStart > 0) { ranges.push([gapStart, gapStart + region.fromStart]); } if (region.fromEnd > 0) { - ranges.push([hunk.additionStart - region.fromEnd, hunk.additionStart]); + ranges.push([hunkStart - region.fromEnd, hunkStart]); } } - ranges.push([hunk.additionStart, hunk.additionStart + hunk.additionCount]); - modeledEnd = hunk.additionStart + hunk.additionCount; + ranges.push([hunkStart, hunkEnd]); + modeledEnd = hunkEnd; } const trailingRegion = getTrailingExpandedRegion({ fileDiff, diff --git a/packages/diffs/test/CodeView.rangeScroll.test.ts b/packages/diffs/test/CodeView.rangeScroll.test.ts index bb7cbe96d..a477fd9a0 100644 --- a/packages/diffs/test/CodeView.rangeScroll.test.ts +++ b/packages/diffs/test/CodeView.rangeScroll.test.ts @@ -44,6 +44,24 @@ function makeInsertedDiffItem(id: string): CodeViewItem { }; } +function makeDeletedDiffItem(id: string): CodeViewItem { + const oldLines = Array.from( + { length: 160 }, + (_, index) => `line ${index + 1}` + ); + const newLines = oldLines.toSpliced(80, 1); + + return { + id, + type: 'diff', + fileDiff: parseDiffFromFile( + { name: 'src/deleted.ts', contents: oldLines.join('\n') }, + { name: 'src/deleted.ts', contents: newLines.join('\n') }, + { context: 0 } + ), + }; +} + function getFileLineTop(lineNumber: number): number { return ( DEFAULT_CODE_VIEW_FILE_METRICS.diffHeaderHeight + @@ -260,4 +278,44 @@ describe('CodeView range scrolling', () => { cleanup(); } }); + + test('places a context-zero deletion between adjacent addition-side lines', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView({ diffStyle: 'split', expandUnchanged: true }); + const root = createRoot({ height: ROOT_HEIGHT, width: ROOT_WIDTH }); + + try { + viewer.setup(root); + await renderItems(viewer, [makeDeletedDiffItem('diff:deleted')]); + + viewer.scrollTo({ + type: 'line', + id: 'diff:deleted', + lineNumber: 80, + side: 'additions', + align: 'center', + behavior: 'instant', + }); + viewer.render(true); + const beforeDeletionScrollTop = root.scrollTop; + + viewer.scrollTo({ + type: 'line', + id: 'diff:deleted', + lineNumber: 81, + side: 'additions', + align: 'center', + behavior: 'instant', + }); + viewer.render(true); + + expect(root.scrollTop - beforeDeletionScrollTop).toBe( + 2 * DEFAULT_CODE_VIEW_FILE_METRICS.lineHeight + ); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); }); diff --git a/packages/diffs/test/DiffHunksRendererRecompute.test.ts b/packages/diffs/test/DiffHunksRendererRecompute.test.ts index 239bfcf8b..5b25d91f1 100644 --- a/packages/diffs/test/DiffHunksRendererRecompute.test.ts +++ b/packages/diffs/test/DiffHunksRendererRecompute.test.ts @@ -68,6 +68,25 @@ function makeTextDocument(lines: string[]): DiffsTextDocument { }; } +function pairingProjection(diff: FileDiffMetadata) { + return diff.hunks.flatMap((hunk) => + hunk.hunkContent.flatMap((content) => { + if (content.type !== 'change') return []; + return Array.from( + { length: Math.max(content.deletions, content.additions) }, + (_, offset) => [ + offset < content.deletions + ? content.deletionLineIndex + offset + : undefined, + offset < content.additions + ? content.additionLineIndex + offset + : undefined, + ] + ); + }) + ); +} + // Builds a renderer with a populated (highlighted) render cache, mirroring the // state the editor operates on mid-session. async function createPrimedRenderer( @@ -229,7 +248,7 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { ); await renderer.asyncRender(diff); renderer.renderDiff(diff); - renderer.beginEditSession(diff); + renderer.beginEditSession(); return { renderer, diff }; } @@ -269,10 +288,69 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { expect(diff.hunks[1].additionLineIndex).toBe(11); }); - // The regression guard for the same-pass contamination trap: a line-count - // pass tokenizes every shifted line as dirty at post-edit indexes while - // diff.additionLines still holds pre-edit content. Region work must wait - // for applyDocumentChange, which scans against the pass snapshot. + test('reports a row-topology change when region bounds stay fixed', async () => { + const renderer = new DiffHunksRenderer({ + theme: 'github-light', + diffStyle: 'split', + }); + const diff = parseDiffFromFile( + { name: 'topology.ts', contents: 'a\nb\nc\nd\n' }, + { name: 'topology.ts', contents: 'a\na\na\nd\n' } + ); + await renderer.asyncRender(diff); + renderer.renderDiff(diff); + renderer.beginEditSession(); + const boundsBefore = { + additionLineIndex: diff.hunks[0].additionLineIndex, + additionCount: diff.hunks[0].additionCount, + deletionLineIndex: diff.hunks[0].deletionLineIndex, + deletionCount: diff.hunks[0].deletionCount, + }; + const splitCountBefore = diff.hunks[0].splitLineCount; + + const regionsChanged = renderer.updateRenderCache( + makeDirtyLines([[2, 'b']]), + 'light' + ); + + expect(regionsChanged).toBe(true); + expect(diff.hunks[0]).toMatchObject(boundsBefore); + expect(diff.hunks[0].splitLineCount).not.toBe(splitCountBefore); + }); + + test('reports a changed split pairing when the row count stays fixed', async () => { + const renderer = new DiffHunksRenderer({ + theme: 'github-light', + diffStyle: 'split', + }); + const diff = parseDiffFromFile( + { name: 'pairing.ts', contents: '\nb\nb\nc\n' }, + { name: 'pairing.ts', contents: '\n!\nb\nc\n' } + ); + await renderer.asyncRender(diff); + renderer.renderDiff(diff); + renderer.beginEditSession(); + const boundsBefore = { + additionLineIndex: diff.hunks[0].additionLineIndex, + additionCount: diff.hunks[0].additionCount, + deletionLineIndex: diff.hunks[0].deletionLineIndex, + deletionCount: diff.hunks[0].deletionCount, + }; + const splitCountBefore = diff.hunks[0].splitLineCount; + + const regionsChanged = renderer.updateRenderCache( + makeDirtyLines([[0, 'b']]), + 'light' + ); + + expect(regionsChanged).toBe(true); + expect(diff.hunks[0]).toMatchObject(boundsBefore); + expect(diff.hunks[0].splitLineCount).toBe(splitCountBefore); + }); + + // A line-count pass tokenizes every shifted line as dirty at post-edit + // indexes while diff.additionLines still holds pre-edit content. Region work + // must wait for applyDocumentChange's authoritative document rebuild. test('an Enter keystroke does not disturb other regions', async () => { const { renderer, diff } = await createSessionRenderer(); const secondRegionBefore = { @@ -309,6 +387,27 @@ 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('')); + const full = parseDiffFromFile( + { name: 'session.ts', contents: SESSION_OLD.join('') }, + { name: 'session.ts', contents: postEditLines.join('') } + ); + expect(pairingProjection(diff)).toEqual(pairingProjection(full)); + }); + + 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(() => + renderer.updateRenderCache( + makeDirtyLines([[0, 'line 1 edited']]), + 'light' + ) + ).not.toThrow(); + + expect(diff.additionLines).toEqual(['line 1 edited\n', '']); + expect(renderer.renderDiff()).toBeDefined(); }); test('a blank line pushed above an edited line keeps the pair aligned', async () => { @@ -381,6 +480,37 @@ describe('DiffHunksRenderer edit-session hunk updates', () => { expect(renderer.renderFullHTML(result)).toContain('change-addition'); }); + test('a same-line undo back to empty reapplies the empty-document shim', async () => { + const { renderer, diff } = await createSessionRenderer(); + renderer.applyDocumentChange(makeTextDocument([''])); + renderer.updateRenderCache(makeDirtyLines([[0, 'hello']]), 'light'); + + const regionsChanged = renderer.updateRenderCache( + makeDirtyLines([[0, '']]), + 'light' + ); + + expect(regionsChanged).toBe(true); + expect(diff.additionLines).toEqual(['']); + const result = renderer.renderDiff(); + expect(result).toBeDefined(); + if (result == null) return; + expect(renderer.renderFullHTML(result)).toContain('change-addition'); + }); + + 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(() => + renderer.updateRenderCache(makeDirtyLines([[0, 'typed']]), 'light') + ).not.toThrow(); + + expect(diff.additionLines).toEqual(['typed\n', '']); + expect(renderer.renderDiff()).toBeDefined(); + }); + test('genuine session end recomputes; a zero-edit session does not', async () => { const { renderer, diff } = await createSessionRenderer(); const untouchedHunks = diff.hunks; diff --git a/packages/diffs/test/diffAcceptRejectHunk.test.ts b/packages/diffs/test/diffAcceptRejectHunk.test.ts index 20692036f..2ead51c3f 100644 --- a/packages/diffs/test/diffAcceptRejectHunk.test.ts +++ b/packages/diffs/test/diffAcceptRejectHunk.test.ts @@ -286,6 +286,50 @@ function assertResolvedHunkMatchesExpected( } describe('diffAcceptRejectHunk', () => { + test('accepts a context-zero deletion without losing trailing context', () => { + const oldLines = Array.from( + { length: 7 }, + (_, index) => `line ${index + 1}\n` + ); + const newLines = oldLines.toSpliced(4, 1); + const diff = parseDiffFromFile( + { name: 'deletion.ts', contents: oldLines.join('') }, + { name: 'deletion.ts', contents: newLines.join('') }, + { context: 0 } + ); + + const result = diffAcceptRejectHunk(diff, 0, 'accept'); + + expect(result.deletionLines).toEqual(newLines); + expect(result.additionLines).toEqual(newLines); + expect(verifyFileDiffHunkValues(result)).toEqual({ + valid: true, + errors: [], + }); + }); + + test('rejects a context-zero deletion without duplicating trailing context', () => { + const oldLines = Array.from( + { length: 7 }, + (_, index) => `line ${index + 1}\n` + ); + const newLines = oldLines.toSpliced(4, 1); + const diff = parseDiffFromFile( + { name: 'deletion.ts', contents: oldLines.join('') }, + { name: 'deletion.ts', contents: newLines.join('') }, + { context: 0 } + ); + + const result = diffAcceptRejectHunk(diff, 0, 'reject'); + + expect(result.deletionLines).toEqual(oldLines); + expect(result.additionLines).toEqual(oldLines); + expect(verifyFileDiffHunkValues(result)).toEqual({ + valid: true, + errors: [], + }); + }); + test('accept keeps later hunk indices accurate after resolving a pure-addition hunk', () => { const diff = createFixture(); const resolvedSnapshot = snapshotHunk(diff, 0); diff --git a/packages/diffs/test/editSessionHunks.test.ts b/packages/diffs/test/editSessionHunks.test.ts index 7e67c0253..1d85572c8 100644 --- a/packages/diffs/test/editSessionHunks.test.ts +++ b/packages/diffs/test/editSessionHunks.test.ts @@ -1,22 +1,22 @@ import { describe, expect, test } from 'bun:test'; +import type { CreatePatchOptionsNonabortable } from 'diff'; import type { FileDiffMetadata, HunkExpansionRegion } from '../src/types'; import { applySessionChangedLines, - applySessionEditWindow, captureExpansionAnchors, - findChangedLineWindow, + findDivergenceCore, finishEditSessionForDiff, normalizeEditorLines, rebuildExpansionFromAnchors, + rebuildSessionHunks, remapExpandedHunksForRegionChange, } from '../src/utils/editSessionHunks'; import { iterateOverDiff } from '../src/utils/iterateOverDiff'; import { parseDiffFromFile } from '../src/utils/parseDiffFromFile'; import { getTrailingContextRangeSize } from '../src/utils/virtualDiffLayout'; +import { verifyFileDiffHunkValues } from './testUtils'; -// 30-line file with two separated changes -> two hunks with a collapsible -// unchanged gap between them and trailing unchanged context after the second. function makeLines( count: number, edits: Record = {} @@ -45,6 +45,10 @@ function hunkBounds(diff: FileDiffMetadata) { })); } +function oldBounds(diff: FileDiffMetadata) { + return diff.hunks.map((hunk) => [hunk.deletionLineIndex, hunk.deletionCount]); +} + function countRenderedRows(diff: FileDiffMetadata): number { let rows = 0; iterateOverDiff({ @@ -58,6 +62,57 @@ function countRenderedRows(diff: FileDiffMetadata): number { return rows; } +function pairingProjection(diff: FileDiffMetadata) { + const rows: Array<{ + deletionIndex: number | undefined; + deletionText: string | undefined; + additionIndex: number | undefined; + additionText: string | undefined; + }> = []; + for (const hunk of diff.hunks) { + for (const content of hunk.hunkContent) { + if (content.type !== 'change') continue; + const rowCount = Math.max(content.deletions, content.additions); + for (let offset = 0; offset < rowCount; offset++) { + const deletionIndex = + offset < content.deletions + ? content.deletionLineIndex + offset + : undefined; + const additionIndex = + offset < content.additions + ? content.additionLineIndex + offset + : undefined; + rows.push({ + deletionIndex, + deletionText: + deletionIndex == null + ? undefined + : diff.deletionLines[deletionIndex], + additionIndex, + additionText: + additionIndex == null + ? undefined + : diff.additionLines[additionIndex], + }); + } + } + } + return rows; +} + +function expectPairingParity( + diff: FileDiffMetadata, + 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 }, + parseDiffOptions + ); + expect(pairingProjection(diff)).toEqual(pairingProjection(expected)); + expect(verifyFileDiffHunkValues(diff)).toEqual({ valid: true, errors: [] }); +} + describe('normalizeEditorLines', () => { test('drops only the phantom trailing empty line', () => { expect(normalizeEditorLines(['a\n', 'b\n', ''])).toEqual(['a\n', 'b\n']); @@ -66,188 +121,168 @@ describe('normalizeEditorLines', () => { }); }); -describe('findChangedLineWindow', () => { - test('locates a replaced line', () => { - expect( - findChangedLineWindow(['a\n', 'b\n', 'c\n'], ['a\n', 'x\n', 'c\n']) - ).toEqual({ start: 1, prevEnd: 2, nextEnd: 2 }); - }); - - test('locates a pure insert', () => { +describe('findDivergenceCore', () => { + test('finds replacement, insertion, and the identical case', () => { expect( - findChangedLineWindow(['a\n', 'c\n'], ['a\n', 'b\n', 'c\n']) - ).toEqual({ start: 1, prevEnd: 1, nextEnd: 2 }); - }); - - test('returns undefined for identical lines', () => { - expect(findChangedLineWindow(['a\n'], ['a\n'])).toBeUndefined(); + 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({ + start: 1, + deletionEnd: 1, + additionEnd: 2, + }); + expect(findDivergenceCore(['a\n'], ['a\n'])).toBeUndefined(); }); }); -describe('applySessionEditWindow', () => { - test('re-diff inside a region keeps boundaries and reports no change', () => { +describe('rebuildSessionHunks', () => { + test('derives downstream coordinates without moving old-side boundaries', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines[2] = 'changed again\n'; + const before = oldBounds(diff); + diff.additionLines.splice(3, 0, 'inserted\n'); - const change = applySessionEditWindow(diff, { - start: 2, - prevEnd: 3, - nextEnd: 3, - }); + const change = rebuildSessionHunks(diff); - expect(change).toBeUndefined(); - expect(hunkBounds(diff)).toEqual(boundsBefore); - expect(diff.editSessionDirty).toBe(true); + expect(change).toBeDefined(); + expect(oldBounds(diff)).toEqual(before); + expect(diff.hunks[0].additionCount).toBe(8); + expect(diff.hunks[1].additionLineIndex).toBe(16); + expect(() => + getTrailingContextRangeSize({ fileDiff: diff, errorPrefix: 'test' }) + ).not.toThrow(); + expectPairingParity(diff); }); - test('a reverted region persists as a context-only hunk', () => { + test('keeps a reverted region as context until session exit', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); + const before = hunkBounds(diff); const rowsBefore = countRenderedRows(diff); diff.additionLines[2] = 'l3\n'; - const change = applySessionEditWindow(diff, { - start: 2, - prevEnd: 3, - nextEnd: 3, - }); + expect(rebuildSessionHunks(diff)).toBeUndefined(); - expect(change).toBeUndefined(); - expect(diff.hunks).toHaveLength(2); - expect(hunkBounds(diff)).toEqual(boundsBefore); + expect(hunkBounds(diff)).toEqual(before); expect(diff.hunks[0].hunkContent).toEqual([ { type: 'context', - lines: boundsBefore[0].additionCount, - additionLineIndex: boundsBefore[0].additionLineIndex, - deletionLineIndex: boundsBefore[0].deletionLineIndex, + lines: before[0].additionCount, + additionLineIndex: before[0].additionLineIndex, + deletionLineIndex: before[0].deletionLineIndex, }, ]); - expect(diff.hunks[0].additionLines).toBe(0); - // The context-only hunk still renders every one of its rows. expect(countRenderedRows(diff)).toBe(rowsBefore); + expectPairingParity(diff); }); - test('an insert inside a region grows it and shifts later regions', () => { + test('synthesizes separate regions for separate canonical blocks in one gap', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines.splice(3, 0, 'inserted\n'); + const before = oldBounds(diff); + diff.additionLines[10] = 'replaced a\n'; + diff.additionLines[13] = 'replaced b\n'; - const change = applySessionEditWindow(diff, { - start: 3, - prevEnd: 3, - nextEnd: 4, - }); + const change = rebuildSessionHunks(diff); - expect(change).toBeUndefined(); - expect(diff.hunks).toHaveLength(2); - expect(diff.hunks[0].additionCount).toBe(boundsBefore[0].additionCount + 1); - expect(diff.hunks[0].deletionCount).toBe(boundsBefore[0].deletionCount); - expect(diff.hunks[1].additionLineIndex).toBe( - boundsBefore[1].additionLineIndex + 1 - ); - expect(diff.hunks[1].deletionLineIndex).toBe( - boundsBefore[1].deletionLineIndex - ); - // Trailing unchanged context must stay symmetric between sides. - expect(() => - getTrailingContextRangeSize({ fileDiff: diff, errorPrefix: 'test' }) - ).not.toThrow(); + expect(change?.regions).toEqual([ + { firstIndex: 0, lastIndex: 0 }, + undefined, + undefined, + { firstIndex: 1, lastIndex: 1 }, + ]); + expect(diff.hunks).toHaveLength(4); + expect(diff.hunks[1].deletionLineIndex).toBe(10); + expect(diff.hunks[2].deletionLineIndex).toBe(13); + expect(oldBounds(diff)[0]).toEqual(before[0]); + expect(oldBounds(diff)[3]).toEqual(before[1]); + expectPairingParity(diff); }); - test('an edit spanning the gap merges both regions and absorbs paired gap lines', () => { + test('merges regions only when a canonical block crosses their gap', () => { const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines[5] = 'edited 6\n'; - diff.additionLines[20] = 'edited 21\n'; - - const change = applySessionEditWindow(diff, { - start: 5, - prevEnd: 21, - nextEnd: 21, - }); + const before = oldBounds(diff); + diff.additionLines.splice(5, 16, 'one bridge\n', 'two bridge\n'); - expect(change).toEqual({ - type: 'merge', - firstIndex: 0, - lastIndex: 1, - previousHunkCount: 2, - }); + const change = rebuildSessionHunks(diff); + + expect(change?.regions).toEqual([{ firstIndex: 0, lastIndex: 1 }]); expect(diff.hunks).toHaveLength(1); - const merged = diff.hunks[0]; - expect(merged.additionLineIndex).toBe(boundsBefore[0].additionLineIndex); - expect(merged.additionCount).toBe( - boundsBefore[1].additionLineIndex + - boundsBefore[1].additionCount - - boundsBefore[0].additionLineIndex - ); - // Gap lines are absorbed in paired correspondence, so the deletion side - // spans the same range. - expect(merged.deletionLineIndex).toBe(boundsBefore[0].deletionLineIndex); - expect(merged.deletionCount).toBe( - boundsBefore[1].deletionLineIndex + - boundsBefore[1].deletionCount - - boundsBefore[0].deletionLineIndex + expect(diff.hunks[0].deletionLineIndex).toBe(before[0][0]); + expect(diff.hunks[0].deletionLineIndex + diff.hunks[0].deletionCount).toBe( + before[1][0] + before[1][1] ); - // The previously collapsed gap now renders as context inside the region. - expect( - merged.hunkContent.some((content) => content.type === 'context') - ).toBe(true); + expectPairingParity(diff); }); - test('a pure insert into a gap synthesizes a region anchored on a context line', () => { - const diff = makeDiff(); - const boundsBefore = hunkBounds(diff); - diff.additionLines.splice(12, 0, 'new a\n', 'new b\n'); + test('keeps full-parse pairing through repeated lines and sequential passes', () => { + const oldContents = [ + '
', + '
', + '', + '', + '', + '', + '', + '', + '
', + '}', + '}', + '}', + '
', + ] + .map((line) => `${line}\n`) + .join(''); + const initialContents = oldContents + .replace('
\n', '
\n') + .replace('