Merge pull request #116 from Zeus-Deus/feature/60-bug-select-all-and-copy-from-the-diff-file-viewer-doesn-t

Emanuele-web04 · web-flow · commit e0624b214137 · 2026-05-14T23:42:49.000+02:00
Fix select-all copy in the diff viewer
diff --git a/apps/web/src/components/DiffPanel.logic.test.ts b/apps/web/src/components/DiffPanel.logic.test.ts
@@ -3,7 +3,7 @@ import { describe, expect, it } from "vitest";
 
 import type { DraftThreadState } from "../composerDraftStore";
 import type { Thread } from "../types";
-import { resolveDiffPanelThread } from "./DiffPanel.logic";
+import { resolveDiffPanelThread, resolveDiffSelectAllArmed } from "./DiffPanel.logic";
 
 const PROJECT_ID = ProjectId.makeUnsafe("project-1");
 const THREAD_ID = ThreadId.makeUnsafe("thread-1");
@@ -103,3 +103,50 @@ describe("resolveDiffPanelThread", () => {
     ).toBeUndefined();
   });
 });
+
+describe("resolveDiffSelectAllArmed", () => {
+  it("arms on Cmd/Ctrl+A inside the diff viewport", () => {
+    expect(
+      resolveDiffSelectAllArmed(false, { key: "a", metaKey: true, ctrlKey: false }, true),
+    ).toBe(true);
+    expect(
+      resolveDiffSelectAllArmed(false, { key: "A", metaKey: false, ctrlKey: true }, true),
+    ).toBe(true);
+  });
+
+  it("does not arm on Cmd/Ctrl+A outside the diff viewport", () => {
+    expect(
+      resolveDiffSelectAllArmed(false, { key: "a", metaKey: true, ctrlKey: false }, false),
+    ).toBe(false);
+    expect(
+      resolveDiffSelectAllArmed(true, { key: "a", metaKey: false, ctrlKey: true }, false),
+    ).toBe(false);
+  });
+
+  it("preserves the armed state through the copy half of the gesture", () => {
+    expect(
+      resolveDiffSelectAllArmed(true, { key: "c", metaKey: true, ctrlKey: false }, false),
+    ).toBe(true);
+    expect(
+      resolveDiffSelectAllArmed(false, { key: "c", metaKey: true, ctrlKey: false }, false),
+    ).toBe(false);
+  });
+
+  it("preserves the armed state through bare modifier keydowns", () => {
+    expect(
+      resolveDiffSelectAllArmed(true, { key: "Meta", metaKey: true, ctrlKey: false }, false),
+    ).toBe(true);
+    expect(
+      resolveDiffSelectAllArmed(true, { key: "Shift", metaKey: false, ctrlKey: false }, false),
+    ).toBe(true);
+  });
+
+  it("disarms on any other key that starts a fresh selection", () => {
+    expect(
+      resolveDiffSelectAllArmed(true, { key: "ArrowDown", metaKey: false, ctrlKey: false }, true),
+    ).toBe(false);
+    expect(
+      resolveDiffSelectAllArmed(true, { key: "x", metaKey: false, ctrlKey: false }, true),
+    ).toBe(false);
+  });
+});
diff --git a/apps/web/src/components/DiffPanel.logic.ts b/apps/web/src/components/DiffPanel.logic.ts
@@ -1,6 +1,6 @@
 // FILE: DiffPanel.logic.ts
 // Purpose: Resolve the thread context the diff panel should use across server-backed and local draft chats.
-// Exports: resolveDiffPanelThread
+// Exports: resolveDiffPanelThread, resolveDiffSelectAllArmed
 // Depends on: ChatView.logic draft-thread normalization.
 
 import { DEFAULT_MODEL_BY_PROVIDER, type ModelSelection, type ThreadId } from "@t3tools/contracts";
@@ -33,3 +33,36 @@ export function resolveDiffPanelThread(input: {
     null,
   );
 }
