From d03e88e50b95eeb8b486cfbfcef352ec319e082c Mon Sep 17 00:00:00 2001 From: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:06:44 -0700 Subject: [PATCH] Render desktop diff view hunks with the shared @pierre/diffs renderer (#13201) * Render desktop diff view hunks with shared @pierre/diffs renderer Replace DiffView's hand-rolled DiffHunk +/- line rows with ToolFileDiff from @cline/ui (backed by @pierre/diffs), matching the chat tool rows. Hunks carrying complete new contents (created files) render with real line numbers; fragment hunks hide them, mirroring ToolCallRow. All of DiffView's chrome (collapse, copy, open-in-editor, counts) is unchanged. Co-authored-by: Saoud Rizwan * Make ToolFileDiff syntax palette follow the app theme, not browser preference @pierre/diffs declares 'color-scheme: light dark' on its shadow :host, so its light-dark() token colors resolve from the browser's preferred scheme. Apps themed by the .dark class (desktop app) got the light palette's near-black text on dark surfaces. Inline colorScheme: inherit on the host wins over the :host rule and follows the app's color-scheme, which the @cline/ui theme already flips with .dark. Skipped when a caller pins an explicit themeType. Also key diff-view hunks by index so repeated same-shaped hunks (a file created twice with identical contents) don't collide. Co-authored-by: Saoud Rizwan --------- Co-authored-by: Saoud Rizwan --- .../components/views/chat/diff-view.test.tsx | 92 +++++++++++++++++++ .../components/views/chat/diff-view.tsx | 86 +++++------------ .../ui/components/agent-chat/tool-diff.tsx | 13 +++ sdk/packages/ui/tests/tool-diff.test.tsx | 77 ++++++++++++++++ 4 files changed, 207 insertions(+), 61 deletions(-) create mode 100644 sdk/packages/ui/tests/tool-diff.test.tsx diff --git a/apps/examples/desktop-app/webview/components/views/chat/diff-view.test.tsx b/apps/examples/desktop-app/webview/components/views/chat/diff-view.test.tsx index 0dcb782f52..77f6a1aff8 100644 --- a/apps/examples/desktop-app/webview/components/views/chat/diff-view.test.tsx +++ b/apps/examples/desktop-app/webview/components/views/chat/diff-view.test.tsx @@ -18,6 +18,11 @@ vi.mock("@/lib/desktop-client", () => ({ desktopClient: { invoke: invokeMock }, })); +// @pierre/diffs' custom element adopts constructable stylesheets, which jsdom +// does not implement; without this the suite exits nonzero on an unhandled +// error even with every test passing. +CSSStyleSheet.prototype.replaceSync ??= function replaceSync() {} as never; + let container: HTMLDivElement; let root: Root; let writeText: ReturnType; @@ -100,6 +105,38 @@ const FILE_DIFF: SessionFileDiff = { hunks: [], }; +const MODIFIED_FILE_DIFF: SessionFileDiff = { + path: "src/app.ts", + additions: 1, + deletions: 1, + hunks: [ + { + oldStart: 4, + newStart: 4, + old: "const total = 1;", + new: "const total = 2;", + }, + ], +}; + +const CREATED_FILE_DIFF: SessionFileDiff = { + path: "src/created.ts", + additions: 2, + deletions: 0, + hunks: [ + { + oldStart: 1, + newStart: 1, + old: "", + new: "export const a = 1;\nexport const b = 2;", + }, + ], +}; + +function diffContainers(): HTMLElement[] { + return Array.from(container.querySelectorAll("diffs-container")); +} + describe("DiffView file actions", () => { it("copies the cwd-resolved absolute file path", async () => { await act(async () => { @@ -180,3 +217,58 @@ describe("DiffView file actions", () => { expect(writeText).toHaveBeenCalledWith("docs/a.mdx"); }); }); + +describe("DiffView hunk rendering", () => { + it("renders each hunk through the shared @pierre/diffs renderer", async () => { + await act(async () => { + root.render( + , + ); + }); + + expect(diffContainers()).toHaveLength(2); + }); + + it("shows the empty-hunks placeholder instead of a diff renderer", async () => { + await act(async () => { + root.render(); + }); + + expect(diffContainers()).toHaveLength(0); + expect(container.textContent).toContain("No hunk details available."); + }); + + it("removes the diff body when a file is collapsed and restores it on expand", async () => { + await act(async () => { + root.render( + , + ); + }); + + expect(diffContainers()).toHaveLength(1); + + const toggle = container.querySelector( + "button:not([aria-label])", + ); + expect(toggle?.textContent).toContain("src/app.ts"); + await click(toggle as Element); + expect(diffContainers()).toHaveLength(0); + + await click(toggle as Element); + expect(diffContainers()).toHaveLength(1); + }); + + it("keeps the per-file add/del counts in the header", async () => { + await act(async () => { + root.render( + , + ); + }); + + expect(container.textContent).toContain("+1"); + expect(container.textContent).toContain("-1"); + }); +}); diff --git a/apps/examples/desktop-app/webview/components/views/chat/diff-view.tsx b/apps/examples/desktop-app/webview/components/views/chat/diff-view.tsx index 55753324aa..7652d7529b 100644 --- a/apps/examples/desktop-app/webview/components/views/chat/diff-view.tsx +++ b/apps/examples/desktop-app/webview/components/views/chat/diff-view.tsx @@ -1,5 +1,6 @@ "use client"; +import { ToolFileDiff } from "@cline/ui/components/agent-chat/tool-diff"; import { AppWindow, Check, @@ -7,8 +8,6 @@ import { ChevronRight, Copy, ExternalLink, - Minus, - Plus, X, } from "lucide-react"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; @@ -23,7 +22,7 @@ import { import { ScrollArea } from "@/components/ui/scroll-area"; import { toast } from "@/hooks/use-toast"; import { desktopClient } from "@/lib/desktop-client"; -import type { SessionFileDiff } from "@/lib/session-diff"; +import type { SessionDiffHunk, SessionFileDiff } from "@/lib/session-diff"; import { cn } from "@/lib/utils"; import { resolveWorkspaceFilePath } from "@/lib/workspace-paths"; import { EditorIcon } from "./editor-icons"; @@ -285,10 +284,14 @@ function DiffFileSection({ No hunk details available.

) : ( - file.hunks.map((hunk) => ( + // The index disambiguates repeated same-shaped hunks (e.g. + // a file created twice with identical contents); hunks + // never reorder within a file, so it is a stable key. + file.hunks.map((hunk, index) => ( )) )} @@ -298,63 +301,24 @@ function DiffFileSection({ ); } -function DiffHunk({ hunk }: { hunk: SessionFileDiff["hunks"][number] }) { - const oldLines = hunk.old.length > 0 ? hunk.old.split("\n") : []; - const newLines = hunk.new.length > 0 ? hunk.new.split("\n") : []; - const oldOccurrences = new Map(); - const oldLineEntries = oldLines.map((line, offset) => { - const occurrence = (oldOccurrences.get(line) ?? 0) + 1; - oldOccurrences.set(line, occurrence); - return { - key: `old-${hunk.oldStart + offset}-${occurrence}-${line}`, - line, - lineNumber: hunk.oldStart + offset, - }; - }); - const newOccurrences = new Map(); - const newLineEntries = newLines.map((line, offset) => { - const occurrence = (newOccurrences.get(line) ?? 0) + 1; - newOccurrences.set(line, occurrence); - return { - key: `new-${hunk.newStart + offset}-${occurrence}-${line}`, - line, - lineNumber: hunk.newStart + offset, - }; - }); +function DiffHunk({ hunk, path }: { hunk: SessionDiffHunk; path: string }) { + // A hunk with no old side that starts at line 1 on both sides carries the + // complete new contents (editor `create`, apply_patch Add File). Chat tool + // rows render those with complete-file semantics (real line numbers); + // everything else is a file fragment, which hides line numbers — see + // ToolCallRow in chat-messages.tsx. Matching that keeps both surfaces + // visually in agreement. + const isCompleteNewContents = + hunk.old.length === 0 && hunk.oldStart === 1 && hunk.newStart === 1; return ( -
- {oldLineEntries.map((entry) => ( -
- - {entry.lineNumber} - - - - - - {entry.line || " "} - -
- ))} - {newLineEntries.map((entry) => ( -
- - {entry.lineNumber} - - - - - - {entry.line || " "} - -
- ))} - {oldLines.length === 0 && newLines.length === 0 && ( -
- No line diff content. -
- )} -
+ ); } diff --git a/sdk/packages/ui/components/agent-chat/tool-diff.tsx b/sdk/packages/ui/components/agent-chat/tool-diff.tsx index 8a22e88093..d1b0e2293f 100644 --- a/sdk/packages/ui/components/agent-chat/tool-diff.tsx +++ b/sdk/packages/ui/components/agent-chat/tool-diff.tsx @@ -100,6 +100,19 @@ export function ToolFileDiff({ { "--diffs-light-bg": background, "--diffs-dark-bg": background, + // @pierre/diffs declares `color-scheme: light dark` on its + // shadow :host, so its light-dark() token colors follow the + // browser's preferred scheme — not the host app's + // class-based theme — leaving e.g. near-black light-palette + // text on a dark app surface. Inheriting the app's + // color-scheme (flipped by `.dark` in the @cline/ui theme) + // keeps the syntax palette in lockstep with the app theme. + // Skipped when a caller pins an explicit themeType, which + // pierre implements as its own :host color-scheme rule. + ...(resolvedOptions.themeType === "system" || + resolvedOptions.themeType === undefined + ? { colorScheme: "inherit" } + : {}), } as CSSProperties } /> diff --git a/sdk/packages/ui/tests/tool-diff.test.tsx b/sdk/packages/ui/tests/tool-diff.test.tsx new file mode 100644 index 0000000000..ff5bbbc618 --- /dev/null +++ b/sdk/packages/ui/tests/tool-diff.test.tsx @@ -0,0 +1,77 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ToolFileDiff } from "../components/agent-chat/tool-diff"; + +// @pierre/diffs' custom element adopts constructable stylesheets, which jsdom +// does not implement; without this the suite exits nonzero on an unhandled +// error even with every test passing. +CSSStyleSheet.prototype.replaceSync ??= function replaceSync() {} as never; + +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + vi.restoreAllMocks(); +}); + +async function render(element: React.ReactNode) { + await act(async () => root.render(element)); +} + +function host(): HTMLElement { + const element = container.querySelector("diffs-container"); + expect(element).not.toBeNull(); + return element as HTMLElement; +} + +describe("ToolFileDiff", () => { + it("renders the @pierre/diffs host with the background derived from the app token", async () => { + await render( + , + ); + + const element = host(); + expect(element.style.getPropertyValue("--diffs-light-bg")).toBe( + "var(--background, light-dark(#fff, #000))", + ); + expect(element.style.getPropertyValue("--diffs-dark-bg")).toBe( + "var(--background, light-dark(#fff, #000))", + ); + }); + + it("inherits the app color-scheme so light-dark() palettes follow the app theme", async () => { + // Regression: pierre's :host declares `color-scheme: light dark`, + // which resolves token colors from the browser preference — light + // syntax palette (near-black text) on a dark app surface. + await render( + , + ); + + expect(host().style.colorScheme).toBe("inherit"); + }); + + it("defers to an explicitly pinned themeType instead of inheriting", async () => { + await render( + , + ); + + expect(host().style.colorScheme).toBe(""); + }); +});