upstream merge 2026-05-08 PR 5: v2 diff pane / file pane polish#459
upstream merge 2026-05-08 PR 5: v2 diff pane / file pane polish#459
Conversation
📝 WalkthroughウォークスルーこのPRでは、Diffビューアペインの UI を大幅に改善しており、diff スタイル制御を専用コンポーネント 変更内容Diff ビューアー UI リファクタリング
推定コードレビュー労力🎯 3 (中程度) | ⏱️ ~25 分 ポエム
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…-lines strip (superset-sh#3899) - Match ChangesHeader chrome on each file header (border-y + bg-muted/30) - Split filename into dir + basename so the basename always stays visible on narrow widths instead of being truncated behind ellipsis - Add a Copy path button next to the filename, drop Copy file contents - Move StatusIndicator next to the +/- diff stats - Blend diff body with the terminal pane surface color - Flatten the "N unmodified lines" expander flush to the pane edges (kills pierre/diffs' wrapper / content / expand-button rounding + inline gaps for both line-info and line-info-basic separators) - Standardize icon-button padding, drop the inline DiffViewModeToggle in favour of a co-located DiffPaneHeaderExtras component
…e pane (superset-sh#3911) - DiffFileHeader: basename now also truncates as a fallback so root-level files (no directory prefix) no longer overlap the right-side actions. Directory still shrinks first via shrink-[1000]. - FilePaneHeaderExtras: add a Copy path button alongside Open in editor, matching the diff pane.
66ae5e2 to
252714d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/StatusIndicator/StatusIndicator.tsx (1)
51-61: ⚡ Quick win
statusをstringではなくFileStatusにして unsafe cast をなくしてください。
status as FileStatusが2箇所にあり、呼び出し側の不正値を型で防げていません。props型を絞るだけで安全性が上がります。✂️ 提案差分
export function StatusIndicator({ status, className, iconClassName = "w-3 h-3", }: { - status: string; + status: FileStatus; className?: string; iconClassName?: string; }) { return ( <span - className={`flex shrink-0 items-center ${STATUS_COLORS[status as FileStatus] ?? ""} ${className ?? ""}`} + className={`flex shrink-0 items-center ${STATUS_COLORS[status] ?? ""} ${className ?? ""}`} > - {getStatusIcon(status as FileStatus, iconClassName)} + {getStatusIcon(status, iconClassName)} </span> ); }As per coding guidelines "Maintain type safety as an agent rule: avoid
anyunless necessary. This applies to all TypeScript code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/StatusIndicator/StatusIndicator.tsx around lines 51 - 61, Change the StatusIndicator props so that status is typed as FileStatus instead of string and remove the unsafe casts; update the component signature that currently defines status: string to status: FileStatus, ensure FileStatus is imported where StatusIndicator is defined, and then call STATUS_COLORS[status] and getStatusIcon(status, iconClassName) without using "as FileStatus" so the type system prevents invalid values (affects the StatusIndicator function, STATUS_COLORS usage, and getStatusIcon invocation).apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx (1)
5-5: ⚡ Quick win
lucide-reactに統一することを検討してください(react-icons/luとの混在)同じファイル内で
lucide-react(Line 3:ChevronDown,ChevronRight,Eye,EyeOff)とreact-icons/luの両方から Lucide アイコンを使用しています。LuCheck/LuCopy/LuUndo2はいずれもlucide-reactにCheck/Copy/Undo2として存在するため、二重依存を排除できます。♻️ 修正提案
-import { ChevronDown, ChevronRight, Eye, EyeOff } from "lucide-react"; +import { Check, ChevronDown, ChevronRight, Copy, Eye, EyeOff, Undo2 } from "lucide-react";-import { LuCheck, LuCopy, LuUndo2 } from "react-icons/lu";- {copied ? ( - <LuCheck className="size-3.5" /> - ) : ( - <LuCopy className="size-3.5" /> - )} + {copied ? ( + <Check className="size-3.5" /> + ) : ( + <Copy className="size-3.5" /> + )}- <LuUndo2 className="size-3.5" /> + <Undo2 className="size-3.5" />Also applies to: 99-117, 181-184
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx at line 5, The file mixes lucide-react icons (ChevronDown, ChevronRight, Eye, EyeOff) with react-icons/lu imports (LuCheck, LuCopy, LuUndo2); replace the react-icons import with lucide-react equivalents by removing `import { LuCheck, LuCopy, LuUndo2 } from "react-icons/lu"` and instead import `Check`, `Copy`, `Undo2` from `lucide-react`, then update all usages of LuCheck/LuCopy/LuUndo2 to Check/Copy/Undo2 (including the other occurrences noted around lines ~99-117 and ~181-184) so the file uses only lucide-react and the duplicate dependency is eliminated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/StatusIndicator/StatusIndicator.tsx:
- Around line 51-61: Change the StatusIndicator props so that status is typed as
FileStatus instead of string and remove the unsafe casts; update the component
signature that currently defines status: string to status: FileStatus, ensure
FileStatus is imported where StatusIndicator is defined, and then call
STATUS_COLORS[status] and getStatusIcon(status, iconClassName) without using "as
FileStatus" so the type system prevents invalid values (affects the
StatusIndicator function, STATUS_COLORS usage, and getStatusIcon invocation).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsx:
- Line 5: The file mixes lucide-react icons (ChevronDown, ChevronRight, Eye,
EyeOff) with react-icons/lu imports (LuCheck, LuCopy, LuUndo2); replace the
react-icons import with lucide-react equivalents by removing `import { LuCheck,
LuCopy, LuUndo2 } from "react-icons/lu"` and instead import `Check`, `Copy`,
`Undo2` from `lucide-react`, then update all usages of LuCheck/LuCopy/LuUndo2 to
Check/Copy/Undo2 (including the other occurrences noted around lines ~99-117 and
~181-184) so the file uses only lucide-react and the duplicate dependency is
eliminated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8450f0d-03c8-4a4a-a948-a3050a29226e
📒 Files selected for processing (11)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/DiffPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileEntry/DiffFileEntry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffFileHeader/DiffFileHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffPaneHeaderExtras/DiffPaneHeaderExtras.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/DiffPaneHeaderExtras/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/utils/gitDecorationColors/gitDecorationColors.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/utils/gitDecorationColors/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/FilePaneHeaderExtras/FilePaneHeaderExtras.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Recorded as integrated via -s ours after batch PRs #455-#464. Taken via individual PRs: - PR 1 (#455): v2 polish 前半 safe set (9 commits) - PR 2 (#456): v2/host-service polish 中盤 (12 commits) - PR 3 (#457): sidebar polish + jwt API (5 commits) - PR 4 (#458): host-service tRPC retry/cache/timeout (3 commits) - PR 5 (#459): v2 diff pane / file pane polish (2 commits) - PR 7 (#462): host-service v2 canonical workspace.create + attachment store (PR1 superset-sh#3893 + PR2 superset-sh#3916) - PR 11 (#463): agents API + onboarding (7 commits + 1 cleanup) - PR 12 (#464): v1→v2 import flow rewrite (11 commits + 2 follow-ups) - PR 13 (#460): host-service shell env probe + typo (2 commits) - PR 16 (#461): marketplace 19 themes (1 commit) Skipped / deferred (recorded as integrated for behind=0): - PR 6: CLI v1 launch (superset-sh#3898 + 30+ CLI/SDK followups) — defer to dedicated migration - PR 9: v2 PR3 (superset-sh#3940) + revert (superset-sh#4017) — net-zero pair - PR 10: pty-daemon (superset-sh#3896, superset-sh#3971, superset-sh#4054) — fork keeps its terminal-host - PR 14: Slack MCP-v2 (superset-sh#4197, superset-sh#4208) — depends on mcp-v2/sdk divergence - PR 15: onboarding remaining (superset-sh#4115, superset-sh#4125, superset-sh#4214, superset-sh#4213, superset-sh#4222, superset-sh#4225) — depends on fork's deleted setup pages Behind: 0 after this merge.
Summary
upstream 同期バッチ第 5 弾。v2 diff pane / file pane polish (2 commits)。
進捗: PR1+2+3+4 完了、PR5 で 31 / 223 (4 まだ未マージ)。
取り込み内容
Fork 側のコンフリクト解決
DiffFileHeader.tsx(style(desktop): polish v2 diff pane header, file path, and unmodified-lines strip superset-sh/superset#3899): wrapper div className を upstream 採用 (flex-nowrap,border-y bg-muted/30)。fork のonDiscardprop は維持、onCopyContentsは upstream の方針で drop。upstream の Copy path button が fork のuseCopyToClipboardをすでに使っているため fork 互換WorkspaceDiff.tsx(style(desktop): polish v2 diff pane header, file path, and unmodified-lines strip superset-sh/superset#3899):useCopyToClipboard/electronTrpcを drop(後段で使われなくなる)、upstream のuseQueryfrom@tanstack/react-queryを採用useQuery({ queryKey, queryFn: () => electronTrpcClient... })形式を採用(cleaner)const utils = workspaceTrpc.useUtils();は維持(fork のhandleDiscard内でutils.git.getDiff/getStatus.invalidateを呼ぶ — 重要 cache 無効化挙動)FilePaneHeaderExtras.tsx(fix(desktop): truncate diff file path and add copy-path action to fil… superset-sh/superset#3911): fork は外側 wrapper (spreadsheet 早期 return) + Inner に分割している構造を維持。upstream が外側に追加したuseCopyToClipboardを Inner に移動してcopyToClipboard(filePath)/copiedをその場で利用Fork 固有機能ヘルスチェック
githubExtendedprocedure 健在Test plan
bun run typecheckexit 0 (28/28 tasks)bun run lintexit 0 (4456 files)次の PR
PR 6: CLI v1 / SDK / packages 大型バッチ
Summary by CodeRabbit
新機能
改善