+
+// Track whether the diff viewport is in a "select all then copy" gesture so the copy
+// handler can substitute the full serialized diff instead of the few mounted rows the
+// virtualizer left in the DOM. Pure so it can be unit tested without a real DOM.
+//
+// The diff surface renders into shadow DOM, so a native Cmd/Ctrl+A actually selects the
+// surrounding light-DOM page and the resulting `copy` event never travels through the
+// viewport element. We instead listen on `document`: the keydown still passes through the
+// viewport (so we can tell the select-all happened there), and this state machine decides
+// whether the very next copy should be hijacked.
+export function resolveDiffSelectAllArmed(
+  previous: boolean,
+  event: Pick<KeyboardEvent, "key" | "metaKey" | "ctrlKey">,
+  isWithinDiffViewport: boolean,
+): boolean {
+  const key = event.key.toLowerCase();
+  const hasShortcutModifier = event.metaKey || event.ctrlKey;
+
+  // Cmd/Ctrl+A arms the gesture, but only when it happens inside the diff viewport.
+  if (hasShortcutModifier && key === "a") {
+    return isWithinDiffViewport;
+  }
+  // Cmd/Ctrl+C is the copy half of the gesture — preserve whatever state we were in.
+  if (hasShortcutModifier && key === "c") {
+    return previous;
+  }
+  // Bare modifier keydowns precede the real shortcut keys; never disarm on them.
+  if (key === "meta" || key === "control" || key === "shift" || key === "alt") {
+    return previous;
+  }
+  // Any other key starts a fresh selection intent, so drop back to native copy behavior.
+  return false;
+}
diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx
@@ -40,6 +40,7 @@ import {
   getRenderablePatch,
   resolveDiffCopyText,
   resolveDiffThemeName,
+  serializeRenderablePatchText,
   summarizePatchStats,
 } from "../lib/diffRendering";
 import { resolveDiffEnvironmentState } from "../lib/threadEnvironment";
@@ -56,7 +57,7 @@ import { getProviderStartOptions, useAppSettings } from "../appSettings";
 import { useComposerDraftStore } from "../composerDraftStore";
 import { formatShortTimestamp } from "../timestampFormat";
 import ChatMarkdown from "./ChatMarkdown";
-import { resolveDiffPanelThread } from "./DiffPanel.logic";
+import { resolveDiffPanelThread, resolveDiffSelectAllArmed } from "./DiffPanel.logic";
 import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./DiffPanelShell";
 import { Button } from "./ui/button";
 import { Menu, MenuPopup, MenuRadioGroup, MenuRadioItem, MenuTrigger } from "./ui/menu";
