mirror of
https://github.com/stablyai/orca
synced 2026-04-21 14:17:16 +00:00
refactor(diff-comments): simplify to copy-only flow in Source Control
Collapse the diff-comments surface to the minimum that's actually needed: drop the Diff Comments tab, the Start-agent chooser, the Clear-all confirmation, and the per-diff banner. The Source Control sidebar now exposes a Comments section with an expand toggle and a single Copy-all button (tooltip: "Copy all comments"); each row in the inline list also gets a hover Copy button so individual comments can be grabbed without copying the whole set. Also strip the always-identical "This comment was left on the modified branch." trailer from the formatted output, unused clearDiffComments action, and openDiffCommentsTab.
This commit is contained in:
brennanb2025
parent
3549abd74a
commit
25652502dc
13 changed files with 512 additions and 506 deletions
|
|
@ -896,6 +896,7 @@
|
|||
position: absolute;
|
||||
left: 4px;
|
||||
width: 18px;
|
||||
height: 18px;
|
||||
display: none;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
|
|
@ -926,10 +927,17 @@
|
|||
|
||||
.orca-diff-comment-inline {
|
||||
width: 100%;
|
||||
padding: 4px 8px 6px;
|
||||
/* 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;
|
||||
|
|
@ -953,8 +961,10 @@
|
|||
}
|
||||
|
||||
.orca-diff-comment-delete {
|
||||
font-size: 11px;
|
||||
padding: 2px 6px;
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
padding: 3px;
|
||||
border-radius: 4px;
|
||||
border: 1px solid transparent;
|
||||
background: transparent;
|
||||
|
|
@ -977,12 +987,15 @@
|
|||
}
|
||||
|
||||
/* Popover for entering a new comment. Positioned absolutely within the
|
||||
section container so it tracks the clicked line via `top` style. */
|
||||
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;
|
||||
width: 320px;
|
||||
padding: 8px;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 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>
|
||||
)
|
||||
}
|
||||
|
|
@ -1,114 +0,0 @@
|
|||
import { useState } from 'react'
|
||||
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { useAppStore } from '@/store'
|
||||
import { buildAgentStartupPlan } from '@/lib/tui-agent-startup'
|
||||
import { formatDiffComments } from '@/lib/diff-comments-format'
|
||||
import { AGENT_CATALOG } from '@/lib/agent-catalog'
|
||||
import { ensureAgentStartupInTerminal, CLIENT_PLATFORM } from '@/lib/new-workspace'
|
||||
import type { DiffComment, TuiAgent } from '../../../../shared/types'
|
||||
|
||||
// Why: we deliberately keep the chooser minimal — a list sourced from
|
||||
// AGENT_CATALOG (same catalog the New Workspace composer uses) plus a Start
|
||||
// button. Funneling through buildAgentStartupPlan keeps argv quoting and the
|
||||
// stdin-after-start follow-up flow identical to the composer path, so agents
|
||||
// like aider/goose still receive the prompt after their TUI boots.
|
||||
|
||||
export function DiffCommentsAgentChooserDialog({
|
||||
open,
|
||||
onOpenChange,
|
||||
worktreeId,
|
||||
comments
|
||||
}: {
|
||||
open: boolean
|
||||
onOpenChange: (open: boolean) => void
|
||||
worktreeId: string
|
||||
comments: DiffComment[]
|
||||
}): React.JSX.Element | null {
|
||||
const settings = useAppStore((s) => s.settings)
|
||||
const [selected, setSelected] = useState<TuiAgent>('claude')
|
||||
const [busy, setBusy] = useState(false)
|
||||
const [error, setError] = useState<string | null>(null)
|
||||
|
||||
const handleStart = async (): Promise<void> => {
|
||||
if (comments.length === 0) {
|
||||
return
|
||||
}
|
||||
setBusy(true)
|
||||
setError(null)
|
||||
try {
|
||||
const prompt = formatDiffComments(comments)
|
||||
const plan = buildAgentStartupPlan({
|
||||
agent: selected,
|
||||
prompt,
|
||||
cmdOverrides: settings?.agentCmdOverrides ?? {},
|
||||
platform: CLIENT_PLATFORM
|
||||
})
|
||||
if (!plan) {
|
||||
setError('Could not build startup plan.')
|
||||
return
|
||||
}
|
||||
const store = useAppStore.getState()
|
||||
const newTab = store.createTab(worktreeId)
|
||||
store.setActiveTab(newTab.id)
|
||||
store.setActiveTabType('terminal')
|
||||
store.queueTabStartupCommand(newTab.id, { command: plan.launchCommand })
|
||||
if (plan.followupPrompt) {
|
||||
// Why: stdin-after-start agents (aider, goose, amp) don't accept a
|
||||
// prompt via CLI flag. Polling for the expected process and typing the
|
||||
// prompt into the live TUI mirrors what the composer does for them.
|
||||
void ensureAgentStartupInTerminal({ worktreeId, startup: plan })
|
||||
}
|
||||
onOpenChange(false)
|
||||
} catch (err) {
|
||||
setError(err instanceof Error ? err.message : 'Failed to start agent.')
|
||||
} finally {
|
||||
setBusy(false)
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={onOpenChange}>
|
||||
<DialogContent className="max-w-sm">
|
||||
<DialogHeader>
|
||||
<DialogTitle>Start agent with comments</DialogTitle>
|
||||
</DialogHeader>
|
||||
<div className="text-xs text-muted-foreground">
|
||||
Opens a new terminal tab and launches the agent preloaded with
|
||||
{comments.length === 1 ? ' 1 comment' : ` ${comments.length} comments`}.
|
||||
</div>
|
||||
<div className="max-h-64 overflow-y-auto rounded-md border border-border">
|
||||
{AGENT_CATALOG.map((entry) => (
|
||||
<label
|
||||
key={entry.id}
|
||||
className={`flex cursor-pointer items-center gap-2 border-b border-border/60 px-3 py-2 text-sm last:border-b-0 hover:bg-accent ${
|
||||
selected === entry.id ? 'bg-accent' : ''
|
||||
}`}
|
||||
>
|
||||
<input
|
||||
type="radio"
|
||||
name="diff-comments-agent"
|
||||
value={entry.id}
|
||||
checked={selected === entry.id}
|
||||
onChange={() => setSelected(entry.id)}
|
||||
/>
|
||||
<span className="font-medium">{entry.label}</span>
|
||||
<span className="ml-auto font-mono text-[10px] text-muted-foreground">
|
||||
{entry.cmd}
|
||||
</span>
|
||||
</label>
|
||||
))}
|
||||
</div>
|
||||
{error && <div className="text-xs text-destructive">{error}</div>}
|
||||
<div className="flex justify-end gap-2">
|
||||
<Button variant="ghost" onClick={() => onOpenChange(false)} disabled={busy}>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button onClick={() => void handleStart()} disabled={busy || comments.length === 0}>
|
||||
Start
|
||||
</Button>
|
||||
</div>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
)
|
||||
}
|
||||
|
|
@ -1,211 +0,0 @@
|
|||
import { useMemo, useState } from 'react'
|
||||
import { Trash2, MessageSquare, FileCode, Play, Clipboard } from 'lucide-react'
|
||||
import { useAppStore } from '@/store'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog'
|
||||
import type { OpenFile } from '@/store/slices/editor'
|
||||
import type { DiffComment } from '../../../../shared/types'
|
||||
import { formatDiffComments } from '@/lib/diff-comments-format'
|
||||
import { DiffCommentsAgentChooserDialog } from './DiffCommentsAgentChooserDialog'
|
||||
|
||||
function formatTimestamp(ts: number): string {
|
||||
const diff = Date.now() - ts
|
||||
const minutes = Math.floor(diff / 60_000)
|
||||
if (minutes < 1) {
|
||||
return 'just now'
|
||||
}
|
||||
if (minutes < 60) {
|
||||
return `${minutes}m ago`
|
||||
}
|
||||
const hours = Math.floor(minutes / 60)
|
||||
if (hours < 24) {
|
||||
return `${hours}h ago`
|
||||
}
|
||||
const days = Math.floor(hours / 24)
|
||||
if (days < 7) {
|
||||
return `${days}d ago`
|
||||
}
|
||||
return new Date(ts).toLocaleDateString()
|
||||
}
|
||||
|
||||
function groupByFile(comments: DiffComment[]): Record<string, DiffComment[]> {
|
||||
const groups: Record<string, DiffComment[]> = {}
|
||||
for (const c of comments) {
|
||||
if (!groups[c.filePath]) {
|
||||
groups[c.filePath] = []
|
||||
}
|
||||
groups[c.filePath].push(c)
|
||||
}
|
||||
for (const list of Object.values(groups)) {
|
||||
list.sort((a, b) => a.lineNumber - b.lineNumber)
|
||||
}
|
||||
return groups
|
||||
}
|
||||
|
||||
export function DiffCommentsTab({ activeFile }: { activeFile: OpenFile }): React.JSX.Element {
|
||||
const worktreeId = activeFile.worktreeId
|
||||
const comments = useAppStore((s) => s.getDiffComments(worktreeId))
|
||||
const deleteDiffComment = useAppStore((s) => s.deleteDiffComment)
|
||||
const clearDiffComments = useAppStore((s) => s.clearDiffComments)
|
||||
const getActiveTab = useAppStore((s) => s.getActiveTab)
|
||||
const [agentDialogOpen, setAgentDialogOpen] = useState(false)
|
||||
const [clearDialogOpen, setClearDialogOpen] = useState(false)
|
||||
const [pasteNotice, setPasteNotice] = useState<string | null>(null)
|
||||
|
||||
const groups = useMemo(() => groupByFile(comments), [comments])
|
||||
const fileEntries = useMemo(() => Object.entries(groups), [groups])
|
||||
|
||||
const handlePaste = (): void => {
|
||||
if (comments.length === 0) {
|
||||
return
|
||||
}
|
||||
const text = formatDiffComments(comments)
|
||||
// Why: the user's request is "paste" — don't send a trailing carriage return
|
||||
// so the AI CLI user can review the text in the terminal before submitting.
|
||||
const state = useAppStore.getState()
|
||||
const active = getActiveTab(worktreeId)
|
||||
if (!active || active.contentType !== 'terminal') {
|
||||
setPasteNotice('Focus a terminal tab in this worktree before pasting.')
|
||||
return
|
||||
}
|
||||
const ptyId = state.ptyIdsByTabId[active.entityId]?.[0]
|
||||
if (!ptyId) {
|
||||
setPasteNotice('Active terminal has no running PTY.')
|
||||
return
|
||||
}
|
||||
window.api.pty.write(ptyId, text)
|
||||
setPasteNotice(`Pasted ${comments.length} comment${comments.length === 1 ? '' : 's'}.`)
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="flex h-full min-h-0 flex-col bg-background">
|
||||
<div className="flex shrink-0 items-center gap-2 border-b border-border bg-card px-3 py-2">
|
||||
<MessageSquare className="size-4 text-muted-foreground" />
|
||||
<div className="text-sm font-medium">Diff Comments ({comments.length})</div>
|
||||
<div className="ml-auto flex items-center gap-1.5">
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
onClick={handlePaste}
|
||||
disabled={comments.length === 0}
|
||||
title="Paste formatted comments into the active terminal (no newline sent)"
|
||||
>
|
||||
<Clipboard className="size-3.5" />
|
||||
Paste into terminal
|
||||
</Button>
|
||||
<Button
|
||||
size="sm"
|
||||
onClick={() => setAgentDialogOpen(true)}
|
||||
disabled={comments.length === 0}
|
||||
title="Start a new terminal tab with an agent preloaded with these comments"
|
||||
>
|
||||
<Play className="size-3.5" />
|
||||
Start agent with comments
|
||||
</Button>
|
||||
<Button
|
||||
size="sm"
|
||||
variant="ghost"
|
||||
onClick={() => {
|
||||
if (comments.length === 0) {
|
||||
return
|
||||
}
|
||||
setClearDialogOpen(true)
|
||||
}}
|
||||
disabled={comments.length === 0}
|
||||
>
|
||||
<Trash2 className="size-3.5" />
|
||||
Clear all
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{pasteNotice && (
|
||||
<div className="shrink-0 border-b border-border/60 bg-accent/30 px-3 py-1.5 text-xs text-muted-foreground">
|
||||
{pasteNotice}
|
||||
</div>
|
||||
)}
|
||||
|
||||
<div className="flex-1 overflow-y-auto">
|
||||
{fileEntries.length === 0 ? (
|
||||
<div className="flex h-full items-center justify-center p-8 text-sm text-muted-foreground">
|
||||
No comments yet. Hover a line in the diff view and click the + in the gutter.
|
||||
</div>
|
||||
) : (
|
||||
<div className="divide-y divide-border/60">
|
||||
{fileEntries.map(([filePath, list]) => (
|
||||
<section key={filePath} className="px-3 py-2">
|
||||
<header className="mb-1.5 flex items-center gap-2 text-xs font-medium text-muted-foreground">
|
||||
<FileCode className="size-3" />
|
||||
<span className="truncate">{filePath}</span>
|
||||
<span className="tabular-nums">({list.length})</span>
|
||||
</header>
|
||||
<ul className="space-y-1.5">
|
||||
{list.map((c) => (
|
||||
<li
|
||||
key={c.id}
|
||||
className="group flex items-start gap-2 rounded-md border border-border/50 bg-card px-2.5 py-1.5"
|
||||
>
|
||||
<span className="mt-0.5 shrink-0 rounded bg-muted px-1.5 py-0.5 text-[10px] tabular-nums text-muted-foreground">
|
||||
L{c.lineNumber}
|
||||
</span>
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="whitespace-pre-wrap break-words text-xs leading-relaxed">
|
||||
{c.body}
|
||||
</div>
|
||||
<div className="mt-0.5 text-[10px] text-muted-foreground">
|
||||
{formatTimestamp(c.createdAt)}
|
||||
</div>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
className="mt-0.5 rounded p-0.5 text-muted-foreground opacity-0 transition-opacity hover:text-destructive group-hover:opacity-100"
|
||||
onClick={() => void deleteDiffComment(worktreeId, c.id)}
|
||||
title="Delete comment"
|
||||
aria-label={`Delete comment on line ${c.lineNumber}`}
|
||||
>
|
||||
<Trash2 className="size-3.5" />
|
||||
</button>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</section>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<Dialog open={clearDialogOpen} onOpenChange={setClearDialogOpen}>
|
||||
<DialogContent className="max-w-sm">
|
||||
<DialogHeader>
|
||||
<DialogTitle>Clear all diff comments?</DialogTitle>
|
||||
</DialogHeader>
|
||||
<div className="text-sm text-muted-foreground">
|
||||
This will permanently delete {comments.length} comment
|
||||
{comments.length === 1 ? '' : 's'} for this worktree. This cannot be undone.
|
||||
</div>
|
||||
<div className="flex justify-end gap-2">
|
||||
<Button variant="ghost" onClick={() => setClearDialogOpen(false)}>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button
|
||||
variant="destructive"
|
||||
onClick={() => {
|
||||
setClearDialogOpen(false)
|
||||
void clearDiffComments(worktreeId)
|
||||
}}
|
||||
>
|
||||
Delete all
|
||||
</Button>
|
||||
</div>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
|
||||
<DiffCommentsAgentChooserDialog
|
||||
open={agentDialogOpen}
|
||||
onOpenChange={setAgentDialogOpen}
|
||||
worktreeId={worktreeId}
|
||||
comments={comments}
|
||||
/>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
|
@ -1,7 +1,9 @@
|
|||
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
|
||||
|
|
@ -20,6 +22,13 @@ type DecoratorArgs = {
|
|||
onDeleteComment: (commentId: string) => void
|
||||
}
|
||||
|
||||
type ZoneEntry = {
|
||||
zoneId: string
|
||||
domNode: HTMLElement
|
||||
root: Root
|
||||
lastBody: string
|
||||
}
|
||||
|
||||
export function useDiffCommentDecorator({
|
||||
editor,
|
||||
filePath,
|
||||
|
|
@ -29,15 +38,12 @@ export function useDiffCommentDecorator({
|
|||
onDeleteComment
|
||||
}: DecoratorArgs): void {
|
||||
const hoverLineRef = useRef<number | null>(null)
|
||||
const viewZoneIdsRef = useRef<Map<string, string>>(new Map())
|
||||
// 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: cache each comment's rendered DOM node and last-applied body so the
|
||||
// view-zone effect can patch an existing card in place when only the body
|
||||
// changed, rather than removing and re-adding every zone on each render.
|
||||
// Rebuilding all zones caused visible flicker and wasted DOM work whenever
|
||||
// a single comment was added, edited, or deleted.
|
||||
const domNodesByCommentIdRef = useRef<Map<string, HTMLElement>>(new Map())
|
||||
const bodyByCommentIdRef = useRef<Map<string, string>>(new Map())
|
||||
// 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
|
||||
|
|
@ -72,11 +78,35 @@ export function useDiffCommentDecorator({
|
|||
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 top = editor.getTopForLineNumber(lineNumber) - editor.getScrollTop()
|
||||
plus.style.top = `${top}px`
|
||||
plus.style.height = `${getLineHeight()}px`
|
||||
plus.style.display = 'flex'
|
||||
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 => {
|
||||
|
|
@ -93,9 +123,19 @@ export function useDiffCommentDecorator({
|
|||
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) {
|
||||
plus.style.display = 'none'
|
||||
setDisplay('none')
|
||||
return
|
||||
}
|
||||
hoverLineRef.current = ln
|
||||
|
|
@ -106,7 +146,7 @@ export function useDiffCommentDecorator({
|
|||
// 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(() => {
|
||||
plus.style.display = 'none'
|
||||
setDisplay('none')
|
||||
})
|
||||
const onScroll = editor.onDidScrollChange(() => {
|
||||
if (hoverLineRef.current != null) {
|
||||
|
|
@ -124,14 +164,15 @@ export function useDiffCommentDecorator({
|
|||
plus.removeEventListener('click', handleClick)
|
||||
plus.remove()
|
||||
// Why: when the editor is swapped or torn down, its view zones go with
|
||||
// it. Clear the tracking Maps 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.
|
||||
viewZoneIdsRef.current.clear()
|
||||
domNodesByCommentIdRef.current.clear()
|
||||
bodyByCommentIdRef.current.clear()
|
||||
// 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])
|
||||
|
||||
|
|
@ -143,60 +184,46 @@ export function useDiffCommentDecorator({
|
|||
const relevant = comments.filter((c) => c.filePath === filePath && c.worktreeId === worktreeId)
|
||||
const relevantMap = new Map(relevant.map((c) => [c.id, c] as const))
|
||||
|
||||
const currentIds = viewZoneIdsRef.current
|
||||
const domNodesByCommentId = domNodesByCommentIdRef.current
|
||||
const bodyByCommentId = bodyByCommentIdRef.current
|
||||
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, zoneId] of currentIds) {
|
||||
for (const [commentId, entry] of zones) {
|
||||
if (!relevantMap.has(commentId)) {
|
||||
accessor.removeZone(zoneId)
|
||||
currentIds.delete(commentId)
|
||||
domNodesByCommentId.delete(commentId)
|
||||
bodyByCommentId.delete(commentId)
|
||||
accessor.removeZone(entry.zoneId)
|
||||
rootsToUnmount.push(entry.root)
|
||||
zones.delete(commentId)
|
||||
}
|
||||
}
|
||||
|
||||
// Add zones for newly-added comments.
|
||||
for (const c of relevant) {
|
||||
if (currentIds.has(c.id)) {
|
||||
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 card = document.createElement('div')
|
||||
card.className = 'orca-diff-comment-card'
|
||||
|
||||
const header = document.createElement('div')
|
||||
header.className = 'orca-diff-comment-header'
|
||||
const meta = document.createElement('span')
|
||||
meta.className = 'orca-diff-comment-meta'
|
||||
meta.textContent = `Comment · line ${c.lineNumber}`
|
||||
const del = document.createElement('button')
|
||||
del.type = 'button'
|
||||
del.className = 'orca-diff-comment-delete'
|
||||
del.title = 'Delete comment'
|
||||
del.textContent = 'Delete'
|
||||
del.addEventListener('mousedown', (ev) => ev.stopPropagation())
|
||||
del.addEventListener('click', (ev) => {
|
||||
ev.preventDefault()
|
||||
ev.stopPropagation()
|
||||
onDeleteCommentRef.current(c.id)
|
||||
})
|
||||
header.appendChild(meta)
|
||||
header.appendChild(del)
|
||||
|
||||
const body = document.createElement('div')
|
||||
body.className = 'orca-diff-comment-body'
|
||||
body.textContent = c.body
|
||||
|
||||
card.appendChild(header)
|
||||
card.appendChild(body)
|
||||
dom.appendChild(card)
|
||||
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
|
||||
|
|
@ -206,38 +233,50 @@ export function useDiffCommentDecorator({
|
|||
const lineCount = c.body.split('\n').length
|
||||
const heightInPx = Math.max(56, 28 + lineCount * 18)
|
||||
|
||||
const id = accessor.addZone({
|
||||
// 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: true
|
||||
suppressMouseDown: false
|
||||
})
|
||||
currentIds.set(c.id, id)
|
||||
domNodesByCommentId.set(c.id, dom)
|
||||
bodyByCommentId.set(c.id, c.body)
|
||||
zones.set(c.id, { zoneId, domNode: dom, root, lastBody: c.body })
|
||||
}
|
||||
|
||||
// Patch existing zones whose body text changed in place — avoids the
|
||||
// full rebuild that would otherwise flicker the card.
|
||||
// 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) {
|
||||
if (!currentIds.has(c.id)) {
|
||||
const entry = zones.get(c.id)
|
||||
if (!entry) {
|
||||
continue
|
||||
}
|
||||
const previousBody = bodyByCommentId.get(c.id)
|
||||
if (previousBody === c.body) {
|
||||
if (entry.lastBody === c.body) {
|
||||
continue
|
||||
}
|
||||
const dom = domNodesByCommentId.get(c.id)
|
||||
if (!dom) {
|
||||
continue
|
||||
}
|
||||
const bodyEl = dom.querySelector('.orca-diff-comment-body')
|
||||
if (bodyEl) {
|
||||
bodyEl.textContent = c.body
|
||||
}
|
||||
bodyByCommentId.set(c.id, c.body)
|
||||
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
|
||||
|
|
@ -4,7 +4,6 @@ restore-on-remount caching, and scroll preservation. Splitting those pieces
|
|||
across smaller files would make the lifecycle edges harder to reason about and
|
||||
more error-prone than keeping the whole viewer flow together. */
|
||||
import React, { useState, useEffect, useCallback, useRef, useLayoutEffect } from 'react'
|
||||
import { MessageSquare } from 'lucide-react'
|
||||
import type { editor as monacoEditor } from 'monaco-editor'
|
||||
import { useAppStore } from '@/store'
|
||||
import { joinPath } from '@/lib/path'
|
||||
|
|
@ -57,8 +56,6 @@ export default function CombinedDiffViewer({
|
|||
const openAllDiffs = useAppStore((s) => s.openAllDiffs)
|
||||
const openConflictReview = useAppStore((s) => s.openConflictReview)
|
||||
const openBranchAllDiffs = useAppStore((s) => s.openBranchAllDiffs)
|
||||
const openDiffCommentsTab = useAppStore((s) => s.openDiffCommentsTab)
|
||||
const diffCommentCount = useAppStore((s) => s.getDiffComments(file.worktreeId).length)
|
||||
const isDark =
|
||||
settings?.theme === 'dark' ||
|
||||
(settings?.theme === 'system' && window.matchMedia('(prefers-color-scheme: dark)').matches)
|
||||
|
|
@ -496,15 +493,6 @@ export default function CombinedDiffViewer({
|
|||
{isBranchMode && branchCompare ? ` vs ${branchCompare.baseRef}` : ''}
|
||||
</span>
|
||||
<div className="flex items-center gap-2">
|
||||
<button
|
||||
type="button"
|
||||
className="flex items-center gap-1 text-xs text-muted-foreground hover:text-foreground transition-colors"
|
||||
onClick={() => openDiffCommentsTab(file.worktreeId, file.filePath)}
|
||||
title="Open diff comments tab"
|
||||
>
|
||||
<MessageSquare className="size-3" />
|
||||
Comments ({diffCommentCount})
|
||||
</button>
|
||||
{file.combinedAlternate && (
|
||||
<button
|
||||
className="text-xs text-muted-foreground hover:text-foreground transition-colors"
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -13,9 +13,6 @@ import { RichMarkdownErrorBoundary } from './RichMarkdownErrorBoundary'
|
|||
const MonacoEditor = lazy(() => import('./MonacoEditor'))
|
||||
const DiffViewer = lazy(() => import('./DiffViewer'))
|
||||
const CombinedDiffViewer = lazy(() => import('./CombinedDiffViewer'))
|
||||
const DiffCommentsTab = lazy(() =>
|
||||
import('../diff-comments/DiffCommentsTab').then((m) => ({ default: m.DiffCommentsTab }))
|
||||
)
|
||||
const RichMarkdownEditor = lazy(() => import('./RichMarkdownEditor'))
|
||||
const MarkdownPreview = lazy(() => import('./MarkdownPreview'))
|
||||
const ImageViewer = lazy(() => import('./ImageViewer'))
|
||||
|
|
@ -226,10 +223,6 @@ export function EditorContent({
|
|||
return <div className="h-full min-h-0">{renderMonacoEditor(fc)}</div>
|
||||
}
|
||||
|
||||
if (activeFile.mode === 'diff-comments') {
|
||||
return <DiffCommentsTab activeFile={activeFile} />
|
||||
}
|
||||
|
||||
if (activeFile.mode === 'conflict-review') {
|
||||
return (
|
||||
<ConflictReviewPanel
|
||||
|
|
@ -362,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,47 @@ 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)
|
||||
const diffCommentsForActive = useAppStore((s) =>
|
||||
activeWorktreeId ? 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 +692,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 +904,7 @@ function SourceControlInner(): React.JSX.Element {
|
|||
onStage={handleStage}
|
||||
onUnstage={handleUnstage}
|
||||
onDiscard={handleDiscard}
|
||||
commentCount={diffCommentCountByPath.get(entry.path) ?? 0}
|
||||
/>
|
||||
)
|
||||
})}
|
||||
|
|
@ -849,6 +958,7 @@ function SourceControlInner(): React.JSX.Element {
|
|||
worktreePath={worktreePath}
|
||||
onRevealInExplorer={revealInExplorer}
|
||||
onOpen={() => openCommittedDiff(entry)}
|
||||
commentCount={diffCommentCountByPath.get(entry.path) ?? 0}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
|
|
@ -1052,6 +1162,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">
|
||||
No comments yet. Hover a line in the diff view and click the + in the gutter.
|
||||
</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 +1355,8 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({
|
|||
onOpen,
|
||||
onStage,
|
||||
onUnstage,
|
||||
onDiscard
|
||||
onDiscard,
|
||||
commentCount
|
||||
}: {
|
||||
entryKey: string
|
||||
entry: GitStatusEntry
|
||||
|
|
@ -1157,6 +1370,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 +1443,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 +1543,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 +1579,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] }}
|
||||
|
|
|
|||
|
|
@ -16,15 +16,10 @@ function makeComment(overrides: Partial<DiffComment> = {}): DiffComment {
|
|||
}
|
||||
|
||||
describe('formatDiffComment', () => {
|
||||
it('emits the fixed four-line structure', () => {
|
||||
it('emits the fixed three-line structure', () => {
|
||||
const out = formatDiffComment(makeComment())
|
||||
expect(out).toBe(
|
||||
[
|
||||
'File: src/app.ts',
|
||||
'Line: 10',
|
||||
'User comment: "Needs validation"',
|
||||
'Comment metadata: This comment was left on the modified branch.'
|
||||
].join('\n')
|
||||
['File: src/app.ts', 'Line: 10', 'User comment: "Needs validation"'].join('\n')
|
||||
)
|
||||
})
|
||||
|
||||
|
|
@ -38,10 +33,10 @@ describe('formatDiffComment', () => {
|
|||
expect(out).toContain('User comment: "path\\\\to\\\\\\"thing\\""')
|
||||
})
|
||||
|
||||
it('escapes newlines so the body cannot break out of the fixed 4-line structure', () => {
|
||||
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(4)
|
||||
expect(out.split('\n')).toHaveLength(3)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -56,12 +51,10 @@ describe('formatDiffComments', () => {
|
|||
'File: src/app.ts',
|
||||
'Line: 1',
|
||||
'User comment: "first"',
|
||||
'Comment metadata: This comment was left on the modified branch.',
|
||||
'',
|
||||
'File: src/app.ts',
|
||||
'Line: 2',
|
||||
'User comment: "second"',
|
||||
'Comment metadata: This comment was left on the modified branch.'
|
||||
'User comment: "second"'
|
||||
].join('\n')
|
||||
)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -10,12 +10,7 @@ export function formatDiffComment(c: DiffComment): string {
|
|||
.replace(/"/g, '\\"')
|
||||
.replace(/\r/g, '\\r')
|
||||
.replace(/\n/g, '\\n')
|
||||
return [
|
||||
`File: ${c.filePath}`,
|
||||
`Line: ${c.lineNumber}`,
|
||||
`User comment: "${escaped}"`,
|
||||
`Comment metadata: This comment was left on the modified branch.`
|
||||
].join('\n')
|
||||
return [`File: ${c.filePath}`, `Line: ${c.lineNumber}`, `User comment: "${escaped}"`].join('\n')
|
||||
}
|
||||
|
||||
export function formatDiffComments(comments: DiffComment[]): string {
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ export type DiffCommentsSlice = {
|
|||
getDiffComments: (worktreeId: string) => DiffComment[]
|
||||
addDiffComment: (input: Omit<DiffComment, 'id' | 'createdAt'>) => Promise<DiffComment | null>
|
||||
deleteDiffComment: (worktreeId: string, commentId: string) => Promise<void>
|
||||
clearDiffComments: (worktreeId: string) => Promise<void>
|
||||
}
|
||||
|
||||
function generateId(): string {
|
||||
|
|
@ -159,20 +158,5 @@ export const createDiffCommentsSlice: StateCreator<AppState, [], [], DiffComment
|
|||
console.error('Failed to persist diff comments:', err)
|
||||
rollback(set, worktreeId, result.previous, result.next)
|
||||
}
|
||||
},
|
||||
|
||||
clearDiffComments: async (worktreeId) => {
|
||||
const result = mutateComments(set, worktreeId, (existing) =>
|
||||
existing.length === 0 ? null : []
|
||||
)
|
||||
if (!result) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
await persist(worktreeId, result.next)
|
||||
} catch (err) {
|
||||
console.error('Failed to persist diff comments:', err)
|
||||
rollback(set, worktreeId, result.previous, result.next)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
|
|
|||
|
|
@ -224,8 +224,6 @@ export type EditorSlice = {
|
|||
compare: GitBranchCompareSummary,
|
||||
alternate?: CombinedDiffAlternate
|
||||
) => void
|
||||
openDiffCommentsTab: (worktreeId: string, worktreePath: string) => void
|
||||
|
||||
// Cursor line tracking per file
|
||||
editorCursorLine: Record<string, number>
|
||||
setEditorCursorLine: (fileId: string, line: number) => void
|
||||
|
|
@ -1239,44 +1237,6 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
|
|||
void openWorkspaceEditorItem(get(), id, worktreeId, label, 'diff')
|
||||
},
|
||||
|
||||
openDiffCommentsTab: (worktreeId, worktreePath) => {
|
||||
const id = `${worktreeId}::diff-comments`
|
||||
const label = 'Diff Comments'
|
||||
set((s) => {
|
||||
const existing = s.openFiles.find((f) => f.id === id)
|
||||
if (existing) {
|
||||
return {
|
||||
activeFileId: id,
|
||||
activeTabType: 'editor',
|
||||
activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id },
|
||||
activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' }
|
||||
}
|
||||
}
|
||||
// Why: the Diff Comments tab is a virtual editor surface, not a file on
|
||||
// disk. `filePath` holds the worktree root (so code that resolves the
|
||||
// containing worktree still works) and `relativePath` is used only as a
|
||||
// human-readable tab label. Anything that would join these with a
|
||||
// filesystem helper must branch on `mode === 'diff-comments'` first.
|
||||
const newFile: OpenFile = {
|
||||
id,
|
||||
filePath: worktreePath,
|
||||
relativePath: label,
|
||||
worktreeId,
|
||||
language: 'plaintext',
|
||||
isDirty: false,
|
||||
mode: 'diff-comments'
|
||||
}
|
||||
return {
|
||||
openFiles: [...s.openFiles, newFile],
|
||||
activeFileId: id,
|
||||
activeTabType: 'editor',
|
||||
activeFileIdByWorktree: { ...s.activeFileIdByWorktree, [worktreeId]: id },
|
||||
activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' }
|
||||
}
|
||||
})
|
||||
void openWorkspaceEditorItem(get(), id, worktreeId, label, 'editor')
|
||||
},
|
||||
|
||||
openConflictFile: (worktreeId, worktreePath, entry, language) => {
|
||||
const absolutePath = joinPath(worktreePath, entry.path)
|
||||
set((s) => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue