fix: compute first changed diff line from patch hunks
Uses hunk contents to find the first modified line instead of the hunk start Handles added, removed, and binary-only patches more accurately Adds tests for patch parsing edge cases
This commit is contained in:
@@ -0,0 +1,26 @@
|
|||||||
|
import { describe, expect, test } from 'bun:test';
|
||||||
|
|
||||||
|
import { getFirstChangedModifiedLineFromPatch } from './diffPatchUtils';
|
||||||
|
|
||||||
|
describe('getFirstChangedModifiedLineFromPatch', () => {
|
||||||
|
test('returns the first added line instead of the hunk context start', () => {
|
||||||
|
expect(getFirstChangedModifiedLineFromPatch(`diff --git a/src/file.ts b/src/file.ts
|
||||||
|
@@ -56,10 +56,11 @@
|
||||||
|
unchanged 58
|
||||||
|
unchanged 59
|
||||||
|
unchanged 60
|
||||||
|
+changed 61
|
||||||
|
unchanged 62`)).toBe(59);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('returns the following modified line for deletion-only hunks', () => {
|
||||||
|
expect(getFirstChangedModifiedLineFromPatch(`@@ -10,4 +10,3 @@
|
||||||
|
context
|
||||||
|
-removed
|
||||||
|
after`)).toBe(11);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('returns null when the patch has no hunk change lines', () => {
|
||||||
|
expect(getFirstChangedModifiedLineFromPatch('Binary files a/image.png and b/image.png differ')).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -39,6 +39,7 @@ import { fileDiffFromPatch } from '@/lib/diff/patchFileDiff';
|
|||||||
import { isVSCodeRuntime } from '@/lib/desktop';
|
import { isVSCodeRuntime } from '@/lib/desktop';
|
||||||
import { startReviewFlow } from '@/lib/reviewFlow';
|
import { startReviewFlow } from '@/lib/reviewFlow';
|
||||||
import { useSessionUIStore } from '@/sync/session-ui-store';
|
import { useSessionUIStore } from '@/sync/session-ui-store';
|
||||||
|
import { getFirstChangedModifiedLineFromPatch } from './diffPatchUtils';
|
||||||
import type { FileDiffMetadata } from '@pierre/diffs';
|
import type { FileDiffMetadata } from '@pierre/diffs';
|
||||||
|
|
||||||
// Minimum width for side-by-side diff view (px)
|
// Minimum width for side-by-side diff view (px)
|
||||||
@@ -160,24 +161,6 @@ const getFirstChangedModifiedLine = (original: string, modified: string): number
|
|||||||
return 1;
|
return 1;
|
||||||
};
|
};
|
||||||
|
|
||||||
const getFirstVisibleModifiedLineFromPatch = (patch: string): number | null => {
|
|
||||||
if (!patch) {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
const match = patch.match(/@@\s*-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s*@@/m);
|
|
||||||
if (!match) {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
const parsed = Number.parseInt(match[1], 10);
|
|
||||||
if (!Number.isFinite(parsed) || parsed < 1) {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
return parsed;
|
|
||||||
};
|
|
||||||
|
|
||||||
const isBinaryPatch = (patch: string): boolean =>
|
const isBinaryPatch = (patch: string): boolean =>
|
||||||
/^Binary files .+ differ$/m.test(patch) || /^GIT binary patch$/m.test(patch);
|
/^Binary files .+ differ$/m.test(patch) || /^GIT binary patch$/m.test(patch);
|
||||||
|
|
||||||
@@ -1422,7 +1405,9 @@ export const DiffView: React.FC<DiffViewProps> = ({
|
|||||||
try {
|
try {
|
||||||
let targetLine: number | null = null;
|
let targetLine: number | null = null;
|
||||||
|
|
||||||
if (cachedDiffData && !cachedDiffData.isBinary && !isImageFile(filePath)) {
|
if (cachedDiffData?.patch && !cachedDiffData.isBinary && !isImageFile(filePath)) {
|
||||||
|
targetLine = getFirstChangedModifiedLineFromPatch(cachedDiffData.patch);
|
||||||
|
} else if (cachedDiffData && cachedDiffData.contextMode === 'full' && !cachedDiffData.isBinary && !isImageFile(filePath)) {
|
||||||
targetLine = getFirstChangedModifiedLine(cachedDiffData.original, cachedDiffData.modified);
|
targetLine = getFirstChangedModifiedLine(cachedDiffData.original, cachedDiffData.modified);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1433,7 +1418,7 @@ export const DiffView: React.FC<DiffViewProps> = ({
|
|||||||
staged: activeDiffStaged,
|
staged: activeDiffStaged,
|
||||||
contextLines: 3,
|
contextLines: 3,
|
||||||
});
|
});
|
||||||
targetLine = getFirstVisibleModifiedLineFromPatch(patchResponse.diff);
|
targetLine = getFirstChangedModifiedLineFromPatch(patchResponse.diff);
|
||||||
} catch {
|
} catch {
|
||||||
targetLine = null;
|
targetLine = null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,36 @@
|
|||||||
|
export const getFirstChangedModifiedLineFromPatch = (patch: string): number | null => {
|
||||||
|
if (!patch) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
const lines = patch.split('\n');
|
||||||
|
let modifiedLine: number | null = null;
|
||||||
|
|
||||||
|
for (const line of lines) {
|
||||||
|
const hunkMatch = line.match(/^@@\s*-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s*@@/);
|
||||||
|
if (hunkMatch) {
|
||||||
|
const parsed = Number.parseInt(hunkMatch[1] ?? '', 10);
|
||||||
|
modifiedLine = Number.isFinite(parsed) && parsed >= 1 ? parsed : null;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (modifiedLine === null) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (line.startsWith(' ')) {
|
||||||
|
modifiedLine += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (line.startsWith('+')) {
|
||||||
|
return modifiedLine;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (line.startsWith('-')) {
|
||||||
|
return Math.max(1, modifiedLine);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
};
|
||||||
Reference in New Issue
Block a user