@@ -195,6 +196,8 @@ export default function DiffPanel({
   const setRepoDiffScope = useRepoDiffScopeStore((store) => store.setScope);
   const [collapsedFiles, setCollapsedFiles] = useState<Set<string>>(() => new Set());
   const patchViewportRef = useRef<HTMLDivElement>(null);
+  // Tracks an in-flight "select all then copy" gesture inside the virtualized diff surface.
+  const diffSelectAllArmedRef = useRef(false);
   const turnStripRef = useRef<HTMLDivElement>(null);
   const previousDiffOpenRef = useRef(false);
   const [canScrollTurnStripLeft, setCanScrollTurnStripLeft] = useState(false);
@@ -403,11 +406,16 @@ export default function DiffPanel({
   const isSidebarMode = mode === "sidebar";
   const { copyToClipboard, isCopied: isSummaryCopied } = useCopyToClipboard();
   const { copyToClipboard: copyDiffToClipboard, isCopied: isDiffCopied } = useCopyToClipboard();
-  const diffCopyText = useMemo(() => resolveDiffCopyText(activeReviewPatch), [activeReviewPatch]);
   const renderablePatch = useMemo(
     () => getRenderablePatch(activeReviewPatch, `diff-panel:${resolvedTheme}`),
     [activeReviewPatch, resolvedTheme],
   );
+  // Serialize the full diff straight from the parsed model so copy paths never depend on
+  // which virtualized rows happen to be mounted in the DOM.
+  const diffCopyText = useMemo(
+    () => serializeRenderablePatchText(renderablePatch) ?? resolveDiffCopyText(activeReviewPatch),
+    [renderablePatch, activeReviewPatch],
+  );
   const renderableFiles = useMemo(() => {
     if (!renderablePatch || renderablePatch.kind !== "files") {
       return [];
@@ -507,16 +515,10 @@ export default function DiffPanel({
         ? "Failed to generate diff summary."
         : null;
   const canShowSummary = Boolean(
-    !diffEnvironmentPending &&
-    activeCwd &&
-    (!hasResolvedRepoPatch || !hasNoRepoChanges),
+    !diffEnvironmentPending && activeCwd && (!hasResolvedRepoPatch || !hasNoRepoChanges),
   );
   const canPrefetchSummary = Boolean(
-    diffOpen &&
-    !diffEnvironmentPending &&
-    activeCwd &&
-    normalizedRepoPatch &&
-    !hasNoRepoChanges,
+    diffOpen && !diffEnvironmentPending && activeCwd && normalizedRepoPatch && !hasNoRepoChanges,
   );
   const canShowTotal = Boolean(!diffEnvironmentPending && activeCwd);
 
@@ -565,6 +567,49 @@ export default function DiffPanel({
     });
   }, []);
 
+  // The diff surface is virtualized and renders into shadow DOM, so a native
+  // "select all + copy" only captures the handful of mounted rows. We watch the
+  // document: a Cmd/Ctrl+A keydown still passes through the viewport element (so we can
+  // tell the gesture started in the diff), and the matching `copy` event — which does
+  // *not* travel through the viewport — is then hijacked to write the fully serialized
+  // diff so every line reaches the clipboard.
+  useEffect(() => {
+    const handleKeyDown = (event: KeyboardEvent) => {
+      const viewport = patchViewportRef.current;
+      const isWithinDiffViewport = viewport ? event.composedPath().includes(viewport) : false;
+      diffSelectAllArmedRef.current = resolveDiffSelectAllArmed(
+        diffSelectAllArmedRef.current,
+        event,
+        isWithinDiffViewport,
+      );
+    };
+    const handlePointerDown = () => {
+      // Any fresh pointer interaction ends the select-all gesture.
+      diffSelectAllArmedRef.current = false;
+    };
+    const handleCopy = (event: ClipboardEvent) => {
+      if (!diffSelectAllArmedRef.current) {
+        return;
+      }
+      // One-shot: the next deliberate select-all must re-arm it.
+      diffSelectAllArmedRef.current = false;
+      if (!diffCopyText || !event.clipboardData) {
+        return;
+      }
+      event.preventDefault();
+      event.clipboardData.setData("text/plain", diffCopyText);
+    };
+
+    document.addEventListener("keydown", handleKeyDown, true);
+    document.addEventListener("pointerdown", handlePointerDown, true);
+    document.addEventListener("copy", handleCopy, true);
+    return () => {
+      document.removeEventListener("keydown", handleKeyDown, true);
+      document.removeEventListener("pointerdown", handlePointerDown, true);
+      document.removeEventListener("copy", handleCopy, true);
+    };
+  }, [diffCopyText]);
+
   const selectTurn = (turnId: TurnId) => {
     if (!activeThread) return;
     if (onUpdatePanelState) {
@@ -943,8 +988,8 @@ export default function DiffPanel({
                 <div className="min-w-0">
                   <p className="text-sm font-medium text-foreground">Repo summary</p>
                   <p className="text-[11px] text-muted-foreground">
-                    Generated from the current{" "}
-                    {REPO_DIFF_SCOPE_LABELS[repoDiffScope].toLowerCase()} diff.
+                    Generated from the current {REPO_DIFF_SCOPE_LABELS[repoDiffScope].toLowerCase()}{" "}
+                    diff.
                   </p>
                 </div>
                 {diffSummaryText ? (
diff --git a/apps/web/src/lib/diffRendering.test.ts b/apps/web/src/lib/diffRendering.test.ts
@@ -4,7 +4,13 @@
 // Depends on: Vitest and diffRendering helpers
 
 import { describe, expect, it } from "vitest";
-import { buildPatchCacheKey, resolveDiffCopyText, summarizePatchStats } from "./diffRendering";
+import {
+  buildPatchCacheKey,
+  getRenderablePatch,
+  resolveDiffCopyText,
+  serializeRenderablePatchText,
+  summarizePatchStats,
+} from "./diffRendering";
 
 describe("buildPatchCacheKey", () => {
   it("returns a stable cache key for identical content", () => {
@@ -48,6 +54,88 @@ describe("resolveDiffCopyText", () => {
   });
 });
 
+describe("serializeRenderablePatchText", () => {
+  it("returns every line for a large diff that would be virtualized in the DOM", () => {
+    const LINE_COUNT = 6000;
+    const bodyLines = Array.from(
+      { length: LINE_COUNT },
+      (_, index) => `+line ${String(index + 1).padStart(4, "0")}`,
+    );
+    const patch = [
+      "diff --git a/big.txt b/big.txt",
+      "new file mode 100644",
+      "index 0000000..1111111",
+      "--- /dev/null",
+      "+++ b/big.txt",
+      `@@ -0,0 +1,${LINE_COUNT} @@`,
+      ...bodyLines,
+      "",
+    ].join("\n");
+
+    const renderable = getRenderablePatch(patch, "diff-panel:test");
+    expect(renderable?.kind).toBe("files");
+
+    const serialized = serializeRenderablePatchText(renderable);
+    expect(serialized).not.toBeNull();
+
+    const serializedAdditions = serialized!.split("\n").filter((line) => line.startsWith("+line "));
+    expect(serializedAdditions).toHaveLength(LINE_COUNT);
+    expect(serializedAdditions[0]).toBe("+line 0001");
+    expect(serializedAdditions[2999]).toBe("+line 3000");
+    expect(serializedAdditions.at(-1)).toBe(`+line ${String(LINE_COUNT).padStart(4, "0")}`);
+    for (const expected of bodyLines) {
+      expect(serialized).toContain(expected);
+    }
+    // The serializer must not inject blank lines between diff rows.
+    expect(serialized).not.toContain("\n\n");
+  });
+
+  it("reconstructs context and change lines in order for a mixed patch", () => {
+    const patch = [
+      "diff --git a/src/example.ts b/src/example.ts",
+      "index 1111111..2222222 100644",
+      "--- a/src/example.ts",
+      "+++ b/src/example.ts",
+      "@@ -1,3 +1,4 @@",
+      " const stable = true;",
+      "-const oldValue = 1;",
+      "+const newValue = 1;",
+      "+const addedValue = 2;",
+      " export { stable };",
+      "",
+    ].join("\n");
+
+    const serialized = serializeRenderablePatchText(getRenderablePatch(patch, "diff-panel:test"));
+
+    expect(serialized).not.toBeNull();
+    const serializedLines = serialized!.split("\n");
+    expect(serializedLines).toContain(" const stable = true;");
+    expect(serializedLines).toContain("-const oldValue = 1;");
+    expect(serializedLines).toContain("+const newValue = 1;");
+    expect(serializedLines).toContain("+const addedValue = 2;");
+    expect(serializedLines).toContain(" export { stable };");
+    // Deletions are emitted before additions within a change block.
+    expect(serializedLines.indexOf("-const oldValue = 1;")).toBeLessThan(
+      serializedLines.indexOf("+const newValue = 1;"),
+    );
+  });
+
+  it("passes raw patches through untouched", () => {
+    const serialized = serializeRenderablePatchText({
+      kind: "raw",
+      text: "not a parseable diff",
+      reason: "Showing raw patch.",
+    });
+
+    expect(serialized).toBe("not a parseable diff");
+  });
+
+  it("returns null when there is nothing to copy", () => {
+    expect(serializeRenderablePatchText(null)).toBeNull();
+    expect(serializeRenderablePatchText({ kind: "files", files: [] })).toBeNull();
+  });
+});
+
 describe("summarizePatchStats", () => {
   it("summarizes additions and deletions from a unified patch", () => {
     const patch = [
diff --git a/apps/web/src/lib/diffRendering.ts b/apps/web/src/lib/diffRendering.ts
