authorgravatar for 62337003+Bil0000@users.noreply.github.comBilal Bakr <62337003+Bil0000@users.noreply.github.com> 2026-09-20 14:49:04+03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-20 17:19:04+05:30
log4a560b4e4ebb37efb7f57805ba79e37f5500bdca
treed42cb0bd16b65fa93b43fed38a33e5ea370f4587
parent7445aa733ada33e45289e5aa5055f79142556513
signature Signed by PGP key B5690EEEBB952194

fix(web): honor whitespace settings in pull request diffs (#12438)

Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>

3 files changed, 230 insertions(+), 15 deletions(-)

apps/web/src/components/pullRequest/PullRequestCodeTab.tsx+37-9
...@@ -20,6 +20,7 @@ import {...@@ -20,6 +20,7 @@ import {
20 InfoIcon,20 InfoIcon,
21 MessageSquareIcon,21 MessageSquareIcon,
22 MessageSquareOffIcon,22 MessageSquareOffIcon,
23 PilcrowIcon,
23 Rows3Icon,24 Rows3Icon,
24 TextWrapIcon,25 TextWrapIcon,
25 TriangleAlertIcon,26 TriangleAlertIcon,
...@@ -233,6 +234,7 @@ function PullRequestCodeTab({...@@ -233,6 +234,7 @@ function PullRequestCodeTab({
233 const diffLayout = settings.diffLayout;234 const diffLayout = settings.diffLayout;
234 const updateClientSettings = useUpdateClientSettings();235 const updateClientSettings = useUpdateClientSettings();
235 const [wordWrap, setWordWrap] = useState(settings.wordWrap);236 const [wordWrap, setWordWrap] = useState(settings.wordWrap);
237 const [ignoreWhitespace, setIgnoreWhitespace] = useState(settings.diffIgnoreWhitespace);
236 const [fileTreeOpen, setFileTreeOpen] = useLocalStorage(238 const [fileTreeOpen, setFileTreeOpen] = useLocalStorage(
237 PULL_REQUEST_FILE_TREE_STORAGE_KEY,239 PULL_REQUEST_FILE_TREE_STORAGE_KEY,
238 false,240 false,
...@@ -390,16 +392,17 @@ function PullRequestCodeTab({...@@ -390,16 +392,17 @@ function PullRequestCodeTab({
390 loadedSlices.map((slice) => {392 loadedSlices.map((slice) => {
391 // The patch's own hash is part of the key: a refreshed page reuses its cursor, and a393 // The patch's own hash is part of the key: a refreshed page reuses its cursor, and a
392 // key of position alone would keep handing back the parse of the patch it replaced.394 // key of position alone would keep handing back the parse of the patch it replaced.
393 const cacheKey = `pull-request:${scopeKey}:${resolvedTheme}:${slice.cursor ?? "first"}:${fnv1a32(slice.patch)}`;395 const cacheKey = `pull-request:${scopeKey}:${resolvedTheme}:${ignoreWhitespace}:${slice.cursor ?? "first"}:${fnv1a32(slice.patch)}`;
394 const cached = parseCache.current.get(cacheKey);396 const cached = parseCache.current.get(cacheKey);
395 if (cached) return cached;397 if (cached) return cached;
396 const parsed = getRenderablePatch(slice.patch, cacheKey, {398 const parsed = getRenderablePatch(slice.patch, cacheKey, {
397 compactPartialHunkOffsets: true,399 compactPartialHunkOffsets: true,
400 ignoreWhitespace,
398 });401 });
399 if (parsed) parseCache.current.set(cacheKey, parsed);402 if (parsed) parseCache.current.set(cacheKey, parsed);
400 return parsed;403 return parsed;
401 }),404 }),
402 [loadedSlices, resolvedTheme, scopeKey],405 [loadedSlices, resolvedTheme, scopeKey, ignoreWhitespace],
403 );406 );
404 // Ordered within a slice rather than across them: ordering the accumulated set would let a late407 // Ordered within a slice rather than across them: ordering the accumulated set would let a late
405 // slice push a file the reader is part way through further down the page.408 // slice push a file the reader is part way through further down the page.
...@@ -702,7 +705,13 @@ function PullRequestCodeTab({...@@ -702,7 +705,13 @@ function PullRequestCodeTab({
702 // that silently lost its first line on the other hosts would be worse than one line.705 // that silently lost its first line on the other hosts would be worse than one line.
703 const path = resolveFileDiffPath(file);706 const path = resolveFileDiffPath(file);
704 const previousPath = resolveFileDiffPreviousPath(file);707 const previousPath = resolveFileDiffPreviousPath(file);
705 const position = resolveDiffReviewPosition(file, range.end, range.endSide ?? range.side);708 const sourceFile = parsedSlices
709 .flatMap((slice) => (slice?.kind === "files" ? slice.sourceFiles : []))
710 .find((candidate) => resolveFileDiffPath(candidate) === path);
711 const side = range.endSide ?? range.side;
712 const position =
713 (sourceFile && resolveDiffReviewPosition(sourceFile, range.end, side)) ??
714 resolveDiffReviewPosition(file, range.end, side);
706 if (position === null) return;715 if (position === null) return;
707 setDraft({716 setDraft({
708 fileKey: item.id,717 fileKey: item.id,
...@@ -712,7 +721,7 @@ function PullRequestCodeTab({...@@ -712,7 +721,7 @@ function PullRequestCodeTab({
712 range,721 range,
713 });722 });
714 },723 },
715 [canCommentOnLines, files],724 [canCommentOnLines, files, parsedSlices],
716 );725 );
717726
718 // Built here because the parsed diff only lives here, and built by the same function the727 // Built here because the parsed diff only lives here, and built by the same function the
...@@ -1123,11 +1132,6 @@ function PullRequestCodeTab({...@@ -1123,11 +1132,6 @@ function PullRequestCodeTab({
1123 }1132 }
1124 }, [commit, onSelectedCommitChange, selectedCommit]);1133 }, [commit, onSelectedCommitChange, selectedCommit]);
1125 const scopeLabel = selectedCommit ? selectedCommit.messageHeadline : "All commits";1134 const scopeLabel = selectedCommit ? selectedCommit.messageHeadline : "All commits";
1126 /**
1127 * The same controls the thread diff panel carries, in the same order, minus the
1128 * ignore-whitespace toggle: that is `git diff -w` on the server, and no host's pull request
1129 * diff API offers it.
1130 */
1131 const toolbar = (1135 const toolbar = (
1132 <div className="flex h-10 min-h-10 shrink-0 items-center justify-between gap-2 border-b border-border/60 bg-background px-4 text-xs text-muted-foreground">1136 <div className="flex h-10 min-h-10 shrink-0 items-center justify-between gap-2 border-b border-border/60 bg-background px-4 text-xs text-muted-foreground">
1133 <div className="flex min-w-0 flex-1 items-center gap-3">1137 <div className="flex min-w-0 flex-1 items-center gap-3">
...@@ -1276,6 +1280,30 @@ function PullRequestCodeTab({...@@ -1276,6 +1280,30 @@ function PullRequestCodeTab({
1276 </PullRequestMetaLine>1280 </PullRequestMetaLine>
1277 </div>1281 </div>
1278 <div className="flex shrink-0 items-center gap-1">1282 <div className="flex shrink-0 items-center gap-1">
1283 <Tooltip>
1284 <TooltipTrigger
1285 render={
1286 <Toggle
1287 aria-label={
1288 ignoreWhitespace ? "Show whitespace changes" : "Hide whitespace changes"
1289 }
1290 variant="ghost"
1291 size="sm"
1292 pressed={ignoreWhitespace}
1293 onPressedChange={(pressed) => {
1294 setIgnoreWhitespace(Boolean(pressed));
1295 setDraft(null);
1296 setSelectedLines(null);
1297 }}
1298 />
1299 }
1300 >
1301 <PilcrowIcon className="size-3.5" />
1302 </TooltipTrigger>
1303 <TooltipPopup side="top">
1304 {ignoreWhitespace ? "Show whitespace changes" : "Hide whitespace changes"}
1305 </TooltipPopup>
1306 </Tooltip>
1279 {fileKeys.length > 0 ? (1307 {fileKeys.length > 0 ? (
1280 <Tooltip>1308 <Tooltip>
1281 <TooltipTrigger1309 <TooltipTrigger
apps/web/src/lib/diffRendering.test.ts+131
...@@ -1,4 +1,6 @@...@@ -1,4 +1,6 @@
1import { hydratePartialDiff } from "@pierre/diffs";
1import { describe, expect, it } from "vite-plus/test";2import { describe, expect, it } from "vite-plus/test";
3import { resolveDiffReviewPosition } from "../reviewCommentContext";
2import {4import {
3 buildFileDiffContentVersion,5 buildFileDiffContentVersion,
4 buildFileDiffIdentityKey,6 buildFileDiffIdentityKey,
...@@ -34,6 +36,135 @@ describe("buildPatchCacheKey", () => {...@@ -34,6 +36,135 @@ describe("buildPatchCacheKey", () => {
34});36});
3537
36describe("getRenderablePatch", () => {38describe("getRenderablePatch", () => {
39 it("hides indentation changes around inserted JSX without moving review lines", () => {
40 const patch = [
41 "diff --git a/item.tsx b/item.tsx",
42 "--- a/item.tsx",
43 "+++ b/item.tsx",
44 "@@ -40,5 +40,7 @@",
45 ' <ItemContent className="min-w-0">',
46 "- <ItemTitle>",
47 '- <h4 className="wrap-break-word">{name}</h4>',
48 "- </ItemTitle>",
49 "+ {showName && (",
50 "+ <ItemTitle>",
51 '+ <h4 className="wrap-break-word">{name}</h4>',
52 "+ </ItemTitle>",
53 "+ )}",
54 " </ItemContent>",
55 "@@ -80 +82 @@",
56 "-const value = 1;",
57 "+const value = 2;",
58 ].join("\n");
59 const shown = getRenderablePatch(patch, "pr", { compactPartialHunkOffsets: true });
60 const hidden = getRenderablePatch(patch, "pr", {
61 compactPartialHunkOffsets: true,
62 ignoreWhitespace: true,
63 });
64 expect(shown?.kind).toBe("files");
65 expect(hidden?.kind).toBe("files");
66 if (shown?.kind !== "files" || hidden?.kind !== "files") return;
67 expect(getDiffLineStat(shown.files)).toEqual({ additions: 6, deletions: 4 });
68 expect(getDiffLineStat(hidden.files)).toEqual({ additions: 3, deletions: 1 });
69 const file = hidden.files[0]!;
70 expect(file.additionLines).toEqual(shown.files[0]!.additionLines);
71 expect(file.deletionLines).toEqual(shown.files[0]!.deletionLines);
72 expect(file.cacheKey).not.toBe(shown.files[0]!.cacheKey);
73 expect(resolveDiffReviewPosition(hidden.sourceFiles[0]!, 43, "additions")).toEqual({
74 kind: "added",
75 newLine: 43,
76 });
77 expect(resolveDiffReviewPosition(hidden.sourceFiles[0]!, 42, "deletions")).toEqual({
78 kind: "deleted",
79 oldLine: 42,
80 });
81 expect(file.hunks[0]?.hunkContent).toContainEqual({
82 type: "context",
83 lines: 3,
84 additionLineIndex: 2,
85 deletionLineIndex: 1,
86 });
87 expect(resolveDiffReviewPosition(file, 41, "additions")).toEqual({
88 kind: "added",
89 newLine: 41,
90 });
91 expect(resolveDiffReviewPosition(file, 43, "additions")).toEqual({
92 kind: "context",
93 oldLine: 42,
94 newLine: 43,
95 side: "right",
96 });
97 expect(resolveDiffReviewPosition(file, 42, "deletions")).toEqual({
98 kind: "context",
99 oldLine: 42,
100 newLine: 43,
101 side: "left",
102 });
103 expect(file.hunks[1]).toMatchObject({
104 additionStart: 82,
105 deletionStart: 80,
106 splitLineStart: 7,
107 unifiedLineStart: 7,
108 });
109 const prefix = "unchanged\n".repeat(39);
110 const gap = "unchanged\n".repeat(35);
111 const hydrated = hydratePartialDiff("clone", file, {
112 oldFile: {
113 name: file.name,
114 contents: prefix + file.deletionLines.slice(0, 5).join("") + gap + "const value = 1;\n",
115 },
116 newFile: {
117 name: file.name,
118 contents: prefix + file.additionLines.slice(0, 7).join("") + gap + "const value = 2;\n",
119 },
120 });
121 expect(getDiffLineStat([hydrated])).toEqual({ additions: 3, deletions: 1 });
122 expect(resolveDiffReviewPosition(hydrated, 43, "additions")).toEqual(
123 resolveDiffReviewPosition(file, 43, "additions"),
124 );
125 });
126
127 it.each([
128 [" const x = 1;\t", "\tconst x=1;", 0, 0],
129 ["const x = 1;", "const x = 2;", 1, 1],
130 ["const x = 1;", "const x = 1;\n", 1, 0],
131 ])("filters whitespace in %j to %j", (before, after, additions, deletions) => {
132 const patch = [
133 "diff --git a/example.ts b/example.ts",
134 "--- a/example.ts",
135 "+++ b/example.ts",
136 `@@ -1 +1,${after.split("\n").length} @@`,
137 `-${before}`,
138 ...after.split("\n").map((line) => `+${line}`),
139 "",
140 ].join("\n");
141 const filtered = getRenderablePatch(patch, "pr", { ignoreWhitespace: true });
142 expect(filtered?.kind).toBe("files");
143 if (filtered?.kind !== "files") return;
144 expect(getDiffLineStat(filtered.files)).toEqual({ additions, deletions });
145 });
146
147 it.each(["+", "-"])("keeps %s blank lines without a final newline", (sign) => {
148 const parsed = getRenderablePatch(
149 [
150 "diff --git a/blank.txt b/blank.txt",
151 "--- a/blank.txt",
152 "+++ b/blank.txt",
153 sign === "+" ? "@@ -0,0 +1 @@" : "@@ -1 +0,0 @@",
154 `${sign} `,
155 "\\ No newline at end of file",
156 ].join("\n"),
157 "pr",
158 { ignoreWhitespace: true },
159 );
160 expect(parsed?.kind).toBe("files");
161 if (parsed?.kind !== "files") return;
162 expect(getDiffLineStat(parsed.files)).toEqual({
163 additions: sign === "+" ? 1 : 0,
164 deletions: sign === "-" ? 1 : 0,
165 });
166 });
167
37 it.each([168 it.each([
38 ["a/example.ts", "a/example.ts", "change"],169 ["a/example.ts", "a/example.ts", "change"],
39 ["b/example.ts", "b/example.ts", "change"],170 ["b/example.ts", "b/example.ts", "change"],
apps/web/src/lib/diffRendering.ts+62-6
...@@ -1,4 +1,5 @@...@@ -1,4 +1,5 @@
1import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles";1import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles";
2import { parseDiffFromFile } from "@pierre/diffs";
2import type { FileDiffMetadata } from "@pierre/diffs/types";3import type { FileDiffMetadata } from "@pierre/diffs/types";
3import { unquoteGitPatchPath } from "@t3tools/shared/gitPatchPath";4import { unquoteGitPatchPath } from "@t3tools/shared/gitPatchPath";
45
...@@ -46,6 +47,7 @@ export type RenderablePatch =...@@ -46,6 +47,7 @@ export type RenderablePatch =
46 | {47 | {
47 kind: "files";48 kind: "files";
48 files: FileDiffMetadata[];49 files: FileDiffMetadata[];
50 sourceFiles: FileDiffMetadata[];
49 }51 }
50 | {52 | {
51 kind: "raw";53 kind: "raw";
...@@ -73,6 +75,7 @@ export function getDiffLineStat(files: ReadonlyArray<FileDiffMetadata>): DiffLin...@@ -73,6 +75,7 @@ export function getDiffLineStat(files: ReadonlyArray<FileDiffMetadata>): DiffLin
73}75}
7476
75interface RenderablePatchOptions {77interface RenderablePatchOptions {
78 ignoreWhitespace?: boolean;
76 /**79 /**
77 * Pierre's partial-patch parser keeps hunk render starts in source-file80 * Pierre's partial-patch parser keeps hunk render starts in source-file
78 * coordinates. Its virtualizer iterates partial patches as compact rows, so81 * coordinates. Its virtualizer iterates partial patches as compact rows, so
...@@ -82,6 +85,59 @@ interface RenderablePatchOptions {...@@ -82,6 +85,59 @@ interface RenderablePatchOptions {
82 compactPartialHunkOffsets?: boolean;85 compactPartialHunkOffsets?: boolean;
83}86}
8487
88function hideWhitespaceChanges(file: FileDiffMetadata): FileDiffMetadata {
89 let splitDelta = 0;
90 let unifiedDelta = 0;
91 const hunks = file.hunks.map((hunk) => {
92 const oldContents = file.deletionLines
93 .slice(hunk.deletionLineIndex, hunk.deletionLineIndex + hunk.deletionCount)
94 .map((line) => `${line.replace(/\s/g, "")}\n`)
95 .join("");
96 const newContents = file.additionLines
97 .slice(hunk.additionLineIndex, hunk.additionLineIndex + hunk.additionCount)
98 .map((line) => `${line.replace(/\s/g, "")}\n`)
99 .join("");
100 const filtered = parseDiffFromFile(
101 { name: file.name, contents: oldContents },
102 { name: file.name, contents: newContents },
103 { context: Infinity },
104 ).hunks[0];
105 const next = {
106 ...hunk,
107 additionLines: filtered?.additionLines ?? 0,
108 deletionLines: filtered?.deletionLines ?? 0,
109 hunkContent: filtered
110 ? filtered.hunkContent.map((content) => ({
111 ...content,
112 additionLineIndex: content.additionLineIndex + hunk.additionLineIndex,
113 deletionLineIndex: content.deletionLineIndex + hunk.deletionLineIndex,
114 }))
115 : [
116 {
117 type: "context" as const,
118 lines: hunk.additionCount,
119 additionLineIndex: hunk.additionLineIndex,
120 deletionLineIndex: hunk.deletionLineIndex,
121 },
122 ],
123 splitLineStart: hunk.splitLineStart + splitDelta,
124 unifiedLineStart: hunk.unifiedLineStart + unifiedDelta,
125 splitLineCount: filtered?.splitLineCount ?? hunk.additionCount,
126 unifiedLineCount: filtered?.unifiedLineCount ?? hunk.additionCount,
127 };
128 splitDelta += next.splitLineCount - hunk.splitLineCount;
129 unifiedDelta += next.unifiedLineCount - hunk.unifiedLineCount;
130 return next;
131 });
132 return {
133 ...file,
134 hunks,
135 splitLineCount: file.splitLineCount + splitDelta,
136 unifiedLineCount: file.unifiedLineCount + unifiedDelta,
137 ...(file.cacheKey ? { cacheKey: `${file.cacheKey}:ignore-whitespace` } : {}),
138 };
139}
140
85function compactPartialHunkOffsets(file: FileDiffMetadata): FileDiffMetadata {141function compactPartialHunkOffsets(file: FileDiffMetadata): FileDiffMetadata {
86 if (!file.isPartial) return file;142 if (!file.isPartial) return file;
87143
...@@ -121,13 +177,13 @@ export function getRenderablePatch(...@@ -121,13 +177,13 @@ export function getRenderablePatch(
121 normalizedPatch,177 normalizedPatch,
122 buildPatchCacheKey(normalizedPatch, cacheScope),178 buildPatchCacheKey(normalizedPatch, cacheScope),
123 );179 );
124 const files = parsedPatches.flatMap((parsedPatch) =>180 const sourceFiles = parsedPatches.flatMap((parsedPatch) => parsedPatch.files);
125 options.compactPartialHunkOffsets181 const files = sourceFiles.map((file) => {
126 ? parsedPatch.files.map(compactPartialHunkOffsets)182 const filtered = options.ignoreWhitespace ? hideWhitespaceChanges(file) : file;
127 : parsedPatch.files,183 return options.compactPartialHunkOffsets ? compactPartialHunkOffsets(filtered) : filtered;
128 );184 });
129 if (files.length > 0) {185 if (files.length > 0) {
130 return { kind: "files", files };186 return { kind: "files", files, sourceFiles };
131 }187 }
132188
133 return {189 return {