Преглед на файлове

fix(tui): align wrapped inline tool rows (#28664)

Kit Langton преди 2 месеца
родител
ревизия
5fb85a6aa3

+ 109 - 33
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx

@@ -1524,6 +1524,8 @@ const PART_MAPPING = {
   reasoning: ReasoningPart,
 }
 
+const INLINE_TOOL_ICON_WIDTH = 2
+
 function ReasoningPart(props: { last: boolean; part: ReasoningPart; message: AssistantMessage }) {
   const { theme } = useTheme()
   const ctx = use()
@@ -1789,12 +1791,12 @@ function InlineTool(props: {
   part: ToolPart
   onClick?: () => void
 }) {
-  const [margin, setMargin] = createSignal(0)
   const { theme } = useTheme()
   const ctx = use()
   const sync = useSync()
   const renderer = useRenderer()
   const [hover, setHover] = createSignal(false)
+  const [errorExpanded, setErrorExpanded] = createSignal(false)
 
   const permission = createMemo(() => {
     const callID = sync.data.permission[ctx.sessionID]?.at(0)?.tool?.callID
@@ -1802,14 +1804,6 @@ function InlineTool(props: {
     return callID === props.part.callID
   })
 
-  const fg = createMemo(() => {
-    if (props.color) return props.color
-    if (permission()) return theme.warning
-    if (hover() && props.onClick) return theme.text
-    if (props.complete) return theme.textMuted
-    return theme.text
-  })
-
   const error = createMemo(() => (props.part.state.status === "error" ? props.part.state.error : undefined))
 
   const denied = createMemo(
@@ -1820,53 +1814,134 @@ function InlineTool(props: {
       error()?.includes("user dismissed"),
   )
 
+  const failed = createMemo(() => Boolean(error() && !denied()))
+  const clickable = createMemo(() => Boolean(props.onClick || failed()))
+  const fg = createMemo(() => {
+    if (props.color) return props.color
+    if (permission()) return theme.warning
+    if (failed()) return theme.error
+    if (hover() && props.onClick) return theme.text
+    if (props.complete) return theme.textMuted
+    return theme.text
+  })
+
   return (
-    <box
-      marginTop={margin()}
-      paddingLeft={3}
-      onMouseOver={() => props.onClick && setHover(true)}
+    <InlineToolRow
+      icon={props.icon}
+      iconColor={props.iconColor}
+      color={fg()}
+      errorColor={theme.error}
+      failed={failed()}
+      denied={Boolean(denied())}
+      error={error()}
+      errorExpanded={errorExpanded()}
+      complete={props.complete}
+      pending={props.pending}
+      spinner={props.spinner}
+      separateAfter={(id) =>
+        sync.data.message[ctx.sessionID]?.some((message) => message.role === "user" && message.id === id) ?? false
+      }
+      onMouseOver={() => clickable() && setHover(true)}
       onMouseOut={() => setHover(false)}
       onMouseUp={() => {
         if (renderer.getSelection()?.getSelectedText()) return
+        if (failed()) {
+          setErrorExpanded((value) => !value)
+          return
+        }
         props.onClick?.()
       }}
+    >
+      {props.children}
+    </InlineToolRow>
+  )
+}
+
+export function InlineToolRow(props: {
+  icon: string
+  iconColor?: RGBA
+  color?: RGBA
+  errorColor?: RGBA
+  failed?: boolean
+  denied?: boolean
+  error?: string
+  errorExpanded?: boolean
+  complete: any
+  pending: string
+  spinner?: boolean
+  children: JSX.Element
+  separateAfter?: (id: string | undefined) => boolean
+  onMouseOver?: () => void
+  onMouseOut?: () => void
+  onMouseUp?: () => void
+}) {
+  const [margin, setMargin] = createSignal(0)
+
+  return (
+    <box
+      marginTop={margin()}
+      paddingLeft={3}
+      onMouseOver={props.onMouseOver}
+      onMouseOut={props.onMouseOut}
+      onMouseUp={props.onMouseUp}
       renderBefore={function () {
         const el = this as BoxRenderable
         const parent = el.parent
         if (!parent) {
           return
         }
-        if (el.height > 1) {
-          setMargin(1)
-          return
-        }
         const children = parent.getChildren()
         const index = children.indexOf(el)
         const previous = children[index - 1]
-        if (!previous) {
-          setMargin(0)
-          return
-        }
-        if (previous.height > 1 || previous.id.startsWith("text-")) {
-          setMargin(1)
-          return
-        }
+        setMargin(
+          previous?.id.startsWith("text-") ||
+            previous?.id.startsWith("tool-block-") ||
+            props.separateAfter?.(previous?.id)
+            ? 1
+            : 0,
+        )
       }}
     >
       <Switch>
         <Match when={props.spinner}>
-          <Spinner color={fg()} children={props.children} />
+          <Spinner color={props.color} children={props.children} />
         </Match>
         <Match when={true}>
-          <text paddingLeft={3} fg={fg()} attributes={denied() ? TextAttributes.STRIKETHROUGH : undefined}>
-            <Show fallback={<>~ {props.pending}</>} when={props.complete}>
-              <span style={{ fg: props.iconColor }}>{props.icon}</span> {props.children}
-            </Show>
-          </text>
+          <Show
+            fallback={
+              <text
+                paddingLeft={3}
+                fg={props.color}
+                attributes={props.denied ? TextAttributes.STRIKETHROUGH : undefined}
+              >
+                ~ {props.pending}
+              </text>
+            }
+            when={props.complete}
+          >
+            <box flexDirection="row">
+              <text
+                width={INLINE_TOOL_ICON_WIDTH}
+                fg={props.failed ? props.errorColor : (props.iconColor ?? props.color)}
+                attributes={props.denied ? TextAttributes.STRIKETHROUGH : undefined}
+              >
+                {props.icon}
+              </text>
+              <text
+                flexGrow={1}
+                fg={props.failed ? props.errorColor : props.color}
+                attributes={props.denied ? TextAttributes.STRIKETHROUGH : undefined}
+              >
+                {props.children}
+              </text>
+            </box>
+          </Show>
         </Match>
       </Switch>
-      <Show when={error() && !denied()}>
-        <text fg={theme.error}>{error()}</text>
+      <Show when={props.failed && props.errorExpanded}>
+        <box paddingLeft={INLINE_TOOL_ICON_WIDTH}>
+          <text fg={props.errorColor}>{props.error}</text>
+        </box>
       </Show>
     </box>
   )
@@ -1885,6 +1960,7 @@ function BlockTool(props: {
   const error = createMemo(() => (props.part?.state.status === "error" ? props.part.state.error : undefined))
   return (
     <box
+      id={props.part ? "tool-block-" + props.part.id : undefined}
       border={["left"]}
       paddingTop={1}
       paddingBottom={1}

+ 54 - 0
packages/opencode/test/cli/tui/__snapshots__/inline-tool-wrap-snapshot.test.tsx.snap

@@ -0,0 +1,54 @@
+// Bun Snapshot v1, https://bun.sh/docs/test/snapshots
+
+exports[`TUI inline tool wrapping snapshots consecutive grep, glob, and read rows at a narrow width 1`] = `
+"   ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
+     *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+   ✱ Glob "**/*db*" in packages/opencode (6 matches)
+   → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
+   → Read packages/opencode/src/index.ts [offset=1, limit=100]
+   ✱ Grep "export const OPENCODE_DB|OPENCODE_DB|OPENCODE_DEV|Global\\.
+     Path\\.data|data =" in packages/opencode/src (115 matches)"
+`;
+
+exports[`TUI inline tool wrapping snapshots expanded tool errors under the tool text 1`] = `
+"   ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
+     *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+   ✱ Glob "**/*db*" in packages/opencode (6 matches)
+   → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
+   → Read packages/opencode/src/index.ts [offset=1, limit=100]
+     No LSP server available for this file type.
+   ✱ Grep "export const OPENCODE_DB|OPENCODE_DB|OPENCODE_DEV|Global\\.
+     Path\\.data|data =" in packages/opencode/src (115 matches)"
+`;
+
+exports[`TUI inline tool wrapping keeps separation after a shell output block 1`] = `
+"
+
+  # List files
+
+  $ ls
+
+  file.ts
+
+   ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
+     *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+   ✱ Glob "**/*db*" in packages/opencode (6 matches)
+   → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
+   → Read packages/opencode/src/index.ts [offset=1, limit=100]
+   ✱ Grep "export const OPENCODE_DB|OPENCODE_DB|OPENCODE_DEV|Global\\.
+     Path\\.data|data =" in packages/opencode/src (115 matches)"
+`;
+
+exports[`TUI inline tool wrapping keeps separation after a padded user message 1`] = `
+"
+  Check whether the next tool remains separated.
+
+
+   ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
+     *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+   ✱ Glob "**/*db*" in packages/opencode (6 matches)
+   → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
+   → Read packages/opencode/src/index.ts [offset=1, limit=100]
+   ✱ Grep "export const OPENCODE_DB|OPENCODE_DB|OPENCODE_DEV|Global\\.
+     Path\\.data|data =" in packages/opencode/src (115 matches)"
+`;

+ 119 - 0
packages/opencode/test/cli/tui/inline-tool-wrap-snapshot.test.tsx

@@ -0,0 +1,119 @@
+import { afterEach, describe, expect, test } from "bun:test"
+import { For } from "solid-js"
+import { testRender, type JSX } from "@opentui/solid"
+import { InlineToolRow } from "../../../src/cli/cmd/tui/routes/session/index"
+
+let testSetup: Awaited<ReturnType<typeof testRender>> | undefined
+
+afterEach(() => {
+  testSetup?.renderer.destroy()
+  testSetup = undefined
+})
+
+type ToolFixture = { icon: string; label: string; error?: string }
+
+const tools: readonly ToolFixture[] = [
+  {
+    icon: "✱",
+    label:
+      'Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.*dir|xdg|APPDATA" in packages/opencode/src (151 matches)',
+  },
+  {
+    icon: "✱",
+    label: 'Glob "**/*db*" in packages/opencode (6 matches)',
+  },
+  {
+    icon: "→",
+    label: "Read packages/opencode/src/storage/db.ts [offset=1, limit=130]",
+  },
+  {
+    icon: "→",
+    label: "Read packages/opencode/src/index.ts [offset=1, limit=100]",
+    error: "No LSP server available for this file type.",
+  },
+  {
+    icon: "✱",
+    label:
+      'Grep "export const OPENCODE_DB|OPENCODE_DB|OPENCODE_DEV|Global\\.Path\\.data|data =" in packages/opencode/src (115 matches)',
+  },
+] as const
+
+function ShellOutput() {
+  return (
+    <box id="tool-block-shell" marginTop={1} paddingTop={1} paddingBottom={1} paddingLeft={2} gap={1}>
+      <text paddingLeft={3}># List files</text>
+      <box gap={1}>
+        <text>$ ls</text>
+        <text>file.ts</text>
+      </box>
+    </box>
+  )
+}
+
+function UserMessage() {
+  return (
+    <box id="message-user">
+      <box paddingTop={1} paddingBottom={1} paddingLeft={2}>
+        <text>Check whether the next tool remains separated.</text>
+      </box>
+    </box>
+  )
+}
+
+function Fixture(props: { errorExpanded?: boolean; before?: "shell" | "user" }) {
+  return (
+    <box flexDirection="column" width={72}>
+      <box flexDirection="column">
+        {props.before === "shell" && <ShellOutput />}
+        {props.before === "user" && <UserMessage />}
+        <For each={tools}>
+          {(item) => (
+            <InlineToolRow
+              icon={item.icon}
+              complete={true}
+              pending=""
+              failed={Boolean(item.error)}
+              error={item.error}
+              errorExpanded={props.errorExpanded}
+              separateAfter={(id) => id === "message-user"}
+            >
+              {item.label}
+            </InlineToolRow>
+          )}
+        </For>
+      </box>
+    </box>
+  )
+}
+
+async function renderFrame(component: () => JSX.Element, options: { width: number; height: number }) {
+  testSetup = await testRender(component, options)
+  await testSetup.renderOnce()
+  await Bun.sleep(25)
+  await testSetup.renderOnce()
+
+  return testSetup
+    .captureCharFrame()
+    .split("\n")
+    .map((line) => line.trimEnd())
+    .join("\n")
+    .trimEnd()
+}
+
+describe("TUI inline tool wrapping", () => {
+  test("snapshots consecutive grep, glob, and read rows at a narrow width", async () => {
+    expect(await renderFrame(() => <Fixture />, { width: 72, height: 12 })).toMatchSnapshot()
+  })
+
+  test("snapshots expanded tool errors under the tool text", async () => {
+    expect(await renderFrame(() => <Fixture errorExpanded />, { width: 72, height: 12 })).toMatchSnapshot()
+  })
+
+  test("keeps separation after a shell output block", async () => {
+    expect(await renderFrame(() => <Fixture before="shell" />, { width: 72, height: 16 })).toMatchSnapshot()
+  })
+
+  test("keeps separation after a padded user message", async () => {
+    expect(await renderFrame(() => <Fixture before="user" />, { width: 72, height: 14 })).toMatchSnapshot()
+  })
+})