diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f905ef7..9d1d4fb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -77,9 +77,6 @@ importers: '@vitejs/plugin-react': specifier: ^6.0.5 version: 6.0.5(vite@8.2.0(@types/node@26.1.2)(jiti@2.7.0)) - enhanced-resolve: - specifier: ^5.24.5 - version: 5.24.5 oxfmt: specifier: ^0.62.0 version: 0.62.0 diff --git a/src/gh.rs b/src/gh.rs index 33f34c1..de37224 100644 --- a/src/gh.rs +++ b/src/gh.rs @@ -28,9 +28,7 @@ pub struct RemoteRepo { #[serde(rename_all = "camelCase")] pub struct PullRequestInfo { pub title: String, - pub state: String, - pub draft: bool, - pub merged: bool, + pub status: PullStatus, pub author: String, pub created_at: chrono::DateTime, pub updated_at: chrono::DateTime, @@ -38,12 +36,18 @@ pub struct PullRequestInfo { pub deletions: i64, pub changed_files: i64, pub commits: i64, - pub head_ref: String, - pub head_label: String, - pub head_repo: String, - pub base_ref: String, - pub base_label: String, - pub base_repo: String, + pub base_branch: String, + pub head_branch: String, +} + +/// Display status of a pull request; serialized as the capitalized label the +/// web UI shows (mirrored by `PullRequestInfo["status"]` in web/src). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] +pub enum PullStatus { + Open, + Draft, + Merged, + Closed, } #[derive(Debug, Deserialize)] @@ -359,9 +363,7 @@ pub async fn pull_request_info( let response = fetch_pull(github_host, org, repo, number).await?; Ok(PullRequestInfo { title: response.title, - state: response.state, - draft: response.draft, - merged: response.merged, + status: pull_status(&response.state, response.draft, response.merged), author: response.user.map(|user| user.login).unwrap_or_default(), created_at: response.created_at, updated_at: response.updated_at, @@ -369,23 +371,40 @@ pub async fn pull_request_info( deletions: response.deletions, changed_files: response.changed_files, commits: response.commits, - head_ref: response.head.ref_name, - head_label: response.head.label, - head_repo: response - .head - .repo - .map(|repo| repo.full_name) - .unwrap_or_default(), - base_ref: response.base.ref_name, - base_label: response.base.label, - base_repo: response - .base - .repo - .map(|repo| repo.full_name) - .unwrap_or_default(), + base_branch: branch_label(&response.base), + head_branch: branch_label(&response.head), }) } +fn pull_status(state: &str, draft: bool, merged: bool) -> PullStatus { + if merged { + PullStatus::Merged + } else if state.eq_ignore_ascii_case("closed") { + PullStatus::Closed + } else if draft { + PullStatus::Draft + } else { + PullStatus::Open + } +} + +/// GitHub's `owner:branch` label, else `repo:ref`, else the bare ref. +fn branch_label(branch: &PullRef) -> String { + let label = branch.label.trim(); + let ref_name = branch.ref_name.trim(); + let repo = branch + .repo + .as_ref() + .map_or("", |repo| repo.full_name.trim()); + if !label.is_empty() { + label.to_string() + } else if !repo.is_empty() && !ref_name.is_empty() { + format!("{repo}:{ref_name}") + } else { + ref_name.to_string() + } +} + fn github_side(side: &str) -> &str { match side { "deletions" => "LEFT", @@ -1038,6 +1057,39 @@ mod tests { } } + #[test] + fn pull_status_prefers_merged_then_closed_then_draft() { + assert_eq!(pull_status("closed", false, true), PullStatus::Merged); + assert_eq!(pull_status("closed", true, false), PullStatus::Closed); + assert_eq!(pull_status("open", true, false), PullStatus::Draft); + assert_eq!(pull_status("open", false, false), PullStatus::Open); + assert_eq!( + serde_json::to_string(&PullStatus::Draft).unwrap(), + "\"Draft\"" + ); + } + + #[test] + fn branch_label_falls_back_to_repo_and_ref() { + let branch = |label: &str, repo: Option<&str>, ref_name: &str| PullRef { + ref_name: ref_name.into(), + label: label.into(), + repo: repo.map(|name| RepoName { + full_name: name.into(), + }), + sha: String::new(), + }; + assert_eq!( + branch_label(&branch("me:feat", Some("me/x"), "feat")), + "me:feat" + ); + assert_eq!( + branch_label(&branch(" ", Some("me/x"), "feat")), + "me/x:feat" + ); + assert_eq!(branch_label(&branch("", None, "feat")), "feat"); + } + #[test] fn encode_path_segments_percent_encodes_each_segment() { assert_eq!(encode_path_segments("src/main.rs"), "src/main.rs"); diff --git a/web/package.json b/web/package.json index fb33a5f..2fc7f8f 100644 --- a/web/package.json +++ b/web/package.json @@ -36,7 +36,6 @@ "@types/react": "^19.2.18", "@types/react-dom": "^19.2.4", "@vitejs/plugin-react": "^6.0.5", - "enhanced-resolve": "^5.24.5", "oxfmt": "^0.62.0", "oxlint": "^1.77.0", "typescript": "~7.0.2", diff --git a/web/src/App.tsx b/web/src/App.tsx index 0d0fd58..2e34d9f 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -1,7 +1,6 @@ import { BrowserRouter, Routes, Route } from "react-router"; -import { Home } from "./components/Home"; +import { Home, NotFound } from "./components/Home"; import { DiffView } from "./components/DiffView"; -import { NotFound } from "./components/NotFound"; export default function App() { return ( diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 15fffc1..8cbbe59 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -58,7 +58,7 @@ import { import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import { DiffAnnotation } from "./diff-view/DiffAnnotation"; import { DiffStatusScreen } from "./diff-view/DiffStatusScreen"; -import { DiffToolbar } from "./diff-view/DiffToolbar"; +import { DiffToolbar, type ToolbarLink } from "./diff-view/DiffToolbar"; import { FileActionsMenu } from "./diff-view/FileActionsMenu"; import { ShortcutsDialog } from "./diff-view/ShortcutsDialog"; import { SidebarTree } from "./diff-view/SidebarTree"; @@ -67,11 +67,6 @@ import type { AppConfig, CodeViewLineSelection, CommentTarget, - DiffOrderBy, - DiffOrderDir, - DiffSettingsProps, - DiffStyle, - DiffThemeId, PendingCommentDraft, PatchLoadState, PullRequestInfo, @@ -79,10 +74,6 @@ import type { } from "./diff-view/types"; import { diffThemeOptions, - isDiffOrderBy, - isDiffOrderDir, - isDiffStyle, - isDiffThemeId, localRepoTitle, prDiffPathFromUrl, selectedRangeEndLine, @@ -93,8 +84,8 @@ import { threadEndLine, threadEndSide, } from "./diff-view/helpers"; -import { decodeStoredBool, usePersistentState } from "./diff-view/usePersistentState"; -import { apiFetch } from "@/lib/api"; +import { useSettings, type SetSetting } from "./diff-view/useSettings"; +import { apiFetch, apiSend } from "@/lib/api"; import { DIFF_SURFACE_FONT_SIZE } from "@/lib/diffTypography"; import { exportDiffToHtml } from "@/lib/exportHtml"; import { DEFAULT_CODE_FONT_FAMILY, DEFAULT_UI_FONT_FAMILY, prependFontFamily } from "@/lib/fonts"; @@ -108,20 +99,18 @@ const codeViewStyle = { "--diffs-font-size": DIFF_SURFACE_FONT_SIZE, } as CSSProperties; -const STORAGE_DIFF_THEME = "diff-theme"; -const STORAGE_DIFF_STYLE = "diffs-diff-style"; -const STORAGE_ORDER_BY = "diffs-order-by"; -const STORAGE_ORDER_DIR = "diffs-order-dir"; -const STORAGE_WORD_WRAP = "diffs-word-wrap"; -const STORAGE_LINE_NUMBERS = "diffs-line-numbers"; -const STORAGE_LINE_BACKGROUNDS = "diffs-line-backgrounds"; -const STORAGE_COLLAPSE_REMOVALS = "diffs-collapse-removals"; -const STORAGE_HIDE_REVIEWED = "diffs-hide-reviewed"; - // Window after a programmatic scroll during which the cursor isn't re-synced. const PROGRAMMATIC_SCROLL_SETTLE_MS = 250; const EDITABLE_TAGS = /^(INPUT|TEXTAREA|SELECT)$/; +type Viewer = CodeViewHandle; +type Item = CodeViewItem; + +// updateItem ignores same-version updates, so every change bumps the version. +function patchItem(viewer: Viewer, item: Item, patch: Partial) { + viewer.updateItem({ ...item, ...patch, version: (item.version ?? 0) + 1 } as Item); +} + // FNV-1a hash, folded to base36. function fnv1a(build: (feed: (s: string) => void) => void): string { let h = 0x811c9dc5; @@ -286,38 +275,19 @@ function createPendingThread(target: CommentTarget, body: string): ReviewThread // Local/branch diffs use an all-zeros object id for the working-tree side // (git diffs against the workdir, not a blob), so that side has to be read // from disk instead of the object database. -function isZeroOid(oid: string): boolean { - return /^0+$/.test(oid); -} - -// Full-file hydration for `loadDiffFiles`: fetches one side of a PR file from -// its GitHub repo/sha via the server-side proxy (fork-aware; see -// gh::pull_request_file). -async function loadPullRequestFileContents( - org: string, - repo: string, - number: string, - path: string, - side: "old" | "new", -): Promise { - const url = `/api/pull/${encodeURIComponent(org)}/${encodeURIComponent(repo)}/${encodeURIComponent(number)}/file?path=${encodeURIComponent(path)}&side=${side}`; - const contents = await apiFetch(url); - return { name: path, contents }; -} - -// Full-file hydration for a committed blob (local/branch diffs' non-worktree -// side), keyed for highlight cache reuse across renders of the same blob. -async function loadBlobFileContents(oid: string, name: string): Promise { - const contents = await apiFetch(`/api/blob?oid=${encodeURIComponent(oid)}`); - return { name, contents, cacheKey: `blob:${oid}` }; +function isZeroOid(oid: string | undefined): boolean { + return !oid || /^0+$/.test(oid); } -// Full-file hydration for the zero-oid working-tree side of a local/branch -// diff (uncommitted content, so there's no blob to fetch by oid). -async function loadWorktreeFileContents(path: string): Promise { - const contents = await apiFetch(`/api/blob?path=${encodeURIComponent(path)}&worktree=1`); - return { name: path, contents }; +// Full-file hydration for `loadDiffFiles`. Blobs are keyed for highlight cache +// reuse; PR files go through the fork-aware server proxy (gh::pull_request_file). +async function fetchFile(url: string, name: string, cacheKey?: string): Promise { + return { name, contents: await apiFetch(url), cacheKey }; } +const fetchBlob = (oid: string, name: string) => + fetchFile(`/api/blob?oid=${encodeURIComponent(oid)}`, name, `blob:${oid}`); +const fetchWorktree = (path: string) => + fetchFile(`/api/blob?${new URLSearchParams({ path, worktree: "1" })}`, path); export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" } = {}) { const { org, repo, number } = useParams<{ @@ -329,51 +299,9 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const baseRef = source === "branch" ? (searchParams.get("base") ?? "") : ""; const includeDirty = source === "branch" && searchParams.get("dirty") === "1"; - const [diffStyle, setDiffStyle, setDiffStyleLocal] = usePersistentState( - STORAGE_DIFF_STYLE, - "split", - (r) => (isDiffStyle(r) ? r : null), - ); - const [orderBy, setOrderBy] = usePersistentState(STORAGE_ORDER_BY, "path", (r) => - isDiffOrderBy(r) ? r : null, - ); - const [orderDir, setOrderDir] = usePersistentState(STORAGE_ORDER_DIR, "asc", (r) => - isDiffOrderDir(r) ? r : null, - ); - const [diffThemeId, setDiffThemeId, setDiffThemeLocal] = usePersistentState( - STORAGE_DIFF_THEME, - "pierre", - (r) => (isDiffThemeId(r) ? r : null), - ); - const [showBackground, setShowBackground, setShowBackgroundLocal] = usePersistentState( - STORAGE_LINE_BACKGROUNDS, - true, - decodeStoredBool, - ); - const [showLineNumbers, setShowLineNumbers, setShowLineNumbersLocal] = usePersistentState( - STORAGE_LINE_NUMBERS, - true, - decodeStoredBool, - ); - const [wordWrap, setWordWrap, setWordWrapLocal] = usePersistentState( - STORAGE_WORD_WRAP, - false, - decodeStoredBool, - ); - const [collapseRemovals, setCollapseRemovals] = usePersistentState( - STORAGE_COLLAPSE_REMOVALS, - false, - decodeStoredBool, - ); - const [hideReviewed, setHideReviewed] = usePersistentState( - STORAGE_HIDE_REVIEWED, - false, - decodeStoredBool, - ); const [appColorScheme, setAppColorScheme] = useState(() => initialColorScheme()); const [systemColorScheme, setSystemColorScheme] = useState(() => resolveColorScheme("system")); const [allCollapsedOverride, setAllCollapsed] = useState(null); - const [settingsOpen, setSettingsOpen] = useState(false); const [shortcutsOpen, setShortcutsOpen] = useState(false); const [sidebarOpen, setSidebarOpen] = useState(true); const [mobileSidebarOpen, setMobileSidebarOpen] = useState(false); @@ -385,7 +313,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" branchBase?: string; } | null>(null); const repoContextRequested = useRef(false); - const viewerRef = useRef | null>(null); + const viewerRef = useRef(null); const [editingItemId, setEditingItemId] = useState(null); // Ref copy for callbacks that shouldn't re-subscribe. const editingItemIdRef = useRef(null); @@ -408,6 +336,18 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" gitBranch: "", githubHost: "github.com", }); + const [settings, setSetting] = useSettings(config); + const { + diffStyle, + orderBy, + orderDir, + diffTheme, + lineBackgrounds, + lineNumbers, + wordWrap, + collapseRemovals, + hideReviewed, + } = settings; const isLocal = source === "local"; const isBranch = source === "branch"; @@ -420,17 +360,18 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const scrollStorageKey = `diffs-scroll:${sessionKey}`; const collapsedStorageKey = `diffs-collapsed:${sessionKey}`; const reviewedStorageKey = `diffs-reviewed:${sessionKey}`; + const hasPr = !usesLocalStore && !!org && !!repo && !!number; const prUrl = org && repo && number ? `https://${config.githubHost}/${org}/${repo}/pull/${number}` : ""; + const prQuery = hasPr ? new URLSearchParams({ org, repo, number }).toString() : ""; const commentsEndpoint = usesLocalStore ? "/api/comments" - : org && repo && number - ? `/api/comments?org=${encodeURIComponent(org)}&repo=${encodeURIComponent(repo)}&number=${encodeURIComponent(number)}` - : null; - const pullRequestInfoEndpoint = - !usesLocalStore && org && repo && number - ? `/api/pull/${encodeURIComponent(org)}/${encodeURIComponent(repo)}/${encodeURIComponent(number)}` + : hasPr + ? `/api/comments?${prQuery}` : null; + const pullRequestInfoEndpoint = hasPr + ? `/api/pull/${[org, repo, number].map(encodeURIComponent).join("/")}` + : null; const baseTitle = isBranch ? `${baseRef || "base"} ← ${config.gitBranch.trim() || "HEAD"}` : isLocal @@ -452,37 +393,15 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" if (ignore) return; applyConfigFontFamilies(nextConfig); setConfig(nextConfig); - const unset = (key: string) => localStorage.getItem(key) == null; if (isAppColorScheme(nextConfig.colorScheme) && storedColorScheme() == null) { setAppColorScheme(nextConfig.colorScheme); } - if (isDiffThemeId(nextConfig.diffTheme) && unset(STORAGE_DIFF_THEME)) { - setDiffThemeLocal(nextConfig.diffTheme); - } - if (isDiffStyle(nextConfig.diffStyle) && unset(STORAGE_DIFF_STYLE)) { - setDiffStyleLocal(nextConfig.diffStyle); - } - if (typeof nextConfig.wordWrap === "boolean" && unset(STORAGE_WORD_WRAP)) { - setWordWrapLocal(nextConfig.wordWrap); - } - if (typeof nextConfig.lineNumbers === "boolean" && unset(STORAGE_LINE_NUMBERS)) { - setShowLineNumbersLocal(nextConfig.lineNumbers); - } - if (typeof nextConfig.lineBackgrounds === "boolean" && unset(STORAGE_LINE_BACKGROUNDS)) { - setShowBackgroundLocal(nextConfig.lineBackgrounds); - } }) .catch(() => {}); return () => { ignore = true; }; - }, [ - setDiffStyleLocal, - setDiffThemeLocal, - setShowBackgroundLocal, - setShowLineNumbersLocal, - setWordWrapLocal, - ]); + }, []); useEffect(() => { document.title = pageTitle; @@ -597,26 +516,23 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const loading = effectivePatchState.status === "loading"; const error = effectivePatchState.status === "error" ? effectivePatchState.error : null; - const files = useMemo(() => { - if (effectivePatchState.status !== "loaded" || !effectivePatchState.patch) return []; - const parsed = parsePatchFiles( - effectivePatchState.patch, - patchCacheKeyPrefix(effectivePatchState.patch), - ); - return sortFiles( - parsed.flatMap((p) => p.files), - orderBy, - orderDir, - ); - }, [effectivePatchState, orderBy, orderDir]); + const patch = effectivePatchState.status === "loaded" ? effectivePatchState.patch : null; + const parsedFiles = useMemo( + () => (patch ? parsePatchFiles(patch, patchCacheKeyPrefix(patch)).flatMap((p) => p.files) : []), + [patch], + ); + const files = useMemo( + () => sortFiles(parsedFiles, orderBy, orderDir), + [parsedFiles, orderBy, orderDir], + ); const fileSignatures = useMemo(() => { const map = new Map(); for (const f of files) if (!map.has(f.name)) map.set(f.name, fileDiffSignature(f)); return map; }, [files]); const filePatchSections = useMemo( - () => splitPatchByFile(effectivePatchState.patch), - [effectivePatchState.patch], + () => splitPatchByFile(patch, parsedFiles), + [patch, parsedFiles], ); const loadRepoContext = useCallback(() => { if (!usesLocalStore || repoContextRequested.current) return; @@ -667,7 +583,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" pullRequestInfo?.endpoint === pullRequestInfoEndpoint ? pullRequestInfo.info : null; const filePaths = useMemo(() => [...new Set(visibleFiles.map((f) => f.name))], [visibleFiles]); - const initialItems = useMemo[]>(() => { + const initialItems = useMemo(() => { const collapsed = readCollapsedPaths(collapsedStorageKey); const reviewedSigs = readReviewedSignatures(reviewedStorageKey); return files @@ -706,8 +622,8 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" return map; }, [files, hiddenReviewedNames]); const selectedDiffTheme = useMemo( - () => diffThemeOptions.find((option) => option.id === diffThemeId) ?? diffThemeOptions[0], - [diffThemeId], + () => diffThemeOptions.find((option) => option.id === diffTheme) ?? diffThemeOptions[0], + [diffTheme], ); const resolvedAppColorScheme = appColorScheme === "system" ? systemColorScheme : appColorScheme; const workerPool = useWorkerPool(); @@ -824,11 +740,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const item = viewer?.getItem(itemId); if (item == null) return; const nextCollapsed = !item.collapsed; - viewer!.updateItem({ - ...item, - version: (item.version ?? 0) + 1, - collapsed: nextCollapsed, - }); + patchItem(viewer!, item, { collapsed: nextCollapsed }); if (item.type === "diff" && item.fileDiff) { const stored = readCollapsedPaths(collapsedStorageKey); if (nextCollapsed) stored.add(item.fileDiff.name); @@ -847,7 +759,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" for (const item of initialItems) { const current = viewer.getItem(item.id); if (current && current.collapsed !== next) { - viewer.updateItem({ ...current, version: (current.version ?? 0) + 1, collapsed: next }); + patchItem(viewer, current, { collapsed: next }); } if (next && item.type === "diff" && item.fileDiff) collapsedPaths.add(item.fileDiff.name); } @@ -869,9 +781,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" else map.delete(name); persistReviewedSignatures(reviewedStorageKey, map); setReviewed({ key: reviewedStorageKey, map }); - viewer!.updateItem({ - ...item, - version: (item.version ?? 0) + 1, + patchItem(viewer!, item, { collapsed: computeCollapsed( item.fileDiff, readCollapsedPaths(collapsedStorageKey), @@ -956,17 +866,10 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" setAppColorScheme(value); persistColorScheme(value); }, []); - const handleDiffStyleToggle = useCallback( - () => setDiffStyle(diffStyle === "split" ? "unified" : "split"), - [diffStyle, setDiffStyle], - ); - const handleOrderDirToggle = useCallback( - () => setOrderDir(orderDir === "asc" ? "desc" : "asc"), - [orderDir, setOrderDir], - ); - const handleCollapseRemovalsChange = useCallback( - (value: boolean) => { - setCollapseRemovals(value); + const handleSettingChange = useCallback( + (name, value) => { + setSetting(name, value); + if (name !== "collapseRemovals") return; const viewer = viewerRef.current; if (!viewer) return; const manualCollapsed = readCollapsedPaths(collapsedStorageKey); @@ -979,14 +882,12 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" manualCollapsed, reviewed.map, fileSignatures.get(item.fileDiff.name), - value, + value as boolean, ); - if (current.collapsed !== next) { - viewer.updateItem({ ...current, version: (current.version ?? 0) + 1, collapsed: next }); - } + if (current.collapsed !== next) patchItem(viewer, current, { collapsed: next }); } }, - [initialItems, collapsedStorageKey, reviewed, fileSignatures, setCollapseRemovals], + [initialItems, collapsedStorageKey, reviewed, fileSignatures, setSetting], ); const clearCommentTarget = useCallback(() => { @@ -995,7 +896,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" }, []); const openCommentTarget = useCallback( - (range: SelectedLineRange | null, context: { item: CodeViewItem }) => { + (range: SelectedLineRange | null, context: { item: Item }) => { if (range == null || context.item.type !== "diff") return; const target = { itemId: context.item.id, @@ -1024,18 +925,8 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" clearCommentTarget(); return; } - apiFetch(commentsEndpoint, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - path: commentTarget.path, - line: commentTarget.line, - side: commentTarget.side, - endLine: commentTarget.endLine, - endSide: commentTarget.endSide, - body, - }), - }) + const { path, line, side, endLine, endSide } = commentTarget; + apiSend(commentsEndpoint, "POST", { path, line, side, endLine, endSide, body }) .then((thread) => { setCommentThreads((prev) => [...prev.filter((t) => t.id !== thread.id), thread]); clearCommentTarget(); @@ -1060,11 +951,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" for (const pendingThread of pendingCommentThreads) { const draft = pendingThread.draft; if (!draft) continue; - const submittedThread = await apiFetch(commentsEndpoint, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(draft), - }); + const submittedThread = await apiSend(commentsEndpoint, "POST", draft); setCommentThreads((prev) => [ ...prev.filter( (thread) => thread.id !== pendingThread.id && thread.id !== submittedThread.id, @@ -1135,9 +1022,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" if (!annotationsChanged(current.annotations, annotations)) continue; - viewer.updateItem({ - ...current, - version: (current.version ?? 0) + 1, + patchItem(viewer, current, { annotations: annotations.length > 0 ? annotations : undefined, }); } @@ -1145,46 +1030,36 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const loadDiffFiles = useCallback( async (fileDiff: FileDiffMetadata): Promise => { + const oldName = fileDiff.prevName ?? fileDiff.name; + const newName = fileDiff.name; if (!usesLocalStore) { - if (!org || !repo || !number) throw new Error("missing pull request target"); - const newPath = fileDiff.name; - if (fileDiff.type === "rename-pure") { - const newFile = await loadPullRequestFileContents(org, repo, number, newPath, "new"); - return { oldFile: null, newFile }; - } - const oldPath = fileDiff.prevName ?? fileDiff.name; + if (!hasPr) throw new Error("missing pull request target"); + const prFile = (path: string, side: "old" | "new") => + fetchFile(`${pullRequestInfoEndpoint}/file?${new URLSearchParams({ path, side })}`, path); + if (fileDiff.type === "rename-pure") + return { oldFile: null, newFile: await prFile(newName, "new") }; const [oldFile, newFile] = await Promise.all([ - loadPullRequestFileContents(org, repo, number, oldPath, "old"), - loadPullRequestFileContents(org, repo, number, newPath, "new"), + prFile(oldName, "old"), + prFile(newName, "new"), ]); return { oldFile, newFile }; } - - const oldName = fileDiff.prevName ?? fileDiff.name; - const newName = fileDiff.name; - const prevObjectId = fileDiff.prevObjectId; - const newObjectId = fileDiff.newObjectId; // Pure renames carry no `index` line (and thus no object ids); their // content is unchanged, so read the new path from the worktree. - if (fileDiff.type === "rename-pure") { - const newFile = await loadWorktreeFileContents(newName); - return { oldFile: null, newFile }; - } - if (!prevObjectId || isZeroOid(prevObjectId)) { - throw new Error(`missing prevObjectId for ${fileDiff.name}`); - } + if (fileDiff.type === "rename-pure") + return { oldFile: null, newFile: await fetchWorktree(newName) }; + const { prevObjectId, newObjectId } = fileDiff; + if (isZeroOid(prevObjectId)) throw new Error(`missing prevObjectId for ${fileDiff.name}`); const [oldFile, newFile] = await Promise.all([ - loadBlobFileContents(prevObjectId, oldName), + fetchBlob(prevObjectId!, oldName), // Local worktree oids usually aren't in the object db; read the file. - isLocal && fileDiff.type !== "deleted" - ? loadWorktreeFileContents(newName) - : newObjectId && !isZeroOid(newObjectId) - ? loadBlobFileContents(newObjectId, newName) - : loadWorktreeFileContents(newName), + (isLocal && fileDiff.type !== "deleted") || isZeroOid(newObjectId) + ? fetchWorktree(newName) + : fetchBlob(newObjectId!, newName), ]); return { oldFile, newFile }; }, - [usesLocalStore, isLocal, org, repo, number], + [usesLocalStore, isLocal, hasPr, pullRequestInfoEndpoint], ); // Only local diffs edit the worktree. Pure renames have no rows to edit. @@ -1212,13 +1087,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" cacheKey: `${item.fileDiff.cacheKey ?? item.id}:complete`, } : undefined; - // updateItem ignores same-version updates. - viewer.updateItem({ - ...item, - ...(fileDiff ? { fileDiff } : {}), - edit: true, - version: (item.version ?? 0) + 1, - }); + patchItem(viewer, item, { ...(fileDiff ? { fileDiff } : {}), edit: true }); }, []); const finishEditingFile = useCallback((itemId: string, decision: "accept" | "reject") => { @@ -1227,7 +1096,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const item = viewer?.getItem(itemId); if (viewer && item) { // Ends the session; handleItemEditComplete applies the decision. - viewer.updateItem({ ...item, edit: false, version: (item.version ?? 0) + 1 }); + patchItem(viewer, item, { edit: false }); } else { editingItemIdRef.current = null; setEditingItemId(null); @@ -1236,11 +1105,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const saveWorktreeFile = useCallback(async (path: string, contents: string) => { try { - await apiFetch("/api/worktree-file", { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ path, contents }), - }); + await apiSend("/api/worktree-file", "PUT", { path, contents }); } catch (err) { console.error(`Failed to save ${path}:`, err); // Disk still has the old content; re-sync the view. @@ -1291,8 +1156,8 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" diffStyle, hunkSeparators: "line-info" as const, stickyHeaders: true, - disableBackground: !showBackground, - disableLineNumbers: !showLineNumbers, + disableBackground: !lineBackgrounds, + disableLineNumbers: !lineNumbers, overflow: (wordWrap ? "wrap" : "scroll") as "wrap" | "scroll", enableGutterUtility: true, enableLineSelection: true, @@ -1308,8 +1173,8 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" selectedDiffTheme.themeType, resolvedAppColorScheme, diffStyle, - showBackground, - showLineNumbers, + lineBackgrounds, + lineNumbers, wordWrap, openCommentTarget, loadDiffFiles, @@ -1364,7 +1229,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" ); const renderHeaderPrefix = useCallback( - (item: CodeViewItem) => { + (item: Item) => { const isCollapsed = item.collapsed ?? false; return ( @@ -1393,7 +1258,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" ); const renderHeaderMetadata = useCallback( - (item: CodeViewItem) => { + (item: Item) => { if (item.type !== "diff" || !item.fileDiff) return null; if (editingItemId === item.id) { return ( @@ -1470,6 +1335,11 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" ], ); + const branchDiffPath = + isLocal && repoContext?.branchBase + ? `/branch?base=${encodeURIComponent(repoContext.branchBase)}` + : undefined; + if (loading) { return (
Loading diff...
@@ -1487,23 +1357,15 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" } if (files.length === 0) { - const emptyTitle = isBranch - ? "No commits ahead" - : isLocal - ? "No file changes yet" - : "No files changed"; - const emptyMessage = isBranch - ? `No commits ahead of ${baseRef || "base"}.` + const [emptyTitle, emptyMessage] = isBranch + ? ["No commits ahead", `No commits ahead of ${baseRef || "base"}.`] : isLocal - ? "The latest diffs are no longer available." - : "This pull request doesn't change any files."; + ? ["No file changes yet", "The latest diffs are no longer available."] + : ["No files changed", "This pull request doesn't change any files."]; return ( } title={emptyTitle} description={emptyMessage}> - {isLocal && repoContext?.branchBase ? ( - + {branchDiffPath ? ( + View branch diff ) : !isLocal && !isBranch && prUrl !== "" ? ( @@ -1525,18 +1387,18 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" ); } - const githubRepoUrl = - !usesLocalStore && org && repo - ? `https://${config.githubHost}/${org}/${repo}` - : repoContext?.repoUrl; - const githubPrUrl = usesLocalStore ? repoContext?.prUrl : prUrl || undefined; - const prDiffPath = - usesLocalStore && repoContext?.prUrl ? prDiffPathFromUrl(repoContext.prUrl) : undefined; - const branchDiffPath = - isLocal && repoContext?.branchBase - ? `/branch?base=${encodeURIComponent(repoContext.branchBase)}` - : undefined; - const localDiffPath = isBranch ? "/local" : undefined; + const menuLinks: ToolbarLink[] = [ + [branchDiffPath, "branch"], + [isBranch ? "/local" : undefined, "local"], + [usesLocalStore && repoContext?.prUrl ? prDiffPathFromUrl(repoContext.prUrl) : undefined, "pr"], + [usesLocalStore ? repoContext?.prUrl : prUrl, "github-pr"], + [ + !usesLocalStore && org && repo + ? `https://${config.githubHost}/${org}/${repo}` + : repoContext?.repoUrl, + "github-repo", + ], + ]; const sidebarTreeProps = { paths: filePaths, @@ -1548,31 +1410,6 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" colorScheme: resolvedAppColorScheme, }; - const settingsProps: DiffSettingsProps = { - appColorScheme, - onColorSchemeChange: handleColorSchemeChange, - diffStyle, - onDiffStyleToggle: handleDiffStyleToggle, - orderBy, - orderDir, - onOrderByChange: setOrderBy, - onOrderDirToggle: handleOrderDirToggle, - diffThemeId, - onDiffThemeChange: setDiffThemeId, - selectedDiffThemeLabel: selectedDiffTheme.label, - showBackground, - setShowBackground, - showLineNumbers, - setShowLineNumbers, - wordWrap, - setWordWrap, - collapseRemovals, - setCollapseRemovals: handleCollapseRemovalsChange, - hideReviewed, - setHideReviewed, - onShortcutsOpen: () => setShortcutsOpen(true), - }; - return (
@@ -1582,23 +1419,23 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" isLocal={usesLocalStore} baseRef={isBranch ? baseRef : undefined} includeDirty={isBranch ? includeDirty : false} - onSettingsOpenChange={setSettingsOpen} onSidebarToggle={openSidebar} onSubmitPendingComments={submitPendingComments} onToggleAllCollapsed={toggleAllFilesCollapsed} onExport={handleExport} exporting={exporting} onMenuOpen={loadRepoContext} - githubRepoUrl={githubRepoUrl} - githubPrUrl={githubPrUrl} - prDiffPath={prDiffPath} - branchDiffPath={branchDiffPath} - localDiffPath={localDiffPath} + links={menuLinks} pendingCommentCount={pendingCommentThreads.length} pullRequestInfo={currentPullRequestInfo} prUrl={prUrl} - settings={settingsProps} - settingsOpen={settingsOpen} + settings={{ + settings, + onSettingChange: handleSettingChange, + appColorScheme, + onColorSchemeChange: handleColorSchemeChange, + onShortcutsOpen: () => setShortcutsOpen(true), + }} sidebarOpen={sidebarOpen} submittingPendingComments={submittingPendingComments} /> @@ -1631,7 +1468,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch"
- + > +

diffs

+ + ); +} + +export function NotFound() { + const location = useLocation(); + return ( + +/:org/:repo/pull/:number`} + > +

+ 404 +

+

Page not found

+

+ No route matches{" "} + + {location.pathname} + + . +

+ ); } diff --git a/web/src/components/NotFound.tsx b/web/src/components/NotFound.tsx deleted file mode 100644 index 8048b64..0000000 --- a/web/src/components/NotFound.tsx +++ /dev/null @@ -1,51 +0,0 @@ -import { Link, useLocation } from "react-router"; -import { useEffect } from "react"; - -export function NotFound() { - const location = useLocation(); - - useEffect(() => { - document.title = "Not found · diffs"; - }, []); - - return ( -
-
-

- 404 -

-

Page not found

-

- No route matches{" "} - - {location.pathname} - - . -

-
- - Go home - - - Open Local Diff - -
- 
-          {`/
-/local
-/branch?base=
-/:org/:repo/pull/:number`}
-
-
-
- ); -} diff --git a/web/src/components/diff-view/CommentAvatar.tsx b/web/src/components/diff-view/CommentAvatar.tsx deleted file mode 100644 index ce1cac8..0000000 --- a/web/src/components/diff-view/CommentAvatar.tsx +++ /dev/null @@ -1,21 +0,0 @@ -function escapeSvgText(value: string): string { - return value.replaceAll("&", "&").replaceAll("<", "<").replaceAll(">", ">"); -} - -function avatarDataUri(author: string): string { - const label = escapeSvgText(author.trim().slice(0, 1).toUpperCase() || "?"); - const svg = `${label}`; - return `data:image/svg+xml,${encodeURIComponent(svg)}`; -} - -export function CommentAvatar({ author }: { author?: string }) { - const name = author?.trim() || "Commenter"; - - return ( - {name} - ); -} diff --git a/web/src/components/diff-view/DiffAnnotation.tsx b/web/src/components/diff-view/DiffAnnotation.tsx index baf91a5..75f5e58 100644 --- a/web/src/components/diff-view/DiffAnnotation.tsx +++ b/web/src/components/diff-view/DiffAnnotation.tsx @@ -1,7 +1,5 @@ import { CommentInput } from "@/components/CommentInput"; -import { IconTrash } from "@tabler/icons-react"; -import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; -import { CommentAvatar } from "./CommentAvatar"; +import { CommentAvatar, DeleteThreadButton, ThreadBadges } from "./ThreadParts"; import type { AnnotationMeta, ReviewThread } from "./types"; type AnnotationLike = { @@ -36,37 +34,17 @@ export function DiffAnnotation({
{latestComment?.author ? `${latestComment.author} commented` : "Commented"} - {meta.thread.comments.length > 1 && ( - - {meta.thread.comments.length} - - )} - {meta.thread.pending && ( - - Pending - - )} +

{latestComment?.body ?? ""}

{canDelete && ( - - onDeleteComment(meta.thread)} - > - - - } - /> - Delete comment - + onDeleteComment(meta.thread)} + /> )} ); diff --git a/web/src/components/diff-view/DiffSettingsPopover.tsx b/web/src/components/diff-view/DiffSettingsPopover.tsx index 6485b6a..21d6ec1 100644 --- a/web/src/components/diff-view/DiffSettingsPopover.tsx +++ b/web/src/components/diff-view/DiffSettingsPopover.tsx @@ -22,16 +22,14 @@ import { IconSortDescending, type TablerIcon, } from "@tabler/icons-react"; -import type { ReactNode } from "react"; +import { useState, type ReactNode } from "react"; import { isAppColorScheme } from "@/lib/colorScheme"; -import type { DiffStyle, DiffSettingsProps } from "./types"; +import type { DiffSettingsProps, DiffStyle } from "./types"; import { colorSchemeOptions, diffOrderByOptions, diffThemeOptions, headerIconButtonClass, - isDiffOrderBy, - isDiffThemeId, } from "./helpers"; const settingsPanelClass = @@ -73,16 +71,14 @@ function SelectRow({ onValueChange, options, width, - fallbackLabel, contentClassName, trailing, }: { label: string; value: string; - onValueChange: (value: string | null) => void; + onValueChange: (value: string) => void; options: readonly SelectRowOption[]; width: string; - fallbackLabel?: string; contentClassName?: string; trailing?: ReactNode; }) { @@ -90,12 +86,11 @@ function SelectRow({ ); } @@ -198,31 +190,12 @@ function pullRequestTitle(prUrl: string) { return { repo: prUrl, pullRequest: "" }; } -function pullRequestBranchLabel(info: PullRequestInfo, side: "base" | "head") { - const { label, ref, repo } = - side === "base" - ? { label: info.baseLabel, ref: info.baseRef, repo: info.baseRepo } - : { label: info.headLabel, ref: info.headRef, repo: info.headRepo }; - if (label.trim() !== "") return label.trim(); - if (repo.trim() !== "" && ref.trim() !== "") return `${repo.trim()}:${ref.trim()}`; - return ref.trim(); -} - -function pullRequestStatus(info: PullRequestInfo) { - if (info.merged) return "Merged"; - if (info.state.toLowerCase() === "closed") return "Closed"; - if (info.draft) return "Draft"; - return "Open"; -} - -function pullRequestStatusClass(info: PullRequestInfo) { - const status = pullRequestStatus(info); - if (status === "Open") return "bg-[#1f883d] text-white"; - if (status === "Merged") return "bg-[#8250df] text-white"; - if (status === "Draft") - return "bg-neutral-200 text-neutral-700 dark:bg-neutral-700 dark:text-neutral-200"; - return "bg-[#cf222e] text-white"; -} +const pullRequestStatusClass: Record = { + Open: "bg-[#1f883d] text-white", + Merged: "bg-[#8250df] text-white", + Draft: "bg-neutral-200 text-neutral-700 dark:bg-neutral-700 dark:text-neutral-200", + Closed: "bg-[#cf222e] text-white", +}; function formatPullRequestDate(value: string) { if (value.trim() === "") return ""; @@ -235,7 +208,7 @@ function formatPullRequestDate(value: string) { }).format(date); } -const pullRequestCountFormat = new Intl.NumberFormat(); +const count = new Intl.NumberFormat(); export function DiffToolbar({ allCollapsed, @@ -243,23 +216,17 @@ export function DiffToolbar({ isLocal, baseRef, includeDirty = false, - onSettingsOpenChange, onSidebarToggle, onSubmitPendingComments, onToggleAllCollapsed, onExport, exporting, onMenuOpen, - githubRepoUrl, - githubPrUrl, - prDiffPath, - branchDiffPath, - localDiffPath, + links, pendingCommentCount, pullRequestInfo, prUrl, settings, - settingsOpen, sidebarOpen, submittingPendingComments, }: { @@ -268,40 +235,30 @@ export function DiffToolbar({ isLocal: boolean; baseRef?: string; includeDirty?: boolean; - onSettingsOpenChange: (open: boolean) => void; onSidebarToggle: () => void; onSubmitPendingComments: () => void; onToggleAllCollapsed: () => void; onExport: () => void; exporting: boolean; onMenuOpen: () => void; - githubRepoUrl?: string; - githubPrUrl?: string; - prDiffPath?: string; - branchDiffPath?: string; - localDiffPath?: string; + links: readonly ToolbarLink[]; pendingCommentCount: number; pullRequestInfo: PullRequestInfo | null; prUrl: string; settings: DiffSettingsProps; - settingsOpen: boolean; sidebarOpen: boolean; submittingPendingComments: boolean; }) { const remoteTitle = pullRequestTitle(prUrl); - const baseBranch = pullRequestInfo ? pullRequestBranchLabel(pullRequestInfo, "base") : ""; - const headBranch = pullRequestInfo ? pullRequestBranchLabel(pullRequestInfo, "head") : ""; + const baseBranch = pullRequestInfo?.baseBranch ?? ""; + const headBranch = pullRequestInfo?.headBranch ?? ""; const createdAt = pullRequestInfo ? formatPullRequestDate(pullRequestInfo.createdAt) : ""; const updatedAt = pullRequestInfo ? formatPullRequestDate(pullRequestInfo.updatedAt) : ""; + const submitLabel = `Submit ${pendingCommentCount} pending ${pendingCommentCount === 1 ? "comment" : "comments"}`; return (
- + @@ -344,7 +301,7 @@ export function DiffToolbar({ render={
diff --git a/web/src/components/diff-view/SidebarTree.tsx b/web/src/components/diff-view/SidebarTree.tsx index 3346d44..1e415bb 100644 --- a/web/src/components/diff-view/SidebarTree.tsx +++ b/web/src/components/diff-view/SidebarTree.tsx @@ -14,14 +14,13 @@ import { IconMessageCircle, IconMessageCirclePlus, IconSearch, - IconTrash, IconX, } from "@tabler/icons-react"; import { Button } from "@/components/ui/button"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import type { ResolvedColorScheme } from "@/lib/colorScheme"; import { DIFF_SURFACE_FONT_SIZE } from "@/lib/diffTypography"; -import { CommentAvatar } from "./CommentAvatar"; +import { CommentAvatar, DeleteThreadButton, ThreadBadges } from "./ThreadParts"; import type { ReviewThread } from "./types"; import { latestThreadComment, threadEndLine, threadLineLabel } from "./helpers"; import { DiffStats } from "./DiffStats"; @@ -289,21 +288,7 @@ export function SidebarTree({ {threadLineLabel(thread)} - {thread.status === "resolved" && ( - - Resolved - - )} - {thread.pending && ( - - Pending - - )} - {thread.comments.length > 1 && ( - - {thread.comments.length} - - )} +

{latestComment?.body ?? ""} @@ -311,21 +296,10 @@ export function SidebarTree({ {canDelete && ( - - onDeleteComment(thread)} - > - - - } - /> - Delete comment - + onDeleteComment(thread)} + /> )} ); diff --git a/web/src/components/diff-view/ThreadParts.tsx b/web/src/components/diff-view/ThreadParts.tsx new file mode 100644 index 0000000..9edeca9 --- /dev/null +++ b/web/src/components/diff-view/ThreadParts.tsx @@ -0,0 +1,71 @@ +import { IconTrash } from "@tabler/icons-react"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; +import type { ReviewThread } from "./types"; + +function escapeSvgText(value: string): string { + return value.replaceAll("&", "&").replaceAll("<", "<").replaceAll(">", ">"); +} + +function avatarDataUri(author: string): string { + const label = escapeSvgText(author.trim().slice(0, 1).toUpperCase() || "?"); + const svg = `${label}`; + return `data:image/svg+xml,${encodeURIComponent(svg)}`; +} + +export function CommentAvatar({ author }: { author?: string }) { + const name = author?.trim() || "Commenter"; + + return ( + {name} + ); +} + +const badgeClass = "rounded-full px-1.5 py-0.5 text-[10px]"; +const neutralBadge = `${badgeClass} bg-neutral-200 text-neutral-600 dark:bg-neutral-700 dark:text-neutral-300`; + +export function ThreadBadges({ thread }: { thread: ReviewThread }) { + return ( + <> + {thread.status === "resolved" && Resolved} + {thread.pending && ( + + Pending + + )} + {thread.comments.length > 1 && {thread.comments.length}} + + ); +} + +// `className` supplies placement and the parent's group-hover reveal. +export function DeleteThreadButton({ + onClick, + className, +}: { + onClick: () => void; + className: string; +}) { + return ( + + + + + } + /> + Delete comment + + ); +} diff --git a/web/src/components/diff-view/helpers.ts b/web/src/components/diff-view/helpers.ts index e5e5661..22a8079 100644 --- a/web/src/components/diff-view/helpers.ts +++ b/web/src/components/diff-view/helpers.ts @@ -4,8 +4,6 @@ import type { DiffOrderBy, DiffOrderByOption, DiffOrderDir, - DiffStyle, - DiffThemeId, DiffThemeOption, ColorSchemeOption, ReviewThread, @@ -45,28 +43,12 @@ export const colorSchemeOptions: readonly ColorSchemeOption[] = [ { id: "dark", label: "Dark", icon: IconMoon }, ]; -export function isDiffThemeId(value: unknown): value is DiffThemeId { - return typeof value === "string" && diffThemeOptions.some((option) => option.id === value); -} - -export function isDiffStyle(value: unknown): value is DiffStyle { - return value === "split" || value === "unified"; -} - export const diffOrderByOptions: readonly DiffOrderByOption[] = [ { id: "path", label: "Path" }, { id: "changes", label: "Changes" }, { id: "type", label: "File type" }, ]; -export function isDiffOrderBy(value: unknown): value is DiffOrderBy { - return value === "path" || value === "changes" || value === "type"; -} - -export function isDiffOrderDir(value: unknown): value is DiffOrderDir { - return value === "asc" || value === "desc"; -} - function fileChangeCount(file: FileDiffMetadata): number { let count = 0; for (const hunk of file.hunks) count += hunk.additionLines + hunk.deletionLines; @@ -116,12 +98,6 @@ export function threadEndSide(thread: ReviewThread): "additions" | "deletions" { return thread.endSide ?? thread.side; } -export function threadRangeLabel(thread: ReviewThread): string | null { - const endLine = threadEndLine(thread); - if (endLine === thread.line && threadEndSide(thread) === thread.side) return null; - return `Lines ${thread.line}-${endLine}`; -} - export function threadLineLabel(thread: ReviewThread): string { const sign = thread.side === "additions" ? "+" : "-"; const endLine = threadEndLine(thread); @@ -171,52 +147,23 @@ export function prDiffPathFromUrl(prUrl: string): string | undefined { } } -// Do NOT decode C-style escapes (\NNN, \t, …) in quoted paths — the patch parser leaves them as-is. -function stripDiffPathPrefix(value: string): string { - let path = value; - const tab = path.indexOf("\t"); - if (tab !== -1) path = path.slice(0, tab); - path = path.trim(); - if (path.length >= 2 && path.startsWith('"') && path.endsWith('"')) { - path = path.slice(1, -1); - } - if (path.startsWith("a/") || path.startsWith("b/")) path = path.slice(2); - return path; -} - -function patchTargetPath(block: readonly string[]): string | null { - let fromPath: string | null = null; - for (const line of block) { - if (line.startsWith("+++ ")) { - const path = stripDiffPathPrefix(line.slice(4)); - if (path !== "" && path !== "/dev/null") return path; - } else if (line.startsWith("rename to ")) { - return stripDiffPathPrefix(line.slice(10)); - } else if (line.startsWith("--- ") && fromPath == null) { - const path = stripDiffPathPrefix(line.slice(4)); - if (path !== "" && path !== "/dev/null") fromPath = path; +// Byte-for-byte `diff --git` blocks for copy-diff. parsePatchFiles returns one +// file per block in patch order; on any mismatch return nothing (and say so) +// rather than pair a file with the wrong block. +export function splitPatchByFile( + patch: string | null, + files: readonly FileDiffMetadata[], +): Map { + const blocks = (patch ?? "") + .split(/^(?=diff --git )/m) + .filter((b) => b.startsWith("diff --git ")); + if (blocks.length !== files.length) { + if (files.length > 0) { + console.warn( + `Copy diff unavailable: ${blocks.length} patch blocks vs ${files.length} parsed files`, + ); } + return new Map(); } - if (fromPath != null) return fromPath; - const match = / b\/(.+)$/.exec(block[0]?.slice("diff --git ".length) ?? ""); - return match ? match[1].trim() : null; -} - -// Byte-for-byte slice per diff --git block; preserves exact git output for copy-diff. -export function splitPatchByFile(patch: string | null): Map { - const sections = new Map(); - if (!patch) return sections; - const starts: number[] = []; - let offset = 0; - for (const line of patch.split("\n")) { - if (line.startsWith("diff --git ")) starts.push(offset); - offset += line.length + 1; // + 1 for the "\n" consumed by split - } - for (let i = 0; i < starts.length; i++) { - const end = i + 1 < starts.length ? starts[i + 1] : patch.length; - const section = patch.slice(starts[i], end); - const name = patchTargetPath(section.split("\n")); - if (name != null) sections.set(name, section); - } - return sections; + return new Map(files.map((file, i) => [file.name, blocks[i]])); } diff --git a/web/src/components/diff-view/types.ts b/web/src/components/diff-view/types.ts index 66546b7..5a3f3d3 100644 --- a/web/src/components/diff-view/types.ts +++ b/web/src/components/diff-view/types.ts @@ -1,6 +1,7 @@ import type { DiffsThemeNames, SelectedLineRange, ThemesType, ThemeTypes } from "@pierre/diffs"; import type { TablerIcon } from "@tabler/icons-react"; import type { AppColorScheme } from "@/lib/colorScheme"; +import type { SetSetting, Settings } from "./useSettings"; export type DiffStyle = "split" | "unified"; export type DiffOrderBy = "path" | "changes" | "type"; @@ -57,9 +58,7 @@ export type AppConfig = { export type PullRequestInfo = { title: string; - state: string; - draft: boolean; - merged: boolean; + status: "Open" | "Draft" | "Merged" | "Closed"; author: string; createdAt: string; updatedAt: string; @@ -67,19 +66,8 @@ export type PullRequestInfo = { deletions: number; changedFiles: number; commits: number; - headRef: string; - headLabel: string; - headRepo: string; - baseRef: string; - baseLabel: string; - baseRepo: string; -}; - -export type ReviewComment = { - id: string; - author: string; - body: string; - createdAt: string; + baseBranch: string; + headBranch: string; }; export type PendingCommentDraft = { @@ -101,7 +89,7 @@ export type ReviewThread = { endSide?: "additions" | "deletions"; endLine?: number; status: "open" | "resolved"; - comments: ReviewComment[]; + comments: { id: string; author: string; body: string; createdAt: string }[]; replyToId?: number; url?: string; pending?: boolean; @@ -126,26 +114,9 @@ export type CodeViewLineSelection = { }; export type DiffSettingsProps = { + settings: Settings; + onSettingChange: SetSetting; appColorScheme: AppColorScheme; onColorSchemeChange: (value: AppColorScheme) => void; - diffStyle: DiffStyle; - onDiffStyleToggle: () => void; - orderBy: DiffOrderBy; - orderDir: DiffOrderDir; - onOrderByChange: (value: DiffOrderBy) => void; - onOrderDirToggle: () => void; - diffThemeId: DiffThemeId; - onDiffThemeChange: (value: DiffThemeId) => void; - selectedDiffThemeLabel: string; - showBackground: boolean; - setShowBackground: (value: boolean) => void; - showLineNumbers: boolean; - setShowLineNumbers: (value: boolean) => void; - wordWrap: boolean; - setWordWrap: (value: boolean) => void; - collapseRemovals: boolean; - setCollapseRemovals: (value: boolean) => void; - hideReviewed: boolean; - setHideReviewed: (value: boolean) => void; onShortcutsOpen: () => void; }; diff --git a/web/src/components/diff-view/usePersistentState.ts b/web/src/components/diff-view/usePersistentState.ts deleted file mode 100644 index 25478a0..0000000 --- a/web/src/components/diff-view/usePersistentState.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { useCallback, useState } from "react"; - -// Setter writes via String(value), matching the existing persisted format. -export function usePersistentState( - key: string, - fallback: T, - decode: (raw: string | null) => T | null, -) { - const [value, setValue] = useState(() => decode(localStorage.getItem(key)) ?? fallback); - const set = useCallback( - (next: T) => { - setValue(next); - localStorage.setItem(key, String(next)); - }, - [key], - ); - // setValue (3rd element) updates state without persisting. - return [value, set, setValue] as const; -} - -export function decodeStoredBool(raw: string | null): boolean | null { - if (raw === "true") return true; - if (raw === "false") return false; - return null; -} diff --git a/web/src/components/diff-view/useSettings.ts b/web/src/components/diff-view/useSettings.ts new file mode 100644 index 0000000..4f41786 --- /dev/null +++ b/web/src/components/diff-view/useSettings.ts @@ -0,0 +1,62 @@ +import { useCallback, useMemo, useState } from "react"; +import { diffThemeOptions } from "./helpers"; +import type { AppConfig, DiffOrderBy, DiffOrderDir, DiffStyle } from "./types"; + +// Allowed values per setting; the first is the default. Storage keys predate +// this hook, so keep them to preserve saved preferences. +const SPEC = { + diffStyle: { key: "diffs-diff-style", values: ["split", "unified"] as DiffStyle[] }, + orderBy: { key: "diffs-order-by", values: ["path", "changes", "type"] as DiffOrderBy[] }, + orderDir: { key: "diffs-order-dir", values: ["asc", "desc"] as DiffOrderDir[] }, + diffTheme: { key: "diff-theme", values: diffThemeOptions.map((option) => option.id) }, + lineBackgrounds: { key: "diffs-line-backgrounds", values: [true, false] }, + lineNumbers: { key: "diffs-line-numbers", values: [true, false] }, + wordWrap: { key: "diffs-word-wrap", values: [false, true] }, + collapseRemovals: { key: "diffs-collapse-removals", values: [false, true] }, + hideReviewed: { key: "diffs-hide-reviewed", values: [false, true] }, +}; + +export type Settings = { [K in keyof typeof SPEC]: (typeof SPEC)[K]["values"][number] }; +export type SetSetting = (name: K, value: Settings[K]) => void; + +const NAMES = Object.keys(SPEC) as (keyof Settings)[]; + +// Server-config defaults, read by property so renaming an AppConfig field fails +// to compile here instead of silently dropping the config.toml default. +const CONFIG_DEFAULT: { [K in keyof Settings]?: (config: AppConfig) => unknown } = { + diffStyle: (config) => config.diffStyle, + diffTheme: (config) => config.diffTheme, + lineBackgrounds: (config) => config.lineBackgrounds, + lineNumbers: (config) => config.lineNumbers, + wordWrap: (config) => config.wordWrap, +}; + +// Values are persisted with String(), so compare string forms. +function parse(name: K, raw: unknown): Settings[K] | undefined { + return (SPEC[name].values as Settings[K][]).find((value) => String(value) === String(raw)); +} + +// Resolves each setting as: saved in localStorage, else server config, else default. +export function useSettings(config: AppConfig) { + const [stored, setStored] = useState( + () => + Object.fromEntries( + NAMES.map((name) => [name, parse(name, localStorage.getItem(SPEC[name].key))]), + ) as Partial, + ); + const settings = useMemo( + () => + Object.fromEntries( + NAMES.map((name) => [ + name, + stored[name] ?? parse(name, CONFIG_DEFAULT[name]?.(config)) ?? SPEC[name].values[0], + ]), + ) as Settings, + [stored, config], + ); + const setSetting = useCallback((name, value) => { + localStorage.setItem(SPEC[name].key, String(value)); + setStored((current) => ({ ...current, [name]: value })); + }, []); + return [settings, setSetting] as const; +} diff --git a/web/src/components/ui/drawer.tsx b/web/src/components/ui/drawer.tsx index 715f619..5852742 100644 --- a/web/src/components/ui/drawer.tsx +++ b/web/src/components/ui/drawer.tsx @@ -7,18 +7,10 @@ function Drawer({ ...props }: React.ComponentProps) return ; } -function DrawerTrigger({ ...props }: React.ComponentProps) { - return ; -} - function DrawerPortal({ ...props }: React.ComponentProps) { return ; } -function DrawerClose({ ...props }: React.ComponentProps) { - return ; -} - function DrawerOverlay({ className, ...props @@ -71,16 +63,6 @@ function DrawerHeader({ className, ...props }: React.ComponentProps<"div">) { ); } -function DrawerFooter({ className, ...props }: React.ComponentProps<"div">) { - return ( -

- ); -} - function DrawerTitle({ className, ...props }: React.ComponentProps) { return ( - ); -} - -export { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuSeparator, - DropdownMenuTrigger, -}; +export { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger }; diff --git a/web/src/components/ui/select.tsx b/web/src/components/ui/select.tsx index 779f11a..eedbc1c 100644 --- a/web/src/components/ui/select.tsx +++ b/web/src/components/ui/select.tsx @@ -94,16 +94,6 @@ function SelectContent({ ); } -function SelectLabel({ className, ...props }: SelectPrimitive.GroupLabel.Props) { - return ( - - ); -} - function SelectItem({ className, children, ...props }: SelectPrimitive.Item.Props) { return ( - ); -} - function SelectScrollUpButton({ className, ...props @@ -174,15 +154,4 @@ function SelectScrollDownButton({ ); } -export { - Select, - SelectContent, - SelectGroup, - SelectItem, - SelectLabel, - SelectScrollDownButton, - SelectScrollUpButton, - SelectSeparator, - SelectTrigger, - SelectValue, -}; +export { Select, SelectContent, SelectGroup, SelectItem, SelectTrigger, SelectValue }; diff --git a/web/src/components/ui/toggle.tsx b/web/src/components/ui/toggle.tsx index f61ced5..176b921 100644 --- a/web/src/components/ui/toggle.tsx +++ b/web/src/components/ui/toggle.tsx @@ -1,9 +1,4 @@ -"use client"; - -import { Toggle as TogglePrimitive } from "@base-ui/react/toggle"; -import { cva, type VariantProps } from "class-variance-authority"; - -import { cn } from "@/lib/utils"; +import { cva } from "class-variance-authority"; const toggleVariants = cva( "group/toggle inline-flex items-center justify-center gap-1 rounded-lg text-sm font-medium whitespace-nowrap transition-all outline-none hover:bg-muted hover:text-foreground focus-visible:border-ring focus-visible:ring-[3px] focus-visible:ring-ring/50 disabled:pointer-events-none disabled:opacity-50 aria-invalid:border-destructive aria-invalid:ring-destructive/20 aria-pressed:bg-muted data-[state=on]:bg-muted dark:aria-invalid:ring-destructive/40 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", @@ -27,19 +22,4 @@ const toggleVariants = cva( }, ); -function Toggle({ - className, - variant = "default", - size = "default", - ...props -}: TogglePrimitive.Props & VariantProps) { - return ( - - ); -} - -export { Toggle, toggleVariants }; +export { toggleVariants }; diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index e894d8d..622b6ef 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -8,3 +8,11 @@ export async function apiFetch(path: string, init?: RequestInit): P if (contentType.includes("application/json")) return res.json() as Promise; return res.text() as Promise; } + +export function apiSend(path: string, method: string, body: unknown): Promise { + return apiFetch(path, { + method, + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); +} diff --git a/web/src/lib/colorScheme.ts b/web/src/lib/colorScheme.ts index b8053da..1ca1856 100644 --- a/web/src/lib/colorScheme.ts +++ b/web/src/lib/colorScheme.ts @@ -1,7 +1,7 @@ export type AppColorScheme = "dark" | "light" | "system"; export type ResolvedColorScheme = "dark" | "light"; -export const colorSchemeStorageKey = "diffs-color-scheme"; +const colorSchemeStorageKey = "diffs-color-scheme"; const colorSchemeQuery = typeof window !== "undefined" ? window.matchMedia("(prefers-color-scheme: dark)") : null; diff --git a/web/src/lib/exportHtml.ts b/web/src/lib/exportHtml.ts index 654eeff..f98bcb7 100644 --- a/web/src/lib/exportHtml.ts +++ b/web/src/lib/exportHtml.ts @@ -4,11 +4,13 @@ import jetbrainsMono400Url from "@fontsource/jetbrains-mono/files/jetbrains-mono import jetbrainsMono700Url from "@fontsource/jetbrains-mono/files/jetbrains-mono-latin-700-normal.woff2?url"; import interVariableUrl from "@fontsource-variable/inter/files/inter-latin-wght-normal.woff2?url"; import { DIFF_SURFACE_FONT_SIZE } from "@/lib/diffTypography"; +// Export page shell; mirrors web/src/index.css palette, keep roughly in sync. +import shellCss from "./exportShell.css?raw"; import { DEFAULT_CODE_FONT_FAMILY, DEFAULT_UI_FONT_FAMILY, prependFontFamily } from "@/lib/fonts"; // Subset of @pierre/diffs render options that affect static rasterization; mirrors the // live CodeView `options` (a superset object is accepted). -export interface ExportDiffOptions { +interface ExportDiffOptions { theme: DiffsThemeNames | ThemesType; themeType: ThemeTypes | undefined; diffStyle: "split" | "unified"; @@ -109,58 +111,6 @@ function splitFragment(html: string): SplitFragment { return { styles, sprite, content: content.join("") }; } -// Shell CSS + design tokens; mirrors web/src/index.css palette — keep roughly in sync if that changes. -const SHELL_CSS = ` -:root{ - color-scheme:light; - --bg:#ffffff; - --fg:#171717; - --muted-fg:#737373; - --border:#e5e5e5; - --surface:#fafafa; -} -.dark{ - color-scheme:dark; - --bg:#0a0a0a; - --fg:#fafafa; - --muted-fg:#a3a3a3; - --border:#262626; - --surface:#171717; -} -*{box-sizing:border-box;} -html,body{margin:0;padding:0;} -body{ - background:var(--bg); - color:var(--fg); - font-family:var(--font-sans); - -webkit-font-smoothing:antialiased; -} -.export-header{ - padding:16px 20px; - border-bottom:1px solid var(--border); - background:var(--surface); -} -.export-header h1{ - margin:0; - font-size:15px; - font-weight:600; - line-height:1.3; - word-break:break-word; -} -.export-header p{ - margin:4px 0 0; - font-size:12px; - color:var(--muted-fg); - word-break:break-word; -} -.export-main{ - padding:16px 20px 48px; -} -.diffs-root{ - display:block; -} -`.trim(); - // Mirrors applyConfigFontFamilies; values inherit across the shadow boundary. Note: // --diffs-header-font-family is left unset on purpose to match the live app's fallback. function fontVarsCss(codeFontFamily: string | undefined, uiFontFamily: string | undefined): string { @@ -223,7 +173,7 @@ async function buildDocument(params: ExportDiffParams): Promise { ${escapeHtml(title)} - +

${escapeHtml(title)}

${subtitleHtml}
diff --git a/web/src/lib/exportShell.css b/web/src/lib/exportShell.css new file mode 100644 index 0000000..a127ea1 --- /dev/null +++ b/web/src/lib/exportShell.css @@ -0,0 +1,54 @@ +:root { + color-scheme: light; + --bg: #ffffff; + --fg: #171717; + --muted-fg: #737373; + --border: #e5e5e5; + --surface: #fafafa; +} +.dark { + color-scheme: dark; + --bg: #0a0a0a; + --fg: #fafafa; + --muted-fg: #a3a3a3; + --border: #262626; + --surface: #171717; +} +* { + box-sizing: border-box; +} +html, +body { + margin: 0; + padding: 0; +} +body { + background: var(--bg); + color: var(--fg); + font-family: var(--font-sans); + -webkit-font-smoothing: antialiased; +} +.export-header { + padding: 16px 20px; + border-bottom: 1px solid var(--border); + background: var(--surface); +} +.export-header h1 { + margin: 0; + font-size: 15px; + font-weight: 600; + line-height: 1.3; + word-break: break-word; +} +.export-header p { + margin: 4px 0 0; + font-size: 12px; + color: var(--muted-fg); + word-break: break-word; +} +.export-main { + padding: 16px 20px 48px; +} +.diffs-root { + display: block; +}