Explorar el Código

fix(tui): preserve diff hunk boundaries (#39693)

Co-authored-by: James Long <17031+jlongster@users.noreply.github.com>
opencode-agent[bot] hace 2 semanas
padre
commit
f92e490bb6

+ 83 - 0
packages/tui/src/component/patch-diff.tsx

@@ -0,0 +1,83 @@
+/** @jsxImportSource @opentui/solid */
+import { DiffRenderable, LineNumberRenderable, type ColorInput } from "@opentui/core"
+import type { JSX } from "@opentui/solid"
+import { createMemo, For, Show, splitProps } from "solid-js"
+import { splitPatchHunks } from "../util/diff"
+import { stringWidth } from "../util/string-width"
+
+export interface PatchDiffRef {
+  readonly hunks: () => readonly DiffRenderable[]
+}
+
+type Props = Omit<JSX.IntrinsicElements["diff"], "diff" | "lineNumberBg" | "ref"> & {
+  diff: string
+  hunkFg: ColorInput
+  lineNumberBg: ColorInput
+  ref?: (value: PatchDiffRef) => void
+}
+
+export function PatchDiff(props: Props) {
+  const [local, diffProps] = splitProps(props, ["diff", "hunkFg", "lineNumberBg", "ref"])
+  const hunks = createMemo(() => splitPatchHunks(local.diff))
+  const nodes = new Map<number, DiffRenderable>()
+  local.ref?.({
+    hunks: () =>
+      [...nodes.entries()]
+        .sort(([left], [right]) => left - right)
+        .map(([, node]) => node)
+        .filter((node) => !node.isDestroyed),
+  })
+  const syncGutters = (attempt = 0) => {
+    requestAnimationFrame(() => {
+      const sides = [...nodes.values()]
+        .filter((item) => !item.isDestroyed)
+        .flatMap((item) => item.getChildren().filter((side) => side instanceof LineNumberRenderable))
+      const lineNumbers = sides.map((side) => new Map([...side.getLineNumbers()].filter(([line]) => line >= 0)))
+      const digits = lineNumbers.map((numbers) => Math.max(0, ...numbers.values()).toString().length)
+      const after = sides.map((side) =>
+        Math.max(
+          0,
+          ...[...side.getLineSigns()].filter(([line]) => line >= 0).map(([, sign]) => stringWidth(sign.after ?? "")),
+        ),
+      )
+      const maxDigits = Math.max(...digits)
+      const maxAfter = Math.max(...after)
+      if (!maxDigits && attempt < 2) return syncGutters(attempt + 1)
+      if (!maxDigits) return
+      sides.forEach((side) => {
+        const index = sides.indexOf(side)
+        const signs = new Map([...side.getLineSigns()].filter(([line]) => line >= 0))
+        signs.set(-1, { after: " ".repeat(maxAfter + maxDigits - digits[index]) })
+        side.setLineNumbers(lineNumbers[index])
+        side.setLineSigns(signs)
+      })
+    })
+  }
+  const register = (index: number, node: DiffRenderable) => {
+    nodes.set(index, node)
+    syncGutters()
+  }
+
+  return (
+    <For each={hunks()}>
+      {(hunk, index) => (
+        <>
+          <Show when={index() > 0}>
+            <box width="100%" height={1} backgroundColor={local.lineNumberBg}>
+              <text fg={local.hunkFg} bg={local.lineNumberBg}>
+                {` ${hunk.header ?? ""}`}
+              </text>
+            </box>
+          </Show>
+          <diff
+            {...diffProps}
+            ref={(node: DiffRenderable) => register(index(), node)}
+            diff={hunk.patch}
+            minHeight={hunk.rows}
+            lineNumberBg={local.lineNumberBg}
+          />
+        </>
+      )}
+    </For>
+  )
+}

+ 22 - 21
packages/tui/src/feature-plugins/system/diff-viewer.tsx

@@ -2,13 +2,7 @@
 import type { FileDiffInfo } from "@opencode-ai/client"
 import { Plugin } from "@opencode-ai/plugin/tui"
 import type { KeymapCommand, Route } from "@opencode-ai/plugin/tui/context"
-import {
-  TextAttributes,
-  type BorderSides,
-  type BoxRenderable,
-  type DiffRenderable,
-  type ScrollBoxRenderable,
-} from "@opentui/core"
+import { TextAttributes, type BorderSides, type BoxRenderable, type ScrollBoxRenderable } from "@opentui/core"
 import { LANGUAGE_EXTENSIONS } from "../../util/filetype"
 import { useTerminalDimensions } from "@opentui/solid"
 import path from "path"
@@ -19,6 +13,7 @@ import { DialogSelect } from "../../ui/dialog-select"
 import { getScrollAcceleration } from "../../util/scroll"
 import { useConfig } from "../../config"
 import { useThemes } from "../../context/theme"
+import { PatchDiff, type PatchDiffRef } from "../../component/patch-diff"
 import {
   allExpandedFileTreeDirectories,
   buildFileTree,
@@ -154,7 +149,7 @@ function DiffViewer(props: { context: Plugin.Context }) {
   const helpShortcut = shortcut("diff.help")
   let scroll: ScrollBoxRenderable | undefined
   const patchNodeByFileIndex = new Map<number, BoxRenderable>()
-  const diffNodeByFileIndex = new Map<number, DiffRenderable>()
+  const patchDiffByFileIndex = new Map<number, PatchDiffRef>()
   const [selectedHunk, setSelectedHunk] = createSignal<SelectedHunk | undefined>()
   const [pendingPatchScrollFileIndex, setPendingPatchScrollFileIndex] = createSignal<number | undefined>()
   const [patchFillerHeight, setPatchFillerHeight] = createSignal(0)
@@ -270,17 +265,16 @@ function DiffViewer(props: { context: Plugin.Context }) {
     if (!patchScroll) return
     const hunks = visiblePatchFiles()
       .flatMap((entry) => {
-        const node = diffNodeByFileIndex.get(entry.fileIndex)
-        if (!node || node.isDestroyed) return []
-        const contentY = patchScroll.scrollTop + node.y - patchScroll.viewport.y
-        return node.diff
-          .split("\n")
-          .flatMap((line, row) => (line.startsWith("@@") ? [row] : []))
-          .map((row, hunkIndex) => ({
-            fileIndex: entry.fileIndex,
-            hunkIndex,
-            contentY: contentY + row,
-          }))
+        return (
+          patchDiffByFileIndex
+            .get(entry.fileIndex)
+            ?.hunks()
+            .map((node, hunkIndex) => ({
+              fileIndex: entry.fileIndex,
+              hunkIndex,
+              contentY: patchScroll.scrollTop + node.y - patchScroll.viewport.y - (hunkIndex > 0 ? 1 : 0),
+            })) ?? []
+        )
       })
       .sort((left, right) => left.contentY - right.contentY)
     const selected = selectedHunk()
@@ -831,9 +825,10 @@ function DiffViewer(props: { context: Plugin.Context }) {
                             >
                               {(patch) => (
                                 <box border={patchLeftBorder()} borderColor={theme.border.default}>
-                                  <diff
-                                    ref={(element: DiffRenderable) => diffNodeByFileIndex.set(entry.fileIndex, element)}
+                                  <PatchDiff
+                                    ref={(component) => patchDiffByFileIndex.set(entry.fileIndex, component)}
                                     diff={patch()}
+                                    hunkFg={reviewed() ? theme.text.subdued : theme.diff.text.hunkHeader}
                                     view={view()}
                                     filetype={reviewed() ? PLAIN_TEXT_FILETYPE : filetype(entry.file.file)}
                                     syntaxStyle={currentSyntax()}
@@ -847,9 +842,15 @@ function DiffViewer(props: { context: Plugin.Context }) {
                                     removedBg={
                                       reviewed() ? theme.background.surface.overlay : theme.diff.background.removed
                                     }
+                                    contextBg={
+                                      reviewed() ? theme.background.surface.overlay : theme.diff.background.context
+                                    }
                                     addedSignColor={reviewed() ? theme.text.subdued : theme.diff.highlight.added}
                                     removedSignColor={reviewed() ? theme.text.subdued : theme.diff.highlight.removed}
                                     lineNumberFg={theme.diff.lineNumber.text}
+                                    lineNumberBg={
+                                      reviewed() ? theme.background.surface.overlay : theme.diff.background.context
+                                    }
                                     addedLineNumberBg={
                                       reviewed()
                                         ? theme.background.surface.overlay

+ 3 - 1
packages/tui/src/mini/footer.permission.tsx

@@ -32,6 +32,7 @@ import { footerWidthPolicy } from "./footer.width"
 import { toolFiletype } from "./tool"
 import { transparent, type RunBlockTheme, type RunFooterTheme } from "./theme"
 import type { MiniPermissionRequest, PermissionReply } from "./types"
+import { PatchDiff } from "../component/patch-diff"
 
 function buttons(
   list: PermissionOption[],
@@ -405,8 +406,9 @@ export function RunPermissionBody(props: {
                       </Show>
                     }
                   >
-                    <diff
+                    <PatchDiff
                       diff={info().diff!}
+                      hunkFg={props.block.diffLineNumber}
                       view="unified"
                       filetype={ft()}
                       syntaxStyle={props.block.syntax}

+ 3 - 1
packages/tui/src/mini/scrollback.writer.tsx

@@ -13,6 +13,7 @@ import { entryColor, entryLook, entrySyntax } from "./scrollback.shared"
 import { toolFiletype, toolStructuredFinal } from "./tool"
 import { RUN_THEME_FALLBACK, transparent, type RunTheme } from "./theme"
 import type { EntryLayout, RunEntryBody, ScrollbackOptions, StreamCommit, TurnSummary } from "./types"
+import { PatchDiff } from "../component/patch-diff"
 
 export function entryGroupKey(commit: StreamCommit): string | undefined {
   if (!commit.partID) {
@@ -178,8 +179,9 @@ export function RunEntryContent(props: {
               </text>
               {item.diff.trim() ? (
                 <box width="100%" paddingLeft={1}>
-                  <diff
+                  <PatchDiff
                     diff={item.diff}
+                    hunkFg={theme().block.diffLineNumber}
                     view="unified"
                     filetype={toolFiletype(item.file)}
                     syntaxStyle={syntax()}

+ 5 - 2
packages/tui/src/routes/session/index.tsx

@@ -22,6 +22,7 @@ import { useData } from "../../context/data"
 import { SplitBorder } from "../../ui/border"
 import { useTuiPaths, useTuiTerminalEnvironment } from "../../context/runtime"
 import { Spinner, SPINNER_FRAMES } from "../../component/spinner"
+import { PatchDiff } from "../../component/patch-diff"
 import { ThemeContextProvider, useTheme, useThemes } from "../../context/theme"
 import { BoxRenderable, ScrollBoxRenderable, addDefaultParsers, TextAttributes, RGBA } from "@opentui/core"
 import { Prompt, type PromptRef } from "../../component/prompt"
@@ -3038,8 +3039,9 @@ function Edit(props: ToolProps) {
         {(item) => (
           <BlockTool path={{ label: "← Edit", value: pathFormatter.format(path()) }} part={props.part}>
             <box paddingLeft={1}>
-              <diff
+              <PatchDiff
                 diff={item().patch}
+                hunkFg={theme.diff.text.hunkHeader}
                 view={view()}
                 filetype={filetype(path())}
                 syntaxStyle={syntax()}
@@ -3127,8 +3129,9 @@ function ApplyPatch(props: ToolProps) {
                   }
                 >
                   <box paddingLeft={1}>
-                    <diff
+                    <PatchDiff
                       diff={file.patch}
+                      hunkFg={theme.diff.text.hunkHeader}
                       view={view()}
                       filetype={filetype(file.relativePath)}
                       syntaxStyle={syntax()}

+ 3 - 1
packages/tui/src/routes/session/permission.tsx

@@ -14,6 +14,7 @@ import { useConfig } from "../../config"
 import { Keymap } from "../../context/keymap"
 import { usePathFormatter } from "../../context/path-format"
 import { SimulationSemantics } from "../../simulation/semantics"
+import { PatchDiff } from "../../component/patch-diff"
 
 type PermissionStage = "permission" | "always" | "reject"
 
@@ -50,8 +51,9 @@ function EditBody(props: { file?: string; diff?: string; patch?: string }) {
             },
           }}
         >
-          <diff
+          <PatchDiff
             diff={diff()}
+            hunkFg={theme.diff.text.hunkHeader}
             view={view()}
             filetype={ft()}
             syntaxStyle={syntax()}

+ 56 - 0
packages/tui/src/util/diff.ts

@@ -0,0 +1,56 @@
+export interface PatchHunk {
+  readonly patch: string
+  readonly header?: string
+  readonly rows?: number
+}
+
+export function splitPatchHunks(patch: string): PatchHunk[] {
+  const starts = [
+    ...patch.matchAll(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@.*$/gm),
+  ].map((match) => match.index)
+  if (starts.length <= 1) return [{ patch }]
+
+  const prefix = patch.slice(0, starts[0])
+  return starts.map((start, index) => {
+    const end = starts[index + 1] ?? patch.length
+    const lineEnd = patch.indexOf("\n", start)
+    return {
+      header: patch.slice(start, lineEnd === -1 ? end : lineEnd),
+      patch: prefix + patch.slice(start, end),
+      rows: splitRows(patch.slice(start, end)),
+    }
+  })
+}
+
+function splitRows(hunk: string) {
+  const lines = hunk.replace(/\n$/, "").split("\n").slice(1)
+  let rows = 0
+  let index = 0
+
+  while (index < lines.length) {
+    const prefix = lines[index][0]
+    if (prefix === " " || !prefix) {
+      rows++
+      index++
+      continue
+    }
+    if (prefix === "\\") {
+      index++
+      continue
+    }
+
+    let additions = 0
+    let deletions = 0
+    while (
+      index < lines.length &&
+      (lines[index][0] === "+" || lines[index][0] === "-")
+    ) {
+      if (lines[index][0] === "+") additions++
+      if (lines[index][0] === "-") deletions++
+      index++
+    }
+    rows += Math.max(additions, deletions)
+  }
+
+  return rows
+}

+ 8 - 0
packages/tui/test/cli/tui/diff-viewer.test.tsx

@@ -103,6 +103,8 @@ test("brackets navigate diff hunks", async () => {
     await viewer.app.waitForFrame((frame) => frame.includes("const first"))
     await viewer.app.waitFor(() => Boolean(findScrollBox(viewer.app.renderer.root)))
     await viewer.app.flush()
+    expect(viewer.app.captureCharFrame()).toContain("@@ -20,3 +20,3 @@")
+    expect(countDiffs(viewer.app.renderer.root)).toBe(3)
     const scroll = findScrollBox(viewer.app.renderer.root)!
     const initial = scroll.scrollTop
 
@@ -256,6 +258,12 @@ function containsDiff(root: Renderable): boolean {
   return root.getChildren().some(containsDiff)
 }
 
+function countDiffs(root: Renderable): number {
+  return (
+    (root instanceof DiffRenderable ? 1 : 0) + root.getChildren().reduce((total, child) => total + countDiffs(child), 0)
+  )
+}
+
 const session = {
   id: "session-1",
   projectID: "project-1",