mirror of
https://github.com/stablyai/orca
synced 2026-04-21 14:17:16 +00:00
diff-comments: copy-only flow in Source Control (#884)
This commit is contained in:
Brennan Benson
GitHub
parent
bf21a028b9
commit
933d59f165
21 changed files with 1434 additions and 25 deletions
13
.gitignore
vendored
13
.gitignore
vendored
|
|
@ -1,6 +1,19 @@
|
|||
# Build artifacts
|
||||
tsconfig.*.tsbuildinfo
|
||||
|
||||
# TypeScript emit artifacts next to sources (tsc produced these accidentally;
|
||||
# real source lives in .ts/.tsx). Hand-authored declaration files are
|
||||
# re-included below.
|
||||
src/**/*.js
|
||||
src/**/*.d.ts
|
||||
/electron.vite.config.js
|
||||
/electron.vite.config.d.ts
|
||||
!src/main/types/hosted-git-info.d.ts
|
||||
!src/preload/api-types.d.ts
|
||||
!src/preload/index.d.ts
|
||||
!src/renderer/src/env.d.ts
|
||||
!src/renderer/src/mermaid.d.ts
|
||||
|
||||
# Dependencies
|
||||
node_modules/
|
||||
|
||||
|
|
|
|||
|
|
@ -169,7 +169,11 @@ export function mergeWorktree(
|
|||
isUnread: meta?.isUnread ?? false,
|
||||
isPinned: meta?.isPinned ?? false,
|
||||
sortOrder: meta?.sortOrder ?? 0,
|
||||
lastActivityAt: meta?.lastActivityAt ?? 0
|
||||
lastActivityAt: meta?.lastActivityAt ?? 0,
|
||||
// Why: diff comments are persisted on WorktreeMeta (see `WorktreeMeta` in
|
||||
// shared/types) and forwarded verbatim so the renderer store mirrors
|
||||
// on-disk state. `undefined` here means the worktree has no comments yet.
|
||||
diffComments: meta?.diffComments
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
9
src/preload/api-types.d.ts
vendored
9
src/preload/api-types.d.ts
vendored
|
|
@ -269,7 +269,11 @@ export type PreloadApi = {
|
|||
}
|
||||
repos: {
|
||||
list: () => Promise<Repo[]>
|
||||
add: (args: { path: string; kind?: 'git' | 'folder' }) => Promise<Repo>
|
||||
// Why: error union matches the IPC handler's return shape; renderer callers branch on `'error' in result`.
|
||||
add: (args: {
|
||||
path: string
|
||||
kind?: 'git' | 'folder'
|
||||
}) => Promise<{ repo: Repo } | { error: string }>
|
||||
remove: (args: { repoId: string }) => Promise<void>
|
||||
update: (args: {
|
||||
repoId: string
|
||||
|
|
@ -281,12 +285,13 @@ export type PreloadApi = {
|
|||
pickDirectory: () => Promise<string | null>
|
||||
clone: (args: { url: string; destination: string }) => Promise<Repo>
|
||||
cloneAbort: () => Promise<void>
|
||||
// Why: error union matches the IPC handler's return shape; renderer callers branch on `'error' in result`.
|
||||
addRemote: (args: {
|
||||
connectionId: string
|
||||
remotePath: string
|
||||
displayName?: string
|
||||
kind?: 'git' | 'folder'
|
||||
}) => Promise<Repo>
|
||||
}) => Promise<{ repo: Repo } | { error: string }>
|
||||
onCloneProgress: (callback: (data: { phase: string; percent: number }) => void) => () => void
|
||||
getGitUsername: (args: { repoId: string }) => Promise<string>
|
||||
getBaseRefDefault: (args: { repoId: string }) => Promise<string>
|
||||
|
|
|
|||
5
src/preload/index.d.ts
vendored
5
src/preload/index.d.ts
vendored
|
|
@ -104,11 +104,6 @@ type GhApi = {
|
|||
baseSha: string
|
||||
}) => Promise<GitHubPRFileContents>
|
||||
listIssues: (args: { repoPath: string; limit?: number }) => Promise<IssueInfo[]>
|
||||
createIssue: (args: {
|
||||
repoPath: string
|
||||
title: string
|
||||
body: string
|
||||
}) => Promise<{ ok: true; number: number; url: string } | { ok: false; error: string }>
|
||||
listWorkItems: (args: {
|
||||
repoPath: string
|
||||
limit?: number
|
||||
|
|
|
|||
|
|
@ -885,3 +885,158 @@
|
|||
.animate-update-card-exit {
|
||||
animation: update-card-exit 150ms ease-in both;
|
||||
}
|
||||
|
||||
/* ── Diff comment decorations ────────────────────────────────────── */
|
||||
|
||||
/* Why: the "+" button is appended into Monaco's editor DOM node by
|
||||
useDiffCommentDecorator. Keep it visually subtle until hovered so it does
|
||||
not distract from the diff itself. Left position overlaps the glyph margin
|
||||
so the affordance reads as "add comment on this line". */
|
||||
.orca-diff-comment-add-btn {
|
||||
position: absolute;
|
||||
left: 4px;
|
||||
width: 18px;
|
||||
height: 18px;
|
||||
display: none;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
padding: 0;
|
||||
border: 1px solid color-mix(in srgb, var(--border) 60%, transparent);
|
||||
border-radius: 4px;
|
||||
background: var(--background);
|
||||
color: var(--muted-foreground);
|
||||
cursor: pointer;
|
||||
z-index: 5;
|
||||
opacity: 0.7;
|
||||
transition:
|
||||
opacity 100ms ease,
|
||||
color 100ms ease,
|
||||
background-color 100ms ease;
|
||||
}
|
||||
|
||||
.orca-diff-comment-add-btn:hover {
|
||||
opacity: 1;
|
||||
color: var(--primary);
|
||||
background: color-mix(in srgb, var(--primary) 12%, var(--background));
|
||||
}
|
||||
|
||||
.orca-diff-comment-add-btn:focus-visible {
|
||||
outline: 2px solid var(--primary);
|
||||
outline-offset: 1px;
|
||||
}
|
||||
|
||||
.orca-diff-comment-inline {
|
||||
width: 100%;
|
||||
/* Why: match the popover's horizontal inset so the saved card lines up with
|
||||
the new-comment popover that preceded it. Both anchor at `left: 56px` and
|
||||
cap at 420px wide. */
|
||||
padding: 4px 24px 6px 56px;
|
||||
box-sizing: border-box;
|
||||
}
|
||||
|
||||
.orca-diff-comment-inline > .orca-diff-comment-card {
|
||||
max-width: 420px;
|
||||
}
|
||||
|
||||
.orca-diff-comment-card {
|
||||
border: 1px solid color-mix(in srgb, var(--border) 70%, transparent);
|
||||
border-radius: 6px;
|
||||
background: color-mix(in srgb, var(--muted) 40%, var(--background));
|
||||
padding: 6px 8px;
|
||||
font-family: var(--font-sans, system-ui, sans-serif);
|
||||
}
|
||||
|
||||
.orca-diff-comment-header {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: space-between;
|
||||
gap: 8px;
|
||||
margin-bottom: 4px;
|
||||
}
|
||||
|
||||
.orca-diff-comment-meta {
|
||||
font-size: 11px;
|
||||
color: var(--muted-foreground);
|
||||
font-weight: 500;
|
||||
}
|
||||
|
||||
.orca-diff-comment-delete {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
padding: 3px;
|
||||
border-radius: 4px;
|
||||
border: 1px solid transparent;
|
||||
background: transparent;
|
||||
color: var(--muted-foreground);
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
.orca-diff-comment-delete:hover {
|
||||
color: var(--destructive);
|
||||
border-color: color-mix(in srgb, var(--destructive) 40%, transparent);
|
||||
background: color-mix(in srgb, var(--destructive) 10%, transparent);
|
||||
}
|
||||
|
||||
.orca-diff-comment-body {
|
||||
font-size: 12px;
|
||||
line-height: 1.4;
|
||||
color: var(--foreground);
|
||||
white-space: pre-wrap;
|
||||
word-break: break-word;
|
||||
}
|
||||
|
||||
/* Popover for entering a new comment. Positioned absolutely within the
|
||||
section container so it tracks the clicked line via `top` style. Anchored
|
||||
near the content (past the gutter) rather than the far right, so it reads
|
||||
as attached to the line it comments on instead of floating in empty space. */
|
||||
.orca-diff-comment-popover {
|
||||
position: absolute;
|
||||
left: 56px;
|
||||
right: 24px;
|
||||
max-width: 420px;
|
||||
z-index: 1000;
|
||||
padding: 8px;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 6px;
|
||||
background: var(--popover, var(--background));
|
||||
color: var(--popover-foreground, var(--foreground));
|
||||
box-shadow: 0 10px 24px rgba(0, 0, 0, 0.18);
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 6px;
|
||||
}
|
||||
|
||||
.orca-diff-comment-popover-label {
|
||||
font-size: 10px;
|
||||
font-weight: 600;
|
||||
text-transform: uppercase;
|
||||
letter-spacing: 0.04em;
|
||||
color: var(--muted-foreground);
|
||||
}
|
||||
|
||||
.orca-diff-comment-popover-textarea {
|
||||
width: 100%;
|
||||
min-height: 56px;
|
||||
max-height: 240px;
|
||||
resize: none;
|
||||
padding: 6px 8px;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 4px;
|
||||
background: var(--background);
|
||||
color: var(--foreground);
|
||||
font-family: inherit;
|
||||
font-size: 12px;
|
||||
line-height: 1.4;
|
||||
outline: none;
|
||||
}
|
||||
|
||||
.orca-diff-comment-popover-textarea:focus {
|
||||
border-color: color-mix(in srgb, var(--primary) 60%, var(--border));
|
||||
}
|
||||
|
||||
.orca-diff-comment-popover-footer {
|
||||
display: flex;
|
||||
justify-content: flex-end;
|
||||
gap: 6px;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,37 @@
|
|||
import { Trash } from 'lucide-react'
|
||||
|
||||
// Why: the saved-comment card lives inside a Monaco view zone's DOM node.
|
||||
// useDiffCommentDecorator creates a React root per zone and renders this
|
||||
// component into it so we can use normal lucide icons and JSX instead of
|
||||
// hand-built DOM + inline SVG strings.
|
||||
|
||||
type Props = {
|
||||
lineNumber: number
|
||||
body: string
|
||||
onDelete: () => void
|
||||
}
|
||||
|
||||
export function DiffCommentCard({ lineNumber, body, onDelete }: Props): React.JSX.Element {
|
||||
return (
|
||||
<div className="orca-diff-comment-card">
|
||||
<div className="orca-diff-comment-header">
|
||||
<span className="orca-diff-comment-meta">Comment · line {lineNumber}</span>
|
||||
<button
|
||||
type="button"
|
||||
className="orca-diff-comment-delete"
|
||||
title="Delete comment"
|
||||
aria-label="Delete comment"
|
||||
onMouseDown={(ev) => ev.stopPropagation()}
|
||||
onClick={(ev) => {
|
||||
ev.preventDefault()
|
||||
ev.stopPropagation()
|
||||
onDelete()
|
||||
}}
|
||||
>
|
||||
<Trash className="size-3.5" />
|
||||
</button>
|
||||
</div>
|
||||
<div className="orca-diff-comment-body">{body}</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
150
src/renderer/src/components/diff-comments/DiffCommentPopover.tsx
Normal file
150
src/renderer/src/components/diff-comments/DiffCommentPopover.tsx
Normal file
|
|
@ -0,0 +1,150 @@
|
|||
import { useEffect, useId, useRef, useState } from 'react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
|
||||
// Why: rendered as a DOM sibling overlay inside the editor container rather
|
||||
// than as a Monaco content widget because it owns a React textarea with
|
||||
// auto-resize behaviour. Positioning mirrors what useDiffCommentDecorator does
|
||||
// for the "+" button so scroll updates from the parent keep the popover
|
||||
// aligned with its anchor line.
|
||||
|
||||
type Props = {
|
||||
lineNumber: number
|
||||
top: number
|
||||
onCancel: () => void
|
||||
onSubmit: (body: string) => Promise<void>
|
||||
}
|
||||
|
||||
export function DiffCommentPopover({
|
||||
lineNumber,
|
||||
top,
|
||||
onCancel,
|
||||
onSubmit
|
||||
}: Props): React.JSX.Element {
|
||||
const [body, setBody] = useState('')
|
||||
// Why: `submitting` prevents duplicate comment rows when the user
|
||||
// double-clicks the Comment button or hits Cmd/Ctrl+Enter twice before the
|
||||
// IPC round-trip resolves. Iteration 1 made submission async and keeps the
|
||||
// popover open on failure (to preserve the draft); that widened the window
|
||||
// between the first click and `setPopover(null)` during which a second
|
||||
// trigger would call `addDiffComment` again and produce a second row with a
|
||||
// fresh id/createdAt. Tracked in React state (not a ref) so the button can
|
||||
// reflect the in-flight status to the user.
|
||||
const [submitting, setSubmitting] = useState(false)
|
||||
const textareaRef = useRef<HTMLTextAreaElement | null>(null)
|
||||
const popoverRef = useRef<HTMLDivElement | null>(null)
|
||||
// Why: stash onCancel in a ref so the document mousedown listener below can
|
||||
// read the freshest callback without listing `onCancel` in its dependency
|
||||
// array. Parents (DiffSectionItem, DiffViewer) pass a new arrow function on
|
||||
// every render and the popover re-renders frequently (scroll tracking updates
|
||||
// `top`, font zoom, etc.), which would otherwise tear down and re-attach the
|
||||
// document listener on every parent render. Mirrors the pattern in
|
||||
// useDiffCommentDecorator.tsx.
|
||||
const onCancelRef = useRef(onCancel)
|
||||
onCancelRef.current = onCancel
|
||||
// Why: stable id per-instance so multiple popovers (should they ever coexist)
|
||||
// don't collide on aria-labelledby references. Screen readers announce the
|
||||
// "Line N" label as the dialog's accessible name.
|
||||
const labelId = useId()
|
||||
|
||||
useEffect(() => {
|
||||
textareaRef.current?.focus()
|
||||
}, [])
|
||||
|
||||
// Why: Monaco's editor area does not bubble a synthetic React click up to
|
||||
// the popover's onClick. Without a document-level mousedown listener, the
|
||||
// popover has no way to detect clicks outside its own bounds. We keep the
|
||||
// `onMouseDown={ev.stopPropagation()}` on the popover root so that this
|
||||
// listener sees outside-clicks only.
|
||||
useEffect(() => {
|
||||
const onDocumentMouseDown = (ev: MouseEvent): void => {
|
||||
if (!popoverRef.current) {
|
||||
return
|
||||
}
|
||||
if (popoverRef.current.contains(ev.target as Node)) {
|
||||
return
|
||||
}
|
||||
// Why: read the latest onCancel from the ref rather than closing over it
|
||||
// so the listener does not need to be re-registered on every parent
|
||||
// render (see onCancelRef comment above).
|
||||
onCancelRef.current()
|
||||
}
|
||||
document.addEventListener('mousedown', onDocumentMouseDown)
|
||||
return () => {
|
||||
document.removeEventListener('mousedown', onDocumentMouseDown)
|
||||
}
|
||||
}, [])
|
||||
|
||||
const autoResize = (el: HTMLTextAreaElement): void => {
|
||||
el.style.height = 'auto'
|
||||
el.style.height = `${Math.min(el.scrollHeight, 240)}px`
|
||||
}
|
||||
|
||||
const handleSubmit = async (): Promise<void> => {
|
||||
if (submitting) {
|
||||
return
|
||||
}
|
||||
const trimmed = body.trim()
|
||||
if (!trimmed) {
|
||||
return
|
||||
}
|
||||
setSubmitting(true)
|
||||
try {
|
||||
await onSubmit(trimmed)
|
||||
} finally {
|
||||
setSubmitting(false)
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
ref={popoverRef}
|
||||
className="orca-diff-comment-popover"
|
||||
style={{ top: `${top}px` }}
|
||||
role="dialog"
|
||||
aria-modal="true"
|
||||
aria-labelledby={labelId}
|
||||
onMouseDown={(ev) => ev.stopPropagation()}
|
||||
onClick={(ev) => ev.stopPropagation()}
|
||||
>
|
||||
<div id={labelId} className="orca-diff-comment-popover-label">
|
||||
Line {lineNumber}
|
||||
</div>
|
||||
<textarea
|
||||
ref={textareaRef}
|
||||
className="orca-diff-comment-popover-textarea"
|
||||
placeholder="Add comment for the AI"
|
||||
value={body}
|
||||
onChange={(e) => {
|
||||
setBody(e.target.value)
|
||||
autoResize(e.currentTarget)
|
||||
}}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === 'Escape') {
|
||||
e.preventDefault()
|
||||
onCancel()
|
||||
return
|
||||
}
|
||||
if ((e.metaKey || e.ctrlKey) && e.key === 'Enter') {
|
||||
e.preventDefault()
|
||||
// Why: guard against a second Cmd/Ctrl+Enter while an earlier
|
||||
// submit is still awaiting IPC — otherwise it would enqueue a
|
||||
// duplicate addDiffComment call.
|
||||
if (submitting) {
|
||||
return
|
||||
}
|
||||
void handleSubmit()
|
||||
}
|
||||
}}
|
||||
rows={3}
|
||||
/>
|
||||
<div className="orca-diff-comment-popover-footer">
|
||||
<Button variant="ghost" size="sm" onClick={onCancel}>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button size="sm" onClick={handleSubmit} disabled={submitting || body.trim().length === 0}>
|
||||
{submitting ? 'Saving…' : 'Comment'}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
|
@ -0,0 +1,286 @@
|
|||
import { useEffect, useRef } from 'react'
|
||||
import * as monaco from 'monaco-editor'
|
||||
import type { editor as monacoEditor, IDisposable } from 'monaco-editor'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import type { DiffComment } from '../../../../shared/types'
|
||||
import { DiffCommentCard } from './DiffCommentCard'
|
||||
|
||||
// Why: Monaco glyph-margin *decorations* don't expose click events in a way
|
||||
// that lets us show a polished popover anchored to a line. So instead we own a
|
||||
// single absolutely-positioned "+" button inside the editor DOM node, and we
|
||||
// move it to follow the mouse-hovered line. Clicking calls the consumer which
|
||||
// opens a React popover. This keeps all interactive UI as React/DOM rather
|
||||
// than Monaco decorations, and we get pixel-accurate positioning via Monaco's
|
||||
// getTopForLineNumber.
|
||||
|
||||
type DecoratorArgs = {
|
||||
editor: monacoEditor.ICodeEditor | null
|
||||
filePath: string
|
||||
worktreeId: string
|
||||
comments: DiffComment[]
|
||||
onAddCommentClick: (args: { lineNumber: number; top: number }) => void
|
||||
onDeleteComment: (commentId: string) => void
|
||||
}
|
||||
|
||||
type ZoneEntry = {
|
||||
zoneId: string
|
||||
domNode: HTMLElement
|
||||
root: Root
|
||||
lastBody: string
|
||||
}
|
||||
|
||||
export function useDiffCommentDecorator({
|
||||
editor,
|
||||
filePath,
|
||||
worktreeId,
|
||||
comments,
|
||||
onAddCommentClick,
|
||||
onDeleteComment
|
||||
}: DecoratorArgs): void {
|
||||
const hoverLineRef = useRef<number | null>(null)
|
||||
// Why: one React root per view zone. Body updates re-render into the
|
||||
// existing root, so Monaco's zone DOM stays in place and only the card
|
||||
// contents update — matching the diff-based pass that replaced the previous
|
||||
// hand-built DOM implementation.
|
||||
const zonesRef = useRef<Map<string, ZoneEntry>>(new Map())
|
||||
const disposablesRef = useRef<IDisposable[]>([])
|
||||
// Why: stash the consumer callbacks in refs so the decorator effect's
|
||||
// cleanup does not run on every parent render. The parent passes inline
|
||||
// arrow functions; without this, each render would tear down and re-attach
|
||||
// the "+" button and all view zones, producing visible flicker.
|
||||
const onAddCommentClickRef = useRef(onAddCommentClick)
|
||||
const onDeleteCommentRef = useRef(onDeleteComment)
|
||||
onAddCommentClickRef.current = onAddCommentClick
|
||||
onDeleteCommentRef.current = onDeleteComment
|
||||
|
||||
useEffect(() => {
|
||||
if (!editor) {
|
||||
return
|
||||
}
|
||||
|
||||
const editorDomNode = editor.getDomNode()
|
||||
if (!editorDomNode) {
|
||||
return
|
||||
}
|
||||
|
||||
const plus = document.createElement('button')
|
||||
plus.type = 'button'
|
||||
plus.className = 'orca-diff-comment-add-btn'
|
||||
plus.title = 'Add comment for the AI'
|
||||
plus.setAttribute('aria-label', 'Add comment for the AI')
|
||||
plus.innerHTML =
|
||||
'<svg viewBox="0 0 16 16" width="12" height="12" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round"><path d="M8 3v10M3 8h10"/></svg>'
|
||||
plus.style.display = 'none'
|
||||
editorDomNode.appendChild(plus)
|
||||
|
||||
const getLineHeight = (): number => {
|
||||
const h = editor.getOption(monaco.editor.EditorOption.lineHeight)
|
||||
return typeof h === 'number' && h > 0 ? h : 19
|
||||
}
|
||||
|
||||
// Why: cache last-applied style values so positionAtLine skips redundant
|
||||
// DOM writes during mousemove. Monaco's onMouseMove fires at high
|
||||
// frequency, and every style assignment to an element currently under the
|
||||
// cursor can retrigger hover state and cause flicker.
|
||||
let lastTop: number | null = null
|
||||
let lastDisplay: string | null = null
|
||||
|
||||
const setDisplay = (value: string): void => {
|
||||
if (lastDisplay === value) {
|
||||
return
|
||||
}
|
||||
plus.style.display = value
|
||||
lastDisplay = value
|
||||
}
|
||||
|
||||
// Why: keep the button a fixed 18px square (height set in CSS) and
|
||||
// vertically center it within the hovered line's box. Previously the
|
||||
// height tracked the line height, producing a rectangle on editors with
|
||||
// taller line-heights. Centering relative to lineHeight keeps the button
|
||||
// sitting neatly on whatever line the cursor is on.
|
||||
const BUTTON_SIZE = 18
|
||||
const positionAtLine = (lineNumber: number): void => {
|
||||
const lineTop = editor.getTopForLineNumber(lineNumber) - editor.getScrollTop()
|
||||
const top = Math.round(lineTop + (getLineHeight() - BUTTON_SIZE) / 2)
|
||||
if (top !== lastTop) {
|
||||
plus.style.top = `${top}px`
|
||||
lastTop = top
|
||||
}
|
||||
setDisplay('flex')
|
||||
}
|
||||
|
||||
const handleClick = (ev: MouseEvent): void => {
|
||||
ev.preventDefault()
|
||||
ev.stopPropagation()
|
||||
const ln = hoverLineRef.current
|
||||
if (ln == null) {
|
||||
return
|
||||
}
|
||||
const top = editor.getTopForLineNumber(ln) - editor.getScrollTop()
|
||||
onAddCommentClickRef.current({ lineNumber: ln, top })
|
||||
}
|
||||
plus.addEventListener('mousedown', (ev) => ev.stopPropagation())
|
||||
plus.addEventListener('click', handleClick)
|
||||
|
||||
const onMouseMove = editor.onMouseMove((e) => {
|
||||
// Why: Monaco reports null position when the cursor is over overlay DOM
|
||||
// that sits inside the editor — including our own "+" button. Hiding on
|
||||
// null would create a flicker loop: cursor enters button → null → hide
|
||||
// → cursor is now over line text → show → repeat. Keep the button
|
||||
// visible at its last line while the cursor is on it. The onMouseLeave
|
||||
// handler still hides it when the cursor leaves the editor entirely.
|
||||
const srcEvent = e.event?.browserEvent as MouseEvent | undefined
|
||||
if (srcEvent && plus.contains(srcEvent.target as Node)) {
|
||||
return
|
||||
}
|
||||
const ln = e.target.position?.lineNumber ?? null
|
||||
if (ln == null) {
|
||||
setDisplay('none')
|
||||
return
|
||||
}
|
||||
hoverLineRef.current = ln
|
||||
positionAtLine(ln)
|
||||
})
|
||||
// Why: only hide the button on mouse-leave; keep hoverLineRef so that a
|
||||
// click which lands on the button (possible during the brief window after
|
||||
// Monaco's content area reports leave but before the button element does)
|
||||
// still resolves to the last-hovered line instead of silently dropping.
|
||||
const onMouseLeave = editor.onMouseLeave(() => {
|
||||
setDisplay('none')
|
||||
})
|
||||
const onScroll = editor.onDidScrollChange(() => {
|
||||
if (hoverLineRef.current != null) {
|
||||
positionAtLine(hoverLineRef.current)
|
||||
}
|
||||
})
|
||||
|
||||
disposablesRef.current = [onMouseMove, onMouseLeave, onScroll]
|
||||
|
||||
return () => {
|
||||
for (const d of disposablesRef.current) {
|
||||
d.dispose()
|
||||
}
|
||||
disposablesRef.current = []
|
||||
plus.removeEventListener('click', handleClick)
|
||||
plus.remove()
|
||||
// Why: when the editor is swapped or torn down, its view zones go with
|
||||
// it. Unmount the React roots and clear tracking so a subsequent editor
|
||||
// mount starts from a known-empty state rather than trying to remove
|
||||
// stale zone ids from a dead editor. The diff effect below deliberately
|
||||
// has no cleanup so comment-only changes don't cause a full zone
|
||||
// rebuild; this cleanup is the single place we reset zone tracking.
|
||||
for (const entry of zonesRef.current.values()) {
|
||||
entry.root.unmount()
|
||||
}
|
||||
zonesRef.current.clear()
|
||||
}
|
||||
}, [editor])
|
||||
|
||||
useEffect(() => {
|
||||
if (!editor) {
|
||||
return
|
||||
}
|
||||
|
||||
const relevant = comments.filter((c) => c.filePath === filePath && c.worktreeId === worktreeId)
|
||||
const relevantMap = new Map(relevant.map((c) => [c.id, c] as const))
|
||||
|
||||
const zones = zonesRef.current
|
||||
// Why: unmounting a React root inside Monaco's changeViewZones callback
|
||||
// triggers synchronous DOM mutations that Monaco isn't expecting mid-flush
|
||||
// and can race with its zone bookkeeping. Collect roots to unmount, run
|
||||
// the Monaco batch, then unmount afterwards.
|
||||
const rootsToUnmount: Root[] = []
|
||||
|
||||
editor.changeViewZones((accessor) => {
|
||||
// Why: remove only the zones whose comments are gone. Rebuilding all
|
||||
// zones on every change caused flicker and dropped focus/selection in
|
||||
// adjacent UI; a diff-based pass keeps the untouched cards stable.
|
||||
for (const [commentId, entry] of zones) {
|
||||
if (!relevantMap.has(commentId)) {
|
||||
accessor.removeZone(entry.zoneId)
|
||||
rootsToUnmount.push(entry.root)
|
||||
zones.delete(commentId)
|
||||
}
|
||||
}
|
||||
|
||||
// Add zones for newly-added comments.
|
||||
for (const c of relevant) {
|
||||
if (zones.has(c.id)) {
|
||||
continue
|
||||
}
|
||||
const dom = document.createElement('div')
|
||||
dom.className = 'orca-diff-comment-inline'
|
||||
// Why: swallow mousedown on the whole zone so the editor does not
|
||||
// steal focus (or start a selection drag) when the user interacts
|
||||
// with anything inside the card. Delete still fires because click is
|
||||
// attached directly on the button.
|
||||
dom.addEventListener('mousedown', (ev) => ev.stopPropagation())
|
||||
|
||||
const root = createRoot(dom)
|
||||
root.render(
|
||||
<DiffCommentCard
|
||||
lineNumber={c.lineNumber}
|
||||
body={c.body}
|
||||
onDelete={() => onDeleteCommentRef.current(c.id)}
|
||||
/>
|
||||
)
|
||||
|
||||
// Why: estimate height from line count so the zone is close to the
|
||||
// right size on first paint. Monaco sets heightInPx authoritatively at
|
||||
// insertion and does not re-measure the DOM node, so a fixed 72 clipped
|
||||
// multi-line bodies. The per-line estimate handles typical review
|
||||
// notes without needing a post-attach measurement pass.
|
||||
const lineCount = c.body.split('\n').length
|
||||
const heightInPx = Math.max(56, 28 + lineCount * 18)
|
||||
|
||||
// Why: suppressMouseDown: false so clicks inside the zone (Delete
|
||||
// button) reach our DOM listeners. With true, Monaco intercepts the
|
||||
// mousedown and routes it to the editor, so the Delete button never
|
||||
// fires. The delete/body mousedown listeners stopPropagation so the
|
||||
// editor still doesn't steal focus on interaction.
|
||||
const zoneId = accessor.addZone({
|
||||
afterLineNumber: c.lineNumber,
|
||||
heightInPx,
|
||||
domNode: dom,
|
||||
suppressMouseDown: false
|
||||
})
|
||||
zones.set(c.id, { zoneId, domNode: dom, root, lastBody: c.body })
|
||||
}
|
||||
|
||||
// Patch existing zones whose body text changed in place — re-render the
|
||||
// same root with new props instead of removing/re-adding the zone.
|
||||
for (const c of relevant) {
|
||||
const entry = zones.get(c.id)
|
||||
if (!entry) {
|
||||
continue
|
||||
}
|
||||
if (entry.lastBody === c.body) {
|
||||
continue
|
||||
}
|
||||
entry.root.render(
|
||||
<DiffCommentCard
|
||||
lineNumber={c.lineNumber}
|
||||
body={c.body}
|
||||
onDelete={() => onDeleteCommentRef.current(c.id)}
|
||||
/>
|
||||
)
|
||||
entry.lastBody = c.body
|
||||
}
|
||||
})
|
||||
|
||||
// Why: deferred unmount so Monaco has finished its zone batch before we
|
||||
// tear down the React trees that were inside those zones.
|
||||
if (rootsToUnmount.length > 0) {
|
||||
queueMicrotask(() => {
|
||||
for (const root of rootsToUnmount) {
|
||||
root.unmount()
|
||||
}
|
||||
})
|
||||
}
|
||||
// Why: intentionally no cleanup. React would run cleanup BEFORE the next
|
||||
// effect body on every `comments` identity change, wiping all zones and
|
||||
// forcing a full rebuild — exactly the flicker this diff-based pass is
|
||||
// meant to avoid. Zone teardown lives in the editor-scoped effect above,
|
||||
// which only fires when the editor itself is replaced/unmounted.
|
||||
}, [editor, filePath, worktreeId, comments])
|
||||
}
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { lazy, useMemo, type MutableRefObject } from 'react'
|
||||
import React, { lazy, useEffect, useMemo, useState, type MutableRefObject } from 'react'
|
||||
import { LazySection } from './LazySection'
|
||||
import { ChevronDown, ChevronRight, ExternalLink } from 'lucide-react'
|
||||
import { DiffEditor, type DiffOnMount } from '@monaco-editor/react'
|
||||
|
|
@ -7,7 +7,10 @@ import { joinPath } from '@/lib/path'
|
|||
import { detectLanguage } from '@/lib/language-detect'
|
||||
import { useAppStore } from '@/store'
|
||||
import { computeEditorFontSize } from '@/lib/editor-font-zoom'
|
||||
import type { GitDiffResult } from '../../../../shared/types'
|
||||
import { findWorktreeById } from '@/store/slices/worktree-helpers'
|
||||
import { useDiffCommentDecorator } from '../diff-comments/useDiffCommentDecorator'
|
||||
import { DiffCommentPopover } from '../diff-comments/DiffCommentPopover'
|
||||
import type { DiffComment, GitDiffResult } from '../../../../shared/types'
|
||||
|
||||
const ImageDiffViewer = lazy(() => import('./ImageDiffViewer'))
|
||||
|
||||
|
|
@ -107,6 +110,19 @@ export function DiffSectionItem({
|
|||
}): React.JSX.Element {
|
||||
const openFile = useAppStore((s) => s.openFile)
|
||||
const editorFontZoomLevel = useAppStore((s) => s.editorFontZoomLevel)
|
||||
const addDiffComment = useAppStore((s) => s.addDiffComment)
|
||||
const deleteDiffComment = useAppStore((s) => s.deleteDiffComment)
|
||||
// Why: subscribe to the raw comments array on the worktree (reference-
|
||||
// stable across unrelated store updates) and filter by filePath inside a
|
||||
// memo. Selecting a fresh `.filter(...)` result would invalidate on every
|
||||
// store change and cause needless re-renders of this section.
|
||||
const allDiffComments = useAppStore(
|
||||
(s): DiffComment[] | undefined => findWorktreeById(s.worktreesByRepo, worktreeId)?.diffComments
|
||||
)
|
||||
const diffComments = useMemo(
|
||||
() => (allDiffComments ?? []).filter((c) => c.filePath === section.path),
|
||||
[allDiffComments, section.path]
|
||||
)
|
||||
const language = detectLanguage(section.path)
|
||||
const isEditable = section.area === 'unstaged'
|
||||
const editorFontSize = computeEditorFontSize(
|
||||
|
|
@ -114,6 +130,61 @@ export function DiffSectionItem({
|
|||
editorFontZoomLevel
|
||||
)
|
||||
|
||||
const [modifiedEditor, setModifiedEditor] = useState<monacoEditor.ICodeEditor | null>(null)
|
||||
const [popover, setPopover] = useState<{ lineNumber: number; top: number } | null>(null)
|
||||
|
||||
useDiffCommentDecorator({
|
||||
editor: modifiedEditor,
|
||||
filePath: section.path,
|
||||
worktreeId,
|
||||
comments: diffComments,
|
||||
onAddCommentClick: ({ lineNumber, top }) => setPopover({ lineNumber, top }),
|
||||
onDeleteComment: (id) => void deleteDiffComment(worktreeId, id)
|
||||
})
|
||||
|
||||
useEffect(() => {
|
||||
if (!modifiedEditor || !popover) {
|
||||
return
|
||||
}
|
||||
const update = (): void => {
|
||||
const top =
|
||||
modifiedEditor.getTopForLineNumber(popover.lineNumber) - modifiedEditor.getScrollTop()
|
||||
setPopover((prev) => (prev ? { ...prev, top } : prev))
|
||||
}
|
||||
const scrollSub = modifiedEditor.onDidScrollChange(update)
|
||||
const contentSub = modifiedEditor.onDidContentSizeChange(update)
|
||||
return () => {
|
||||
scrollSub.dispose()
|
||||
contentSub.dispose()
|
||||
}
|
||||
// Why: depend on popover.lineNumber (not the whole popover object) so the
|
||||
// effect doesn't re-subscribe on every top update it dispatches. The guard
|
||||
// on `popover` above handles the popover-closed case.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [modifiedEditor, popover?.lineNumber])
|
||||
|
||||
const handleSubmitComment = async (body: string): Promise<void> => {
|
||||
if (!popover) {
|
||||
return
|
||||
}
|
||||
// Why: await persistence before closing the popover. If addDiffComment
|
||||
// resolves to null, the store rolled back the optimistic insert; keeping
|
||||
// the popover open preserves the user's draft so they can retry instead
|
||||
// of silently losing their text.
|
||||
const result = await addDiffComment({
|
||||
worktreeId,
|
||||
filePath: section.path,
|
||||
lineNumber: popover.lineNumber,
|
||||
body,
|
||||
side: 'modified'
|
||||
})
|
||||
if (result) {
|
||||
setPopover(null)
|
||||
} else {
|
||||
console.error('Failed to add diff comment — draft preserved')
|
||||
}
|
||||
}
|
||||
|
||||
const lineStats = useMemo(
|
||||
() =>
|
||||
section.loading
|
||||
|
|
@ -135,7 +206,7 @@ export function DiffSectionItem({
|
|||
}
|
||||
|
||||
const handleMount: DiffOnMount = (editor, monaco) => {
|
||||
const modifiedEditor = editor.getModifiedEditor()
|
||||
const modified = editor.getModifiedEditor()
|
||||
|
||||
const updateHeight = (): void => {
|
||||
const contentHeight = editor.getModifiedEditor().getContentHeight()
|
||||
|
|
@ -146,19 +217,30 @@ export function DiffSectionItem({
|
|||
return { ...prev, [index]: contentHeight }
|
||||
})
|
||||
}
|
||||
modifiedEditor.onDidContentSizeChange(updateHeight)
|
||||
modified.onDidContentSizeChange(updateHeight)
|
||||
updateHeight()
|
||||
|
||||
setModifiedEditor(modified)
|
||||
// Why: Monaco disposes inner editors when the DiffEditor container is
|
||||
// unmounted (e.g. section collapse, tab change). Clearing the state
|
||||
// prevents decorator effects and scroll subscriptions from invoking
|
||||
// methods on a disposed editor instance, and avoids `popover` pointing
|
||||
// at a line in an editor that no longer exists.
|
||||
modified.onDidDispose(() => {
|
||||
setModifiedEditor(null)
|
||||
setPopover(null)
|
||||
})
|
||||
|
||||
if (!isEditable) {
|
||||
return
|
||||
}
|
||||
|
||||
modifiedEditorsRef.current.set(index, modifiedEditor)
|
||||
modifiedEditor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () =>
|
||||
modifiedEditorsRef.current.set(index, modified)
|
||||
modified.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () =>
|
||||
handleSectionSaveRef.current(index)
|
||||
)
|
||||
modifiedEditor.onDidChangeModelContent(() => {
|
||||
const current = modifiedEditor.getValue()
|
||||
modified.onDidChangeModelContent(() => {
|
||||
const current = modified.getValue()
|
||||
setSections((prev) =>
|
||||
prev.map((s, i) => (i === index ? { ...s, dirty: current !== s.modifiedContent } : s))
|
||||
)
|
||||
|
|
@ -233,6 +315,7 @@ export function DiffSectionItem({
|
|||
|
||||
{!section.collapsed && (
|
||||
<div
|
||||
className="relative"
|
||||
style={{
|
||||
height: sectionHeight
|
||||
? sectionHeight + 19
|
||||
|
|
@ -247,6 +330,18 @@ export function DiffSectionItem({
|
|||
)
|
||||
}}
|
||||
>
|
||||
{popover && (
|
||||
// Why: key by lineNumber so the popover remounts when the anchor
|
||||
// line changes, resetting the internal draft body and textarea
|
||||
// focus per anchor line instead of leaking state across lines.
|
||||
<DiffCommentPopover
|
||||
key={popover.lineNumber}
|
||||
lineNumber={popover.lineNumber}
|
||||
top={popover.top}
|
||||
onCancel={() => setPopover(null)}
|
||||
onSubmit={handleSubmitComment}
|
||||
/>
|
||||
)}
|
||||
{section.loading ? (
|
||||
<div className="flex items-center justify-center h-full text-muted-foreground text-xs">
|
||||
Loading...
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useCallback, useLayoutEffect, useRef } from 'react'
|
||||
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'
|
||||
import { DiffEditor, type DiffOnMount } from '@monaco-editor/react'
|
||||
import type { editor } from 'monaco-editor'
|
||||
import { useAppStore } from '@/store'
|
||||
|
|
@ -6,6 +6,10 @@ import { diffViewStateCache, setWithLRU } from '@/lib/scroll-cache'
|
|||
import '@/lib/monaco-setup'
|
||||
import { computeEditorFontSize } from '@/lib/editor-font-zoom'
|
||||
import { useContextualCopySetup } from './useContextualCopySetup'
|
||||
import { findWorktreeById } from '@/store/slices/worktree-helpers'
|
||||
import { useDiffCommentDecorator } from '../diff-comments/useDiffCommentDecorator'
|
||||
import { DiffCommentPopover } from '../diff-comments/DiffCommentPopover'
|
||||
import type { DiffComment } from '../../../../shared/types'
|
||||
|
||||
type DiffViewerProps = {
|
||||
modelKey: string
|
||||
|
|
@ -16,6 +20,10 @@ type DiffViewerProps = {
|
|||
relativePath: string
|
||||
sideBySide: boolean
|
||||
editable?: boolean
|
||||
// Why: optional because DiffViewer is also used by GitHubItemDrawer for PR
|
||||
// review, where there is no local worktree to attach comments to. When
|
||||
// omitted, the per-line comment decorator is skipped.
|
||||
worktreeId?: string
|
||||
onContentChange?: (content: string) => void
|
||||
onSave?: (content: string) => void
|
||||
}
|
||||
|
|
@ -29,11 +37,24 @@ export default function DiffViewer({
|
|||
relativePath,
|
||||
sideBySide,
|
||||
editable,
|
||||
worktreeId,
|
||||
onContentChange,
|
||||
onSave
|
||||
}: DiffViewerProps): React.JSX.Element {
|
||||
const settings = useAppStore((s) => s.settings)
|
||||
const editorFontZoomLevel = useAppStore((s) => s.editorFontZoomLevel)
|
||||
const addDiffComment = useAppStore((s) => s.addDiffComment)
|
||||
const deleteDiffComment = useAppStore((s) => s.deleteDiffComment)
|
||||
// Why: subscribe to the raw comments array on the worktree so selector
|
||||
// identity only changes when diffComments actually changes on this worktree.
|
||||
// Filtering by relativePath happens in a memo below.
|
||||
const allDiffComments = useAppStore((s): DiffComment[] | undefined =>
|
||||
worktreeId ? findWorktreeById(s.worktreesByRepo, worktreeId)?.diffComments : undefined
|
||||
)
|
||||
const diffComments = useMemo(
|
||||
() => (allDiffComments ?? []).filter((c) => c.filePath === relativePath),
|
||||
[allDiffComments, relativePath]
|
||||
)
|
||||
const editorFontSize = computeEditorFontSize(
|
||||
settings?.terminalFontSize ?? 13,
|
||||
editorFontZoomLevel
|
||||
|
|
@ -43,6 +64,66 @@ export default function DiffViewer({
|
|||
(settings?.theme === 'system' && window.matchMedia('(prefers-color-scheme: dark)').matches)
|
||||
|
||||
const diffEditorRef = useRef<editor.IStandaloneDiffEditor | null>(null)
|
||||
const [modifiedEditor, setModifiedEditor] = useState<editor.ICodeEditor | null>(null)
|
||||
const [popover, setPopover] = useState<{ lineNumber: number; top: number } | null>(null)
|
||||
|
||||
// Why: gate the decorator on having a worktreeId. DiffViewer is reused by
|
||||
// GitHubItemDrawer (PR review) where there is no local worktree to own the
|
||||
// comment. Pass a nulled editor so the hook no-ops rather than calling it
|
||||
// conditionally, which would violate the rules of hooks.
|
||||
useDiffCommentDecorator({
|
||||
editor: worktreeId ? modifiedEditor : null,
|
||||
filePath: relativePath,
|
||||
worktreeId: worktreeId ?? '',
|
||||
comments: diffComments,
|
||||
onAddCommentClick: ({ lineNumber, top }) => setPopover({ lineNumber, top }),
|
||||
onDeleteComment: (id) => {
|
||||
if (worktreeId) {
|
||||
void deleteDiffComment(worktreeId, id)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
useEffect(() => {
|
||||
if (!modifiedEditor || !popover) {
|
||||
return
|
||||
}
|
||||
const update = (): void => {
|
||||
const top =
|
||||
modifiedEditor.getTopForLineNumber(popover.lineNumber) - modifiedEditor.getScrollTop()
|
||||
setPopover((prev) => (prev ? { ...prev, top } : prev))
|
||||
}
|
||||
const scrollSub = modifiedEditor.onDidScrollChange(update)
|
||||
const contentSub = modifiedEditor.onDidContentSizeChange(update)
|
||||
return () => {
|
||||
scrollSub.dispose()
|
||||
contentSub.dispose()
|
||||
}
|
||||
// Why: depend on popover.lineNumber (not the whole popover object) so the
|
||||
// effect doesn't re-subscribe on every top update it dispatches.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [modifiedEditor, popover?.lineNumber])
|
||||
|
||||
const handleSubmitComment = async (body: string): Promise<void> => {
|
||||
if (!popover || !worktreeId) {
|
||||
return
|
||||
}
|
||||
// Why: await persistence before closing — if addDiffComment resolves null
|
||||
// (store rolled back after IPC failure), keep the popover open so the user
|
||||
// can retry instead of silently losing their draft.
|
||||
const result = await addDiffComment({
|
||||
worktreeId,
|
||||
filePath: relativePath,
|
||||
lineNumber: popover.lineNumber,
|
||||
body,
|
||||
side: 'modified'
|
||||
})
|
||||
if (result) {
|
||||
setPopover(null)
|
||||
} else {
|
||||
console.error('Failed to add diff comment — draft preserved')
|
||||
}
|
||||
}
|
||||
|
||||
// Keep refs to latest callbacks so the mounted editor always calls current versions
|
||||
const onSaveRef = useRef(onSave)
|
||||
|
|
@ -64,6 +145,7 @@ export default function DiffViewer({
|
|||
|
||||
setupCopy(originalEditor, monaco, filePath, propsRef)
|
||||
setupCopy(modifiedEditor, monaco, filePath, propsRef)
|
||||
setModifiedEditor(modifiedEditor)
|
||||
|
||||
// Why: restoring the full diff view state matches VS Code more closely
|
||||
// than replaying scrollTop alone, and avoids divergent cursor/selection
|
||||
|
|
@ -109,7 +191,16 @@ export default function DiffViewer({
|
|||
|
||||
return (
|
||||
<div className="flex flex-col flex-1 min-h-0">
|
||||
<div className="flex-1 min-h-0">
|
||||
<div className="flex-1 min-h-0 relative">
|
||||
{popover && worktreeId && (
|
||||
<DiffCommentPopover
|
||||
key={popover.lineNumber}
|
||||
lineNumber={popover.lineNumber}
|
||||
top={popover.top}
|
||||
onCancel={() => setPopover(null)}
|
||||
onSubmit={handleSubmitComment}
|
||||
/>
|
||||
)}
|
||||
<DiffEditor
|
||||
height="100%"
|
||||
language={language}
|
||||
|
|
|
|||
|
|
@ -355,6 +355,7 @@ export function EditorContent({
|
|||
relativePath={activeFile.relativePath}
|
||||
sideBySide={sideBySide}
|
||||
editable={isEditable}
|
||||
worktreeId={activeFile.worktreeId}
|
||||
onContentChange={isEditable ? handleContentChange : undefined}
|
||||
onSave={isEditable ? handleSave : undefined}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -12,9 +12,13 @@ import {
|
|||
FilePlus,
|
||||
FileQuestion,
|
||||
ArrowRightLeft,
|
||||
Check,
|
||||
Copy,
|
||||
FolderOpen,
|
||||
GitMerge,
|
||||
GitPullRequestArrow,
|
||||
MessageSquare,
|
||||
Trash,
|
||||
TriangleAlert,
|
||||
CircleCheck,
|
||||
Search,
|
||||
|
|
@ -43,6 +47,7 @@ import {
|
|||
DialogTitle
|
||||
} from '@/components/ui/dialog'
|
||||
import { BaseRefPicker } from '@/components/settings/BaseRefPicker'
|
||||
import { formatDiffComment, formatDiffComments } from '@/lib/diff-comments-format'
|
||||
import {
|
||||
notifyEditorExternalFileChange,
|
||||
requestEditorSaveQuiesce
|
||||
|
|
@ -50,6 +55,7 @@ import {
|
|||
import { getConnectionId } from '@/lib/connection-context'
|
||||
import { PullRequestIcon } from './checks-helpers'
|
||||
import type {
|
||||
DiffComment,
|
||||
GitBranchChangeEntry,
|
||||
GitBranchCompareSummary,
|
||||
GitConflictKind,
|
||||
|
|
@ -119,6 +125,50 @@ function SourceControlInner(): React.JSX.Element {
|
|||
const openBranchDiff = useAppStore((s) => s.openBranchDiff)
|
||||
const openAllDiffs = useAppStore((s) => s.openAllDiffs)
|
||||
const openBranchAllDiffs = useAppStore((s) => s.openBranchAllDiffs)
|
||||
const deleteDiffComment = useAppStore((s) => s.deleteDiffComment)
|
||||
// Why: pass activeWorktreeId directly (even when null/undefined) so the
|
||||
// slice's getDiffComments returns its stable EMPTY_COMMENTS sentinel. An
|
||||
// inline `[]` fallback would allocate a new array each store update, break
|
||||
// Zustand's Object.is equality, and cause this component plus the
|
||||
// diffCommentCountByPath memo to churn on every unrelated store change.
|
||||
const diffCommentsForActive = useAppStore((s) => s.getDiffComments(activeWorktreeId))
|
||||
const diffCommentCount = diffCommentsForActive.length
|
||||
// Why: per-file counts are fed into each UncommittedEntryRow so a comment
|
||||
// badge can appear next to the status letter. Compute once per render so
|
||||
// rows don't each re-filter the full list.
|
||||
const diffCommentCountByPath = useMemo(() => {
|
||||
const map = new Map<string, number>()
|
||||
for (const c of diffCommentsForActive) {
|
||||
map.set(c.filePath, (map.get(c.filePath) ?? 0) + 1)
|
||||
}
|
||||
return map
|
||||
}, [diffCommentsForActive])
|
||||
const [diffCommentsExpanded, setDiffCommentsExpanded] = useState(false)
|
||||
const [diffCommentsCopied, setDiffCommentsCopied] = useState(false)
|
||||
|
||||
const handleCopyDiffComments = useCallback(async (): Promise<void> => {
|
||||
if (diffCommentsForActive.length === 0) {
|
||||
return
|
||||
}
|
||||
const text = formatDiffComments(diffCommentsForActive)
|
||||
try {
|
||||
await window.api.ui.writeClipboardText(text)
|
||||
setDiffCommentsCopied(true)
|
||||
} catch {
|
||||
// Why: swallow — clipboard write can fail when the window isn't focused.
|
||||
// No dedicated error surface is warranted for a best-effort copy action.
|
||||
}
|
||||
}, [diffCommentsForActive])
|
||||
|
||||
// Why: auto-dismiss the "copied" indicator so the button returns to its
|
||||
// default icon after a brief confirmation window.
|
||||
useEffect(() => {
|
||||
if (!diffCommentsCopied) {
|
||||
return
|
||||
}
|
||||
const handle = window.setTimeout(() => setDiffCommentsCopied(false), 1500)
|
||||
return () => window.clearTimeout(handle)
|
||||
}, [diffCommentsCopied])
|
||||
|
||||
const [scope, setScope] = useState<SourceControlScope>('all')
|
||||
const [collapsedSections, setCollapsedSections] = useState<Set<string>>(new Set())
|
||||
|
|
@ -645,6 +695,67 @@ function SourceControlInner(): React.JSX.Element {
|
|||
</div>
|
||||
)}
|
||||
|
||||
{/* Why: Diff-comments live on the worktree and apply across every diff
|
||||
view the user opens. The header row expands inline to show per-file
|
||||
comment previews plus a Copy-all action so the user can hand the
|
||||
set off to whichever tool they want without leaving the sidebar. */}
|
||||
{activeWorktreeId && worktreePath && (
|
||||
<div className="border-b border-border">
|
||||
<div className="flex items-center gap-1 pl-3 pr-2 py-1.5">
|
||||
<button
|
||||
type="button"
|
||||
className="flex min-w-0 flex-1 items-center gap-1.5 text-left text-xs text-muted-foreground hover:text-foreground transition-colors"
|
||||
onClick={() => setDiffCommentsExpanded((prev) => !prev)}
|
||||
aria-expanded={diffCommentsExpanded}
|
||||
title={diffCommentsExpanded ? 'Collapse comments' : 'Expand comments'}
|
||||
>
|
||||
<ChevronDown
|
||||
className={cn(
|
||||
'size-3 shrink-0 transition-transform',
|
||||
!diffCommentsExpanded && '-rotate-90'
|
||||
)}
|
||||
/>
|
||||
<MessageSquare className="size-3.5 shrink-0" />
|
||||
<span>Comments</span>
|
||||
{diffCommentCount > 0 && (
|
||||
<span className="text-[11px] leading-none text-muted-foreground tabular-nums">
|
||||
{diffCommentCount}
|
||||
</span>
|
||||
)}
|
||||
</button>
|
||||
{diffCommentCount > 0 && (
|
||||
<TooltipProvider delayDuration={400}>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<button
|
||||
type="button"
|
||||
className="inline-flex size-6 items-center justify-center rounded text-muted-foreground transition-colors hover:bg-accent hover:text-foreground"
|
||||
onClick={() => void handleCopyDiffComments()}
|
||||
aria-label="Copy all comments to clipboard"
|
||||
>
|
||||
{diffCommentsCopied ? (
|
||||
<Check className="size-3.5" />
|
||||
) : (
|
||||
<Copy className="size-3.5" />
|
||||
)}
|
||||
</button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={6}>
|
||||
Copy all comments
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</TooltipProvider>
|
||||
)}
|
||||
</div>
|
||||
{diffCommentsExpanded && (
|
||||
<DiffCommentsInlineList
|
||||
comments={diffCommentsForActive}
|
||||
onDelete={(id) => void deleteDiffComment(activeWorktreeId, id)}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Filter input for searching changed files across all sections */}
|
||||
<div className="flex items-center gap-1.5 border-b border-border px-3 py-1.5">
|
||||
<Search className="size-3.5 shrink-0 text-muted-foreground" />
|
||||
|
|
@ -796,6 +907,7 @@ function SourceControlInner(): React.JSX.Element {
|
|||
onStage={handleStage}
|
||||
onUnstage={handleUnstage}
|
||||
onDiscard={handleDiscard}
|
||||
commentCount={diffCommentCountByPath.get(entry.path) ?? 0}
|
||||
/>
|
||||
)
|
||||
})}
|
||||
|
|
@ -849,6 +961,7 @@ function SourceControlInner(): React.JSX.Element {
|
|||
worktreePath={worktreePath}
|
||||
onRevealInExplorer={revealInExplorer}
|
||||
onOpen={() => openCommittedDiff(entry)}
|
||||
commentCount={diffCommentCountByPath.get(entry.path) ?? 0}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
|
|
@ -1052,6 +1165,108 @@ function SectionHeader({
|
|||
)
|
||||
}
|
||||
|
||||
function DiffCommentsInlineList({
|
||||
comments,
|
||||
onDelete
|
||||
}: {
|
||||
comments: DiffComment[]
|
||||
onDelete: (commentId: string) => void
|
||||
}): React.JSX.Element {
|
||||
// Why: group by filePath so the inline list mirrors the structure in the
|
||||
// Comments tab — a compact section per file with line-number prefixes.
|
||||
const groups = useMemo(() => {
|
||||
const map = new Map<string, DiffComment[]>()
|
||||
for (const c of comments) {
|
||||
const list = map.get(c.filePath) ?? []
|
||||
list.push(c)
|
||||
map.set(c.filePath, list)
|
||||
}
|
||||
for (const list of map.values()) {
|
||||
list.sort((a, b) => a.lineNumber - b.lineNumber)
|
||||
}
|
||||
return Array.from(map.entries())
|
||||
}, [comments])
|
||||
|
||||
const [copiedId, setCopiedId] = useState<string | null>(null)
|
||||
|
||||
// Why: auto-dismiss the per-row "copied" indicator so the button returns to
|
||||
// its default icon after a brief confirmation window. Matches the top-level
|
||||
// Copy button's behavior.
|
||||
useEffect(() => {
|
||||
if (!copiedId) {
|
||||
return
|
||||
}
|
||||
const handle = window.setTimeout(() => setCopiedId(null), 1500)
|
||||
return () => window.clearTimeout(handle)
|
||||
}, [copiedId])
|
||||
|
||||
const handleCopyOne = useCallback(async (c: DiffComment): Promise<void> => {
|
||||
try {
|
||||
await window.api.ui.writeClipboardText(formatDiffComment(c))
|
||||
setCopiedId(c.id)
|
||||
} catch {
|
||||
// Why: swallow — clipboard write can fail when the window isn't focused.
|
||||
}
|
||||
}, [])
|
||||
|
||||
if (comments.length === 0) {
|
||||
return (
|
||||
<div className="px-6 py-2 text-[11px] text-muted-foreground">
|
||||
Hover over a line in the diff view and click the + to add a comment.
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="bg-muted/20">
|
||||
{groups.map(([filePath, list]) => (
|
||||
<div key={filePath} className="px-3 py-1.5">
|
||||
<div className="truncate text-[10px] font-medium text-muted-foreground">{filePath}</div>
|
||||
<ul className="mt-1 space-y-1">
|
||||
{list.map((c) => (
|
||||
<li
|
||||
key={c.id}
|
||||
className="group flex items-center gap-1.5 rounded px-1 py-0.5 hover:bg-accent/40"
|
||||
>
|
||||
<span className="shrink-0 rounded bg-muted px-1 py-0.5 text-[10px] leading-none tabular-nums text-muted-foreground">
|
||||
L{c.lineNumber}
|
||||
</span>
|
||||
<div className="min-w-0 flex-1 whitespace-pre-wrap break-words text-[11px] leading-snug text-foreground">
|
||||
{c.body}
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
className="shrink-0 rounded p-0.5 text-muted-foreground opacity-0 transition-opacity hover:text-foreground group-hover:opacity-100"
|
||||
onClick={(ev) => {
|
||||
ev.stopPropagation()
|
||||
void handleCopyOne(c)
|
||||
}}
|
||||
title="Copy comment"
|
||||
aria-label={`Copy comment on line ${c.lineNumber}`}
|
||||
>
|
||||
{copiedId === c.id ? <Check className="size-3" /> : <Copy className="size-3" />}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
className="shrink-0 rounded p-0.5 text-muted-foreground opacity-0 transition-opacity hover:text-destructive group-hover:opacity-100"
|
||||
onClick={(ev) => {
|
||||
ev.stopPropagation()
|
||||
onDelete(c.id)
|
||||
}}
|
||||
title="Delete comment"
|
||||
aria-label={`Delete comment on line ${c.lineNumber}`}
|
||||
>
|
||||
<Trash className="size-3" />
|
||||
</button>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function ConflictSummaryCard({
|
||||
conflictOperation,
|
||||
unresolvedCount,
|
||||
|
|
@ -1143,7 +1358,8 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({
|
|||
onOpen,
|
||||
onStage,
|
||||
onUnstage,
|
||||
onDiscard
|
||||
onDiscard,
|
||||
commentCount
|
||||
}: {
|
||||
entryKey: string
|
||||
entry: GitStatusEntry
|
||||
|
|
@ -1157,6 +1373,7 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({
|
|||
onStage: (filePath: string) => Promise<void>
|
||||
onUnstage: (filePath: string) => Promise<void>
|
||||
onDiscard: (filePath: string) => Promise<void>
|
||||
commentCount: number
|
||||
}): React.JSX.Element {
|
||||
const StatusIcon = STATUS_ICONS[entry.status] ?? FileQuestion
|
||||
const fileName = basename(entry.path)
|
||||
|
|
@ -1229,6 +1446,18 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({
|
|||
<div className="truncate text-[11px] text-muted-foreground">{conflictLabel}</div>
|
||||
)}
|
||||
</div>
|
||||
{commentCount > 0 && (
|
||||
// Why: show a small comment marker on any row that has diff comments
|
||||
// so the user can tell at a glance which files have review notes
|
||||
// attached, without opening the Comments tab.
|
||||
<span
|
||||
className="flex shrink-0 items-center gap-0.5 text-[10px] text-muted-foreground"
|
||||
title={`${commentCount} comment${commentCount === 1 ? '' : 's'}`}
|
||||
>
|
||||
<MessageSquare className="size-3" />
|
||||
<span className="tabular-nums">{commentCount}</span>
|
||||
</span>
|
||||
)}
|
||||
{entry.conflictStatus ? (
|
||||
<ConflictBadge entry={entry} />
|
||||
) : (
|
||||
|
|
@ -1317,13 +1546,15 @@ function BranchEntryRow({
|
|||
currentWorktreeId,
|
||||
worktreePath,
|
||||
onRevealInExplorer,
|
||||
onOpen
|
||||
onOpen,
|
||||
commentCount
|
||||
}: {
|
||||
entry: GitBranchChangeEntry
|
||||
currentWorktreeId: string
|
||||
worktreePath: string
|
||||
onRevealInExplorer: (worktreeId: string, absolutePath: string) => void
|
||||
onOpen: () => void
|
||||
commentCount: number
|
||||
}): React.JSX.Element {
|
||||
const StatusIcon = STATUS_ICONS[entry.status] ?? FileQuestion
|
||||
const fileName = basename(entry.path)
|
||||
|
|
@ -1351,6 +1582,15 @@ function BranchEntryRow({
|
|||
<span className="text-foreground">{fileName}</span>
|
||||
{dirPath && <span className="ml-1.5 text-[11px] text-muted-foreground">{dirPath}</span>}
|
||||
</span>
|
||||
{commentCount > 0 && (
|
||||
<span
|
||||
className="flex shrink-0 items-center gap-0.5 text-[10px] text-muted-foreground"
|
||||
title={`${commentCount} comment${commentCount === 1 ? '' : 's'}`}
|
||||
>
|
||||
<MessageSquare className="size-3" />
|
||||
<span className="tabular-nums">{commentCount}</span>
|
||||
</span>
|
||||
)}
|
||||
<span
|
||||
className="w-4 shrink-0 text-center text-[10px] font-bold"
|
||||
style={{ color: STATUS_COLORS[entry.status] }}
|
||||
|
|
|
|||
65
src/renderer/src/lib/diff-comments-format.test.ts
Normal file
65
src/renderer/src/lib/diff-comments-format.test.ts
Normal file
|
|
@ -0,0 +1,65 @@
|
|||
import { describe, it, expect } from 'vitest'
|
||||
import type { DiffComment } from '../../../shared/types'
|
||||
import { formatDiffComment, formatDiffComments } from './diff-comments-format'
|
||||
|
||||
function makeComment(overrides: Partial<DiffComment> = {}): DiffComment {
|
||||
return {
|
||||
id: 'id-1',
|
||||
worktreeId: 'wt-1',
|
||||
filePath: 'src/app.ts',
|
||||
lineNumber: 10,
|
||||
body: 'Needs validation',
|
||||
createdAt: 0,
|
||||
side: 'modified',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('formatDiffComment', () => {
|
||||
it('emits the fixed three-line structure', () => {
|
||||
const out = formatDiffComment(makeComment())
|
||||
expect(out).toBe(
|
||||
['File: src/app.ts', 'Line: 10', 'User comment: "Needs validation"'].join('\n')
|
||||
)
|
||||
})
|
||||
|
||||
it('escapes embedded quotes in the body', () => {
|
||||
const out = formatDiffComment(makeComment({ body: 'why "this" path?' }))
|
||||
expect(out).toContain('User comment: "why \\"this\\" path?"')
|
||||
})
|
||||
|
||||
it('escapes backslashes before quotes so the body cannot break out of the literal', () => {
|
||||
const out = formatDiffComment(makeComment({ body: 'path\\to\\"thing"' }))
|
||||
expect(out).toContain('User comment: "path\\\\to\\\\\\"thing\\""')
|
||||
})
|
||||
|
||||
it('escapes newlines so the body cannot break out of the fixed 3-line structure', () => {
|
||||
const out = formatDiffComment(makeComment({ body: 'first\nsecond' }))
|
||||
expect(out).toContain('User comment: "first\\nsecond"')
|
||||
expect(out.split('\n')).toHaveLength(3)
|
||||
})
|
||||
})
|
||||
|
||||
describe('formatDiffComments', () => {
|
||||
it('joins multiple comments with a blank line', () => {
|
||||
const out = formatDiffComments([
|
||||
makeComment({ id: 'a', lineNumber: 1, body: 'first' }),
|
||||
makeComment({ id: 'b', lineNumber: 2, body: 'second' })
|
||||
])
|
||||
expect(out).toBe(
|
||||
[
|
||||
'File: src/app.ts',
|
||||
'Line: 1',
|
||||
'User comment: "first"',
|
||||
'',
|
||||
'File: src/app.ts',
|
||||
'Line: 2',
|
||||
'User comment: "second"'
|
||||
].join('\n')
|
||||
)
|
||||
})
|
||||
|
||||
it('returns an empty string for an empty input', () => {
|
||||
expect(formatDiffComments([])).toBe('')
|
||||
})
|
||||
})
|
||||
18
src/renderer/src/lib/diff-comments-format.ts
Normal file
18
src/renderer/src/lib/diff-comments-format.ts
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
import type { DiffComment } from '../../../shared/types'
|
||||
|
||||
// Why: the pasted format is the contract between this feature and whatever
|
||||
// agent consumes it. Keep it stable and deterministic — quote escaping matters
|
||||
// because the body is surfaced inside literal quotes. Escape backslashes
|
||||
// first so that `\"` in user input does not decay into an unescaped quote.
|
||||
export function formatDiffComment(c: DiffComment): string {
|
||||
const escaped = c.body
|
||||
.replace(/\\/g, '\\\\')
|
||||
.replace(/"/g, '\\"')
|
||||
.replace(/\r/g, '\\r')
|
||||
.replace(/\n/g, '\\n')
|
||||
return [`File: ${c.filePath}`, `Line: ${c.lineNumber}`, `User comment: "${escaped}"`].join('\n')
|
||||
}
|
||||
|
||||
export function formatDiffComments(comments: DiffComment[]): string {
|
||||
return comments.map(formatDiffComment).join('\n\n')
|
||||
}
|
||||
|
|
@ -14,6 +14,7 @@ import { createCodexUsageSlice } from './slices/codex-usage'
|
|||
import { createBrowserSlice } from './slices/browser'
|
||||
import { createRateLimitSlice } from './slices/rate-limits'
|
||||
import { createSshSlice } from './slices/ssh'
|
||||
import { createDiffCommentsSlice } from './slices/diffComments'
|
||||
import { e2eConfig } from '@/lib/e2e-config'
|
||||
|
||||
export const useAppStore = create<AppState>()((...a) => ({
|
||||
|
|
@ -30,7 +31,8 @@ export const useAppStore = create<AppState>()((...a) => ({
|
|||
...createCodexUsageSlice(...a),
|
||||
...createBrowserSlice(...a),
|
||||
...createRateLimitSlice(...a),
|
||||
...createSshSlice(...a)
|
||||
...createSshSlice(...a),
|
||||
...createDiffCommentsSlice(...a)
|
||||
}))
|
||||
|
||||
export type { AppState } from './types'
|
||||
|
|
|
|||
226
src/renderer/src/store/slices/diffComments.ts
Normal file
226
src/renderer/src/store/slices/diffComments.ts
Normal file
|
|
@ -0,0 +1,226 @@
|
|||
import type { StateCreator } from 'zustand'
|
||||
import type { AppState } from '../types'
|
||||
import type { DiffComment, Worktree } from '../../../../shared/types'
|
||||
import { findWorktreeById, getRepoIdFromWorktreeId } from './worktree-helpers'
|
||||
|
||||
export type DiffCommentsSlice = {
|
||||
getDiffComments: (worktreeId: string | null | undefined) => DiffComment[]
|
||||
addDiffComment: (input: Omit<DiffComment, 'id' | 'createdAt'>) => Promise<DiffComment | null>
|
||||
deleteDiffComment: (worktreeId: string, commentId: string) => Promise<void>
|
||||
}
|
||||
|
||||
function generateId(): string {
|
||||
return globalThis.crypto.randomUUID()
|
||||
}
|
||||
|
||||
// Why: return a stable reference when no comments exist so selectors don't
|
||||
// produce a fresh `[]` on every store update. A new array identity would
|
||||
// trigger re-renders in any consumer using referential equality.
|
||||
// Frozen + typed `readonly` so an accidental `list.push(...)` on the returned
|
||||
// value is both a runtime TypeError and a TypeScript compile error, preventing
|
||||
// the sentinel from being corrupted globally.
|
||||
const EMPTY_COMMENTS: readonly DiffComment[] = Object.freeze([])
|
||||
|
||||
async function persist(worktreeId: string, diffComments: DiffComment[]): Promise<void> {
|
||||
await window.api.worktrees.updateMeta({
|
||||
worktreeId,
|
||||
updates: { diffComments }
|
||||
})
|
||||
}
|
||||
|
||||
// Why: IPC writes from `persist` are not ordered with respect to each other.
|
||||
// If two mutations (e.g. rapid add then delete, or two adds) are in flight
|
||||
// concurrently, their `updateMeta` resolutions can arrive out of call order,
|
||||
// letting an older snapshot overwrite a newer one on disk. We serialize per
|
||||
// worktree so only one write runs at a time. We also defer reading the
|
||||
// snapshot until the queued work actually starts — at dequeue time we pull
|
||||
// the LATEST `diffComments` from the store — which collapses a burst of N
|
||||
// mutations into at most 2 in-flight writes per worktree (1 running + 1
|
||||
// queued) and guarantees the last disk write reflects the newest state.
|
||||
const persistQueueByWorktree: Map<string, Promise<void>> = new Map()
|
||||
|
||||
// Why: chain each new write onto the prior promise for this worktree so
|
||||
// writes land in call order. We use `.then(..., ..)` with both handlers so a
|
||||
// failing previous write doesn't break the chain — we still proceed with the
|
||||
// next write. The queued work reads the latest list from the store via
|
||||
// `get()` at dequeue time (not via a captured parameter) so it writes the
|
||||
// most recent snapshot rather than a stale one from when it was enqueued.
|
||||
// The returned promise resolves/rejects when THIS specific write commits so
|
||||
// callers can preserve their optimistic-update + rollback flow.
|
||||
function enqueuePersist(worktreeId: string, get: () => AppState): Promise<void> {
|
||||
const prior = persistQueueByWorktree.get(worktreeId) ?? Promise.resolve()
|
||||
const run = async (): Promise<void> => {
|
||||
const repoId = getRepoIdFromWorktreeId(worktreeId)
|
||||
const repoList = get().worktreesByRepo[repoId]
|
||||
const target = repoList?.find((w) => w.id === worktreeId)
|
||||
const latest = target?.diffComments ?? []
|
||||
await persist(worktreeId, latest)
|
||||
}
|
||||
const next = prior.then(run, run)
|
||||
persistQueueByWorktree.set(worktreeId, next)
|
||||
// Why: once this write settles, clear the queue entry only if no later
|
||||
// write has been chained on top. Otherwise the map should keep pointing at
|
||||
// the latest tail so subsequent enqueues chain onto the real in-flight
|
||||
// tail, not a stale resolved promise. Use `then(cleanup, cleanup)` (not
|
||||
// `finally`) so a rejection on `next` is fully consumed by this branch —
|
||||
// otherwise the `.finally()` chain propagates the rejection as an
|
||||
// unhandledRejection even though the caller `await`s `next` in its own
|
||||
// try/catch.
|
||||
const cleanup = (): void => {
|
||||
if (persistQueueByWorktree.get(worktreeId) === next) {
|
||||
persistQueueByWorktree.delete(worktreeId)
|
||||
}
|
||||
}
|
||||
next.then(cleanup, cleanup)
|
||||
return next
|
||||
}
|
||||
|
||||
// Why: derive the next comment list from the latest store snapshot inside
|
||||
// the `set` updater so two concurrent writes (rapid add+delete, or a
|
||||
// delete-while-add-in-flight) can't clobber each other via a stale closure.
|
||||
function mutateComments(
|
||||
set: Parameters<StateCreator<AppState, [], [], DiffCommentsSlice>>[0],
|
||||
worktreeId: string,
|
||||
mutate: (existing: DiffComment[]) => DiffComment[] | null
|
||||
): { previous: DiffComment[] | undefined; next: DiffComment[] } | null {
|
||||
const repoId = getRepoIdFromWorktreeId(worktreeId)
|
||||
let previous: DiffComment[] | undefined
|
||||
let next: DiffComment[] | null = null
|
||||
set((s) => {
|
||||
const repoList = s.worktreesByRepo[repoId]
|
||||
if (!repoList) {
|
||||
return {}
|
||||
}
|
||||
const target = repoList.find((w) => w.id === worktreeId)
|
||||
if (!target) {
|
||||
return {}
|
||||
}
|
||||
previous = target.diffComments
|
||||
const computed = mutate(previous ?? [])
|
||||
if (computed === null) {
|
||||
return {}
|
||||
}
|
||||
next = computed
|
||||
const nextList: Worktree[] = repoList.map((w) =>
|
||||
w.id === worktreeId ? { ...w, diffComments: computed } : w
|
||||
)
|
||||
return { worktreesByRepo: { ...s.worktreesByRepo, [repoId]: nextList } }
|
||||
})
|
||||
if (next === null) {
|
||||
return null
|
||||
}
|
||||
return { previous, next }
|
||||
}
|
||||
|
||||
// Why: if the IPC write fails, the optimistic renderer state drifts from
|
||||
// disk. Roll back so what the user sees always matches what will survive a
|
||||
// reload.
|
||||
//
|
||||
// Identity guard: we only revert when the current diffComments array is
|
||||
// strictly identical (===) to the `next` array this mutation produced. If
|
||||
// another mutation has already landed (e.g. Add B succeeded while Add A was
|
||||
// still in flight), it will have replaced the array with a different
|
||||
// identity. In that case we must leave the newer state alone — rolling back
|
||||
// to our stale `previous` would erase B along with the failed A.
|
||||
function rollback(
|
||||
set: Parameters<StateCreator<AppState, [], [], DiffCommentsSlice>>[0],
|
||||
worktreeId: string,
|
||||
previous: DiffComment[] | undefined,
|
||||
expectedCurrent: DiffComment[]
|
||||
): void {
|
||||
const repoId = getRepoIdFromWorktreeId(worktreeId)
|
||||
set((s) => {
|
||||
const repoList = s.worktreesByRepo[repoId]
|
||||
if (!repoList) {
|
||||
return {}
|
||||
}
|
||||
const target = repoList.find((w) => w.id === worktreeId)
|
||||
// Why: if the worktree was removed between the optimistic mutation and
|
||||
// this rollback, there is nothing to restore. Bail out before remapping
|
||||
// `repoList` so we don't allocate a new outer-array identity and trigger
|
||||
// spurious subscriber notifications.
|
||||
if (!target) {
|
||||
return {}
|
||||
}
|
||||
// Why: only roll back if no other mutation landed since this one. If a
|
||||
// later write already replaced the comments array with a different
|
||||
// identity, our stale `previous` would erase that newer state.
|
||||
if (target.diffComments !== expectedCurrent) {
|
||||
return {}
|
||||
}
|
||||
const nextList: Worktree[] = repoList.map((w) =>
|
||||
w.id === worktreeId ? { ...w, diffComments: previous } : w
|
||||
)
|
||||
return { worktreesByRepo: { ...s.worktreesByRepo, [repoId]: nextList } }
|
||||
})
|
||||
}
|
||||
|
||||
export const createDiffCommentsSlice: StateCreator<AppState, [], [], DiffCommentsSlice> = (
|
||||
set,
|
||||
get
|
||||
) => ({
|
||||
getDiffComments: (worktreeId) => {
|
||||
// Why: accept null/undefined so callers with an optional active worktree
|
||||
// can pass it through without allocating a fresh `[]` fallback each
|
||||
// render, which would defeat the `EMPTY_COMMENTS` sentinel's referential
|
||||
// stability and trigger spurious re-renders in useAppStore selectors.
|
||||
if (!worktreeId) {
|
||||
return EMPTY_COMMENTS as DiffComment[]
|
||||
}
|
||||
const worktree = findWorktreeById(get().worktreesByRepo, worktreeId)
|
||||
if (!worktree?.diffComments) {
|
||||
// Why: cast the frozen sentinel to the mutable `DiffComment[]` return
|
||||
// type. The array is frozen at runtime so accidental mutation throws;
|
||||
// the cast only hides the `readonly` marker from consumers that never
|
||||
// mutate the list in practice.
|
||||
return EMPTY_COMMENTS as DiffComment[]
|
||||
}
|
||||
return worktree.diffComments
|
||||
},
|
||||
|
||||
addDiffComment: async (input) => {
|
||||
const comment: DiffComment = {
|
||||
...input,
|
||||
id: generateId(),
|
||||
createdAt: Date.now()
|
||||
}
|
||||
const result = mutateComments(set, input.worktreeId, (existing) => [...existing, comment])
|
||||
if (!result) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
// Why: enqueue through the per-worktree queue so concurrent mutations
|
||||
// cannot land on disk out of call order. The queued write reads the
|
||||
// latest store snapshot at dequeue time, so it will reflect any newer
|
||||
// mutation that landed after this one was enqueued.
|
||||
await enqueuePersist(input.worktreeId, get)
|
||||
return comment
|
||||
} catch (err) {
|
||||
console.error('Failed to persist diff comments:', err)
|
||||
// Why: rollback's identity guard will no-op if a later mutation has
|
||||
// already replaced the in-memory list, so losing a successful newer
|
||||
// write is not possible here even though we queued in order.
|
||||
rollback(set, input.worktreeId, result.previous, result.next)
|
||||
return null
|
||||
}
|
||||
},
|
||||
|
||||
deleteDiffComment: async (worktreeId, commentId) => {
|
||||
const result = mutateComments(set, worktreeId, (existing) => {
|
||||
const next = existing.filter((c) => c.id !== commentId)
|
||||
return next.length === existing.length ? null : next
|
||||
})
|
||||
if (!result) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
// Why: enqueue through the per-worktree queue so concurrent mutations
|
||||
// cannot land on disk out of call order. See enqueuePersist for the
|
||||
// ordering invariant.
|
||||
await enqueuePersist(worktreeId, get)
|
||||
} catch (err) {
|
||||
console.error('Failed to persist diff comments:', err)
|
||||
rollback(set, worktreeId, result.previous, result.next)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
|
@ -98,6 +98,7 @@ import { createCodexUsageSlice } from './codex-usage'
|
|||
import { createBrowserSlice } from './browser'
|
||||
import { createRateLimitSlice } from './rate-limits'
|
||||
import { createSshSlice } from './ssh'
|
||||
import { createDiffCommentsSlice } from './diffComments'
|
||||
|
||||
function createTestStore() {
|
||||
return create<AppState>()((...a) => ({
|
||||
|
|
@ -114,7 +115,8 @@ function createTestStore() {
|
|||
...createCodexUsageSlice(...a),
|
||||
...createBrowserSlice(...a),
|
||||
...createRateLimitSlice(...a),
|
||||
...createSshSlice(...a)
|
||||
...createSshSlice(...a),
|
||||
...createDiffCommentsSlice(...a)
|
||||
}))
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import { createCodexUsageSlice } from './codex-usage'
|
|||
import { createBrowserSlice } from './browser'
|
||||
import { createRateLimitSlice } from './rate-limits'
|
||||
import { createSshSlice } from './ssh'
|
||||
import { createDiffCommentsSlice } from './diffComments'
|
||||
|
||||
export const TEST_REPO = {
|
||||
id: 'repo1',
|
||||
|
|
@ -46,7 +47,8 @@ export function createTestStore() {
|
|||
...createCodexUsageSlice(...a),
|
||||
...createBrowserSlice(...a),
|
||||
...createRateLimitSlice(...a),
|
||||
...createSshSlice(...a)
|
||||
...createSshSlice(...a),
|
||||
...createDiffCommentsSlice(...a)
|
||||
}))
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -93,6 +93,7 @@ import { createCodexUsageSlice } from './codex-usage'
|
|||
import { createBrowserSlice } from './browser'
|
||||
import { createRateLimitSlice } from './rate-limits'
|
||||
import { createSshSlice } from './ssh'
|
||||
import { createDiffCommentsSlice } from './diffComments'
|
||||
|
||||
const WT = 'repo1::/tmp/feature'
|
||||
|
||||
|
|
@ -111,7 +112,8 @@ function createTestStore() {
|
|||
...createCodexUsageSlice(...a),
|
||||
...createBrowserSlice(...a),
|
||||
...createRateLimitSlice(...a),
|
||||
...createSshSlice(...a)
|
||||
...createSshSlice(...a),
|
||||
...createDiffCommentsSlice(...a)
|
||||
}))
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import type { CodexUsageSlice } from './slices/codex-usage'
|
|||
import type { BrowserSlice } from './slices/browser'
|
||||
import type { RateLimitSlice } from './slices/rate-limits'
|
||||
import type { SshSlice } from './slices/ssh'
|
||||
import type { DiffCommentsSlice } from './slices/diffComments'
|
||||
|
||||
export type AppState = RepoSlice &
|
||||
WorktreeSlice &
|
||||
|
|
@ -26,4 +27,5 @@ export type AppState = RepoSlice &
|
|||
CodexUsageSlice &
|
||||
BrowserSlice &
|
||||
RateLimitSlice &
|
||||
SshSlice
|
||||
SshSlice &
|
||||
DiffCommentsSlice
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ export type Worktree = {
|
|||
isPinned: boolean
|
||||
sortOrder: number
|
||||
lastActivityAt: number
|
||||
diffComments?: DiffComment[]
|
||||
} & GitWorktreeInfo
|
||||
|
||||
// ─── Worktree metadata (persisted user-authored fields only) ─────────
|
||||
|
|
@ -58,6 +59,23 @@ export type WorktreeMeta = {
|
|||
isPinned: boolean
|
||||
sortOrder: number
|
||||
lastActivityAt: number
|
||||
diffComments?: DiffComment[]
|
||||
}
|
||||
|
||||
// ─── Diff line comments ──────────────────────────────────────────────
|
||||
// Why: users leave review notes on specific lines of the modified side of
|
||||
// a diff so they can be handed back to an AI agent (pasted into a terminal
|
||||
// or used to bootstrap a new agent session). Stored on WorktreeMeta so the
|
||||
// existing persistence layer writes them to orca-data.json automatically.
|
||||
export type DiffComment = {
|
||||
id: string
|
||||
worktreeId: string
|
||||
filePath: string
|
||||
lineNumber: number
|
||||
body: string
|
||||
createdAt: number
|
||||
// Reserved for future "comments on the original side" — always 'modified' in v1.
|
||||
side: 'modified'
|
||||
}
|
||||
|
||||
// ─── Tab Group Layout ───────────────────────────────────────────────
|
||||
|
|
|
|||
Loading…
Reference in a new issue